From 23014c263b7ded7828c721fd36bfdcf3160fa847 Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Fri, 7 Feb 2025 10:06:58 -0500 Subject: [PATCH] feat(api): set person color (#15937) --- e2e/src/api/specs/person.e2e-spec.ts | 19 ++++++++++++++++ .../openapi/lib/model/people_update_item.dart | Bin 6195 -> 6485 bytes .../openapi/lib/model/person_create_dto.dart | Bin 5108 -> 5398 bytes .../lib/model/person_response_dto.dart | Bin 5459 -> 6140 bytes .../openapi/lib/model/person_update_dto.dart | Bin 5984 -> 6274 bytes .../model/person_with_faces_response_dto.dart | Bin 5919 -> 6600 bytes open-api/immich-openapi-specs.json | 21 +++++++++++++++++- open-api/typescript-sdk/src/fetch-client.ts | 7 ++++++ server/src/db.d.ts | 1 + server/src/dtos/person.dto.ts | 16 ++++++++++++- server/src/dtos/tag.dto.ts | 8 +++---- server/src/entities/person.entity.ts | 3 +++ .../1738889177573-AddPersonColor.ts | 14 ++++++++++++ server/src/services/person.service.spec.ts | 4 ++-- server/src/services/person.service.ts | 12 ++++++---- server/src/services/search.service.spec.ts | 2 ++ server/src/services/search.service.ts | 7 +++--- server/src/validation.ts | 10 +++++++++ 18 files changed, 108 insertions(+), 16 deletions(-) create mode 100644 server/src/migrations/1738889177573-AddPersonColor.ts diff --git a/e2e/src/api/specs/person.e2e-spec.ts b/e2e/src/api/specs/person.e2e-spec.ts index f3e053069..1a589da1f 100644 --- a/e2e/src/api/specs/person.e2e-spec.ts +++ b/e2e/src/api/specs/person.e2e-spec.ts @@ -195,6 +195,7 @@ describe('/people', () => { .send({ name: 'New Person', birthDate: '1990-01-01', + color: '#333', }); expect(status).toBe(201); expect(body).toMatchObject({ @@ -273,6 +274,24 @@ describe('/people', () => { expect(body).toMatchObject({ birthDate: null }); }); + it('should set a color', async () => { + const { status, body } = await request(app) + .put(`/people/${visiblePerson.id}`) + .set('Authorization', `Bearer ${admin.accessToken}`) + .send({ color: '#555' }); + expect(status).toBe(200); + expect(body).toMatchObject({ color: '#555' }); + }); + + it('should clear a color', async () => { + const { status, body } = await request(app) + .put(`/people/${visiblePerson.id}`) + .set('Authorization', `Bearer ${admin.accessToken}`) + .send({ color: null }); + expect(status).toBe(200); + expect(body.color).toBeUndefined(); + }); + it('should mark a person as favorite', async () => { const person = await utils.createPerson(admin.accessToken, { name: 'visible_person', diff --git a/mobile/openapi/lib/model/people_update_item.dart b/mobile/openapi/lib/model/people_update_item.dart index 6f8e312959ac3b45811e3b23224a236ae05d6801..ce324b859eb79b015823b7a18d7d038df15408ec 100644 GIT binary patch delta 180 zcmdmNaMftTTt?R9{G9xv$&(mg^9Gj`W#*;ZD}Y5dS1>(g;{!7lY;B=p{+z~)l5nxS z(wrOxdj$gpD~R-DLvG>COm0nU9x^1L0zpPE-A{)OSe}53vUi!l4aooGZk!Y zpB2dVhIlcvz-bZ0h1yG60_P1NddE#4cr5>sS()* DlrRwL diff --git a/mobile/openapi/lib/model/person_with_faces_response_dto.dart b/mobile/openapi/lib/model/person_with_faces_response_dto.dart index 7d61db11f373ff63bb35271561a7fe295abf52f7..0bd38b087063e087871230cabd120e0da8d63334 100644 GIT binary patch delta 196 zcmbQQcfxqX5k}VJ{G9xv$vYV@O4>W8}+BQ_v{M$Sj7K pHrbm;QUolk9;;xhfFx1REeCPj<_BDFn4#{J; faceAssetId: string | null; id: Generated; diff --git a/server/src/dtos/person.dto.ts b/server/src/dtos/person.dto.ts index 8bf041be3..ca705154a 100644 --- a/server/src/dtos/person.dto.ts +++ b/server/src/dtos/person.dto.ts @@ -7,7 +7,14 @@ import { AuthDto } from 'src/dtos/auth.dto'; import { AssetFaceEntity } from 'src/entities/asset-face.entity'; import { PersonEntity } from 'src/entities/person.entity'; import { SourceType } from 'src/enum'; -import { IsDateStringFormat, MaxDateString, Optional, ValidateBoolean, ValidateUUID } from 'src/validation'; +import { + IsDateStringFormat, + MaxDateString, + Optional, + ValidateBoolean, + ValidateHexColor, + ValidateUUID, +} from 'src/validation'; export class PersonCreateDto { /** @@ -35,6 +42,10 @@ export class PersonCreateDto { @ValidateBoolean({ optional: true }) isFavorite?: boolean; + + @Optional({ emptyToNull: true, nullable: true }) + @ValidateHexColor() + color?: string | null; } export class PersonUpdateDto extends PersonCreateDto { @@ -102,6 +113,8 @@ export class PersonResponseDto { updatedAt?: Date; @PropertyLifecycle({ addedAt: 'v1.126.0' }) isFavorite?: boolean; + @PropertyLifecycle({ addedAt: 'v1.126.0' }) + color?: string; } export class PersonWithFacesResponseDto extends PersonResponseDto { @@ -176,6 +189,7 @@ export function mapPerson(person: PersonEntity): PersonResponseDto { thumbnailPath: person.thumbnailPath, isHidden: person.isHidden, isFavorite: person.isFavorite, + color: person.color ?? undefined, updatedAt: person.updatedAt, }; } diff --git a/server/src/dtos/tag.dto.ts b/server/src/dtos/tag.dto.ts index cff11962d..17200a887 100644 --- a/server/src/dtos/tag.dto.ts +++ b/server/src/dtos/tag.dto.ts @@ -1,8 +1,7 @@ import { ApiProperty } from '@nestjs/swagger'; -import { Transform } from 'class-transformer'; import { IsHexColor, IsNotEmpty, IsString } from 'class-validator'; import { TagEntity } from 'src/entities/tag.entity'; -import { Optional, ValidateUUID } from 'src/validation'; +import { Optional, ValidateHexColor, ValidateUUID } from 'src/validation'; export class TagCreateDto { @IsString() @@ -18,9 +17,8 @@ export class TagCreateDto { } export class TagUpdateDto { - @Optional({ nullable: true, emptyToNull: true }) - @IsHexColor() - @Transform(({ value }) => (typeof value === 'string' && value[0] !== '#' ? `#${value}` : value)) + @Optional({ emptyToNull: true, nullable: true }) + @ValidateHexColor() color?: string | null; } diff --git a/server/src/entities/person.entity.ts b/server/src/entities/person.entity.ts index 8cf416b76..3785e1985 100644 --- a/server/src/entities/person.entity.ts +++ b/server/src/entities/person.entity.ts @@ -52,4 +52,7 @@ export class PersonEntity { @Column({ default: false }) isFavorite!: boolean; + + @Column({ type: 'varchar', nullable: true, default: null }) + color?: string | null; } diff --git a/server/src/migrations/1738889177573-AddPersonColor.ts b/server/src/migrations/1738889177573-AddPersonColor.ts new file mode 100644 index 000000000..ebdc86f52 --- /dev/null +++ b/server/src/migrations/1738889177573-AddPersonColor.ts @@ -0,0 +1,14 @@ +import { MigrationInterface, QueryRunner } from "typeorm"; + +export class AddPersonColor1738889177573 implements MigrationInterface { + name = 'AddPersonColor1738889177573' + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query(`ALTER TABLE "person" ADD "color" character varying`); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query(`ALTER TABLE "person" DROP COLUMN "color"`); + } + +} diff --git a/server/src/services/person.service.spec.ts b/server/src/services/person.service.spec.ts index 5407821fa..1cd1b34ec 100644 --- a/server/src/services/person.service.spec.ts +++ b/server/src/services/person.service.spec.ts @@ -355,7 +355,7 @@ describe(PersonService.name, () => { sut.reassignFaces(authStub.admin, personStub.noName.id, { data: [{ personId: personStub.withName.id, assetId: assetStub.image.id }], }), - ).resolves.toEqual([personStub.noName]); + ).resolves.toBeDefined(); expect(jobMock.queueAll).toHaveBeenCalledWith([ { @@ -448,7 +448,7 @@ describe(PersonService.name, () => { it('should create a new person', async () => { personMock.create.mockResolvedValue(personStub.primaryPerson); - await expect(sut.create(authStub.admin, {})).resolves.toBe(personStub.primaryPerson); + await expect(sut.create(authStub.admin, {})).resolves.toBeDefined(); expect(personMock.create).toHaveBeenCalledWith({ ownerId: authStub.admin.user.id }); }); diff --git a/server/src/services/person.service.ts b/server/src/services/person.service.ts index 2f4a6bb0d..116d2ec6c 100644 --- a/server/src/services/person.service.ts +++ b/server/src/services/person.service.ts @@ -104,7 +104,7 @@ export class PersonService extends BaseService { await this.personRepository.reassignFace(face.id, personId); } - result.push(person); + result.push(mapPerson(person)); } if (changeFeaturePhoto.length > 0) { // Remove duplicates @@ -178,20 +178,23 @@ export class PersonService extends BaseService { }); } - create(auth: AuthDto, dto: PersonCreateDto): Promise { - return this.personRepository.create({ + async create(auth: AuthDto, dto: PersonCreateDto): Promise { + const person = await this.personRepository.create({ ownerId: auth.user.id, name: dto.name, birthDate: dto.birthDate, isHidden: dto.isHidden, isFavorite: dto.isFavorite, + color: dto.color, }); + + return mapPerson(person); } async update(auth: AuthDto, id: string, dto: PersonUpdateDto): Promise { await this.requireAccess({ auth, permission: Permission.PERSON_UPDATE, ids: [id] }); - const { name, birthDate, isHidden, featureFaceAssetId: assetId, isFavorite } = dto; + const { name, birthDate, isHidden, featureFaceAssetId: assetId, isFavorite, color } = dto; // TODO: set by faceId directly let faceId: string | undefined = undefined; if (assetId) { @@ -211,6 +214,7 @@ export class PersonService extends BaseService { birthDate, isHidden, isFavorite, + color, }); if (assetId) { diff --git a/server/src/services/search.service.spec.ts b/server/src/services/search.service.spec.ts index 5c59e24b2..9f16ddf82 100644 --- a/server/src/services/search.service.spec.ts +++ b/server/src/services/search.service.spec.ts @@ -31,6 +31,8 @@ describe(SearchService.name, () => { it('should pass options to search', async () => { const { name } = personStub.withName; + personMock.getByName.mockResolvedValue([]); + await sut.searchPerson(authStub.user1, { name, withHidden: false }); expect(personMock.getByName).toHaveBeenCalledWith(authStub.user1.user.id, name, { withHidden: false }); diff --git a/server/src/services/search.service.ts b/server/src/services/search.service.ts index b833d0184..b74d3d3cb 100644 --- a/server/src/services/search.service.ts +++ b/server/src/services/search.service.ts @@ -1,8 +1,9 @@ import { BadRequestException, Injectable } from '@nestjs/common'; import { AssetMapOptions, AssetResponseDto, mapAsset } from 'src/dtos/asset-response.dto'; import { AuthDto } from 'src/dtos/auth.dto'; -import { PersonResponseDto } from 'src/dtos/person.dto'; +import { mapPerson, PersonResponseDto } from 'src/dtos/person.dto'; import { + mapPlaces, MetadataSearchDto, PlacesResponseDto, RandomSearchDto, @@ -12,7 +13,6 @@ import { SearchSuggestionRequestDto, SearchSuggestionType, SmartSearchDto, - mapPlaces, } from 'src/dtos/search.dto'; import { AssetEntity } from 'src/entities/asset.entity'; import { AssetOrder } from 'src/enum'; @@ -24,7 +24,8 @@ import { isSmartSearchEnabled } from 'src/utils/misc'; @Injectable() export class SearchService extends BaseService { async searchPerson(auth: AuthDto, dto: SearchPeopleDto): Promise { - return this.personRepository.getByName(auth.user.id, dto.name, { withHidden: dto.withHidden }); + const people = await this.personRepository.getByName(auth.user.id, dto.name, { withHidden: dto.withHidden }); + return people.map((person) => mapPerson(person)); } async searchPlaces(dto: SearchPlacesDto): Promise { diff --git a/server/src/validation.ts b/server/src/validation.ts index 177e43991..29e402826 100644 --- a/server/src/validation.ts +++ b/server/src/validation.ts @@ -12,6 +12,7 @@ import { IsArray, IsBoolean, IsDate, + IsHexColor, IsNotEmpty, IsOptional, IsString, @@ -97,6 +98,15 @@ export function Optional({ nullable, emptyToNull, ...validationOptions }: Option return applyDecorators(...decorators); } +export const ValidateHexColor = () => { + const decorators = [ + IsHexColor(), + Transform(({ value }) => (typeof value === 'string' && value[0] !== '#' ? `#${value}` : value)), + ]; + + return applyDecorators(...decorators); +}; + type UUIDOptions = { optional?: boolean; each?: boolean; nullable?: boolean }; export const ValidateUUID = (options?: UUIDOptions) => { const { optional, each, nullable } = { optional: false, each: false, nullable: false, ...options };