pacnpal/thrilltrack-explorer
1
0
Fork 0
mirror of https://github.com/pacnpal/thrilltrack-explorer.git synced 2025-12-20 06:31:13 -05:00
Code Issues Packages Projects Releases Wiki Activity

Refactor validation to edge function

Browse Source 
Centralize all business logic validation within the edge function for the submission pipeline. Remove validation logic from React hooks, retaining only basic UX validation (e.g., checking for empty fields). This ensures a single source of truth for validation, preventing inconsistencies between the frontend and backend.
This commit is contained in:
gpt-engineer-app[bot]
2025-11-06 16:18:34 +00:00
parent 1cc80e0dc4
commit e7f5aa9d17
3 changed files with 224 additions and 203 deletions
Download Patch File Download Diff File Expand all files Collapse all files

194
docs/VALIDATION_CENTRALIZATION.md Normal file
View File

@@ -0,0 +1,194 @@
# Validation Centralization - Critical Issue #3 Fixed
## Overview
This document describes the changes made to centralize all business logic validation in the edge function, removing duplicate validation from the React frontend.
## Problem Statement
Previously, validation was duplicated in two places:
1. **React Frontend** (`useModerationActions.ts`): Performed full business logic validation using Zod schemas before calling the edge function
2. **Edge Function** (`process-selective-approval`): Also performed full business logic validation
This created several issues:
- **Duplicate Code**: Same validation logic maintained in two places
- **Inconsistency Risk**: Frontend and backend could have different validation rules
- **Performance**: Unnecessary network round-trips for validation data fetching
- **Single Source of Truth Violation**: No clear authority on what's valid
## Solution: Edge Function as Single Source of Truth
### Architecture Changes
```
┌─────────────────────────────────────────────────────────────────┐
│ BEFORE (Duplicate) │
├─────────────────────────────────────────────────────────────────┤
│ │
│ React Frontend Edge Function │
│ ┌──────────────┐ ┌──────────────┐ │
│ │ UX Validation│ │ Business │ │
│ │ + │──────────────▶│ Validation │ │
│ │ Business │ If valid │ │ │
│ │ Validation │ call edge │ (Duplicate) │ │
│ └──────────────┘ └──────────────┘ │
│ ❌ Duplicate validation logic │
└─────────────────────────────────────────────────────────────────┘
┌─────────────────────────────────────────────────────────────────┐
│ AFTER (Centralized) ✅ │
├─────────────────────────────────────────────────────────────────┤
│ │
│ React Frontend Edge Function │
│ ┌──────────────┐ ┌──────────────┐ │
│ │ UX Validation│ │ Business │ │
│ │ Only │──────────────▶│ Validation │ │
│ │ (non-empty, │ Always │ (Authority) │ │
│ │ format) │ call edge │ │ │
│ └──────────────┘ └──────────────┘ │
│ ✅ Single source of truth │
└─────────────────────────────────────────────────────────────────┘
```
### Changes Made
#### 1. React Frontend (`src/hooks/moderation/useModerationActions.ts`)
**Removed:**
- Import of `validateMultipleItems` from `entityValidationSchemas`
- 200+ lines of validation code that:
- Fetched full item data with relational joins
- Ran Zod validation on all items
- Blocked approval if validation failed
- Logged validation errors
**Added:**
- Clear comment explaining validation happens server-side only
- Enhanced error handling to detect validation errors from edge function
**What Remains:**
- Basic error handling for edge function responses
- Toast notifications for validation failures
- Proper error logging with validation flag
#### 2. Validation Schemas (`src/lib/entityValidationSchemas.ts`)
**Updated:**
- Added comprehensive documentation header
- Marked schemas as "documentation only" for React app
- Clarified that edge function is the authority
- Noted these schemas should mirror edge function validation
**Status:**
- File retained for documentation and future reference
- Not imported anywhere in production React code
- Can be used for basic client-side UX validation if needed
#### 3. Edge Function (`supabase/functions/process-selective-approval/index.ts`)
**No Changes Required:**
- Already has comprehensive validation via `validateEntityDataStrict()`
- Already returns proper 400 errors for validation failures
- Already includes detailed error messages
## Validation Responsibilities
### Client-Side (React Forms)
**Allowed:**
- ✅ Non-empty field validation (required fields)
- ✅ Basic format validation (email, URL format)
- ✅ Character length limits
- ✅ Input masking and formatting
- ✅ Immediate user feedback for UX
**Not Allowed:**
- ❌ Business rule validation (e.g., closing date after opening date)
- ❌ Cross-field validation
- ❌ Database constraint validation
- ❌ Entity relationship validation
- ❌ Status/state validation
### Server-Side (Edge Function)
**Authoritative For:**
- ✅ All business logic validation
- ✅ Cross-field validation
- ✅ Database constraint validation
- ✅ Entity relationship validation
- ✅ Status/state validation
- ✅ Security validation
- ✅ Data integrity checks
## Error Handling Flow
```typescript
// 1. User clicks "Approve" in UI
// 2. React calls edge function immediately (no validation)
const { data, error } = await invokeWithTracking('process-selective-approval', {
itemIds: [...],
submissionId: '...'
});
// 3. Edge function validates and returns error if invalid
if (error) {
// Error contains validation details from edge function
// React displays the error message
toast({
title: 'Validation Failed',
description: error.message // e.g., "Park name is required"
});
}
```
## Benefits
1. **Single Source of Truth**: Edge function is the authority
2. **Consistency**: No risk of frontend/backend validation diverging
3. **Performance**: No pre-validation data fetching in frontend
4. **Maintainability**: Update validation in one place
5. **Security**: Can't bypass validation by manipulating frontend
6. **Simplicity**: Frontend code is simpler and cleaner
## Testing Validation
To test that validation works:
1. Submit a park without required fields
2. Submit a park with invalid dates (closing before opening)
3. Submit a ride without a park_id
4. Submit a company with invalid email format
Expected: Edge function should return 400 error with detailed message, React should display error toast.
## Migration Guide
If you need to add new validation rules:
1. ✅ **Add to edge function** (`process-selective-approval/index.ts`)
- Update `validateEntityDataStrict()` function
- Add to appropriate entity type case
2. ✅ **Update documentation schemas** (`entityValidationSchemas.ts`)
- Keep schemas in sync for reference
- Update comments if rules change
3. ❌ **DO NOT add to React validation**
- React should only do basic UX validation
- Business logic belongs in edge function
## Related Issues
This fix addresses:
- ✅ Critical Issue #3: Validation centralization
- ✅ Removes ~200 lines of duplicate code
- ✅ Eliminates validation timing gap
- ✅ Simplifies frontend logic
- ✅ Improves maintainability
## Files Changed
- `src/hooks/moderation/useModerationActions.ts` - Removed validation logic
- `src/lib/entityValidationSchemas.ts` - Updated documentation
- `docs/VALIDATION_CENTRALIZATION.md` - This document

