From fad446459b28692e3df06e5e30c0d68fe2bdf3e5 Mon Sep 17 00:00:00 2001 From: Hugo de Sousa Pereira Date: Mon, 23 Mar 2026 01:25:45 +0000 Subject: [PATCH 01/10] Fix #26168: Allow assigning zero-asset named people to new faces When a named person has all their faces removed, they were not being shown from repository queries. This prevented them from appearing in the /people endpoint and the "add face" selection list. - Update PersonRepository queries (HAVING/EXISTS clauses) to include people with a name, even if their asset or face count is zero. - Modify PersonService.deleteFace to skip deleting a person if they are named, deleting only unnamed people with zero remaining assets. - Fix frontend +page.svelte logic to avoid a "crash" when deleting the last face of an unnamed person, by preventing a redirect to a person page that no longer exists. - Add e2e and unit tests in person.e2e-spec.ts and person.service.spec.ts to verify the zero-asset retention rules. --- e2e/src/specs/server/api/person.e2e-spec.ts | 16 +++++- server/src/repositories/person.repository.ts | 53 ++++++++++--------- server/src/services/person.service.spec.ts | 36 +++++++++++++ server/src/services/person.service.ts | 18 ++++++- .../[[assetId=id]]/+page.svelte | 4 ++ 5 files changed, 99 insertions(+), 28 deletions(-) diff --git a/e2e/src/specs/server/api/person.e2e-spec.ts b/e2e/src/specs/server/api/person.e2e-spec.ts index 20290cd941..91a2101093 100644 --- a/e2e/src/specs/server/api/person.e2e-spec.ts +++ b/e2e/src/specs/server/api/person.e2e-spec.ts @@ -134,6 +134,7 @@ describe('/people', () => { expect.objectContaining({ name: 'visible_person' }), expect.objectContaining({ id: nameNullPerson4Assets.id, name: '' }), expect.objectContaining({ id: nameNullPerson3Assets.id, name: '' }), + expect.objectContaining({ id: nameNullPerson1Asset.id, name: '' }), expect.objectContaining({ name: 'hidden_person' }), // Should really be before the null names ], }); @@ -159,6 +160,7 @@ describe('/people', () => { visiblePerson.id, // name: 'visible_person', count: 1 nameNullPerson4Assets.id, // name: '', count: 4 nameNullPerson3Assets.id, // name: '', count: 3 + nameNullPerson1Asset.id, // name: '', count: 1 ]); expect(people.some((p) => p.id === hiddenPerson.id)).toBe(false); @@ -182,10 +184,22 @@ describe('/people', () => { expect.objectContaining({ name: 'visible_person' }), expect.objectContaining({ id: nameNullPerson4Assets.id, name: '' }), expect.objectContaining({ id: nameNullPerson3Assets.id, name: '' }), + expect.objectContaining({ id: nameNullPerson1Asset.id, name: '' }), ], }); }); + it('should include named people even when they have no assets', async () => { + const namedWithoutAssets = await utils.createPerson(admin.accessToken, { + name: 'named_without_assets', + }); + + const { status, body } = await request(app).get('/people').set('Authorization', `Bearer ${admin.accessToken}`); + + expect(status).toBe(200); + expect((body.people as PersonResponseDto[]).some((person) => person.id === namedWithoutAssets.id)).toBe(true); + }); + it('should support pagination', async () => { const { status, body } = await request(app) .get('/people') @@ -195,7 +209,7 @@ describe('/people', () => { expect(status).toBe(200); expect(body).toEqual({ hasNextPage: true, - total: 11, + total: 12, hidden: 1, people: [expect.objectContaining({ name: 'Alice' })], }); diff --git a/server/src/repositories/person.repository.ts b/server/src/repositories/person.repository.ts index 2a9f822e94..eb6a99cbfa 100644 --- a/server/src/repositories/person.repository.ts +++ b/server/src/repositories/person.repository.ts @@ -153,24 +153,22 @@ export class PersonRepository { const items = await this.db .selectFrom('person') .selectAll('person') - .innerJoin('asset_face', 'asset_face.personId', 'person.id') - .innerJoin('asset', (join) => + .leftJoin('asset_face', (join) => + join + .onRef('asset_face.personId', '=', 'person.id') + .on('asset_face.deletedAt', 'is', null) + .on('asset_face.isVisible', 'is', true), + ) + .leftJoin('asset', (join) => join .onRef('asset_face.assetId', '=', 'asset.id') .on('asset.visibility', '=', sql.lit(AssetVisibility.Timeline)) .on('asset.deletedAt', 'is', null), ) .where('person.ownerId', '=', userId) - .where('asset_face.deletedAt', 'is', null) - .where('asset_face.isVisible', 'is', true) .orderBy('person.isHidden', 'asc') .orderBy('person.isFavorite', 'desc') - .having((eb) => - eb.or([ - eb('person.name', '!=', ''), - eb((innerEb) => innerEb.fn.count('asset_face.assetId'), '>=', options?.minimumFaceCount || 1), - ]), - ) + .having((eb) => eb.or([eb('person.name', '!=', ''), eb(eb.fn.count('asset.id').distinct(), '>=', 1)])) .groupBy('person.id') .$if(!!options?.closestFaceAssetId, (qb) => qb.orderBy((eb) => @@ -192,7 +190,7 @@ export class PersonRepository { .$if(!options?.closestFaceAssetId, (qb) => qb .orderBy(sql`NULLIF(person.name, '') is null`, 'asc') - .orderBy((eb) => eb.fn.count('asset_face.assetId'), 'desc') + .orderBy((eb) => eb.fn.count('asset.id').distinct(), 'desc') .orderBy(sql`NULLIF(person.name, '')`, (om) => om.asc().nullsLast()) .orderBy('person.createdAt'), ) @@ -361,22 +359,25 @@ export class PersonRepository { return this.db .selectFrom('person') .where((eb) => - eb.exists((eb) => - eb - .selectFrom('asset_face') - .whereRef('asset_face.personId', '=', 'person.id') - .where('asset_face.deletedAt', 'is', null) - .where('asset_face.isVisible', '=', true) - .where((eb) => - eb.exists((eb) => - eb - .selectFrom('asset') - .whereRef('asset.id', '=', 'asset_face.assetId') - .where('asset.visibility', '=', sql.lit(AssetVisibility.Timeline)) - .where('asset.deletedAt', 'is', null), + eb.or([ + eb('person.name', '!=', ''), + eb.exists((eb) => + eb + .selectFrom('asset_face') + .whereRef('asset_face.personId', '=', 'person.id') + .where('asset_face.deletedAt', 'is', null) + .where('asset_face.isVisible', '=', true) + .where((eb) => + eb.exists((eb) => + eb + .selectFrom('asset') + .whereRef('asset.id', '=', 'asset_face.assetId') + .where('asset.visibility', '=', sql.lit(AssetVisibility.Timeline)) + .where('asset.deletedAt', 'is', null), + ), ), - ), - ), + ), + ]), ) .where('person.ownerId', '=', userId) .select((eb) => eb.fn.coalesce(eb.fn.countAll(), zero).as('total')) diff --git a/server/src/services/person.service.spec.ts b/server/src/services/person.service.spec.ts index 8b303d04f6..f33e402db1 100644 --- a/server/src/services/person.service.spec.ts +++ b/server/src/services/person.service.spec.ts @@ -525,6 +525,42 @@ describe(PersonService.name, () => { }); }); + describe('deleteFace', () => { + it('should delete an unnamed person when deleting their last face', async () => { + const auth = AuthFactory.create(); + const face = AssetFaceFactory.from().person({ name: '' }).build(); + + mocks.access.person.checkFaceOwnerAccess.mockResolvedValue(new Set([face.id])); + mocks.person.getFaceById.mockResolvedValue(getForAssetFace(face)); + mocks.person.softDeleteAssetFaces.mockResolvedValue(); + mocks.person.getStatistics.mockResolvedValue({ assets: 0 }); + + await expect(sut.deleteFace(auth, face.id, { force: false })).resolves.toBeUndefined(); + + expect(mocks.person.softDeleteAssetFaces).toHaveBeenCalledWith(face.id); + expect(mocks.person.getStatistics).toHaveBeenCalledWith(face.person!.id); + expect(mocks.person.delete).toHaveBeenCalledWith([face.person!.id]); + expect(mocks.storage.unlink).toHaveBeenCalledWith(face.person!.thumbnailPath); + }); + + it('should keep a named person when deleting their last face', async () => { + const auth = AuthFactory.create(); + const face = AssetFaceFactory.from().person({ name: 'Alice' }).build(); + + mocks.access.person.checkFaceOwnerAccess.mockResolvedValue(new Set([face.id])); + mocks.person.getFaceById.mockResolvedValue(getForAssetFace(face)); + mocks.person.softDeleteAssetFaces.mockResolvedValue(); + mocks.person.getStatistics.mockResolvedValue({ assets: 0 }); + + await expect(sut.deleteFace(auth, face.id, { force: false })).resolves.toBeUndefined(); + + expect(mocks.person.softDeleteAssetFaces).toHaveBeenCalledWith(face.id); + expect(mocks.person.getStatistics).not.toHaveBeenCalled(); + expect(mocks.person.delete).not.toHaveBeenCalled(); + expect(mocks.storage.unlink).not.toHaveBeenCalled(); + }); + }); + describe('handlePersonCleanup', () => { it('should delete people without faces', async () => { const person = PersonFactory.create(); diff --git a/server/src/services/person.service.ts b/server/src/services/person.service.ts index fde5313f4d..aea3ef2066 100644 --- a/server/src/services/person.service.ts +++ b/server/src/services/person.service.ts @@ -702,6 +702,22 @@ export class PersonService extends BaseService { async deleteFace(auth: AuthDto, id: string, dto: AssetFaceDeleteDto): Promise { await this.requireAccess({ auth, permission: Permission.FaceDelete, ids: [id] }); - return dto.force ? this.personRepository.deleteAssetFace(id) : this.personRepository.softDeleteAssetFaces(id); + const face = await this.personRepository.getFaceById(id); + + if (!face) { + return; + } + + await (dto.force ? this.personRepository.deleteAssetFace(id) : this.personRepository.softDeleteAssetFaces(id)); + + const person = face.person; + if (!person || person.name) { + return; + } + + const { assets } = await this.personRepository.getStatistics(person.id); + if (assets === 0) { + await this.removeAllPeople([{ id: person.id, thumbnailPath: person.thumbnailPath }]); + } } } diff --git a/web/src/routes/(user)/people/[personId]/[[photos=photos]]/[[assetId=id]]/+page.svelte b/web/src/routes/(user)/people/[personId]/[[photos=photos]]/[[assetId=id]]/+page.svelte index c32f0bce70..a9c025c52f 100644 --- a/web/src/routes/(user)/people/[personId]/[[photos=photos]]/[[assetId=id]]/+page.svelte +++ b/web/src/routes/(user)/people/[personId]/[[photos=photos]]/[[assetId=id]]/+page.svelte @@ -300,6 +300,10 @@ return; } timelineManager.removeAssets([assetId]); + if (!person.name && numberOfAssets <= 1) { + await goto(Route.viewAsset({ id: assetId }), { replaceState: true }); + return; + } await updateAssetCount(); }; From aab4950f11d845ba6e60b4f1abfd04c038289807 Mon Sep 17 00:00:00 2001 From: Hugo de Sousa Pereira Date: Fri, 27 Mar 2026 23:29:11 +0000 Subject: [PATCH 02/10] Revert "Fix #26168: Allow assigning zero-asset named people to new faces" This reverts commit 8cae3894ff4bc0ffaf9674187862f90a944eee46. --- e2e/src/specs/server/api/person.e2e-spec.ts | 16 +----- server/src/repositories/person.repository.ts | 53 +++++++++---------- server/src/services/person.service.spec.ts | 36 ------------- server/src/services/person.service.ts | 18 +------ .../[[assetId=id]]/+page.svelte | 4 -- 5 files changed, 28 insertions(+), 99 deletions(-) diff --git a/e2e/src/specs/server/api/person.e2e-spec.ts b/e2e/src/specs/server/api/person.e2e-spec.ts index 91a2101093..20290cd941 100644 --- a/e2e/src/specs/server/api/person.e2e-spec.ts +++ b/e2e/src/specs/server/api/person.e2e-spec.ts @@ -134,7 +134,6 @@ describe('/people', () => { expect.objectContaining({ name: 'visible_person' }), expect.objectContaining({ id: nameNullPerson4Assets.id, name: '' }), expect.objectContaining({ id: nameNullPerson3Assets.id, name: '' }), - expect.objectContaining({ id: nameNullPerson1Asset.id, name: '' }), expect.objectContaining({ name: 'hidden_person' }), // Should really be before the null names ], }); @@ -160,7 +159,6 @@ describe('/people', () => { visiblePerson.id, // name: 'visible_person', count: 1 nameNullPerson4Assets.id, // name: '', count: 4 nameNullPerson3Assets.id, // name: '', count: 3 - nameNullPerson1Asset.id, // name: '', count: 1 ]); expect(people.some((p) => p.id === hiddenPerson.id)).toBe(false); @@ -184,22 +182,10 @@ describe('/people', () => { expect.objectContaining({ name: 'visible_person' }), expect.objectContaining({ id: nameNullPerson4Assets.id, name: '' }), expect.objectContaining({ id: nameNullPerson3Assets.id, name: '' }), - expect.objectContaining({ id: nameNullPerson1Asset.id, name: '' }), ], }); }); - it('should include named people even when they have no assets', async () => { - const namedWithoutAssets = await utils.createPerson(admin.accessToken, { - name: 'named_without_assets', - }); - - const { status, body } = await request(app).get('/people').set('Authorization', `Bearer ${admin.accessToken}`); - - expect(status).toBe(200); - expect((body.people as PersonResponseDto[]).some((person) => person.id === namedWithoutAssets.id)).toBe(true); - }); - it('should support pagination', async () => { const { status, body } = await request(app) .get('/people') @@ -209,7 +195,7 @@ describe('/people', () => { expect(status).toBe(200); expect(body).toEqual({ hasNextPage: true, - total: 12, + total: 11, hidden: 1, people: [expect.objectContaining({ name: 'Alice' })], }); diff --git a/server/src/repositories/person.repository.ts b/server/src/repositories/person.repository.ts index eb6a99cbfa..2a9f822e94 100644 --- a/server/src/repositories/person.repository.ts +++ b/server/src/repositories/person.repository.ts @@ -153,22 +153,24 @@ export class PersonRepository { const items = await this.db .selectFrom('person') .selectAll('person') - .leftJoin('asset_face', (join) => - join - .onRef('asset_face.personId', '=', 'person.id') - .on('asset_face.deletedAt', 'is', null) - .on('asset_face.isVisible', 'is', true), - ) - .leftJoin('asset', (join) => + .innerJoin('asset_face', 'asset_face.personId', 'person.id') + .innerJoin('asset', (join) => join .onRef('asset_face.assetId', '=', 'asset.id') .on('asset.visibility', '=', sql.lit(AssetVisibility.Timeline)) .on('asset.deletedAt', 'is', null), ) .where('person.ownerId', '=', userId) + .where('asset_face.deletedAt', 'is', null) + .where('asset_face.isVisible', 'is', true) .orderBy('person.isHidden', 'asc') .orderBy('person.isFavorite', 'desc') - .having((eb) => eb.or([eb('person.name', '!=', ''), eb(eb.fn.count('asset.id').distinct(), '>=', 1)])) + .having((eb) => + eb.or([ + eb('person.name', '!=', ''), + eb((innerEb) => innerEb.fn.count('asset_face.assetId'), '>=', options?.minimumFaceCount || 1), + ]), + ) .groupBy('person.id') .$if(!!options?.closestFaceAssetId, (qb) => qb.orderBy((eb) => @@ -190,7 +192,7 @@ export class PersonRepository { .$if(!options?.closestFaceAssetId, (qb) => qb .orderBy(sql`NULLIF(person.name, '') is null`, 'asc') - .orderBy((eb) => eb.fn.count('asset.id').distinct(), 'desc') + .orderBy((eb) => eb.fn.count('asset_face.assetId'), 'desc') .orderBy(sql`NULLIF(person.name, '')`, (om) => om.asc().nullsLast()) .orderBy('person.createdAt'), ) @@ -359,25 +361,22 @@ export class PersonRepository { return this.db .selectFrom('person') .where((eb) => - eb.or([ - eb('person.name', '!=', ''), - eb.exists((eb) => - eb - .selectFrom('asset_face') - .whereRef('asset_face.personId', '=', 'person.id') - .where('asset_face.deletedAt', 'is', null) - .where('asset_face.isVisible', '=', true) - .where((eb) => - eb.exists((eb) => - eb - .selectFrom('asset') - .whereRef('asset.id', '=', 'asset_face.assetId') - .where('asset.visibility', '=', sql.lit(AssetVisibility.Timeline)) - .where('asset.deletedAt', 'is', null), - ), + eb.exists((eb) => + eb + .selectFrom('asset_face') + .whereRef('asset_face.personId', '=', 'person.id') + .where('asset_face.deletedAt', 'is', null) + .where('asset_face.isVisible', '=', true) + .where((eb) => + eb.exists((eb) => + eb + .selectFrom('asset') + .whereRef('asset.id', '=', 'asset_face.assetId') + .where('asset.visibility', '=', sql.lit(AssetVisibility.Timeline)) + .where('asset.deletedAt', 'is', null), ), - ), - ]), + ), + ), ) .where('person.ownerId', '=', userId) .select((eb) => eb.fn.coalesce(eb.fn.countAll(), zero).as('total')) diff --git a/server/src/services/person.service.spec.ts b/server/src/services/person.service.spec.ts index f33e402db1..8b303d04f6 100644 --- a/server/src/services/person.service.spec.ts +++ b/server/src/services/person.service.spec.ts @@ -525,42 +525,6 @@ describe(PersonService.name, () => { }); }); - describe('deleteFace', () => { - it('should delete an unnamed person when deleting their last face', async () => { - const auth = AuthFactory.create(); - const face = AssetFaceFactory.from().person({ name: '' }).build(); - - mocks.access.person.checkFaceOwnerAccess.mockResolvedValue(new Set([face.id])); - mocks.person.getFaceById.mockResolvedValue(getForAssetFace(face)); - mocks.person.softDeleteAssetFaces.mockResolvedValue(); - mocks.person.getStatistics.mockResolvedValue({ assets: 0 }); - - await expect(sut.deleteFace(auth, face.id, { force: false })).resolves.toBeUndefined(); - - expect(mocks.person.softDeleteAssetFaces).toHaveBeenCalledWith(face.id); - expect(mocks.person.getStatistics).toHaveBeenCalledWith(face.person!.id); - expect(mocks.person.delete).toHaveBeenCalledWith([face.person!.id]); - expect(mocks.storage.unlink).toHaveBeenCalledWith(face.person!.thumbnailPath); - }); - - it('should keep a named person when deleting their last face', async () => { - const auth = AuthFactory.create(); - const face = AssetFaceFactory.from().person({ name: 'Alice' }).build(); - - mocks.access.person.checkFaceOwnerAccess.mockResolvedValue(new Set([face.id])); - mocks.person.getFaceById.mockResolvedValue(getForAssetFace(face)); - mocks.person.softDeleteAssetFaces.mockResolvedValue(); - mocks.person.getStatistics.mockResolvedValue({ assets: 0 }); - - await expect(sut.deleteFace(auth, face.id, { force: false })).resolves.toBeUndefined(); - - expect(mocks.person.softDeleteAssetFaces).toHaveBeenCalledWith(face.id); - expect(mocks.person.getStatistics).not.toHaveBeenCalled(); - expect(mocks.person.delete).not.toHaveBeenCalled(); - expect(mocks.storage.unlink).not.toHaveBeenCalled(); - }); - }); - describe('handlePersonCleanup', () => { it('should delete people without faces', async () => { const person = PersonFactory.create(); diff --git a/server/src/services/person.service.ts b/server/src/services/person.service.ts index aea3ef2066..fde5313f4d 100644 --- a/server/src/services/person.service.ts +++ b/server/src/services/person.service.ts @@ -702,22 +702,6 @@ export class PersonService extends BaseService { async deleteFace(auth: AuthDto, id: string, dto: AssetFaceDeleteDto): Promise { await this.requireAccess({ auth, permission: Permission.FaceDelete, ids: [id] }); - const face = await this.personRepository.getFaceById(id); - - if (!face) { - return; - } - - await (dto.force ? this.personRepository.deleteAssetFace(id) : this.personRepository.softDeleteAssetFaces(id)); - - const person = face.person; - if (!person || person.name) { - return; - } - - const { assets } = await this.personRepository.getStatistics(person.id); - if (assets === 0) { - await this.removeAllPeople([{ id: person.id, thumbnailPath: person.thumbnailPath }]); - } + return dto.force ? this.personRepository.deleteAssetFace(id) : this.personRepository.softDeleteAssetFaces(id); } } diff --git a/web/src/routes/(user)/people/[personId]/[[photos=photos]]/[[assetId=id]]/+page.svelte b/web/src/routes/(user)/people/[personId]/[[photos=photos]]/[[assetId=id]]/+page.svelte index a9c025c52f..c32f0bce70 100644 --- a/web/src/routes/(user)/people/[personId]/[[photos=photos]]/[[assetId=id]]/+page.svelte +++ b/web/src/routes/(user)/people/[personId]/[[photos=photos]]/[[assetId=id]]/+page.svelte @@ -300,10 +300,6 @@ return; } timelineManager.removeAssets([assetId]); - if (!person.name && numberOfAssets <= 1) { - await goto(Route.viewAsset({ id: assetId }), { replaceState: true }); - return; - } await updateAssetCount(); }; From bf857a160d7339290c99e4509a100d8feed0f3db Mon Sep 17 00:00:00 2001 From: Hugo de Sousa Pereira Date: Fri, 27 Mar 2026 23:46:26 +0000 Subject: [PATCH 03/10] fix(search): exclude people without faces from name search results --- server/src/services/search.service.spec.ts | 13 +++++++++++++ server/src/services/search.service.ts | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/server/src/services/search.service.spec.ts b/server/src/services/search.service.spec.ts index f1cbccb7ec..2bb2c460b6 100644 --- a/server/src/services/search.service.spec.ts +++ b/server/src/services/search.service.spec.ts @@ -4,6 +4,7 @@ import { SearchSuggestionType } from 'src/dtos/search.dto'; import { SearchService } from 'src/services/search.service'; import { AssetFactory } from 'test/factories/asset.factory'; import { AuthFactory } from 'test/factories/auth.factory'; +import { PersonFactory } from 'test/factories/person.factory'; import { authStub } from 'test/fixtures/auth.stub'; import { getForAsset } from 'test/mappers'; import { newTestService, ServiceMocks } from 'test/utils'; @@ -39,6 +40,18 @@ describe(SearchService.name, () => { expect(mocks.person.getByName).toHaveBeenCalledWith(auth.user.id, name, { withHidden: true }); }); + + it('should exclude people without faces', async () => { + const auth = AuthFactory.create(); + const withFace = PersonFactory.create({ ownerId: auth.user.id, faceAssetId: 'face-id', name: 'Alice' }); + const withoutFace = PersonFactory.create({ ownerId: auth.user.id, faceAssetId: null, name: 'Alina' }); + + mocks.person.getByName.mockResolvedValue([withFace, withoutFace]); + + const result = await sut.searchPerson(auth, { name: 'Ali', withHidden: false }); + + expect(result).toEqual([expect.objectContaining({ id: withFace.id, name: withFace.name })]); + }); }); describe('searchPlaces', () => { diff --git a/server/src/services/search.service.ts b/server/src/services/search.service.ts index 9a6f8321a9..55cd52120d 100644 --- a/server/src/services/search.service.ts +++ b/server/src/services/search.service.ts @@ -30,7 +30,7 @@ export class SearchService extends BaseService { async searchPerson(auth: AuthDto, dto: SearchPeopleDto): Promise { const people = await this.personRepository.getByName(auth.user.id, dto.name, { withHidden: dto.withHidden }); - return people.map((person) => mapPerson(person)); + return people.filter((person) => person.faceAssetId !== null).map((person) => mapPerson(person)); } async searchPlaces(dto: SearchPlacesDto): Promise { From 5be4b0ca40ab3896f9dbc3b983072e00ac6cbe2f Mon Sep 17 00:00:00 2001 From: Hugo de Sousa Pereira Date: Sat, 28 Mar 2026 01:38:17 +0000 Subject: [PATCH 04/10] chore(person): replace asset face existence check with hasFace utility function --- server/src/repositories/person.repository.ts | 22 +++----------------- server/src/utils/database.ts | 19 +++++++++++++++++ 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/server/src/repositories/person.repository.ts b/server/src/repositories/person.repository.ts index 2a9f822e94..b1787bf267 100644 --- a/server/src/repositories/person.repository.ts +++ b/server/src/repositories/person.repository.ts @@ -316,7 +316,8 @@ export class PersonRepository { .selectFrom(['similarity_threshold', 'person']) .selectAll('person') .where('person.ownerId', '=', userId) - .where(() => sql`f_unaccent("person"."name") %> f_unaccent(${personName})`) + .where(hasFace) + .where(() => sql`f_unaccent("person"."name") %>> f_unaccent(${personName})`) .orderBy(sql`f_unaccent("person"."name") <->>> f_unaccent(${personName})`) .limit(100) .$if(!withHidden, (qb) => qb.where('person.isHidden', '=', false)) @@ -360,24 +361,7 @@ export class PersonRepository { const zero = sql.lit(0); return this.db .selectFrom('person') - .where((eb) => - eb.exists((eb) => - eb - .selectFrom('asset_face') - .whereRef('asset_face.personId', '=', 'person.id') - .where('asset_face.deletedAt', 'is', null) - .where('asset_face.isVisible', '=', true) - .where((eb) => - eb.exists((eb) => - eb - .selectFrom('asset') - .whereRef('asset.id', '=', 'asset_face.assetId') - .where('asset.visibility', '=', sql.lit(AssetVisibility.Timeline)) - .where('asset.deletedAt', 'is', null), - ), - ), - ), - ) + .where((eb) => hasFace(eb)) .where('person.ownerId', '=', userId) .select((eb) => eb.fn.coalesce(eb.fn.countAll(), zero).as('total')) .select((eb) => eb.fn.coalesce(eb.fn.countAll().filterWhere('isHidden', '=', true), zero).as('hidden')) diff --git a/server/src/utils/database.ts b/server/src/utils/database.ts index bc530f2b03..516926cae2 100644 --- a/server/src/utils/database.ts +++ b/server/src/utils/database.ts @@ -247,6 +247,25 @@ export function hasPeople(qb: SelectQueryBuilder, personIds: ); } +export function hasFace(eb: ExpressionBuilder) { + return eb.exists((eb) => + eb + .selectFrom('asset_face') + .whereRef('asset_face.personId', '=', 'person.id') + .where('asset_face.deletedAt', 'is', null) + .where('asset_face.isVisible', '=', true) + .where((eb) => + eb.exists((eb) => + eb + .selectFrom('asset') + .whereRef('asset.id', '=', 'asset_face.assetId') + .where('asset.visibility', '=', sql.lit(AssetVisibility.Timeline)) + .where('asset.deletedAt', 'is', null), + ), + ), + ); +} + export function inAlbums(qb: SelectQueryBuilder, albumIds: string[]) { return qb.innerJoin( (eb) => From 65a1584f8f7521677bf86a36296fa00d12d94671 Mon Sep 17 00:00:00 2001 From: Hugo de Sousa Pereira Date: Sat, 28 Mar 2026 13:14:20 +0000 Subject: [PATCH 05/10] chore(sql): sync generated SQL queries --- server/src/queries/person.repository.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/queries/person.repository.sql b/server/src/queries/person.repository.sql index 318c151cca..c52acbe467 100644 --- a/server/src/queries/person.repository.sql +++ b/server/src/queries/person.repository.sql @@ -211,7 +211,7 @@ where order by f_unaccent ("person"."name") <->>> f_unaccent ($3) limit - $4 + $5 -- PersonRepository.getDistinctNames select distinct From dfe3da2e330153717c20a03b28d2bb88098a2d24 Mon Sep 17 00:00:00 2001 From: Hugo de Sousa Pereira Date: Mon, 6 Apr 2026 18:36:12 +0100 Subject: [PATCH 06/10] chore(sql): re-sync generated SQL queries --- server/src/queries/person.repository.sql | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/server/src/queries/person.repository.sql b/server/src/queries/person.repository.sql index c52acbe467..47791942e2 100644 --- a/server/src/queries/person.repository.sql +++ b/server/src/queries/person.repository.sql @@ -207,9 +207,27 @@ from "person" where "person"."ownerId" = $1 - and f_unaccent ("person"."name") %> f_unaccent ($2) + and exists ( + select + from + "asset_face" + where + "asset_face"."personId" = "person"."id" + and "asset_face"."deletedAt" is null + and "asset_face"."isVisible" = $2 + and exists ( + select + from + "asset" + where + "asset"."id" = "asset_face"."assetId" + and "asset"."visibility" = 'timeline' + and "asset"."deletedAt" is null + ) + ) + and f_unaccent ("person"."name") %>> f_unaccent ($3) order by - f_unaccent ("person"."name") <->>> f_unaccent ($3) + f_unaccent ("person"."name") <->>> f_unaccent ($4) limit $5 From 81cc58fa178cfaa6069ab5c9a106d279a9087638 Mon Sep 17 00:00:00 2001 From: Hugo Pereira Date: Sat, 9 May 2026 18:09:30 +0100 Subject: [PATCH 07/10] fix(person): simplify face existence check in person query Co-authored-by: Daniel Dietzler <36593685+danieldietzler@users.noreply.github.com> --- server/src/repositories/person.repository.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/repositories/person.repository.ts b/server/src/repositories/person.repository.ts index b1787bf267..d5e07c677a 100644 --- a/server/src/repositories/person.repository.ts +++ b/server/src/repositories/person.repository.ts @@ -361,7 +361,7 @@ export class PersonRepository { const zero = sql.lit(0); return this.db .selectFrom('person') - .where((eb) => hasFace(eb)) + .where(hasFace) .where('person.ownerId', '=', userId) .select((eb) => eb.fn.coalesce(eb.fn.countAll(), zero).as('total')) .select((eb) => eb.fn.coalesce(eb.fn.countAll().filterWhere('isHidden', '=', true), zero).as('hidden')) From 65e603874ffb3af16492a1d4eef9863a54cd797a Mon Sep 17 00:00:00 2001 From: Hugo de Sousa Pereira Date: Sat, 9 May 2026 18:19:24 +0100 Subject: [PATCH 08/10] chore(person): move hasFace function to person.repository.ts --- server/src/repositories/person.repository.ts | 19 +++++++++++++++++++ server/src/utils/database.ts | 19 ------------------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/server/src/repositories/person.repository.ts b/server/src/repositories/person.repository.ts index d5e07c677a..ffef661442 100644 --- a/server/src/repositories/person.repository.ts +++ b/server/src/repositories/person.repository.ts @@ -12,6 +12,25 @@ import { PersonTable } from 'src/schema/tables/person.table'; import { dummy, removeUndefinedKeys, withFilePath } from 'src/utils/database'; import { paginationHelper, PaginationOptions } from 'src/utils/pagination'; +function hasFace(eb: ExpressionBuilder) { + return eb.exists((eb) => + eb + .selectFrom('asset_face') + .whereRef('asset_face.personId', '=', 'person.id') + .where('asset_face.deletedAt', 'is', null) + .where('asset_face.isVisible', '=', true) + .where((eb) => + eb.exists((eb) => + eb + .selectFrom('asset') + .whereRef('asset.id', '=', 'asset_face.assetId') + .where('asset.visibility', '=', sql.lit(AssetVisibility.Timeline)) + .where('asset.deletedAt', 'is', null), + ), + ), + ); +} + export interface PersonSearchOptions { minimumFaceCount: number; withHidden: boolean; diff --git a/server/src/utils/database.ts b/server/src/utils/database.ts index 516926cae2..bc530f2b03 100644 --- a/server/src/utils/database.ts +++ b/server/src/utils/database.ts @@ -247,25 +247,6 @@ export function hasPeople(qb: SelectQueryBuilder, personIds: ); } -export function hasFace(eb: ExpressionBuilder) { - return eb.exists((eb) => - eb - .selectFrom('asset_face') - .whereRef('asset_face.personId', '=', 'person.id') - .where('asset_face.deletedAt', 'is', null) - .where('asset_face.isVisible', '=', true) - .where((eb) => - eb.exists((eb) => - eb - .selectFrom('asset') - .whereRef('asset.id', '=', 'asset_face.assetId') - .where('asset.visibility', '=', sql.lit(AssetVisibility.Timeline)) - .where('asset.deletedAt', 'is', null), - ), - ), - ); -} - export function inAlbums(qb: SelectQueryBuilder, albumIds: string[]) { return qb.innerJoin( (eb) => From eceaa3b68d43d5cb24992838edd78d79bdca999c Mon Sep 17 00:00:00 2001 From: Hugo de Sousa Pereira Date: Sat, 9 May 2026 20:01:36 +0100 Subject: [PATCH 09/10] fix(person): correct SQL syntax in person name search query --- server/src/queries/person.repository.sql | 2 +- server/src/repositories/person.repository.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/queries/person.repository.sql b/server/src/queries/person.repository.sql index 47791942e2..de5e824a5a 100644 --- a/server/src/queries/person.repository.sql +++ b/server/src/queries/person.repository.sql @@ -225,7 +225,7 @@ where and "asset"."deletedAt" is null ) ) - and f_unaccent ("person"."name") %>> f_unaccent ($3) + and f_unaccent ("person"."name") %> f_unaccent ($3) order by f_unaccent ("person"."name") <->>> f_unaccent ($4) limit diff --git a/server/src/repositories/person.repository.ts b/server/src/repositories/person.repository.ts index ffef661442..86de979fbf 100644 --- a/server/src/repositories/person.repository.ts +++ b/server/src/repositories/person.repository.ts @@ -336,7 +336,7 @@ export class PersonRepository { .selectAll('person') .where('person.ownerId', '=', userId) .where(hasFace) - .where(() => sql`f_unaccent("person"."name") %>> f_unaccent(${personName})`) + .where(() => sql`f_unaccent("person"."name") %> f_unaccent(${personName})`) .orderBy(sql`f_unaccent("person"."name") <->>> f_unaccent(${personName})`) .limit(100) .$if(!withHidden, (qb) => qb.where('person.isHidden', '=', false)) From 3a44f4f8463355bd0e61ae7a5eb29cec17556489 Mon Sep 17 00:00:00 2001 From: Frost-Phoenix Date: Mon, 11 May 2026 11:46:19 +0100 Subject: [PATCH 10/10] fix(search): remove face asset filter from searchPerson method --- server/src/services/search.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/services/search.service.ts b/server/src/services/search.service.ts index 55cd52120d..9a6f8321a9 100644 --- a/server/src/services/search.service.ts +++ b/server/src/services/search.service.ts @@ -30,7 +30,7 @@ export class SearchService extends BaseService { async searchPerson(auth: AuthDto, dto: SearchPeopleDto): Promise { const people = await this.personRepository.getByName(auth.user.id, dto.name, { withHidden: dto.withHidden }); - return people.filter((person) => person.faceAssetId !== null).map((person) => mapPerson(person)); + return people.map((person) => mapPerson(person)); } async searchPlaces(dto: SearchPlacesDto): Promise {