fix: Add SALEOR_API_TOKEN auth for COD transaction creation
- Add SALEOR_API_TOKEN environment variable support - Update Apollo client to include auth header - Enable COD transaction creation after checkout
This commit is contained in:
317
docs/CHECKOUT_ARCHITECTURE_ANALYSIS.md
Normal file
317
docs/CHECKOUT_ARCHITECTURE_ANALYSIS.md
Normal file
@@ -0,0 +1,317 @@
|
||||
# Checkout Architecture Analysis
|
||||
|
||||
## What Broke: Root Cause Analysis
|
||||
|
||||
### The Incident
|
||||
Yesterday, checkout confirmation emails were working correctly in the customer's selected language. Today, they started arriving in English regardless of the customer's language preference.
|
||||
|
||||
### Root Cause
|
||||
**Implicit Dependency on Step Ordering**
|
||||
|
||||
The checkout flow had a critical implicit requirement: the `languageCode` field MUST be set on the checkout object BEFORE calling `checkoutComplete`. This was discovered through trial and error, not through explicit architecture.
|
||||
|
||||
### Why Small Changes Broke It
|
||||
|
||||
The checkout flow was implemented as a **procedural monolith** in `page.tsx`:
|
||||
|
||||
```typescript
|
||||
// ❌ BEFORE: Monolithic function (440+ lines)
|
||||
const handleSubmit = async () => {
|
||||
// Step 1: Email
|
||||
await updateEmail()
|
||||
|
||||
// Step 2: Language ← This was added today
|
||||
await updateLanguage() // <- Without this, emails are in wrong language!
|
||||
|
||||
// Step 3: Addresses
|
||||
await updateBillingAddress()
|
||||
|
||||
// Step 4: Shipping
|
||||
await updateShippingMethod()
|
||||
|
||||
// Step 5: Metadata
|
||||
await updateMetadata()
|
||||
|
||||
// Step 6: Complete
|
||||
await checkoutComplete()
|
||||
}
|
||||
```
|
||||
|
||||
**Problems with this approach:**
|
||||
|
||||
1. **No explicit contracts**: Nothing says "language must be set before complete"
|
||||
2. **Ordering is fragile**: Moving steps around breaks functionality
|
||||
3. **No isolation**: Can't test individual steps
|
||||
4. **Tight coupling**: UI, validation, API calls, and business logic all mixed
|
||||
5. **No failure boundaries**: One failure stops everything, but unclear where
|
||||
|
||||
## The Fix: Proper Abstraction
|
||||
|
||||
### New Architecture
|
||||
|
||||
```
|
||||
┌─────────────────────────────────────────────────────────────┐
|
||||
│ UI Layer (Page Component) │
|
||||
│ - Form handling │
|
||||
│ - Display logic │
|
||||
│ - Error display │
|
||||
└───────────────────────┬─────────────────────────────────────┘
|
||||
│
|
||||
▼
|
||||
┌─────────────────────────────────────────────────────────────┐
|
||||
│ Checkout Service Layer │
|
||||
│ - executeCheckoutPipeline() │
|
||||
│ - Enforces step ordering │
|
||||
│ - Validates inputs │
|
||||
│ - Handles failures │
|
||||
└───────────────────────┬─────────────────────────────────────┘
|
||||
│
|
||||
▼
|
||||
┌─────────────────────────────────────────────────────────────┐
|
||||
│ Individual Steps (Composable) │
|
||||
│ - updateCheckoutEmail() │
|
||||
│ - updateCheckoutLanguage() ← CRITICAL: Before complete! │
|
||||
│ - updateShippingAddress() │
|
||||
│ - updateBillingAddress() │
|
||||
│ - updateShippingMethod() │
|
||||
│ - updateCheckoutMetadata() │
|
||||
│ - completeCheckout() │
|
||||
└───────────────────────┬─────────────────────────────────────┘
|
||||
│
|
||||
▼
|
||||
┌─────────────────────────────────────────────────────────────┐
|
||||
│ Saleor API Client │
|
||||
└─────────────────────────────────────────────────────────────┘
|
||||
```
|
||||
|
||||
### Key Improvements
|
||||
|
||||
#### 1. **Explicit Pipeline**
|
||||
```typescript
|
||||
// ✅ AFTER: Explicit pipeline with enforced ordering
|
||||
export async function executeCheckoutPipeline(input: CheckoutInput) {
|
||||
// Step 1: Email
|
||||
const emailResult = await updateCheckoutEmail(checkoutId, email);
|
||||
if (!emailResult.success) return { success: false, error: emailResult.error };
|
||||
|
||||
// Step 2: Language (CRITICAL for email language)
|
||||
const languageResult = await updateCheckoutLanguage(checkoutId, languageCode);
|
||||
if (!languageResult.success) return { success: false, error: languageResult.error };
|
||||
// ^^^ This MUST happen before complete - enforced by structure!
|
||||
|
||||
// Step 3: Addresses
|
||||
// ...
|
||||
|
||||
// Step 7: Complete
|
||||
return completeCheckout(checkoutId);
|
||||
}
|
||||
```
|
||||
|
||||
**Benefits:**
|
||||
- Order is enforced by code structure, not comments
|
||||
- Each step validates its result before continuing
|
||||
- Clear failure points
|
||||
|
||||
#### 2. **Composable Steps**
|
||||
Each step is an independent, testable function:
|
||||
|
||||
```typescript
|
||||
// Can be tested in isolation
|
||||
export async function updateCheckoutLanguage(
|
||||
checkoutId: string,
|
||||
languageCode: string
|
||||
): Promise<CheckoutStepResult> {
|
||||
const { data } = await saleorClient.mutate({
|
||||
mutation: CHECKOUT_LANGUAGE_CODE_UPDATE,
|
||||
variables: { checkoutId, languageCode },
|
||||
});
|
||||
|
||||
if (data?.checkoutLanguageCodeUpdate?.errors?.length) {
|
||||
return { success: false, error: data.checkoutLanguageCodeUpdate.errors[0].message };
|
||||
}
|
||||
|
||||
return { success: true };
|
||||
}
|
||||
```
|
||||
|
||||
**Benefits:**
|
||||
- Unit testable
|
||||
- Can be reused in other flows
|
||||
- Can be mocked for testing
|
||||
- Clear input/output contracts
|
||||
|
||||
#### 3. **Validation Separation**
|
||||
```typescript
|
||||
// Pure validation functions
|
||||
export function validateAddress(address: Partial<Address>): string | null {
|
||||
if (!address.firstName?.trim()) return "First name is required";
|
||||
if (!address.phone?.trim() || address.phone.length < 8) return "Valid phone is required";
|
||||
return null;
|
||||
}
|
||||
```
|
||||
|
||||
**Benefits:**
|
||||
- Validation is deterministic and testable
|
||||
- No UI dependencies
|
||||
- Can be reused
|
||||
|
||||
#### 4. **Service Class for Complex Use Cases**
|
||||
```typescript
|
||||
// For cases that need step-by-step control
|
||||
const checkoutService = createCheckoutService(checkoutId);
|
||||
await checkoutService.updateEmail(email);
|
||||
await checkoutService.updateLanguage(locale); // Explicitly called
|
||||
// ... custom logic ...
|
||||
await checkoutService.complete();
|
||||
```
|
||||
|
||||
## Comparison: Before vs After
|
||||
|
||||
| Aspect | Before (Monolithic) | After (Service Layer) |
|
||||
|--------|--------------------|----------------------|
|
||||
| **Lines of code** | 440+ in one function | ~50 in UI, 300 in service |
|
||||
| **Testability** | ❌ Can't unit test | ✅ Each step testable |
|
||||
| **Step ordering** | ❌ Implicit/fragile | ✅ Enforced by structure |
|
||||
| **Failure handling** | ❌ Try/catch spaghetti | ✅ Result-based, explicit |
|
||||
| **Reusability** | ❌ Copy-paste only | ✅ Import and compose |
|
||||
| **Type safety** | ⚠️ Inline types | ✅ Full TypeScript |
|
||||
| **Documentation** | ❌ Comments only | ✅ Code is self-documenting |
|
||||
|
||||
## Critical Business Rules Now Explicit
|
||||
|
||||
```typescript
|
||||
// These rules are now ENFORCED by code, not comments:
|
||||
|
||||
// Rule 1: Language must be set before checkout completion
|
||||
const languageResult = await updateCheckoutLanguage(checkoutId, languageCode);
|
||||
if (!languageResult.success) {
|
||||
return { success: false, error: languageResult.error }; // Pipeline stops!
|
||||
}
|
||||
// Only after success do we proceed to complete...
|
||||
|
||||
// Rule 2: Any step failure stops the pipeline
|
||||
const emailResult = await updateCheckoutEmail(checkoutId, email);
|
||||
if (!emailResult.success) {
|
||||
return { success: false, error: emailResult.error }; // Early return!
|
||||
}
|
||||
|
||||
// Rule 3: Validation happens before any API calls
|
||||
const validationError = validateCheckoutInput(input);
|
||||
if (validationError) {
|
||||
return { success: false, error: validationError }; // Fail fast!
|
||||
}
|
||||
```
|
||||
|
||||
## Why This Won't Break Again
|
||||
|
||||
### 1. **Enforced Ordering**
|
||||
The pipeline function physically cannot complete checkout without first setting the language. It's not a comment—it's code structure.
|
||||
|
||||
### 2. **Fail Fast**
|
||||
Validation happens before any API calls. Invalid data never reaches Saleor.
|
||||
|
||||
### 3. **Explicit Error Handling**
|
||||
Each step returns a `CheckoutStepResult` with `success` boolean. No exceptions for flow control.
|
||||
|
||||
### 4. **Composable Design**
|
||||
If we need to add a new step (e.g., "apply coupon"), we insert it into the pipeline:
|
||||
```typescript
|
||||
const couponResult = await applyCoupon(checkoutId, couponCode);
|
||||
if (!couponResult.success) return { success: false, error: couponResult.error };
|
||||
```
|
||||
The location in the pipeline shows its dependency order.
|
||||
|
||||
### 5. **Type Safety**
|
||||
TypeScript enforces that all required fields are present before the pipeline runs.
|
||||
|
||||
## Migration Path
|
||||
|
||||
### Phase 1: Keep Both (Current)
|
||||
- Old code in `page.tsx` continues to work
|
||||
- New service available for new features
|
||||
- Gradual migration
|
||||
|
||||
### Phase 2: Migrate UI
|
||||
Replace the monolithic `handleSubmit` with service call:
|
||||
```typescript
|
||||
// In page.tsx
|
||||
import { createCheckoutService } from '@/lib/services/checkoutService';
|
||||
|
||||
const handleSubmit = async () => {
|
||||
const checkoutService = createCheckoutService(checkout.id);
|
||||
|
||||
const result = await checkoutService.execute({
|
||||
email: shippingAddress.email,
|
||||
shippingAddress: transformToServiceAddress(shippingAddress),
|
||||
billingAddress: transformToServiceAddress(billingAddress),
|
||||
shippingMethodId: selectedShippingMethod,
|
||||
languageCode: locale,
|
||||
metadata: { phone: shippingAddress.phone, userLanguage: locale },
|
||||
});
|
||||
|
||||
if (result.success) {
|
||||
setOrderNumber(result.order!.number);
|
||||
clearCheckout();
|
||||
} else {
|
||||
setError(result.error);
|
||||
}
|
||||
};
|
||||
```
|
||||
|
||||
### Phase 3: Remove Old Code
|
||||
Once confirmed working, remove the inline mutations from `page.tsx`.
|
||||
|
||||
## Testing Strategy
|
||||
|
||||
With the new architecture, we can test each component:
|
||||
|
||||
```typescript
|
||||
// Test individual steps
|
||||
import { updateCheckoutLanguage, validateAddress } from './checkoutService';
|
||||
|
||||
describe('updateCheckoutLanguage', () => {
|
||||
it('should fail if checkout does not exist', async () => {
|
||||
const result = await updateCheckoutLanguage('invalid-id', 'EN');
|
||||
expect(result.success).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('validateAddress', () => {
|
||||
it('should require phone number', () => {
|
||||
const error = validateAddress({ ...validAddress, phone: '' });
|
||||
expect(error).toContain('phone');
|
||||
});
|
||||
});
|
||||
|
||||
// Test full pipeline
|
||||
import { executeCheckoutPipeline } from './checkoutService';
|
||||
|
||||
describe('executeCheckoutPipeline', () => {
|
||||
it('should stop if language update fails', async () => {
|
||||
// Mock language failure
|
||||
jest.spyOn(checkoutService, 'updateCheckoutLanguage').mockResolvedValue({
|
||||
success: false, error: 'Language not supported'
|
||||
});
|
||||
|
||||
const result = await executeCheckoutPipeline(validInput);
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.error).toBe('Language not supported');
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
## Conclusion
|
||||
|
||||
The previous architecture was **accidentally fragile** because:
|
||||
1. Business rules were implicit (language must be set before complete)
|
||||
2. Step ordering was by convention, not enforcement
|
||||
3. Everything was tightly coupled in one function
|
||||
4. No clear boundaries between concerns
|
||||
|
||||
The new architecture is **intentionally robust** because:
|
||||
1. Business rules are enforced by code structure
|
||||
2. Step ordering is physically enforced by the pipeline
|
||||
3. Each component has a single, clear responsibility
|
||||
4. Strong TypeScript contracts prevent misuse
|
||||
|
||||
**Small changes will no longer break critical functionality** because the architecture makes dependencies explicit and enforces them at compile time and runtime.
|
||||
Reference in New Issue
Block a user