179 lines
6.2 KiB
Markdown
179 lines
6.2 KiB
Markdown
# Architecture Fixes Summary
|
|
|
|
**Date**: October 9, 2025
|
|
**Status**: In Progress
|
|
|
|
---
|
|
|
|
## ✅ Completed Fixes
|
|
|
|
### 1. **Fixed Circular Dependency**
|
|
- **Issue**: `common/index.ts` tried to re-export `Address` from `customer` domain
|
|
- **Fix**: Removed circular import. `Address` should be imported directly from `@customer-portal/domain/customer`
|
|
|
|
### 2. **Added Missing Utility Functions**
|
|
- **getCurrencyLocale()** in `toolkit/formatting/currency.ts`
|
|
- **AsyncState types** in `toolkit/typing/helpers.ts` (createLoadingState, isSuccess, etc.)
|
|
- **response-helpers.ts** in portal (getNullableData, etc.)
|
|
|
|
### 3. **Fixed Domain Exports**
|
|
- Added `ProfileEditFormData`, `ProfileDisplayData`, `UserProfile`, `AuthenticatedUser` to customer domain
|
|
- Added `profileEditFormSchema` and `profileFormToRequest()` helper
|
|
- Added Address field helpers: `getStreet()`, `getStreetLine2()`, `getPostalCode()`
|
|
|
|
### 4. **Corrected Import Paths** (60+ files)
|
|
- ❌ **Before**: Everything imported from `@customer-portal/domain/billing`
|
|
- ✅ **After**: Imports from correct domains:
|
|
- `Address` → `@customer-portal/domain/customer`
|
|
- `Subscription` → `@customer-portal/domain/subscriptions`
|
|
- `PaymentMethod` → `@customer-portal/domain/payments`
|
|
- `formatCurrency` → `@customer-portal/domain/toolkit`
|
|
- Catalog types → `@customer-portal/domain/catalog`
|
|
|
|
### 5. **Normalized User Field Names**
|
|
- **Issue**: User schema had mixed naming (snake_case from WHMCS vs camelCase)
|
|
- **Fix**: Normalized to proper camelCase in User schema:
|
|
- `firstname` → `firstName`
|
|
- `lastname` → `lastName`
|
|
- `phonenumber` → `phoneNumber`
|
|
- `fullname` → `fullName`
|
|
- `companyname` → `companyName`
|
|
- **Mapping**: `combineToUser()` now maps WHMCS snake_case → User camelCase at the boundary
|
|
|
|
### 6. **Improved formatCurrency Signature**
|
|
- **Old**: `formatCurrency(amount, currency, options)` - rigid signature
|
|
- **New**: `formatCurrency(amount, currencyOrOptions?, options?)` - flexible:
|
|
- `formatCurrency(1000, "JPY")` - pass currency string
|
|
- `formatCurrency(1000, user.currencyCode)` - use user's currency from profile
|
|
- `formatCurrency(1000, { showSymbol: false })` - pass options only
|
|
|
|
### 7. **Address Field Strategy**
|
|
- **Decision**: Keep WHMCS field names (`address1`, `address2`, `postcode`) in Address type
|
|
- **Reason**: These match backend/database, avoiding unnecessary mapping
|
|
- **Helpers**: Provided `getStreet()`, `getPostalCode()` for backwards compatibility where needed
|
|
|
|
---
|
|
|
|
## 🔧 Issues Identified (Pending Fixes)
|
|
|
|
### 1. **⚠️ Business Logic in Frontend** (CRITICAL)
|
|
|
|
**Location**: `apps/portal/src/features/catalog/utils/catalog.utils.ts`
|
|
|
|
**Issue**: Pricing calculations (monthly price, setup fees, etc.) are being done in the frontend utils.
|
|
|
|
**Why This Is Wrong**:
|
|
- Business logic should be in the BFF or domain layer
|
|
- Frontend should only display data, not calculate it
|
|
- Makes it harder to ensure pricing consistency
|
|
- Violates separation of concerns
|
|
|
|
**Correct Approach**:
|
|
```typescript
|
|
// ❌ BAD: Frontend calculates pricing
|
|
const monthlyPrice = getMonthlyPrice(product); // In catalog.utils.ts
|
|
|
|
// ✅ GOOD: BFF sends pre-calculated prices
|
|
const monthlyPrice = product.monthlyPrice; // Already calculated by BFF
|
|
```
|
|
|
|
**Action Required**:
|
|
1. Move pricing logic to BFF catalog service
|
|
2. Have BFF calculate and include all display prices in API response
|
|
3. Frontend just displays `product.monthlyPrice`, `product.setupFee`, etc.
|
|
4. Delete `getMonthlyPrice()` and similar functions from frontend utils
|
|
|
|
---
|
|
|
|
### 2. **Field Name Inconsistencies** (Remaining ~40 errors)
|
|
|
|
**User Fields**: Need to update all portal code using User type:
|
|
- Change `user.firstName` → `user.firstName` ✅ (now correct)
|
|
- Change `user.phone` → `user.phoneNumber` (still needs updates in portal)
|
|
|
|
**Address Fields**: Components expecting different field names:
|
|
- Some code expects `address.street` → should use `address.address1` or `getStreet(address)`
|
|
- Some code expects `address.postalCode` → should use `address.postcode` or `getPostalCode(address)`
|
|
|
|
---
|
|
|
|
### 3. **formatCurrency Call Sites** (~10 errors)
|
|
|
|
Many components still passing currency as object instead of string:
|
|
```typescript
|
|
// ❌ Current (wrong)
|
|
formatCurrency(amount, { currency: "JPY", locale: "ja-JP" })
|
|
|
|
// ✅ Should be
|
|
formatCurrency(amount, "JPY", { locale: "ja-JP" })
|
|
// OR simply
|
|
formatCurrency(amount, invoice.currency)
|
|
```
|
|
|
|
---
|
|
|
|
## 📋 Remaining Work
|
|
|
|
### High Priority
|
|
1. [ ] Fix all `user.firstName`/`user.phoneNumber` references in portal
|
|
2. [ ] Fix all `formatCurrency` call signatures
|
|
3. [ ] Fix remaining catalog component imports
|
|
|
|
### Medium Priority
|
|
4. [ ] **Move pricing logic from frontend to BFF** (architectural fix)
|
|
5. [ ] Add missing schema properties (scheduledAt, billingCycle, SIM properties)
|
|
6. [ ] Fix Address field references (use helpers or direct WHMCS names)
|
|
|
|
### Low Priority
|
|
7. [ ] Update any remaining documentation
|
|
8. [ ] Add tests for new helper functions
|
|
|
|
---
|
|
|
|
## Design Decisions Made
|
|
|
|
### 1. **Single Field Naming Convention**
|
|
**Decision**: Normalize to camelCase at the domain boundary
|
|
**Rationale**:
|
|
- TypeScript/JavaScript convention
|
|
- Single transformation point (domain mapper)
|
|
- Consistent with frontend expectations
|
|
- Clear separation: WHMCS uses snake_case, domain exposes camelCase
|
|
|
|
### 2. **Address Field Names**
|
|
**Decision**: Keep WHMCS names (`address1`, `postcode`)
|
|
**Rationale**:
|
|
- Matches backend/database structure
|
|
- Avoids unnecessary mapping layer
|
|
- Provides helpers for backwards compatibility
|
|
|
|
### 3. **Currency Handling**
|
|
**Decision**: Get from User.currencyCode, not pass everywhere
|
|
**Rationale**:
|
|
- Currency is user property (from WHMCS profile)
|
|
- Reduces parameter passing
|
|
- Single source of truth
|
|
|
|
### 4. **Business Logic Location**
|
|
**Decision**: Pricing calculations belong in BFF
|
|
**Rationale**:
|
|
- Separation of concerns
|
|
- Frontend displays, doesn't calculate
|
|
- Easier to maintain consistency
|
|
- Follows clean architecture principles
|
|
|
|
---
|
|
|
|
## Next Steps
|
|
|
|
1. Complete remaining type error fixes (~40 errors)
|
|
2. Run full type check to verify
|
|
3. Move pricing logic to BFF (separate task/PR)
|
|
4. Test thoroughly
|
|
|
|
---
|
|
|
|
**Progress**: 10/16 major tasks completed (62%)
|
|
**Estimated Remaining**: 50-80 file edits for full type safety
|
|
|