feat: people merge improvements (#31456)
This commit is contained in:
@@ -9984,6 +9984,65 @@
|
||||
"x-immich-state": "Stable"
|
||||
}
|
||||
},
|
||||
"/people/merge": {
|
||||
"post": {
|
||||
"description": "Merge an ordered list of people together into a single person. The final name and birth date are always the first defined value, following the order. Also automatically merges people for other users in the cluster group, skipping people that would result in overriding a previously set name or birth date.",
|
||||
"operationId": "mergePeople",
|
||||
"parameters": [],
|
||||
"requestBody": {
|
||||
"content": {
|
||||
"application/json": {
|
||||
"schema": {
|
||||
"$ref": "#/components/schemas/MergePersonDto"
|
||||
}
|
||||
}
|
||||
},
|
||||
"required": true
|
||||
},
|
||||
"responses": {
|
||||
"200": {
|
||||
"content": {
|
||||
"application/json": {
|
||||
"schema": {
|
||||
"items": {
|
||||
"$ref": "#/components/schemas/BulkIdResponseDto"
|
||||
},
|
||||
"type": "array"
|
||||
}
|
||||
}
|
||||
},
|
||||
"description": ""
|
||||
}
|
||||
},
|
||||
"security": [
|
||||
{
|
||||
"bearer": []
|
||||
},
|
||||
{
|
||||
"cookie": []
|
||||
},
|
||||
{
|
||||
"api_key": []
|
||||
}
|
||||
],
|
||||
"summary": "Merge people",
|
||||
"tags": [
|
||||
"People"
|
||||
],
|
||||
"x-immich-history": [
|
||||
{
|
||||
"version": "v3.2.1",
|
||||
"state": "Added"
|
||||
},
|
||||
{
|
||||
"version": "v3.2.1",
|
||||
"state": "Stable"
|
||||
}
|
||||
],
|
||||
"x-immich-permission": "person.merge",
|
||||
"x-immich-state": "Stable"
|
||||
}
|
||||
},
|
||||
"/people/{id}": {
|
||||
"delete": {
|
||||
"description": "Delete an individual person.",
|
||||
@@ -10175,8 +10234,9 @@
|
||||
},
|
||||
"/people/{id}/merge": {
|
||||
"post": {
|
||||
"deprecated": true,
|
||||
"description": "Merge a list of people into the person specified in the path parameter.",
|
||||
"operationId": "mergePerson",
|
||||
"operationId": "mergePersonLegacy",
|
||||
"parameters": [
|
||||
{
|
||||
"name": "id",
|
||||
@@ -10227,7 +10287,8 @@
|
||||
],
|
||||
"summary": "Merge people",
|
||||
"tags": [
|
||||
"People"
|
||||
"People",
|
||||
"Deprecated"
|
||||
],
|
||||
"x-immich-history": [
|
||||
{
|
||||
@@ -10241,10 +10302,15 @@
|
||||
{
|
||||
"version": "v2",
|
||||
"state": "Stable"
|
||||
},
|
||||
{
|
||||
"version": "v3.2.1",
|
||||
"state": "Deprecated",
|
||||
"replacementId": "mergePeople"
|
||||
}
|
||||
],
|
||||
"x-immich-permission": "person.merge",
|
||||
"x-immich-state": "Stable"
|
||||
"x-immich-state": "Deprecated"
|
||||
}
|
||||
},
|
||||
"/people/{id}/reassign": {
|
||||
|
||||
@@ -2001,6 +2001,10 @@ export type PeopleUpdateDto = {
|
||||
/** People to update */
|
||||
people: PeopleUpdateItem[];
|
||||
};
|
||||
export type MergePersonDto = {
|
||||
/** Person IDs to merge */
|
||||
ids: string[];
|
||||
};
|
||||
export type PersonUpdateDto = {
|
||||
/** Person date of birth */
|
||||
birthDate?: string | null;
|
||||
@@ -2015,10 +2019,6 @@ export type PersonUpdateDto = {
|
||||
/** Person name */
|
||||
name?: string;
|
||||
};
|
||||
export type MergePersonDto = {
|
||||
/** Person IDs to merge */
|
||||
ids: string[];
|
||||
};
|
||||
export type AssetFaceUpdateItem = {
|
||||
/** Asset ID */
|
||||
assetId: string;
|
||||
@@ -6041,6 +6041,21 @@ export function updatePeople({ peopleUpdateDto }: {
|
||||
body: peopleUpdateDto
|
||||
})));
|
||||
}
|
||||
/**
|
||||
* Merge people
|
||||
*/
|
||||
export function mergePeople({ mergePersonDto }: {
|
||||
mergePersonDto: MergePersonDto;
|
||||
}, opts?: Oazapfts.RequestOpts) {
|
||||
return oazapfts.ok(oazapfts.fetchJson<{
|
||||
status: 200;
|
||||
data: BulkIdResponseDto[];
|
||||
}>("/people/merge", oazapfts.json({
|
||||
...opts,
|
||||
method: "POST",
|
||||
body: mergePersonDto
|
||||
})));
|
||||
}
|
||||
/**
|
||||
* Delete person
|
||||
*/
|
||||
@@ -6084,7 +6099,7 @@ export function updatePerson({ id, personUpdateDto }: {
|
||||
/**
|
||||
* Merge people
|
||||
*/
|
||||
export function mergePerson({ id, mergePersonDto }: {
|
||||
export function mergePersonLegacy({ id, mergePersonDto }: {
|
||||
id: string;
|
||||
mergePersonDto: MergePersonDto;
|
||||
}, opts?: Oazapfts.RequestOpts) {
|
||||
|
||||
@@ -187,19 +187,36 @@ export class PersonController {
|
||||
return this.service.reassignFaces(auth, id, dto);
|
||||
}
|
||||
|
||||
@Post('merge')
|
||||
@Authenticated({ permission: Permission.PersonMerge })
|
||||
@HttpCode(HttpStatus.OK)
|
||||
@Endpoint({
|
||||
summary: 'Merge people',
|
||||
description:
|
||||
'Merge an ordered list of people together into a single person. The final name and birth date are always the first defined value, following the order. Also automatically merges people for other users in the cluster group, skipping people that would result in overriding a previously set name or birth date.',
|
||||
history: new HistoryBuilder().added('v3.2.1').stable('v3.2.1'),
|
||||
})
|
||||
mergePeople(@Auth() auth: AuthDto, @Body() dto: MergePersonDto): Promise<BulkIdResponseDto[]> {
|
||||
return this.service.mergePeople(auth, dto);
|
||||
}
|
||||
|
||||
@Post(':id/merge')
|
||||
@Authenticated({ permission: Permission.PersonMerge })
|
||||
@HttpCode(HttpStatus.OK)
|
||||
@Endpoint({
|
||||
summary: 'Merge people',
|
||||
description: 'Merge a list of people into the person specified in the path parameter.',
|
||||
history: new HistoryBuilder().added('v1').beta('v1').stable('v2'),
|
||||
history: new HistoryBuilder()
|
||||
.added('v1')
|
||||
.beta('v1')
|
||||
.stable('v2')
|
||||
.deprecated('v3.2.1', { replacementId: 'mergePeople' }),
|
||||
})
|
||||
mergePerson(
|
||||
mergePersonLegacy(
|
||||
@Auth() auth: AuthDto,
|
||||
@Param() { id }: UUIDParamDto,
|
||||
@Body() dto: MergePersonDto,
|
||||
): Promise<BulkIdResponseDto[]> {
|
||||
return this.service.mergePerson(auth, id, dto);
|
||||
return this.service.mergePeople(auth, { ids: [id, ...dto.ids] });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { BadRequestException, Injectable, NotFoundException } from '@nestjs/common';
|
||||
import { Insertable, Selectable, Updateable } from 'kysely';
|
||||
import { isUndefined, omitBy } from 'lodash-es';
|
||||
import type { JobItem, JobOf } from 'src/types.js';
|
||||
import { Person } from 'src/database.js';
|
||||
import { Chunked, OnJob } from 'src/decorators.js';
|
||||
@@ -578,82 +579,88 @@ export class PersonService extends BaseService {
|
||||
return JobStatus.Success;
|
||||
}
|
||||
|
||||
async mergePerson(auth: AuthDto, personGroupId: string, dto: MergePersonDto): Promise<BulkIdResponseDto[]> {
|
||||
const mergeIds = dto.ids;
|
||||
if (mergeIds.includes(personGroupId)) {
|
||||
async mergePeople(auth: AuthDto, { ids }: MergePersonDto): Promise<BulkIdResponseDto[]> {
|
||||
if (ids.length < 2) {
|
||||
throw new BadRequestException('At least two people are required for merging');
|
||||
}
|
||||
|
||||
if (new Set(ids).size !== ids.length) {
|
||||
throw new BadRequestException('Cannot merge a person into themselves');
|
||||
}
|
||||
|
||||
await this.requireAccess({ auth, permission: Permission.PersonUpdate, ids: [personGroupId] });
|
||||
|
||||
const results: BulkIdResponseDto[] = [];
|
||||
|
||||
const allowedIds = await this.checkAccess({
|
||||
auth,
|
||||
permission: Permission.PersonMerge,
|
||||
ids: mergeIds,
|
||||
});
|
||||
const allowedIds = await this.checkAccess({ auth, permission: Permission.PersonMerge, ids });
|
||||
|
||||
let primaryPerson: Selectable<PersonTable> | undefined;
|
||||
const peopleMap: Record<string, Selectable<PersonTable>[]> = {};
|
||||
|
||||
for (const mergePerson of await this.personRepository.getForMergePerson(mergeIds)) {
|
||||
const mergeId = mergePerson.personGroupId;
|
||||
for (const mergePerson of await this.personRepository.getForMergePerson(ids)) {
|
||||
if (!peopleMap[mergePerson.personGroupId]) {
|
||||
peopleMap[mergePerson.personGroupId] = [];
|
||||
}
|
||||
peopleMap[mergePerson.personGroupId].push(mergePerson);
|
||||
}
|
||||
|
||||
const targetPeople: Record<string, Selectable<PersonTable>> = {};
|
||||
for (const mergeId of ids) {
|
||||
const hasAccess = allowedIds.has(mergeId);
|
||||
if (!hasAccess) {
|
||||
results.push({ id: mergeId, success: false, error: BulkIdErrorReason.NO_PERMISSION });
|
||||
continue;
|
||||
}
|
||||
|
||||
if (!primaryPerson || primaryPerson.ownerId !== mergePerson.ownerId) {
|
||||
primaryPerson = await this.personRepository.getByGroupId({ ownerId: mergePerson.ownerId, personGroupId });
|
||||
if (!primaryPerson) {
|
||||
for (const mergePerson of peopleMap[mergeId]) {
|
||||
if (!targetPeople[mergePerson.ownerId]) {
|
||||
targetPeople[mergePerson.ownerId] = mergePerson;
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
const changes: Updateable<Person> = {};
|
||||
if (!primaryPerson.name && mergePerson.name) {
|
||||
changes.name = mergePerson.name;
|
||||
}
|
||||
const targetPerson = targetPeople[mergePerson.ownerId];
|
||||
|
||||
if (!primaryPerson.birthDate && mergePerson.birthDate) {
|
||||
changes.birthDate = mergePerson.birthDate;
|
||||
}
|
||||
if (
|
||||
mergePerson.ownerId !== auth.user.id &&
|
||||
((targetPerson.name && mergePerson.name) || (targetPerson.birthDate && mergePerson.birthDate))
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (
|
||||
(mergePerson.name && mergePerson.name !== primaryPerson.name) ||
|
||||
(mergePerson.birthDate && mergePerson.birthDate !== primaryPerson.birthDate)
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
const changes: Updateable<Person> = omitBy(
|
||||
{
|
||||
name: mergePerson.name && !targetPerson.name ? mergePerson.name : undefined,
|
||||
birthDate: mergePerson.birthDate && !targetPerson.birthDate ? mergePerson.birthDate : undefined,
|
||||
},
|
||||
isUndefined,
|
||||
);
|
||||
|
||||
if (Object.keys(changes).length > 0) {
|
||||
primaryPerson = await this.personRepository.update({
|
||||
ownerId: primaryPerson.ownerId,
|
||||
personGroupId: primaryPerson.personGroupId,
|
||||
...changes,
|
||||
});
|
||||
}
|
||||
if (Object.keys(changes).length > 0) {
|
||||
targetPeople[mergePerson.ownerId] = await this.personRepository.update({
|
||||
ownerId: targetPerson.ownerId,
|
||||
personGroupId: targetPerson.personGroupId,
|
||||
...changes,
|
||||
});
|
||||
}
|
||||
|
||||
const mergeName = mergePerson.name || mergePerson.personGroupId;
|
||||
const mergeData: UpdateFacesData = {
|
||||
oldPersonGroupId: mergeId,
|
||||
newPersonGroupId: primaryPerson.personGroupId,
|
||||
ownerId: primaryPerson.ownerId,
|
||||
};
|
||||
this.logger.log(`Merging ${mergeName} into ${primaryPerson.name || primaryPerson.personGroupId}`);
|
||||
const mergeName = mergePerson.name || mergePerson.personGroupId;
|
||||
const mergeData: UpdateFacesData = {
|
||||
oldPersonGroupId: mergeId,
|
||||
newPersonGroupId: targetPerson.personGroupId,
|
||||
ownerId: targetPerson.ownerId,
|
||||
};
|
||||
this.logger.log(`Merging ${mergeName} into ${targetPerson.name || targetPerson.personGroupId}`);
|
||||
|
||||
try {
|
||||
await this.personRepository.reassignFaces(mergeData);
|
||||
await this.removeAllPersonGroups([mergeId], primaryPerson.ownerId);
|
||||
try {
|
||||
await this.personRepository.reassignFaces(mergeData);
|
||||
await this.removeAllPersonGroups([mergeId], targetPerson.ownerId);
|
||||
|
||||
this.logger.log(`Merged ${mergeName} into ${primaryPerson.name || primaryPerson.personGroupId}`);
|
||||
results.push({ id: mergeId, success: true });
|
||||
} catch (error: any) {
|
||||
this.logger.error(`Unable to merge ${mergeId} into ${personGroupId}: ${error}`, error?.stack);
|
||||
results.push({ id: mergeId, success: false, error: BulkIdErrorReason.UNKNOWN });
|
||||
this.logger.log(`Merged ${mergeName} into ${targetPerson.name || targetPerson.personGroupId}`);
|
||||
results.push({ id: mergeId, success: true });
|
||||
} catch (error: any) {
|
||||
this.logger.error(`Unable to merge ${mergeId} into ${targetPerson.personGroupId}: ${error}`, error?.stack);
|
||||
results.push({ id: mergeId, success: false, error: BulkIdErrorReason.UNKNOWN });
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return results;
|
||||
}
|
||||
|
||||
|
||||
@@ -201,13 +201,13 @@ describe(PersonService.name, () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('mergePerson', () => {
|
||||
describe('mergePeople', () => {
|
||||
it('should merge people of multiple users', 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: person1 } = await ctx.newPerson({ ownerId: user1.id, name: undefined });
|
||||
const { person: person2 } = await ctx.newPerson({ ownerId: user1.id });
|
||||
await ctx.newPerson({
|
||||
ownerId: user2.id,
|
||||
@@ -216,6 +216,7 @@ describe(PersonService.name, () => {
|
||||
await ctx.newPerson({
|
||||
ownerId: user2.id,
|
||||
personGroupId: person2.personGroupId,
|
||||
name: undefined,
|
||||
});
|
||||
const { asset } = await ctx.newAsset({ ownerId: user2.id });
|
||||
await ctx.newAssetFace({ assetId: asset.id, personGroupId: person2.personGroupId });
|
||||
@@ -223,7 +224,7 @@ describe(PersonService.name, () => {
|
||||
|
||||
const auth = factory.auth({ user: user1 });
|
||||
|
||||
await sut.mergePerson(auth, person1.personGroupId, { ids: [person2.personGroupId] });
|
||||
await sut.mergePeople(auth, { ids: [person1.personGroupId, 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 })]);
|
||||
@@ -254,7 +255,7 @@ describe(PersonService.name, () => {
|
||||
|
||||
const auth = factory.auth({ user: user1 });
|
||||
|
||||
await sut.mergePerson(auth, person1.personGroupId, { ids: [person2.personGroupId] });
|
||||
await sut.mergePeople(auth, { ids: [person1.personGroupId, 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 })]);
|
||||
@@ -266,7 +267,7 @@ describe(PersonService.name, () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('should skip people with a different birthdate', async () => {
|
||||
it('should skip people with a different birth date', async () => {
|
||||
const { sut, ctx } = setup();
|
||||
const storageMock = ctx.getMock(StorageRepository);
|
||||
const { user: user1 } = await ctx.newUser();
|
||||
@@ -287,7 +288,7 @@ describe(PersonService.name, () => {
|
||||
|
||||
const auth = factory.auth({ user: user1 });
|
||||
|
||||
await sut.mergePerson(auth, person1.personGroupId, { ids: [person2.personGroupId] });
|
||||
await sut.mergePeople(auth, { ids: [person1.personGroupId, 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 })]);
|
||||
@@ -316,7 +317,7 @@ describe(PersonService.name, () => {
|
||||
|
||||
const auth = factory.auth({ user: user1 });
|
||||
|
||||
await sut.mergePerson(auth, person1.personGroupId, { ids: [person2.personGroupId] });
|
||||
await sut.mergePeople(auth, { ids: [person1.personGroupId, 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 })]);
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
<script lang="ts">
|
||||
import { getPeopleThumbnailUrl } from '$lib/utils';
|
||||
import { handleError } from '$lib/utils/handle-error';
|
||||
import { mergePerson, type PersonResponseDto } from '@immich/sdk';
|
||||
import { mergePeople, type PersonResponseDto } from '@immich/sdk';
|
||||
import { FormModal, Icon, IconButton, toastManager } from '@immich/ui';
|
||||
import { mdiArrowLeft, mdiCallMerge, mdiSwapHorizontal } from '@mdi/js';
|
||||
import { onMount, tick } from 'svelte';
|
||||
@@ -35,10 +35,7 @@
|
||||
|
||||
const onSubmit = async () => {
|
||||
try {
|
||||
await mergePerson({
|
||||
id: personToBeMergedInto.id,
|
||||
mergePersonDto: { ids: [personToMerge.id] },
|
||||
});
|
||||
await mergePeople({ mergePersonDto: { ids: [personToBeMergedInto.id, personToMerge.id] } });
|
||||
toastManager.primary($t('merge_people_successfully'));
|
||||
onClose([personToMerge, personToBeMergedInto]);
|
||||
} catch (error) {
|
||||
|
||||
+2
-5
@@ -2,7 +2,7 @@
|
||||
import { goto } from '$app/navigation';
|
||||
import { Route } from '$lib/route';
|
||||
import { handleError } from '$lib/utils/handle-error';
|
||||
import { getAllPeople, getPerson, mergePerson, type PersonResponseDto } from '@immich/sdk';
|
||||
import { getAllPeople, getPerson, mergePeople, type PersonResponseDto } from '@immich/sdk';
|
||||
import { Button, Icon, IconButton, modalManager, toastManager } from '@immich/ui';
|
||||
import { mdiCallMerge, mdiMerge, mdiSwapHorizontal } from '@mdi/js';
|
||||
import { onMount } from 'svelte';
|
||||
@@ -67,10 +67,7 @@
|
||||
}
|
||||
|
||||
try {
|
||||
let results = await mergePerson({
|
||||
id: person.id,
|
||||
mergePersonDto: { ids: selectedPeople.map(({ id }) => id) },
|
||||
});
|
||||
let results = await mergePeople({ mergePersonDto: { ids: [person.id, ...selectedPeople.map(({ id }) => id)] } });
|
||||
const mergedPerson = await getPerson({ id: person.id });
|
||||
const count = results.filter(({ success }) => success).length;
|
||||
toastManager.primary($t('merged_people_count', { values: { count } }));
|
||||
|
||||
Reference in New Issue
Block a user