diff --git a/server/src/domain/person/person.service.spec.ts b/server/src/domain/person/person.service.spec.ts index 1b163b9432..c28af12613 100644 --- a/server/src/domain/person/person.service.spec.ts +++ b/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'])); diff --git a/server/src/domain/person/person.service.ts b/server/src/domain/person/person.service.ts index 57ab27a5d8..561630e549 100644 --- a/server/src/domain/person/person.service.ts +++ b/server/src/domain/person/person.service.ts @@ -533,7 +533,7 @@ export class PersonService { async mergePerson(auth: AuthDto, id: string, dto: MergePersonDto): Promise { 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 = {}; + 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; } diff --git a/server/test/fixtures/person.stub.ts b/server/test/fixtures/person.stub.ts index 76e2d14f72..ad83d68001 100644 --- a/server/test/fixtures/person.stub.ts +++ b/server/test/fixtures/person.stub.ts @@ -128,4 +128,18 @@ export const personStub = { faceAsset: null, isHidden: false, }), + randomPerson: Object.freeze({ + 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, + }), }; diff --git a/web/src/lib/components/faces-page/merge-face-selector.svelte b/web/src/lib/components/faces-page/merge-face-selector.svelte index 56c1b86e7c..5dd48b04c6 100644 --- a/web/src/lib/components/faces-page/merge-face-selector.svelte +++ b/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 { diff --git a/web/src/routes/(user)/people/+page.svelte b/web/src/routes/(user)/people/+page.svelte index eec418c6ad..323b160a82 100644 --- a/web/src/routes/(user)/people/+page.svelte +++ b/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', diff --git a/web/src/routes/(user)/people/[personId]/+page.svelte b/web/src/routes/(user)/people/[personId]/+page.svelte index c97d3845cb..6d7bd77865 100644 --- a/web/src/routes/(user)/people/[personId]/+page.svelte +++ b/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} - + handleMerge(detail)} /> {/if}