Assist_Design/docs/decisions/006-thin-controllers.md

# ADR-006: Thin Controllers

**Date**: 2025-01-15
**Status**: Accepted

## Context

NestJS controllers can contain varying amounts of logic:

- **Fat controllers**: Business logic, validation, transformation all in controller
- **Thin controllers**: HTTP handling only, delegate everything else

## Decision

**Controllers handle HTTP concerns only**. All business logic lives in services.

```typescript
// ✅ GOOD: Thin controller
@Get()
async getInvoices(
  @Request() req: RequestWithUser,
  @Query() query: InvoiceListQueryDto
): Promise<InvoiceList> {
  return this.invoiceService.getInvoices(req.user.id, query);
}

// ❌ BAD: Fat controller with business logic
@Get()
async getInvoices(@Request() req, @Query() query) {
  const clientId = await this.mappings.getClientId(req.user.id);
  const raw = await this.whmcs.getInvoices({ clientId });
  const filtered = raw.filter(inv => inv.status !== 'draft');
  return filtered.map(inv => transformInvoice(inv));
}
```

## Rationale

### Why Thin Controllers?

1. **Single responsibility**: Controllers handle HTTP (request parsing, response formatting, status codes). Services handle business logic.

2. **Testability**: Business logic in services can be unit tested without HTTP mocking.

3. **Reusability**: Service methods can be called from:
   - Multiple controllers
   - Background jobs
   - Event handlers
   - CLI commands

4. **Consistency**: Developers know controllers are just HTTP glue.

### Controller Responsibilities

**DO in controllers:**

- Route definition (`@Get()`, `@Post()`, etc.)
- Request parsing (`@Body()`, `@Query()`, `@Param()`)
- Authentication extraction (`@Request() req`)
- HTTP status codes (`@HttpCode()`)
- OpenAPI documentation (`@ZodResponse()`)
- Call ONE service method

**DON'T in controllers:**

- Business logic
- Data transformation
- External API calls
- Database queries
- Multiple service orchestration
- Error handling beyond HTTP concerns

### Alternatives Considered

| Approach             | Pros                    | Cons                     |
| -------------------- | ----------------------- | ------------------------ |
| **Fat controllers**  | Everything in one place | Untestable, not reusable |
| **Thin controllers** | Testable, reusable, SRP | More files               |

## Consequences

### Positive

- Clear separation of concerns
- Testable business logic
- Reusable service methods
- Consistent patterns

### Negative

- More files (controller + service)
- Developers might create pass-through services

## Implementation

### Controller Structure

```typescript
// apps/bff/src/modules/billing/billing.controller.ts
@Controller("invoices")
export class BillingController {
  constructor(private readonly invoiceService: InvoiceRetrievalService) {}

  @Get()
  @ZodResponse({ description: "List invoices", type: InvoiceListDto })
  async getInvoices(
    @Request() req: RequestWithUser,
    @Query() query: InvoiceListQueryDto
  ): Promise<InvoiceList> {
    // One service call - that's it
    return this.invoiceService.getInvoices(req.user.id, query);
  }

  @Get(":id")
  @ZodResponse({ description: "Get invoice", type: InvoiceDto })
  async getInvoice(@Request() req: RequestWithUser, @Param("id") id: string): Promise<Invoice> {
    return this.invoiceService.getInvoice(req.user.id, parseInt(id));
  }
}
```

### Service Structure

```typescript
// apps/bff/src/modules/billing/services/invoice-retrieval.service.ts
@Injectable()
export class InvoiceRetrievalService {
  constructor(
    private readonly whmcsInvoice: WhmcsInvoiceService,
    private readonly mappings: MappingsService
  ) {}

  async getInvoices(userId: string, query: InvoiceQuery): Promise<InvoiceList> {
    // Business logic here
    const clientId = await this.mappings.getWhmcsClientId(userId);

    const invoices = await this.whmcsInvoice.getInvoices(clientId, {
      status: query.status,
      limit: query.limit,
    });

    return {
      items: invoices,
      total: invoices.length,
    };
  }
}
```

### What NOT to Do

```typescript
// ❌ DON'T: Business logic in controller
@Get()
async getInvoices(@Request() req, @Query() query) {
  const clientId = await this.mappings.getClientId(req.user.id);
  const raw = await this.whmcs.api.getInvoices({ clientId });

  // Filtering, transformation, validation - all wrong here
  return raw.invoices
    .filter(inv => query.status ? inv.status === query.status : true)
    .map(inv => transformWhmcsInvoice(inv));
}
```

## Related

- [BFF Integration Patterns](../development/bff/integration-patterns.md)
- [ADR-005: Feature Module Pattern](./005-feature-module-pattern.md)