fix: people ordering by asset count (#19366)

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

@@ -125,12 +125,12 @@ describe('/people', () => {
         total: 11,
         hidden: 1,
         people: [
-          expect.objectContaining({ name: 'Bill' }),
           expect.objectContaining({ name: 'Freddy' }),
-          expect.objectContaining({ name: 'Alice' }),
+          expect.objectContaining({ name: 'Bill' }),
+          expect.objectContaining({ name: 'multiple_assets_person' }),
           expect.objectContaining({ name: 'Bob' }),
+          expect.objectContaining({ name: 'Alice' }),
           expect.objectContaining({ name: 'Charlie' }),
-          expect.objectContaining({ name: 'multiple_assets_person' }),
           expect.objectContaining({ name: 'visible_person' }),
           expect.objectContaining({ id: nameNullPerson4Assets.id, name: '' }),
           expect.objectContaining({ id: nameNullPerson3Assets.id, name: '' }),
@@ -150,12 +150,12 @@ describe('/people', () => {
       const people = body.people as PersonResponseDto[];
 
       expect(people.map((p) => p.id)).toEqual([
-        nameBillPersonFavourite.id, // name: 'Bill', count: 2
         nameFreddyPersonFavourite.id, // name: 'Freddy', count: 2
-        nameAlicePerson.id, // name: 'Alice', count: 1
+        nameBillPersonFavourite.id, // name: 'Bill', count: 1
+        multipleAssetsPerson.id, // name: 'multiple_assets_person', count: 3
         nameBobPerson.id, // name: 'Bob', count: 2
+        nameAlicePerson.id, // name: 'Alice', count: 1
         nameCharliePerson.id, // name: 'Charlie', count: 1
-        multipleAssetsPerson.id, // name: 'multiple_assets_person', count: 3
         visiblePerson.id, // name: 'visible_person', count: 1
         nameNullPerson4Assets.id, // name: '', count: 4
         nameNullPerson3Assets.id, // name: '', count: 3
@@ -173,12 +173,12 @@ describe('/people', () => {
         total: 11,
         hidden: 1,
         people: [
-          expect.objectContaining({ name: 'Bill' }),
           expect.objectContaining({ name: 'Freddy' }),
-          expect.objectContaining({ name: 'Alice' }),
+          expect.objectContaining({ name: 'Bill' }),
+          expect.objectContaining({ name: 'multiple_assets_person' }),
           expect.objectContaining({ name: 'Bob' }),
+          expect.objectContaining({ name: 'Alice' }),
           expect.objectContaining({ name: 'Charlie' }),
-          expect.objectContaining({ name: 'multiple_assets_person' }),
           expect.objectContaining({ name: 'visible_person' }),
           expect.objectContaining({ id: nameNullPerson4Assets.id, name: '' }),
           expect.objectContaining({ id: nameNullPerson3Assets.id, name: '' }),
@@ -197,7 +197,7 @@ describe('/people', () => {
         hasNextPage: true,
         total: 11,
         hidden: 1,
-        people: [expect.objectContaining({ name: 'Charlie' })],
+        people: [expect.objectContaining({ name: 'Alice' })],
       });
     });
   });

server/src/queries/person.repository.sql

@@ -35,8 +35,9 @@ having
 order by
   "person"."isHidden" asc,
   "person"."isFavorite" desc,
-  NULLIF(person.name, '') asc nulls last,
+  NULLIF(person.name, '') is null asc,
   count("asset_faces"."assetId") desc,
+  NULLIF(person.name, '') asc nulls last,
   "person"."createdAt"
 limit
   $5

server/src/repositories/person.repository.ts

@@ -180,8 +180,9 @@ export class PersonRepository {
       )
       .$if(!options?.closestFaceAssetId, (qb) =>
         qb
-          .orderBy(sql`NULLIF(person.name, '')`, (om) => om.asc().nullsLast())
+          .orderBy(sql`NULLIF(person.name, '') is null`, 'asc')
           .orderBy((eb) => eb.fn.count('asset_faces.assetId'), 'desc')
+          .orderBy(sql`NULLIF(person.name, '')`, (om) => om.asc().nullsLast())
           .orderBy('person.createdAt'),
       )
       .$if(!options?.withHidden, (qb) => qb.where('person.isHidden', '=', false))