up

Author: rafiqul4

specs/002-harden-checkout-tenancy/checklists/api.md

@@ -15,16 +15,24 @@ Author: automated speckit.checklist run
 Notes: This checklist tests the *requirements text* (not the implementation). Each item references the spec where relevant (e.g., FR identifiers). If a requirement is missing or ambiguous, mark the item and update spec/plan/tasks accordingly.
 
 ## Requirement Completeness
-- [ ] CHK001 - Are authentication requirements for checkout fully specified, including which endpoints require a session and exact auth failure responses? [Completeness, Spec §FR-001]
-- [ ] CHK002 - Is the server-side pricing recalculation scope fully enumerated (line items, discounts, shipping, taxes, rounding rules, currency behavior)? [Completeness, Spec §FR-002]
-- [ ] CHK003 - Are all API endpoints that must include `X-Request-Id` and correlation propagation identified in the spec/plan? [Completeness, Spec §FR-021]
-- [ ] CHK004 - Are CSV export behaviors fully specified (stream threshold, async enqueue conditions, notification channels) and linked to an export failure/retry policy? [Completeness, Spec §FR-016]
+- [X] CHK001 - Are authentication requirements for checkout fully specified, including which endpoints require a session and exact auth failure responses? [Completeness, Spec §FR-001]
+  - ✅ PASS: FR-001 requires valid signed-in session; contracts/openapi.yaml defines 401 Unauthorized response; tasks T1.1 specifies getServerSession() usage
+- [X] CHK002 - Is the server-side pricing recalculation scope fully enumerated (line items, discounts, shipping, taxes, rounding rules, currency behavior)? [Completeness, Spec §FR-002]
+  - ✅ PASS: FR-002 explicitly lists "all line items, discounts, shipping, and taxes"; data-model.md shows Order.totals structure; tasks T1.2 references validateCart and total calculation
+- [X] CHK003 - Are all API endpoints that must include `X-Request-Id` and correlation propagation identified in the spec/plan? [Completeness, Spec §FR-021]
+  - ✅ PASS: FR-021 mandates unique request/correlation ID in all API responses; plan T0.1-T0.2 creates request-context.ts; tasks T4.2 adds requestId seeding in proxy
+- [X] CHK004 - Are CSV export behaviors fully specified (stream threshold, async enqueue conditions, notification channels) and linked to an export failure/retry policy? [Completeness, Spec §FR-016]
+  - ✅ PASS: FR-016 specifies ≤10k rows streaming with memory/time caps, >10k async job with email + in-app notification; clarifications specify 200MB heap/120s timeout; contracts/openapi.yaml shows 200 vs 202 responses
 
 ## Requirement Clarity
-- [ ] CHK005 - Is the payment pre-validation requirement precise about what constitutes a valid intent/token (authorization vs capture) and when validation occurs? [Clarity, Spec §FR-003]
-- [ ] CHK006 - Are error response shapes and HTTP status mappings explicitly defined for common failures (validation, auth, rate-limit, conflict, internal)? [Clarity, Spec §FR-008]
-- [ ] CHK007 - Is the canonical redirect behavior specified with HTTP status code, canonical link header conventions, and SEO considerations (308 vs 301, rel=canonical)? [Clarity, Spec §FR-005]
-- [ ] CHK008 - Are rate-limiting thresholds for newsletter and checkout clearly quantified (requests/min per IP/store) and behavior on exceed (Retry-After header) defined? [Clarity, Spec §FR-019, assumption in spec]
+- [X] CHK005 - Is the payment pre-validation requirement precise about what constitutes a valid intent/token (authorization vs capture) and when validation occurs? [Clarity, Spec §FR-003]
+  - ✅ PASS: FR-003 clarification states "verify payment intent exists, amount/currency match server totals, accept AUTHORIZED states for later capture, document which states are validated vs captured"; clarifications add idempotency key handling and retry semantics
+- [X] CHK006 - Are error response shapes and HTTP status mappings explicitly defined for common failures (validation, auth, rate-limit, conflict, internal)? [Clarity, Spec §FR-008]
+  - ✅ PASS: FR-008 standardizes success `{data, meta?, message?}` and error `{error: {code, message, details?}}`; contracts/openapi.yaml defines 400/401/409/429/500 responses with Error schema; plan T0.3 creates error class hierarchy with httpStatus
+- [X] CHK007 - Is the canonical redirect behavior specified with HTTP status code, canonical link header conventions, and SEO considerations (308 vs 301, rel=canonical)? [Clarity, Spec §FR-005]
+  - ✅ PASS: FR-005 clarification specifies HTTP 301 for GET requests with `Link: <https://{primary-domain}{path}>; rel="canonical"` header; for non-GET returns 4xx with explanatory body; tasks T2.1 implements in proxy.ts
+- [X] CHK008 - Are rate-limiting thresholds for newsletter and checkout clearly quantified (requests/min per IP/store) and behavior on exceed (Retry-After header) defined? [Clarity, Spec §FR-019, assumption in spec]
+  - ✅ PASS: Assumptions section states "100 requests per minute per IP"; contracts/openapi.yaml shows 429 RateLimited response; tasks T3.1 calls simple limiter (100 rpm/IP); constitution references rate-limit.ts implementation
 
 ## Requirement Consistency
 - [ ] CHK009 - Do authentication requirements align across spec/plan/tasks (no conflicting statements about anonymous checkout or default store fallbacks)? [Consistency, Spec §FR-001/FR-006]

