Assist_Design/REFACTORING_FINDINGS.md

3.4 KiB
Raw Blame History

Codebase Audit - Redundancies and Inconsistencies

🔍 Findings

1. Duplicate Address Type Aliases ⚠️

Location: packages/domain/auth/contract.ts and packages/domain/customer/contract.ts

Both files export Address as an alias for CustomerAddress:

// customer/contract.ts
export type Address = CustomerAddress;

// auth/contract.ts  
export type Address = CustomerAddress;

Issue: Redundant alias in auth domain Fix: Remove from auth/contract.ts, import from customer domain


2. Inconsistent Error Handling in UsersService ⚠️

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

Issues:

  • Some methods throw generic Error() instead of NestJS exceptions
  • Inconsistent error messages
  • Some errors expose technical details

Examples:

// ❌ Generic Error (lines 68, 84, 98, 119, 172, 194, 218, 469)
throw new Error("Failed to find user");
throw new Error("Failed to retrieve customer profile");

// ✅ Proper NestJS exception (lines 129, 133, 233, 248-256)
throw new NotFoundException("User not found");
throw new BadRequestException("Unable to update profile.");

Fix: Use NestJS exceptions consistently:

  • NotFoundException for not found errors
  • BadRequestException for validation/client errors
  • Never throw generic Error() in services

3. Unused/Redundant Imports in UsersService

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

import { Activity } from "@customer-portal/domain/auth";  // Used
import type { CustomerAddress } from "@customer-portal/domain/customer";  // Not used (can use Address alias)

Fix: Use Address alias from domain for consistency


4. Hardcoded Currency in Dashboard ⚠️

Location: apps/bff/src/modules/users/users.service.ts:460

currency: "JPY",  // Hardcoded

Issue: Should come from user's WHMCS profile or config Fix: Fetch from WHMCS client data


5. TODO Comment Not Implemented

Location: apps/bff/src/modules/users/users.service.ts:459

openCases: 0, // TODO: Implement support cases when ready

Fix: Either implement or remove TODO if not planned


6. SalesforceService Unused Health Check ⚠️

Location: apps/bff/src/modules/users/users.service.ts:136-163

In the old getProfile() method:

// Check Salesforce health flag (do not override fields)
if (mapping?.sfAccountId) {
  try {
    await this.salesforceService.getAccount(mapping.sfAccountId);
  } catch (error) {
    // Just logs error, doesn't use the data
  }
}

Issue: Fetches Salesforce account but doesn't use it Fix: Already fixed in new implementation - removed


Previously Fixed Issues

  1. Removed Profile Fields from DB - firstName, lastName, company, phone
  2. Unified Update Endpoint - Single PATCH /me for profile + address
  3. Removed Deprecated Types - UpdateProfileRequest, UpdateAddressRequest, UserProfile, User
  4. Cleaned Mapper Utilities - Renamed to mapPrismaUserToAuthState
  5. Single Source of Truth - WHMCS for all profile data

Priority 1 (Critical)

  1. Fix inconsistent error handling in UsersService
  2. Remove duplicate Address alias from auth domain

Priority 2 (Important)

  1. Fix hardcoded currency
  2. Clean up redundant imports

Priority 3 (Nice to have)

  1. Remove or implement TODO comments