diff --git a/docs/architecture/website/BLOCKERS.md b/docs/architecture/website/BLOCKERS.md index a6291191a..e685daea7 100644 --- a/docs/architecture/website/BLOCKERS.md +++ b/docs/architecture/website/BLOCKERS.md @@ -2,6 +2,8 @@ This document defines **Blockers** as UX-only prevention mechanisms in the website. +**IMPORTANT**: Blockers are **optional UX helpers**. They are NOT enforced by ESLint rules and do not belong in the strict architecture contract. + Shared contract: [`docs/architecture/shared/BLOCKERS_AND_GUARDS.md`](docs/architecture/shared/BLOCKERS_AND_GUARDS.md:1) ## 1) Definition @@ -10,42 +12,64 @@ A Blocker is a website mechanism that prevents an action from being executed. Blockers exist solely to improve UX and reduce unnecessary requests. -Blockers are not security. +**Blockers are not security.** They are best-effort helpers that can be bypassed. -## 2) Responsibilities +## 2) Purpose -Blockers MAY: +Use Blockers to: +- Prevent multiple form submissions +- Debounce rapid button clicks +- Temporarily disable actions during loading +- Show/hide UI elements based on state -- prevent multiple submissions -- disable actions temporarily -- debounce or throttle interactions -- hide or disable UI elements -- prevent navigation under certain conditions +**Do NOT use Blockers for:** +- Authorization checks +- Security enforcement +- Permanent access control -Blockers MUST: +## 3) Placement -- be reversible -- be local to the website -- be treated as best-effort helpers +Since Blockers are UX-only, they belong in: +- `apps/website/components/**` (component-specific blockers) +- `apps/website/hooks/**` (shared blocker hooks) +- `apps/website/utils/**` (blocker utilities) -## 3) Restrictions +**NOT in `lib/`** - `lib/` is for business logic and architecture contracts. -Blockers MUST NOT: +## 4) Example -- enforce security -- claim authorization -- block access permanently -- replace API Guards -- make assumptions about backend state +```typescript +// ✅ OK: Component-level blocker +export function useSubmitBlocker() { + const [isSubmitting, setIsSubmitting] = useState(false); + + return { + isSubmitting, + block: () => setIsSubmitting(true), + release: () => setIsSubmitting(false), + }; +} -## 4) Common Blockers +// Usage +const blocker = useSubmitBlocker(); -- SubmitBlocker -- ThrottleBlocker -- NavigationBlocker -- FeatureBlocker +async function handleSubmit() { + if (blocker.isSubmitting) return; + + blocker.block(); + await submitForm(); + blocker.release(); +} +``` -## 5) Canonical placement +## 5) Key Principle -- `apps/website/lib/blockers/**` +**Blockers are optional.** The backend must never rely on them. + +If a blocker prevents a submission, the backend should still: +- Validate the request +- Return appropriate errors +- Handle duplicate submissions gracefully + +This is why Blockers don't need ESLint enforcement - they're just UX sugar. diff --git a/docs/architecture/website/COMMAND_MODELS.md b/docs/architecture/website/COMMAND_MODELS.md index 3c01bc53e..44a8d1730 100644 --- a/docs/architecture/website/COMMAND_MODELS.md +++ b/docs/architecture/website/COMMAND_MODELS.md @@ -1,108 +1,64 @@ Command Models -This document defines Command Models as a first-class concept in the frontend architecture. -Command Models are UX-only write models used to collect, validate, and prepare user input -before it is sent to the backend as a Command DTO. +This document defines Command Models as a concept for frontend form handling. -Command Models are not View Models and not Domain Models. +**IMPORTANT**: Command Models are **optional UX helpers**. They are NOT enforced by ESLint rules and do not belong in the strict architecture contract. -⸻ +## 1) Definition -Purpose +Command Models (also called Form Models) are UX-only write models used to: +- Collect user input +- Track form state (dirty, touched, submitting) +- Perform basic UX validation +- Build Command DTOs for submission -A Form Model answers the question: +**Command Models are NOT:** +- Domain models +- View models +- Security boundaries +- Required for the architecture -“What does the UI need in order to safely submit user input?” +## 2) Purpose -Command Models exist to: - • centralize form state - • reduce logic inside components - • provide consistent client-side validation - • build Command DTOs explicitly +Use Command Models when: +- Forms have complex state management +- Multiple fields need validation +- You want to centralize form logic +- Building DTOs is non-trivial -⸻ +**Don't use Command Models when:** +- Forms are simple (use React state directly) +- You're building a quick prototype +- The form logic is trivial -Core Rules +## 3) Core Rules -Command Models: - • exist only in the frontend - • are write-only (never reused for reads) - • are created per form - • are discarded after submission +If you use Command Models: -Command Models MUST NOT: - • contain business logic - • enforce domain rules - • reference View Models - • reference Domain Entities or Value Objects - • be sent to the API directly +**They MUST:** +- Live in `components/` or `hooks/` (not `lib/`) +- Be write-only (never reused for reads) +- Be discarded after submission +- Only perform UX validation -⸻ +**They MUST NOT:** +- Contain business logic +- Enforce domain rules +- Reference View Models or Domain Entities +- Be sent to the API directly (use `toCommand()`) -Relationship to Other Models - -API DTO (read) → ViewModel → UI - -UI Input → FormModel → Command DTO → API - - • View Models are read-only - • Command Models are write-only - • No model is reused across read/write boundaries - -⸻ - -Typical Responsibilities - -A Form Model MAY: - • store field values - • track dirty / touched state - • perform basic UX validation - • expose isValid, canSubmit - • build a Command DTO - -A Form Model MUST NOT: - • decide if an action is allowed - • perform authorization checks - • validate cross-aggregate rules - -⸻ - -Validation Guidelines - -Client-side validation is UX validation, not business validation. - -Allowed validation examples: - • required fields - • min / max length - • email format - • numeric ranges - -Forbidden validation examples: - • “user is not allowed” - • “league already exists” - • “quota exceeded” - -Server validation is the source of truth. - -⸻ - -Example: Simple Form Model (with class-validator) - -import { IsEmail, IsNotEmpty, MinLength } from 'class-validator'; +## 4) Example +```typescript +// In your component file or hooks/ export class SignupFormModel { - @IsEmail() email = ''; - - @IsNotEmpty() - @MinLength(8) password = ''; - isSubmitting = false; - reset(): void { - this.email = ''; - this.password = ''; + isValid(): boolean { + // UX validation only + return this.email.includes('@') && this.password.length >= 8; } toCommand(): SignupCommandDto { @@ -113,44 +69,51 @@ export class SignupFormModel { } } +// Usage +export function SignupForm() { + const [form] = useState(() => new SignupFormModel()); -⸻ + async function handleSubmit() { + if (!form.isValid()) return; + + form.isSubmitting = true; + const result = await signupMutation.mutateAsync(form.toCommand()); + form.isSubmitting = false; + } -Usage in UI Component - -const form = useFormModel(SignupFormModel); - -async function onSubmit() { - if (!form.isValid()) return; - - form.isSubmitting = true; - - await authService.signup(form.toCommand()); + return ( +
+ form.email = e.target.value} /> + {/* ... */} +
+ ); } +``` -The component: - • binds inputs to the Form Model - • reacts to validation state - • never builds DTOs manually +## 5) Key Principle -⸻ +**Command Models are optional.** The backend must validate everything. -Testing +If you don't use Command Models, that's fine! Just: +- Use React state for form data +- Let the backend handle validation +- Return clear errors from mutations -Command Models SHOULD be tested when they contain: - • validation rules - • non-trivial state transitions - • command construction logic +## 6) Comparison -Command Models do NOT need tests if they only hold fields without logic. +| Approach | When to Use | Where | +|----------|-------------|-------| +| **React State** | Simple forms, prototypes | Component | +| **Command Model** | Complex forms, multi-step | Component/Hook | +| **View Model** | Read-only UI state | `lib/view-models/` | +| **Service** | Business orchestration | `lib/services/` | -⸻ +## 7) Summary -Summary - • Command Models are UX helpers for writes - • They protect components from complexity - • They never replace backend validation - • They never leak into read flows +Command Models are **optional UX sugar**. They: +- Help organize complex forms +- Are NOT required by the architecture +- Don't need ESLint enforcement +- Should stay in `components/` or `hooks/` -Command Models help users. -Use Cases protect the system. \ No newline at end of file +Use them if they make your life easier. Skip them if they don't. \ No newline at end of file diff --git a/docs/architecture/website/DEPENDENCY_CONSTRUCTION.md b/docs/architecture/website/DEPENDENCY_CONSTRUCTION.md new file mode 100644 index 000000000..948c34163 --- /dev/null +++ b/docs/architecture/website/DEPENDENCY_CONSTRUCTION.md @@ -0,0 +1,281 @@ +# Dependency Construction Architecture + +## The Decision: Manual Construction (Not DI Container) + +### Why Not Dependency Injection in RSC? + +**Problem**: Next.js RSC pages run on every request, but DI containers are singletons. + +**Risks with DI:** +1. **Data Leakage**: Singleton container could share auth tokens between users +2. **Request Context**: Can't inject request-specific data (user ID, auth token) +3. **Scoping Complexity**: Would need request-scoped containers +4. **Overhead**: DI adds complexity without benefit for RSC + +**Example of the Problem:** +```typescript +// ❌ DON'T DO THIS - DI in RSC +export default async function DashboardPage() { + const container = ContainerManager.getInstance().getContainer(); + const service = container.get(DASHBOARD_SERVICE_TOKEN); + + // Problem: What if DashboardService needs the user's auth token? + // The singleton container doesn't know which user this request is for! +} +``` + +### The Solution: Manual Construction + +**Pattern:** +``` +RSC Page + ↓ +PageQuery (constructs Service) + ↓ +Service (constructs API Client, Logger, ErrorReporter) + ↓ +API Client (makes HTTP calls) +``` + +**Example:** +```typescript +// ✅ CORRECT: Manual construction in RSC +export default async function DashboardPage() { + const query = new DashboardPageQuery(); + const result = await query.execute(); + // ... +} + +// In DashboardPageQuery +export class DashboardPageQuery { + async execute() { + const service = new DashboardService(); // Manual construction + return await service.getDashboardOverview(); + } +} + +// In DashboardService +export class DashboardService { + private apiClient: DashboardApiClient; + + constructor() { + // Service creates its own dependencies + this.apiClient = new DashboardApiClient( + process.env.NEXT_PUBLIC_API_URL || '', + new ConsoleErrorReporter(), + new ConsoleLogger() + ); + } + + async getDashboardOverview() { + return await this.apiClient.getOverview(); + } +} +``` + +### Benefits of Manual Construction + +1. **Explicit**: Dependencies are clear and visible +2. **Simple**: No magic, no container, no configuration +3. **Safe**: No singleton issues, no data leakage +4. **Testable**: Easy to pass mocks in constructor +5. **Flexible**: Can change dependencies without affecting callers + +### What About the Existing `lib/di/`? + +The project has an Inversify DI system, but it's designed for: + +**Client Components:** +```typescript +// ✅ OK in client components +export function UserDashboard() { + const service = useInject(DASHBOARD_SERVICE_TOKEN); + // ... +} +``` + +**Testing:** +```typescript +// ✅ OK in tests +const mockApiClient = new MockDashboardApiClient(); +const service = new DashboardService(mockApiClient); +``` + +**RSC (PageQueries/Mutations):** +```typescript +// ❌ DON'T use DI container in RSC +// ✅ DO use manual construction +``` + +### ESLint Enforcement + +The `services-no-instantiation` rule is **removed** because it was wrong. + +**Correct Rules:** +- ✅ `clean-error-handling`: PageQueries must use Services, not API Clients +- ✅ `services-implement-contract`: Services must return Result types +- ✅ `lib-no-next-imports`: No Next.js imports in lib/ directory + +**What the Rules Allow:** +```typescript +// In PageQuery - ALLOWED +const service = new DashboardService(); + +// In Service - ALLOWED +this.apiClient = new DashboardApiClient(...); +this.logger = new ConsoleLogger(); + +// In PageQuery - FORBIDDEN +const apiClient = new DashboardApiClient(...); // Use Service instead! +``` + +### Complete Example: Read Flow + +```typescript +// apps/website/app/dashboard/page.tsx +export default async function DashboardPage() { + const query = new DashboardPageQuery(); + const result = await query.execute(); + + if (result.isErr()) { + // Handle presentation errors + return ; + } + + return ; +} + +// apps/website/lib/page-queries/page-queries/DashboardPageQuery.ts +export class DashboardPageQuery implements PageQuery { + async execute(): Promise> { + const service = new DashboardService(); + const result = await service.getDashboardOverview(); + + if (result.isErr()) { + // Map domain error to presentation error + return Result.err(mapToPresentationError(result.error)); + } + + const viewData = DashboardViewDataBuilder.build(result.value); + return Result.ok(viewData); + } +} + +// apps/website/lib/services/analytics/DashboardService.ts +export class DashboardService { + private apiClient: DashboardApiClient; + + constructor() { + this.apiClient = new DashboardApiClient( + process.env.NEXT_PUBLIC_API_URL || '', + new ConsoleErrorReporter(), + new ConsoleLogger() + ); + } + + async getDashboardOverview(): Promise> { + try { + const data = await this.apiClient.getOverview(); + return Result.ok(data); + } catch (error) { + // Convert HTTP errors to domain errors + if (error instanceof HttpNotFoundError) { + return Result.err(new NotFoundError('Dashboard not found')); + } + return Result.err(new UnknownError('Failed to fetch dashboard')); + } + } +} + +// apps/website/lib/api/dashboard/DashboardApiClient.ts +export class DashboardApiClient { + constructor( + private baseUrl: string, + private errorReporter: ErrorReporter, + private logger: Logger + ) {} + + async getOverview(): Promise { + const response = await fetch(`${this.baseUrl}/dashboard/overview`); + + if (!response.ok) { + if (response.status === 404) { + throw new HttpNotFoundError('Dashboard not found'); + } + throw new HttpError(`HTTP ${response.status}`); + } + + return response.json(); + } +} +``` + +### Complete Example: Write Flow + +```typescript +// apps/website/app/actions/userActions.ts +'use server'; + +export async function updateUserStatus(input: UpdateUserStatusInput) { + const mutation = new UpdateUserStatusMutation(); + const result = await mutation.execute(input); + + if (result.isErr()) { + return { success: false, error: result.error }; + } + + revalidatePath('/admin/users'); + return { success: true }; +} + +// apps/website/lib/mutations/UpdateUserStatusMutation.ts +export class UpdateUserStatusMutation implements Mutation { + async execute(input: UpdateUserStatusInput): Promise> { + const service = new UserService(); + const result = await service.updateUserStatus(input.userId, input.status); + + if (result.isErr()) { + return Result.err(mapToMutationError(result.error)); + } + + return Result.ok(undefined); + } +} + +// apps/website/lib/services/user/UserService.ts +export class UserService { + private apiClient: UserApiClient; + + constructor() { + this.apiClient = new UserApiClient( + process.env.NEXT_PUBLIC_API_URL || '', + new ConsoleErrorReporter(), + new ConsoleLogger() + ); + } + + async updateUserStatus(userId: string, status: string): Promise> { + try { + await this.apiClient.updateUserStatus(userId, status); + return Result.ok(undefined); + } catch (error) { + if (error instanceof HttpForbiddenError) { + return Result.err(new ForbiddenError('Insufficient permissions')); + } + return Result.err(new UnknownError('Failed to update user')); + } + } +} +``` + +### Summary + +| Aspect | RSC (PageQueries/Mutations) | Client Components | +|--------|----------------------------|-------------------| +| **Construction** | Manual (`new Service()`) | Manual or DI hooks | +| **DI Container** | ❌ Never use | ✅ Can use | +| **Dependencies** | Service creates its own | Injected or manual | +| **Testability** | Pass mocks to constructor | Use DI mocks | +| **Complexity** | Low | Medium | + +**Golden Rule**: In RSC, always use manual construction. It's simpler, safer, and more explicit. diff --git a/docs/architecture/website/PAGE_QUERY_ERROR_HANDLING.md b/docs/architecture/website/PAGE_QUERY_ERROR_HANDLING.md index 24cf83179..b6fd7d564 100644 --- a/docs/architecture/website/PAGE_QUERY_ERROR_HANDLING.md +++ b/docs/architecture/website/PAGE_QUERY_ERROR_HANDLING.md @@ -62,11 +62,15 @@ class HttpServerError extends HttpError {} ### Layer 2: Service (Technical → Domain Errors) -The Service catches HTTP errors and converts them to domain errors: +The Service creates its own dependencies and converts HTTP errors to domain errors. + +**See**: [DEPENDENCY_CONSTRUCTION.md](./DEPENDENCY_CONSTRUCTION.md) for why Services create their own dependencies. ```typescript // apps/website/lib/services/dashboard/DashboardService.ts import { DashboardApiClient } from '@/lib/api/dashboard/DashboardApiClient'; +import { ConsoleErrorReporter } from '@/lib/infrastructure/logging/ConsoleErrorReporter'; +import { ConsoleLogger } from '@/lib/infrastructure/logging/ConsoleLogger'; export type DashboardServiceError = | { type: 'notFound'; message: string } @@ -76,7 +80,17 @@ export type DashboardServiceError = | { type: 'unknown'; message: string }; export class DashboardService { - constructor(private apiClient: DashboardApiClient) {} + private apiClient: DashboardApiClient; + + constructor() { + // Service creates its own dependencies + const baseUrl = process.env.NEXT_PUBLIC_API_URL || ''; + this.apiClient = new DashboardApiClient( + baseUrl, + new ConsoleErrorReporter(), + new ConsoleLogger() + ); + } async getDashboardOverview(): Promise> { try { @@ -105,9 +119,17 @@ export class DashboardService { } ``` +**Key Points:** +- ✅ Service creates its own API Client +- ✅ Service creates its own Logger and ErrorReporter +- ✅ Catches HTTP errors and converts to domain errors +- ✅ Returns Result type + ### Layer 3: PageQuery (Domain → Presentation Errors) -PageQueries use Services and map domain errors to presentation errors: +PageQueries use Services and map domain errors to presentation errors. + +**See**: [DEPENDENCY_CONSTRUCTION.md](./DEPENDENCY_CONSTRUCTION.md) for why we use manual construction. ```typescript // apps/website/lib/page-queries/page-queries/DashboardPageQuery.ts @@ -118,12 +140,8 @@ type DashboardPageError = 'notFound' | 'redirect' | 'DASHBOARD_FETCH_FAILED' | ' export class DashboardPageQuery implements PageQuery { async execute(): Promise> { - // Manual wiring - const errorReporter = new ConsoleErrorReporter(); - const logger = new ConsoleLogger(); - const baseUrl = process.env.NEXT_PUBLIC_API_URL || ''; - const apiClient = new DashboardApiClient(baseUrl, errorReporter, logger); - const dashboardService = new DashboardService(apiClient); + // Manual construction: Service creates its own dependencies + const dashboardService = new DashboardService(); // Call service const serviceResult = await dashboardService.getDashboardOverview(); @@ -154,6 +172,13 @@ export class DashboardPageQuery implements PageQuery { } ``` +**Key Points:** +- ✅ PageQuery constructs only the Service +- ✅ Service handles its own dependencies (API Client, Logger, etc.) +- ❌ No API Client instantiation in PageQuery +- ✅ Map domain errors to presentation errors +- ✅ Transform API DTO to ViewData using Builder + ### Layer 4: RSC Page (Presentation → User) The RSC page handles presentation errors: @@ -282,6 +307,10 @@ export class UserApiClient { ```typescript // apps/website/lib/services/user/UserService.ts +import { UserApiClient } from '@/lib/api/user/UserApiClient'; +import { ConsoleErrorReporter } from '@/lib/infrastructure/logging/ConsoleErrorReporter'; +import { ConsoleLogger } from '@/lib/infrastructure/logging/ConsoleLogger'; + export type UserServiceError = | { type: 'notFound'; message: string } | { type: 'forbidden'; message: string } @@ -289,7 +318,17 @@ export type UserServiceError = | { type: 'serverError'; message: string }; export class UserService { - constructor(private apiClient: UserApiClient) {} + private apiClient: UserApiClient; + + constructor() { + // Service creates its own dependencies + const baseUrl = process.env.NEXT_PUBLIC_API_URL || ''; + this.apiClient = new UserApiClient( + baseUrl, + new ConsoleErrorReporter(), + new ConsoleLogger() + ); + } async updateUserStatus(userId: string, status: string): Promise> { try { @@ -314,6 +353,12 @@ export class UserService { } ``` +**Key Points:** +- ✅ Service creates its own API Client +- ✅ Service creates its own Logger and ErrorReporter +- ✅ Catches HTTP errors and converts to domain errors +- ✅ Returns Result type + ### Layer 3: Mutation (Domain → Presentation Errors) ```typescript @@ -358,18 +403,11 @@ export class UpdateUserStatusMutation implements Mutation { - return this.apiClient.updateUserStatus(userId, status); + constructor() { + // Service creates its own dependencies + const baseUrl = process.env.NEXT_PUBLIC_API_URL || ''; + this.apiClient = new AdminApiClient( + baseUrl, + new ConsoleErrorReporter(), + new ConsoleLogger() + ); + } + + async updateUserStatus(userId: string, status: string): Promise> { + try { + const result = await this.apiClient.updateUserStatus(userId, status); + return Result.ok(result); + } catch (error) { + // Convert HTTP errors to domain errors + if (error instanceof HttpForbiddenError) { + return Result.err(new ForbiddenError('Insufficient permissions')); + } + return Result.err(new UnknownError('Failed to update user')); + } } } ``` @@ -54,101 +77,113 @@ export class AdminService { ```typescript // apps/website/lib/page-queries/AdminDashboardPageQuery.ts -import { ConsoleLogger } from '@/lib/infrastructure/logging/ConsoleLogger'; -import { EnhancedErrorReporter } from '@/lib/infrastructure/EnhancedErrorReporter'; -import { AdminApiClient } from '@/lib/api/admin/AdminApiClient'; import { AdminService } from '@/lib/services/admin/AdminService'; +import { AdminDashboardViewDataBuilder } from '@/lib/builders/view-data/AdminDashboardViewDataBuilder'; -export class AdminDashboardPageQuery { - async execute(): Promise> { - // Create infrastructure - const logger = new ConsoleLogger(); - const errorReporter = new EnhancedErrorReporter(logger, {...}); - const baseUrl = process.env.NEXT_PUBLIC_API_BASE_URL || 'http://localhost:3001'; - const apiClient = new AdminApiClient(baseUrl, errorReporter, logger); - const service = new AdminService(apiClient); +export class AdminDashboardPageQuery implements PageQuery { + async execute(): Promise> { + // Manual construction: Service creates its own dependencies + const service = new AdminService(); // Use service - const stats = await service.getDashboardStats(); + const result = await service.getDashboardStats(); - // Transform to Page DTO - return { status: 'ok', dto: transformToPageDto(stats) }; + if (result.isErr()) { + return Result.err(mapToPresentationError(result.error)); + } + + // Transform to ViewData using Builder + const viewData = AdminDashboardViewDataBuilder.build(result.value); + return Result.ok(viewData); } } ``` -### Usage in Server Actions (Writes) +### Usage in Mutations (Writes) ```typescript -// apps/website/app/admin/actions.ts -'use server'; - -import { ConsoleLogger } from '@/lib/infrastructure/logging/ConsoleLogger'; -import { EnhancedErrorReporter } from '@/lib/infrastructure/EnhancedErrorReporter'; -import { AdminApiClient } from '@/lib/api/admin/AdminApiClient'; +// apps/website/lib/mutations/UpdateUserStatusMutation.ts import { AdminService } from '@/lib/services/admin/AdminService'; -import { revalidatePath } from 'next/cache'; -export async function updateUserStatus(userId: string, status: string): Promise { - try { - // Create infrastructure - const logger = new ConsoleLogger(); - const errorReporter = new EnhancedErrorReporter(logger, { - showUserNotifications: true, - logToConsole: true, - reportToExternal: process.env.NODE_ENV === 'production', - }); +export class UpdateUserStatusMutation implements Mutation { + async execute(input: UpdateUserStatusInput): Promise> { + // Manual construction: Service creates its own dependencies + const service = new AdminService(); - const baseUrl = process.env.NEXT_PUBLIC_API_BASE_URL || 'http://localhost:3001'; - const apiClient = new AdminApiClient(baseUrl, errorReporter, logger); - const service = new AdminService(apiClient); + const result = await service.updateUserStatus(input.userId, input.status); - // Use service (NOT API client directly) - await service.updateUserStatus(userId, status); + if (result.isErr()) { + return Result.err(mapToMutationError(result.error)); + } - // Revalidate - revalidatePath('/admin/users'); - } catch (error) { - console.error('updateUserStatus failed:', error); - throw new Error('Failed to update user status'); + return Result.ok(undefined); } } ``` -## Infrastructure Concerns +### Usage in Server Actions -**Where should logging/error reporting live?** +```typescript +// app/admin/actions.ts +'use server'; -In the current architecture, **server actions and PageQueries create infrastructure**. This is acceptable because: -1. Next.js serverless functions are stateless -2. Each request needs fresh infrastructure -3. Manual DI is clearer than magic containers +import { UpdateUserStatusMutation } from '@/lib/mutations/UpdateUserStatusMutation'; +import { revalidatePath } from 'next/cache'; -**Key principle**: Services orchestrate, they don't create infrastructure. +export async function updateUserStatus(input: UpdateUserStatusInput) { + // Manual construction: Mutation creates Service + const mutation = new UpdateUserStatusMutation(); + + const result = await mutation.execute(input); + + if (result.isErr()) { + return { success: false, error: result.error }; + } + + revalidatePath('/admin/users'); + return { success: true }; +} +``` ## Dependency Chain ``` -Server Action / PageQuery - ↓ (creates infrastructure) +RSC Page / Server Action + ↓ (manual construction) +PageQuery / Mutation + ↓ (manual construction) Service - ↓ (orchestrates) + ↓ (creates own dependencies) API Client ↓ (makes HTTP calls) API ``` +**Key Principle**: Each layer manually constructs the next layer. Services create their own infrastructure (API Client, Logger, ErrorReporter). + +## Why Manual Construction? + +**See**: [DEPENDENCY_CONSTRUCTION.md](./DEPENDENCY_CONSTRUCTION.md) + +**Summary**: +- ✅ Explicit and clear +- ✅ No singleton issues +- ✅ No request-scoping problems +- ✅ Easy to test (pass mocks to constructor) +- ✅ Works perfectly with Next.js RSC +- ❌ No DI container needed + ## Naming - Service classes: `*Service` -- Service methods: Return DTOs (not ViewModels) -- Variable names: `apiDto`, `pageDto` (never just `dto`) +- Service methods: Return `Result` +- Variable names: `apiDto`, `viewData` (never just `dto`) ## Comparison with Other Layers | Layer | Purpose | Example | |-------|---------|---------| -| **Website Service** | Orchestrate API calls | `AdminService` | +| **Website Service** | Orchestrate API calls, handle errors | `AdminService` | | **API Client** | HTTP infrastructure | `AdminApiClient` | | **Core Use Case** | Business rules | `CreateLeagueUseCase` | | **Domain Service** | Cross-entity logic | `StrengthOfFieldCalculator` | @@ -170,8 +205,13 @@ class AdminService { ```typescript // CORRECT class AdminService { - async getUser(userId: string): Promise { - return this.apiClient.getUser(userId); // ✅ DTOs are fine + async getUser(userId: string): Promise> { + try { + const dto = await this.apiClient.getUser(userId); + return Result.ok(dto); // ✅ DTOs are fine + } catch (error) { + return Result.err(new NotFoundError('User not found')); + } } } ``` @@ -191,8 +231,13 @@ class AdminService { ```typescript // CORRECT class AdminService { - async getUser(userId: string): Promise { - return this.apiClient.getUser(userId); + async getUser(userId: string): Promise> { + try { + const dto = await this.apiClient.getUser(userId); + return Result.ok(dto); + } catch (error) { + return Result.err(new NotFoundError('User not found')); + } } } // Business logic in core use case or page query @@ -208,24 +253,48 @@ export async function updateUserStatus(userId: string, status: string) { } ``` -✅ **Correct**: Server action uses service +✅ **Correct**: Server action uses Mutation ```typescript // CORRECT 'use server'; -export async function updateUserStatus(userId: string, status: string) { - const apiClient = new AdminApiClient(...); - const service = new AdminService(apiClient); - await service.updateUserStatus(userId, status); // ✅ Uses service +export async function updateUserStatus(input: UpdateUserStatusInput) { + const mutation = new UpdateUserStatusMutation(); + const result = await mutation.execute(input); + // ... +} +``` + +❌ **Wrong**: PageQuery creates API Client +```typescript +// WRONG +export class DashboardPageQuery { + async execute() { + const apiClient = new DashboardApiClient(...); // ❌ Should use Service + return await apiClient.getOverview(); + } +} +``` + +✅ **Correct**: PageQuery uses Service +```typescript +// CORRECT +export class DashboardPageQuery { + async execute() { + const service = new DashboardService(); // ✅ Service creates API Client + return await service.getDashboardOverview(); + } } ``` ## Summary -Website services are **thin orchestration wrappers** around API clients. They handle infrastructure concerns so that PageQueries and Server Actions can focus on composition and validation. +Website services are **thin orchestration wrappers** that create their own dependencies and handle error conversion. **Key principles**: -1. Services orchestrate API calls -2. Server actions/PageQueries create infrastructure -3. Services don't create ViewModels -4. Services don't contain business rules -5. **Server actions MUST use services, not API clients directly** \ No newline at end of file +1. ✅ Services create their own dependencies (API Client, Logger, ErrorReporter) +2. ✅ Services return `Result` +3. ✅ Services convert HTTP errors to Domain errors +4. ❌ Services don't create ViewModels +5. ❌ Services don't contain business rules +6. ✅ PageQueries/Mutations use Services, not API Clients directly +7. ✅ Manual construction (no DI container in RSC) \ No newline at end of file diff --git a/docs/architecture/website/WEBSITE_CONTRACT.md b/docs/architecture/website/WEBSITE_CONTRACT.md index b8ea18f7a..12b9825e6 100644 --- a/docs/architecture/website/WEBSITE_CONTRACT.md +++ b/docs/architecture/website/WEBSITE_CONTRACT.md @@ -148,10 +148,11 @@ Purpose: eliminate exceptions and provide explicit error paths. Rules: -- All PageQueries return `Result` -- All Mutations return `Result` -- Use `ResultFactory.ok(value)` for success -- Use `ResultFactory.error(message)` for errors +- All PageQueries return `Result` +- All Mutations return `Result` +- All Services return `Result` +- Use `Result.ok(value)` for success +- Use `Result.err(error)` for errors - Never throw exceptions See [`Result.ts`](apps/website/lib/contracts/Result.ts:1). @@ -183,13 +184,19 @@ Canonical placement in this repo: ```text RSC page.tsx ↓ -PageQuery.execute() +PageQuery (manual construction) ↓ -API client (infra) +Service (creates own API Client, Logger, ErrorReporter) + ↓ +API Client (makes HTTP calls) ↓ API Transport DTO ↓ -Result +Result + ↓ +PageQuery (maps DomainError → PresentationError) + ↓ +Result ↓ ViewData Builder (lib/builders/view-data/) ↓ @@ -198,6 +205,13 @@ ViewData Template ``` +**Key Points:** +- PageQuery constructs Service +- Service creates its own dependencies +- Service returns Result +- PageQuery maps errors to presentation layer +- Builder transforms API DTO to ViewData + ### Client Components ```text Client Component @@ -241,40 +255,55 @@ Allowed: import { AdminService } from '@/lib/services/admin/AdminService'; export async function updateUserStatus(userId: string, status: string) { - const service = new AdminService(...); + const service = new AdminService(); await service.updateUserStatus(userId, status); // ❌ Should use mutation } // ✅ CORRECT - Mutation usage 'use server'; -import { AdminUserMutation } from '@/lib/mutations/admin/AdminUserMutation'; +import { UpdateUserStatusMutation } from '@/lib/mutations/UpdateUserStatusMutation'; import { revalidatePath } from 'next/cache'; -export async function updateUserStatus(userId: string, status: string) { - const mutation = new AdminUserMutation(); - const result = await mutation.updateUserStatus(userId, status); +export async function updateUserStatus(input: UpdateUserStatusInput) { + const mutation = new UpdateUserStatusMutation(); + const result = await mutation.execute(input); if (result.isErr()) { - console.error('updateUserStatus failed:', result.getError()); - throw new Error('Failed to update user status'); + console.error('updateUserStatus failed:', result.error); + return { success: false, error: result.error }; } revalidatePath('/admin/users'); + return { success: true }; } ``` **Pattern**: -1. Server Action (thin wrapper) - handles framework concerns (revalidation) -2. Mutation (framework-agnostic) - creates infrastructure, calls service -3. Service (business logic) - orchestrates API calls -4. API Client (infrastructure) - makes HTTP requests -5. Result - type-safe error handling +1. **Server Action** (thin wrapper) - handles framework concerns (revalidation, returns to client) +2. **Mutation** (framework-agnostic) - constructs Service, calls service methods +3. **Service** - constructs own dependencies (API Client, Logger), returns Result +4. **API Client** (infrastructure) - makes HTTP requests, throws HTTP errors +5. **Result** - type-safe error handling at every layer + +**Dependency Flow**: +``` +Server Action + ↓ (constructs) +Mutation + ↓ (constructs) +Service (creates API Client, Logger, ErrorReporter) + ↓ (calls) +API Client + ↓ (HTTP) +Backend API +``` **Rationale**: - Mutations are framework-agnostic (can be tested without Next.js) - Consistent pattern with PageQueries - Type-safe error handling with Result - Makes infrastructure explicit and testable +- Manual construction (no DI container issues) See [`MUTATIONS.md`](docs/architecture/website/MUTATIONS.md:1) and [`SERVICES.md`](docs/architecture/website/SERVICES.md:1).