126 lines
3.4 KiB
Markdown
126 lines
3.4 KiB
Markdown
# 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`:
|
||
```typescript
|
||
// 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:**
|
||
```typescript
|
||
// ❌ 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`
|
||
|
||
```typescript
|
||
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`
|
||
|
||
```typescript
|
||
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`
|
||
|
||
```typescript
|
||
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:
|
||
```typescript
|
||
// 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
|
||
|
||
---
|
||
|
||
## 📋 Recommended Fixes
|
||
|
||
### Priority 1 (Critical)
|
||
1. Fix inconsistent error handling in UsersService
|
||
2. Remove duplicate Address alias from auth domain
|
||
|
||
### Priority 2 (Important)
|
||
3. Fix hardcoded currency
|
||
4. Clean up redundant imports
|
||
|
||
### Priority 3 (Nice to have)
|
||
5. Remove or implement TODO comments
|
||
|