From 661409bac7c79cad5ef137fa27cadae98656abf3 Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Thu, 7 Mar 2024 15:34:57 -0500 Subject: [PATCH] feat(server): create a person with optional values (#7706) * feat: create person dto * chore: open api * fix: e2e * fix: web usage --- e2e/src/api/specs/person.e2e-spec.ts | 70 ++++++++++++------ e2e/src/utils.ts | 14 +--- mobile/openapi/.openapi-generator/FILES | 3 + mobile/openapi/README.md | Bin 25011 -> 25057 bytes mobile/openapi/doc/PersonApi.md | Bin 20867 -> 21092 bytes mobile/openapi/doc/PersonCreateDto.md | Bin 0 -> 638 bytes mobile/openapi/lib/api.dart | Bin 8612 -> 8649 bytes mobile/openapi/lib/api/person_api.dart | Bin 17065 -> 17337 bytes mobile/openapi/lib/api_client.dart | Bin 24184 -> 24266 bytes .../openapi/lib/model/person_create_dto.dart | Bin 0 -> 4361 bytes mobile/openapi/test/person_api_test.dart | Bin 1896 -> 1927 bytes .../openapi/test/person_create_dto_test.dart | Bin 0 -> 905 bytes open-api/immich-openapi-specs.json | 29 ++++++++ open-api/typescript-sdk/src/fetch-client.ts | 20 ++++- server/src/domain/person/person.dto.ts | 21 +++--- .../src/domain/person/person.service.spec.ts | 69 ++--------------- server/src/domain/person/person.service.ts | 36 ++++----- .../immich/controllers/person.controller.ts | 25 ++++--- .../faces-page/person-side-panel.svelte | 2 +- .../faces-page/unmerge-face-selector.svelte | 2 +- 20 files changed, 153 insertions(+), 138 deletions(-) create mode 100644 mobile/openapi/doc/PersonCreateDto.md create mode 100644 mobile/openapi/lib/model/person_create_dto.dart create mode 100644 mobile/openapi/test/person_create_dto_test.dart diff --git a/e2e/src/api/specs/person.e2e-spec.ts b/e2e/src/api/specs/person.e2e-spec.ts index 55cb982f9d..8015580430 100644 --- a/e2e/src/api/specs/person.e2e-spec.ts +++ b/e2e/src/api/specs/person.e2e-spec.ts @@ -5,7 +5,15 @@ import { app, utils } from 'src/utils'; import request from 'supertest'; import { beforeAll, beforeEach, describe, expect, it } from 'vitest'; -describe('/activity', () => { +const invalidBirthday = [ + // TODO: enable after replacing `@Type(() => Date)` + // { birthDate: 'false', response: 'Invalid date' }, + // { birthDate: '123567', response: 'Invalid date }, + // { birthDate: 123_567, response: ['Birth date cannot be in the future'] }, + { birthDate: new Date(9999, 0, 0).toISOString(), response: ['Birth date cannot be in the future'] }, +]; + +describe('/person', () => { let admin: LoginResponseDto; let visiblePerson: PersonResponseDto; let hiddenPerson: PersonResponseDto; @@ -14,10 +22,6 @@ describe('/activity', () => { beforeAll(async () => { await utils.resetDatabase(); admin = await utils.adminSetup(); - }); - - beforeEach(async () => { - await utils.resetDatabase(['person']); [visiblePerson, hiddenPerson, multipleAssetsPerson] = await Promise.all([ utils.createPerson(admin.accessToken, { @@ -141,6 +145,41 @@ describe('/activity', () => { }); }); + describe('POST /person', () => { + it('should require authentication', async () => { + const { status, body } = await request(app).post(`/person`); + expect(status).toBe(401); + expect(body).toEqual(errorDto.unauthorized); + }); + + it('should not accept invalid birth dates', async () => { + for (const { birthDate, response } of invalidBirthday) { + const { status, body } = await request(app) + .post(`/person`) + .set('Authorization', `Bearer ${admin.accessToken}`) + .send({ birthDate }); + expect(status).toBe(400); + expect(body).toEqual(errorDto.badRequest(response)); + } + }); + + it('should create a person', async () => { + const { status, body } = await request(app) + .post(`/person`) + .set('Authorization', `Bearer ${admin.accessToken}`) + .send({ + name: 'New Person', + birthDate: '1990-01-01T05:00:00.000Z', + }); + expect(status).toBe(201); + expect(body).toMatchObject({ + id: expect.any(String), + name: 'New Person', + birthDate: '1990-01-01T05:00:00.000Z', + }); + }); + }); + describe('PUT /person/:id', () => { it('should require authentication', async () => { const { status, body } = await request(app).put(`/person/${uuidDto.notFound}`); @@ -164,17 +203,9 @@ describe('/activity', () => { } it('should not accept invalid birth dates', async () => { - for (const { birthDate, response } of [ - { birthDate: false, response: 'Not found or no person.write access' }, - { birthDate: 'false', response: ['birthDate must be a Date instance'] }, - { - birthDate: '123567', - response: 'Not found or no person.write access', - }, - { birthDate: 123_567, response: 'Not found or no person.write access' }, - ]) { + for (const { birthDate, response } of invalidBirthday) { const { status, body } = await request(app) - .put(`/person/${uuidDto.notFound}`) + .put(`/person/${visiblePerson.id}`) .set('Authorization', `Bearer ${admin.accessToken}`) .send({ birthDate }); expect(status).toBe(400); @@ -192,15 +223,8 @@ describe('/activity', () => { }); it('should clear a date of birth', async () => { - // TODO ironically this uses the update endpoint to create the person - const person = await utils.createPerson(admin.accessToken, { - birthDate: new Date('1990-01-01').toISOString(), - }); - - expect(person.birthDate).toBeDefined(); - const { status, body } = await request(app) - .put(`/person/${person.id}`) + .put(`/person/${visiblePerson.id}`) .set('Authorization', `Bearer ${admin.accessToken}`) .send({ birthDate: null }); expect(status).toBe(200); diff --git a/e2e/src/utils.ts b/e2e/src/utils.ts index 5547a2c128..49ac2b8122 100644 --- a/e2e/src/utils.ts +++ b/e2e/src/utils.ts @@ -5,7 +5,7 @@ import { CreateAssetDto, CreateLibraryDto, CreateUserDto, - PersonUpdateDto, + PersonCreateDto, SharedLinkCreateDto, ValidateLibraryDto, createAlbum, @@ -20,7 +20,6 @@ import { login, setAdminOnboarding, signUpAdmin, - updatePerson, validate, } from '@immich/sdk'; import { BrowserContext } from '@playwright/test'; @@ -252,16 +251,11 @@ export const utils = { deleteAssets: (accessToken: string, ids: string[]) => deleteAssets({ assetBulkDeleteDto: { ids } }, { headers: asBearerAuth(accessToken) }), - createPerson: async (accessToken: string, dto?: PersonUpdateDto) => { - // TODO fix createPerson to accept a body - const person = await createPerson({ headers: asBearerAuth(accessToken) }); + createPerson: async (accessToken: string, dto?: PersonCreateDto) => { + const person = await createPerson({ personCreateDto: dto || {} }, { headers: asBearerAuth(accessToken) }); await utils.setPersonThumbnail(person.id); - if (!dto) { - return person; - } - - return updatePerson({ id: person.id, personUpdateDto: dto }, { headers: asBearerAuth(accessToken) }); + return person; }, createFace: async ({ assetId, personId }: { assetId: string; personId: string }) => { diff --git a/mobile/openapi/.openapi-generator/FILES b/mobile/openapi/.openapi-generator/FILES index 6144510b10..bdd8e1d4bc 100644 --- a/mobile/openapi/.openapi-generator/FILES +++ b/mobile/openapi/.openapi-generator/FILES @@ -104,6 +104,7 @@ doc/PeopleResponseDto.md doc/PeopleUpdateDto.md doc/PeopleUpdateItem.md doc/PersonApi.md +doc/PersonCreateDto.md doc/PersonResponseDto.md doc/PersonStatisticsResponseDto.md doc/PersonUpdateDto.md @@ -306,6 +307,7 @@ lib/model/path_type.dart lib/model/people_response_dto.dart lib/model/people_update_dto.dart lib/model/people_update_item.dart +lib/model/person_create_dto.dart lib/model/person_response_dto.dart lib/model/person_statistics_response_dto.dart lib/model/person_update_dto.dart @@ -485,6 +487,7 @@ test/people_response_dto_test.dart test/people_update_dto_test.dart test/people_update_item_test.dart test/person_api_test.dart +test/person_create_dto_test.dart test/person_response_dto_test.dart test/person_statistics_response_dto_test.dart test/person_update_dto_test.dart diff --git a/mobile/openapi/README.md b/mobile/openapi/README.md index 1d16c60d401438bd0391209b39f5f6ea9f11f9ca..536c671b8df40abcd6c06920e4071a80c6f3673b 100644 GIT binary patch delta 54 zcmdmdnDOCZ#tonRrJajX6H8KEO7dehQu34a^#f9iiu3cpqLT#!geN2JWL6Yc delta 14 WcmaEOm~r!A#tonRH!B1j6a)Y|AqN8h diff --git a/mobile/openapi/doc/PersonApi.md b/mobile/openapi/doc/PersonApi.md index f9e3100186232cd419e4b1e2841df7a32ca5f090..2ade49aec7411a07be63ee39d7baf42fa21f9565 100644 GIT binary patch delta 174 zcmZo(%=lyp;|3uX&VtmU;`}`4qSVb3JhfO1;oQWMRF{%`1zUvxm;#81hNiWGzCJ>9 zGNYa0%ZDZe(z8X>N2N wWN&42AZ~ATAZ}%4WFTQ~c_46Ma$#*{bY*fblYc-Kldn+ClWr2~v$+eaMseUF8UO$Q diff --git a/mobile/openapi/doc/PersonCreateDto.md b/mobile/openapi/doc/PersonCreateDto.md new file mode 100644 index 0000000000000000000000000000000000000000..427c23382e5370d58fa1d60dfb2f3aecb332634d GIT binary patch literal 638 zcma)3!D<3A5WVLs20W+>yWZ`|c7+zINb5}&jGL&T*-S{rg3yoeByOp-hc=hYWZs)M zFEar2+DUJ!4wbgmN{5Kz{~bc9<1mGSrwaQ~md*eG}HAwHFmChkSdkR52vS)~z zGA5HQEeOG#tgVtRW>eBOz%!ZnXq3_}kHjM)+8p-!?y?6WhihIc z-inFM=AIV?Jj)_&7OaWXG^47Tyv%3yV$RkF3foi1He~wva*v_RD~@CMX3thn|K#$* TOYM|LdOmWWP%Z!bPMl|_;h Z?zqkOES@q>-XOGVvXrUk=6H55YXJKBP&5Dl delta 81 zcmV-X0IvVJhXJXD0kHA`lY8ZwN{|2HqZp diff --git a/mobile/openapi/lib/api_client.dart b/mobile/openapi/lib/api_client.dart index d73b505937a2f201322e6f968d365a32cc299b7b..5e5f702996d9a5a9e8b8dfd5132126bd2f7f437e 100644 GIT binary patch delta 32 ocmeydhw;>2#tl6dlixeaat5Rp73b$U7o|>~Ag8vu&Ek_P0QLC}8vp)Gh(@5iUdpI@IHAJNIX561}|jp%fAMjuB< z?@s>Off!4^%7t-*=jpG{d;F*_N^Rt1rp;ug3VA?PSy`D+WFZ%_bV+!)6^<@NQAz4oR+Sgt$fJ7Y;87F7 zUP5!dl9$pXUgScS(qL^lNs6xYS8S_U`U}oWuoXayh&X7@gsZ#w%rCn zPoBUsh4oE~fb~s9-js>_V3Fqg4J#ZxuWsFhDji**C(MPNzu^KH41EAipXc^o%1l}- z%$nOX`p8^bWxZIx>0;gkDufQ{869k*-ASYAJ^Cf+G+}a38 zjBg!NJE_xOUDn^j{9tBXlPu7vLwrDayJd!@mcJQdObFc-VHEHI`au`p-0we1dA&Q` zdm`NBnin^2`RRU_jeOUg@e|%>hAmvj!TZq3Pom5z9Dgh+XTy}TG&6bud)%zYesJNi zFoVD@A11~i)H!&@6!$G;rEJ~d>m%0zCYqe#5aZ83rpnNF?1Veg1jiw#vI$O3bDeW( zh8NxTa_B?w6s;*6LW?|OCWeWM)`WQCd%`qsG1-fxnXv8_r~BcU!c42P*eSWb=M%3t zTt1+MSjMAApQ|OIHr4rQcQ2fYSTRz?>;1_GfYIy>Gx;S?%wc07inUc4@>^=5iZ_a$ z7mdGIH$LuqPmwU58Fi{ZD5Rsa*&!8pdji;RM~qgQ+oLOJ8uVP$e1lO9rT zq~&uC+DF&+_&m^?L1T+NPB;k62T$Ep*;DO~7fZK}CV?YHRll~ZS9~yG)y;ZDIrZDsvi?|(4@>;X+zL% z$=z%*$F>3y_a2f0sdlUGT4}fJm~(i&H|1V#5Foj$X3!HhMINs}6Wl^@+2Tu8n6@|I z!&|eC0H*w;q>Y>5q=yat6i9C;^PaA`3#K<0pH|cY}5Nbwgxma=vDy@S)Nz6@*<&! z6)0@98MEEfzv0aTDe(|`)iu^C{-3f3+q#Kx2Zog4X%g2WQPR}src#rJSzF2+o?3+1 z$L&!z*vDT72sE>}Bne>5H?*nbS!m&wD?rfuFQZew4b{Ea4@h*K@YBK{tlONwhF6fo I$qS9+A6+tfp8x;= literal 0 HcmV?d00001 diff --git a/mobile/openapi/test/person_api_test.dart b/mobile/openapi/test/person_api_test.dart index dd112eeaaee34310b5d830ae341fe0f8aca2f327..959230cc590ae167675199db6ac851dcf7ac0b77 100644 GIT binary patch delta 45 rcmaFC*UrD;4WmpzYEf~1o^w%ZVo9n?Nxnh>5`VG*tJG#@rc_n{mrD>w delta 16 XcmZqYf5Erm4ddjA%;KAsm{M5*IAaB= diff --git a/mobile/openapi/test/person_create_dto_test.dart b/mobile/openapi/test/person_create_dto_test.dart new file mode 100644 index 0000000000000000000000000000000000000000..96f1fe6d39e0d79c392352c51d9891a4cfe49ec3 GIT binary patch literal 905 zcmbVKO>fjN5WVMD45!^n%LaNvttwHGEmVRHf^g``^(IbsL}Huqq$ooD?~I);LhVXL z9O7u4H}7L6d7kAtTz;#{m+v>*&6mq+Q^3{r$IS|g61HUppUUF;>gI*OBJypci7!sq zuTQgBVrLy0)jCow_T~&mI|kDz)0@FqUL9&2ydyr^q5GeD3d)bH$ov?Qq}}R4POnMc zY$ymhh2!nDvw^i`g^dVei-uBrI4ahoH9_^xb^_zQw(89*Rqr*0ESo*T!t46G?#!9< z#%hnbJnNWxasC9tCU72)10agVI)ID>K8tl{-;vR53U=9@!v+S8c6kE7G5`U$Xmt;m zGSmhx9``Iy&ND%F("/person", { + }>("/person", oazapfts.json({ ...opts, - method: "POST" - })); + method: "POST", + body: personCreateDto + }))); } export function updatePeople({ peopleUpdateDto }: { peopleUpdateDto: PeopleUpdateDto; diff --git a/server/src/domain/person/person.dto.ts b/server/src/domain/person/person.dto.ts index b8ad8f0451..e76ce3308e 100644 --- a/server/src/domain/person/person.dto.ts +++ b/server/src/domain/person/person.dto.ts @@ -1,11 +1,11 @@ import { AssetFaceEntity, PersonEntity } from '@app/infra/entities'; import { ApiProperty } from '@nestjs/swagger'; import { Transform, Type } from 'class-transformer'; -import { IsArray, IsBoolean, IsDate, IsNotEmpty, IsString, ValidateNested } from 'class-validator'; +import { IsArray, IsBoolean, IsDate, IsNotEmpty, IsString, MaxDate, ValidateNested } from 'class-validator'; import { AuthDto } from '../auth'; import { Optional, ValidateUUID, toBoolean } from '../domain.util'; -export class PersonUpdateDto { +export class PersonCreateDto { /** * Person name. */ @@ -20,16 +20,10 @@ export class PersonUpdateDto { @Optional({ nullable: true }) @IsDate() @Type(() => Date) + @MaxDate(() => new Date(), { message: 'Birth date cannot be in the future' }) @ApiProperty({ format: 'date' }) birthDate?: Date | null; - /** - * Asset is used to get the feature face thumbnail. - */ - @Optional() - @IsString() - featureFaceAssetId?: string; - /** * Person visibility */ @@ -38,6 +32,15 @@ export class PersonUpdateDto { isHidden?: boolean; } +export class PersonUpdateDto extends PersonCreateDto { + /** + * Asset is used to get the feature face thumbnail. + */ + @Optional() + @IsString() + featureFaceAssetId?: string; +} + export class PeopleUpdateDto { @IsArray() @ValidateNested({ each: true }) diff --git a/server/src/domain/person/person.service.spec.ts b/server/src/domain/person/person.service.spec.ts index deb7474e2a..191356d2c0 100644 --- a/server/src/domain/person/person.service.spec.ts +++ b/server/src/domain/person/person.service.spec.ts @@ -227,8 +227,7 @@ describe(PersonService.name, () => { }); it('should throw an error when personId is invalid', async () => { - personMock.getById.mockResolvedValue(null); - accessMock.person.checkOwnerAccess.mockResolvedValue(new Set(['person-1'])); + accessMock.person.checkOwnerAccess.mockResolvedValue(new Set()); await expect(sut.update(authStub.admin, 'person-1', { name: 'Person 1' })).rejects.toBeInstanceOf( BadRequestException, ); @@ -237,20 +236,17 @@ describe(PersonService.name, () => { }); it("should update a person's name", async () => { - personMock.getById.mockResolvedValue(personStub.noName); personMock.update.mockResolvedValue(personStub.withName); personMock.getAssets.mockResolvedValue([assetStub.image]); accessMock.person.checkOwnerAccess.mockResolvedValue(new Set(['person-1'])); await expect(sut.update(authStub.admin, 'person-1', { name: 'Person 1' })).resolves.toEqual(responseDto); - expect(personMock.getById).toHaveBeenCalledWith('person-1'); expect(personMock.update).toHaveBeenCalledWith({ id: 'person-1', name: 'Person 1' }); expect(accessMock.person.checkOwnerAccess).toHaveBeenCalledWith(authStub.admin.user.id, new Set(['person-1'])); }); it("should update a person's date of birth", async () => { - personMock.getById.mockResolvedValue(personStub.noBirthDate); personMock.update.mockResolvedValue(personStub.withBirthDate); personMock.getAssets.mockResolvedValue([assetStub.image]); accessMock.person.checkOwnerAccess.mockResolvedValue(new Set(['person-1'])); @@ -262,71 +258,24 @@ describe(PersonService.name, () => { thumbnailPath: '/path/to/thumbnail.jpg', isHidden: false, }); - expect(personMock.getById).toHaveBeenCalledWith('person-1'); expect(personMock.update).toHaveBeenCalledWith({ id: 'person-1', birthDate: new Date('1976-06-30') }); expect(jobMock.queue).not.toHaveBeenCalled(); expect(jobMock.queueAll).not.toHaveBeenCalled(); expect(accessMock.person.checkOwnerAccess).toHaveBeenCalledWith(authStub.admin.user.id, new Set(['person-1'])); }); - it('should throw BadRequestException if birthDate is in the future', async () => { - personMock.getById.mockResolvedValue(personStub.noBirthDate); - personMock.update.mockResolvedValue(personStub.withBirthDate); - personMock.getAssets.mockResolvedValue([assetStub.image]); - accessMock.person.checkOwnerAccess.mockResolvedValue(new Set(['person-1'])); - - const futureDate = new Date(); - futureDate.setMinutes(futureDate.getMinutes() + 1); // Set birthDate to one minute in the future - - await expect(sut.update(authStub.admin, 'person-1', { birthDate: futureDate })).rejects.toThrow( - new BadRequestException('Date of birth cannot be in the future'), - ); - }); - - it('should not throw an error if birthdate is in the past', async () => { - const pastDate = new Date(); - pastDate.setMinutes(pastDate.getMinutes() - 1); // Set birthDate to one minute in the past - - personMock.getById.mockResolvedValue(personStub.noBirthDate); - personMock.update.mockResolvedValue({ ...personStub.withBirthDate, birthDate: pastDate }); - personMock.getAssets.mockResolvedValue([assetStub.image]); - accessMock.person.checkOwnerAccess.mockResolvedValue(new Set(['person-1'])); - - const result = await sut.update(authStub.admin, 'person-1', { birthDate: pastDate }); - - expect(result.birthDate).toEqual(pastDate); - expect(personMock.update).toHaveBeenCalledWith({ id: 'person-1', birthDate: pastDate }); - }); - - it('should not throw an error if birthdate is today', async () => { - const today = new Date(); // Set birthDate to now() - - personMock.getById.mockResolvedValue(personStub.noBirthDate); - personMock.update.mockResolvedValue({ ...personStub.withBirthDate, birthDate: today }); - personMock.getAssets.mockResolvedValue([assetStub.image]); - accessMock.person.checkOwnerAccess.mockResolvedValue(new Set(['person-1'])); - - const result = await sut.update(authStub.admin, 'person-1', { birthDate: today }); - - expect(result.birthDate).toEqual(today); - expect(personMock.update).toHaveBeenCalledWith({ id: 'person-1', birthDate: today }); - }); - it('should update a person visibility', async () => { - personMock.getById.mockResolvedValue(personStub.hidden); personMock.update.mockResolvedValue(personStub.withName); personMock.getAssets.mockResolvedValue([assetStub.image]); accessMock.person.checkOwnerAccess.mockResolvedValue(new Set(['person-1'])); await expect(sut.update(authStub.admin, 'person-1', { isHidden: false })).resolves.toEqual(responseDto); - expect(personMock.getById).toHaveBeenCalledWith('person-1'); expect(personMock.update).toHaveBeenCalledWith({ id: 'person-1', isHidden: false }); expect(accessMock.person.checkOwnerAccess).toHaveBeenCalledWith(authStub.admin.user.id, new Set(['person-1'])); }); it("should update a person's thumbnailPath", async () => { - personMock.getById.mockResolvedValue(personStub.withName); personMock.update.mockResolvedValue(personStub.withName); personMock.getFacesByIds.mockResolvedValue([faceStub.face1]); accessMock.asset.checkOwnerAccess.mockResolvedValue(new Set([assetStub.image.id])); @@ -336,7 +285,6 @@ describe(PersonService.name, () => { sut.update(authStub.admin, 'person-1', { featureFaceAssetId: faceStub.face1.assetId }), ).resolves.toEqual(responseDto); - expect(personMock.getById).toHaveBeenCalledWith('person-1'); expect(personMock.update).toHaveBeenCalledWith({ id: 'person-1', faceAssetId: faceStub.face1.id }); expect(personMock.getFacesByIds).toHaveBeenCalledWith([ { @@ -362,12 +310,11 @@ describe(PersonService.name, () => { describe('updateAll', () => { it('should throw an error when personId is invalid', async () => { - personMock.getById.mockResolvedValue(null); - accessMock.person.checkOwnerAccess.mockResolvedValue(new Set(['person-1'])); + accessMock.person.checkOwnerAccess.mockResolvedValue(new Set()); - await expect( - sut.updatePeople(authStub.admin, { people: [{ id: 'person-1', name: 'Person 1' }] }), - ).resolves.toEqual([{ error: BulkIdErrorReason.UNKNOWN, id: 'person-1', success: false }]); + await expect(sut.updateAll(authStub.admin, { people: [{ id: 'person-1', name: 'Person 1' }] })).resolves.toEqual([ + { error: BulkIdErrorReason.UNKNOWN, id: 'person-1', success: false }, + ]); expect(personMock.update).not.toHaveBeenCalled(); expect(accessMock.person.checkOwnerAccess).toHaveBeenCalledWith(authStub.admin.user.id, new Set(['person-1'])); }); @@ -488,10 +435,10 @@ describe(PersonService.name, () => { describe('createPerson', () => { it('should create a new person', async () => { personMock.create.mockResolvedValue(personStub.primaryPerson); - personMock.getFaceById.mockResolvedValue(faceStub.face1); - accessMock.person.checkFaceOwnerAccess.mockResolvedValue(new Set([faceStub.face1.id])); - await expect(sut.createPerson(authStub.admin)).resolves.toBe(personStub.primaryPerson); + await expect(sut.create(authStub.admin, {})).resolves.toBe(personStub.primaryPerson); + + expect(personMock.create).toHaveBeenCalledWith({ ownerId: authStub.admin.user.id }); }); }); diff --git a/server/src/domain/person/person.service.ts b/server/src/domain/person/person.service.ts index 6300cc743c..235867314e 100644 --- a/server/src/domain/person/person.service.ts +++ b/server/src/domain/person/person.service.ts @@ -36,6 +36,7 @@ import { MergePersonDto, PeopleResponseDto, PeopleUpdateDto, + PersonCreateDto, PersonResponseDto, PersonSearchDto, PersonStatisticsResponseDto, @@ -91,10 +92,6 @@ export class PersonService { }; } - createPerson(auth: AuthDto): Promise { - return this.repository.create({ ownerId: auth.user.id }); - } - async reassignFaces(auth: AuthDto, personId: string, dto: AssetFaceUpdateDto): Promise { await this.access.requirePermission(auth, Permission.PERSON_WRITE, personId); const person = await this.findOrFail(personId); @@ -199,21 +196,21 @@ export class PersonService { return assets.map((asset) => mapAsset(asset)); } + create(auth: AuthDto, dto: PersonCreateDto): Promise { + return this.repository.create({ + ownerId: auth.user.id, + name: dto.name, + birthDate: dto.birthDate, + isHidden: dto.isHidden, + }); + } + async update(auth: AuthDto, id: string, dto: PersonUpdateDto): Promise { await this.access.requirePermission(auth, Permission.PERSON_WRITE, id); - let person = await this.findOrFail(id); const { name, birthDate, isHidden, featureFaceAssetId: assetId } = dto; - - // Check if the birthDate is in the future - if (birthDate && new Date(birthDate) > new Date()) { - throw new BadRequestException('Date of birth cannot be in the future'); - } - - if (name !== undefined || birthDate !== undefined || isHidden !== undefined) { - person = await this.repository.update({ id, name, birthDate, isHidden }); - } - + // TODO: set by faceId directly + let faceId: string | undefined = undefined; if (assetId) { await this.access.requirePermission(auth, Permission.ASSET_READ, assetId); const [face] = await this.repository.getFacesByIds([{ personId: id, assetId }]); @@ -221,14 +218,19 @@ export class PersonService { throw new BadRequestException('Invalid assetId for feature face'); } - person = await this.repository.update({ id, faceAssetId: face.id }); + faceId = face.id; + } + + const person = await this.repository.update({ id, faceAssetId: faceId, name, birthDate, isHidden }); + + if (assetId) { await this.jobRepository.queue({ name: JobName.GENERATE_PERSON_THUMBNAIL, data: { id } }); } return mapPerson(person); } - async updatePeople(auth: AuthDto, dto: PeopleUpdateDto): Promise { + async updateAll(auth: AuthDto, dto: PeopleUpdateDto): Promise { const results: BulkIdResponseDto[] = []; for (const person of dto.people) { try { diff --git a/server/src/immich/controllers/person.controller.ts b/server/src/immich/controllers/person.controller.ts index 6582d4461f..2447f982b4 100644 --- a/server/src/immich/controllers/person.controller.ts +++ b/server/src/immich/controllers/person.controller.ts @@ -6,6 +6,7 @@ import { MergePersonDto, PeopleResponseDto, PeopleUpdateDto, + PersonCreateDto, PersonResponseDto, PersonSearchDto, PersonService, @@ -32,22 +33,13 @@ export class PersonController { } @Post() - createPerson(@Auth() auth: AuthDto): Promise { - return this.service.createPerson(auth); - } - - @Put(':id/reassign') - reassignFaces( - @Auth() auth: AuthDto, - @Param() { id }: UUIDParamDto, - @Body() dto: AssetFaceUpdateDto, - ): Promise { - return this.service.reassignFaces(auth, id, dto); + createPerson(@Auth() auth: AuthDto, @Body() dto: PersonCreateDto): Promise { + return this.service.create(auth, dto); } @Put() updatePeople(@Auth() auth: AuthDto, @Body() dto: PeopleUpdateDto): Promise { - return this.service.updatePeople(auth, dto); + return this.service.updateAll(auth, dto); } @Get(':id') @@ -85,6 +77,15 @@ export class PersonController { return this.service.getAssets(auth, id); } + @Put(':id/reassign') + reassignFaces( + @Auth() auth: AuthDto, + @Param() { id }: UUIDParamDto, + @Body() dto: AssetFaceUpdateDto, + ): Promise { + return this.service.reassignFaces(auth, id, dto); + } + @Post(':id/merge') mergePerson( @Auth() auth: AuthDto, diff --git a/web/src/lib/components/faces-page/person-side-panel.svelte b/web/src/lib/components/faces-page/person-side-panel.svelte index 4b596b3601..3cb705a1f3 100644 --- a/web/src/lib/components/faces-page/person-side-panel.svelte +++ b/web/src/lib/components/faces-page/person-side-panel.svelte @@ -122,7 +122,7 @@ faceDto: { id: peopleWithFace.id }, }); } else if (selectedPersonToCreate[index]) { - const data = await createPerson(); + const data = await createPerson({ personCreateDto: {} }); numberOfPersonToCreate.push(data.id); await reassignFacesById({ id: data.id, diff --git a/web/src/lib/components/faces-page/unmerge-face-selector.svelte b/web/src/lib/components/faces-page/unmerge-face-selector.svelte index 254594988d..590cc916ee 100644 --- a/web/src/lib/components/faces-page/unmerge-face-selector.svelte +++ b/web/src/lib/components/faces-page/unmerge-face-selector.svelte @@ -74,7 +74,7 @@ try { disableButtons = true; - const data = await createPerson(); + const data = await createPerson({ personCreateDto: {} }); await reassignFaces({ id: data.id, assetFaceUpdateDto: { data: selectedPeople } }); notificationController.show({