Assist_Design/docs/_archive/VALIDATION-CLEANUP-COMPLETE.md

9.0 KiB

Validation Cleanup - Complete Implementation

Summary

Successfully removed all unnecessary validation wrapper functions and replaced them with direct schema usage. This eliminates 3+ layers of wrapper functions and makes the validation logic clear and maintainable.

Changes Made

1. Domain Layer - Removed Redundant Wrapper Functions

File: packages/domain/mappings/validation.ts

Removed Functions (these just wrapped schemas with safeParse):

  • validateCreateRequest() - Just called schema.safeParse and returned result
  • validateUpdateRequest() - Just called schema.safeParse and returned result
  • validateExistingMapping() - Just called schema.safeParse and returned result
  • validateBulkMappings() - Just mapped over validateCreateRequest

Kept Functions (these have actual business logic):

  • validateNoConflicts() - Checks against existing mappings (business rule)
  • validateDeletion() - Adds warnings about deletion impact (business rule)
  • checkMappingCompleteness() - Returns warnings for optional fields (business logic)
  • sanitizeCreateRequest() - Pure sanitization (no validation)
  • sanitizeUpdateRequest() - Pure sanitization (no validation)

Before:

export function validateCreateRequest(request: CreateMappingRequest): MappingValidationResult {
  const validationResult = createMappingRequestSchema.safeParse(request);
  if (validationResult.success) {
    const warnings: string[] = [];
    if (!request.sfAccountId) {
      warnings.push("Salesforce account ID not provided - mapping will be incomplete");
    }
    return { isValid: true, errors: [], warnings };
  }
  const errors = validationResult.error.issues.map(issue => issue.message);
  return { isValid: false, errors, warnings: [] };
}

After:

export function checkMappingCompleteness(request: CreateMappingRequest | UserIdMapping): string[] {
  const warnings: string[] = [];
  if (!request.sfAccountId) {
    warnings.push("Salesforce account ID not provided - mapping will be incomplete");
  }
  return warnings;
}
// Use schema directly: createMappingRequestSchema.parse(request)

2. BFF Layer - Deleted Wrapper Service

Deleted File: apps/bff/src/modules/id-mappings/validation/mapping-validator.service.ts

This service was just another wrapper that added logging around domain validation functions.

Before (3 layers of wrapping):

Schema -> Domain Validation Function -> BFF Validator Service -> Actual Usage

After (direct schema usage):

Schema -> Actual Usage (with business validation functions when needed)

3. BFF Layer - Updated MappingsService

File: apps/bff/src/modules/id-mappings/mappings.service.ts

Before:

const validation = this.validator.validateCreateRequest(request);
if (!validation.isValid) {
  throw new BadRequestException(`Invalid mapping data: ${validation.errors.join(", ")}`);
}
const sanitizedRequest = this.validator.sanitizeCreateRequest(request);

After:

try {
  const sanitized = sanitizeCreateRequest(request);
  validatedRequest = createMappingRequestSchema.parse(sanitized);
} catch (error) {
  if (error instanceof ZodError) {
    const errors = error.issues.map(issue => issue.message);
    this.logger.warn({ request, errors }, "Create mapping request validation failed");
    throw new BadRequestException(`Invalid mapping data: ${errors.join(", ")}`);
  }
  throw error;
}
const warnings = checkMappingCompleteness(validatedRequest);

Benefits:

  • Direct schema usage - no intermediary wrappers
  • Clear error handling with ZodError
  • Still gets warnings for business logic
  • Better type safety (validatedRequest is properly typed)

4. BFF Layer - Simplified UsersService

File: apps/bff/src/modules/users/users.service.ts

Removed unnecessary private wrapper methods:

  • private validateEmail(email: string) - Just called normalizeAndValidateEmail
  • private validateUserId(id: string) - Just called validateUuidV4OrThrow

Before:

private validateEmail(email: string): string {
  return normalizeAndValidateEmail(email);
}

async findByEmail(email: string): Promise<User | null> {
  const validEmail = this.validateEmail(email);
  // ...
}

After:

async findByEmail(email: string): Promise<User | null> {
  const validEmail = normalizeAndValidateEmail(email);
  // ...
}

