diff --git a/server/src/queries/person.repository.sql b/server/src/queries/person.repository.sql index 8f07b9180..821041c5c 100644 --- a/server/src/queries/person.repository.sql +++ b/server/src/queries/person.repository.sql @@ -4,8 +4,11 @@ update "asset_face" set "personGroupId" = $1 +from + "asset" where - "asset_face"."personGroupId" = $2 + "asset_face"."assetId" = "asset"."id" + and "asset_face"."personGroupId" = $2 -- PersonRepository.unassignFaces update "asset_face" diff --git a/server/src/repositories/person.repository.ts b/server/src/repositories/person.repository.ts index caa3d79e5..3e6e6d137 100644 --- a/server/src/repositories/person.repository.ts +++ b/server/src/repositories/person.repository.ts @@ -100,17 +100,15 @@ export class PersonRepository { async reassignFaces({ oldPersonGroupId, faceIds, ownerId, newPersonGroupId }: UpdateFacesData): Promise { const result = await this.db .updateTable('asset_face') + .from('asset') + .whereRef('asset_face.assetId', '=', 'asset.id') .set({ personGroupId: newPersonGroupId }) .$if(!!oldPersonGroupId, (qb) => qb.where('asset_face.personGroupId', '=', oldPersonGroupId!)) .$if(!!faceIds, (qb) => qb.where('asset_face.id', 'in', faceIds!)) - .$if(!!ownerId, (qb) => - qb.where('asset_face.personGroupId', 'in', (eb) => - eb.selectFrom('person').select('person.personGroupId').where('person.ownerId', '=', ownerId!), - ), - ) + .$if(!!ownerId, (qb) => qb.where('asset.ownerId', '=', ownerId!)) .executeTakeFirst(); - return Number(result.numChangedRows ?? 0); + return Number(result.numUpdatedRows ?? 0); } @GenerateSql({ params: [{ sourceType: SourceType.MachineLearning, clusterGroupId: DummyValue.UUID }] }) diff --git a/server/src/services/person.service.ts b/server/src/services/person.service.ts index f4740a42b..dcb0c73be 100644 --- a/server/src/services/person.service.ts +++ b/server/src/services/person.service.ts @@ -441,7 +441,6 @@ export class PersonService extends BaseService { const { waiting } = await this.jobRepository.getJobCounts(QueueName.FacialRecognition); if (force) { - console.log('unassigning faces'); await this.personRepository.unassignFaces({ clusterGroupId, sourceType: SourceType.MachineLearning }); await this.handlePersonCleanup(); await this.personRepository.vacuum({ reindexVectors: false }); diff --git a/server/test/medium/specs/services/person.service.spec.ts b/server/test/medium/specs/services/person.service.spec.ts index 860406fa7..2adefe7e7 100644 --- a/server/test/medium/specs/services/person.service.spec.ts +++ b/server/test/medium/specs/services/person.service.spec.ts @@ -182,6 +182,8 @@ describe(PersonService.name, () => { ownerId: user2.id, personGroupId: person2.personGroupId, }); + const { asset } = await ctx.newAsset({ ownerId: user2.id }); + await ctx.newAssetFace({ assetId: asset.id, personGroupId: person2.personGroupId }); storageMock.unlink.mockResolvedValue(); const auth = factory.auth({ user: user1 }); @@ -191,6 +193,9 @@ describe(PersonService.name, () => { const user2People = await Array.fromAsync(ctx.get(PersonRepository).getAll({ ownerId: user2.id })); expect(user1People).toEqual([expect.objectContaining({ personGroupId: person1.personGroupId })]); expect(user2People).toEqual([expect.objectContaining({ personGroupId: person1.personGroupId })]); + await expect(ctx.get(PersonRepository).getFaces(asset.id, { viewingUserId: asset.ownerId })).resolves.toEqual([ + expect.objectContaining({ personGroupId: person1.personGroupId }), + ]); }); it('should skip people with a different name', async () => { @@ -258,6 +263,33 @@ describe(PersonService.name, () => { ]), ); }); + + it('should not merge into person another user does not have', async () => { + const { sut, ctx } = setup(); + const storageMock = ctx.getMock(StorageRepository); + const { user: user1 } = await ctx.newUser(); + const { user: user2 } = await ctx.newUser({ clusterGroupId: user1.clusterGroupId }); + const { person: person1 } = await ctx.newPerson({ ownerId: user1.id }); + const { person: person2 } = await ctx.newPerson({ ownerId: user1.id }); + await ctx.newPerson({ + ownerId: user2.id, + personGroupId: person2.personGroupId, + }); + const { asset } = await ctx.newAsset({ ownerId: user2.id }); + await ctx.newAssetFace({ assetId: asset.id, personGroupId: person2.personGroupId }); + storageMock.unlink.mockResolvedValue(); + + const auth = factory.auth({ user: user1 }); + + await sut.mergePerson(auth, person1.personGroupId, { ids: [person2.personGroupId] }); + const user1People = await Array.fromAsync(ctx.get(PersonRepository).getAll({ ownerId: user1.id })); + const user2People = await Array.fromAsync(ctx.get(PersonRepository).getAll({ ownerId: user2.id })); + expect(user1People).toEqual([expect.objectContaining({ personGroupId: person1.personGroupId })]); + expect(user2People).toEqual([expect.objectContaining({ personGroupId: person2.personGroupId })]); + await expect(ctx.get(PersonRepository).getFaces(asset.id, { viewingUserId: asset.ownerId })).resolves.toEqual([ + expect.objectContaining({ personGroupId: person2.personGroupId }), + ]); + }); }); describe('createFace', () => {