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 1d16c60d40..536c671b8d 100644 --- a/mobile/openapi/README.md +++ b/mobile/openapi/README.md @@ -303,6 +303,7 @@ Class | Method | HTTP request | Description - [PeopleResponseDto](doc//PeopleResponseDto.md) - [PeopleUpdateDto](doc//PeopleUpdateDto.md) - [PeopleUpdateItem](doc//PeopleUpdateItem.md) + - [PersonCreateDto](doc//PersonCreateDto.md) - [PersonResponseDto](doc//PersonResponseDto.md) - [PersonStatisticsResponseDto](doc//PersonStatisticsResponseDto.md) - [PersonUpdateDto](doc//PersonUpdateDto.md) diff --git a/mobile/openapi/doc/PersonApi.md b/mobile/openapi/doc/PersonApi.md index f9e3100186..2ade49aec7 100644 --- a/mobile/openapi/doc/PersonApi.md +++ b/mobile/openapi/doc/PersonApi.md @@ -22,7 +22,7 @@ Method | HTTP request | Description # **createPerson** -> PersonResponseDto createPerson() +> PersonResponseDto createPerson(personCreateDto) @@ -45,9 +45,10 @@ import 'package:openapi/api.dart'; //defaultApiClient.getAuthentication('bearer').setAccessToken(yourTokenGeneratorFunction); final api_instance = PersonApi(); +final personCreateDto = PersonCreateDto(); // PersonCreateDto | try { - final result = api_instance.createPerson(); + final result = api_instance.createPerson(personCreateDto); print(result); } catch (e) { print('Exception when calling PersonApi->createPerson: $e\n'); @@ -55,7 +56,10 @@ try { ``` ### Parameters -This endpoint does not need any parameter. + +Name | Type | Description | Notes +------------- | ------------- | ------------- | ------------- + **personCreateDto** | [**PersonCreateDto**](PersonCreateDto.md)| | ### Return type @@ -67,7 +71,7 @@ This endpoint does not need any parameter. ### HTTP request headers - - **Content-Type**: Not defined + - **Content-Type**: application/json - **Accept**: application/json [[Back to top]](#) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to Model list]](../README.md#documentation-for-models) [[Back to README]](../README.md) diff --git a/mobile/openapi/doc/PersonCreateDto.md b/mobile/openapi/doc/PersonCreateDto.md new file mode 100644 index 0000000000..427c23382e --- /dev/null +++ b/mobile/openapi/doc/PersonCreateDto.md @@ -0,0 +1,17 @@ +# openapi.model.PersonCreateDto + +## Load the model package +```dart +import 'package:openapi/api.dart'; +``` + +## Properties +Name | Type | Description | Notes +------------ | ------------- | ------------- | ------------- +**birthDate** | [**DateTime**](DateTime.md) | Person date of birth. Note: the mobile app cannot currently set the birth date to null. | [optional] +**isHidden** | **bool** | Person visibility | [optional] +**name** | **String** | Person name. | [optional] + +[[Back to Model list]](../README.md#documentation-for-models) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to README]](../README.md) + + diff --git a/mobile/openapi/lib/api.dart b/mobile/openapi/lib/api.dart index 2dfe3a3bee..0a093e4536 100644 --- a/mobile/openapi/lib/api.dart +++ b/mobile/openapi/lib/api.dart @@ -138,6 +138,7 @@ part 'model/path_type.dart'; part 'model/people_response_dto.dart'; part 'model/people_update_dto.dart'; part 'model/people_update_item.dart'; +part 'model/person_create_dto.dart'; part 'model/person_response_dto.dart'; part 'model/person_statistics_response_dto.dart'; part 'model/person_update_dto.dart'; diff --git a/mobile/openapi/lib/api/person_api.dart b/mobile/openapi/lib/api/person_api.dart index 02dae625bb..411c75d715 100644 --- a/mobile/openapi/lib/api/person_api.dart +++ b/mobile/openapi/lib/api/person_api.dart @@ -17,18 +17,21 @@ class PersonApi { final ApiClient apiClient; /// Performs an HTTP 'POST /person' operation and returns the [Response]. - Future createPersonWithHttpInfo() async { + /// Parameters: + /// + /// * [PersonCreateDto] personCreateDto (required): + Future createPersonWithHttpInfo(PersonCreateDto personCreateDto,) async { // ignore: prefer_const_declarations final path = r'/person'; // ignore: prefer_final_locals - Object? postBody; + Object? postBody = personCreateDto; final queryParams = []; final headerParams = {}; final formParams = {}; - const contentTypes = []; + const contentTypes = ['application/json']; return apiClient.invokeAPI( @@ -42,8 +45,11 @@ class PersonApi { ); } - Future createPerson() async { - final response = await createPersonWithHttpInfo(); + /// Parameters: + /// + /// * [PersonCreateDto] personCreateDto (required): + Future createPerson(PersonCreateDto personCreateDto,) async { + final response = await createPersonWithHttpInfo(personCreateDto,); if (response.statusCode >= HttpStatus.badRequest) { throw ApiException(response.statusCode, await _decodeBodyBytes(response)); } diff --git a/mobile/openapi/lib/api_client.dart b/mobile/openapi/lib/api_client.dart index d73b505937..5e5f702996 100644 --- a/mobile/openapi/lib/api_client.dart +++ b/mobile/openapi/lib/api_client.dart @@ -358,6 +358,8 @@ class ApiClient { return PeopleUpdateDto.fromJson(value); case 'PeopleUpdateItem': return PeopleUpdateItem.fromJson(value); + case 'PersonCreateDto': + return PersonCreateDto.fromJson(value); case 'PersonResponseDto': return PersonResponseDto.fromJson(value); case 'PersonStatisticsResponseDto': diff --git a/mobile/openapi/lib/model/person_create_dto.dart b/mobile/openapi/lib/model/person_create_dto.dart new file mode 100644 index 0000000000..4811de3efe --- /dev/null +++ b/mobile/openapi/lib/model/person_create_dto.dart @@ -0,0 +1,138 @@ +// +// AUTO-GENERATED FILE, DO NOT MODIFY! +// +// @dart=2.12 + +// ignore_for_file: unused_element, unused_import +// ignore_for_file: always_put_required_named_parameters_first +// ignore_for_file: constant_identifier_names +// ignore_for_file: lines_longer_than_80_chars + +part of openapi.api; + +class PersonCreateDto { + /// Returns a new [PersonCreateDto] instance. + PersonCreateDto({ + this.birthDate, + this.isHidden, + this.name, + }); + + /// Person date of birth. Note: the mobile app cannot currently set the birth date to null. + DateTime? birthDate; + + /// Person visibility + /// + /// Please note: This property should have been non-nullable! Since the specification file + /// does not include a default value (using the "default:" property), however, the generated + /// source code must fall back to having a nullable type. + /// Consider adding a "default:" property in the specification file to hide this note. + /// + bool? isHidden; + + /// Person name. + /// + /// Please note: This property should have been non-nullable! Since the specification file + /// does not include a default value (using the "default:" property), however, the generated + /// source code must fall back to having a nullable type. + /// Consider adding a "default:" property in the specification file to hide this note. + /// + String? name; + + @override + bool operator ==(Object other) => identical(this, other) || other is PersonCreateDto && + other.birthDate == birthDate && + other.isHidden == isHidden && + other.name == name; + + @override + int get hashCode => + // ignore: unnecessary_parenthesis + (birthDate == null ? 0 : birthDate!.hashCode) + + (isHidden == null ? 0 : isHidden!.hashCode) + + (name == null ? 0 : name!.hashCode); + + @override + String toString() => 'PersonCreateDto[birthDate=$birthDate, isHidden=$isHidden, name=$name]'; + + Map toJson() { + final json = {}; + if (this.birthDate != null) { + json[r'birthDate'] = _dateFormatter.format(this.birthDate!.toUtc()); + } else { + // json[r'birthDate'] = null; + } + if (this.isHidden != null) { + json[r'isHidden'] = this.isHidden; + } else { + // json[r'isHidden'] = null; + } + if (this.name != null) { + json[r'name'] = this.name; + } else { + // json[r'name'] = null; + } + return json; + } + + /// Returns a new [PersonCreateDto] instance and imports its values from + /// [value] if it's a [Map], null otherwise. + // ignore: prefer_constructors_over_static_methods + static PersonCreateDto? fromJson(dynamic value) { + if (value is Map) { + final json = value.cast(); + + return PersonCreateDto( + birthDate: mapDateTime(json, r'birthDate', r''), + isHidden: mapValueOfType(json, r'isHidden'), + name: mapValueOfType(json, r'name'), + ); + } + return null; + } + + static List listFromJson(dynamic json, {bool growable = false,}) { + final result = []; + if (json is List && json.isNotEmpty) { + for (final row in json) { + final value = PersonCreateDto.fromJson(row); + if (value != null) { + result.add(value); + } + } + } + return result.toList(growable: growable); + } + + static Map mapFromJson(dynamic json) { + final map = {}; + if (json is Map && json.isNotEmpty) { + json = json.cast(); // ignore: parameter_assignments + for (final entry in json.entries) { + final value = PersonCreateDto.fromJson(entry.value); + if (value != null) { + map[entry.key] = value; + } + } + } + return map; + } + + // maps a json object with a list of PersonCreateDto-objects as value to a dart map + static Map> mapListFromJson(dynamic json, {bool growable = false,}) { + final map = >{}; + if (json is Map && json.isNotEmpty) { + // ignore: parameter_assignments + json = json.cast(); + for (final entry in json.entries) { + map[entry.key] = PersonCreateDto.listFromJson(entry.value, growable: growable,); + } + } + return map; + } + + /// The list of required keys that must be present in a JSON. + static const requiredKeys = { + }; +} + diff --git a/mobile/openapi/test/person_api_test.dart b/mobile/openapi/test/person_api_test.dart index dd112eeaae..959230cc59 100644 --- a/mobile/openapi/test/person_api_test.dart +++ b/mobile/openapi/test/person_api_test.dart @@ -17,7 +17,7 @@ void main() { // final instance = PersonApi(); group('tests for PersonApi', () { - //Future createPerson() async + //Future createPerson(PersonCreateDto personCreateDto) async test('test createPerson', () async { // TODO }); 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 0000000000..96f1fe6d39 --- /dev/null +++ b/mobile/openapi/test/person_create_dto_test.dart @@ -0,0 +1,40 @@ +// +// AUTO-GENERATED FILE, DO NOT MODIFY! +// +// @dart=2.12 + +// ignore_for_file: unused_element, unused_import +// ignore_for_file: always_put_required_named_parameters_first +// ignore_for_file: constant_identifier_names +// ignore_for_file: lines_longer_than_80_chars + +import 'package:openapi/api.dart'; +import 'package:test/test.dart'; + +// tests for PersonCreateDto +void main() { + // final instance = PersonCreateDto(); + + group('test PersonCreateDto', () { + // Person date of birth. Note: the mobile app cannot currently set the birth date to null. + // DateTime birthDate + test('to test the property `birthDate`', () async { + // TODO + }); + + // Person visibility + // bool isHidden + test('to test the property `isHidden`', () async { + // TODO + }); + + // Person name. + // String name + test('to test the property `name`', () async { + // TODO + }); + + + }); + +} diff --git a/open-api/immich-openapi-specs.json b/open-api/immich-openapi-specs.json index 1baa40179d..bd99b24765 100644 --- a/open-api/immich-openapi-specs.json +++ b/open-api/immich-openapi-specs.json @@ -4047,6 +4047,16 @@ "post": { "operationId": "createPerson", "parameters": [], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/PersonCreateDto" + } + } + }, + "required": true + }, "responses": { "201": { "content": { @@ -8720,6 +8730,25 @@ ], "type": "object" }, + "PersonCreateDto": { + "properties": { + "birthDate": { + "description": "Person date of birth.\nNote: the mobile app cannot currently set the birth date to null.", + "format": "date", + "nullable": true, + "type": "string" + }, + "isHidden": { + "description": "Person visibility", + "type": "boolean" + }, + "name": { + "description": "Person name.", + "type": "string" + } + }, + "type": "object" + }, "PersonResponseDto": { "properties": { "birthDate": { diff --git a/open-api/typescript-sdk/src/fetch-client.ts b/open-api/typescript-sdk/src/fetch-client.ts index 4cb2e74aa5..e9ce467127 100644 --- a/open-api/typescript-sdk/src/fetch-client.ts +++ b/open-api/typescript-sdk/src/fetch-client.ts @@ -529,6 +529,15 @@ export type PeopleResponseDto = { people: PersonResponseDto[]; total: number; }; +export type PersonCreateDto = { + /** Person date of birth. + Note: the mobile app cannot currently set the birth date to null. */ + birthDate?: string | null; + /** Person visibility */ + isHidden?: boolean; + /** Person name. */ + name?: string; +}; export type PeopleUpdateItem = { /** Person date of birth. Note: the mobile app cannot currently set the birth date to null. */ @@ -2051,14 +2060,17 @@ export function getAllPeople({ withHidden }: { ...opts })); } -export function createPerson(opts?: Oazapfts.RequestOpts) { +export function createPerson({ personCreateDto }: { + personCreateDto: PersonCreateDto; +}, opts?: Oazapfts.RequestOpts) { return oazapfts.ok(oazapfts.fetchJson<{ status: 201; data: PersonResponseDto; - }>("/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({