mirror of
https://github.com/immich-app/immich.git
synced 2024-11-15 18:08:48 -07:00
fix(server): correct person birth date across timezones (#11369)
* fix(server): correct person birth date across timezones * fix test * update e2e tests * use Optional decorator
This commit is contained in:
parent
ebc71e428d
commit
434bcec5cc
@ -6,10 +6,19 @@ import request from 'supertest';
|
|||||||
import { beforeAll, beforeEach, describe, expect, it } from 'vitest';
|
import { beforeAll, beforeEach, describe, expect, it } from 'vitest';
|
||||||
|
|
||||||
const invalidBirthday = [
|
const invalidBirthday = [
|
||||||
{ birthDate: 'false', response: 'birthDate must be a date string' },
|
{
|
||||||
{ birthDate: '123567', response: 'birthDate must be a date string' },
|
birthDate: 'false',
|
||||||
{ birthDate: 123_567, response: 'birthDate must be a date string' },
|
response: ['birthDate must be a string in the format yyyy-MM-dd', 'Birth date cannot be in the future'],
|
||||||
{ birthDate: new Date(9999, 0, 0).toISOString(), response: ['Birth date cannot be in the future'] },
|
},
|
||||||
|
{
|
||||||
|
birthDate: '123567',
|
||||||
|
response: ['birthDate must be a string in the format yyyy-MM-dd', 'Birth date cannot be in the future'],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
birthDate: 123_567,
|
||||||
|
response: ['birthDate must be a string in the format yyyy-MM-dd', 'Birth date cannot be in the future'],
|
||||||
|
},
|
||||||
|
{ birthDate: '9999-01-01', response: ['Birth date cannot be in the future'] },
|
||||||
];
|
];
|
||||||
|
|
||||||
describe('/people', () => {
|
describe('/people', () => {
|
||||||
@ -185,13 +194,13 @@ describe('/people', () => {
|
|||||||
.set('Authorization', `Bearer ${admin.accessToken}`)
|
.set('Authorization', `Bearer ${admin.accessToken}`)
|
||||||
.send({
|
.send({
|
||||||
name: 'New Person',
|
name: 'New Person',
|
||||||
birthDate: '1990-01-01T05:00:00.000Z',
|
birthDate: '1990-01-01',
|
||||||
});
|
});
|
||||||
expect(status).toBe(201);
|
expect(status).toBe(201);
|
||||||
expect(body).toMatchObject({
|
expect(body).toMatchObject({
|
||||||
id: expect.any(String),
|
id: expect.any(String),
|
||||||
name: 'New Person',
|
name: 'New Person',
|
||||||
birthDate: '1990-01-01T05:00:00.000Z',
|
birthDate: '1990-01-01',
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
@ -233,7 +242,7 @@ describe('/people', () => {
|
|||||||
const { status, body } = await request(app)
|
const { status, body } = await request(app)
|
||||||
.put(`/people/${visiblePerson.id}`)
|
.put(`/people/${visiblePerson.id}`)
|
||||||
.set('Authorization', `Bearer ${admin.accessToken}`)
|
.set('Authorization', `Bearer ${admin.accessToken}`)
|
||||||
.send({ birthDate: '1990-01-01T05:00:00.000Z' });
|
.send({ birthDate: '1990-01-01' });
|
||||||
expect(status).toBe(200);
|
expect(status).toBe(200);
|
||||||
expect(body).toMatchObject({ birthDate: '1990-01-01' });
|
expect(body).toMatchObject({ birthDate: '1990-01-01' });
|
||||||
});
|
});
|
||||||
|
@ -1,11 +1,11 @@
|
|||||||
import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger';
|
import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger';
|
||||||
import { Type } from 'class-transformer';
|
import { Type } from 'class-transformer';
|
||||||
import { IsArray, IsInt, IsNotEmpty, IsString, Max, MaxDate, Min, ValidateNested } from 'class-validator';
|
import { IsArray, IsInt, IsNotEmpty, IsString, Max, Min, ValidateNested } from 'class-validator';
|
||||||
import { PropertyLifecycle } from 'src/decorators';
|
import { PropertyLifecycle } from 'src/decorators';
|
||||||
import { AuthDto } from 'src/dtos/auth.dto';
|
import { AuthDto } from 'src/dtos/auth.dto';
|
||||||
import { AssetFaceEntity } from 'src/entities/asset-face.entity';
|
import { AssetFaceEntity } from 'src/entities/asset-face.entity';
|
||||||
import { PersonEntity } from 'src/entities/person.entity';
|
import { PersonEntity } from 'src/entities/person.entity';
|
||||||
import { Optional, ValidateBoolean, ValidateDate, ValidateUUID } from 'src/validation';
|
import { IsDateStringFormat, MaxDateString, Optional, ValidateBoolean, ValidateUUID } from 'src/validation';
|
||||||
|
|
||||||
export class PersonCreateDto {
|
export class PersonCreateDto {
|
||||||
/**
|
/**
|
||||||
@ -19,9 +19,11 @@ export class PersonCreateDto {
|
|||||||
* Person date of birth.
|
* Person date of birth.
|
||||||
* Note: the mobile app cannot currently set the birth date to null.
|
* Note: the mobile app cannot currently set the birth date to null.
|
||||||
*/
|
*/
|
||||||
@MaxDate(() => new Date(), { message: 'Birth date cannot be in the future' })
|
@ApiProperty({ format: 'date' })
|
||||||
@ValidateDate({ optional: true, nullable: true, format: 'date' })
|
@MaxDateString(() => new Date(), { message: 'Birth date cannot be in the future' })
|
||||||
birthDate?: Date | null;
|
@IsDateStringFormat('yyyy-MM-dd')
|
||||||
|
@Optional({ nullable: true })
|
||||||
|
birthDate?: string | null;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Person visibility
|
* Person visibility
|
||||||
@ -84,7 +86,7 @@ export class PersonResponseDto {
|
|||||||
id!: string;
|
id!: string;
|
||||||
name!: string;
|
name!: string;
|
||||||
@ApiProperty({ format: 'date' })
|
@ApiProperty({ format: 'date' })
|
||||||
birthDate!: Date | null;
|
birthDate!: string | null;
|
||||||
thumbnailPath!: string;
|
thumbnailPath!: string;
|
||||||
isHidden!: boolean;
|
isHidden!: boolean;
|
||||||
@PropertyLifecycle({ addedAt: 'v1.107.0' })
|
@PropertyLifecycle({ addedAt: 'v1.107.0' })
|
||||||
|
@ -33,7 +33,7 @@ export class PersonEntity {
|
|||||||
name!: string;
|
name!: string;
|
||||||
|
|
||||||
@Column({ type: 'date', nullable: true })
|
@Column({ type: 'date', nullable: true })
|
||||||
birthDate!: Date | null;
|
birthDate!: string | null;
|
||||||
|
|
||||||
@Column({ default: '' })
|
@Column({ default: '' })
|
||||||
thumbnailPath!: string;
|
thumbnailPath!: string;
|
||||||
|
@ -256,15 +256,15 @@ describe(PersonService.name, () => {
|
|||||||
personMock.getAssets.mockResolvedValue([assetStub.image]);
|
personMock.getAssets.mockResolvedValue([assetStub.image]);
|
||||||
accessMock.person.checkOwnerAccess.mockResolvedValue(new Set(['person-1']));
|
accessMock.person.checkOwnerAccess.mockResolvedValue(new Set(['person-1']));
|
||||||
|
|
||||||
await expect(sut.update(authStub.admin, 'person-1', { birthDate: new Date('1976-06-30') })).resolves.toEqual({
|
await expect(sut.update(authStub.admin, 'person-1', { birthDate: '1976-06-30' })).resolves.toEqual({
|
||||||
id: 'person-1',
|
id: 'person-1',
|
||||||
name: 'Person 1',
|
name: 'Person 1',
|
||||||
birthDate: new Date('1976-06-30'),
|
birthDate: '1976-06-30',
|
||||||
thumbnailPath: '/path/to/thumbnail.jpg',
|
thumbnailPath: '/path/to/thumbnail.jpg',
|
||||||
isHidden: false,
|
isHidden: false,
|
||||||
updatedAt: expect.any(Date),
|
updatedAt: expect.any(Date),
|
||||||
});
|
});
|
||||||
expect(personMock.update).toHaveBeenCalledWith({ id: 'person-1', birthDate: new Date('1976-06-30') });
|
expect(personMock.update).toHaveBeenCalledWith({ id: 'person-1', birthDate: '1976-06-30' });
|
||||||
expect(jobMock.queue).not.toHaveBeenCalled();
|
expect(jobMock.queue).not.toHaveBeenCalled();
|
||||||
expect(jobMock.queueAll).not.toHaveBeenCalled();
|
expect(jobMock.queueAll).not.toHaveBeenCalled();
|
||||||
expect(accessMock.person.checkOwnerAccess).toHaveBeenCalledWith(authStub.admin.user.id, new Set(['person-1']));
|
expect(accessMock.person.checkOwnerAccess).toHaveBeenCalledWith(authStub.admin.user.id, new Set(['person-1']));
|
||||||
|
56
server/src/validation.spec.ts
Normal file
56
server/src/validation.spec.ts
Normal file
@ -0,0 +1,56 @@
|
|||||||
|
import { plainToInstance } from 'class-transformer';
|
||||||
|
import { validate } from 'class-validator';
|
||||||
|
import { IsDateStringFormat, MaxDateString } from 'src/validation';
|
||||||
|
|
||||||
|
describe('Validation', () => {
|
||||||
|
describe('MaxDateString', () => {
|
||||||
|
const maxDate = new Date(2000, 0, 1);
|
||||||
|
|
||||||
|
class MyDto {
|
||||||
|
@MaxDateString(maxDate)
|
||||||
|
date!: string;
|
||||||
|
}
|
||||||
|
|
||||||
|
it('passes when date is before maxDate', async () => {
|
||||||
|
const dto = plainToInstance(MyDto, { date: '1999-12-31' });
|
||||||
|
await expect(validate(dto)).resolves.toHaveLength(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('passes when date is equal to maxDate', async () => {
|
||||||
|
const dto = plainToInstance(MyDto, { date: '2000-01-01' });
|
||||||
|
await expect(validate(dto)).resolves.toHaveLength(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('fails when date is after maxDate', async () => {
|
||||||
|
const dto = plainToInstance(MyDto, { date: '2010-01-01' });
|
||||||
|
await expect(validate(dto)).resolves.toHaveLength(1);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('IsDateStringFormat', () => {
|
||||||
|
class MyDto {
|
||||||
|
@IsDateStringFormat('yyyy-MM-dd')
|
||||||
|
date!: string;
|
||||||
|
}
|
||||||
|
|
||||||
|
it('passes when date is valid', async () => {
|
||||||
|
const dto = plainToInstance(MyDto, { date: '1999-12-31' });
|
||||||
|
await expect(validate(dto)).resolves.toHaveLength(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('fails when date has invalid format', async () => {
|
||||||
|
const dto = plainToInstance(MyDto, { date: '2000-01-01T00:00:00Z' });
|
||||||
|
await expect(validate(dto)).resolves.toHaveLength(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('fails when empty string', async () => {
|
||||||
|
const dto = plainToInstance(MyDto, { date: '' });
|
||||||
|
await expect(validate(dto)).resolves.toHaveLength(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('fails when undefined', async () => {
|
||||||
|
const dto = plainToInstance(MyDto, {});
|
||||||
|
await expect(validate(dto)).resolves.toHaveLength(1);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
@ -16,11 +16,15 @@ import {
|
|||||||
IsOptional,
|
IsOptional,
|
||||||
IsString,
|
IsString,
|
||||||
IsUUID,
|
IsUUID,
|
||||||
|
ValidateBy,
|
||||||
ValidateIf,
|
ValidateIf,
|
||||||
ValidationOptions,
|
ValidationOptions,
|
||||||
|
buildMessage,
|
||||||
isDateString,
|
isDateString,
|
||||||
|
maxDate,
|
||||||
} from 'class-validator';
|
} from 'class-validator';
|
||||||
import { CronJob } from 'cron';
|
import { CronJob } from 'cron';
|
||||||
|
import { DateTime } from 'luxon';
|
||||||
import sanitize from 'sanitize-filename';
|
import sanitize from 'sanitize-filename';
|
||||||
|
|
||||||
@Injectable()
|
@Injectable()
|
||||||
@ -165,3 +169,46 @@ export const isValidInteger = (value: number, options: { min?: number; max?: num
|
|||||||
const { min = Number.MIN_SAFE_INTEGER, max = Number.MAX_SAFE_INTEGER } = options;
|
const { min = Number.MIN_SAFE_INTEGER, max = Number.MAX_SAFE_INTEGER } = options;
|
||||||
return Number.isInteger(value) && value >= min && value <= max;
|
return Number.isInteger(value) && value >= min && value <= max;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
export function isDateStringFormat(value: unknown, format: string) {
|
||||||
|
if (typeof value !== 'string') {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
return DateTime.fromFormat(value, format, { zone: 'utc' }).isValid;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function IsDateStringFormat(format: string, validationOptions?: ValidationOptions) {
|
||||||
|
return ValidateBy(
|
||||||
|
{
|
||||||
|
name: 'isDateStringFormat',
|
||||||
|
constraints: [format],
|
||||||
|
validator: {
|
||||||
|
validate(value: unknown) {
|
||||||
|
return isDateStringFormat(value, format);
|
||||||
|
},
|
||||||
|
defaultMessage: () => `$property must be a string in the format ${format}`,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
validationOptions,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
export function MaxDateString(date: Date | (() => Date), validationOptions?: ValidationOptions): PropertyDecorator {
|
||||||
|
return ValidateBy(
|
||||||
|
{
|
||||||
|
name: 'maxDateString',
|
||||||
|
constraints: [date],
|
||||||
|
validator: {
|
||||||
|
validate: (value, args) => {
|
||||||
|
const date = DateTime.fromISO(value, { zone: 'utc' }).toJSDate();
|
||||||
|
return maxDate(date, args?.constraints[0]);
|
||||||
|
},
|
||||||
|
defaultMessage: buildMessage(
|
||||||
|
(eachPrefix) => 'maximal allowed date for ' + eachPrefix + '$property is $constraint1',
|
||||||
|
validationOptions,
|
||||||
|
),
|
||||||
|
},
|
||||||
|
},
|
||||||
|
validationOptions,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
2
server/test/fixtures/person.stub.ts
vendored
2
server/test/fixtures/person.stub.ts
vendored
@ -65,7 +65,7 @@ export const personStub = {
|
|||||||
ownerId: userStub.admin.id,
|
ownerId: userStub.admin.id,
|
||||||
owner: userStub.admin,
|
owner: userStub.admin,
|
||||||
name: 'Person 1',
|
name: 'Person 1',
|
||||||
birthDate: new Date('1976-06-30'),
|
birthDate: '1976-06-30',
|
||||||
thumbnailPath: '/path/to/thumbnail.jpg',
|
thumbnailPath: '/path/to/thumbnail.jpg',
|
||||||
faces: [],
|
faces: [],
|
||||||
faceAssetId: null,
|
faceAssetId: null,
|
||||||
|
Loading…
Reference in New Issue
Block a user