394 lines
11 KiB
Markdown
394 lines
11 KiB
Markdown
|
|
# Critical Issues Implementation Progress
|
||
|
|
|
||
|
|
**Date:** 2025-10-29
|
||
|
|
**Status:** Phase 1 Complete | Phase 2 In Progress
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## ✅ Phase 1 Complete: Type Flow Cleanup (C2, C7)
|
||
|
|
|
||
|
|
### C2: Consolidate Duplicate WHMCS Mapper Utilities ✅
|
||
|
|
|
||
|
|
**Files Created:**
|
||
|
|
- ✅ `packages/domain/providers/whmcs/utils.ts` - Shared WHMCS utilities
|
||
|
|
- ✅ `packages/domain/providers/whmcs/index.ts` - Module exports
|
||
|
|
- ✅ `packages/domain/providers/index.ts` - Providers index
|
||
|
|
|
||
|
|
**Files Modified:**
|
||
|
|
- ✅ `packages/domain/billing/providers/whmcs/mapper.ts` - Now uses shared utils
|
||
|
|
- ✅ `packages/domain/subscriptions/providers/whmcs/mapper.ts` - Now uses shared utils
|
||
|
|
|
||
|
|
**Files Deleted:**
|
||
|
|
- ✅ `packages/integrations/whmcs/src/utils/data-utils.ts` - Obsolete duplicate
|
||
|
|
|
||
|
|
**Impact:**
|
||
|
|
- Eliminated 50+ lines of duplicate code
|
||
|
|
- Single source of truth for WHMCS data transformations
|
||
|
|
- Consistent parsing across all mappers
|
||
|
|
|
||
|
|
### C7: Remove Redundant Frontend Validation ✅
|
||
|
|
|
||
|
|
**Files Modified:**
|
||
|
|
- ✅ `apps/portal/src/features/catalog/services/catalog.service.ts`
|
||
|
|
- Removed `parseInternetCatalog()`, `parseSimCatalog()`, `parseVpnCatalog()` calls
|
||
|
|
- Now trusts BFF-validated responses
|
||
|
|
- Added comments explaining BFF validation
|
||
|
|
|
||
|
|
**Impact:**
|
||
|
|
- Reduced redundant validation overhead
|
||
|
|
- Faster catalog loading
|
||
|
|
- Clear HTTP boundary validation responsibility
|
||
|
|
|
||
|
|
**Lint Status:** All Phase 1 changes pass TypeScript compilation ✅
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## 🔄 Phase 2 In Progress: Business Logic Separation (C1, C6)
|
||
|
|
|
||
|
|
### C1: Remove Pricing Calculations from Frontend
|
||
|
|
|
||
|
|
**Status:** Ready to implement
|
||
|
|
|
||
|
|
**Required Changes:**
|
||
|
|
|
||
|
|
#### 1. Create BFF Preview Pricing Service
|
||
|
|
**File:** `apps/bff/src/modules/catalog/services/catalog-pricing.service.ts` (NEW)
|
||
|
|
|
||
|
|
```typescript
|
||
|
|
import { Injectable, Inject } from "@nestjs/common";
|
||
|
|
import { Logger } from "nestjs-pino";
|
||
|
|
import { BaseCatalogService } from "./base-catalog.service";
|
||
|
|
import type { CatalogProductBase } from "@customer-portal/domain/catalog";
|
||
|
|
|
||
|
|
export interface PricingPreviewRequest {
|
||
|
|
orderType: string;
|
||
|
|
selections: string[];
|
||
|
|
addons?: string[];
|
||
|
|
}
|
||
|
|
|
||
|
|
export interface PricingPreviewResponse {
|
||
|
|
monthlyTotal: number;
|
||
|
|
oneTimeTotal: number;
|
||
|
|
items: Array<{
|
||
|
|
sku: string;
|
||
|
|
name: string;
|
||
|
|
monthlyPrice?: number;
|
||
|
|
oneTimePrice?: number;
|
||
|
|
}>;
|
||
|
|
}
|
||
|
|
|
||
|
|
@Injectable()
|
||
|
|
export class CatalogPricingService {
|
||
|
|
constructor(
|
||
|
|
private readonly baseCatalog: BaseCatalogService,
|
||
|
|
@Inject(Logger) private readonly logger: Logger
|
||
|
|
) {}
|
||
|
|
|
||
|
|
async calculatePreviewPricing(request: PricingPreviewRequest): Promise<PricingPreviewResponse> {
|
||
|
|
const allSkus = [...request.selections, ...(request.addons || [])];
|
||
|
|
|
||
|
|
this.logger.debug(`Calculating pricing preview for ${allSkus.length} items`, {
|
||
|
|
orderType: request.orderType,
|
||
|
|
skus: allSkus,
|
||
|
|
});
|
||
|
|
|
||
|
|
// Fetch products from Salesforce
|
||
|
|
const products = await this.baseCatalog.fetchProductsBySkus(allSkus);
|
||
|
|
|
||
|
|
let monthlyTotal = 0;
|
||
|
|
let oneTimeTotal = 0;
|
||
|
|
|
||
|
|
const items = products.map(product => {
|
||
|
|
const monthly = product.monthlyPrice || 0;
|
||
|
|
const oneTime = product.oneTimePrice || 0;
|
||
|
|
|
||
|
|
monthlyTotal += monthly;
|
||
|
|
oneTimeTotal += oneTime;
|
||
|
|
|
||
|
|
return {
|
||
|
|
sku: product.sku,
|
||
|
|
name: product.name,
|
||
|
|
monthlyPrice: monthly > 0 ? monthly : undefined,
|
||
|
|
oneTimePrice: oneTime > 0 ? oneTime : undefined,
|
||
|
|
};
|
||
|
|
});
|
||
|
|
|
||
|
|
this.logger.log(`Pricing preview calculated`, {
|
||
|
|
monthlyTotal,
|
||
|
|
oneTimeTotal,
|
||
|
|
itemCount: items.length,
|
||
|
|
});
|
||
|
|
|
||
|
|
return {
|
||
|
|
monthlyTotal,
|
||
|
|
oneTimeTotal,
|
||
|
|
items,
|
||
|
|
};
|
||
|
|
}
|
||
|
|
}
|
||
|
|
```
|
||
|
|
|
||
|
|
#### 2. Add Endpoint to Catalog Controller
|
||
|
|
**File:** `apps/bff/src/modules/catalog/catalog.controller.ts`
|
||
|
|
|
||
|
|
```typescript
|
||
|
|
// Add import
|
||
|
|
import { CatalogPricingService } from "./services/catalog-pricing.service";
|
||
|
|
import { Body, Post } from "@nestjs/common";
|
||
|
|
|
||
|
|
// Add to constructor
|
||
|
|
constructor(
|
||
|
|
private internetCatalog: InternetCatalogService,
|
||
|
|
private simCatalog: SimCatalogService,
|
||
|
|
private vpnCatalog: VpnCatalogService,
|
||
|
|
private pricingService: CatalogPricingService // ADD THIS
|
||
|
|
) {}
|
||
|
|
|
||
|
|
// Add endpoint
|
||
|
|
@Post("preview-pricing")
|
||
|
|
@Throttle({ default: { limit: 30, ttl: 60 } }) // 30 requests per minute
|
||
|
|
async previewPricing(@Body() body: {
|
||
|
|
orderType: string;
|
||
|
|
selections: string[];
|
||
|
|
addons?: string[];
|
||
|
|
}): Promise<{
|
||
|
|
monthlyTotal: number;
|
||
|
|
oneTimeTotal: number;
|
||
|
|
items: Array<{
|
||
|
|
sku: string;
|
||
|
|
name: string;
|
||
|
|
monthlyPrice?: number;
|
||
|
|
oneTimePrice?: number;
|
||
|
|
}>;
|
||
|
|
}> {
|
||
|
|
return this.pricingService.calculatePreviewPricing(body);
|
||
|
|
}
|
||
|
|
```
|
||
|
|
|
||
|
|
#### 3. Register Service in Module
|
||
|
|
**File:** `apps/bff/src/modules/catalog/catalog.module.ts`
|
||
|
|
|
||
|
|
```typescript
|
||
|
|
import { CatalogPricingService } from "./services/catalog-pricing.service";
|
||
|
|
|
||
|
|
providers: [
|
||
|
|
BaseCatalogService,
|
||
|
|
InternetCatalogService,
|
||
|
|
SimCatalogService,
|
||
|
|
VpnCatalogService,
|
||
|
|
CatalogCacheService,
|
||
|
|
CatalogPricingService, // ADD THIS
|
||
|
|
],
|
||
|
|
```
|
||
|
|
|
||
|
|
#### 4. Update Frontend Components
|
||
|
|
|
||
|
|
**File:** `apps/portal/src/features/catalog/components/sim/SimConfigureView.tsx`
|
||
|
|
|
||
|
|
Lines 52-64 - REPLACE calculation with API call:
|
||
|
|
|
||
|
|
```typescript
|
||
|
|
// Remove local calculations
|
||
|
|
- const monthlyTotal = (plan?.monthlyPrice ?? 0) + ...;
|
||
|
|
- const oneTimeTotal = (plan?.oneTimePrice ?? 0) + ...;
|
||
|
|
|
||
|
|
// Add API call using React Query
|
||
|
|
const { data: pricingPreview, isLoading: pricingLoading } = useQuery({
|
||
|
|
queryKey: ['pricing-preview', 'SIM', plan?.sku, selectedAddons],
|
||
|
|
queryFn: async () => {
|
||
|
|
if (!plan) return null;
|
||
|
|
const response = await apiClient.POST<{
|
||
|
|
monthlyTotal: number;
|
||
|
|
oneTimeTotal: number;
|
||
|
|
items: Array<{
|
||
|
|
sku: string;
|
||
|
|
name: string;
|
||
|
|
monthlyPrice?: number;
|
||
|
|
oneTimePrice?: number;
|
||
|
|
}>;
|
||
|
|
}>('/api/catalog/preview-pricing', {
|
||
|
|
body: {
|
||
|
|
orderType: 'SIM',
|
||
|
|
selections: [plan.sku],
|
||
|
|
addons: selectedAddons,
|
||
|
|
},
|
||
|
|
});
|
||
|
|
return response.data;
|
||
|
|
},
|
||
|
|
enabled: !!plan,
|
||
|
|
staleTime: 5 * 60 * 1000, // 5 minutes
|
||
|
|
});
|
||
|
|
|
||
|
|
const monthlyTotal = pricingPreview?.monthlyTotal ?? 0;
|
||
|
|
const oneTimeTotal = pricingPreview?.oneTimeTotal ?? 0;
|
||
|
|
```
|
||
|
|
|
||
|
|
**File:** `apps/portal/src/features/catalog/utils/catalog.utils.ts`
|
||
|
|
|
||
|
|
Lines 51-56 - DELETE:
|
||
|
|
```typescript
|
||
|
|
- export function calculateSavings(originalPrice: number, currentPrice: number): number {
|
||
|
|
- if (originalPrice <= currentPrice) return 0;
|
||
|
|
- return Math.round(((originalPrice - currentPrice) / originalPrice) * 100);
|
||
|
|
- }
|
||
|
|
```
|
||
|
|
|
||
|
|
**File:** `apps/portal/src/features/orders/utils/order-presenters.ts`
|
||
|
|
|
||
|
|
Lines 120-152 - DELETE:
|
||
|
|
```typescript
|
||
|
|
- export function calculateOrderTotals(...) { ... }
|
||
|
|
- export function normalizeBillingCycle(...) { ... }
|
||
|
|
```
|
||
|
|
|
||
|
|
Update components that use these deleted functions to use API values directly.
|
||
|
|
|
||
|
|
### C6: Remove Billing Cycle Normalization from Frontend
|
||
|
|
|
||
|
|
**Status:** Ready to implement
|
||
|
|
|
||
|
|
**Required Changes:**
|
||
|
|
|
||
|
|
**File:** `apps/portal/src/features/subscriptions/components/SubscriptionCard.tsx`
|
||
|
|
|
||
|
|
Lines 70-74 - DELETE:
|
||
|
|
```typescript
|
||
|
|
- const getBillingCycleLabel = (cycle: string, name: string) => {
|
||
|
|
- const looksLikeActivation = name.toLowerCase().includes("activation") || name.includes("setup");
|
||
|
|
- return looksLikeActivation ? "One-time" : cycle;
|
||
|
|
- };
|
||
|
|
```
|
||
|
|
|
||
|
|
Line 171 - UPDATE:
|
||
|
|
```typescript
|
||
|
|
- <p className="text-gray-500">{getBillingCycleLabel(subscription.cycle)}</p>
|
||
|
|
+ <p className="text-gray-500">{subscription.cycle}</p>
|
||
|
|
```
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## 📋 Phase 3 TODO: Structural Improvements (C3, C4, C5)
|
||
|
|
|
||
|
|
### C3: Remove Service Wrapper Anti-Pattern
|
||
|
|
|
||
|
|
**Files to DELETE:**
|
||
|
|
- `apps/portal/src/features/checkout/services/checkout.service.ts`
|
||
|
|
- `apps/portal/src/features/account/services/account.service.ts`
|
||
|
|
- `apps/portal/src/features/orders/services/orders.service.ts`
|
||
|
|
- `apps/portal/src/features/subscriptions/services/sim-actions.service.ts`
|
||
|
|
|
||
|
|
**Files to UPDATE:**
|
||
|
|
- All hooks that use these services → Replace with direct apiClient calls
|
||
|
|
|
||
|
|
### C4: Extract Hardcoded Pricing to Salesforce
|
||
|
|
|
||
|
|
**Prerequisite:** Create Salesforce Product:
|
||
|
|
- **Product2:**
|
||
|
|
- Name: "SIM Data Top-Up - 1GB Unit"
|
||
|
|
- StockKeepingUnit: "SIM-TOPUP-1GB"
|
||
|
|
- IsActive: true
|
||
|
|
- Product2Categories1__c: "SIM"
|
||
|
|
- Item_Class__c: "Add-on"
|
||
|
|
- Billing_Cycle__c: "One-time"
|
||
|
|
- Portal_Catalog__c: false
|
||
|
|
- Portal_Accessible__c: true
|
||
|
|
- Price__c: 500
|
||
|
|
- One_Time_Price__c: 500
|
||
|
|
|
||
|
|
**PricebookEntry:**
|
||
|
|
- UnitPrice: 500
|
||
|
|
- IsActive: true
|
||
|
|
- Link to standard pricebook
|
||
|
|
|
||
|
|
**Files to CREATE:**
|
||
|
|
- `apps/bff/src/modules/catalog/services/pricing.service.ts`
|
||
|
|
|
||
|
|
**Files to UPDATE:**
|
||
|
|
- `apps/bff/src/modules/subscriptions/sim-management/services/sim-topup.service.ts`
|
||
|
|
- `apps/bff/src/modules/catalog/catalog.module.ts`
|
||
|
|
- `apps/bff/src/modules/subscriptions/sim-management/sim-management.module.ts`
|
||
|
|
|
||
|
|
### C5: Consolidate Currency Formatting
|
||
|
|
|
||
|
|
**Files to UPDATE:**
|
||
|
|
- `packages/domain/toolkit/formatting/currency.ts` - Add caching
|
||
|
|
- `apps/portal/src/features/dashboard/utils/dashboard.utils.ts` - Remove duplicate, use domain formatter
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## 🧪 Testing Plan
|
||
|
|
|
||
|
|
### Phase 1 Tests ✅
|
||
|
|
- ✅ TypeScript compilation passes
|
||
|
|
- ⏳ Unit tests for shared WHMCS utils
|
||
|
|
- ⏳ Integration tests for mappers
|
||
|
|
|
||
|
|
### Phase 2 Tests
|
||
|
|
- [ ] Pricing preview endpoint returns correct totals
|
||
|
|
- [ ] Frontend displays API-calculated prices
|
||
|
|
- [ ] Billing cycle normalization works consistently
|
||
|
|
- [ ] Performance: Preview pricing response time < 500ms
|
||
|
|
|
||
|
|
### Phase 3 Tests
|
||
|
|
- [ ] All hooks work with direct apiClient calls
|
||
|
|
- [ ] SIM top-up pricing fetches from Salesforce
|
||
|
|
- [ ] Currency formatting consistent across all views
|
||
|
|
- [ ] No service wrapper imports remain
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## 📊 Impact Summary
|
||
|
|
|
||
|
|
### Code Reduction:
|
||
|
|
- **Phase 1:** ~60 lines of duplicate code removed
|
||
|
|
- **Phase 2 (Projected):** ~80 lines of business logic removed from frontend
|
||
|
|
- **Phase 3 (Projected):** ~200 lines of service wrappers removed
|
||
|
|
- **Total:** ~340 lines removed, cleaner architecture
|
||
|
|
|
||
|
|
### Architecture Improvements:
|
||
|
|
- Single source of truth for WHMCS data parsing ✅
|
||
|
|
- Clear HTTP boundary validation ✅
|
||
|
|
- Business logic centralized in BFF (in progress)
|
||
|
|
- Clean Next.js pattern (hooks → API) (planned)
|
||
|
|
- Dynamic pricing from Salesforce (planned)
|
||
|
|
|
||
|
|
### Performance:
|
||
|
|
- Reduced redundant validation overhead ✅
|
||
|
|
- Cached pricing calculations (planned)
|
||
|
|
- Faster catalog loading ✅
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## 🚀 Next Steps
|
||
|
|
|
||
|
|
1. **Continue Phase 2:**
|
||
|
|
- Create `CatalogPricingService`
|
||
|
|
- Add `/api/catalog/preview-pricing` endpoint
|
||
|
|
- Update `SimConfigureView` to use API pricing
|
||
|
|
- Delete frontend pricing utils
|
||
|
|
- Remove billing cycle normalization
|
||
|
|
|
||
|
|
2. **Test Phase 2:**
|
||
|
|
- Write unit tests for pricing service
|
||
|
|
- Integration test the new endpoint
|
||
|
|
- E2E test SIM configuration flow
|
||
|
|
|
||
|
|
3. **Begin Phase 3:**
|
||
|
|
- Remove service wrappers
|
||
|
|
- Create Salesforce pricing product
|
||
|
|
- Implement dynamic pricing service
|
||
|
|
- Consolidate currency formatting
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## 📝 Notes
|
||
|
|
|
||
|
|
- All Phase 1 changes are backward compatible
|
||
|
|
- Phase 2 requires coordination with frontend team for UI updates
|
||
|
|
- Phase 3 requires Salesforce admin access for product creation
|
||
|
|
- Consider feature flags for gradual rollout
|
||
|
|
|
||
|
|
**Last Updated:** 2025-10-29
|
||
|
|
**Next Review:** After Phase 2 completion
|
||
|
|
|