From cdc71c7f69b59090a8407f3bd10c1adb98defce5 Mon Sep 17 00:00:00 2001 From: Mariano Fuentes Date: Wed, 25 Mar 2026 15:01:01 -0400 Subject: [PATCH 1/2] fix(vendors): fix PATCH 400 error for vendors with empty descriptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PartialType(CreateVendorDto) preserves @IsNotEmpty() from the create DTO. Combined with @IsOptional() added by PartialType, this means: - null/undefined → @IsOptional() skips validators → OK - "" (empty string) → @IsOptional() does NOT skip → @IsNotEmpty() fails → 400 Vendors created during onboarding can have empty descriptions, causing all subsequent PATCH updates to fail with 400 Bad Request. Replace PartialType with an explicit DTO that drops @IsNotEmpty() on description (empty strings are valid for updates) while keeping it on name (a vendor must always have a non-empty name). Co-Authored-By: Claude Opus 4.6 (1M context) --- apps/api/src/vendors/dto/update-vendor.dto.ts | 86 ++++++++++++++++++- 1 file changed, 83 insertions(+), 3 deletions(-) diff --git a/apps/api/src/vendors/dto/update-vendor.dto.ts b/apps/api/src/vendors/dto/update-vendor.dto.ts index 66b0101e6c..cea91edff5 100644 --- a/apps/api/src/vendors/dto/update-vendor.dto.ts +++ b/apps/api/src/vendors/dto/update-vendor.dto.ts @@ -1,4 +1,84 @@ -import { PartialType } from '@nestjs/swagger'; -import { CreateVendorDto } from './create-vendor.dto'; +import { ApiPropertyOptional } from '@nestjs/swagger'; +import { + IsString, + IsNotEmpty, + IsOptional, + IsEnum, + IsUrl, + IsBoolean, +} from 'class-validator'; +import { Transform } from 'class-transformer'; +import { + VendorCategory, + VendorStatus, + Likelihood, + Impact, +} from '@trycompai/db'; -export class UpdateVendorDto extends PartialType(CreateVendorDto) {} +/** + * DTO for PATCH /vendors/:id + * + * Defined explicitly rather than using PartialType(CreateVendorDto) because + * PartialType preserves @IsNotEmpty() — which rejects empty strings even + * when @IsOptional() is added. For PATCH, empty-string fields like + * `description: ""` (common for vendors created during onboarding) should + * not cause a 400. + */ +export class UpdateVendorDto { + @ApiPropertyOptional({ description: 'Vendor name' }) + @IsOptional() + @IsString() + @IsNotEmpty() + name?: string; + + @ApiPropertyOptional({ description: 'Vendor description' }) + @IsOptional() + @IsString() + description?: string; + + @ApiPropertyOptional({ description: 'Vendor category', enum: VendorCategory }) + @IsOptional() + @IsEnum(VendorCategory) + category?: VendorCategory; + + @ApiPropertyOptional({ description: 'Assessment status', enum: VendorStatus }) + @IsOptional() + @IsEnum(VendorStatus) + status?: VendorStatus; + + @ApiPropertyOptional({ description: 'Inherent probability', enum: Likelihood }) + @IsOptional() + @IsEnum(Likelihood) + inherentProbability?: Likelihood; + + @ApiPropertyOptional({ description: 'Inherent impact', enum: Impact }) + @IsOptional() + @IsEnum(Impact) + inherentImpact?: Impact; + + @ApiPropertyOptional({ description: 'Residual probability', enum: Likelihood }) + @IsOptional() + @IsEnum(Likelihood) + residualProbability?: Likelihood; + + @ApiPropertyOptional({ description: 'Residual impact', enum: Impact }) + @IsOptional() + @IsEnum(Impact) + residualImpact?: Impact; + + @ApiPropertyOptional({ description: 'Vendor website URL' }) + @IsOptional() + @IsUrl() + @Transform(({ value }) => (value === '' ? undefined : value)) + website?: string; + + @ApiPropertyOptional({ description: 'Whether the vendor is a sub-processor' }) + @IsOptional() + @IsBoolean() + isSubProcessor?: boolean; + + @ApiPropertyOptional({ description: 'Assignee member ID' }) + @IsOptional() + @IsString() + assigneeId?: string; +} From 762d9b98ecd388931822bf4835953cc5b618e1e5 Mon Sep 17 00:00:00 2001 From: Mariano Fuentes Date: Wed, 25 Mar 2026 15:04:35 -0400 Subject: [PATCH 2/2] test(vendors): add UpdateVendorDto validation tests Tests cover the PATCH validation fix: - Empty description accepted (the bug: onboarding vendors have "") - Empty name still rejected - assigneeId: null accepted (unassigned vendors) - Empty website transformed to undefined - Invalid URLs, enums, and unknown properties rejected Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/vendors/dto/update-vendor.dto.spec.ts | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 apps/api/src/vendors/dto/update-vendor.dto.spec.ts diff --git a/apps/api/src/vendors/dto/update-vendor.dto.spec.ts b/apps/api/src/vendors/dto/update-vendor.dto.spec.ts new file mode 100644 index 0000000000..cc25082e2d --- /dev/null +++ b/apps/api/src/vendors/dto/update-vendor.dto.spec.ts @@ -0,0 +1,113 @@ +import { plainToInstance } from 'class-transformer'; +import { validate } from 'class-validator'; +import { UpdateVendorDto } from './update-vendor.dto'; + +/** + * Mirrors the global ValidationPipe config from main.ts: + * whitelist: true, transform: true, enableImplicitConversion: true + */ +function toDto(plain: Record): UpdateVendorDto { + return plainToInstance(UpdateVendorDto, plain, { + enableImplicitConversion: true, + }); +} + +describe('UpdateVendorDto', () => { + it('should accept a valid full update payload', async () => { + const dto = toDto({ + name: 'Acronis', + description: 'Backup solutions provider', + category: 'software_as_a_service', + status: 'assessed', + website: 'https://www.acronis.com', + isSubProcessor: false, + assigneeId: 'mem_abc123', + }); + const errors = await validate(dto, { whitelist: true, forbidNonWhitelisted: true }); + expect(errors).toHaveLength(0); + }); + + it('should accept a minimal update (single field)', async () => { + const dto = toDto({ website: 'https://www.acronis.com' }); + const errors = await validate(dto, { whitelist: true, forbidNonWhitelisted: true }); + expect(errors).toHaveLength(0); + }); + + it('should accept an empty body (no fields to update)', async () => { + const dto = toDto({}); + const errors = await validate(dto, { whitelist: true, forbidNonWhitelisted: true }); + expect(errors).toHaveLength(0); + }); + + // ── The bug this DTO fix addresses ──────────────────────────────── + it('should accept empty description (vendors from onboarding)', async () => { + const dto = toDto({ + name: 'Acronis', + description: '', + category: 'software_as_a_service', + status: 'assessed', + website: 'https://www.acronis.com', + isSubProcessor: false, + }); + const errors = await validate(dto, { whitelist: true, forbidNonWhitelisted: true }); + expect(errors).toHaveLength(0); + }); + + it('should still reject empty name', async () => { + const dto = toDto({ name: '' }); + const errors = await validate(dto, { whitelist: true, forbidNonWhitelisted: true }); + expect(errors.length).toBeGreaterThan(0); + expect(errors[0].property).toBe('name'); + }); + + // ── assigneeId: null (unassigned vendor) ────────────────────────── + it('should accept assigneeId: null', async () => { + const dto = toDto({ assigneeId: null }); + const errors = await validate(dto, { whitelist: true, forbidNonWhitelisted: true }); + expect(errors).toHaveLength(0); + }); + + // ── website handling ────────────────────────────────────────────── + it('should transform empty website to undefined (skip validation)', async () => { + const dto = toDto({ website: '' }); + const errors = await validate(dto, { whitelist: true, forbidNonWhitelisted: true }); + expect(errors).toHaveLength(0); + expect(dto.website).toBeUndefined(); + }); + + it('should accept a valid website URL', async () => { + const dto = toDto({ website: 'https://www.cloudflare.com' }); + const errors = await validate(dto, { whitelist: true, forbidNonWhitelisted: true }); + expect(errors).toHaveLength(0); + }); + + it('should reject an invalid website URL', async () => { + const dto = toDto({ website: 'not-a-url' }); + const errors = await validate(dto, { whitelist: true, forbidNonWhitelisted: true }); + expect(errors.length).toBeGreaterThan(0); + expect(errors[0].property).toBe('website'); + }); + + // ── enum validation ─────────────────────────────────────────────── + it('should reject invalid category enum', async () => { + const dto = toDto({ category: 'invalid_category' }); + const errors = await validate(dto, { whitelist: true, forbidNonWhitelisted: true }); + expect(errors.length).toBeGreaterThan(0); + expect(errors[0].property).toBe('category'); + }); + + it('should reject invalid status enum', async () => { + const dto = toDto({ status: 'invalid_status' }); + const errors = await validate(dto, { whitelist: true, forbidNonWhitelisted: true }); + expect(errors.length).toBeGreaterThan(0); + expect(errors[0].property).toBe('status'); + }); + + // ── forbidNonWhitelisted ────────────────────────────────────────── + it('should reject unknown properties', async () => { + const dto = toDto({ name: 'Acronis', unknownField: 'value' }); + const errors = await validate(dto, { whitelist: true, forbidNonWhitelisted: true }); + expect(errors.length).toBeGreaterThan(0); + expect(errors.some((e) => e.property === 'unknownField')).toBe(true); + }); +});