9.5 KiB
Validation Cleanup - Implementation Summary
Overview
This document summarizes the validation cleanup implementation completed on the customer portal codebase to eliminate redundant validation logic and establish consistent patterns.
Changes Implemented
Phase 1: Eliminate Duplicate Logic ✅
1. Removed Duplicate Password Validation
File: apps/bff/src/modules/auth/infra/workflows/workflows/password-workflow.service.ts
Before: Manual regex validation duplicated the passwordSchema
if (
newPassword.length < 8 ||
!/^(?=.*[a-z])(?=.*[A-Z])(?=.*\d)(?=.*[@$!%*?&])[A-Za-z\d@$!%*?&]*$/.test(newPassword)
) {
throw new BadRequestException("Password must be at least 8 characters...");
}
After: Relies on schema validation only
// Password validation is handled by changePasswordRequestSchema (uses passwordSchema from domain)
// No need for duplicate validation here
Impact: Eliminated 8 lines of duplicate validation logic. Password rules are now defined in one place: domain/common/schema.ts
2. Consolidated SKU Validation
File: packages/domain/orders/validation.ts
Before: Refinements had verbose if-else logic
.refine((data) => {
if (data.orderType === "SIM") {
return hasSimServicePlan(data.skus);
}
return true;
})
After: Concise validation using helper functions
.refine(
(data) => data.orderType !== "SIM" || hasSimServicePlan(data.skus),
{ message: "SIM orders must include a SIM service plan", path: ["skus"] }
)
Impact: Improved readability and ensured helper functions are the single source of truth for SKU validation logic.
3. Simplified Sanitization Functions
File: packages/domain/mappings/validation.ts
Before: Sanitization functions performed validation
export function sanitizeCreateRequest(request: CreateMappingRequest): CreateMappingRequest {
const validationResult = createMappingRequestSchema.safeParse({
userId: request.userId?.trim(),
whmcsClientId: request.whmcsClientId,
sfAccountId: request.sfAccountId?.trim() || undefined,
});
if (validationResult.success) {
return validationResult.data;
}
// Fallback to original behavior if validation fails
return { /* ... */ };
}
After: Pure sanitization, no validation
/**
* Sanitize and normalize a create mapping request
*
* Note: This performs basic string trimming before validation.
* The schema handles validation; this is purely for data cleanup.
*/
export function sanitizeCreateRequest(request: CreateMappingRequest): CreateMappingRequest {
return {
userId: request.userId?.trim(),
whmcsClientId: request.whmcsClientId,
sfAccountId: request.sfAccountId?.trim() || undefined,
};
}
Impact: Clear separation of concerns - sanitization does not validate, schemas validate.
Phase 2: Establish Patterns ✅
Documented in docs/VALIDATION_PATTERNS.md:
- Controller validation pattern:
@UsePipes(ZodValidationPipe) - Service validation pattern: Use domain validation functions
- Domain structure:
schema.tsfor schemas,validation.tsfor business logic
Phase 3: Refactor Problem Areas ✅
1. Fixed Manual AccountId Validation
File: apps/bff/src/modules/orders/services/order-fulfillment-validator.service.ts
Before: Manual type checking
const accountId = sfOrder.AccountId;
if (typeof accountId !== "string" || accountId.length === 0) {
throw new BadRequestException("Salesforce order is missing AccountId");
}
After: Schema-based validation
// Schema for validating Salesforce Account ID
const salesforceAccountIdSchema = z.string().min(1, "Salesforce AccountId is required");
// Validate AccountId using schema instead of manual type checks
const accountId = salesforceAccountIdSchema.parse(sfOrder.AccountId);
Impact: Consistent validation approach; type safety guaranteed by schema.
2. Simplified Common Validation Wrappers
File: packages/domain/common/validation.ts
Before: Unnecessary complexity with safeParse
export function validateUuidV4OrThrow(id: string): string {
const result = uuidSchema.safeParse(id);
if (!result.success) {
throw new Error("Invalid user ID format");
}
return result.data;
}
After: Direct use of .parse() with documentation
/**
* Validate a UUID (v4)
*
* This is a convenience wrapper that throws on invalid input.
* For validation without throwing, use the uuidSchema directly with .safeParse()
*
* @throws Error if UUID format is invalid
*/
export function validateUuidV4OrThrow(id: string): string {
try {
return uuidSchema.parse(id);
} catch {
throw new Error("Invalid user ID format");
}
}
Impact: Cleaner code with clear documentation on when to use wrappers vs schemas directly.
Phase 4: Documentation ✅
Created Comprehensive Guide
File: docs/VALIDATION_PATTERNS.md
Contents:
- Core principles (Schema-First, Single Source of Truth, Layer Separation, No Duplicates)
- Architecture overview (Domain layer structure, BFF layer structure)
- 6 validation patterns with ✅ DO / ❌ DON'T examples
- Common anti-patterns to avoid
- Migration guide for refactoring existing code
- Testing guidelines
Impact: Establishes consistent validation patterns for all future development.
Files Modified
High Priority Changes
- ✅
apps/bff/src/modules/auth/infra/workflows/workflows/password-workflow.service.ts - ✅
packages/domain/orders/validation.ts - ✅
packages/domain/mappings/validation.ts - ✅
apps/bff/src/modules/orders/services/order-fulfillment-validator.service.ts - ✅
packages/domain/common/validation.ts
Documentation
- ✅
docs/VALIDATION_PATTERNS.md(new) - ✅
docs/VALIDATION_CLEANUP_SUMMARY.md(this file)
Validation Issues Remaining
The following validation issues were identified but are acceptable as-is or low priority:
Acceptable (Business Logic, Not Validation Issues)
1. SIM Validation Service Manual Field Matching
Location: apps/bff/src/modules/subscriptions/sim-management/services/sim-validation.service.ts
Status: Acceptable - This service deals with WHMCS API response structure, not input validation. The manual field name matching is necessary to handle dynamic custom fields from WHMCS.
Rationale: External API responses require flexible parsing; this is not input validation.
2. Order Validator Internet Duplication Check
Location: apps/bff/src/modules/orders/services/order-validator.service.ts:147
const hasInternet = existing.some((product: WhmcsProduct) =>
(product.groupname || product.translated_groupname || "").toLowerCase().includes("internet")
);
Status: Acceptable - Querying external WHMCS data to check business rules.
Rationale: This is business logic operating on external system data, not input validation.
Low Priority (Pattern Consistency)
1. Inconsistent Validation Patterns Across Controllers
Some controllers use @UsePipes, some validate in services. This should be standardized over time to always use @UsePipes in controllers.
Effort: Low Impact: Low (functionality works correctly) Recommendation: Address during future refactoring
Benefits Achieved
- Reduced Duplication: Eliminated duplicate validation logic in 5 files
- Single Source of Truth: Each validation rule now defined once
- Improved Maintainability: Changes to validation rules only need to happen in one place
- Better Documentation: Clear guidelines for future development
- Type Safety: Consistent use of Zod schemas ensures runtime type safety
- Cleaner Code: Removed verbose if-else chains and manual type checks
Testing Recommendations
All modified files should be tested to ensure:
- Password validation still works correctly in auth flows
- Order validation catches invalid SKU combinations
- Mapping sanitization properly trims whitespace
- Order fulfillment validates AccountId correctly
- Common validation wrappers throw appropriate errors
Test Commands
# Run unit tests
npm test
# Run integration tests
npm run test:e2e
# Check for linting errors
npm run lint
Future Improvements
Short Term
- Apply consistent validation patterns to remaining controllers
- Add unit tests for validation functions
- Document validation error codes
Long Term
- Consider extracting SIM field mapping to configuration
- Implement validation middleware for global error handling
- Create validation composition utilities for complex scenarios
Success Criteria Met ✅
- ✅ No duplicate validation logic between schemas and manual checks
- ✅ Consistent validation patterns documented
- ✅ Clear separation: schemas for format, functions for business rules
- ✅ All validation consolidated in domain layer where appropriate
- ✅ Comprehensive documentation created
Conclusion
The validation cleanup successfully eliminated redundant logic, established consistent patterns, and provided comprehensive documentation for future development. The codebase now follows a clear schema-first validation approach with proper separation of concerns between format validation (schemas) and business validation (functions).
All high-priority issues have been resolved, and the remaining items are either acceptable as-is (business logic, not validation) or low-priority pattern consistency improvements that can be addressed incrementally.