fix(server): people count doesn't match array

e2e/src/api/specs/person.e2e-spec.ts

@@ -122,7 +122,7 @@
       expect(status).toBe(200);
       expect(body).toEqual({
         hasNextPage: false,
-        total: 11,
+        total: 10,
         hidden: 1,
         people: [
           expect.objectContaining({ name: 'Freddy' }),
@@ -144,7 +144,7 @@
 
       expect(status).toBe(200);
       expect(body.hasNextPage).toBe(false);
-      expect(body.total).toBe(11); // All persons
+      expect(body.total).toBe(10); // All people
       expect(body.hidden).toBe(1); // 'hidden_person'
 
       const people = body.people as PersonResponseDto[];
@@ -168,9 +168,9 @@
       const { status, body } = await request(app).get('/people').set('Authorization', `Bearer ${admin.accessToken}`);
 
       expect(status).toBe(200);
       expect(body).toEqual({
         hasNextPage: false,
-        total: 11,
+        total: 10,
         hidden: 1,
         people: [
           expect.objectContaining({ name: 'Freddy' }),
@@ -195,7 +195,7 @@
       expect(status).toBe(200);
       expect(body).toEqual({
         hasNextPage: true,
-        total: 11,
+        total: 10,
         hidden: 1,
         people: [expect.objectContaining({ name: 'Alice' })],
       });

server/src/queries/person.repository.sql

@@ -23,38 +23,6 @@ where
 limit
   3
 
--- PersonRepository.getAllForUser
-select
-  "person".*
-from
-  "person"
-  inner join "asset_face" on "asset_face"."personId" = "person"."id"
-  inner join "asset" on "asset_face"."assetId" = "asset"."id"
-  and "asset"."visibility" = 'timeline'
-  and "asset"."deletedAt" is null
-where
-  "person"."ownerId" = $1
-  and "asset_face"."deletedAt" is null
-  and "person"."isHidden" = $2
-group by
-  "person"."id"
-having
-  (
-    "person"."name" != $3
-    or count("asset_face"."assetId") >= $4
-  )
-order by
-  "person"."isHidden" asc,
-  "person"."isFavorite" desc,
-  NULLIF(person.name, '') is null asc,
-  count("asset_face"."assetId") desc,
-  NULLIF(person.name, '') asc nulls last,
-  "person"."createdAt"
-limit
-  $5
-offset
-  $6
-
 -- PersonRepository.getAllWithoutFaces
 select
   "person".*
@@ -230,38 +198,6 @@ from
 where
   "asset_face"."deletedAt" is null
 
--- PersonRepository.getNumberOfPeople
-select
-  coalesce(count(*), 0) as "total",
-  coalesce(
-    count(*) filter (
-      where
-        "isHidden" = $1
-    ),
-    0
-  ) as "hidden"
-from
-  "person"
-where
-  exists (
-    select
-    from
-      "asset_face"
-    where
-      "asset_face"."personId" = "person"."id"
-      and "asset_face"."deletedAt" is null
-      and exists (
-        select
-        from
-          "asset"
-        where
-          "asset"."id" = "asset_face"."assetId"
-          and "asset"."visibility" = 'timeline'
-          and "asset"."deletedAt" is null
-      )
-  )
-  and "person"."ownerId" = $2
-
 -- PersonRepository.refreshFaces
 with
   "added_embeddings" as (

server/src/repositories/person.repository.ts

@@ -147,10 +147,9 @@ export class PersonRepository {
   }
 
   @GenerateSql({ params: [{ take: 1, skip: 0 }, DummyValue.UUID] })
-  async getAllForUser(pagination: PaginationOptions, userId: string, options?: PersonSearchOptions) {
-    const items = await this.db
+  async getAllForUser(pagination: PaginationOptions, userId: string, options: PersonSearchOptions) {
+    const baseQuery = this.db
       .selectFrom('person')
-      .selectAll('person')
       .innerJoin('asset_face', 'asset_face.personId', 'person.id')
       .innerJoin('asset', (join) =>
         join
@@ -160,45 +159,62 @@ export class PersonRepository {
       )
       .where('person.ownerId', '=', userId)
       .where('asset_face.deletedAt', 'is', null)
-      .orderBy('person.isHidden', 'asc')
-      .orderBy('person.isFavorite', 'desc')
+      .groupBy('person.id')
       .having((eb) =>
-        eb.or([
-          eb('person.name', '!=', ''),
-          eb((innerEb) => innerEb.fn.count('asset_face.assetId'), '>=', options?.minimumFaceCount || 1),
-        ]),
+        eb.or([eb('person.name', '!=', ''), eb(eb.fn.count('asset_face.assetId'), '>=', options.minimumFaceCount)]),
       )
-      .groupBy('person.id')
-      .$if(!!options?.closestFaceAssetId, (qb) =>
-        qb.orderBy((eb) =>
-          eb(
-            (eb) =>
-              eb
-                .selectFrom('face_search')
-                .select('face_search.embedding')
-                .whereRef('face_search.faceId', '=', 'person.faceAssetId'),
-            '<=>',
-            (eb) =>
-              eb
-                .selectFrom('face_search')
-                .select('face_search.embedding')
-                .where('face_search.faceId', '=', options!.closestFaceAssetId!),
-          ),
+      .selectAll('person')
+      .select((eb) => [
+        eb.fn.count('asset_face.assetId').as('faceCount'),
+
+        eb.fn.countAll().over().as('totalCount'),
+
+        eb.fn
+          .sum(sql<number>`CASE WHEN ${sql.ref('person.isHidden')} THEN 1 ELSE 0 END`)
+          .over()
+          .as('hiddenCount'),
+      ]);
+
+    let sorted = baseQuery.orderBy('person.isHidden', 'asc').orderBy('person.isFavorite', 'desc');
+
+    if (options.closestFaceAssetId) {
+      const closestId = options.closestFaceAssetId!;
+
+      sorted = sorted.orderBy((eb) =>
+        eb(
+          (eb) =>
+            eb
+              .selectFrom('face_search')
+              .select('face_search.embedding')
+              .whereRef('face_search.faceId', '=', 'person.faceAssetId'),
+          '<=>',
+          (eb) =>
+            eb.selectFrom('face_search').select('face_search.embedding').where('face_search.faceId', '=', closestId),
         ),
-      )
-      .$if(!options?.closestFaceAssetId, (qb) =>
-        qb
-          .orderBy(sql`NULLIF(person.name, '') is null`, 'asc')
-          .orderBy((eb) => eb.fn.count('asset_face.assetId'), 'desc')
-          .orderBy(sql`NULLIF(person.name, '')`, (om) => om.asc().nullsLast())
-          .orderBy('person.createdAt'),
-      )
-      .$if(!options?.withHidden, (qb) => qb.where('person.isHidden', '=', false))
+      );
+    } else {
+      sorted = sorted
+        .orderBy(sql`NULLIF(person.name, '') IS NULL`, 'asc')
+        .orderBy((eb) => eb.fn.count('asset_face.assetId'), 'desc')
+        .orderBy(sql`NULLIF(person.name, '')`, (o) => o.asc().nullsLast())
+        .orderBy('person.createdAt');
+    }
+
+    if (!options.withHidden) {
+      sorted = sorted.where('person.isHidden', '=', false);
+    }
+
+    const rows = await sorted
       .offset(pagination.skip ?? 0)
       .limit(pagination.take + 1)
       .execute();
 
-    return paginationHelper(items, pagination.take);
+    const total = rows[0]?.totalCount ?? 0;
+    const hidden = rows[0]?.hiddenCount ?? 0;
+
+    const { items, hasNextPage } = paginationHelper(rows, pagination.take);
+
+    return { items, hasNextPage, total, hidden };
   }
 
   @GenerateSql()
@@ -357,34 +373,6 @@ export class PersonRepository {
     };
   }
 
-  @GenerateSql({ params: [DummyValue.UUID] })
-  getNumberOfPeople(userId: string) {
-    const zero = sql.lit(0);
-    return this.db
-      .selectFrom('person')
-      .where((eb) =>
-        eb.exists((eb) =>
-          eb
-            .selectFrom('asset_face')
-            .whereRef('asset_face.personId', '=', 'person.id')
-            .where('asset_face.deletedAt', 'is', null)
-            .where((eb) =>
-              eb.exists((eb) =>
-                eb
-                  .selectFrom('asset')
-                  .whereRef('asset.id', '=', 'asset_face.assetId')
-                  .where('asset.visibility', '=', sql.lit(AssetVisibility.Timeline))
-                  .where('asset.deletedAt', 'is', null),
-              ),
-            ),
-        ),
-      )
-      .where('person.ownerId', '=', userId)
-      .select((eb) => eb.fn.coalesce(eb.fn.countAll<number>(), zero).as('total'))
-      .select((eb) => eb.fn.coalesce(eb.fn.countAll<number>().filterWhere('isHidden', '=', true), zero).as('hidden'))
-      .executeTakeFirstOrThrow();
-  }
-
   create(person: Insertable<PersonTable>) {
     return this.db.insertInto('person').values(person).returningAll().executeTakeFirstOrThrow();
   }

server/src/services/person.service.spec.ts

@@ -70,11 +70,28 @@ describe(PersonService.name, () => {
 
   describe('getAll', () => {
     it('should get all hidden and visible people with thumbnails', async () => {
+      // Updated stubs with the required aggregated fields
+      const personWithCounts = {
+        ...personStub.withName,
+        faceCount: 3,
+        totalCount: 2,
+        hiddenCount: 1,
+      };
+
+      const hiddenPersonWithCounts = {
+        ...personStub.hidden,
+        faceCount: 1,
+        totalCount: 2,
+        hiddenCount: 1,
+      };
+
       mocks.person.getAllForUser.mockResolvedValue({
-        items: [personStub.withName, personStub.hidden],
+        items: [personWithCounts, hiddenPersonWithCounts],
         hasNextPage: false,
+        total: 2,
+        hidden: 1,
       });
-      mocks.person.getNumberOfPeople.mockResolvedValue({ total: 2, hidden: 1 });
+
       await expect(sut.getAll(authStub.admin, { withHidden: true, page: 1, size: 10 })).resolves.toEqual({
         hasNextPage: false,
         total: 2,
@@ -93,18 +110,36 @@ describe(PersonService.name, () => {
           },
         ],
       });
+
       expect(mocks.person.getAllForUser).toHaveBeenCalledWith({ skip: 0, take: 10 }, authStub.admin.user.id, {
         minimumFaceCount: 3,
         withHidden: true,
+        closestFaceAssetId: undefined,
       });
     });
 
     it('should get all visible people and favorites should be first in the array', async () => {
+      const favoritePersonWithCounts = {
+        ...personStub.isFavorite,
+        faceCount: 3,
+        totalCount: 2,
+        hiddenCount: 1,
+      };
+
+      const namedPersonWithCounts = {
+        ...personStub.withName,
+        faceCount: 3,
+        totalCount: 2,
+        hiddenCount: 1,
+      };
+
       mocks.person.getAllForUser.mockResolvedValue({
-        items: [personStub.isFavorite, personStub.withName],
+        items: [favoritePersonWithCounts, namedPersonWithCounts],
         hasNextPage: false,
+        total: 2,
+        hidden: 1,
       });
-      mocks.person.getNumberOfPeople.mockResolvedValue({ total: 2, hidden: 1 });
+
       await expect(sut.getAll(authStub.admin, { withHidden: false, page: 1, size: 10 })).resolves.toEqual({
         hasNextPage: false,
         total: 2,
@@ -123,9 +158,11 @@ describe(PersonService.name, () => {
           responseDto,
         ],
       });
+
       expect(mocks.person.getAllForUser).toHaveBeenCalledWith({ skip: 0, take: 10 }, authStub.admin.user.id, {
         minimumFaceCount: 3,
         withHidden: false,
+        closestFaceAssetId: undefined,
       });
     });
   });

server/src/services/person.service.ts

@@ -48,32 +48,34 @@ import { isFacialRecognitionEnabled } from 'src/utils/misc';
 export class PersonService extends BaseService {
   async getAll(auth: AuthDto, dto: PersonSearchDto): Promise<PeopleResponseDto> {
     const { withHidden = false, closestAssetId, closestPersonId, page, size } = dto;
-    let closestFaceAssetId = closestAssetId;
+
     const pagination = {
       take: size,
       skip: (page - 1) * size,
     };
 
+    let closestFaceAssetId = closestAssetId;
     if (closestPersonId) {
       const person = await this.personRepository.getById(closestPersonId);
       if (!person?.faceAssetId) {
         throw new NotFoundException('Person not found');
       }
       closestFaceAssetId = person.faceAssetId;
     }
+
     const { machineLearning } = await this.getConfig({ withCache: false });
-    const { items, hasNextPage } = await this.personRepository.getAllForUser(pagination, auth.user.id, {
+
+    const { items, hasNextPage, total, hidden } = await this.personRepository.getAllForUser(pagination, auth.user.id, {
       minimumFaceCount: machineLearning.facialRecognition.minFaces,
       withHidden,
       closestFaceAssetId,
     });
-    const { total, hidden } = await this.personRepository.getNumberOfPeople(auth.user.id);
 
     return {
       people: items.map((person) => mapPerson(person)),
       hasNextPage,
-      total,
-      hidden,
+      total: Number(total),
+      hidden: Number(hidden),
     };
   }