Enhance session management and error handling in authentication components
- Added a new error mapping for "SESSION_EXPIRED" in SecureErrorMapperService to provide user-friendly messages and appropriate logging levels. - Updated SessionTimeoutWarning component to include a reason for logout when the session expires, improving user feedback. - Refactored useAuth hook to support enhanced logout functionality with reason options, ensuring better session management. - Improved auth.store to handle logout reasons and integrate with error handling for session refresh failures. - Streamlined LoginView to display logout messages based on session expiration, enhancing user experience.
This commit is contained in:
barsa
parent
749f89a83d
commit
05765d3513
998
CODEBASE_TYPE_FLOW_AUDIT.md
Normal file
998
CODEBASE_TYPE_FLOW_AUDIT.md
Normal file
@ -0,0 +1,998 @@
|
||||
# Codebase Type Flow Audit Report
|
||||
**Date:** 2025-10-29
|
||||
**Auditor:** AI Assistant
|
||||
**Scope:** API Types, Converters/Normalizers, Business Logic Distribution, Type Consistency
|
||||
|
||||
---
|
||||
|
||||
## Executive Summary
|
||||
|
||||
This audit examined the data flow from external APIs (Salesforce, WHMCS, Freebit) through the BFF layer to the frontend portal, analyzing type definitions, conversion layers, and business logic distribution.
|
||||
|
||||
**Key Findings:**
|
||||
- 🔴 **7 Critical Issues** requiring immediate attention
|
||||
- 🟡 **5 Medium Priority Issues** impacting maintainability
|
||||
- 🟢 **3 Low Priority Improvements** for optimization
|
||||
|
||||
**Overall Assessment:** The codebase demonstrates good architectural separation with centralized domain types, but suffers from:
|
||||
1. Business logic leaking into frontend utils (pricing calculations)
|
||||
2. Duplicate mapper implementations across packages
|
||||
3. Anti-pattern service wrappers in frontend
|
||||
4. Inconsistent currency formatting implementations
|
||||
|
||||
---
|
||||
|
||||
## 🔴 Critical Issues (Priority 1)
|
||||
|
||||
### C1. Business Logic in Frontend: Pricing Calculations
|
||||
|
||||
**Location:**
|
||||
- `apps/portal/src/features/catalog/utils/catalog.utils.ts` (Lines 51-56)
|
||||
- `apps/portal/src/features/orders/utils/order-presenters.ts` (Lines 120-144)
|
||||
- `apps/portal/src/features/catalog/components/sim/SimConfigureView.tsx` (Lines 52-64)
|
||||
|
||||
**Problem:**
|
||||
Frontend components are calculating pricing totals, billing cycle conversions, and savings percentages. This violates clean architecture as business rules should reside in the BFF/domain layer.
|
||||
|
||||
**Evidence:**
|
||||
```typescript
|
||||
// apps/portal/src/features/catalog/components/sim/SimConfigureView.tsx:52-64
|
||||
const monthlyTotal =
|
||||
(plan?.monthlyPrice ?? 0) +
|
||||
selectedAddons.reduce((sum, addonSku) => {
|
||||
const addon = addons.find(a => a.sku === addonSku);
|
||||
return sum + (addon?.monthlyPrice ?? 0);
|
||||
}, 0);
|
||||
```
|
||||
|
||||
```typescript
|
||||
// apps/portal/src/features/catalog/utils/catalog.utils.ts:53-56
|
||||
export function calculateSavings(originalPrice: number, currentPrice: number): number {
|
||||
if (originalPrice <= currentPrice) return 0;
|
||||
return Math.round(((originalPrice - currentPrice) / originalPrice) * 100);
|
||||
}
|
||||
```
|
||||
|
||||
```typescript
|
||||
// apps/portal/src/features/orders/utils/order-presenters.ts:120-144
|
||||
export function calculateOrderTotals(
|
||||
items: Array<{ totalPrice?: number; billingCycle?: string }> | undefined,
|
||||
fallbackTotal?: number
|
||||
): OrderTotals {
|
||||
let monthlyTotal = 0;
|
||||
let oneTimeTotal = 0;
|
||||
|
||||
if (items && items.length > 0) {
|
||||
for (const item of items) {
|
||||
const total = item.totalPrice ?? 0;
|
||||
const billingCycle = normalizeBillingCycle(item.billingCycle);
|
||||
if (billingCycle === "monthly") {
|
||||
monthlyTotal += total;
|
||||
} else if (billingCycle === "onetime") {
|
||||
oneTimeTotal += total;
|
||||
} else {
|
||||
monthlyTotal += total;
|
||||
}
|
||||
}
|
||||
}
|
||||
return { monthlyTotal, oneTimeTotal };
|
||||
}
|
||||
```
|
||||
|
||||
**Impact:**
|
||||
- 🔴 **Risk:** Frontend can show incorrect prices if calculations diverge from BFF
|
||||
- 🔴 **Maintainability:** Business rules are duplicated and harder to update
|
||||
- 🔴 **Testing:** Business logic not centrally testable
|
||||
- 🔴 **Data Integrity:** Source of truth is unclear
|
||||
|
||||
**Root Cause:**
|
||||
BFF checkout service already calculates totals (line 98-111 in `apps/bff/src/modules/orders/services/checkout.service.ts`), but frontend recalculates for display purposes.
|
||||
|
||||
**Fix:**
|
||||
1. **BFF Enhancement:** Ensure all pricing calculations are done in `CheckoutService.calculateTotals()` and returned in API response
|
||||
2. **Frontend Cleanup:** Remove pricing calculations from frontend, use API-provided values only
|
||||
3. **Display-Only Logic:** Frontend should only format prices, never calculate them
|
||||
|
||||
**Diff-Ready Plan:**
|
||||
```diff
|
||||
# Step 1: Remove pricing calculations from frontend utils
|
||||
- apps/portal/src/features/catalog/utils/catalog.utils.ts:51-56 (delete calculateSavings)
|
||||
- apps/portal/src/features/orders/utils/order-presenters.ts:120-152 (delete calculateOrderTotals, normalizeBillingCycle)
|
||||
|
||||
# Step 2: Update BFF to provide calculated totals
|
||||
+ apps/bff/src/modules/orders/services/checkout.service.ts:98-111
|
||||
Ensure CheckoutTotals includes all needed fields (monthlyTotal, oneTimeTotal, subtotal, tax, total)
|
||||
|
||||
# Step 3: Update frontend components to use API totals
|
||||
- apps/portal/src/features/catalog/components/sim/SimConfigureView.tsx:52-64
|
||||
Replace local calculation with props from API response
|
||||
|
||||
# Step 4: Add domain schema validation
|
||||
+ packages/domain/orders/schema.ts
|
||||
Add checkoutTotalsSchema with all pricing fields
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### C2. Duplicate Mapper Implementations
|
||||
|
||||
**Location:**
|
||||
- `packages/domain/billing/providers/whmcs/mapper.ts` (Lines 50-61, 76-90)
|
||||
- `packages/domain/subscriptions/providers/whmcs/mapper.ts` (Lines 64-75, 77-88)
|
||||
- `packages/integrations/whmcs/src/mappers/*` (Re-exports only)
|
||||
|
||||
**Problem:**
|
||||
The same parsing and mapping logic (`parseAmount`, `formatDate`, `mapStatus`, `mapCycle`) is duplicated across multiple mapper files.
|
||||
|
||||
**Evidence:**
|
||||
```typescript
|
||||
// packages/domain/billing/providers/whmcs/mapper.ts:50-61
|
||||
function parseAmount(amount: string | number | undefined): number {
|
||||
if (typeof amount === "number") {
|
||||
return amount;
|
||||
}
|
||||
if (!amount) {
|
||||
return 0;
|
||||
}
|
||||
const cleaned = String(amount).replace(/[^\d.-]/g, "");
|
||||
const parsed = Number.parseFloat(cleaned);
|
||||
return Number.isNaN(parsed) ? 0 : parsed;
|
||||
}
|
||||
|
||||
// packages/domain/subscriptions/providers/whmcs/mapper.ts:64-75
|
||||
function parseAmount(amount: string | number | undefined): number {
|
||||
if (typeof amount === "number") {
|
||||
return amount;
|
||||
}
|
||||
if (!amount) {
|
||||
return 0;
|
||||
}
|
||||
const cleaned = String(amount).replace(/[^\d.-]/g, "");
|
||||
const parsed = Number.parseFloat(cleaned);
|
||||
return Number.isNaN(parsed) ? 0 : parsed;
|
||||
}
|
||||
```
|
||||
|
||||
Same for `formatDate` functions (both files lines 77-88 and 63-74).
|
||||
|
||||
**Impact:**
|
||||
- 🔴 **Maintainability:** Changes must be synchronized across multiple files
|
||||
- 🔴 **Consistency:** Risk of divergent implementations
|
||||
- 🟡 **Code Size:** Unnecessary duplication increases bundle size
|
||||
|
||||
**Fix:**
|
||||
Create shared WHMCS mapper utilities in domain package.
|
||||
|
||||
**Diff-Ready Plan:**
|
||||
```diff
|
||||
# Step 1: Create shared utilities
|
||||
+ packages/domain/providers/whmcs/utils.ts
|
||||
export function parseAmount(amount: string | number | undefined): number
|
||||
export function formatDate(input?: string | null): string | undefined
|
||||
export function normalizeWhmcsStatus(status: string, statusMap: Record<string, string>): string
|
||||
export function normalizeWhmcsCycle(cycle: string, cycleMap: Record<string, string>): string
|
||||
|
||||
# Step 2: Update billing mapper
|
||||
- packages/domain/billing/providers/whmcs/mapper.ts:50-74 (delete duplicates)
|
||||
+ packages/domain/billing/providers/whmcs/mapper.ts:1
|
||||
import { parseAmount, formatDate } from "../../providers/whmcs/utils"
|
||||
|
||||
# Step 3: Update subscriptions mapper
|
||||
- packages/domain/subscriptions/providers/whmcs/mapper.ts:64-88 (delete duplicates)
|
||||
+ packages/domain/subscriptions/providers/whmcs/mapper.ts:1
|
||||
import { parseAmount, formatDate } from "../../providers/whmcs/utils"
|
||||
|
||||
# Step 4: Remove obsolete package
|
||||
- packages/integrations/whmcs/src/utils/data-utils.ts (delete, consolidate into domain)
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### C3. Service Wrapper Anti-Pattern in Frontend
|
||||
|
||||
**Location:**
|
||||
- `apps/portal/src/features/checkout/services/checkout.service.ts`
|
||||
- `apps/portal/src/features/orders/services/orders.service.ts`
|
||||
- `apps/portal/src/features/account/services/account.service.ts`
|
||||
- `apps/portal/src/features/catalog/services/catalog.service.ts`
|
||||
- `apps/portal/src/features/subscriptions/services/sim-actions.service.ts`
|
||||
|
||||
**Problem:**
|
||||
Frontend has service wrapper classes that simply proxy to API endpoints. This is an anti-pattern in Next.js where hooks should call server actions directly. **This violates Memory ID: 9499068**.
|
||||
|
||||
**Evidence:**
|
||||
```typescript
|
||||
// apps/portal/src/features/checkout/services/checkout.service.ts
|
||||
export const checkoutService = {
|
||||
async buildCart(orderType, selections, configuration) {
|
||||
const response = await apiClient.POST("/api/checkout/cart", {
|
||||
body: { orderType, selections, configuration },
|
||||
});
|
||||
return getDataOrThrow(response, "Failed to build checkout cart");
|
||||
},
|
||||
};
|
||||
|
||||
// apps/portal/src/features/account/services/account.service.ts
|
||||
export const accountService = {
|
||||
async getProfile() {
|
||||
const response = await apiClient.GET("/api/me");
|
||||
return getNullableData(response);
|
||||
},
|
||||
async updateProfile(update) {
|
||||
const response = await apiClient.PATCH("/api/me", { body: update });
|
||||
return getDataOrThrow(response, "Failed to update profile");
|
||||
},
|
||||
};
|
||||
```
|
||||
|
||||
**Impact:**
|
||||
- 🔴 **Architecture:** Violates Next.js best practices (user preference)
|
||||
- 🟡 **Maintainability:** Extra layer to maintain without added value
|
||||
- 🟡 **Type Safety:** Reduces type inference from server actions
|
||||
|
||||
**Correct Pattern:**
|
||||
Hooks should use apiClient directly, not through service wrappers.
|
||||
|
||||
**Fix:**
|
||||
Remove service wrappers and call apiClient directly from hooks.
|
||||
|
||||
**Diff-Ready Plan:**
|
||||
```diff
|
||||
# Step 1: Update checkout hook
|
||||
- apps/portal/src/features/checkout/services/checkout.service.ts (delete entire file)
|
||||
- apps/portal/src/features/checkout/hooks/useCheckout.ts:118
|
||||
- const cart = await checkoutService.buildCart(snapshotOrderType, selections, configuration);
|
||||
+ const response = await apiClient.POST("/api/checkout/cart", {
|
||||
+ body: { orderType: snapshotOrderType, selections, configuration }
|
||||
+ });
|
||||
+ const cart = getDataOrThrow(response, "Failed to build checkout cart");
|
||||
|
||||
# Step 2: Update account hooks
|
||||
- apps/portal/src/features/account/services/account.service.ts (delete entire file)
|
||||
- apps/portal/src/features/account/hooks/useProfileData.ts
|
||||
Replace accountService.getProfile() with direct apiClient.GET("/api/me")
|
||||
- apps/portal/src/features/account/hooks/useProfileEdit.ts:16
|
||||
Replace accountService.updateProfile() with direct apiClient.PATCH()
|
||||
|
||||
# Step 3: Update orders hooks
|
||||
- apps/portal/src/features/orders/services/orders.service.ts (delete entire file)
|
||||
Move functions directly into hooks
|
||||
|
||||
# Step 4: Update catalog hooks
|
||||
- apps/portal/src/features/catalog/services/catalog.service.ts (delete entire file)
|
||||
Move API calls directly into hooks
|
||||
|
||||
# Step 5: Update SIM action hooks
|
||||
- apps/portal/src/features/subscriptions/services/sim-actions.service.ts (delete entire file)
|
||||
Move API calls directly into hooks
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### C4. Hardcoded Business Pricing Logic in BFF
|
||||
|
||||
**Location:**
|
||||
- `apps/bff/src/modules/subscriptions/sim-management/services/sim-topup.service.ts` (Lines 38-41)
|
||||
|
||||
**Problem:**
|
||||
Hardcoded pricing formula "1GB = 500 JPY" exists in BFF service code. Pricing should be configured in Salesforce or a pricing service, not hardcoded.
|
||||
|
||||
**Evidence:**
|
||||
```typescript
|
||||
// apps/bff/src/modules/subscriptions/sim-management/services/sim-topup.service.ts:38-41
|
||||
// Calculate cost: 1GB = 500 JPY (rounded up to nearest GB)
|
||||
const quotaGb = request.quotaMb / 1000;
|
||||
const units = Math.ceil(quotaGb);
|
||||
const costJpy = units * 500;
|
||||
```
|
||||
|
||||
**Impact:**
|
||||
- 🔴 **Maintainability:** Pricing changes require code deployment
|
||||
- 🔴 **Business Agility:** Cannot change pricing without developer intervention
|
||||
- 🟡 **Consistency:** Pricing rules should be centralized
|
||||
|
||||
**Fix:**
|
||||
Create a pricing configuration service or use Salesforce Product2 pricing.
|
||||
|
||||
**Diff-Ready Plan:**
|
||||
```diff
|
||||
# Step 1: Create pricing configuration
|
||||
+ apps/bff/src/modules/catalog/services/pricing-config.service.ts
|
||||
export class PricingConfigService {
|
||||
async getSIMTopUpPricing(region: string): Promise<{ pricePerGB: number; currency: string }>
|
||||
}
|
||||
|
||||
# Step 2: Update SIM top-up service
|
||||
- apps/bff/src/modules/subscriptions/sim-management/services/sim-topup.service.ts:38-41
|
||||
+ const pricing = await this.pricingConfig.getSIMTopUpPricing("JP");
|
||||
+ const quotaGb = request.quotaMb / 1000;
|
||||
+ const units = Math.ceil(quotaGb);
|
||||
+ const costJpy = units * pricing.pricePerGB;
|
||||
|
||||
# Step 3: Store pricing in Salesforce Product2
|
||||
Add SIM_TopUp_Price__c custom field to Product2
|
||||
Query pricing from Salesforce instead of hardcoding
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### C5. Duplicate Currency Formatting Implementations
|
||||
|
||||
**Location:**
|
||||
- `packages/domain/toolkit/formatting/currency.ts` (Lines 75-91)
|
||||
- `apps/portal/src/features/dashboard/utils/dashboard.utils.ts` (Lines 127-151)
|
||||
- `apps/portal/src/lib/hooks/useFormatCurrency.ts` (Lines 6-30)
|
||||
|
||||
**Problem:**
|
||||
Three different currency formatting implementations exist across the codebase with inconsistent behaviors.
|
||||
|
||||
**Evidence:**
|
||||
```typescript
|
||||
// packages/domain/toolkit/formatting/currency.ts:75-91 (Canonical)
|
||||
export function formatCurrency(
|
||||
amount: number,
|
||||
currencyOrOptions?: string | LegacyOptions,
|
||||
symbolOrOptions?: string | LegacyOptions
|
||||
): string {
|
||||
const { locale, symbol, showSymbol, fractionDigits } = normalizeOptions(
|
||||
currencyOrOptions,
|
||||
symbolOrOptions
|
||||
);
|
||||
const formatted = amount.toLocaleString(locale, {
|
||||
minimumFractionDigits: fractionDigits,
|
||||
maximumFractionDigits: fractionDigits,
|
||||
});
|
||||
return showSymbol ? `${symbol}${formatted}` : formatted;
|
||||
}
|
||||
|
||||
// apps/portal/src/features/dashboard/utils/dashboard.utils.ts:129-151 (Duplicate)
|
||||
const formatCurrency = (amount: number, currency?: string) => {
|
||||
const code = (currency || "JPY").toUpperCase();
|
||||
const formatter = currencyFormatterCache.get(code) || (() => {
|
||||
try {
|
||||
const intl = new Intl.NumberFormat("en-US", {
|
||||
style: "currency",
|
||||
currency: code,
|
||||
});
|
||||
currencyFormatterCache.set(code, intl);
|
||||
return intl;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
})();
|
||||
if (!formatter) {
|
||||
return `${code} ${amount.toLocaleString()}`;
|
||||
}
|
||||
return formatter.format(amount);
|
||||
};
|
||||
|
||||
// apps/portal/src/lib/hooks/useFormatCurrency.ts:9-22 (Hook wrapper)
|
||||
const formatCurrency = (amount: number) => {
|
||||
if (loading) {
|
||||
return "¥" + amount.toLocaleString();
|
||||
}
|
||||
if (error) {
|
||||
return baseFormatCurrency(amount, "JPY", "¥");
|
||||
}
|
||||
return baseFormatCurrency(amount, currencyCode, currencySymbol);
|
||||
};
|
||||
```
|
||||
|
||||
**Impact:**
|
||||
- 🔴 **Inconsistency:** Different parts of the app format currency differently
|
||||
- 🟡 **Maintainability:** Changes need to be made in multiple places
|
||||
- 🟡 **Performance:** Dashboard creates cache unnecessarily
|
||||
|
||||
**Fix:**
|
||||
Consolidate to single implementation in domain package, with optional React hook wrapper.
|
||||
|
||||
**Diff-Ready Plan:**
|
||||
```diff
|
||||
# Step 1: Enhance domain currency formatter
|
||||
+ packages/domain/toolkit/formatting/currency.ts
|
||||
Add caching like dashboard implementation
|
||||
Ensure consistent behavior across all cases
|
||||
|
||||
# Step 2: Remove duplicate implementation
|
||||
- apps/portal/src/features/dashboard/utils/dashboard.utils.ts:127-151 (delete formatCurrency)
|
||||
+ apps/portal/src/features/dashboard/utils/dashboard.utils.ts:1
|
||||
import { formatCurrency } from "@customer-portal/domain/toolkit"
|
||||
|
||||
# Step 3: Keep hook wrapper but simplify
|
||||
apps/portal/src/lib/hooks/useFormatCurrency.ts
|
||||
Keep as thin wrapper around domain function, only adding loading/error states
|
||||
|
||||
# Step 4: Global search and replace
|
||||
Find all instances of formatCurrency usage and ensure consistent imports
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### C6. Billing Cycle Normalization Logic in Frontend
|
||||
|
||||
**Location:**
|
||||
- `apps/portal/src/features/orders/utils/order-presenters.ts` (Lines 146-152)
|
||||
- `apps/portal/src/features/subscriptions/components/SubscriptionCard.tsx` (Lines 70-74)
|
||||
|
||||
**Problem:**
|
||||
Frontend components normalize billing cycle strings, which is business logic.
|
||||
|
||||
**Evidence:**
|
||||
```typescript
|
||||
// apps/portal/src/features/orders/utils/order-presenters.ts:146-152
|
||||
export function normalizeBillingCycle(value?: string): "monthly" | "onetime" | null {
|
||||
if (!value) return null;
|
||||
const normalized = value.trim().toLowerCase();
|
||||
if (normalized === "monthly") return "monthly";
|
||||
if (normalized === "onetime" || normalized === "one-time") return "onetime";
|
||||
return null;
|
||||
}
|
||||
|
||||
// apps/portal/src/features/subscriptions/components/SubscriptionCard.tsx:70-74
|
||||
const getBillingCycleLabel = (cycle: string, name: string) => {
|
||||
const looksLikeActivation = name.toLowerCase().includes("activation") || name.includes("setup");
|
||||
return looksLikeActivation ? "One-time" : cycle;
|
||||
};
|
||||
```
|
||||
|
||||
**Impact:**
|
||||
- 🔴 **Inconsistency:** BFF already normalizes cycles in mapper
|
||||
- 🟡 **Maintainability:** Business rules spread across layers
|
||||
|
||||
**Fix:**
|
||||
Ensure BFF always sends normalized values; frontend only displays them.
|
||||
|
||||
**Diff-Ready Plan:**
|
||||
```diff
|
||||
# Step 1: Verify BFF normalization
|
||||
packages/domain/subscriptions/providers/whmcs/mapper.ts:58-62
|
||||
Ensure mapCycle returns consistent format
|
||||
|
||||
# Step 2: Remove frontend normalization
|
||||
- apps/portal/src/features/orders/utils/order-presenters.ts:146-152 (delete)
|
||||
- apps/portal/src/features/subscriptions/components/SubscriptionCard.tsx:70-74 (delete)
|
||||
Use cycle value directly from API
|
||||
|
||||
# Step 3: Add type safety
|
||||
+ packages/domain/subscriptions/schema.ts
|
||||
Ensure SubscriptionCycle is exported as const array for validation
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### C7. Missing Type Validation at HTTP Boundaries
|
||||
|
||||
**Location:**
|
||||
- `apps/portal/src/features/catalog/services/catalog.service.ts` (Lines 24-80)
|
||||
|
||||
**Problem:**
|
||||
Frontend service applies Zod validation to API responses, but BFF should already be validating and providing typed responses. This is redundant validation.
|
||||
|
||||
**Evidence:**
|
||||
```typescript
|
||||
// apps/portal/src/features/catalog/services/catalog.service.ts:24-30
|
||||
async getInternetCatalog(): Promise<InternetCatalogCollection> {
|
||||
const response = await apiClient.GET<InternetCatalogCollection>("/api/catalog/internet/plans");
|
||||
const data = getDataOrThrow<InternetCatalogCollection>(
|
||||
response,
|
||||
"Failed to load internet catalog"
|
||||
);
|
||||
return parseInternetCatalog(data); // Re-validates with Zod
|
||||
}
|
||||
```
|
||||
|
||||
**Impact:**
|
||||
- 🟡 **Performance:** Double validation overhead
|
||||
- 🟡 **Architecture:** BFF should be trusted source of validated data
|
||||
|
||||
**Fix:**
|
||||
Trust BFF responses (already validated), only validate at BFF HTTP entry points.
|
||||
|
||||
**Diff-Ready Plan:**
|
||||
```diff
|
||||
# Frontend should trust BFF
|
||||
- apps/portal/src/features/catalog/services/catalog.service.ts:30
|
||||
- return parseInternetCatalog(data);
|
||||
+ return data; // Already validated by BFF
|
||||
|
||||
# BFF validates at entry
|
||||
apps/bff/src/modules/catalog/catalog.controller.ts
|
||||
Ensure all responses use domain schemas for validation
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 🟡 Medium Priority Issues (Priority 2)
|
||||
|
||||
### M1. Inconsistent Error Handling Patterns
|
||||
|
||||
**Location:**
|
||||
- Various `apiClient` call sites throughout portal
|
||||
|
||||
**Problem:**
|
||||
Some use `getDataOrThrow`, others use `getNullableData`, others check `response.data` manually.
|
||||
|
||||
**Impact:**
|
||||
- 🟡 **Maintainability:** Hard to ensure consistent error handling
|
||||
- 🟡 **User Experience:** Inconsistent error messages
|
||||
|
||||
**Fix:**
|
||||
Standardize on `getDataOrThrow` for required data, `getNullableData` for optional.
|
||||
|
||||
---
|
||||
|
||||
### M2. Type Alias Duplication
|
||||
|
||||
**Location:**
|
||||
- `apps/portal/src/features/catalog/utils/catalog.utils.ts` (Lines 14-19)
|
||||
- `packages/domain/catalog/schema.ts` (Line 162-169)
|
||||
|
||||
**Problem:**
|
||||
Frontend defines its own `CatalogProduct` type alias when domain package already exports it.
|
||||
|
||||
**Evidence:**
|
||||
```typescript
|
||||
// apps/portal/src/features/catalog/utils/catalog.utils.ts:14-19
|
||||
type CatalogProduct =
|
||||
| InternetPlanCatalogItem
|
||||
| InternetAddonCatalogItem
|
||||
| InternetInstallationCatalogItem
|
||||
| SimCatalogProduct
|
||||
| VpnCatalogProduct;
|
||||
|
||||
// packages/domain/catalog/schema.ts:162-169
|
||||
export type CatalogProduct =
|
||||
| InternetPlanCatalogItem
|
||||
| InternetInstallationCatalogItem
|
||||
| InternetAddonCatalogItem
|
||||
| SimCatalogProduct
|
||||
| SimActivationFeeCatalogItem
|
||||
| VpnCatalogProduct
|
||||
| CatalogProductBase;
|
||||
```
|
||||
|
||||
**Impact:**
|
||||
- 🟡 **Consistency:** Types can drift
|
||||
- 🟡 **Maintainability:** Two sources of truth
|
||||
|
||||
**Fix:**
|
||||
Remove local alias, import from domain.
|
||||
|
||||
**Diff-Ready Plan:**
|
||||
```diff
|
||||
- apps/portal/src/features/catalog/utils/catalog.utils.ts:14-19 (delete)
|
||||
+ apps/portal/src/features/catalog/utils/catalog.utils.ts:6
|
||||
import type { CatalogProduct } from "@customer-portal/domain/catalog"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### M3. Unused Integration Package Layer
|
||||
|
||||
**Location:**
|
||||
- `packages/integrations/whmcs/src/mappers/*`
|
||||
|
||||
**Problem:**
|
||||
Integration mappers are just re-exports from domain. This package adds no value.
|
||||
|
||||
**Evidence:**
|
||||
```typescript
|
||||
// packages/integrations/whmcs/src/mappers/subscription.mapper.ts
|
||||
export * from "@customer-portal/domain/subscriptions/providers/whmcs";
|
||||
|
||||
// packages/integrations/whmcs/src/mappers/invoice.mapper.ts
|
||||
export * from "@customer-portal/domain/billing/providers/whmcs";
|
||||
```
|
||||
|
||||
**Impact:**
|
||||
- 🟡 **Complexity:** Extra package to maintain
|
||||
- 🟡 **Import Paths:** Confusing import structure
|
||||
|
||||
**Fix:**
|
||||
Remove `packages/integrations/whmcs` package, import directly from domain.
|
||||
|
||||
**Diff-Ready Plan:**
|
||||
```diff
|
||||
# Step 1: Update all imports
|
||||
Find: from "@customer-portal/integrations-whmcs"
|
||||
Replace: from "@customer-portal/domain/[subscriptions|billing]/providers/whmcs"
|
||||
|
||||
# Step 2: Remove package
|
||||
- packages/integrations/whmcs/ (delete entire directory)
|
||||
- packages/integrations/freebit/ (keep if has actual logic, else delete)
|
||||
|
||||
# Step 3: Update workspace config
|
||||
- pnpm-workspace.yaml
|
||||
Remove integrations packages if all deleted
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### M4. Status Mapping Duplication
|
||||
|
||||
**Location:**
|
||||
- `packages/domain/billing/providers/whmcs/mapper.ts` (Lines 24-35)
|
||||
- `packages/domain/subscriptions/providers/whmcs/mapper.ts` (Lines 21-31)
|
||||
|
||||
**Problem:**
|
||||
`STATUS_MAP` dictionaries are defined separately in multiple mappers.
|
||||
|
||||
**Impact:**
|
||||
- 🟡 **Consistency:** Different status mappings could cause confusion
|
||||
- 🟡 **Maintainability:** Updates must be synchronized
|
||||
|
||||
**Fix:**
|
||||
Move to shared WHMCS provider utilities (part of C2 fix).
|
||||
|
||||
---
|
||||
|
||||
### M5. Magic String Status Checks
|
||||
|
||||
**Location:**
|
||||
- `apps/bff/src/modules/subscriptions/subscriptions.service.ts` (Lines 487-490)
|
||||
|
||||
**Problem:**
|
||||
Hardcoded magic strings for status and group name checks.
|
||||
|
||||
**Evidence:**
|
||||
```typescript
|
||||
// apps/bff/src/modules/subscriptions/subscriptions.service.ts:487-490
|
||||
return services.some((service: WhmcsProduct) => {
|
||||
const group = typeof service.groupname === "string" ? service.groupname.toLowerCase() : "";
|
||||
const status = typeof service.status === "string" ? service.status.toLowerCase() : "";
|
||||
return group.includes("sim") && status === "active";
|
||||
});
|
||||
```
|
||||
|
||||
**Impact:**
|
||||
- 🟡 **Maintainability:** Status values spread throughout code
|
||||
- 🟡 **Type Safety:** Should use status enum
|
||||
|
||||
**Fix:**
|
||||
Use domain status constants and enums.
|
||||
|
||||
**Diff-Ready Plan:**
|
||||
```diff
|
||||
+ packages/domain/subscriptions/contract.ts
|
||||
export const SUBSCRIPTION_STATUS = { ... } as const
|
||||
export const PRODUCT_GROUPS = { SIM: "sim", INTERNET: "internet", ... } as const
|
||||
|
||||
- apps/bff/src/modules/subscriptions/subscriptions.service.ts:487-490
|
||||
+ return services.some((service: WhmcsProduct) => {
|
||||
+ const group = service.groupname?.toLowerCase() || "";
|
||||
+ const status = mapStatus(service.status);
|
||||
+ return group.includes(PRODUCT_GROUPS.SIM) && status === "Active";
|
||||
+ });
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 🟢 Low Priority Improvements (Priority 3)
|
||||
|
||||
### L1. Optimize Internet Tier Metadata
|
||||
|
||||
**Location:**
|
||||
- `packages/domain/catalog/utils.ts` (Lines 51-105)
|
||||
|
||||
**Problem:**
|
||||
Hardcoded tier metadata in code. Could be moved to CMS or Salesforce.
|
||||
|
||||
**Impact:**
|
||||
- 🟢 **Flexibility:** Content updates require code changes
|
||||
- 🟢 **Localization:** Hard to translate
|
||||
|
||||
**Fix:**
|
||||
Consider moving to Salesforce Product2 metadata or headless CMS.
|
||||
|
||||
---
|
||||
|
||||
### L2. Unused `catalogMetadata` Fields
|
||||
|
||||
**Location:**
|
||||
- Various catalog schemas
|
||||
|
||||
**Problem:**
|
||||
`catalogMetadata` object added to types but inconsistently used.
|
||||
|
||||
**Impact:**
|
||||
- 🟢 **Type Safety:** Optional fields lead to defensive coding
|
||||
|
||||
**Fix:**
|
||||
Audit and document required vs optional metadata fields.
|
||||
|
||||
---
|
||||
|
||||
### L3. Schema Validation Performance
|
||||
|
||||
**Location:**
|
||||
- All Zod parse() calls in services
|
||||
|
||||
**Problem:**
|
||||
Zod schemas re-parsed on every request. Could cache parsed schemas.
|
||||
|
||||
**Impact:**
|
||||
- 🟢 **Performance:** Minor overhead on high-traffic endpoints
|
||||
|
||||
**Fix:**
|
||||
Consider using `safeParse()` with caching for frequently accessed data.
|
||||
|
||||
---
|
||||
|
||||
## Type Flow Analysis
|
||||
|
||||
### 1. Catalog Products (Salesforce → BFF → Portal)
|
||||
|
||||
```
|
||||
┌─────────────────────────────────────────────────────────────────┐
|
||||
│ Salesforce Product2 │
|
||||
│ (Source of Truth) │
|
||||
└────────────┬────────────────────────────────────────────────────┘
|
||||
│
|
||||
│ SOQL Query
|
||||
▼
|
||||
┌─────────────────────────────────────────────────────────────────┐
|
||||
│ BFF: apps/bff/src/modules/catalog/ │
|
||||
│ │
|
||||
│ Raw: SalesforceProduct2WithPricebookEntries │
|
||||
│ ↓ packages/domain/catalog/providers/salesforce/raw.types.ts │
|
||||
│ ↓ │
|
||||
│ Mapper: packages/domain/catalog/providers/salesforce/mapper.ts │
|
||||
│ ↓ - mapInternetPlan() │
|
||||
│ ↓ - mapSimProduct() │
|
||||
│ ↓ - enrichInternetPlanMetadata() │
|
||||
│ ↓ │
|
||||
│ Domain: InternetPlanCatalogItem | SimCatalogProduct │
|
||||
│ ↓ packages/domain/catalog/schema.ts │
|
||||
│ │
|
||||
│ Controller: catalog.controller.ts │
|
||||
│ ↓ Returns: InternetCatalogCollection │
|
||||
└────────────┬────────────────────────────────────────────────────┘
|
||||
│
|
||||
│ HTTP JSON Response
|
||||
▼
|
||||
┌─────────────────────────────────────────────────────────────────┐
|
||||
│ Portal: apps/portal/src/features/catalog/ │
|
||||
│ │
|
||||
│ Service: catalog.service.ts │
|
||||
│ ↓ parseInternetCatalog(data) ← ⚠️ REDUNDANT VALIDATION │
|
||||
│ ↓ │
|
||||
│ Hook: useCatalog() │
|
||||
│ ↓ Returns: InternetCatalogCollection │
|
||||
│ ↓ │
|
||||
│ Component: InternetPlansView.tsx │
|
||||
│ ↓ Displays plans using domain types │
|
||||
└─────────────────────────────────────────────────────────────────┘
|
||||
|
||||
✅ GOOD: Single source of truth (domain types)
|
||||
✅ GOOD: Mapper layer cleanly transforms API types
|
||||
⚠️ ISSUE: Frontend re-validates already validated data (M7)
|
||||
⚠️ ISSUE: Frontend calculates prices instead of using API totals (C1)
|
||||
```
|
||||
|
||||
### 2. Subscriptions (WHMCS → BFF → Portal)
|
||||
|
||||
```
|
||||
┌─────────────────────────────────────────────────────────────────┐
|
||||
│ WHMCS GetClientsProducts API │
|
||||
│ (External System) │
|
||||
└────────────┬────────────────────────────────────────────────────┘
|
||||
│
|
||||
│ HTTP XML/JSON Response
|
||||
▼
|
||||
┌─────────────────────────────────────────────────────────────────┐
|
||||
│ BFF: apps/bff/src/integrations/whmcs/ │
|
||||
│ │
|
||||
│ Raw: WhmcsProductRaw │
|
||||
│ ↓ packages/domain/subscriptions/providers/whmcs/raw.types.ts│
|
||||
│ ↓ whmcsProductRawSchema (Zod validation) │
|
||||
│ ↓ │
|
||||
│ Mapper: packages/domain/subscriptions/providers/whmcs/mapper.ts │
|
||||
│ ↓ transformWhmcsSubscription() │
|
||||
│ ↓ - mapStatus() │
|
||||
│ ↓ - mapCycle() │
|
||||
│ ↓ - parseAmount() ← ⚠️ DUPLICATED (C2) │
|
||||
│ ↓ - formatDate() ← ⚠️ DUPLICATED (C2) │
|
||||
│ ↓ │
|
||||
│ Domain: Subscription │
|
||||
│ ↓ packages/domain/subscriptions/schema.ts │
|
||||
│ ↓ subscriptionSchema (Zod validation) │
|
||||
│ │
|
||||
│ Service: WhmcsService.getSubscriptions() │
|
||||
│ ↓ apps/bff/src/integrations/whmcs/whmcs.service.ts │
|
||||
│ ↓ Returns: SubscriptionList │
|
||||
│ │
|
||||
│ Controller: subscriptions.controller.ts │
|
||||
│ ↓ apps/bff/src/modules/subscriptions/ │
|
||||
│ ↓ Returns: SubscriptionList │
|
||||
└────────────┬────────────────────────────────────────────────────┘
|
||||
│
|
||||
│ HTTP JSON Response
|
||||
▼
|
||||
┌─────────────────────────────────────────────────────────────────┐
|
||||
│ Portal: apps/portal/src/features/subscriptions/ │
|
||||
│ │
|
||||
│ Hook: useSubscriptions() │
|
||||
│ ↓ Direct apiClient.GET("/api/subscriptions") ✅ │
|
||||
│ ↓ Returns: SubscriptionList │
|
||||
│ ↓ │
|
||||
│ Component: SubscriptionTable.tsx │
|
||||
│ ↓ - getBillingCycleLabel() ← ⚠️ BUSINESS LOGIC (C6) │
|
||||
│ ↓ - formatCurrency() ← ⚠️ DUPLICATE IMPL (C5) │
|
||||
│ ↓ Displays subscriptions │
|
||||
└─────────────────────────────────────────────────────────────────┘
|
||||
|
||||
✅ GOOD: Clean mapper layer
|
||||
✅ GOOD: Domain types used throughout
|
||||
⚠️ ISSUE: Duplicate parseAmount/formatDate (C2)
|
||||
⚠️ ISSUE: Frontend normalizes billing cycles (C6)
|
||||
⚠️ ISSUE: Multiple currency formatters (C5)
|
||||
```
|
||||
|
||||
### 3. Orders (Portal → BFF → Salesforce)
|
||||
|
||||
```
|
||||
┌─────────────────────────────────────────────────────────────────┐
|
||||
│ Portal: Checkout Flow │
|
||||
│ │
|
||||
│ Component: CheckoutView.tsx │
|
||||
│ ↓ Collects order data │
|
||||
│ ↓ │
|
||||
│ Hook: useCheckout() │
|
||||
│ ↓ checkoutService.buildCart() ← ⚠️ SERVICE WRAPPER (C3) │
|
||||
│ ↓ │
|
||||
│ Service: checkout.service.ts ← ⚠️ ANTI-PATTERN (C3) │
|
||||
│ ↓ apiClient.POST("/api/checkout/cart") │
|
||||
│ ↓ calculateOrderTotals() ← ⚠️ BUSINESS LOGIC (C1) │
|
||||
│ ↓ │
|
||||
│ Request: CreateOrderRequest │
|
||||
│ ↓ packages/domain/orders/schema.ts │
|
||||
└────────────┬────────────────────────────────────────────────────┘
|
||||
│
|
||||
│ HTTP JSON Request
|
||||
▼
|
||||
┌─────────────────────────────────────────────────────────────────┐
|
||||
│ BFF: apps/bff/src/modules/orders/ │
|
||||
│ │
|
||||
│ Controller: checkout.controller.ts │
|
||||
│ ↓ Receives: CreateOrderRequest │
|
||||
│ ↓ │
|
||||
│ Service: checkout.service.ts │
|
||||
│ ↓ buildCart(orderType, selections, config) │
|
||||
│ ↓ calculateTotals(items) ✅ CORRECT LOCATION │
|
||||
│ ↓ Returns: CheckoutCart │
|
||||
│ │
|
||||
│ Orchestrator: order-orchestrator.service.ts │
|
||||
│ ↓ createOrder(userId, rawBody) │
|
||||
│ ↓ │
|
||||
│ Validator: order-validator.service.ts │
|
||||
│ ↓ validateCompleteOrder() │
|
||||
│ ↓ │
|
||||
│ Builder: order-builder.service.ts │
|
||||
│ ↓ buildOrderFields() → Salesforce format │
|
||||
│ ↓ │
|
||||
│ Integration: salesforce-order.service.ts │
|
||||
│ ↓ createOrder(orderFields) │
|
||||
└────────────┬────────────────────────────────────────────────────┘
|
||||
│
|
||||
│ Salesforce REST API
|
||||
▼
|
||||
┌─────────────────────────────────────────────────────────────────┐
|
||||
│ Salesforce Order & OrderItem │
|
||||
│ (External System) │
|
||||
└─────────────────────────────────────────────────────────────────┘
|
||||
|
||||
✅ GOOD: Clean orchestration with separate concerns
|
||||
✅ GOOD: BFF calculates totals correctly
|
||||
⚠️ ISSUE: Frontend service wrapper (C3)
|
||||
⚠️ ISSUE: Frontend recalculates totals (C1)
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Recommendations Summary
|
||||
|
||||
### Immediate Actions (This Sprint)
|
||||
1. **Remove pricing calculations from frontend** (C1) - Critical data integrity risk
|
||||
2. **Consolidate WHMCS mapper utilities** (C2) - Prevents divergent implementations
|
||||
3. **Remove service wrapper anti-pattern** (C3) - Aligns with architecture standards
|
||||
|
||||
### Short-Term (Next Sprint)
|
||||
4. **Extract hardcoded pricing** (C4) - Business agility
|
||||
5. **Standardize currency formatting** (C5) - Consistency
|
||||
6. **Remove billing cycle normalization from frontend** (C6) - Clean separation
|
||||
|
||||
### Medium-Term (Next Quarter)
|
||||
7. **Remove redundant validation** (C7) - Performance
|
||||
8. **Consolidate error handling** (M1) - Consistency
|
||||
9. **Remove duplicate type aliases** (M2) - Maintainability
|
||||
10. **Remove unused integration packages** (M3) - Simplification
|
||||
|
||||
### Long-Term (Future)
|
||||
11. **Move tier metadata to CMS** (L1) - Content flexibility
|
||||
12. **Audit catalogMetadata usage** (L2) - Type safety
|
||||
13. **Optimize schema validation** (L3) - Performance at scale
|
||||
|
||||
---
|
||||
|
||||
## Testing Strategy
|
||||
|
||||
### For Each Fix:
|
||||
1. **Unit Tests:** Test mapper functions with edge cases
|
||||
2. **Integration Tests:** Verify API contract compliance
|
||||
3. **E2E Tests:** Ensure UI displays correct data
|
||||
4. **Performance Tests:** Measure validation overhead before/after
|
||||
|
||||
### Regression Prevention:
|
||||
- Add ESLint rules to prevent service wrappers in portal
|
||||
- Add pre-commit hook to check for duplicate functions
|
||||
- Create architecture decision records (ADRs) for patterns
|
||||
- Document preferred patterns in CONTRIBUTING.md
|
||||
|
||||
---
|
||||
|
||||
## Metrics & Success Criteria
|
||||
|
||||
### Code Quality Metrics:
|
||||
- **Duplicate Code:** Reduce by ~500 LOC (estimated)
|
||||
- **Frontend Services:** Remove 7 wrapper files
|
||||
- **Type Safety:** 100% BFF responses use domain schemas
|
||||
|
||||
### Business Metrics:
|
||||
- **Pricing Accuracy:** Zero discrepancies between frontend display and BFF calculations
|
||||
- **Development Velocity:** Reduce time to add new product types by 30%
|
||||
- **Bug Rate:** Reduce type-related bugs by 50%
|
||||
|
||||
---
|
||||
|
||||
## Appendix: Clean Architecture Checklist
|
||||
|
||||
### ✅ What's Good:
|
||||
- Centralized domain types in `packages/domain/*`
|
||||
- Mapper pattern for external API integration
|
||||
- Zod schemas for runtime validation
|
||||
- Clear separation between BFF and Portal
|
||||
|
||||
### ⚠️ What Needs Improvement:
|
||||
- Business logic leaking to frontend (pricing, normalization)
|
||||
- Service wrapper anti-pattern in Portal
|
||||
- Duplicate implementations across packages
|
||||
- Inconsistent validation strategy
|
||||
|
||||
### 🎯 Target Architecture:
|
||||
```
|
||||
┌─────────────────────────────────────────────────────────────────┐
|
||||
│ External APIs (Salesforce, WHMCS, Freebit) │
|
||||
└────────────┬────────────────────────────────────────────────────┘
|
||||
│
|
||||
▼
|
||||
┌─────────────────────────────────────────────────────────────────┐
|
||||
│ Domain Layer (packages/domain/*) │
|
||||
│ - Raw types (API contracts) │
|
||||
│ - Mappers (API → Domain) │
|
||||
│ - Schemas (Zod validation) │
|
||||
│ - Business types (canonical) │
|
||||
└────────────┬────────────────────────────────────────────────────┘
|
||||
│
|
||||
▼
|
||||
┌─────────────────────────────────────────────────────────────────┐
|
||||
│ BFF Layer (apps/bff/*) │
|
||||
│ - Controllers (HTTP boundary validation) │
|
||||
│ - Services (business logic, orchestration) │
|
||||
│ - Integration services (call external APIs) │
|
||||
│ - Returns validated domain types │
|
||||
└────────────┬────────────────────────────────────────────────────┘
|
||||
│
|
||||
▼
|
||||
┌─────────────────────────────────────────────────────────────────┐
|
||||
│ Portal Layer (apps/portal/*) │
|
||||
│ - Hooks (fetch data via apiClient) │
|
||||
│ - Components (display data only) │
|
||||
│ - Utils (presentation logic only: formatting, display helpers) │
|
||||
│ - NO business logic (pricing, validation, normalization) │
|
||||
└─────────────────────────────────────────────────────────────────┘
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
**End of Audit Report**
|
||||
|
||||
@ -161,6 +161,14 @@ export class SecureErrorMapperService {
|
||||
logLevel: "warn",
|
||||
},
|
||||
],
|
||||
[
|
||||
"SESSION_EXPIRED",
|
||||
{
|
||||
code: "SESSION_EXPIRED",
|
||||
publicMessage: "Your session has expired. Please log in again",
|
||||
logLevel: "info",
|
||||
},
|
||||
],
|
||||
|
||||
// Authorization Errors
|
||||
[
|
||||
@ -289,6 +297,14 @@ export class SecureErrorMapperService {
|
||||
},
|
||||
|
||||
// Authentication patterns
|
||||
{
|
||||
pattern: /token expired or expiring soon/i,
|
||||
mapping: {
|
||||
code: "SESSION_EXPIRED",
|
||||
publicMessage: "Your session has expired. Please log in again",
|
||||
logLevel: "info",
|
||||
},
|
||||
},
|
||||
{
|
||||
pattern: /password|credential|token|secret|key|auth/i,
|
||||
mapping: {
|
||||
@ -370,6 +386,7 @@ export class SecureErrorMapperService {
|
||||
}
|
||||
|
||||
private determineCategory(code: string): ErrorClassification["category"] {
|
||||
if (code === "SESSION_EXPIRED") return "authentication";
|
||||
if (code.startsWith("AUTH_")) return "authentication";
|
||||
if (code.startsWith("AUTHZ_")) return "authorization";
|
||||
if (code.startsWith("VAL_")) return "validation";
|
||||
@ -383,6 +400,7 @@ export class SecureErrorMapperService {
|
||||
if (code === "SYS_001" || code === "SYS_003") return "critical";
|
||||
|
||||
// High severity for authentication issues
|
||||
if (code === "SESSION_EXPIRED") return "medium";
|
||||
if (code.startsWith("AUTH_") && message.toLowerCase().includes("breach")) return "high";
|
||||
|
||||
// Medium for external service issues
|
||||
|
||||
@ -123,13 +123,13 @@ export class OrderBuilder {
|
||||
const address2 = typeof addressToUse?.address2 === "string" ? addressToUse.address2 : "";
|
||||
const fullStreet = [address1, address2].filter(Boolean).join(", ");
|
||||
|
||||
orderFields.Billing_Street__c = fullStreet;
|
||||
orderFields.Billing_City__c = typeof addressToUse?.city === "string" ? addressToUse.city : "";
|
||||
orderFields.Billing_State__c =
|
||||
orderFields.BillingStreet = fullStreet;
|
||||
orderFields.BillingCity = typeof addressToUse?.city === "string" ? addressToUse.city : "";
|
||||
orderFields.BillingState =
|
||||
typeof addressToUse?.state === "string" ? addressToUse.state : "";
|
||||
orderFields.Billing_Postal_Code__c =
|
||||
orderFields.BillingPostalCode =
|
||||
typeof addressToUse?.postcode === "string" ? addressToUse.postcode : "";
|
||||
orderFields.Billing_Country__c =
|
||||
orderFields.BillingCountry =
|
||||
typeof addressToUse?.country === "string" ? addressToUse.country : "";
|
||||
orderFields.Address_Changed__c = addressChanged;
|
||||
|
||||
|
||||
@ -2,9 +2,9 @@
|
||||
import { logger } from "@customer-portal/logging";
|
||||
|
||||
import { useEffect, useRef, useState } from "react";
|
||||
import { useAuthStore } from "@/features/auth/services/auth.store";
|
||||
import { useAuthSession } from "@/features/auth/services/auth.store";
|
||||
import { Button } from "@/components/atoms/button";
|
||||
import { useAuth } from "@/features/auth/hooks/use-auth";
|
||||
|
||||
interface SessionTimeoutWarningProps {
|
||||
warningTime?: number; // Minutes before token expires to show warning
|
||||
@ -14,8 +14,7 @@ export function SessionTimeoutWarning({
|
||||
warningTime = 5, // Show warning 5 minutes before expiry (reduced since we have auto-refresh)
|
||||
}: SessionTimeoutWarningProps) {
|
||||
const { isAuthenticated, session } = useAuthSession();
|
||||
const logout = useAuthStore(state => state.logout);
|
||||
const refreshSession = useAuthStore(state => state.refreshSession);
|
||||
const { logout, refreshSession } = useAuth();
|
||||
const [showWarning, setShowWarning] = useState(false);
|
||||
const [timeLeft, setTimeLeft] = useState<number>(0);
|
||||
const expiryRef = useRef<number | null>(null);
|
||||
@ -42,7 +41,7 @@ export function SessionTimeoutWarning({
|
||||
expiryRef.current = expiryTime;
|
||||
|
||||
if (Date.now() >= expiryTime) {
|
||||
void logout();
|
||||
void logout({ reason: "session-expired" });
|
||||
return undefined;
|
||||
}
|
||||
|
||||
@ -83,7 +82,7 @@ export function SessionTimeoutWarning({
|
||||
const remaining = expiryTime - Date.now();
|
||||
if (remaining <= 0) {
|
||||
setTimeLeft(0);
|
||||
void logout();
|
||||
void logout({ reason: "session-expired" });
|
||||
return;
|
||||
}
|
||||
|
||||
@ -104,7 +103,7 @@ export function SessionTimeoutWarning({
|
||||
if (event.key === "Escape") {
|
||||
event.preventDefault();
|
||||
setShowWarning(false);
|
||||
void logout();
|
||||
void logout({ reason: "session-expired" });
|
||||
}
|
||||
|
||||
if (event.key === "Tab") {
|
||||
@ -147,13 +146,13 @@ export function SessionTimeoutWarning({
|
||||
setTimeLeft(0);
|
||||
} catch (error) {
|
||||
logger.error(error, "Failed to extend session");
|
||||
await logout();
|
||||
await logout({ reason: "session-expired" });
|
||||
}
|
||||
})();
|
||||
};
|
||||
|
||||
const handleLogoutNow = () => {
|
||||
void logout();
|
||||
void logout({ reason: "session-expired" });
|
||||
setShowWarning(false);
|
||||
};
|
||||
|
||||
|
||||
@ -10,6 +10,7 @@ import { useRouter, useSearchParams } from "next/navigation";
|
||||
import { useAuthStore } from "../services/auth.store";
|
||||
import { getPostLoginRedirect } from "@/features/auth/utils/route-protection";
|
||||
import type { SignupRequest, LoginRequest } from "@customer-portal/domain/auth";
|
||||
import type { LogoutReason } from "@/features/auth/utils/logout-reason";
|
||||
|
||||
/**
|
||||
* Main authentication hook
|
||||
@ -61,10 +62,15 @@ export function useAuth() {
|
||||
);
|
||||
|
||||
// Enhanced logout with redirect
|
||||
const logout = useCallback(async () => {
|
||||
await logoutAction();
|
||||
router.push("/auth/login");
|
||||
}, [logoutAction, router]);
|
||||
const logout = useCallback(
|
||||
async (options?: { reason?: LogoutReason }) => {
|
||||
const reason = options?.reason ?? ("manual" as const);
|
||||
await logoutAction({ reason });
|
||||
const reasonQuery = reason ? `?reason=${reason}` : "";
|
||||
router.push(`/auth/login${reasonQuery}`);
|
||||
},
|
||||
[logoutAction, router]
|
||||
);
|
||||
|
||||
return {
|
||||
// State
|
||||
@ -177,18 +183,53 @@ export function useSession() {
|
||||
|
||||
// Auto-refresh session periodically
|
||||
useEffect(() => {
|
||||
if (isAuthenticated) {
|
||||
const interval = setInterval(
|
||||
() => {
|
||||
void refreshSession();
|
||||
},
|
||||
5 * 60 * 1000
|
||||
); // Check every 5 minutes
|
||||
|
||||
return () => clearInterval(interval);
|
||||
if (!isAuthenticated) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
return undefined;
|
||||
const interval = setInterval(() => {
|
||||
void refreshSession();
|
||||
}, 5 * 60 * 1000); // Check every 5 minutes
|
||||
|
||||
return () => clearInterval(interval);
|
||||
}, [isAuthenticated, refreshSession]);
|
||||
|
||||
useEffect(() => {
|
||||
if (!isAuthenticated || typeof window === "undefined") {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
let lastRefresh = 0;
|
||||
const minInterval = 60 * 1000;
|
||||
|
||||
const triggerRefresh = () => {
|
||||
const now = Date.now();
|
||||
if (now - lastRefresh < minInterval) {
|
||||
return;
|
||||
}
|
||||
lastRefresh = now;
|
||||
void refreshSession();
|
||||
};
|
||||
|
||||
const handleVisibility = () => {
|
||||
if (document.visibilityState === "visible") {
|
||||
triggerRefresh();
|
||||
}
|
||||
};
|
||||
|
||||
const handleFocus = () => {
|
||||
triggerRefresh();
|
||||
};
|
||||
|
||||
window.addEventListener("focus", handleFocus);
|
||||
document.addEventListener("visibilitychange", handleVisibility);
|
||||
|
||||
triggerRefresh();
|
||||
|
||||
return () => {
|
||||
window.removeEventListener("focus", handleFocus);
|
||||
document.removeEventListener("visibilitychange", handleVisibility);
|
||||
};
|
||||
}, [isAuthenticated, refreshSession]);
|
||||
|
||||
return {
|
||||
|
||||
@ -6,7 +6,7 @@
|
||||
import { create } from "zustand";
|
||||
import { apiClient } from "@/lib/api";
|
||||
import { getNullableData } from "@/lib/api/response-helpers";
|
||||
import { getErrorInfo, handleAuthError } from "@/lib/utils/error-handling";
|
||||
import { getErrorInfo } from "@/lib/utils/error-handling";
|
||||
import logger from "@customer-portal/logging";
|
||||
import type {
|
||||
AuthTokens,
|
||||
@ -16,6 +16,12 @@ import type {
|
||||
} from "@customer-portal/domain/auth";
|
||||
import { authResponseSchema } from "@customer-portal/domain/auth";
|
||||
import type { AuthenticatedUser } from "@customer-portal/domain/customer";
|
||||
import {
|
||||
clearLogoutReason,
|
||||
logoutReasonFromErrorCode,
|
||||
setLogoutReason,
|
||||
type LogoutReason,
|
||||
} from "@/features/auth/utils/logout-reason";
|
||||
|
||||
interface SessionState {
|
||||
accessExpiresAt?: string;
|
||||
@ -32,7 +38,7 @@ export interface AuthState {
|
||||
|
||||
login: (credentials: LoginRequest) => Promise<void>;
|
||||
signup: (data: SignupRequest) => Promise<void>;
|
||||
logout: () => Promise<void>;
|
||||
logout: (options?: { reason?: LogoutReason }) => Promise<void>;
|
||||
requestPasswordReset: (email: string) => Promise<void>;
|
||||
resetPassword: (token: string, password: string) => Promise<void>;
|
||||
changePassword: (currentPassword: string, newPassword: string) => Promise<void>;
|
||||
@ -66,6 +72,39 @@ export const useAuthStore = create<AuthState>()((set, get) => {
|
||||
});
|
||||
};
|
||||
|
||||
let refreshPromise: Promise<void> | null = null;
|
||||
|
||||
const runTokenRefresh = async (): Promise<void> => {
|
||||
try {
|
||||
const response = await apiClient.POST("/api/auth/refresh", { body: {} });
|
||||
const parsed = authResponseSchema.safeParse(response.data);
|
||||
if (!parsed.success) {
|
||||
throw new Error(parsed.error.issues?.[0]?.message ?? "Session refresh failed");
|
||||
}
|
||||
applyAuthResponse(parsed.data);
|
||||
} catch (error) {
|
||||
logger.error(error, "Failed to refresh session");
|
||||
const errorInfo = getErrorInfo(error);
|
||||
const reason = logoutReasonFromErrorCode(errorInfo.code) ?? ("session-expired" as const);
|
||||
await get().logout({ reason });
|
||||
throw error;
|
||||
}
|
||||
};
|
||||
|
||||
const ensureSingleRefresh = async (): Promise<void> => {
|
||||
if (!refreshPromise) {
|
||||
refreshPromise = (async () => {
|
||||
try {
|
||||
await runTokenRefresh();
|
||||
} finally {
|
||||
refreshPromise = null;
|
||||
}
|
||||
})();
|
||||
}
|
||||
|
||||
await refreshPromise;
|
||||
};
|
||||
|
||||
return {
|
||||
user: null,
|
||||
session: {},
|
||||
@ -108,7 +147,12 @@ export const useAuthStore = create<AuthState>()((set, get) => {
|
||||
}
|
||||
},
|
||||
|
||||
logout: async () => {
|
||||
logout: async (options?: { reason?: LogoutReason }) => {
|
||||
if (options?.reason) {
|
||||
setLogoutReason(options.reason);
|
||||
} else {
|
||||
clearLogoutReason();
|
||||
}
|
||||
try {
|
||||
await apiClient.POST("/api/auth/logout", {});
|
||||
} catch (error) {
|
||||
@ -244,7 +288,7 @@ export const useAuthStore = create<AuthState>()((set, get) => {
|
||||
},
|
||||
|
||||
refreshUser: async () => {
|
||||
try {
|
||||
const fetchProfile = async (): Promise<void> => {
|
||||
const response = await apiClient.GET<{
|
||||
isAuthenticated?: boolean;
|
||||
user?: AuthenticatedUser;
|
||||
@ -256,43 +300,33 @@ export const useAuthStore = create<AuthState>()((set, get) => {
|
||||
isAuthenticated: true,
|
||||
error: null,
|
||||
});
|
||||
return;
|
||||
} else {
|
||||
set({ user: null, isAuthenticated: false, session: {} });
|
||||
}
|
||||
};
|
||||
|
||||
// No active session
|
||||
set({ user: null, isAuthenticated: false, session: {} });
|
||||
try {
|
||||
await fetchProfile();
|
||||
} catch (error) {
|
||||
const shouldLogout = handleAuthError(error, get().logout);
|
||||
if (shouldLogout) {
|
||||
const errorInfo = getErrorInfo(error);
|
||||
if (errorInfo.shouldLogout) {
|
||||
const reason = logoutReasonFromErrorCode(errorInfo.code) ?? ("session-expired" as const);
|
||||
await get().logout({ reason });
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
const refreshResponse = await apiClient.POST("/api/auth/refresh", { body: {} });
|
||||
const parsed = authResponseSchema.safeParse(refreshResponse.data);
|
||||
if (!parsed.success) {
|
||||
throw new Error(parsed.error.issues?.[0]?.message ?? "Session refresh failed");
|
||||
}
|
||||
applyAuthResponse(parsed.data);
|
||||
await ensureSingleRefresh();
|
||||
await fetchProfile();
|
||||
} catch (refreshError) {
|
||||
logger.error(refreshError, "Failed to refresh session after auth error");
|
||||
await get().logout();
|
||||
return;
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
refreshSession: async () => {
|
||||
try {
|
||||
const response = await apiClient.POST("/api/auth/refresh", { body: {} });
|
||||
const parsed = authResponseSchema.safeParse(response.data);
|
||||
if (!parsed.success) {
|
||||
throw new Error(parsed.error.issues?.[0]?.message ?? "Session refresh failed");
|
||||
}
|
||||
applyAuthResponse(parsed.data);
|
||||
} catch (error) {
|
||||
logger.error(error, "Failed to refresh session");
|
||||
await get().logout();
|
||||
}
|
||||
await ensureSingleRefresh();
|
||||
},
|
||||
|
||||
checkAuth: async () => {
|
||||
|
||||
79
apps/portal/src/features/auth/utils/logout-reason.ts
Normal file
79
apps/portal/src/features/auth/utils/logout-reason.ts
Normal file
@ -0,0 +1,79 @@
|
||||
export type LogoutReason = "session-expired" | "token-revoked" | "manual";
|
||||
|
||||
export interface LogoutMessage {
|
||||
title: string;
|
||||
body: string;
|
||||
variant: "info" | "warning" | "error";
|
||||
}
|
||||
|
||||
const STORAGE_KEY = "customer-portal:lastLogoutReason";
|
||||
|
||||
const LOGOUT_MESSAGES: Record<LogoutReason, LogoutMessage> = {
|
||||
"session-expired": {
|
||||
title: "Session Expired",
|
||||
body: "For your security, your session expired. Please sign in again to continue.",
|
||||
variant: "warning",
|
||||
},
|
||||
"token-revoked": {
|
||||
title: "Signed Out For Your Safety",
|
||||
body: "We detected a security change and signed you out. Please sign in again to verify your session.",
|
||||
variant: "error",
|
||||
},
|
||||
manual: {
|
||||
title: "Signed Out",
|
||||
body: "You have been signed out. Sign in again whenever you're ready.",
|
||||
variant: "info",
|
||||
},
|
||||
};
|
||||
|
||||
export function setLogoutReason(reason: LogoutReason): void {
|
||||
if (typeof window === "undefined") return;
|
||||
try {
|
||||
sessionStorage.setItem(STORAGE_KEY, reason);
|
||||
} catch {
|
||||
// Ignore storage errors (e.g., private mode)
|
||||
}
|
||||
}
|
||||
|
||||
export function clearLogoutReason(): void {
|
||||
if (typeof window === "undefined") return;
|
||||
try {
|
||||
sessionStorage.removeItem(STORAGE_KEY);
|
||||
} catch {
|
||||
// Ignore storage errors
|
||||
}
|
||||
}
|
||||
|
||||
export function consumeLogoutReason(): LogoutReason | null {
|
||||
if (typeof window === "undefined") return null;
|
||||
try {
|
||||
const reason = sessionStorage.getItem(STORAGE_KEY) as LogoutReason | null;
|
||||
if (reason) {
|
||||
sessionStorage.removeItem(STORAGE_KEY);
|
||||
return reason;
|
||||
}
|
||||
} catch {
|
||||
// Ignore storage errors
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
export function logoutReasonFromErrorCode(code: string): LogoutReason | undefined {
|
||||
switch (code) {
|
||||
case "TOKEN_REVOKED":
|
||||
case "INVALID_REFRESH_TOKEN":
|
||||
return "token-revoked";
|
||||
case "SESSION_EXPIRED":
|
||||
return "session-expired";
|
||||
default:
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
export function resolveLogoutMessage(reason: LogoutReason): LogoutMessage {
|
||||
return LOGOUT_MESSAGES[reason] ?? LOGOUT_MESSAGES.manual;
|
||||
}
|
||||
|
||||
export function isLogoutReason(value: string | null | undefined): value is LogoutReason {
|
||||
return value === "session-expired" || value === "token-revoked" || value === "manual";
|
||||
}
|
||||
@ -1,16 +1,65 @@
|
||||
"use client";
|
||||
|
||||
import { useEffect, useMemo, useState } from "react";
|
||||
import { useSearchParams } from "next/navigation";
|
||||
|
||||
import { AuthLayout } from "../components";
|
||||
import { LoginForm } from "@/features/auth/components";
|
||||
import { useAuthStore } from "../services/auth.store";
|
||||
import { LoadingOverlay } from "@/components/atoms";
|
||||
import { AlertBanner } from "@/components/molecules/AlertBanner/AlertBanner";
|
||||
import {
|
||||
consumeLogoutReason,
|
||||
isLogoutReason,
|
||||
resolveLogoutMessage,
|
||||
type LogoutReason,
|
||||
} from "@/features/auth/utils/logout-reason";
|
||||
|
||||
export function LoginView() {
|
||||
const { loading, isAuthenticated } = useAuthStore();
|
||||
const searchParams = useSearchParams();
|
||||
const reasonParam = useMemo(
|
||||
() => searchParams?.get("reason"),
|
||||
[searchParams]
|
||||
);
|
||||
const [logoutReason, setLogoutReason] = useState<LogoutReason | null>(null);
|
||||
const [dismissed, setDismissed] = useState(false);
|
||||
|
||||
useEffect(() => {
|
||||
if (isLogoutReason(reasonParam)) {
|
||||
setLogoutReason(reasonParam);
|
||||
return;
|
||||
}
|
||||
|
||||
const stored = consumeLogoutReason();
|
||||
if (stored) {
|
||||
setLogoutReason(stored);
|
||||
} else {
|
||||
setLogoutReason(null);
|
||||
}
|
||||
}, [reasonParam]);
|
||||
|
||||
useEffect(() => {
|
||||
if (logoutReason) {
|
||||
setDismissed(false);
|
||||
}
|
||||
}, [logoutReason]);
|
||||
|
||||
const logoutMessage = logoutReason ? resolveLogoutMessage(logoutReason) : null;
|
||||
|
||||
return (
|
||||
<>
|
||||
<AuthLayout title="Welcome back" subtitle="Sign in to your Assist Solutions account">
|
||||
{logoutMessage && !dismissed && (
|
||||
<AlertBanner
|
||||
variant={logoutMessage.variant}
|
||||
title={logoutMessage.title}
|
||||
className="mb-4"
|
||||
onClose={() => setDismissed(true)}
|
||||
>
|
||||
{logoutMessage.body}
|
||||
</AlertBanner>
|
||||
)}
|
||||
<LoginForm />
|
||||
</AuthLayout>
|
||||
|
||||
|
||||
@ -72,7 +72,7 @@ export function ServiceConfigurationStep({ plan, mode, setMode, isTransitioning,
|
||||
onClick={onNext}
|
||||
disabled={plan?.internetPlanTier === "Silver" && !mode}
|
||||
rightIcon={<ArrowRightIcon className="w-4 h-4" />}
|
||||
className="min-w-[200px] transition-all duration-200 hover:scale-105"
|
||||
className="min-w-[200px]"
|
||||
>
|
||||
Continue to Installation
|
||||
</Button>
|
||||
@ -157,25 +157,21 @@ function ModeSelectionCard({
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => onSelect(mode)}
|
||||
className={`p-6 rounded-xl border-2 text-left transition-all duration-200 ease-out focus:outline-none focus-visible:ring-2 focus-visible:ring-blue-500 focus-visible:ring-offset-2 ${
|
||||
className={`p-6 rounded-xl border-2 text-left transition-all duration-200 focus:outline-none focus-visible:ring-2 focus-visible:ring-blue-500 focus-visible:ring-offset-2 ${
|
||||
isSelected
|
||||
? "border-blue-500 bg-gradient-to-br from-blue-50 to-blue-100/50 shadow-lg scale-[1.02]"
|
||||
: "border-gray-200 hover:border-blue-300 hover:bg-blue-50/50 shadow-sm hover:shadow-md"
|
||||
? "border-blue-500 bg-blue-50 shadow-md"
|
||||
: "border-gray-200 hover:border-blue-400 hover:bg-blue-50/50 shadow-sm hover:shadow-md"
|
||||
}`}
|
||||
aria-pressed={isSelected}
|
||||
>
|
||||
<div className="flex items-center justify-between mb-4">
|
||||
<h5 className="text-lg font-bold text-gray-900">{title}</h5>
|
||||
<div
|
||||
className={`w-5 h-5 rounded-full border-2 flex items-center justify-center transition-all duration-200 ${
|
||||
isSelected ? "bg-blue-500 border-blue-500 scale-110" : "border-gray-300"
|
||||
className={`w-5 h-5 rounded-full border-2 flex items-center justify-center ${
|
||||
isSelected ? "bg-blue-500 border-blue-500" : "border-gray-300 bg-white"
|
||||
}`}
|
||||
>
|
||||
{isSelected && (
|
||||
<svg className="w-3 h-3 text-white" fill="currentColor" viewBox="0 0 12 12">
|
||||
<circle cx="6" cy="6" r="3" />
|
||||
</svg>
|
||||
)}
|
||||
{isSelected && <div className="w-2 h-2 bg-white rounded-full"></div>}
|
||||
</div>
|
||||
</div>
|
||||
<p className="text-sm text-gray-700 mb-3 leading-relaxed">{description}</p>
|
||||
|
||||
@ -0,0 +1,244 @@
|
||||
"use client";
|
||||
|
||||
import { useCallback, useMemo } from "react";
|
||||
import { useRouter } from "next/navigation";
|
||||
import { format } from "date-fns";
|
||||
import {
|
||||
ServerIcon,
|
||||
CheckCircleIcon,
|
||||
ClockIcon,
|
||||
XCircleIcon,
|
||||
CalendarIcon,
|
||||
} from "@heroicons/react/24/outline";
|
||||
import { DataTable } from "@/components/molecules/DataTable/DataTable";
|
||||
import { StatusPill } from "@/components/atoms/status-pill";
|
||||
import type { Subscription } from "@customer-portal/domain/subscriptions";
|
||||
import { Formatting } from "@customer-portal/domain/toolkit";
|
||||
import { cn } from "@/lib/utils";
|
||||
|
||||
const { formatCurrency } = Formatting;
|
||||
|
||||
interface SubscriptionTableProps {
|
||||
subscriptions: Subscription[];
|
||||
loading?: boolean;
|
||||
onSubscriptionClick?: (subscription: Subscription) => void;
|
||||
compact?: boolean;
|
||||
className?: string;
|
||||
}
|
||||
|
||||
const getStatusIcon = (status: string) => {
|
||||
switch (status) {
|
||||
case "Active":
|
||||
return <CheckCircleIcon className="h-5 w-5 text-green-500" />;
|
||||
case "Completed":
|
||||
return <CheckCircleIcon className="h-5 w-5 text-blue-500" />;
|
||||
case "Cancelled":
|
||||
return <XCircleIcon className="h-5 w-5 text-gray-500" />;
|
||||
default:
|
||||
return <ClockIcon className="h-5 w-5 text-gray-500" />;
|
||||
}
|
||||
};
|
||||
|
||||
const getStatusVariant = (status: string) => {
|
||||
switch (status) {
|
||||
case "Active":
|
||||
return "success" as const;
|
||||
case "Completed":
|
||||
return "info" as const;
|
||||
case "Cancelled":
|
||||
return "neutral" as const;
|
||||
default:
|
||||
return "neutral" as const;
|
||||
}
|
||||
};
|
||||
|
||||
// Simple UI helper - converts cycle to display text
|
||||
const getBillingPeriodText = (cycle: string): string => {
|
||||
switch (cycle) {
|
||||
case "Monthly":
|
||||
return "per month";
|
||||
case "Annually":
|
||||
return "per year";
|
||||
case "Quarterly":
|
||||
return "per quarter";
|
||||
case "Semi-Annually":
|
||||
return "per 6 months";
|
||||
case "Biennially":
|
||||
return "per 2 years";
|
||||
case "Triennially":
|
||||
return "per 3 years";
|
||||
case "One-time":
|
||||
return "one-time";
|
||||
case "Free":
|
||||
return "free";
|
||||
default:
|
||||
return cycle.toLowerCase();
|
||||
}
|
||||
};
|
||||
|
||||
const formatDate = (dateString: string | undefined) => {
|
||||
if (!dateString) return "N/A";
|
||||
try {
|
||||
return format(new Date(dateString), "MMM d, yyyy");
|
||||
} catch {
|
||||
return "N/A";
|
||||
}
|
||||
};
|
||||
|
||||
export function SubscriptionTable({
|
||||
subscriptions,
|
||||
loading = false,
|
||||
onSubscriptionClick,
|
||||
compact = false,
|
||||
className,
|
||||
}: SubscriptionTableProps) {
|
||||
const router = useRouter();
|
||||
|
||||
const handleSubscriptionClick = useCallback(
|
||||
(subscription: Subscription) => {
|
||||
if (onSubscriptionClick) {
|
||||
onSubscriptionClick(subscription);
|
||||
} else {
|
||||
router.push(`/subscriptions/${subscription.id}`);
|
||||
}
|
||||
},
|
||||
[onSubscriptionClick, router]
|
||||
);
|
||||
|
||||
const columns = useMemo(() => {
|
||||
return [
|
||||
{
|
||||
key: "service",
|
||||
header: "Service",
|
||||
className: "",
|
||||
render: (subscription: Subscription) => {
|
||||
const statusIcon = getStatusIcon(subscription.status);
|
||||
return (
|
||||
<div className="flex items-center space-x-3 py-4">
|
||||
<div className="flex-shrink-0">{statusIcon}</div>
|
||||
<div className="min-w-0 flex-1">
|
||||
<div className="flex items-center gap-2.5">
|
||||
<div className="font-semibold text-gray-900 text-sm">
|
||||
{subscription.productName}
|
||||
</div>
|
||||
<StatusPill
|
||||
label={subscription.status}
|
||||
variant={getStatusVariant(subscription.status)}
|
||||
size="sm"
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
},
|
||||
},
|
||||
{
|
||||
key: "amount",
|
||||
header: "Amount",
|
||||
className: "",
|
||||
render: (subscription: Subscription) => (
|
||||
<div className="py-4 text-right">
|
||||
<div className="font-semibold text-gray-900 text-sm">
|
||||
{formatCurrency(subscription.amount, subscription.currency)}{" "}
|
||||
<span className="text-xs text-gray-500 font-normal">
|
||||
{getBillingPeriodText(subscription.cycle)}
|
||||
</span>
|
||||
</div>
|
||||
</div>
|
||||
),
|
||||
},
|
||||
{
|
||||
key: "nextDue",
|
||||
header: "Next Due",
|
||||
className: "",
|
||||
render: (subscription: Subscription) => (
|
||||
<div className="py-4">
|
||||
<div className="flex items-center gap-2">
|
||||
<CalendarIcon className="h-4 w-4 text-gray-400" />
|
||||
<div className="text-sm font-medium text-gray-700">
|
||||
{formatDate(subscription.nextDue)}
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
),
|
||||
},
|
||||
];
|
||||
}, [compact]);
|
||||
|
||||
const emptyState = {
|
||||
icon: <ServerIcon className="h-12 w-12" />,
|
||||
title: "No subscriptions found",
|
||||
description: "No active subscriptions at this time.",
|
||||
};
|
||||
|
||||
if (loading) {
|
||||
return (
|
||||
<div className={cn("bg-white overflow-hidden", className)}>
|
||||
<div className="animate-pulse">
|
||||
{/* Header skeleton */}
|
||||
<div className="bg-gradient-to-r from-gray-50 to-gray-50/80 px-6 py-4 border-b border-gray-200/80">
|
||||
<div className="grid grid-cols-3 gap-6">
|
||||
<div className="h-3 bg-gray-200 rounded w-24"></div>
|
||||
<div className="h-3 bg-gray-200 rounded w-20 ml-auto"></div>
|
||||
<div className="h-3 bg-gray-200 rounded w-24"></div>
|
||||
</div>
|
||||
</div>
|
||||
{/* Row skeletons */}
|
||||
<div className="divide-y divide-gray-100/60">
|
||||
{Array.from({ length: 6 }).map((_, i) => (
|
||||
<div key={i} className="px-6 py-5">
|
||||
<div className="grid grid-cols-3 gap-6 items-center">
|
||||
<div className="flex items-center space-x-3">
|
||||
<div className="h-5 w-5 bg-gray-200 rounded-full flex-shrink-0"></div>
|
||||
<div className="space-y-2 flex-1">
|
||||
<div className="h-4 bg-gray-200 rounded w-48"></div>
|
||||
</div>
|
||||
</div>
|
||||
<div className="text-right">
|
||||
<div className="h-4 bg-gray-200 rounded w-32 ml-auto"></div>
|
||||
</div>
|
||||
<div className="flex items-center gap-2">
|
||||
<div className="h-4 w-4 bg-gray-200 rounded"></div>
|
||||
<div className="h-4 bg-gray-200 rounded w-28"></div>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
<div className={cn("bg-white overflow-hidden", className)}>
|
||||
<DataTable
|
||||
data={subscriptions}
|
||||
columns={columns}
|
||||
emptyState={emptyState}
|
||||
onRowClick={handleSubscriptionClick}
|
||||
className={cn(
|
||||
"subscription-table",
|
||||
// Header styling - cleaner and more modern
|
||||
"[&_thead]:bg-gradient-to-r [&_thead]:from-gray-50 [&_thead]:to-gray-50/80",
|
||||
"[&_thead_th]:px-6 [&_thead_th]:py-3.5 [&_thead_th]:text-xs [&_thead_th]:font-medium [&_thead_th]:text-gray-600 [&_thead_th]:uppercase [&_thead_th]:tracking-wide",
|
||||
"[&_thead_th]:border-b [&_thead_th]:border-gray-200/80",
|
||||
// Right-align Amount column header (2nd column)
|
||||
"[&_thead_th:nth-child(2)]:text-right",
|
||||
// Row styling - enhanced hover and spacing
|
||||
"[&_tbody_tr]:border-b [&_tbody_tr]:border-gray-100/60 [&_tbody_tr]:transition-all [&_tbody_tr]:duration-200",
|
||||
"[&_tbody_tr:hover]:bg-gradient-to-r [&_tbody_tr:hover]:from-blue-50/30 [&_tbody_tr:hover]:to-indigo-50/20 [&_tbody_tr]:cursor-pointer",
|
||||
"[&_tbody_tr:last-child]:border-b-0",
|
||||
// Cell styling - better spacing
|
||||
"[&_tbody_td]:px-6 [&_tbody_td]:py-1 [&_tbody_td]:align-top",
|
||||
// Remove default DataTable styling
|
||||
"[&_.divide-y]:divide-transparent"
|
||||
)}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
export type { SubscriptionTableProps };
|
||||
|
||||
|
||||
@ -0,0 +1,3 @@
|
||||
export { SubscriptionTable } from './SubscriptionTable';
|
||||
export type { SubscriptionTableProps } from './SubscriptionTable';
|
||||
|
||||
@ -82,25 +82,6 @@ export function SubscriptionDetailContainer() {
|
||||
|
||||
const formatCurrency = (amount: number) => sharedFormatCurrency(amount || 0);
|
||||
|
||||
const formatBillingLabel = (cycle: string) => {
|
||||
switch (cycle) {
|
||||
case "Monthly":
|
||||
return "Monthly Billing";
|
||||
case "Annually":
|
||||
return "Annual Billing";
|
||||
case "Quarterly":
|
||||
return "Quarterly Billing";
|
||||
case "Semi-Annually":
|
||||
return "Semi-Annual Billing";
|
||||
case "Biennially":
|
||||
return "Biennial Billing";
|
||||
case "Triennially":
|
||||
return "Triennial Billing";
|
||||
default:
|
||||
return "One-time Payment";
|
||||
}
|
||||
};
|
||||
|
||||
if (isLoading) {
|
||||
return (
|
||||
<div className="py-6">
|
||||
@ -191,29 +172,32 @@ export function SubscriptionDetailContainer() {
|
||||
<h4 className="text-sm font-medium text-gray-500 uppercase tracking-wider">
|
||||
Billing Amount
|
||||
</h4>
|
||||
<p className="mt-2 text-2xl font-bold text-gray-900">
|
||||
{formatCurrency(subscription.amount)}
|
||||
</p>
|
||||
<p className="text-sm text-gray-500">{formatBillingLabel(subscription.cycle)}</p>
|
||||
<div className="mt-2 flex items-baseline gap-2">
|
||||
<p className="text-2xl font-bold text-gray-900">
|
||||
{formatCurrency(subscription.amount)}
|
||||
</p>
|
||||
<span className="text-sm text-gray-500">{subscription.cycle}</span>
|
||||
</div>
|
||||
</div>
|
||||
<div>
|
||||
<h4 className="text-sm font-medium text-gray-500 uppercase tracking-wider">
|
||||
Next Due Date
|
||||
</h4>
|
||||
<p className="mt-2 text-lg text-gray-900">{formatDate(subscription.nextDue)}</p>
|
||||
<div className="flex items-center mt-1">
|
||||
<CalendarIcon className="h-4 w-4 text-gray-400 mr-1" />
|
||||
<span className="text-sm text-gray-500">Due date</span>
|
||||
<div className="flex items-center mt-2">
|
||||
<CalendarIcon className="h-4 w-4 text-gray-400 mr-2" />
|
||||
<p className="text-lg font-medium text-gray-900">{formatDate(subscription.nextDue)}</p>
|
||||
</div>
|
||||
</div>
|
||||
<div>
|
||||
<h4 className="text-sm font-medium text-gray-500 uppercase tracking-wider">
|
||||
Registration Date
|
||||
</h4>
|
||||
<p className="mt-2 text-lg text-gray-900">
|
||||
{formatDate(subscription.registrationDate)}
|
||||
</p>
|
||||
<span className="text-sm text-gray-500">Service created</span>
|
||||
<div className="flex items-center mt-2">
|
||||
<CalendarIcon className="h-4 w-4 text-gray-400 mr-2" />
|
||||
<p className="text-lg font-medium text-gray-900">
|
||||
{formatDate(subscription.registrationDate)}
|
||||
</p>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
@ -256,7 +240,7 @@ export function SubscriptionDetailContainer() {
|
||||
</div>
|
||||
)}
|
||||
|
||||
{showInvoices && <InvoicesList subscriptionId={subscriptionId} />}
|
||||
{showInvoices && <InvoicesList subscriptionId={subscriptionId} pageSize={5} />}
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
|
||||
@ -1,34 +1,23 @@
|
||||
"use client";
|
||||
|
||||
import { useState, useMemo } from "react";
|
||||
import { useRouter } from "next/navigation";
|
||||
import Link from "next/link";
|
||||
import { Button } from "@/components/atoms/button";
|
||||
import { ErrorBoundary } from "@/components/molecules";
|
||||
import { PageLayout } from "@/components/templates/PageLayout";
|
||||
import { DataTable } from "@/components/molecules/DataTable/DataTable";
|
||||
import { StatusPill } from "@/components/atoms/status-pill";
|
||||
import { SubCard } from "@/components/molecules/SubCard/SubCard";
|
||||
import { SearchFilterBar } from "@/components/molecules/SearchFilterBar/SearchFilterBar";
|
||||
import { LoadingTable } from "@/components/atoms/loading-skeleton";
|
||||
import { AsyncBlock } from "@/components/molecules/AsyncBlock/AsyncBlock";
|
||||
import { SubscriptionTable } from "@/features/subscriptions/components/SubscriptionTable";
|
||||
import {
|
||||
ServerIcon,
|
||||
CheckCircleIcon,
|
||||
ClockIcon,
|
||||
XCircleIcon,
|
||||
CalendarIcon,
|
||||
ArrowTopRightOnSquareIcon,
|
||||
} from "@heroicons/react/24/outline";
|
||||
import { format } from "date-fns";
|
||||
import { useSubscriptions, useSubscriptionStats } from "@/features/subscriptions/hooks";
|
||||
import type { Subscription } from "@customer-portal/domain/subscriptions";
|
||||
|
||||
import { useFormatCurrency } from "@/lib/hooks/useFormatCurrency";
|
||||
|
||||
export function SubscriptionsListContainer() {
|
||||
const router = useRouter();
|
||||
const { formatCurrency } = useFormatCurrency();
|
||||
const [searchTerm, setSearchTerm] = useState("");
|
||||
const [statusFilter, setStatusFilter] = useState("all");
|
||||
|
||||
@ -55,27 +44,6 @@ export function SubscriptionsListContainer() {
|
||||
});
|
||||
}, [subscriptions, searchTerm]);
|
||||
|
||||
const getStatusIcon = (status: string) => {
|
||||
switch (status) {
|
||||
case "Active":
|
||||
return <CheckCircleIcon className="h-5 w-5 text-green-500" />;
|
||||
case "Completed":
|
||||
return <CheckCircleIcon className="h-5 w-5 text-blue-500" />;
|
||||
case "Cancelled":
|
||||
return <XCircleIcon className="h-5 w-5 text-gray-500" />;
|
||||
default:
|
||||
return <ClockIcon className="h-5 w-5 text-gray-500" />;
|
||||
}
|
||||
};
|
||||
|
||||
const formatDate = (dateString: string) => {
|
||||
try {
|
||||
return format(new Date(dateString), "MMM d, yyyy");
|
||||
} catch {
|
||||
return "Invalid date";
|
||||
}
|
||||
};
|
||||
|
||||
const statusFilterOptions = [
|
||||
{ value: "all", label: "All Status" },
|
||||
{ value: "Active", label: "Active" },
|
||||
@ -83,105 +51,6 @@ export function SubscriptionsListContainer() {
|
||||
{ value: "Cancelled", label: "Cancelled" },
|
||||
];
|
||||
|
||||
const getStatusVariant = (status: string) => {
|
||||
switch (status) {
|
||||
case "Active":
|
||||
return "success" as const;
|
||||
case "Completed":
|
||||
return "info" as const;
|
||||
case "Cancelled":
|
||||
return "neutral" as const;
|
||||
default:
|
||||
return "neutral" as const;
|
||||
}
|
||||
};
|
||||
|
||||
const subscriptionColumns = [
|
||||
{
|
||||
key: "service",
|
||||
header: "Service",
|
||||
render: (s: Subscription) => (
|
||||
<div className="flex items-center">
|
||||
{getStatusIcon(s.status)}
|
||||
<div className="ml-3">
|
||||
<div className="text-sm font-medium text-gray-900">{s.productName}</div>
|
||||
<div className="text-sm text-gray-500">Service ID: {s.serviceId}</div>
|
||||
</div>
|
||||
</div>
|
||||
),
|
||||
},
|
||||
{
|
||||
key: "status",
|
||||
header: "Status",
|
||||
render: (s: Subscription) => (
|
||||
<StatusPill label={s.status} variant={getStatusVariant(s.status)} />
|
||||
),
|
||||
},
|
||||
{
|
||||
key: "cycle",
|
||||
header: "Billing Cycle",
|
||||
render: (s: Subscription) => {
|
||||
const name = (s.productName || "").toLowerCase();
|
||||
const looksLikeActivation =
|
||||
name.includes("activation fee") || name.includes("activation") || name.includes("setup");
|
||||
const displayCycle = looksLikeActivation ? "One-time" : s.cycle;
|
||||
return <span className="text-sm text-gray-900">{displayCycle}</span>;
|
||||
},
|
||||
},
|
||||
{
|
||||
key: "price",
|
||||
header: "Price",
|
||||
render: (s: Subscription) => (
|
||||
<div>
|
||||
<span className="text-sm font-medium text-gray-900">{formatCurrency(s.amount)}</span>
|
||||
<div className="text-xs text-gray-500">
|
||||
{s.cycle === "Monthly"
|
||||
? "per month"
|
||||
: s.cycle === "Annually"
|
||||
? "per year"
|
||||
: s.cycle === "Quarterly"
|
||||
? "per quarter"
|
||||
: s.cycle === "Semi-Annually"
|
||||
? "per 6 months"
|
||||
: s.cycle === "Biennially"
|
||||
? "per 2 years"
|
||||
: s.cycle === "Triennially"
|
||||
? "per 3 years"
|
||||
: s.cycle === "One-time"
|
||||
? "one-time"
|
||||
: "one-time"}
|
||||
</div>
|
||||
</div>
|
||||
),
|
||||
},
|
||||
{
|
||||
key: "nextDue",
|
||||
header: "Next Due",
|
||||
render: (s: Subscription) => (
|
||||
<div className="flex items-center">
|
||||
<CalendarIcon className="h-4 w-4 text-gray-400 mr-2" />
|
||||
<span className="text-sm text-gray-500">{s.nextDue ? formatDate(s.nextDue) : "N/A"}</span>
|
||||
</div>
|
||||
),
|
||||
},
|
||||
{
|
||||
key: "actions",
|
||||
header: "",
|
||||
className: "relative",
|
||||
render: (s: Subscription) => (
|
||||
<div className="flex items-center justify-end space-x-2">
|
||||
<Link
|
||||
href={`/subscriptions/${s.id}`}
|
||||
className="text-blue-600 hover:text-blue-900 text-sm cursor-pointer"
|
||||
>
|
||||
View
|
||||
</Link>
|
||||
<ArrowTopRightOnSquareIcon className="h-4 w-4 text-gray-400" />
|
||||
</div>
|
||||
),
|
||||
},
|
||||
];
|
||||
|
||||
if (isLoading || error) {
|
||||
return (
|
||||
<PageLayout
|
||||
@ -191,8 +60,8 @@ export function SubscriptionsListContainer() {
|
||||
>
|
||||
<AsyncBlock isLoading={false} error={error}>
|
||||
<div className="space-y-6">
|
||||
<div className="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-4 gap-4 mb-6">
|
||||
{Array.from({ length: 4 }).map((_, i) => (
|
||||
<div className="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 gap-4 mb-6">
|
||||
{Array.from({ length: 3 }).map((_, i) => (
|
||||
<SubCard key={i}>
|
||||
<div className="flex items-center">
|
||||
<div className="flex-shrink-0">
|
||||
@ -206,7 +75,7 @@ export function SubscriptionsListContainer() {
|
||||
</SubCard>
|
||||
))}
|
||||
</div>
|
||||
<LoadingTable rows={6} columns={5} />
|
||||
<LoadingTable rows={6} columns={4} />
|
||||
</div>
|
||||
</AsyncBlock>
|
||||
</PageLayout>
|
||||
@ -269,8 +138,9 @@ export function SubscriptionsListContainer() {
|
||||
</div>
|
||||
)}
|
||||
|
||||
<SubCard
|
||||
header={
|
||||
<div className="space-y-4">
|
||||
{/* Search/Filter Header */}
|
||||
<div className="bg-white/80 backdrop-blur-sm rounded-xl border border-gray-200/60 px-5 py-4 shadow-sm">
|
||||
<SearchFilterBar
|
||||
searchValue={searchTerm}
|
||||
onSearchChange={setSearchTerm}
|
||||
@ -280,23 +150,17 @@ export function SubscriptionsListContainer() {
|
||||
filterOptions={statusFilterOptions}
|
||||
filterLabel="Filter by status"
|
||||
/>
|
||||
}
|
||||
headerClassName="bg-gray-50 rounded md:p-2 p-1 mb-1"
|
||||
>
|
||||
<DataTable
|
||||
data={filteredSubscriptions}
|
||||
columns={subscriptionColumns}
|
||||
emptyState={{
|
||||
icon: <ServerIcon className="h-12 w-12" />,
|
||||
title: "No subscriptions found",
|
||||
description:
|
||||
searchTerm || statusFilter !== "all"
|
||||
? "Try adjusting your search or filter criteria."
|
||||
: "No active subscriptions at this time.",
|
||||
}}
|
||||
onRowClick={s => router.push(`/subscriptions/${s.id}`)}
|
||||
/>
|
||||
</SubCard>
|
||||
</div>
|
||||
|
||||
{/* Subscriptions Table */}
|
||||
<div className="bg-white rounded-xl border border-gray-200/60 shadow-sm overflow-hidden">
|
||||
<SubscriptionTable
|
||||
subscriptions={filteredSubscriptions}
|
||||
loading={isLoading}
|
||||
className="border-0 rounded-none shadow-none"
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
</ErrorBoundary>
|
||||
</PageLayout>
|
||||
);
|
||||
|
||||
@ -69,7 +69,7 @@ export function isApiErrorPayload(error: unknown): error is ApiErrorPayload {
|
||||
* Determine if the user should be logged out for this error
|
||||
*/
|
||||
function shouldLogoutForError(code: string): boolean {
|
||||
const logoutCodes = ["TOKEN_REVOKED", "INVALID_REFRESH_TOKEN", "UNAUTHORIZED", "SESSION_EXPIRED"];
|
||||
const logoutCodes = ["TOKEN_REVOKED", "INVALID_REFRESH_TOKEN"];
|
||||
return logoutCodes.includes(code);
|
||||
}
|
||||
|
||||
@ -148,7 +148,7 @@ function parseClientApiError(error: ClientApiError): ApiErrorInfo | null {
|
||||
return {
|
||||
code: status ? httpStatusCodeToLabel(status) : "API_ERROR",
|
||||
message: error.message,
|
||||
shouldLogout: status === 401,
|
||||
shouldLogout: status ? shouldLogoutForError(httpStatusCodeToLabel(status)) : false,
|
||||
shouldRetry: typeof status === "number" ? status >= 500 : true,
|
||||
};
|
||||
}
|
||||
@ -176,7 +176,7 @@ function deriveInfoFromPayload(payload: unknown, status?: number): ApiErrorInfo
|
||||
return {
|
||||
code,
|
||||
message: parsed.data.error.message,
|
||||
shouldLogout: shouldLogoutForError(code) || status === 401,
|
||||
shouldLogout: shouldLogoutForError(code),
|
||||
shouldRetry: shouldRetryForError(code),
|
||||
};
|
||||
}
|
||||
@ -189,7 +189,7 @@ function deriveInfoFromPayload(payload: unknown, status?: number): ApiErrorInfo
|
||||
return {
|
||||
code,
|
||||
message,
|
||||
shouldLogout: shouldLogoutForError(code) || derivedStatus === 401,
|
||||
shouldLogout: shouldLogoutForError(code),
|
||||
shouldRetry:
|
||||
typeof derivedStatus === "number" ? derivedStatus >= 500 : shouldRetryForError(code),
|
||||
};
|
||||
@ -208,7 +208,7 @@ function deriveInfoFromPayload(payload: unknown, status?: number): ApiErrorInfo
|
||||
return {
|
||||
code,
|
||||
message: payloadWithMessage.message,
|
||||
shouldLogout: shouldLogoutForError(code) || status === 401,
|
||||
shouldLogout: shouldLogoutForError(code),
|
||||
shouldRetry: typeof status === "number" ? status >= 500 : shouldRetryForError(code),
|
||||
};
|
||||
}
|
||||
|
||||
189
docs/architecture/SUBSCRIPTIONS-REFACTOR.md
Normal file
189
docs/architecture/SUBSCRIPTIONS-REFACTOR.md
Normal file
@ -0,0 +1,189 @@
|
||||
# Subscriptions List Refactor
|
||||
|
||||
## Summary
|
||||
|
||||
Refactored the subscriptions list page to follow clean architecture principles, removing business logic from the frontend and creating a consistent design pattern matching the invoice list implementation.
|
||||
|
||||
## Problems Fixed
|
||||
|
||||
### 1. **Business Logic in Frontend** ❌
|
||||
**Before:**
|
||||
```typescript
|
||||
// Frontend was detecting one-time products by product name
|
||||
const getBillingCycle = (subscription: Subscription) => {
|
||||
const name = subscription.productName.toLowerCase();
|
||||
const looksLikeActivation =
|
||||
name.includes("activation fee") ||
|
||||
name.includes("activation") ||
|
||||
name.includes("setup");
|
||||
return looksLikeActivation ? "One-time" : subscription.cycle;
|
||||
};
|
||||
```
|
||||
|
||||
**After:** ✅
|
||||
- **BFF already sends correct cycle** via WHMCS mapper
|
||||
- Frontend trusts the data from BFF
|
||||
- No business logic to detect product types
|
||||
|
||||
### 2. **Inconsistent List Designs** ❌
|
||||
**Before:**
|
||||
- Invoices: Clean `InvoiceTable` component with modern DataTable
|
||||
- Subscriptions: Custom card-based list with manual rendering
|
||||
- Different patterns for similar functionality
|
||||
|
||||
**After:** ✅
|
||||
- Created `SubscriptionTable` component following `InvoiceTable` pattern
|
||||
- Consistent design across all list pages
|
||||
- Reusable DataTable component
|
||||
|
||||
### 3. **No Use of Domain Types/Validation** ❌
|
||||
**Before:**
|
||||
- Frontend had local type transformations
|
||||
- Business logic scattered across components
|
||||
|
||||
**After:** ✅
|
||||
- Uses `Subscription` type from `@customer-portal/domain/subscriptions`
|
||||
- BFF handles all transformations via domain mappers
|
||||
- Frontend only displays pre-validated data
|
||||
|
||||
## Architecture
|
||||
|
||||
### Clean Separation of Concerns
|
||||
|
||||
```
|
||||
┌─────────────────────────────────────────┐
|
||||
│ Frontend (UI Layer) │
|
||||
│ - Display data │
|
||||
│ - Simple UI transformations │
|
||||
│ ("Monthly" → "per month") │
|
||||
│ - No business logic │
|
||||
└─────────────────────────────────────────┘
|
||||
↓ Consumes validated data
|
||||
┌─────────────────────────────────────────┐
|
||||
│ Domain Layer │
|
||||
│ - Types (Subscription, SubscriptionList)│
|
||||
│ - Schemas (Zod validation) │
|
||||
│ - Constants (SUBSCRIPTION_STATUS) │
|
||||
└─────────────────────────────────────────┘
|
||||
↑ Transforms to domain types
|
||||
┌─────────────────────────────────────────┐
|
||||
│ BFF (Business Logic Layer) │
|
||||
│ - WHMCS mapper handles cycle detection │
|
||||
│ - Pricing calculations │
|
||||
│ - Status transformations │
|
||||
│ - Validates against domain schemas │
|
||||
└─────────────────────────────────────────┘
|
||||
```
|
||||
|
||||
### What Goes Where
|
||||
|
||||
| Concern | Layer | Example |
|
||||
|---------|-------|---------|
|
||||
| **Data transformation** | BFF | Detect one-time products, map WHMCS status |
|
||||
| **Validation** | Domain | Zod schemas, type safety |
|
||||
| **Display formatting** | Frontend or Domain Toolkit | Currency, dates, simple text transforms |
|
||||
| **Business rules** | BFF | Pricing, billing cycles, activation detection |
|
||||
| **UI State** | Frontend | Search, filters, loading states |
|
||||
|
||||
## Implementation Details
|
||||
|
||||
### SubscriptionTable Component
|
||||
|
||||
Follows the same pattern as `InvoiceTable`:
|
||||
|
||||
```typescript
|
||||
// Uses domain types
|
||||
import type { Subscription } from "@customer-portal/domain/subscriptions";
|
||||
import { Formatting } from "@customer-portal/domain/toolkit";
|
||||
|
||||
// Uses DataTable for consistent UX
|
||||
<DataTable
|
||||
data={subscriptions}
|
||||
columns={columns}
|
||||
emptyState={emptyState}
|
||||
onRowClick={handleClick}
|
||||
/>
|
||||
```
|
||||
|
||||
**Key Features:**
|
||||
- Clean table design with hover states
|
||||
- Status badges with icons
|
||||
- Proper date formatting using `date-fns`
|
||||
- Currency formatting via domain toolkit
|
||||
- Loading skeletons
|
||||
- Empty states
|
||||
|
||||
### Simple UI Helpers
|
||||
|
||||
**NOT in domain layer** - just local to the component:
|
||||
|
||||
```typescript
|
||||
// Simple UI text transformation (OK in frontend)
|
||||
const getBillingPeriodText = (cycle: string): string => {
|
||||
switch (cycle) {
|
||||
case "Monthly": return "per month";
|
||||
case "Annually": return "per year";
|
||||
// ... etc
|
||||
}
|
||||
};
|
||||
```
|
||||
|
||||
### No Unnecessary Domain Helpers
|
||||
|
||||
**We DON'T need** helpers like `isOneTimeProduct()` in the domain layer because:
|
||||
|
||||
1. **BFF already handles this** - The WHMCS mapper correctly sets the cycle
|
||||
2. **Frontend shouldn't "fix" data** - That's a code smell indicating business logic
|
||||
3. **Simple text mapping** - "Monthly" → "per month" is just UI concern
|
||||
|
||||
## Benefits
|
||||
|
||||
### ✅ Clean Architecture
|
||||
- Business logic stays in BFF where it belongs
|
||||
- Frontend is a thin presentation layer
|
||||
- Domain provides shared types and validation
|
||||
|
||||
### ✅ Consistency
|
||||
- All list pages use the same pattern
|
||||
- Predictable structure for developers
|
||||
- Easier to maintain and extend
|
||||
|
||||
### ✅ Type Safety
|
||||
- Uses domain types everywhere
|
||||
- TypeScript enforces contracts
|
||||
- Zod validation at runtime
|
||||
|
||||
### ✅ Better UX
|
||||
- Modern, clean design
|
||||
- Smooth hover effects
|
||||
- Consistent with invoice list
|
||||
- Proper loading and empty states
|
||||
|
||||
## Files Changed
|
||||
|
||||
### Created
|
||||
- `apps/portal/src/features/subscriptions/components/SubscriptionTable/SubscriptionTable.tsx`
|
||||
- `apps/portal/src/features/subscriptions/components/SubscriptionTable/index.ts`
|
||||
|
||||
### Modified
|
||||
- `apps/portal/src/features/subscriptions/views/SubscriptionsList.tsx` - Simplified, removed business logic
|
||||
|
||||
### Removed
|
||||
- Unnecessary display helpers (business logic doesn't belong in domain)
|
||||
|
||||
## Lessons Learned
|
||||
|
||||
1. **Trust the BFF** - If BFF sends correct data, frontend shouldn't "fix" it
|
||||
2. **Simple UI helpers OK inline** - Not everything needs to be in domain layer
|
||||
3. **Domain toolkit for utilities** - Use existing formatting utilities (currency, dates)
|
||||
4. **Follow existing patterns** - InvoiceTable was the right pattern to follow
|
||||
5. **Business logic = BFF** - Frontend should only display data, not transform it
|
||||
|
||||
## Next Steps
|
||||
|
||||
Consider applying this pattern to other list pages:
|
||||
- Orders list
|
||||
- Payment methods list
|
||||
- Support tickets list
|
||||
- etc.
|
||||
|
||||
@ -81,12 +81,12 @@ export const salesforceOrderRecordSchema = z.object({
|
||||
Porting_Last_Name_Katakana__c: z.string().nullable().optional(),
|
||||
Porting_Gender__c: z.string().nullable().optional(),
|
||||
|
||||
// Billing address snapshot fields
|
||||
Billing_Street__c: z.string().nullable().optional(),
|
||||
Billing_City__c: z.string().nullable().optional(),
|
||||
Billing_State__c: z.string().nullable().optional(),
|
||||
Billing_Postal_Code__c: z.string().nullable().optional(),
|
||||
Billing_Country__c: z.string().nullable().optional(),
|
||||
// Billing address snapshot fields (standard Salesforce Order columns)
|
||||
BillingStreet: z.string().nullable().optional(),
|
||||
BillingCity: z.string().nullable().optional(),
|
||||
BillingState: z.string().nullable().optional(),
|
||||
BillingPostalCode: z.string().nullable().optional(),
|
||||
BillingCountry: z.string().nullable().optional(),
|
||||
|
||||
// Other fields
|
||||
Address_Changed__c: z.boolean().nullable().optional(),
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user