From 07b874eddab9432af829ab8ecc2810d7796fdf9d Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 19 Jan 2024 11:34:20 -0600 Subject: [PATCH] fix(web): revert smart merge (#6504) * revert smart merge * fix test * fix test * Remove Slider file --- .../src/domain/person/person.service.spec.ts | 33 +-------- server/src/domain/person/person.service.ts | 16 +---- server/test/fixtures/person.stub.ts | 14 ---- .../admin-page/settings/setting-switch.svelte | 66 ++++++++++++++++- web/src/lib/components/elements/slider.svelte | 72 ------------------- .../faces-page/merge-face-selector.svelte | 7 +- web/src/routes/(user)/people/+page.svelte | 4 -- .../(user)/people/[personId]/+page.svelte | 9 +-- 8 files changed, 74 insertions(+), 147 deletions(-) delete mode 100644 web/src/lib/components/elements/slider.svelte diff --git a/server/src/domain/person/person.service.spec.ts b/server/src/domain/person/person.service.spec.ts index 0d289e783d..1b163b9432 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 without smart merge', async () => { + it('should merge two people', async () => { personMock.getById.mockResolvedValueOnce(personStub.primaryPerson); personMock.getById.mockResolvedValueOnce(personStub.mergePerson); accessMock.person.checkOwnerAccess.mockResolvedValueOnce(new Set(['person-1'])); @@ -1014,33 +1014,6 @@ describe(PersonService.name, () => { oldPersonId: personStub.mergePerson.id, }); - expect(personMock.update).not.toHaveBeenCalled(); - - 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(personMock.delete).toHaveBeenCalledWith([personStub.primaryPerson]); expect(accessMock.person.checkOwnerAccess).toHaveBeenCalledWith(authStub.admin.user.id, new Set(['person-1'])); }); @@ -1072,8 +1045,8 @@ describe(PersonService.name, () => { }); it('should handle an error reassigning faces', async () => { - personMock.getById.mockResolvedValueOnce(personStub.primaryPerson); - personMock.getById.mockResolvedValueOnce(personStub.mergePerson); + personMock.getById.mockResolvedValue(personStub.primaryPerson); + personMock.getById.mockResolvedValue(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 561630e549..57ab27a5d8 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); - let primaryPerson = await this.findOrFail(id); + const primaryPerson = await this.findOrFail(id); const primaryName = primaryPerson.name || primaryPerson.id; const results: BulkIdResponseDto[] = []; @@ -554,19 +554,6 @@ 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}`); @@ -581,6 +568,7 @@ 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 ad83d68001..76e2d14f72 100644 --- a/server/test/fixtures/person.stub.ts +++ b/server/test/fixtures/person.stub.ts @@ -128,18 +128,4 @@ 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/admin-page/settings/setting-switch.svelte b/web/src/lib/components/admin-page/settings/setting-switch.svelte index d26144913f..6797423a55 100644 --- a/web/src/lib/components/admin-page/settings/setting-switch.svelte +++ b/web/src/lib/components/admin-page/settings/setting-switch.svelte @@ -2,7 +2,6 @@ import { quintOut } from 'svelte/easing'; import { fly } from 'svelte/transition'; import { createEventDispatcher } from 'svelte'; - import Slider from '$lib/components/elements/slider.svelte'; export let title: string; export let subtitle = ''; @@ -11,6 +10,7 @@ export let isEdited = false; const dispatch = createEventDispatcher<{ toggle: boolean }>(); + const onToggle = (event: Event) => dispatch('toggle', (event.target as HTMLInputElement).checked);
@@ -31,5 +31,67 @@

{subtitle}

- dispatch('toggle', checked)} /> + + + + diff --git a/web/src/lib/components/elements/slider.svelte b/web/src/lib/components/elements/slider.svelte deleted file mode 100644 index 77fe3da940..0000000000 --- a/web/src/lib/components/elements/slider.svelte +++ /dev/null @@ -1,72 +0,0 @@ - - - - - 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 5dd48b04c6..56c1b86e7c 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: PersonResponseDto; + merge: void; }>(); $: hasSelection = selectedPeople.length > 0; @@ -68,17 +68,16 @@ const handleMerge = async () => { try { - let { data: results } = await api.personApi.mergePerson({ + const { 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', mergedPerson); + dispatch('merge'); } 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 323b160a82..eec418c6ad 100644 --- a/web/src/routes/(user)/people/+page.svelte +++ b/web/src/routes/(user)/people/+page.svelte @@ -165,12 +165,8 @@ 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 6d7bd77865..c97d3845cb 100644 --- a/web/src/routes/(user)/people/[personId]/+page.svelte +++ b/web/src/routes/(user)/people/[personId]/+page.svelte @@ -185,13 +185,8 @@ } }; - const handleMerge = async (person: PersonResponseDto) => { - const { data: statistics } = await api.personApi.getPersonStatistics({ id: person.id }); - numberOfAssets = statistics.assets; + const handleMerge = () => { handleGoBack(); - - data.person = person; - refreshAssetGrid = !refreshAssetGrid; }; @@ -379,7 +374,7 @@ {/if} {#if viewMode === ViewMode.MERGE_PEOPLE} - handleMerge(detail)} /> + {/if}