chore(server): replace usage of deprecated orderBy and remove unnecessary instruction (#19072)

* replace usage of deprecated orderBy instruction
remove unnecesarry extra order instruction
update e2e test

* rename symbols

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

@@ -11,11 +11,24 @@ describe('/people', () => {
   let hiddenPerson: PersonResponseDto;
   let multipleAssetsPerson: PersonResponseDto;
 
+  let nameAlicePerson: PersonResponseDto;
+  let nameBobPerson: PersonResponseDto;
+  let nameCharliePerson: PersonResponseDto;
+  let nameNullPerson: PersonResponseDto;
+
   beforeAll(async () => {
     await utils.resetDatabase();
     admin = await utils.adminSetup();
 
-    [visiblePerson, hiddenPerson, multipleAssetsPerson] = await Promise.all([
+    [
+      visiblePerson,
+      hiddenPerson,
+      multipleAssetsPerson,
+      nameCharliePerson,
+      nameBobPerson,
+      nameAlicePerson,
+      nameNullPerson,
+    ] = await Promise.all([
       utils.createPerson(admin.accessToken, {
         name: 'visible_person',
       }),
@@ -26,10 +39,24 @@ describe('/people', () => {
       utils.createPerson(admin.accessToken, {
         name: 'multiple_assets_person',
       }),
+      // --- Setup for the specific sorting test ---
+      utils.createPerson(admin.accessToken, {
+        name: 'Charlie',
+      }),
+      utils.createPerson(admin.accessToken, {
+        name: 'Bob',
+      }),
+      utils.createPerson(admin.accessToken, {
+        name: 'Alice',
+      }),
+      utils.createPerson(admin.accessToken, {
+        name: '',
+      }),
     ]);
 
     const asset1 = await utils.createAsset(admin.accessToken);
     const asset2 = await utils.createAsset(admin.accessToken);
+    const asset3 = await utils.createAsset(admin.accessToken);
 
     await Promise.all([
       utils.createFace({ assetId: asset1.id, personId: visiblePerson.id }),
@@ -37,6 +64,15 @@ describe('/people', () => {
       utils.createFace({ assetId: asset1.id, personId: multipleAssetsPerson.id }),
       utils.createFace({ assetId: asset1.id, personId: multipleAssetsPerson.id }),
       utils.createFace({ assetId: asset2.id, personId: multipleAssetsPerson.id }),
+      utils.createFace({ assetId: asset3.id, personId: multipleAssetsPerson.id }),
+      // Named persons
+      utils.createFace({ assetId: asset1.id, personId: nameCharliePerson.id }), // 1 asset
+      utils.createFace({ assetId: asset1.id, personId: nameBobPerson.id }),
+      utils.createFace({ assetId: asset2.id, personId: nameBobPerson.id }), // 2 assets
+      utils.createFace({ assetId: asset1.id, personId: nameAlicePerson.id }), // 1 asset
+      // Null-named person
+      utils.createFace({ assetId: asset1.id, personId: nameNullPerson.id }),
+      utils.createFace({ assetId: asset2.id, personId: nameNullPerson.id }), // 2 assets
     ]);
   });
 
@@ -51,26 +87,53 @@ describe('/people', () => {
       expect(status).toBe(200);
       expect(body).toEqual({
         hasNextPage: false,
-        total: 3,
+        total: 7,
         hidden: 1,
         people: [
           expect.objectContaining({ name: 'multiple_assets_person' }),
+          expect.objectContaining({ name: 'Bob' }),
+          expect.objectContaining({ name: 'Alice' }),
+          expect.objectContaining({ name: 'Charlie' }),
           expect.objectContaining({ name: 'visible_person' }),
           expect.objectContaining({ name: 'hidden_person' }),
         ],
       });
     });
 
+    it('should sort visible people by asset count (desc), then by name (asc, nulls last)', async () => {
+      const { status, body } = await request(app).get('/people').set('Authorization', `Bearer ${admin.accessToken}`);
+
+      expect(status).toBe(200);
+      expect(body.hasNextPage).toBe(false);
+      expect(body.total).toBe(7); // All persons
+      expect(body.hidden).toBe(1); // 'hidden_person'
+
+      const people = body.people as PersonResponseDto[];
+
+      expect(people.map((p) => p.id)).toEqual([
+        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
+        visiblePerson.id, // name: 'visible_person', count: 1
+      ]);
+
+      expect(people.some((p) => p.id === hiddenPerson.id)).toBe(false);
+    });
+
     it('should return only visible people', async () => {
       const { status, body } = await request(app).get('/people').set('Authorization', `Bearer ${admin.accessToken}`);
 
       expect(status).toBe(200);
       expect(body).toEqual({
         hasNextPage: false,
-        total: 3,
+        total: 7,
         hidden: 1,
         people: [
           expect.objectContaining({ name: 'multiple_assets_person' }),
+          expect.objectContaining({ name: 'Bob' }),
+          expect.objectContaining({ name: 'Alice' }),
+          expect.objectContaining({ name: 'Charlie' }),
           expect.objectContaining({ name: 'visible_person' }),
         ],
       });
@@ -80,12 +143,12 @@ describe('/people', () => {
       const { status, body } = await request(app)
         .get('/people')
         .set('Authorization', `Bearer ${admin.accessToken}`)
-        .query({ withHidden: true, page: 2, size: 1 });
+        .query({ withHidden: true, page: 5, size: 1 });
 
       expect(status).toBe(200);
       expect(body).toEqual({
         hasNextPage: true,
-        total: 3,
+        total: 7,
         hidden: 1,
         people: [expect.objectContaining({ name: 'visible_person' })],
       });
@@ -128,7 +191,7 @@ describe('/people', () => {
         .set('Authorization', `Bearer ${admin.accessToken}`);
 
       expect(status).toBe(200);
-      expect(body).toEqual(expect.objectContaining({ assets: 2 }));
+      expect(body).toEqual(expect.objectContaining({ assets: 3 }));
     });
   });
 

server/src/repositories/person.repository.ts

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