# 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.

