architecture-refactor

What this skill does

# Architecture Refactor

Analyze backend code for a domain concept and refactor to follow the entity/service separation pattern.

## Architecture Overview

```
Controller (HTTP only)
    ↓
Service (orchestration, enforcement, side effects)
    ↓
Entity (business rules as predicates, data access)
    ↓
Database
```

### Layer Responsibilities

| Layer          | Does                                                                                            | Does Not                                          |
| -------------- | ----------------------------------------------------------------------------------------------- | ------------------------------------------------- |
| **Controller** | Parse request, auth, permissions, call service, format response                                 | Business logic, call entities, transactions       |
| **Service**    | Orchestrate entities, enforce rules (throw), business logic, transactions, trigger side effects | Define rules, write SQL, know HTTP                |
| **Entity**     | Define rules (canX → boolean), CRUD, queries, data transformation                               | Throw on rules, call other entities, side effects |

### Key Principle: Rules vs Enforcement

```
ENTITY (defines rules):
  canConvert(deal) → { allowed: false, reason: 'Already converted' }

SERVICE (enforces rules):
  const { allowed, reason } = dealEntity.canConvert(deal);
  if (!allowed) throw new ValidationError(reason);  ← SERVICE THROWS
```

## Workflow

### Step 1: Identify Target Files

When user asks to refactor a concept (e.g., "deals"), locate:

```bash
# Find relevant files
find . -name "*deal*" -type f | grep -E "\.(ts|js)$"
```

Look for:

- `{concept}Service.ts` (e.g., `dealsService.ts`)
- `{concept}Controller.ts` (e.g., `dealsController.ts`)
- `{concept}Entity.ts` if it exists
- Related types in `@fsai/sdk` or local `.types.ts` files

Report what you found:

```
📁 Found files for "deals":
  • api/deals/dealsService.ts (523 lines)
  • api/deals/dealsController.ts (412 lines)
  • contact/deal/dealEntity.ts (89 lines) - partial implementation
  • Types: @fsai/sdk Deal, DealOverview, DealSummary
```

### Step 2: Analyze Current State

Scan the service file and categorize code:

**🔴 SCATTERED BUSINESS RULES** (should be entity predicates)

```typescript
// These patterns should become entity predicates:
if (deal.convertedAt) throw new ValidationError('...')  → canConvert()
if (!deal.applicationId) throw new ValidationError('...')  → canConvert()
if (deal.status === 'won') ...  → canDelete(), isWon()
if (!deal.applicationId && deal.franchiseeOrgId) ...  → isEntityDeal()
```

**🟡 DATA ACCESS** (should move to entity)

```typescript
// Direct database calls should move to entity:
await database.query.deals.findFirst(...)  → entity.getById()
await database.insert(drizzleSchema.deals).values(...)  → entity.create()
await database.update(drizzleSchema.deals).set(...)  → entity.update()
```

**🟠 SIDE EFFECTS** (should be extracted to notification/event classes)

```typescript
// These should be separate:
await notificationsService.franchisees.sendBatch(...)  → dealNotifications.onConverted()
await portalEventsService.fireEvent(...)  → dealEvents.emitConverted()
logger.info('Business event...')  → dealEvents.emit*()
```

**🟢 ORCHESTRATION** (correct location - stays in service)

```typescript
// This is correct for service:
await database.transaction(async () => { ... })
const result = await entityA.create(); await entityB.update();
if (featureFlag) { ... } else { ... }
```

Report findings:

```
🔍 Analysis of dealsService.ts:

BUSINESS RULES (scattered - move to entity):
  • Line 340: if (deal.convertedAt) throw → canConvert()
  • Line 336: if (!deal.applicationId) throw → canConvert()
  • Line 89: implicit - deals require applicationId → document as rule

DATA ACCESS (move to entity):
  • getDealOverview() - 80 lines of joins
  • createDeal() - insert with displayId generation
  • updateDeal() - direct update

SIDE EFFECTS (extract):
  • Line 412: notification batch → dealNotifications.onConverted()
  • Line 380: invitation service → keep in service but after transaction

ORCHESTRATION (correct - keep in service):
  • convertDealToFranchisee() - coordinates multiple entities
  • Transaction at line 350
```

