Merge pull request #2373 from trycompai/mariano/fix-vendor-patch-400

[dev] [Marfuen] mariano/fix-vendor-patch-400

Author: Marfuen

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<string, unknown>): 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);
+  });
+});

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;
+}