From 0f6e665d99c02ec84d3c86f1b3bb8d173f498af3 Mon Sep 17 00:00:00 2001 From: martabal <74269598+martabal@users.noreply.github.com> Date: Sun, 12 May 2024 17:25:49 +0200 Subject: [PATCH] pr feedback --- e2e/src/api/specs/asset.e2e-spec.ts | 4 +-- mobile/lib/services/asset.service.dart | 2 +- .../openapi/doc/PeopleWithFacesResponseDto.md | Bin 535 -> 543 bytes .../model/people_with_faces_response_dto.dart | Bin 3322 -> 3418 bytes .../people_with_faces_response_dto_test.dart | Bin 756 -> 772 bytes open-api/immich-openapi-specs.json | 12 ++++----- open-api/typescript-sdk/src/fetch-client.ts | 2 +- server/src/dtos/asset-response.dto.ts | 2 +- server/src/dtos/person.dto.ts | 2 +- server/src/services/person.service.spec.ts | 24 ++++++++++++++++++ .../asset-viewer/detail-panel.svelte | 4 +-- .../faces-page/person-side-panel.svelte | 20 ++++++++++++++- web/src/lib/utils/thumbnail-util.ts | 2 +- 13 files changed, 58 insertions(+), 16 deletions(-) diff --git a/e2e/src/api/specs/asset.e2e-spec.ts b/e2e/src/api/specs/asset.e2e-spec.ts index 54a4c0160c..864fe47196 100644 --- a/e2e/src/api/specs/asset.e2e-spec.ts +++ b/e2e/src/api/specs/asset.e2e-spec.ts @@ -214,7 +214,7 @@ describe('/asset', () => { id: user1Assets[0].id, isFavorite: false, people: { - faces: [ + visiblePeople: [ { birthDate: null, id: expect.any(String), @@ -484,7 +484,7 @@ describe('/asset', () => { id: user1Assets[0].id, isFavorite: true, people: { - faces: [ + visiblePeople: [ { birthDate: null, id: expect.any(String), diff --git a/mobile/lib/services/asset.service.dart b/mobile/lib/services/asset.service.dart index a9fd6678f8..f85808dece 100644 --- a/mobile/lib/services/asset.service.dart +++ b/mobile/lib/services/asset.service.dart @@ -72,7 +72,7 @@ class AssetService { final AssetResponseDto? dto = await _apiService.assetApi.getAssetInfo(remoteId); - return dto?.people?.faces; + return dto?.people?.visiblePeople; } catch (error, stack) { log.severe( 'Error while getting remote asset info: ${error.toString()}', diff --git a/mobile/openapi/doc/PeopleWithFacesResponseDto.md b/mobile/openapi/doc/PeopleWithFacesResponseDto.md index acaa413389d8855eaf7436a00b610e411145465e..a8399ec40cf4020da91bd7a89b5404b30b79acd5 100644 GIT binary patch delta 33 pcmbQvGM{C_)5$@M3jA7HWtqj9Nja$jsrdytsS{tNO)g?w3INY74Ez89 delta 23 fcmbQwGM#0@Q`WS^~w6i~RDlV39hP7Y=k7gf*(nx~+kfg(LQ zf!SyBE@l}yG;OvjsLHM}M@<{9 delta 284 zcmca5^-FTYB1W;2jLc%aw8Z4pVg*};sLJ_RqC~*T)nkDUM!0@*B#Y(dqb%)=tYF#6 o^VkeIq3SesCOfnFY<|w>#-<7p(g8a}9mQmIkYKGfS1lJ807rCO?f?J) diff --git a/mobile/openapi/test/people_with_faces_response_dto_test.dart b/mobile/openapi/test/people_with_faces_response_dto_test.dart index 1a260f645674fc3f6e56a55c9312206b45d5a298..ff8c3aea0d939b6467c3cd6b90a9e7f495347d3d 100644 GIT binary patch delta 59 zcmeyu+QPQs6XWDorqs!GOze|CF)H$wWfo^9<)j9r<`?9oPL^P@MiYr(Qk(pWi3 { await expect(sut.unassignFace(authStub.admin, faceStub.face1.id)).resolves.toStrictEqual( mapFaces(faceStub.unassignedFace, authStub.admin), ); + + expect(mediaMock.generateThumbnail).not.toHaveBeenCalled(); + }); + + it('should not unassign a face if user has no create access', async () => { + personMock.getFaceById.mockResolvedValueOnce(faceStub.face1); + accessMock.person.checkOwnerAccess.mockResolvedValue(new Set([personStub.noName.id])); + personMock.reassignFace.mockResolvedValue(1); + personMock.getRandomFace.mockResolvedValue(null); + personMock.getFaceById.mockResolvedValueOnce(faceStub.unassignedFace); + + await expect(sut.unassignFace(authStub.admin, faceStub.face1.id)).rejects.toBeInstanceOf(BadRequestException); }); }); @@ -465,6 +477,18 @@ describe(PersonService.name, () => { sut.unassignFaces(authStub.admin, { data: [{ assetId: faceStub.face1.id, personId: 'person-1' }] }), ).resolves.toStrictEqual([{ id: 'assetFaceId1', success: true }]); }); + + it('should not unassign a face if the user has no create access', async () => { + personMock.getFacesByIds.mockResolvedValueOnce([faceStub.face1]); + accessMock.person.checkOwnerAccess.mockResolvedValue(new Set([personStub.noName.id])); + personMock.reassignFace.mockResolvedValue(1); + personMock.getRandomFace.mockResolvedValue(null); + personMock.getFaceById.mockResolvedValueOnce(faceStub.unassignedFace); + + await expect( + sut.unassignFaces(authStub.admin, { data: [{ assetId: faceStub.face1.id, personId: 'person-1' }] }), + ).rejects.toBeInstanceOf(BadRequestException); + }); }); describe('handlePersonCleanup', () => { diff --git a/web/src/lib/components/asset-viewer/detail-panel.svelte b/web/src/lib/components/asset-viewer/detail-panel.svelte index f850c70c55..ed687e9aa9 100644 --- a/web/src/lib/components/asset-viewer/detail-panel.svelte +++ b/web/src/lib/components/asset-viewer/detail-panel.svelte @@ -218,7 +218,7 @@

PEOPLE

- {#if people.faces.some((person) => person.isHidden)} + {#if people.visiblePeople.some((person) => person.isHidden)}
- {#each people.faces as person (person.id)} + {#each people.visiblePeople as person (person.id)} {#if showingHiddenPeople || !person.isHidden} ; let automaticRefreshTimeout: ReturnType; + $: mapFacesToBeCreated = Object.entries(selectedPersonToAdd) + .filter(([_, value]) => value.person === null) + .map(([key, _]) => key); + const thumbnailWidth = '90px'; const generatePeopleWithoutFaces = async () => { @@ -94,9 +98,23 @@ isShowLoadingPeople = false; } + /* + * we wait for the server to create the feature photo for: + * - people which has been reassigned to a new person + * - faces removed assigned to a new person + * + * if after 15 seconds the server has not generated the feature photos, + * we go back to the detail-panel + */ const onPersonThumbnail = (personId: string) => { assetFaceGenerated.push(personId); - if (isEqual(assetFaceGenerated, peopleToCreate) && loaderLoadingDoneTimeout && automaticRefreshTimeout) { + + if ( + isEqual(assetFaceGenerated, peopleToCreate) && + isEqual(assetFaceGenerated, mapFacesToBeCreated) && + loaderLoadingDoneTimeout && + automaticRefreshTimeout + ) { clearTimeout(loaderLoadingDoneTimeout); clearTimeout(automaticRefreshTimeout); onRefresh(); diff --git a/web/src/lib/utils/thumbnail-util.ts b/web/src/lib/utils/thumbnail-util.ts index e693c5f68b..b1bb6b82c8 100644 --- a/web/src/lib/utils/thumbnail-util.ts +++ b/web/src/lib/utils/thumbnail-util.ts @@ -45,7 +45,7 @@ export function getAltText(asset: AssetResponseDto) { altText += ` in ${asset.exifInfo.city}, ${asset.exifInfo.country}`; } - const names = asset.people?.faces.filter((p) => p.name).map((p) => p.name) ?? []; + const names = asset.people?.visiblePeople.filter((p) => p.name).map((p) => p.name) ?? []; if (names.length == 1) { altText += ` with ${names[0]}`; }