From f0c402d9a613ba4443f79b59c679f6f4a19e81f5 Mon Sep 17 00:00:00 2001 From: Pranay Date: Sat, 26 Oct 2024 22:24:10 +0530 Subject: [PATCH 1/9] fix: remove duplicateIds on unique assets --- .../[[photos=photos]]/[[assetId=id]]/+page.ts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.ts b/web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.ts index a7faaed3c3013..f6b4a4bb8b2e2 100644 --- a/web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.ts +++ b/web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.ts @@ -1,13 +1,28 @@ import { authenticate } from '$lib/utils/auth'; import { getFormatter } from '$lib/utils/i18n'; import { getAssetInfoFromParam } from '$lib/utils/navigation'; -import { getAssetDuplicates } from '@immich/sdk'; +import { getAssetDuplicates, updateAssets, type DuplicateResponseDto } from '@immich/sdk'; import type { PageLoad } from './$types'; +const rectifyDuplicate = (duplicate: DuplicateResponseDto) => { + updateAssets({ assetBulkUpdateDto: { ids: duplicate.assets.map((asset) => asset.id), duplicateId: null } }); +}; + +const processDuplicates = (duplicates: DuplicateResponseDto[]) => { + return duplicates.filter((duplicate) => { + if (duplicate.assets.length <= 1) { + rectifyDuplicate(duplicate); + return false; + } + return true; + }); +}; + export const load = (async ({ params }) => { await authenticate(); const asset = await getAssetInfoFromParam(params); - const duplicates = await getAssetDuplicates(); + const Allduplicates = await getAssetDuplicates(); + const duplicates = processDuplicates(Allduplicates); const $t = await getFormatter(); return { From e490b02581dec963c078af27ff1b6aec5c069c6d Mon Sep 17 00:00:00 2001 From: Pranay Date: Sat, 26 Oct 2024 22:46:45 +0530 Subject: [PATCH 2/9] chore: handle bulk update for rectifying unique assets --- .../[[photos=photos]]/[[assetId=id]]/+page.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.ts b/web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.ts index f6b4a4bb8b2e2..e99949d94dc13 100644 --- a/web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.ts +++ b/web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.ts @@ -4,18 +4,23 @@ import { getAssetInfoFromParam } from '$lib/utils/navigation'; import { getAssetDuplicates, updateAssets, type DuplicateResponseDto } from '@immich/sdk'; import type { PageLoad } from './$types'; -const rectifyDuplicate = (duplicate: DuplicateResponseDto) => { - updateAssets({ assetBulkUpdateDto: { ids: duplicate.assets.map((asset) => asset.id), duplicateId: null } }); +const rectifyDuplicate = (uniqueAssetIds: string[]) => { + if (uniqueAssetIds.length > 0) { + updateAssets({ assetBulkUpdateDto: { ids: uniqueAssetIds, duplicateId: null } }); + } }; const processDuplicates = (duplicates: DuplicateResponseDto[]) => { - return duplicates.filter((duplicate) => { + let uniqueAssetIds: string[] = []; + const validDuplicates = duplicates.filter((duplicate) => { if (duplicate.assets.length <= 1) { - rectifyDuplicate(duplicate); + uniqueAssetIds.push(duplicate.assets[0].id); return false; } return true; }); + rectifyDuplicate(uniqueAssetIds); + return validDuplicates; }; export const load = (async ({ params }) => { From 7a90fa9391039ce76a0b500acf106bc2c0a1d600 Mon Sep 17 00:00:00 2001 From: Pranay Date: Sat, 26 Oct 2024 23:01:26 +0530 Subject: [PATCH 3/9] chore: fix linting issues --- .../duplicates/[[photos=photos]]/[[assetId=id]]/+page.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.ts b/web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.ts index e99949d94dc13..20f20b7875e61 100644 --- a/web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.ts +++ b/web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.ts @@ -6,12 +6,12 @@ import type { PageLoad } from './$types'; const rectifyDuplicate = (uniqueAssetIds: string[]) => { if (uniqueAssetIds.length > 0) { - updateAssets({ assetBulkUpdateDto: { ids: uniqueAssetIds, duplicateId: null } }); + void updateAssets({ assetBulkUpdateDto: { ids: uniqueAssetIds, duplicateId: null } }); } }; const processDuplicates = (duplicates: DuplicateResponseDto[]) => { - let uniqueAssetIds: string[] = []; + const uniqueAssetIds: string[] = []; const validDuplicates = duplicates.filter((duplicate) => { if (duplicate.assets.length <= 1) { uniqueAssetIds.push(duplicate.assets[0].id); From fec4eec0cb414790eab35a4d0ec9cfa0f64ee4cc Mon Sep 17 00:00:00 2001 From: Pranay Date: Sun, 27 Oct 2024 02:46:00 +0530 Subject: [PATCH 4/9] revert: remove identify unique assets from duplicates in web --- .../[[photos=photos]]/[[assetId=id]]/+page.ts | 22 +------------------ 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.ts b/web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.ts index 20f20b7875e61..bdb4e60461bf1 100644 --- a/web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.ts +++ b/web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.ts @@ -4,30 +4,10 @@ import { getAssetInfoFromParam } from '$lib/utils/navigation'; import { getAssetDuplicates, updateAssets, type DuplicateResponseDto } from '@immich/sdk'; import type { PageLoad } from './$types'; -const rectifyDuplicate = (uniqueAssetIds: string[]) => { - if (uniqueAssetIds.length > 0) { - void updateAssets({ assetBulkUpdateDto: { ids: uniqueAssetIds, duplicateId: null } }); - } -}; - -const processDuplicates = (duplicates: DuplicateResponseDto[]) => { - const uniqueAssetIds: string[] = []; - const validDuplicates = duplicates.filter((duplicate) => { - if (duplicate.assets.length <= 1) { - uniqueAssetIds.push(duplicate.assets[0].id); - return false; - } - return true; - }); - rectifyDuplicate(uniqueAssetIds); - return validDuplicates; -}; - export const load = (async ({ params }) => { await authenticate(); const asset = await getAssetInfoFromParam(params); - const Allduplicates = await getAssetDuplicates(); - const duplicates = processDuplicates(Allduplicates); + const duplicates = await getAssetDuplicates(); const $t = await getFormatter(); return { From 34a6551f7ea66f867f2983dc53685869340d12a8 Mon Sep 17 00:00:00 2001 From: Pranay Date: Sun, 27 Oct 2024 02:48:12 +0530 Subject: [PATCH 5/9] feat: Add unique assets checking after fetching duplicates --- server/src/services/duplicate.service.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/server/src/services/duplicate.service.ts b/server/src/services/duplicate.service.ts index e76b80b04391c..110d0fdd2ff0c 100644 --- a/server/src/services/duplicate.service.ts +++ b/server/src/services/duplicate.service.ts @@ -15,8 +15,18 @@ import { usePagination } from 'src/utils/pagination'; export class DuplicateService extends BaseService { async getDuplicates(auth: AuthDto): Promise { const res = await this.assetRepository.getDuplicates({ userIds: [auth.user.id] }); - - return mapDuplicateResponse(res.map((a) => mapAsset(a, { auth, withStack: true }))); + const uniqueAssetIds: string[] = []; + const duplicates = mapDuplicateResponse(res.map((a) => mapAsset(a, { auth, withStack: true }))).filter((duplicate) => { + if (duplicate.assets.length === 1) { + uniqueAssetIds.push(duplicate.assets[0].id); + return false; + } + return true; + }); + if (uniqueAssetIds.length > 0) { + void this.assetRepository.updateAll(uniqueAssetIds, { duplicateId: null }); + } + return duplicates; } async handleQueueSearchDuplicates({ force }: IBaseJob): Promise { From 0dd31059b7e557efe769c2ae7373f57ea2e60889 Mon Sep 17 00:00:00 2001 From: Pranay Date: Sun, 27 Oct 2024 03:03:16 +0530 Subject: [PATCH 6/9] format the code using prettier --- server/src/services/duplicate.service.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/server/src/services/duplicate.service.ts b/server/src/services/duplicate.service.ts index 110d0fdd2ff0c..d4fa4988dd14c 100644 --- a/server/src/services/duplicate.service.ts +++ b/server/src/services/duplicate.service.ts @@ -16,13 +16,15 @@ export class DuplicateService extends BaseService { async getDuplicates(auth: AuthDto): Promise { const res = await this.assetRepository.getDuplicates({ userIds: [auth.user.id] }); const uniqueAssetIds: string[] = []; - const duplicates = mapDuplicateResponse(res.map((a) => mapAsset(a, { auth, withStack: true }))).filter((duplicate) => { - if (duplicate.assets.length === 1) { - uniqueAssetIds.push(duplicate.assets[0].id); - return false; - } - return true; - }); + const duplicates = mapDuplicateResponse(res.map((a) => mapAsset(a, { auth, withStack: true }))).filter( + (duplicate) => { + if (duplicate.assets.length === 1) { + uniqueAssetIds.push(duplicate.assets[0].id); + return false; + } + return true; + }, + ); if (uniqueAssetIds.length > 0) { void this.assetRepository.updateAll(uniqueAssetIds, { duplicateId: null }); } From 09c10b9af1b66c5ecdea90440ccaebb855cdaa84 Mon Sep 17 00:00:00 2001 From: Pranay Date: Sun, 27 Oct 2024 03:14:33 +0530 Subject: [PATCH 7/9] refactor: remove redundant imports --- .../duplicates/[[photos=photos]]/[[assetId=id]]/+page.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.ts b/web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.ts index bdb4e60461bf1..a7faaed3c3013 100644 --- a/web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.ts +++ b/web/src/routes/(user)/utilities/duplicates/[[photos=photos]]/[[assetId=id]]/+page.ts @@ -1,7 +1,7 @@ import { authenticate } from '$lib/utils/auth'; import { getFormatter } from '$lib/utils/i18n'; import { getAssetInfoFromParam } from '$lib/utils/navigation'; -import { getAssetDuplicates, updateAssets, type DuplicateResponseDto } from '@immich/sdk'; +import { getAssetDuplicates } from '@immich/sdk'; import type { PageLoad } from './$types'; export const load = (async ({ params }) => { From ca500b092891887cc58d92e6e2ec9b1b2c367417 Mon Sep 17 00:00:00 2001 From: Pranay Date: Sun, 27 Oct 2024 03:42:24 +0530 Subject: [PATCH 8/9] update tests for getDuplicates --- server/src/services/duplicate.service.spec.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/server/src/services/duplicate.service.spec.ts b/server/src/services/duplicate.service.spec.ts index 095d53dde6570..75af1ef6f1f49 100644 --- a/server/src/services/duplicate.service.spec.ts +++ b/server/src/services/duplicate.service.spec.ts @@ -31,11 +31,23 @@ describe(SearchService.name, () => { describe('getDuplicates', () => { it('should get duplicates', async () => { - assetMock.getDuplicates.mockResolvedValue([assetStub.hasDupe]); + assetMock.getDuplicates.mockResolvedValue([assetStub.hasDupe, assetStub.hasDupe]); await expect(sut.getDuplicates(authStub.admin)).resolves.toEqual([ - { duplicateId: assetStub.hasDupe.duplicateId, assets: [expect.objectContaining({ id: assetStub.hasDupe.id })] }, + { + duplicateId: assetStub.hasDupe.duplicateId, + assets: [ + expect.objectContaining({ id: assetStub.hasDupe.id }), + expect.objectContaining({ id: assetStub.hasDupe.id }), + ], + }, ]); }); + + it('should update assets with duplicateId', async () => { + assetMock.getDuplicates.mockResolvedValue([assetStub.hasDupe]); + await expect(sut.getDuplicates(authStub.admin)).resolves.toEqual([]); + expect(assetMock.updateAll).toHaveBeenCalledWith([assetStub.hasDupe.id], { duplicateId: null }); + }); }); describe('handleQueueSearchDuplicates', () => { From 566e5c2d1ab3ab480f202a07b6a31ad32b18b8d7 Mon Sep 17 00:00:00 2001 From: Pranay Date: Sun, 27 Oct 2024 13:32:04 +0530 Subject: [PATCH 9/9] await updateAll in getDuplicates to handle potential errors --- server/src/services/duplicate.service.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/server/src/services/duplicate.service.ts b/server/src/services/duplicate.service.ts index d4fa4988dd14c..7bb4d2f581863 100644 --- a/server/src/services/duplicate.service.ts +++ b/server/src/services/duplicate.service.ts @@ -26,7 +26,11 @@ export class DuplicateService extends BaseService { }, ); if (uniqueAssetIds.length > 0) { - void this.assetRepository.updateAll(uniqueAssetIds, { duplicateId: null }); + try { + await this.assetRepository.updateAll(uniqueAssetIds, { duplicateId: null }); + } catch (error: any) { + this.logger.error(`Failed to remove duplicateId from assets: ${error.message}`); + } } return duplicates; }