### Step 3: Propose Entity Interface

Based on analysis, propose the entity structure:

```typescript
// Proposed: dealEntity.ts

class DealEntity {
  // ═══════════ Business Rules (Predicates) ═══════════
  // Return boolean or { allowed, reason } - NEVER throw

  isEntityDeal(deal): boolean;
  isApplicationDeal(deal): boolean;
  canConvert(deal): { allowed: boolean; reason?: string };
  canHaveProposedLocations(deal): boolean;
  canHaveAdditionalApplications(deal): boolean;
  canDelete(deal): { allowed: boolean; reason?: string };

  // ═══════════ Data Access ═══════════

  getById(dealId): Promise<Deal | null>;
  getOverview(dealId): Promise<DealOverview | null>;
  getByApplication(applicationId): Promise<string | null>;
  getByBrand(brandId): Promise<DealSummary[]>;

  create(params): Promise<string>;
  update(dealId, updates): Promise<void>;
  markConverted(dealId, franchiseeOrgId): Promise<void>;
  delete(dealId): Promise<void>;
}
```

**Ask user to confirm before proceeding with implementation.**

### Step 4: Implement Entity

Create or update the entity file following this pattern:

```typescript
import { eq, and, desc } from "drizzle-orm";
import { drizzleSchema } from "@fsai/supabase";
import { database } from "../../db/db.js";
import type { Deal, DealOverview } from "@fsai/sdk";

class DealEntity {
  // ═══════════════════════════════════════════════════════════════
  // BUSINESS RULES (Predicates)
  // - Return boolean or { allowed, reason }
  // - NEVER throw
  // - No side effects
  // - Testable in isolation
  // ═══════════════════════════════════════════════════════════════

  isEntityDeal(
    deal: Pick<DealOverview, "applicationId" | "franchiseeOrgId">
  ): boolean {
    return !deal.applicationId && Boolean(deal.franchiseeOrgId);
  }

  isApplicationDeal(deal: Pick<DealOverview, "applicationId">): boolean {
    return Boolean(deal.applicationId);
  }

  canConvert(
    deal: Pick<
      DealOverview,
      "applicationId" | "convertedAt" | "franchiseeOrgId"
    >
  ): { allowed: boolean; reason?: string } {
    if (deal.convertedAt) {
      return { allowed: false, reason: "Deal has already been converted" };
    }
    if (this.isEntityDeal(deal)) {
      return {
        allowed: false,
        reason: "Entity-based deals cannot be converted",
      };
    }
    if (!deal.applicationId) {
      return { allowed: false, reason: "Deal has no application to convert" };
    }
    return { allowed: true };
  }

  canHaveProposedLocations(
    deal: Pick<DealOverview, "applicationId" | "franchiseeOrgId">
  ): boolean {
    return this.isApplicationDeal(deal);
  }

  canDelete(deal: Pick<DealOverview, "convertedAt" | "status">): {
    allowed: boolean;
    reason?: string;
  } {
    if (deal.convertedAt) {
      return { allowed: false, reason: "Cannot delete converted deals" };
    }
    return { allowed: true };
  }

  // ═══════════════════════════════════════════════════════════════
  // DATA ACCESS
  // - Encapsulate all database operations
  // - Handle joins and transformations
  // - Return null for not found (don't throw usually)
  // ═══════════════════════════════════════════════════════════════

  async getById(dealId: string): Promise<Deal | null> {
    const data = await database.query.deals.findFirst({
      where: eq(drizzleSchema.deals.id, dealId),
    });
    return data ?? null;
  }

  async getOverview(dealId: string): Promise<DealOverview | null> {
    // Complex query with joins, transformed to domain shape
    const data = await database.query.deals.findFirst({
      where: eq(drizzleSchema.deals.id, dealId),
      with: {
        dealsAgreementsFees: { with: { agreementFee: tru