mirror of
https://github.com/ever-co/ever-gauzy.git
synced 2026-10-02 01:54:50 +08:00
fix(employee,candidate): cap rates, carry the minimum rate on hire, honor transformers under MikroORM
Follow-ups from review of this PR: - @Max(999999999999.99) on all four rate fields, so a value the numeric(14,2) column cannot hold is a clean 400 from the DTO instead of a database overflow, and cannot block a rollback to the old integer column. - CandidateHiredHandler now copies minimumBillingRate into the new employee. It only copied billRateValue, so a candidate's minimum rate was silently dropped on hire (bug, pre-existing). - MikroORM ignored the TypeORM `transformer` column option: @MultiORMColumn handed it to @Property, which drops it, so with DB_ORM=mikro-orm money columns skipped rounding and validation and int-backed enum columns (actor type, availability status) were stored and read raw. parseMikroOrmColumnOptions now wraps the transformer in a MikroORM type and keeps the declared column DDL (numeric(14,2)). - Specs for each, including the hire handler (new) and the transformer bridge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
ca19d325c0
commit
7affd5ab71
@@ -96,6 +96,14 @@ describe('UpdateCandidateDTO billing rates', () => {
|
||||
expect(await rateErrors(dto)).toEqual([]);
|
||||
});
|
||||
|
||||
it('rejects a rate the numeric(14,2) column cannot hold, instead of failing in the database', async () => {
|
||||
const tooBig = plainToInstance(UpdateCandidateDTO, { billRateValue: 1e12 });
|
||||
const largest = plainToInstance(UpdateCandidateDTO, { billRateValue: 999999999999.99 });
|
||||
|
||||
expect(await rateErrors(tooBig)).toEqual(['billRateValue']);
|
||||
expect(await rateErrors(largest)).toEqual([]);
|
||||
});
|
||||
|
||||
it("keeps parseInt's leading-number parsing and still rejects non-numeric rates", async () => {
|
||||
const loose = plainToInstance(UpdateCandidateDTO, { billRateValue: '10,50' });
|
||||
const invalid = plainToInstance(UpdateCandidateDTO, { billRateValue: 'abc' });
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger';
|
||||
import { JoinColumn, RelationId, JoinTable } from 'typeorm';
|
||||
import { IsDateString, IsEnum, IsNumber, IsOptional, IsString, MaxLength } from 'class-validator';
|
||||
import { IsDateString, IsEnum, IsNumber, IsOptional, IsString, Max, MaxLength } from 'class-validator';
|
||||
import { Transform, TransformFnParams } from 'class-transformer';
|
||||
import {
|
||||
ICandidate,
|
||||
@@ -40,7 +40,7 @@ import {
|
||||
TenantOrganizationBaseEntity,
|
||||
User
|
||||
} from '../core/entities/internal';
|
||||
import { billingRateColumn, ColumnNumericTransformerPipe, toBillingRate } from './../shared/pipes';
|
||||
import { BILLING_RATE_MAX, billingRateColumn, ColumnNumericTransformerPipe, toBillingRate } from './../shared/pipes';
|
||||
import {
|
||||
ColumnIndex,
|
||||
MultiORMColumn,
|
||||
@@ -113,6 +113,7 @@ export class Candidate extends TenantOrganizationBaseEntity implements ICandidat
|
||||
@ApiPropertyOptional({ type: () => Number })
|
||||
@IsOptional()
|
||||
@IsNumber()
|
||||
@Max(BILLING_RATE_MAX)
|
||||
@Transform(toBillingRate)
|
||||
@MultiORMColumn(billingRateColumn())
|
||||
billRateValue?: number;
|
||||
@@ -120,6 +121,7 @@ export class Candidate extends TenantOrganizationBaseEntity implements ICandidat
|
||||
@ApiPropertyOptional({ type: () => Number })
|
||||
@IsOptional()
|
||||
@IsNumber()
|
||||
@Max(BILLING_RATE_MAX)
|
||||
@Transform(toBillingRate)
|
||||
@MultiORMColumn(billingRateColumn())
|
||||
minimumBillingRate?: number;
|
||||
|
||||
@@ -0,0 +1,88 @@
|
||||
/**
|
||||
* Load the decorator graph first, as the application boot does, or importing the handler's services
|
||||
* hits the "IsEmployeeBelongsToOrganization is not a function" cycle.
|
||||
*/
|
||||
import '../../../core/entities/internal';
|
||||
|
||||
import { ConflictException } from '@nestjs/common';
|
||||
import { CandidateStatusEnum, CurrenciesEnum, RolesEnum } from '@gauzy/contracts';
|
||||
import { CandidateHiredCommand } from '../candidate.hired.command';
|
||||
import { CandidateHiredHandler } from './candidate.hired.handler';
|
||||
|
||||
/**
|
||||
* Hiring a candidate creates the employee from the candidate's rates. Both sides are `numeric(14,2)`
|
||||
* since #10203 (employee) and this change (candidate), so the cents must survive — and
|
||||
* `minimumBillingRate` has to be carried over, not dropped.
|
||||
*/
|
||||
describe('CandidateHiredHandler', () => {
|
||||
const candidate = {
|
||||
id: 'candidate-1',
|
||||
alreadyHired: false,
|
||||
billRateValue: 10.49,
|
||||
minimumBillingRate: 5.25,
|
||||
billRateCurrency: CurrenciesEnum.USD,
|
||||
reWeeklyLimit: 37,
|
||||
tenantId: 'tenant-1',
|
||||
organizationId: 'org-1',
|
||||
userId: 'user-1',
|
||||
tags: []
|
||||
};
|
||||
|
||||
const buildHandler = (overrides: Partial<typeof candidate> = {}) => {
|
||||
const candidateService = {
|
||||
findOneByIdString: jest.fn(async () => ({ ...candidate, ...overrides })),
|
||||
create: jest.fn(async (input: any) => input)
|
||||
};
|
||||
const employeeService = { create: jest.fn(async (input: any) => ({ id: 'employee-1', ...input })) };
|
||||
const userService = { create: jest.fn(async (input: any) => input) };
|
||||
const roleService = {
|
||||
findOneByWhereOptions: jest.fn(async () => ({ id: 'role-1', name: RolesEnum.EMPLOYEE }))
|
||||
};
|
||||
|
||||
const handler = new CandidateHiredHandler(
|
||||
candidateService as any,
|
||||
employeeService as any,
|
||||
userService as any,
|
||||
roleService as any
|
||||
);
|
||||
|
||||
return { handler, candidateService, employeeService };
|
||||
};
|
||||
|
||||
it('carries both rates, with their cents, into the new employee', async () => {
|
||||
const { handler, employeeService } = buildHandler();
|
||||
|
||||
await handler.execute(new CandidateHiredCommand(candidate.id));
|
||||
|
||||
expect(employeeService.create).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ billRateValue: 10.49, minimumBillingRate: 5.25 })
|
||||
);
|
||||
});
|
||||
|
||||
it('passes a missing minimum rate through as-is', async () => {
|
||||
const { handler, employeeService } = buildHandler({ minimumBillingRate: null });
|
||||
|
||||
await handler.execute(new CandidateHiredCommand(candidate.id));
|
||||
|
||||
expect(employeeService.create).toHaveBeenCalledWith(expect.objectContaining({ minimumBillingRate: null }));
|
||||
});
|
||||
|
||||
it('marks the candidate hired', async () => {
|
||||
const { handler, candidateService } = buildHandler();
|
||||
|
||||
await handler.execute(new CandidateHiredCommand(candidate.id));
|
||||
|
||||
expect(candidateService.create).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ id: candidate.id, status: CandidateStatusEnum.HIRED })
|
||||
);
|
||||
});
|
||||
|
||||
it('refuses to hire an already hired candidate', async () => {
|
||||
const { handler, employeeService } = buildHandler({ alreadyHired: true });
|
||||
|
||||
await expect(handler.execute(new CandidateHiredCommand(candidate.id))).rejects.toBeInstanceOf(
|
||||
ConflictException
|
||||
);
|
||||
expect(employeeService.create).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -44,6 +44,7 @@ export class CandidateHiredHandler implements ICommandHandler<CandidateHiredComm
|
||||
// Step 1: Create an employee for the respective candidate
|
||||
const employee = await this.employeeService.create({
|
||||
billRateValue: candidate.billRateValue,
|
||||
minimumBillingRate: candidate.minimumBillingRate,
|
||||
billRateCurrency: candidate.billRateCurrency,
|
||||
reWeeklyLimit: candidate.reWeeklyLimit,
|
||||
payPeriod: candidate.payPeriod,
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { DataSourceOptions } from 'typeorm';
|
||||
import { ColumnDataType, MikroORMColumnOptions } from './column-options.types';
|
||||
import { declaredColumnType, ValueTransformerType } from './value-transformer.type';
|
||||
|
||||
/**
|
||||
* Resolve the database column type.
|
||||
@@ -37,6 +38,18 @@ export function parseMikroOrmColumnOptions<T>({ type, options }): MikroORMColumn
|
||||
if (options?.relationId) {
|
||||
options.persist = false;
|
||||
}
|
||||
|
||||
// MikroORM has no `transformer` option and would ignore it, so a column that declares one would
|
||||
// be stored and hydrated raw under `DB_ORM=mikro-orm`. Run it through a MikroORM type instead.
|
||||
if (options?.transformer) {
|
||||
const { transformer, ...rest } = options;
|
||||
return {
|
||||
...rest,
|
||||
type: new ValueTransformerType(transformer, declaredColumnType(type, options)),
|
||||
columnType: options.columnType ?? declaredColumnType(type, options)
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
type: type,
|
||||
...options
|
||||
|
||||
@@ -0,0 +1,63 @@
|
||||
import { ValueTransformer } from 'typeorm';
|
||||
import { ColumnNumericTransformerPipe } from '../../../shared/pipes';
|
||||
import { parseMikroOrmColumnOptions } from './column.helper';
|
||||
import { ValueTransformerType } from './value-transformer.type';
|
||||
|
||||
/**
|
||||
* MikroORM has no `transformer` column option. Before the bridge, `@MultiORMColumn({ transformer })`
|
||||
* handed it to `@Property`, which ignored it: under `DB_ORM=mikro-orm` money columns skipped rounding
|
||||
* and validation, and int-backed enum columns were stored raw.
|
||||
*/
|
||||
describe('parseMikroOrmColumnOptions: TypeORM transformer bridge', () => {
|
||||
const moneyOptions = () =>
|
||||
parseMikroOrmColumnOptions({
|
||||
type: 'numeric',
|
||||
options: { nullable: true, precision: 14, scale: 2, transformer: new ColumnNumericTransformerPipe(2) }
|
||||
}) as any;
|
||||
|
||||
it('wraps the transformer in a MikroORM type and drops the option MikroORM ignores', () => {
|
||||
const options = moneyOptions();
|
||||
|
||||
expect(options.type).toBeInstanceOf(ValueTransformerType);
|
||||
expect(options.transformer).toBeUndefined();
|
||||
expect(options.nullable).toBe(true);
|
||||
});
|
||||
|
||||
it('keeps the declared column DDL, with precision and scale', () => {
|
||||
const options = moneyOptions();
|
||||
|
||||
expect(options.columnType).toBe('numeric(14,2)');
|
||||
expect(options.type.getColumnType()).toBe('numeric(14,2)');
|
||||
});
|
||||
|
||||
it('rounds on write and returns a number on read, as TypeORM does', () => {
|
||||
const { type } = moneyOptions();
|
||||
|
||||
expect(type.convertToDatabaseValue(10.499)).toBe(10.5);
|
||||
expect(type.convertToDatabaseValue(1.005)).toBe(1.01);
|
||||
expect(type.convertToJSValue('10.49')).toBe(10.49);
|
||||
expect(type.convertToJSValue(null)).toBeNull();
|
||||
});
|
||||
|
||||
it('works for a non-numeric transformer too (int-backed enums)', () => {
|
||||
const transformer: ValueTransformer = {
|
||||
to: (value: string) => (value === 'ON' ? 1 : 0),
|
||||
from: (value: number) => (value === 1 ? 'ON' : 'OFF')
|
||||
};
|
||||
|
||||
const { type, columnType } = parseMikroOrmColumnOptions({ type: 'int', options: { transformer } }) as any;
|
||||
|
||||
expect(columnType).toBe('int');
|
||||
expect(type.convertToDatabaseValue('ON')).toBe(1);
|
||||
expect(type.convertToJSValue(0)).toBe('OFF');
|
||||
});
|
||||
|
||||
it('leaves columns without a transformer untouched', () => {
|
||||
const options = parseMikroOrmColumnOptions({
|
||||
type: 'varchar',
|
||||
options: { nullable: true, length: 255 }
|
||||
}) as any;
|
||||
|
||||
expect(options).toEqual({ type: 'varchar', nullable: true, length: 255 });
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,67 @@
|
||||
import { Type } from '@mikro-orm/core';
|
||||
import { ValueTransformer } from 'typeorm';
|
||||
|
||||
/**
|
||||
* Runs a TypeORM `ValueTransformer` under MikroORM.
|
||||
*
|
||||
* MikroORM has no `transformer` column option, and `@MultiORMColumn` passed ours straight into
|
||||
* `@Property`, where it was ignored: with `DB_ORM=mikro-orm` a money column skipped the rounding and
|
||||
* validation in `ColumnNumericTransformerPipe`, and an int-backed enum column (actor type,
|
||||
* availability status) was read and written as a raw number. Wrapping the transformer in a MikroORM
|
||||
* `Type` makes both ORMs store and hydrate a column the same way.
|
||||
*/
|
||||
export class ValueTransformerType extends Type<any, any> {
|
||||
constructor(
|
||||
private readonly transformer: ValueTransformer,
|
||||
private readonly declaredColumnType?: string
|
||||
) {
|
||||
super();
|
||||
}
|
||||
|
||||
/**
|
||||
* @param value - The entity value to store.
|
||||
* @returns The database representation, as TypeORM's transformer produces it.
|
||||
*/
|
||||
convertToDatabaseValue(value: any): any {
|
||||
return this.transformer.to(value);
|
||||
}
|
||||
|
||||
/**
|
||||
* @param value - The raw database value.
|
||||
* @returns The entity representation, as TypeORM's transformer produces it.
|
||||
*/
|
||||
convertToJSValue(value: any): any {
|
||||
return this.transformer.from(value);
|
||||
}
|
||||
|
||||
/**
|
||||
* Keep the column DDL the entity declared (e.g. `numeric(14,2)`), rather than letting MikroORM
|
||||
* infer it from this wrapper.
|
||||
*/
|
||||
getColumnType(): string | undefined {
|
||||
return this.declaredColumnType;
|
||||
}
|
||||
|
||||
/**
|
||||
* The transformer decides the stored shape, so compare the raw values as they come.
|
||||
*/
|
||||
compareAsType(): string {
|
||||
return 'any';
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Builds the column DDL for {@link ValueTransformerType} from the declared column options, so
|
||||
* precision and scale survive (`numeric` + 14/2 → `numeric(14,2)`).
|
||||
*
|
||||
* @param type - The declared column type.
|
||||
* @param options - The column options.
|
||||
* @returns The column type string, or undefined to let MikroORM infer it.
|
||||
*/
|
||||
export function declaredColumnType(type: unknown, options: { precision?: number; scale?: number } = {}) {
|
||||
if (typeof type !== 'string') {
|
||||
return undefined;
|
||||
}
|
||||
const { precision, scale } = options;
|
||||
return precision != null && scale != null ? `${type}(${precision},${scale})` : type;
|
||||
}
|
||||
@@ -2,7 +2,17 @@ import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger';
|
||||
import { JoinColumn, JoinTable, RelationId } from 'typeorm';
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
import { EntityRepositoryType } from '@mikro-orm/core';
|
||||
import { IsBoolean, IsDateString, IsEnum, IsNumber, IsOptional, IsString, IsUrl, MaxLength } from 'class-validator';
|
||||
import {
|
||||
IsBoolean,
|
||||
IsDateString,
|
||||
IsEnum,
|
||||
IsNumber,
|
||||
IsOptional,
|
||||
IsString,
|
||||
IsUrl,
|
||||
Max,
|
||||
MaxLength
|
||||
} from 'class-validator';
|
||||
import { Transform, TransformFnParams } from 'class-transformer';
|
||||
import {
|
||||
CurrenciesEnum,
|
||||
@@ -94,7 +104,7 @@ import {
|
||||
TypeOrmEmployeeEntityCustomFields
|
||||
} from '../core/entities/custom-entity-fields/employee';
|
||||
import { Trimmed } from '../shared/decorators';
|
||||
import { billingRateColumn, ColumnNumericTransformerPipe, toBillingRate } from '../shared/pipes';
|
||||
import { BILLING_RATE_MAX, billingRateColumn, ColumnNumericTransformerPipe, toBillingRate } from '../shared/pipes';
|
||||
import { Taggable } from '../tags/tag.types';
|
||||
import { MikroOrmEmployeeRepository } from './repository/mikro-orm-employee.repository';
|
||||
import { OrganizationProjectModuleEmployee } from '../organization-project-module/organization-project-module-employee.entity';
|
||||
@@ -140,6 +150,7 @@ export class Employee extends TenantOrganizationBaseEntity implements IEmployee,
|
||||
@ApiPropertyOptional({ type: () => Number })
|
||||
@IsOptional()
|
||||
@IsNumber()
|
||||
@Max(BILLING_RATE_MAX)
|
||||
@Transform(toBillingRate)
|
||||
@MultiORMColumn(billingRateColumn())
|
||||
billRateValue?: number;
|
||||
@@ -147,6 +158,7 @@ export class Employee extends TenantOrganizationBaseEntity implements IEmployee,
|
||||
@ApiPropertyOptional({ type: () => Number })
|
||||
@IsOptional()
|
||||
@IsNumber()
|
||||
@Max(BILLING_RATE_MAX)
|
||||
@Transform(toBillingRate)
|
||||
@MultiORMColumn(billingRateColumn())
|
||||
minimumBillingRate?: number;
|
||||
|
||||
@@ -1,6 +1,13 @@
|
||||
import { TransformFnParams } from 'class-transformer';
|
||||
import { ColumnNumericTransformerPipe, roundToScale } from './column-numeric-transformer.pipe';
|
||||
|
||||
/**
|
||||
* Largest value `numeric(14,2)` / `decimal(14,2)` can hold. Validate against it (`@Max`) so an
|
||||
* out-of-range rate is a clean 400 from the DTO instead of a database overflow, and so a rollback
|
||||
* to the old `integer` column cannot be blocked by a value it could never hold.
|
||||
*/
|
||||
export const BILLING_RATE_MAX = 999999999999.99;
|
||||
|
||||
/**
|
||||
* Column options for a money rate (`billRateValue`, `minimumBillingRate` on Employee and Candidate).
|
||||
* `numeric(14,2)` holds every value the old `integer` column could (up to 2,147,483,647) and keeps
|
||||
|
||||
Reference in New Issue
Block a user