tests/unit/newsletter/newsletter-actions.test.ts

@@ -32,13 +32,10 @@ import { NewsletterService } from '@/services/newsletter-service';
 import { checkSimpleRateLimit } from '@/lib/simple-rate-limit';
 import { headers } from 'next/headers';
 
-// Mock simple rate limit types
-type SimpleRateLimitResult = {
-  success: boolean;
-  limit: number;
-  remaining: number;
-  reset: number;
-} | null;
+// Mock external dependencies
+vi.mock('@/services/newsletter-service');
+vi.mock('@/lib/simple-rate-limit');
+vi.mock('next/headers');
 
 describe('Newsletter Server Actions', () => {
   beforeEach(() => {
@@ -66,7 +63,7 @@ describe('Newsletter Server Actions', () => {
       };
 
       // Mock rate limit check (not exceeded)
-      vi.mocked(checkSimpleRateLimit).mockResolvedValue(null);
+      vi.mocked(checkSimpleRateLimit).mockResolvedValue({ success: true, limit: 100, remaining: 99, reset: Date.now() + 60000 });
 
       // Mock successful subscription
       vi.mocked(NewsletterService.subscribe).mockResolvedValue({
@@ -79,7 +76,7 @@ describe('Newsletter Server Actions', () => {
       formData.append('email', 'user@example.com');
       formData.append('storeId', 'store-123');
 
-      const result = await subscribeToNewsletter(formData);
+      const result = await subscribeToNewsletter(null, formData);
 
       expect(result).toEqual({
         success: true,
@@ -110,7 +107,7 @@ describe('Newsletter Server Actions', () => {
         updatedAt: new Date(),
       };
 
-      vi.mocked(checkSimpleRateLimit).mockResolvedValue(null);
+      vi.mocked(checkSimpleRateLimit).mockResolvedValue({ success: true, limit: 100, remaining: 99, reset: Date.now() + 60000 });
 
       vi.mocked(NewsletterService.subscribe).mockResolvedValue({
         subscription: mockNewsletter,
@@ -122,7 +119,7 @@ describe('Newsletter Server Actions', () => {
       formData.append('email', 'user@example.com');
       formData.append('storeId', 'store-123');
 
-      const result = await subscribeToNewsletter(formData);
+      const result = await subscribeToNewsletter(null, formData);
 
       expect(result).toEqual({
         success: true,
@@ -132,18 +129,18 @@ describe('Newsletter Server Actions', () => {
     });
 
     it('should enforce rate limiting', async () => {
-      const mockRateLimitResponse = new Response(
-        JSON.stringify({ error: 'Too many requests' }),
-        { status: 429 }
-      );
-
-      vi.mocked(checkSimpleRateLimit).mockResolvedValue(mockRateLimitResponse);
+      vi.mocked(checkSimpleRateLimit).mockResolvedValue({
+        success: false,
+        limit: 100,
+        remaining: 0,
+        reset: Date.now() + 60000,
+      });
 
       const formData = new FormData();
       formData.append('email', 'user@example.com');
       formData.append('storeId', 'store-123');
 
-      const result = await subscribeToNewsletter(formData);
+      const result = await subscribeToNewsletter(null, formData);
 
       expect(result).toEqual({
         success: false,
@@ -154,71 +151,71 @@ describe('Newsletter Server Actions', () => {
     });
 
     it('should validate email format with Zod', async () => {
-      vi.mocked(checkSimpleRateLimit).mockResolvedValue(null);
+      vi.mocked(checkSimpleRateLimit).mockResolvedValue({ success: true, limit: 100, remaining: 99, reset: Date.now() + 60000 });
 
       const formData = new FormData();
       formData.append('email', 'invalid-email'); // Invalid format
       formData.append('storeId', 'store-123');
 
-      const result = await subscribeToNewsletter(formData);
+      const result = await subscribeToNewsletter(null, formData);
 
       expect(result.success).toBe(false);
       expect(result.error).toContain('Invalid email');
       expect(NewsletterService.subscribe).not.toHaveBeenCalled();
     });
 
     it('should validate email length (min 3 chars)', async () => {
-      vi.mocked(checkSimpleRateLimit).mockResolvedValue(null);
+      vi.mocked(checkSimpleRateLimit).mockResolvedValue({ success: true, limit: 100, remaining: 99, reset: Date.now() + 60000 });
 
       const formData = new FormData();
       formData.append('email', 'ab'); // Too short
       formData.append('storeId', 'store-123');
 
-      const result = await subscribeToNewsletter(formData);
+      const result = await subscribeToNewsletter(null, formData);
 
       expect(result.success).toBe(false);
       expect(result.error).toBeDefined();
       expect(NewsletterService.subscribe).not.toHaveBeenCalled();
     });
 
     it('should validate email length (max 255 chars)', async () => {
-      vi.mocked(checkSimpleRateLimit).mockResolvedValue(null);
+      vi.mocked(checkSimpleRateLimit).mockResolvedValue({ success: true, limit: 100, remaining: 99, reset: Date.now() + 60000 });
 
       const longEmail = 'a'.repeat(250) + '@example.com'; // > 255 chars
 
       const formData = new FormData();
       formData.append('email', longEmail);
       formData.append('storeId', 'store-123');
 
-      const result = await subscribeToNewsletter(formData);
+      const result = await subscribeToNewsletter(null, formData);
 
       expect(result.success).toBe(false);
       expect(result.error).toBeDefined();
       expect(NewsletterService.subscribe).not.toHaveBeenCalled();
     });
 
     it('should require email field', async () => {
-      vi.mocked(checkSimpleRateLimit).mockResolvedValue(null);
+      vi.mocked(checkSimpleRateLimit).mockResolvedValue({ success: true, limit: 100, remaining: 99, reset: Date.now() + 60000 });
 
       const formData = new FormData();
       formData.append('storeId', 'store-123');
       // Missing email
 
-      const result = await subscribeToNewsletter(formData);
+      const result = await subscribeToNewsletter(null, formData);
 
       expect(result.success).toBe(false);
       expect(result.error).toBeDefined();
       expect(NewsletterService.subscribe).not.toHaveBeenCalled();
     });
 
     it('should require storeId field', async () => {
-      vi.mocked(checkSimpleRateLimit).mockResolvedValue(null);
+      vi.mocked(checkSimpleRateLimit).mockResolvedValue({ success: true, limit: 100, remaining: 99, reset: Date.now() + 60000 });
 
       const formData = new FormData();
       formData.append('email', 'user@example.com');
       // Missing storeId
 
-      const result = await subscribeToNewsletter(formData);
+      const result = await subscribeToNewsletter(null, formData);
 
       expect(result.success).toBe(false);
       expect(result.error).toBeDefined();
@@ -233,7 +230,7 @@ describe('Newsletter Server Actions', () => {
       });
 
       vi.mocked(headers).mockResolvedValue(headersWithDNT);
-      vi.mocked(checkSimpleRateLimit).mockResolvedValue(null);
+      vi.mocked(checkSimpleRateLimit).mockResolvedValue({ success: true, limit: 100, remaining: 99, reset: Date.now() + 60000 });
 
       vi.mocked(NewsletterService.subscribe).mockResolvedValue({
         subscription: {} as Newsletter,
@@ -245,7 +242,7 @@ describe('Newsletter Server Actions', () => {
       formData.append('email', 'user@example.com');
       formData.append('storeId', 'store-123');
 
-      await subscribeToNewsletter(formData);
+      await subscribeToNewsletter(null, formData);
 
       expect(NewsletterService.subscribe).toHaveBeenCalledWith(
         expect.objectContaining({
@@ -257,7 +254,7 @@ describe('Newsletter Server Actions', () => {
     });
 
     it('should handle service errors gracefully', async () => {
-      vi.mocked(checkSimpleRateLimit).mockResolvedValue(null);
+      vi.mocked(checkSimpleRateLimit).mockResolvedValue({ success: true, limit: 100, remaining: 99, reset: Date.now() + 60000 });
 
       vi.mocked(NewsletterService.subscribe).mockRejectedValue(
         new Error('Database connection failed')
@@ -267,7 +264,7 @@ describe('Newsletter Server Actions', () => {
       formData.append('email', 'user@example.com');
       formData.append('storeId', 'store-123');
 
-      const result = await subscribeToNewsletter(formData);
+      const result = await subscribeToNewsletter(null, formData);
 
       expect(result).toEqual({
         success: false,
@@ -290,15 +287,15 @@ describe('Newsletter Server Actions', () => {
         updatedAt: new Date(),
       };
 
-      vi.mocked(checkSimpleRateLimit).mockResolvedValue(null);
+      vi.mocked(checkSimpleRateLimit).mockResolvedValue({ success: true, limit: 100, remaining: 99, reset: Date.now() + 60000 });
 
       vi.mocked(NewsletterService.unsubscribe).mockResolvedValue(mockUnsubscribed);
 
       const formData = new FormData();
       formData.append('email', 'user@example.com');
       formData.append('storeId', 'store-123');
 
-      const result = await unsubscribeFromNewsletter(formData);
+      const result = await unsubscribeFromNewsletter(null, formData);
 
       expect(result).toEqual({
         success: true,
@@ -315,15 +312,15 @@ describe('Newsletter Server Actions', () => {
     });
 
     it('should handle non-existent subscription', async () => {
-      vi.mocked(checkSimpleRateLimit).mockResolvedValue(null);
+      vi.mocked(checkSimpleRateLimit).mockResolvedValue({ success: true, limit: 100, remaining: 99, reset: Date.now() + 60000 });
 
       vi.mocked(NewsletterService.unsubscribe).mockResolvedValue(null);
 
       const formData = new FormData();
       formData.append('email', 'nonexistent@example.com');
       formData.append('storeId', 'store-123');
 
-      const result = await unsubscribeFromNewsletter(formData);
+      const result = await unsubscribeFromNewsletter(null, formData);
 
       expect(result).toEqual({
         success: true,
@@ -332,18 +329,18 @@ describe('Newsletter Server Actions', () => {
     });
 
     it('should enforce rate limiting', async () => {
-      const mockRateLimitResponse = new Response(
-        JSON.stringify({ error: 'Too many requests' }),
-        { status: 429 }
-      );
-
-      vi.mocked(checkSimpleRateLimit).mockResolvedValue(mockRateLimitResponse);
+      vi.mocked(checkSimpleRateLimit).mockResolvedValue({
+        success: false,
+        limit: 100,
+        remaining: 0,
+        reset: Date.now() + 60000,
+      });
 
       const formData = new FormData();
       formData.append('email', 'user@example.com');
       formData.append('storeId', 'store-123');
 
-      const result = await unsubscribeFromNewsletter(formData);
+      const result = await unsubscribeFromNewsletter(null, formData);
 
       expect(result).toEqual({
         success: false,
@@ -354,35 +351,35 @@ describe('Newsletter Server Actions', () => {
     });
 
     it('should require email field', async () => {
-      vi.mocked(checkSimpleRateLimit).mockResolvedValue(null);
+      vi.mocked(checkSimpleRateLimit).mockResolvedValue({ success: true, limit: 100, remaining: 99, reset: Date.now() + 60000 });
 
       const formData = new FormData();
       formData.append('storeId', 'store-123');
       // Missing email
 
-      const result = await unsubscribeFromNewsletter(formData);
+      const result = await unsubscribeFromNewsletter(null, formData);
 
       expect(result.success).toBe(false);
       expect(result.error).toBe('Email is required');
       expect(NewsletterService.unsubscribe).not.toHaveBeenCalled();
     });
 
     it('should require storeId field', async () => {
-      vi.mocked(checkSimpleRateLimit).mockResolvedValue(null);
+      vi.mocked(checkSimpleRateLimit).mockResolvedValue({ success: true, limit: 100, remaining: 99, reset: Date.now() + 60000 });
 
       const formData = new FormData();
       formData.append('email', 'user@example.com');
       // Missing storeId
 
-      const result = await unsubscribeFromNewsletter(formData);
+      const result = await unsubscribeFromNewsletter(null, formData);
 
       expect(result.success).toBe(false);
       expect(result.error).toBe('Store ID is required');
       expect(NewsletterService.unsubscribe).not.toHaveBeenCalled();
     });
 
     it('should handle service errors gracefully', async () => {
-      vi.mocked(checkSimpleRateLimit).mockResolvedValue(null);
+      vi.mocked(checkSimpleRateLimit).mockResolvedValue({ success: true, limit: 100, remaining: 99, reset: Date.now() + 60000 });
 
       vi.mocked(NewsletterService.unsubscribe).mockRejectedValue(
         new Error('Database connection failed')
@@ -392,7 +389,7 @@ describe('Newsletter Server Actions', () => {
       formData.append('email', 'user@example.com');
       formData.append('storeId', 'store-123');
 
-      const result = await unsubscribeFromNewsletter(formData);
+      const result = await unsubscribeFromNewsletter(null, formData);
 
       expect(result).toEqual({
         success: false,