All 6 usages updated:

  • findByEmail() - now calls normalizeAndValidateEmail directly
  • findByEmailInternal() - now calls normalizeAndValidateEmail directly
  • create() - now calls normalizeAndValidateEmail directly
  • findByIdInternal() - now calls validateUuidV4OrThrow directly
  • findById() - now calls validateUuidV4OrThrow directly
  • update() - now calls validateUuidV4OrThrow directly
  • updateProfile() - now calls validateUuidV4OrThrow directly

5. Module Updates

File: apps/bff/src/modules/id-mappings/mappings.module.ts

Removed MappingValidatorService from providers and exports:

Before:

providers: [MappingsService, MappingCacheService, MappingValidatorService],
exports: [MappingsService, MappingCacheService, MappingValidatorService],

After:

providers: [MappingsService, MappingCacheService],
exports: [MappingsService, MappingCacheService],

Files Modified

  1. packages/domain/mappings/validation.ts - Removed 4 wrapper functions, kept business logic
  2. apps/bff/src/modules/id-mappings/mappings.service.ts - Use schemas directly
  3. apps/bff/src/modules/id-mappings/mappings.module.ts - Removed validator service
  4. apps/bff/src/modules/users/users.service.ts - Removed private wrapper methods
  5. apps/bff/src/modules/id-mappings/validation/mapping-validator.service.ts - DELETED

Benefits Achieved

1. Eliminated Wrapper Hell

Before: Schema -> Domain Function -> Service Wrapper -> Private Method -> Usage After: Schema -> Usage

2. Clearer Code

  • No more unnecessary intermediary functions
  • Direct schema usage makes validation explicit
  • Easier to understand what validation is happening

3. Better Type Safety

  • schema.parse() returns properly typed data
  • No custom result objects to pass around
  • TypeScript can better infer types

4. Reduced Maintenance

  • One place to update validation rules (the schema)
  • No risk of wrappers getting out of sync with schemas
  • Less code to maintain overall

5. Consistent Patterns

  • All services now use schemas directly
  • Common validation functions (from domain/common) used consistently
  • Business validation separated from format validation

Code Reduction

Lines Removed:

  • Domain wrapper functions: ~70 lines
  • BFF validator service: ~83 lines
  • Private wrapper methods: ~10 lines
  • Module configuration: ~2 lines
  • Total: ~165 lines of unnecessary code removed

Validation Pattern Now

Format Validation (Use Schemas Directly)

import { createMappingRequestSchema } from "@customer-portal/domain/mappings";

try {
  const validated = createMappingRequestSchema.parse(request);
  // Use validated data
} catch (error) {
  if (error instanceof ZodError) {
    // Handle validation errors
  }
}

Business Validation (Use Domain Functions)

import { validateNoConflicts, checkMappingCompleteness } from "@customer-portal/domain/mappings";

// Check business rules
const conflicts = validateNoConflicts(request, existingMappings);
if (!conflicts.isValid) {
  throw new ConflictException(conflicts.errors.join(", "));
}

// Check for warnings
const warnings = checkMappingCompleteness(request);
if (warnings.length > 0) {
  logger.warn({ warnings });
}

Sanitization (Use Domain Functions)

import { sanitizeCreateRequest } from "@customer-portal/domain/mappings";

const sanitized = sanitizeCreateRequest(dirtyRequest);
const validated = createMappingRequestSchema.parse(sanitized);

Testing Recommendations

  1. Test schema validation - Ensure all validation rules still work
  2. Test business validation - Ensure validateNoConflicts and validateDeletion work
  3. Test sanitization - Ensure trimming and normalization work
  4. Integration tests - Test full flows (create/update/delete mappings)
  5. User service tests - Test email and UUID validation

Next Steps

This cleanup pattern should be applied to other areas:

  • Order validation (already improved but could simplify further)
  • Auth validation (similar patterns exist)
  • Any other services with private validate*() methods

Success Metrics

No linter errors All wrapper functions removed or simplified Direct schema usage throughout Business validation separated from format validation ~165 lines of code removed Clearer, more maintainable validation logic

Documentation

See also:

  • docs/VALIDATION_PATTERNS.md - Comprehensive validation pattern guide
  • docs/VALIDATION_CLEANUP_SUMMARY.md - Original cleanup summary

Validation is now simple, clear, and maintainable! 🎉