feat: smart merge (#6508)

* pr feedback

* fix: tests

* update assets statistics

* pr feedback

* pr feedback

* fix: linter

* pr feedback

* fix: don't limit the smart merge

* pr feedback

* fix: server code

* remove slider

* fix: tests

---------

Co-authored-by: Jason Rasmussen <jrasm91@gmail.com>

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

@@ -999,7 +999,7 @@ describe(PersonService.name, () => {
       expect(accessMock.person.checkOwnerAccess).toHaveBeenCalledWith(authStub.admin.user.id, new Set(['person-1']));
     });
 
-    it('should merge two people', async () => {
+    it('should merge two people without smart merge', async () => {
       personMock.getById.mockResolvedValueOnce(personStub.primaryPerson);
       personMock.getById.mockResolvedValueOnce(personStub.mergePerson);
       accessMock.person.checkOwnerAccess.mockResolvedValueOnce(new Set(['person-1']));
@@ -1017,6 +1017,30 @@ describe(PersonService.name, () => {
       expect(accessMock.person.checkOwnerAccess).toHaveBeenCalledWith(authStub.admin.user.id, new Set(['person-1']));
     });
 
+    it('should merge two people with smart merge', async () => {
+      personMock.getById.mockResolvedValueOnce(personStub.randomPerson);
+      personMock.getById.mockResolvedValueOnce(personStub.primaryPerson);
+      personMock.update.mockResolvedValue({ ...personStub.randomPerson, name: personStub.primaryPerson.name });
+      accessMock.person.checkOwnerAccess.mockResolvedValueOnce(new Set(['person-3']));
+      accessMock.person.checkOwnerAccess.mockResolvedValueOnce(new Set(['person-1']));
+
+      await expect(sut.mergePerson(authStub.admin, 'person-3', { ids: ['person-1'] })).resolves.toEqual([
+        { id: 'person-1', success: true },
+      ]);
+
+      expect(personMock.reassignFaces).toHaveBeenCalledWith({
+        newPersonId: personStub.randomPerson.id,
+        oldPersonId: personStub.primaryPerson.id,
+      });
+
+      expect(personMock.update).toHaveBeenCalledWith({
+        id: personStub.randomPerson.id,
+        name: personStub.primaryPerson.name,
+      });
+
+      expect(accessMock.person.checkOwnerAccess).toHaveBeenCalledWith(authStub.admin.user.id, new Set(['person-1']));
+    });
+
     it('should throw an error when the primary person is not found', async () => {
       personMock.getById.mockResolvedValue(null);
       accessMock.person.checkOwnerAccess.mockResolvedValue(new Set(['person-1']));
@@ -1045,8 +1069,8 @@ describe(PersonService.name, () => {
     });
 
     it('should handle an error reassigning faces', async () => {
-      personMock.getById.mockResolvedValue(personStub.primaryPerson);
-      personMock.getById.mockResolvedValue(personStub.mergePerson);
+      personMock.getById.mockResolvedValueOnce(personStub.primaryPerson);
+      personMock.getById.mockResolvedValueOnce(personStub.mergePerson);
       personMock.reassignFaces.mockRejectedValue(new Error('update failed'));
       accessMock.person.checkOwnerAccess.mockResolvedValueOnce(new Set(['person-1']));
       accessMock.person.checkOwnerAccess.mockResolvedValueOnce(new Set(['person-2']));

server/src/domain/person/person.service.ts

@@ -533,7 +533,7 @@ export class PersonService {
   async mergePerson(auth: AuthDto, id: string, dto: MergePersonDto): Promise<BulkIdResponseDto[]> {
     const mergeIds = dto.ids;
     await this.access.requirePermission(auth, Permission.PERSON_WRITE, id);
-    const primaryPerson = await this.findOrFail(id);
+    let primaryPerson = await this.findOrFail(id);
     const primaryName = primaryPerson.name || primaryPerson.id;
 
     const results: BulkIdResponseDto[] = [];
@@ -554,6 +554,19 @@ export class PersonService {
           continue;
         }
 
+        const update: Partial<PersonEntity> = {};
+        if (!primaryPerson.name && mergePerson.name) {
+          update.name = mergePerson.name;
+        }
+
+        if (!primaryPerson.birthDate && mergePerson.birthDate) {
+          update.birthDate = mergePerson.birthDate;
+        }
+
+        if (Object.keys(update).length > 0) {
+          primaryPerson = await this.repository.update({ id: primaryPerson.id, ...update });
+        }
+
         const mergeName = mergePerson.name || mergePerson.id;
         const mergeData: UpdateFacesData = { oldPersonId: mergeId, newPersonId: id };
         this.logger.log(`Merging ${mergeName} into ${primaryName}`);
@@ -568,7 +581,6 @@ export class PersonService {
         results.push({ id: mergeId, success: false, error: BulkIdErrorReason.UNKNOWN });
       }
     }
-
     return results;
   }
 

server/test/fixtures/person.stub.ts

@@ -128,4 +128,18 @@ export const personStub = {
     faceAsset: null,
     isHidden: false,
   }),
+  randomPerson: Object.freeze<PersonEntity>({
+    id: 'person-3',
+    createdAt: new Date('2021-01-01'),
+    updatedAt: new Date('2021-01-01'),
+    ownerId: userStub.admin.id,
+    owner: userStub.admin,
+    name: '',
+    birthDate: null,
+    thumbnailPath: '/path/to/thumbnail',
+    faces: [],
+    faceAssetId: null,
+    faceAsset: null,
+    isHidden: false,
+  }),
 };

web/src/lib/components/faces-page/merge-face-selector.svelte

@@ -26,7 +26,7 @@
 
   let dispatch = createEventDispatcher<{
     back: void;
-    merge: void;
+    merge: PersonResponseDto;
   }>();
 
   $: hasSelection = selectedPeople.length > 0;
@@ -68,16 +68,17 @@
 
   const handleMerge = async () => {
     try {
-      const { data: results } = await api.personApi.mergePerson({
+      let { data: results } = await api.personApi.mergePerson({
         id: person.id,
         mergePersonDto: { ids: selectedPeople.map(({ id }) => id) },
       });
+      const { data: mergedPerson } = await api.personApi.getPerson({ id: person.id });
       const count = results.filter(({ success }) => success).length;
       notificationController.show({
         message: `Merged ${count} ${count === 1 ? 'person' : 'people'}`,
         type: NotificationType.Info,
       });
-      dispatch('merge');
+      dispatch('merge', mergedPerson);
     } catch (error) {
       handleError(error, 'Cannot merge people');
     } finally {

web/src/routes/(user)/people/+page.svelte

@@ -165,8 +165,12 @@
         id: personMerge2.id,
         mergePersonDto: { ids: [personToMerge.id] },
       });
+
+      const { data: mergedPerson } = await api.personApi.getPerson({ id: personToMerge.id });
+
       countVisiblePeople--;
       people = people.filter((person: PersonResponseDto) => person.id !== personToMerge.id);
+      people = people.map((person: PersonResponseDto) => (person.id === personMerge2.id ? mergedPerson : person));
 
       notificationController.show({
         message: 'Merge people succesfully',

web/src/routes/(user)/people/[personId]/+page.svelte

@@ -185,8 +185,13 @@
     }
   };
 
-  const handleMerge = () => {
+  const handleMerge = async (person: PersonResponseDto) => {
+    const { data: statistics } = await api.personApi.getPersonStatistics({ id: person.id });
+    numberOfAssets = statistics.assets;
     handleGoBack();
+
+    data.person = person;
+
     refreshAssetGrid = !refreshAssetGrid;
   };
 
@@ -374,7 +379,7 @@
 {/if}
 
 {#if viewMode === ViewMode.MERGE_PEOPLE}
-  <MergeFaceSelector person={data.person} on:back={handleGoBack} on:merge={handleMerge} />
+  <MergeFaceSelector person={data.person} on:back={handleGoBack} on:merge={({ detail }) => handleMerge(detail)} />
 {/if}
 
 <header>