215
src/hooks/moderation/useModerationActions.ts
View File

@@ -4,7 +4,7 @@ import { supabase } from '@/lib/supabaseClient';
import { useToast } from '@/hooks/use-toast';
import { logger } from '@/lib/logger';
import { getErrorMessage, handleError, isSupabaseConnectionError } from '@/lib/errorHandler';
import { validateMultipleItems } from '@/lib/entityValidationSchemas';
// Validation removed from client - edge function is single source of truth
import { invokeWithTracking } from '@/lib/edgeFunctionTracking';
import type { User } from '@supabase/supabase-js';
import type { ModerationItem } from '@/types/moderation';
@@ -133,206 +133,10 @@ export function useModerationActions(config: ModerationActionsConfig): Moderatio
if (submissionItems && submissionItems.length > 0) {
if (action === 'approved') {
// Fetch full item data for validation with relational joins
const { data: fullItems, error: itemError } = await supabase
.from('submission_items')
.select(`
id,
item_type,
park_submission:park_submissions!submission_items_park_submission_id_fkey(*),
ride_submission:ride_submissions!submission_items_ride_submission_id_fkey(*),
company_submission:company_submissions!submission_items_company_submission_id_fkey(*),
ride_model_submission:ride_model_submissions!submission_items_ride_model_submission_id_fkey(*),
timeline_event_submission:timeline_event_submissions!submission_items_timeline_event_submission_id_fkey(*),
photo_submission:photo_submissions!submission_items_photo_submission_id_fkey(*)
`)
.eq('submission_id', item.id)
.in('status', ['pending', 'rejected']);
if (itemError) {
throw new Error(`Failed to fetch submission items: ${itemError.message}`);
}
if (fullItems && fullItems.length > 0) {
console.info('[Submission Flow] Preparing items for validation', {
submissionId: item.id,
itemCount: fullItems.length,
itemTypes: fullItems.map(i => i.item_type),
timestamp: new Date().toISOString()
});
// Transform to include item_data
const itemsWithData = await Promise.all(fullItems.map(async item => {
let itemData = {};
switch (item.item_type) {
case 'park': {
const parkSub = (item.park_submission as any) || {};
let locationData: any = null;
if (parkSub?.id) {
const { data } = await supabase
.from('park_submission_locations')
.select('*')
.eq('park_submission_id', parkSub.id)
.maybeSingle();
locationData = data;
}
itemData = {
...parkSub,
location: locationData || undefined
};
break;
}
case 'ride':
itemData = item.ride_submission || {};
break;
case 'operator':
case 'manufacturer':
case 'designer':
case 'property_owner':
itemData = {
...(item.company_submission || {}),
company_id: item.company_submission?.id // Use company_submission ID for validation
};
break;
case 'ride_model':
itemData = {
...(item.ride_model_submission || {}),
ride_model_id: item.ride_model_submission?.id // Use ride_model_submission ID for validation
};
break;
case 'milestone':
case 'timeline_event':
itemData = item.timeline_event_submission || {};
break;
case 'photo':
case 'photo_edit':
case 'photo_delete':
itemData = item.photo_submission || {};
break;
default:
logger.warn(`Unknown item_type in validation: ${item.item_type}`);
itemData = {};
}
return {
id: item.id,
item_type: item.item_type,
item_data: itemData
};
}));
// Run validation on all items
try {
console.info('[Submission Flow] Starting validation', {
submissionId: item.id,
itemCount: itemsWithData.length,
itemTypes: itemsWithData.map(i => i.item_type),
timestamp: new Date().toISOString()
});
const validationResults = await validateMultipleItems(itemsWithData);
console.info('[Submission Flow] Validation completed', {
submissionId: item.id,
resultsCount: validationResults.size,
timestamp: new Date().toISOString()
});
// Check for blocking errors
const itemsWithBlockingErrors = itemsWithData.filter(item => {
const result = validationResults.get(item.id);
return result && result.blockingErrors.length > 0;
});
// CRITICAL: Block approval if any item has blocking errors
if (itemsWithBlockingErrors.length > 0) {
console.warn('[Submission Flow] Validation found blocking errors', {
submissionId: item.id,
itemsWithErrors: itemsWithBlockingErrors.map(i => ({
itemId: i.id,
itemType: i.item_type,
errors: validationResults.get(i.id)?.blockingErrors
})),
timestamp: new Date().toISOString()
});
// Log detailed blocking errors
itemsWithBlockingErrors.forEach(item => {
const result = validationResults.get(item.id);
logger.error('Validation blocking approval', {
submissionId: item.id,
itemId: item.id,
itemType: item.item_type,
blockingErrors: result?.blockingErrors
});
});
const errorDetails = itemsWithBlockingErrors.map(item => {
const result = validationResults.get(item.id);
const itemName = (item.item_data as any)?.name || item.item_type;
const errors = result?.blockingErrors.map(e => `${e.field}: ${e.message}`).join(', ');
return `${itemName} - ${errors}`;
}).join('; ');
throw new Error(`Validation failed: ${errorDetails}`);
}
// Check for warnings (optional - can proceed but inform user)
const itemsWithWarnings = itemsWithData.filter(item => {
const result = validationResults.get(item.id);
return result && result.warnings.length > 0;
});
if (itemsWithWarnings.length > 0) {
logger.info('Approval proceeding with warnings', {
submissionId: item.id,
warningCount: itemsWithWarnings.length
});
}
} catch (error) {
// Check if this is a validation error or system error
if (getErrorMessage(error).includes('Validation failed:')) {
// This is expected - validation rules preventing approval
handleError(error, {
action: 'Validation Blocked Approval',
userId: user?.id,
metadata: {
submissionId: item.id,
submissionType: item.submission_type,
selectedItemCount: itemsWithData.length
}
});
toast({
title: 'Cannot Approve - Validation Errors',
description: getErrorMessage(error),
variant: 'destructive',
});
// Return early - do NOT proceed with approval
return;
} else {
// Unexpected validation system error
const errorId = handleError(error, {
action: 'Validation System Failure',
userId: user?.id,
metadata: {
submissionId: item.id,
submissionType: item.submission_type,
phase: 'validation'
}
});
toast({
title: 'Validation System Error',
description: `Unable to validate submission (ref: ${errorId.slice(0, 8)})`,
variant: 'destructive',
});
// Return early - do NOT proceed with approval
return;
}
}
}
// ⚠️ VALIDATION CENTRALIZED IN EDGE FUNCTION
// All business logic validation happens in process-selective-approval edge function.
// Client-side only performs basic UX validation (non-empty, format) in forms.
// If server-side validation fails, the edge function returns detailed 400/500 errors.
const { data, error, requestId, attempts } = await invokeWithTracking(
'process-selective-approval',
@@ -465,8 +269,14 @@ export function useModerationActions(config: ModerationActionsConfig): Moderatio
const isNetworkError = isSupabaseConnectionError(error);
const errorMessage = getErrorMessage(error) || `Failed to ${variables.action} content`;
// Check if this is a validation error from edge function
const isValidationError = errorMessage.includes('Validation failed') ||
errorMessage.includes('blocking errors') ||
errorMessage.includes('blockingErrors');
toast({
title: isNetworkError ? 'Connection Error' : 'Action Failed',
title: isNetworkError ? 'Connection Error' :
isValidationError ? 'Validation Failed' : 'Action Failed',
description: errorMessage,
variant: 'destructive',
});
@@ -477,6 +287,7 @@ export function useModerationActions(config: ModerationActionsConfig): Moderatio
error: errorMessage,
errorId: error.errorId,
isNetworkError,
isValidationError,
});
},
onSuccess: (data) => {

18
src/lib/entityValidationSchemas.ts
View File

@@ -3,9 +3,25 @@ import { supabase } from '@/lib/supabaseClient';
import { handleNonCriticalError, getErrorMessage } from '@/lib/errorHandler';
import { logger } from '@/lib/logger';
// ============================================
// VALIDATION SCHEMAS - DOCUMENTATION ONLY
// ============================================
// ⚠️ NOTE: These schemas are currently NOT used in the React application.
// All business logic validation happens server-side in the edge function.
// These schemas are kept for:
// 1. Documentation of validation rules
// 2. Potential future use for client-side UX validation (basic checks only)
// 3. Reference when updating edge function validation logic
//
// DO NOT import these in production code for business logic validation.
// ============================================
// ============================================
// CENTRALIZED VALIDATION SCHEMAS
// Single source of truth for all entity validation
// ⚠️ CRITICAL: These schemas represent the validation rules
// They should mirror the validation in process-selective-approval edge function
// Client-side should NOT perform business logic validation
// Client-side only does basic UX validation (non-empty, format checks) in forms
// ============================================
const currentYear = new Date().getFullYear();