From fcce01eb3f7b471ec37be052eeeb83dd4137a27f Mon Sep 17 00:00:00 2001 From: jmgasper Date: Wed, 29 Apr 2026 11:20:38 +1000 Subject: [PATCH] PM-4904: Allow project billing account clears What was broken Clearing a billing account from an existing project did not persist. The Work UI sent an explicit null billingAccountId, but the Projects API treated that value as omitted, so the old billing account stayed on the project and appeared again after save. Root cause UpdateProjectDto inherited CreateProjectDto billingAccountId parsing, which normalizes null and empty string to undefined. The service clear path depended on a derived flag that is not materialized by class-transformer when only billingAccountId is sent. What was changed UpdateProjectDto now owns the update-time billingAccountId field, preserving null and empty string as explicit clear requests while still parsing numeric billing account ids. ProjectService now clears when the DTO billingAccountId is null, in addition to the existing internal clear flag. Any added/updated tests Added DTO coverage for null, empty string, and numeric billingAccountId updates. Updated the project service clear test to exercise billingAccountId: null directly. --- .../project/dto/update-project.dto.spec.ts | 26 +++++++++++ src/api/project/dto/update-project.dto.ts | 45 +++++++++++++++++-- src/api/project/project.service.spec.ts | 4 +- src/api/project/project.service.ts | 6 ++- 4 files changed, 73 insertions(+), 8 deletions(-) create mode 100644 src/api/project/dto/update-project.dto.spec.ts diff --git a/src/api/project/dto/update-project.dto.spec.ts b/src/api/project/dto/update-project.dto.spec.ts new file mode 100644 index 0000000..9132e10 --- /dev/null +++ b/src/api/project/dto/update-project.dto.spec.ts @@ -0,0 +1,26 @@ +import { plainToInstance } from 'class-transformer'; +import { validate } from 'class-validator'; +import { UpdateProjectDto } from './update-project.dto'; + +describe('UpdateProjectDto', () => { + it.each([null, ''])( + 'preserves %p billingAccountId as an explicit clear request', + async (billingAccountId) => { + const dto = plainToInstance(UpdateProjectDto, { + billingAccountId, + }); + + expect(dto.billingAccountId).toBeNull(); + await expect(validate(dto)).resolves.toHaveLength(0); + }, + ); + + it('parses numeric billingAccountId updates', async () => { + const dto = plainToInstance(UpdateProjectDto, { + billingAccountId: '80001063', + }); + + expect(dto.billingAccountId).toBe(80001063); + await expect(validate(dto)).resolves.toHaveLength(0); + }); +}); diff --git a/src/api/project/dto/update-project.dto.ts b/src/api/project/dto/update-project.dto.ts index 80f6b13..b4aea56 100644 --- a/src/api/project/dto/update-project.dto.ts +++ b/src/api/project/dto/update-project.dto.ts @@ -1,8 +1,33 @@ -import { ApiHideProperty } from '@nestjs/swagger'; +import { ApiHideProperty, ApiPropertyOptional } from '@nestjs/swagger'; import { Transform } from 'class-transformer'; -import { PartialType } from '@nestjs/mapped-types'; +import { IsNumber, IsOptional } from 'class-validator'; +import { OmitType, PartialType } from '@nestjs/mapped-types'; import { CreateProjectDto } from './create-project.dto'; +/** + * Parses optional project billing-account update input. + * + * @param value Raw `billingAccountId` value from request payload. + * @returns Parsed integer, `null` when clearing, or `undefined` when omitted. + */ +function parseOptionalNullableInteger(value: unknown): number | null | undefined { + if (typeof value === 'undefined') { + return undefined; + } + + if (value === null || value === '') { + return null; + } + + const parsed = Number(value); + + if (Number.isNaN(parsed)) { + return undefined; + } + + return Math.trunc(parsed); +} + /** * Resolves whether the patch payload explicitly requests clearing * `billingAccountId`. @@ -17,9 +42,21 @@ function parseClearBillingAccountFlag(value: unknown): boolean { /** * Request DTO for `PATCH /projects/:projectId`. * - * Reuses `CreateProjectDto` and makes all fields optional via `PartialType`. + * Reuses `CreateProjectDto`, makes all fields optional via `PartialType`, and + * allows `billingAccountId` to be explicitly cleared with `null` or `''`. */ -export class UpdateProjectDto extends PartialType(CreateProjectDto) { +export class UpdateProjectDto extends PartialType( + OmitType(CreateProjectDto, ['billingAccountId'] as const), +) { + @ApiPropertyOptional({ + description: 'Project billing account id. Send null or empty string to clear.', + nullable: true, + }) + @IsOptional() + @Transform(({ value }) => parseOptionalNullableInteger(value)) + @IsNumber() + billingAccountId?: number | null; + @ApiHideProperty() @Transform(({ obj }) => parseClearBillingAccountFlag(obj?.billingAccountId)) clearBillingAccountId?: boolean; diff --git a/src/api/project/project.service.spec.ts b/src/api/project/project.service.spec.ts index fc4e9a4..9df8331 100644 --- a/src/api/project/project.service.spec.ts +++ b/src/api/project/project.service.spec.ts @@ -1646,8 +1646,8 @@ describe('ProjectService', () => { await service.updateProject( '1001', { - clearBillingAccountId: true, - } as any, + billingAccountId: null, + }, { userId: '100', isMachine: false, diff --git a/src/api/project/project.service.ts b/src/api/project/project.service.ts index de3d963..9da39d1 100644 --- a/src/api/project/project.service.ts +++ b/src/api/project/project.service.ts @@ -666,8 +666,10 @@ export class ProjectService { throw new ForbiddenException('Insufficient permissions'); } + const shouldClearBillingAccountId = + dto.clearBillingAccountId === true || dto.billingAccountId === null; const requestedBillingAccountId = - dto.clearBillingAccountId === true + shouldClearBillingAccountId ? null : typeof dto.billingAccountId === 'number' ? String(dto.billingAccountId) @@ -726,7 +728,7 @@ export class ProjectService { cancelReason: typeof dto.cancelReason === 'string' ? dto.cancelReason : undefined, billingAccountId: - dto.clearBillingAccountId === true + shouldClearBillingAccountId ? null : typeof dto.billingAccountId === 'number' ? BigInt(dto.billingAccountId)