From 234449f3c662b48ab13dead80d27308c21fb53d3 Mon Sep 17 00:00:00 2001 From: Mohamed BOUSSAID Date: Tue, 19 Dec 2023 17:07:38 +0100 Subject: [PATCH] fix(server, web): Prevent the user from setting a future date of birth (#5803) * Hide the person age if it is negative * Add validation to prevent future birth dates * Add comment * Add test, Add birth date validation and update birth date modal * Add birthDate validation in PersonService and SetBirthDateModal * Running npm run format:fix * Generating the migration file propoerly, and Make the birthdate form logic simpler * Make birthDate type only string * Adding useLocationPin back --- .../src/domain/person/person.service.spec.ts | 44 ++++++++++++++++++- server/src/domain/person/person.service.ts | 5 +++ server/src/infra/entities/person.entity.ts | 2 + ...fyFutureBirthDatesAndAddCheckConstraint.ts | 16 +++++++ .../asset-viewer/detail-panel.svelte | 29 ++++++------ .../faces-page/set-birth-date-modal.svelte | 15 ++++++- 6 files changed, 95 insertions(+), 16 deletions(-) create mode 100644 server/src/infra/migrations/1702938928766-NullifyFutureBirthDatesAndAddCheckConstraint.ts diff --git a/server/src/domain/person/person.service.spec.ts b/server/src/domain/person/person.service.spec.ts index 7056321f4d..2decdb2209 100644 --- a/server/src/domain/person/person.service.spec.ts +++ b/server/src/domain/person/person.service.spec.ts @@ -278,13 +278,55 @@ 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(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); diff --git a/server/src/domain/person/person.service.ts b/server/src/domain/person/person.service.ts index f0190e43e1..fd80e444b2 100644 --- a/server/src/domain/person/person.service.ts +++ b/server/src/domain/person/person.service.ts @@ -199,6 +199,11 @@ export class PersonService { 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 }); } diff --git a/server/src/infra/entities/person.entity.ts b/server/src/infra/entities/person.entity.ts index 3b4545e45c..ecba45dd2c 100644 --- a/server/src/infra/entities/person.entity.ts +++ b/server/src/infra/entities/person.entity.ts @@ -1,4 +1,5 @@ import { + Check, Column, CreateDateColumn, Entity, @@ -11,6 +12,7 @@ import { AssetFaceEntity } from './asset-face.entity'; import { UserEntity } from './user.entity'; @Entity('person') +@Check(`"birthDate" <= CURRENT_DATE`) export class PersonEntity { @PrimaryGeneratedColumn('uuid') id!: string; diff --git a/server/src/infra/migrations/1702938928766-NullifyFutureBirthDatesAndAddCheckConstraint.ts b/server/src/infra/migrations/1702938928766-NullifyFutureBirthDatesAndAddCheckConstraint.ts new file mode 100644 index 0000000000..c646287c81 --- /dev/null +++ b/server/src/infra/migrations/1702938928766-NullifyFutureBirthDatesAndAddCheckConstraint.ts @@ -0,0 +1,16 @@ +import { MigrationInterface, QueryRunner } from "typeorm"; + +export class NullifyFutureBirthDatesAndAddCheckConstraint1702938928766 implements MigrationInterface { + name = 'NullifyFutureBirthDatesAndAddCheckConstraint1702938928766' + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query(`UPDATE "person" SET "birthDate" = NULL WHERE "birthDate" > CURRENT_DATE;`); + await queryRunner.query(`ALTER TABLE "person" ADD CONSTRAINT "CHK_b0f82b0ed662bfc24fbb58bb45" CHECK ("birthDate" <= CURRENT_DATE)`); + } + + public async down(queryRunner: QueryRunner): Promise { + // The down method cannot revert the nullified dates + await queryRunner.query(`ALTER TABLE "person" DROP CONSTRAINT "CHK_b0f82b0ed662bfc24fbb58bb45"`); + } + +} diff --git a/web/src/lib/components/asset-viewer/detail-panel.svelte b/web/src/lib/components/asset-viewer/detail-panel.svelte index 46451f8f0f..c70b3c02f5 100644 --- a/web/src/lib/components/asset-viewer/detail-panel.svelte +++ b/web/src/lib/components/asset-viewer/detail-panel.svelte @@ -264,19 +264,22 @@

{person.name}

{#if person.birthDate} {@const personBirthDate = DateTime.fromISO(person.birthDate)} -

- Age {Math.floor(DateTime.fromISO(asset.fileCreatedAt).diff(personBirthDate, 'years').years)} -

+ {@const age = Math.floor(DateTime.fromISO(asset.fileCreatedAt).diff(personBirthDate, 'years').years)} + {#if age >= 0} +

+ Age {age} +

+ {/if} {/if} diff --git a/web/src/lib/components/faces-page/set-birth-date-modal.svelte b/web/src/lib/components/faces-page/set-birth-date-modal.svelte index 630c60a747..415f5c0f76 100644 --- a/web/src/lib/components/faces-page/set-birth-date-modal.svelte +++ b/web/src/lib/components/faces-page/set-birth-date-modal.svelte @@ -12,8 +12,12 @@ updated: string; }>(); + const todayFormatted = new Date().toISOString().split('T')[0]; + const handleCancel = () => dispatch('close'); - const handleSubmit = () => dispatch('updated', birthDate); + const handleSubmit = () => { + dispatch('updated', birthDate); + }; handleCancel()}> @@ -33,7 +37,14 @@
handleSubmit()} autocomplete="off">
- +