website refactor

2026-01-13 23:33:07 +01:00
parent 87572f5b1f
commit 8d7c709e0c
4 changed files with 318 additions and 25 deletions
--- a/apps/website/.eslintrc.json
+++ b/apps/website/.eslintrc.json
@@ -73,6 +73,7 @@
      ],
      "rules": {
        "gridpilot-rules/mutation-contract": "error",
+        "gridpilot-rules/mutation-must-use-builders": "error",
        "gridpilot-rules/filename-service-match": "error"
      }
    },
@@ -117,7 +118,20 @@
        "gridpilot-rules/rsc-no-di": "error",
        "gridpilot-rules/rsc-no-local-helpers": "error",
        "gridpilot-rules/rsc-no-object-construction": "error",
-        "gridpilot-rules/rsc-no-container-manager-calls": "error"
+        "gridpilot-rules/rsc-no-container-manager-calls": "error",
+        "gridpilot-rules/no-hardcoded-search-params": "error",
+        "gridpilot-rules/no-next-cookies-in-pages": "error"
+      }
+    },
+    {
+      "files": [
+        "lib/services/**/*.ts",
+        "lib/page-queries/**/*.ts",
+        "lib/mutations/**/*.ts",
+        "middleware.ts"
+      ],
+      "rules": {
+        "gridpilot-rules/no-direct-process-env": "error"
      }
    },
    {
@@ -150,17 +164,6 @@
        "gridpilot-rules/no-hardcoded-search-params": "error"
      }
    },
-    {
-      "files": [
-        "lib/mutations/**/*.ts"
-      ],
-      "rules": {
-        "gridpilot-rules/mutation-contract": "error",
-        "gridpilot-rules/clean-error-handling": "error",
-        "gridpilot-rules/single-export-per-file": "error",
-        "gridpilot-rules/filename-matches-export": "error"
-      }
-    },
    {
      "files": [
        "templates/**/*.ts",
@@ -190,7 +193,9 @@
      "rules": {
        "gridpilot-rules/client-only-no-server-code": "error",
        "gridpilot-rules/client-only-must-have-directive": "error",
-        "gridpilot-rules/server-actions-must-use-mutations": "error"
+        "gridpilot-rules/server-actions-must-use-mutations": "error",
+        "gridpilot-rules/server-actions-return-result": "error",
+        "gridpilot-rules/server-actions-interface": "error"
      }
    },
    {
@@ -220,6 +225,18 @@
        "gridpilot-rules/lib-no-next-imports": "error"
      }
    },
