fix: people/face query

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,
   };
 }

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

server/src/repositories/person.repository.ts

@@ -63,19 +63,27 @@ export type UnassignFacesOptions = DeleteFacesOptions;
 
 export type SelectFaceOptions = (keyof Selectable<AssetFaceTable>)[];
 
+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<DB, 'asset_face' | 'asset'>) => {
-  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<DB, 'asset_face'>) =>
+    jsonObjectFrom(
+      eb
+        .selectFrom('person')
+        .selectAll('person')
+        .whereRef('person.personGroupId', '=', 'asset_face.personGroupId')
+        .where('person.ownerId', '=', viewingUserId),
+    ).as('person');
 };
 
 const withFaceSearch = (eb: ExpressionBuilder<DB, 'asset_face'>) => {
@@ -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)

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);

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();
-    });
   });
 });

server/src/services/person.service.ts

@@ -87,9 +87,10 @@ export class PersonService extends BaseService {
     const result: PersonResponseDto[] = [];
     const changeFeaturePhoto = new Map<string, PersonId>();
     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<PersonResponseDto> {
     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<AssetFaceResponseDto[]> {
     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);