diff --git a/server/src/dtos/person.dto.ts b/server/src/dtos/person.dto.ts index bd15cad412..0379b6f3b0 100644 --- a/server/src/dtos/person.dto.ts +++ b/server/src/dtos/person.dto.ts @@ -215,6 +215,6 @@ export function mapFaces( ): AssetFaceResponseDto { return { ...mapFacesWithoutPerson(face, edits, assetDimensions), - person: face.person?.ownerId === auth.user.id ? mapPerson(face.person) : null, + person: face.person ? mapPerson(face.person) : null, }; } diff --git a/server/src/queries/person.repository.sql b/server/src/queries/person.repository.sql index c929b7f5a3..90067594a8 100644 --- a/server/src/queries/person.repository.sql +++ b/server/src/queries/person.repository.sql @@ -131,16 +131,15 @@ select "person" where "person"."personGroupId" = "asset_face"."personGroupId" - and "person"."ownerId" = "asset"."ownerId" + and "person"."ownerId" = $1 ) as obj ) as "person" from "asset_face" - inner join "asset" on "asset"."id" = "asset_face"."assetId" where - "asset_face"."assetId" = $1 + "asset_face"."assetId" = $2 and "asset_face"."deletedAt" is null - and "asset_face"."isVisible" = $2 + and "asset_face"."isVisible" = $3 order by "asset_face"."boundingBoxX1" asc @@ -158,14 +157,13 @@ select "person" where "person"."personGroupId" = "asset_face"."personGroupId" - and "person"."ownerId" = "asset"."ownerId" + and "person"."ownerId" = $1 ) as obj ) as "person" from "asset_face" - inner join "asset" on "asset"."id" = "asset_face"."assetId" where - "asset_face"."id" = $1 + "asset_face"."id" = $2 and "asset_face"."deletedAt" is null -- PersonRepository.getFaceForFacialRecognitionJob @@ -521,15 +519,14 @@ select "person" where "person"."personGroupId" = "asset_face"."personGroupId" - and "person"."ownerId" = "asset"."ownerId" + and "person"."ownerId" = $1 ) as obj ) as "person" from "asset_face" - inner join "asset" on "asset"."id" = "asset_face"."assetId" where - "asset_face"."assetId" in ($1) - and "asset_face"."personGroupId" in ($2) + "asset_face"."assetId" in ($2) + and "asset_face"."personGroupId" in ($3) and "asset_face"."deletedAt" is null -- PersonRepository.getRandomFace diff --git a/server/src/repositories/person.repository.ts b/server/src/repositories/person.repository.ts index 7842a0c97e..f6fa35e4b0 100644 --- a/server/src/repositories/person.repository.ts +++ b/server/src/repositories/person.repository.ts @@ -63,19 +63,27 @@ export type UnassignFacesOptions = DeleteFacesOptions; export type SelectFaceOptions = (keyof Selectable)[]; +export type GetFacesOptions = WithPersonOptions & { isVisible?: boolean }; + /** a person is identified by its owner and the group it belongs to */ export type PersonId = { ownerId: string; personGroupId: string }; export type ReassignCluster = { userId: string; newClusterId: string }; -const withPerson = (eb: ExpressionBuilder) => { - return jsonObjectFrom( - eb - .selectFrom('person') - .selectAll('person') - .whereRef('person.personGroupId', '=', 'asset_face.personGroupId') - .whereRef('person.ownerId', '=', 'asset.ownerId'), - ).as('person'); +export type WithPersonOptions = { + /** whose version of the person to select */ + viewingUserId: string; +}; + +const withPerson = ({ viewingUserId }: WithPersonOptions) => { + return (eb: ExpressionBuilder) => + jsonObjectFrom( + eb + .selectFrom('person') + .selectAll('person') + .whereRef('person.personGroupId', '=', 'asset_face.personGroupId') + .where('person.ownerId', '=', viewingUserId), + ).as('person'); }; const withFaceSearch = (eb: ExpressionBuilder) => { @@ -286,15 +294,14 @@ export class PersonRepository { .execute(); } - @GenerateSql({ params: [DummyValue.UUID] }) - getFaces(assetId: string, options?: { isVisible?: boolean }) { - const isVisible = options === undefined ? true : options.isVisible; + @GenerateSql({ params: [DummyValue.UUID, { viewingUserId: DummyValue.UUID, isVisible: true }] }) + getFaces(assetId: string, options: GetFacesOptions) { + const { viewingUserId, isVisible } = options; return this.db .selectFrom('asset_face') - .innerJoin('asset', 'asset.id', 'asset_face.assetId') .selectAll('asset_face') - .select(withPerson) + .select(withPerson({ viewingUserId })) .where('asset_face.assetId', '=', assetId) .where('asset_face.deletedAt', 'is', null) .$if(isVisible !== undefined, (qb) => qb.where('asset_face.isVisible', '=', isVisible!)) @@ -302,14 +309,13 @@ export class PersonRepository { .execute(); } - @GenerateSql({ params: [DummyValue.UUID] }) - getFaceById(id: string) { + @GenerateSql({ params: [DummyValue.UUID, { viewingUserId: DummyValue.UUID }] }) + getFaceById(id: string, { viewingUserId }: WithPersonOptions) { // TODO return null instead of find or fail return this.db .selectFrom('asset_face') - .innerJoin('asset', 'asset.id', 'asset_face.assetId') .selectAll('asset_face') - .select(withPerson) + .select(withPerson({ viewingUserId })) .where('asset_face.id', '=', id) .where('asset_face.deletedAt', 'is', null) .executeTakeFirstOrThrow(); @@ -644,9 +650,11 @@ export class PersonRepository { .execute(); } - @GenerateSql({ params: [[{ assetId: DummyValue.UUID, personGroupId: DummyValue.UUID }]] }) + @GenerateSql({ + params: [[{ assetId: DummyValue.UUID, personGroupId: DummyValue.UUID }], { viewingUserId: DummyValue.UUID }], + }) @ChunkedArray() - getFacesByIds(ids: AssetFaceId[]) { + getFacesByIds(ids: AssetFaceId[], { viewingUserId }: WithPersonOptions) { if (ids.length === 0) { return Promise.resolve([]); } @@ -660,9 +668,8 @@ export class PersonRepository { return this.db .selectFrom('asset_face') - .innerJoin('asset', 'asset.id', 'asset_face.assetId') .selectAll('asset_face') - .select(withPerson) + .select(withPerson({ viewingUserId })) .where('asset_face.assetId', 'in', assetIds) .where('asset_face.personGroupId', 'in', personGroupIds) .where('asset_face.deletedAt', 'is', null) diff --git a/server/src/services/media.service.ts b/server/src/services/media.service.ts index 236575b103..ca7f4d431d 100644 --- a/server/src/services/media.service.ts +++ b/server/src/services/media.service.ts @@ -835,7 +835,7 @@ export class MediaService extends BaseService { : undefined; const originalDimensions = getDimensions(asset.exifInfo!); - const assetFaces = await this.personRepository.getFaces(asset.id, {}); + const assetFaces = await this.personRepository.getFaces(asset.id, { viewingUserId: asset.ownerId }); const ocrData = await this.ocrRepository.getByAssetId(asset.id, {}); const faceStatuses = checkFaceVisibility(assetFaces, originalDimensions, cropBox); diff --git a/server/src/services/person.service.spec.ts b/server/src/services/person.service.spec.ts index c8277bc3d5..32168b65ce 100644 --- a/server/src/services/person.service.spec.ts +++ b/server/src/services/person.service.spec.ts @@ -1446,11 +1446,5 @@ describe(PersonService.name, () => { it('should not map person if person is null', () => { expect(mapFaces(getForAssetFace(AssetFaceFactory.create()), AuthFactory.create()).person).toBeNull(); }); - - it('should not map person if person does not match auth user id', () => { - expect( - mapFaces(getForAssetFace(AssetFaceFactory.from().person().build()), AuthFactory.create()).person, - ).toBeNull(); - }); }); }); diff --git a/server/src/services/person.service.ts b/server/src/services/person.service.ts index 7eea30ac83..1486d9a9ce 100644 --- a/server/src/services/person.service.ts +++ b/server/src/services/person.service.ts @@ -87,9 +87,10 @@ export class PersonService extends BaseService { const result: PersonResponseDto[] = []; const changeFeaturePhoto = new Map(); for (const data of dto.data) { - const faces = await this.personRepository.getFacesByIds([ - { personGroupId: data.personId, assetId: data.assetId }, - ]); + const faces = await this.personRepository.getFacesByIds( + [{ personGroupId: data.personId, assetId: data.assetId }], + { viewingUserId: auth.user.id }, + ); for (const face of faces) { await this.requireAccess({ auth, permission: Permission.PersonCreate, ids: [face.id] }); @@ -114,7 +115,7 @@ export class PersonService extends BaseService { async reassignFacesById(auth: AuthDto, personGroupId: string, dto: FaceDto): Promise { await this.requireAccess({ auth, permission: Permission.PersonUpdate, ids: [personGroupId] }); await this.requireAccess({ auth, permission: Permission.PersonCreate, ids: [dto.id] }); - const face = await this.personRepository.getFaceById(dto.id); + const face = await this.personRepository.getFaceById(dto.id, { viewingUserId: auth.user.id }); const person = await this.findOrFail(auth, personGroupId); await this.personRepository.reassignFace(face.id, person.personGroupId); @@ -130,7 +131,7 @@ export class PersonService extends BaseService { async getFacesById(auth: AuthDto, dto: FaceDto): Promise { await this.requireAccess({ auth, permission: Permission.AssetRead, ids: [dto.id] }); - const faces = await this.personRepository.getFaces(dto.id); + const faces = await this.personRepository.getFaces(dto.id, { viewingUserId: auth.user.id, isVisible: true }); const asset = await this.assetRepository.getForFaces(dto.id); const assetDimensions = getDimensions(asset); diff --git a/web/src/lib/components/asset-viewer/DetailPanelPeople.svelte b/web/src/lib/components/asset-viewer/DetailPanelPeople.svelte index 6290209e28..949d0e4b77 100644 --- a/web/src/lib/components/asset-viewer/DetailPanelPeople.svelte +++ b/web/src/lib/components/asset-viewer/DetailPanelPeople.svelte @@ -57,42 +57,44 @@ ); -{#if !authManager.isSharedLink && isOwner} +{#if !authManager.isSharedLink}
{$t('people')}
- {#if people.some((person) => person.isHidden)} + {#if isOwner} + {#if people.some((person) => person.isHidden)} + assetViewerManager.toggleHiddenPeople()} + /> + {/if} assetViewerManager.toggleHiddenPeople()} + onclick={() => assetViewerManager.toggleFaceEditMode()} /> - {/if} - assetViewerManager.toggleFaceEditMode()} - /> - {#if faceManager.data.length > 0} - assetViewerManager.openEditFacesPanel()} - /> + {#if faceManager.data.length > 0} + assetViewerManager.openEditFacesPanel()} + /> + {/if} {/if}