+    {
+      "files": [
+        "app/onboarding/**/*.ts",
+        "app/onboarding/**/*.tsx",
+        "lib/auth/RouteGuard.ts",
+        "lib/auth/AuthFlowRouter.ts",
+        "middleware.ts"
+      ],
+      "rules": {
+        "gridpilot-rules/no-console": "error"
+      }
+    },
    {
      "files": [
        "app/**/*.tsx",
@@ -237,7 +254,9 @@
        "app/**/actions/*.ts"
      ],
      "rules": {
-        "gridpilot-rules/no-hardcoded-routes": "error"
+        "gridpilot-rules/no-hardcoded-routes": "error",
+        "gridpilot-rules/server-actions-return-result": "error",
+        "gridpilot-rules/server-actions-interface": "error"
      }
    },
    {
@@ -268,7 +287,6 @@
      ],
      "rules": {
        "gridpilot-rules/service-function-format": "error",
-        "gridpilot-rules/services-must-be-marked": "error",
        "gridpilot-rules/services-must-be-pure": "error",
        "gridpilot-rules/services-no-external-api": "error",
        "gridpilot-rules/services-implement-contract": "error",
--- a/apps/website/lib/contracts/services/Service.ts
+++ b/apps/website/lib/contracts/services/Service.ts
@@ -18,7 +18,6 @@
 * - They return Result<ApiDto, DomainError>
 */

-import { Result } from '@/lib/contracts/Result';

 /**
 * Domain error type for services
@@ -35,16 +34,19 @@ export type DomainError =

 /**
 * Service interface for orchestration operations
- * All service methods must return Result with domain errors
+ *
+ * Design Decision: Services with multiple methods CANNOT use a single generic type
+ * because each method may return different DTOs. Instead:
+ *
+ * 1. Single-method services (PageQueries, Mutations): Use Service<TApiDto, TError>
+ * 2. Multi-method services: Don't implement this interface, just follow the pattern
+ *
+ * All service methods must return Promise<Result<T, DomainError>> for type-safe error handling.
 *
 * Type Parameters:
 * - TApiDto: The API Transport DTO type returned on success
 * - TError: The domain error type (defaults to DomainError)
 */
-export interface Service<TApiDto = unknown, TError extends DomainError = DomainError> {
-  /**
-   * Execute a service operation
-   * Returns Result with API DTO or Domain Error
-   */
-  execute(...args: unknown[]): Promise<Result<TApiDto, TError>>;
+export interface Service {
+  // No specific methods - just a marker type
 }
--- a/apps/website/middleware.ts
+++ b/apps/website/middleware.ts
@@ -4,6 +4,7 @@ import { SessionGateway } from '@/lib/gateways/SessionGateway';
 import { handleAuthFlow } from '@/lib/auth/AuthFlowRouter';
 import { ConsoleLogger } from '@/lib/infrastructure/logging/ConsoleLogger';
 import { routeMatchers } from '@/lib/routing/RouteConfig';
+import { SearchParamBuilder } from '@/lib/routing/search-params/SearchParamBuilder';

 const logger = new ConsoleLogger();

@@ -78,7 +79,7 @@ export async function middleware(request: NextRequest) {
  } catch (error) {
    logger.error('[MIDDLEWARE] Error in auth flow', error instanceof Error ? error : new Error(String(error)));
    // Fallback: redirect to login if there's an error
-    return NextResponse.redirect(new URL(`/auth/login?returnTo=${encodeURIComponent(pathname)}`, request.url));
+    return NextResponse.redirect(new URL(`/auth/login${SearchParamBuilder.auth(pathname)}`, request.url));
  }
  
  logger.info('[MIDDLEWARE] Decision summary', {
@@ -121,4 +122,4 @@ export const config = {
     */
    '/((?!_next/static|_next/image|_next/data|favicon.ico|.*\\.(?:svg|png|jpg|jpeg|gif|webp|mp4|webm|mov|avi)$).*)',
  ],
-};
+};
--- a/docs/architecture/website/SESSION.md
+++ b/docs/architecture/website/SESSION.md
@@ -0,0 +1,272 @@
+# Next.js RSC Session Best Practices
+
+This document defines the authoritative pattern for handling session/authentication in Next.js Server Components and Server Actions within the `apps/website` layer.
+
+## Core Principle
+
+**Server Actions should NOT fetch session separately.** The API handles authentication automatically via cookies.
+
+## The Problem with Manual Session Fetching
+
+### ❌ Anti-Pattern (Current Implementation)
+```typescript
+// apps/website/app/onboarding/actions.ts
+'use server';
+
+import { SessionGateway } from '@/lib/gateways/SessionGateway';
+
+async function getCurrentUserId(): Promise<string | null> {
+  const gateway = new SessionGateway();
+  const session = await gateway.getSession();  // ❌ Extra API call
+  return session?.user?.userId || null;
+}
+
+export async function completeOnboardingAction(input: CompleteOnboardingInputDTO) {
+  const userId = await getCurrentUserId();  // ❌ Performance overhead
+  if (!userId) {
+    return Result.err('Not authenticated');
+  }
+  
+  const mutation = new CompleteOnboardingMutation();
+  // ... rest of logic
+}
+```
+
+**Problems:**
+1. **Performance**: Makes extra API call on every action invocation
+2. **Redundancy**: Manual auth check when API handles it automatically
+3. **Coupling**: Actions depend on session infrastructure
+4. **Inconsistency**: Doesn't match pattern used elsewhere in codebase
+
+## The Correct Pattern
+
+### ✅ Server Actions (Thin Wrappers)
+```typescript
+// apps/website/app/onboarding/actions.ts
+'use server';
+
+import { Result } from '@/lib/contracts/Result';
+import { CompleteOnboardingMutation } from '@/lib/mutations/onboarding/CompleteOnboardingMutation';
+import { GenerateAvatarsMutation } from '@/lib/mutations/onboarding/GenerateAvatarsMutation';
+import { CompleteOnboardingInputDTO } from '@/lib/types/generated/CompleteOnboardingInputDTO';
+import { revalidatePath } from 'next/cache';
+
+/**
+ * Complete onboarding - thin wrapper around mutation
+ * 
+ * Pattern: Server Action → Mutation → Service → API Client
+ * 
+ * Authentication is handled automatically by the API via cookies.
+ * The BaseApiClient includes credentials: 'include', so cookies are sent automatically.
+ * If authentication fails, the API returns 401/403 which gets converted to domain errors.
+ */
+export async function completeOnboardingAction(
+  input: CompleteOnboardingInputDTO
+): Promise<Result<{ success: boolean }, string>> {
+  const mutation = new CompleteOnboardingMutation();
+  const result = await mutation.execute(input);
+  
+  if (result.isErr()) {
+    return Result.err(result.getError());
+  }
+  
+  revalidatePath('/dashboard');
+  return Result.ok({ success: true });
+}
+
+/**
+ * Generate avatars - thin wrapper around mutation
+ * 
+ * Note: This action requires userId to be passed from the client.
+ * The client should get userId from session and pass it as a parameter.
+ */
+export async function generateAvatarsAction(params: {
+  userId: string;
+  facePhotoData: string;
+  suitColor: string;
+}): Promise<Result<{ success: boolean; avatarUrls?: string[] }, string>> {
+  const mutation = new GenerateAvatarsMutation();
+  const result = await mutation.execute(params);
+  
+  if (result.isErr()) {
+    return Result.err(result.getError());
+  }
+  
+  const data = result.unwrap();
+  return Result.ok({ success: data.success, avatarUrls: data.avatarUrls });
+}
+```
+
+### ✅ Client Component Pattern
+```typescript
+// apps/website/app/onboarding/OnboardingWizardClient.tsx
+'use client';
+
+import { completeOnboardingAction, generateAvatarsAction } from '@/app/onboarding/actions';
+import { useAuth } from '@/lib/hooks/auth/useAuth';
+import { useState } from 'react';
+
+export function OnboardingWizardClient() {
+  const { session } = useAuth(); // Get userId from session (client-side)
+  const [isSubmitting, setIsSubmitting] = useState(false);
+
+  const handleCompleteOnboarding = async (input: CompleteOnboardingInputDTO) => {
+    setIsSubmitting(true);
+    try {
+      const result = await completeOnboardingAction(input);
+      
+      if (result.isErr()) {
+        // Handle error (show toast, etc.)
+        console.error('Onboarding failed:', result.getError());
+        return;
+      }
+      
+      // Success - redirect or show success message
+      console.log('Onboarding completed successfully');
+    } finally {
+      setIsSubmitting(false);
+    }
+  };
+
+  const handleGenerateAvatars = async (facePhotoData: string, suitColor: string) => {
+    if (!session?.user?.userId) {
+      console.error('User not authenticated');
+      return Result.err('Not authenticated');
+    }
+
+    setIsSubmitting(true);
+    try {
+      const result = await generateAvatarsAction({
+        userId: session.user.userId,  // Pass userId from session
+        facePhotoData,
+        suitColor,
+      });
+
+      if (result.isErr()) {
+        console.error('Avatar generation failed:', result.getError());
+        return;
+      }
+
+      const data = result.unwrap();
+      console.log('Avatars generated:', data.avatarUrls);
+    } finally {
+      setIsSubmitting(false);
+    }
+  };
+
+  return (
+    <div>
+      {/* Wizard implementation */}
+      <button onClick={() => handleCompleteOnboarding({ firstName: 'John', lastName: 'Doe', displayName: 'JohnD', country: 'US' })}>
+        Complete Onboarding
+      </button>
+    </div>
+  );
+}
+```
+
+## Why This Pattern Works
+
+### 1. Automatic Authentication
+```typescript
+// apps/website/lib/api/base/BaseApiClient.ts
+protected async request<T>(method: string, path: string, data?: object, options = {}): Promise<T> {
+  const config: RequestInit = {
+    method,
+    headers,
+    credentials: 'include',  // ✅ Automatically sends cookies
+    signal,
+  };
+  // ...
+}
+```
+
+The `BaseApiClient` automatically includes `credentials: 'include'`, so:
+- Cookies are sent with every API request
+- The backend API can authenticate the user
+- No manual session fetching needed in server actions
+
+### 2. Error Handling Flow
+```
+User not authenticated
+  ↓
+API returns 401/403
+  ↓
+BaseApiClient throws HttpUnauthorizedError
+  ↓
+Service catches and converts to DomainError
+  ↓
+Mutation returns Result.err()
+  ↓
+Server Action returns error to client
+  ↓
+Client shows appropriate message
+```
+
+### 3. Performance Benefits
+- **No extra API calls**: Session fetched once on client, reused
+- **Reduced latency**: Server actions don't wait for session lookup
+- **Better scalability**: Fewer API calls per user action
+
+### 4. Consistency with Architecture
+This pattern matches your existing codebase:
+```typescript
+// apps/website/app/admin/actions.ts - ✅ Already follows this pattern
+export async function updateUserStatus(userId: string, status: string) {
+  const mutation = new UpdateUserStatusMutation();
+  const result = await mutation.execute({ userId, status });
+  // ... no manual session fetching
+}
+```
+
+## When Manual Session Fetching IS Needed
+
+There are rare cases where you might need to fetch session in a server action:
+
+### Case 1: User ID Required in Request Body
+If the API endpoint requires userId in the request body (not just cookies):
+
+```typescript
+// Client passes userId
+const result = await generateAvatarsAction({
+  userId: session.user.userId,  // ✅ Pass from client
+  facePhotoData,
+  suitColor,
+});
+```
+
+### Case 2: Server-Side Authorization Check
+If you need to check permissions before calling a mutation:
+
+```typescript
+export async function adminAction(input: AdminInput) {
+  // Only fetch session if you need to check permissions server-side
+  const gateway = new SessionGateway();
+  const session = await gateway.getSession();
+  
+  if (!session?.user?.roles?.includes('admin')) {
+    return Result.err('Forbidden');
+  }
+  
+  const mutation = new AdminMutation();
+  return await mutation.execute(input);
+}
+```
+
+**Note**: This is rare. Usually, the API handles authorization and returns 403 if unauthorized.
+
+## Summary
+
+| Aspect | Manual Session Fetching | Automatic (Recommended) |
+|--------|------------------------|------------------------|
+| **Performance** | ❌ Extra API call | ✅ No overhead |
+| **Complexity** | ❌ More code | ✅ Simpler |
+| **Coupling** | ❌ Depends on session infra | ✅ Decoupled |
+| **Consistency** | ❌ Inconsistent | ✅ Matches codebase |
+| **Security** | ⚠️ Manual checks | ✅ API enforces |
+
+## Golden Rule
+
+**Server Actions are thin wrappers. Mutations handle logic. API enforces security.**
+
+Never fetch session in server actions unless you have a specific server-side authorization requirement that cannot be handled by the API.