mirror of
https://github.com/pacnpal/thrilltrack-explorer.git
synced 2025-12-28 01:07:04 -05:00
Compare commits
4 Commits
41a396b063
...
9362479db2
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
9362479db2 | ||
|
|
93a3fb93fa | ||
|
|
e7f5aa9d17 | ||
|
|
1cc80e0dc4 |
194
docs/VALIDATION_CENTRALIZATION.md
Normal file
194
docs/VALIDATION_CENTRALIZATION.md
Normal 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
|
||||
@@ -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) => {
|
||||
|
||||
@@ -4908,6 +4908,72 @@ export type Database = {
|
||||
}
|
||||
Relationships: []
|
||||
}
|
||||
submission_idempotency_keys: {
|
||||
Row: {
|
||||
completed_at: string | null
|
||||
created_at: string
|
||||
duration_ms: number | null
|
||||
error_message: string | null
|
||||
expires_at: string
|
||||
id: string
|
||||
idempotency_key: string
|
||||
item_ids: Json
|
||||
moderator_id: string
|
||||
request_id: string | null
|
||||
result_data: Json | null
|
||||
status: string
|
||||
submission_id: string
|
||||
trace_id: string | null
|
||||
}
|
||||
Insert: {
|
||||
completed_at?: string | null
|
||||
created_at?: string
|
||||
duration_ms?: number | null
|
||||
error_message?: string | null
|
||||
expires_at?: string
|
||||
id?: string
|
||||
idempotency_key: string
|
||||
item_ids: Json
|
||||
moderator_id: string
|
||||
request_id?: string | null
|
||||
result_data?: Json | null
|
||||
status?: string
|
||||
submission_id: string
|
||||
trace_id?: string | null
|
||||
}
|
||||
Update: {
|
||||
completed_at?: string | null
|
||||
created_at?: string
|
||||
duration_ms?: number | null
|
||||
error_message?: string | null
|
||||
expires_at?: string
|
||||
id?: string
|
||||
idempotency_key?: string
|
||||
item_ids?: Json
|
||||
moderator_id?: string
|
||||
request_id?: string | null
|
||||
result_data?: Json | null
|
||||
status?: string
|
||||
submission_id?: string
|
||||
trace_id?: string | null
|
||||
}
|
||||
Relationships: [
|
||||
{
|
||||
foreignKeyName: "submission_idempotency_keys_submission_id_fkey"
|
||||
columns: ["submission_id"]
|
||||
isOneToOne: false
|
||||
referencedRelation: "content_submissions"
|
||||
referencedColumns: ["id"]
|
||||
},
|
||||
{
|
||||
foreignKeyName: "submission_idempotency_keys_submission_id_fkey"
|
||||
columns: ["submission_id"]
|
||||
isOneToOne: false
|
||||
referencedRelation: "moderation_queue_with_entities"
|
||||
referencedColumns: ["id"]
|
||||
},
|
||||
]
|
||||
}
|
||||
submission_item_temp_refs: {
|
||||
Row: {
|
||||
created_at: string
|
||||
@@ -5603,6 +5669,17 @@ export type Database = {
|
||||
}
|
||||
Relationships: []
|
||||
}
|
||||
idempotency_stats: {
|
||||
Row: {
|
||||
avg_duration_ms: number | null
|
||||
hour: string | null
|
||||
p95_duration_ms: number | null
|
||||
status: string | null
|
||||
total_requests: number | null
|
||||
unique_moderators: number | null
|
||||
}
|
||||
Relationships: []
|
||||
}
|
||||
moderation_queue_with_entities: {
|
||||
Row: {
|
||||
approval_mode: string | null
|
||||
@@ -5783,6 +5860,7 @@ export type Database = {
|
||||
}
|
||||
Returns: boolean
|
||||
}
|
||||
cleanup_expired_idempotency_keys: { Args: never; Returns: number }
|
||||
cleanup_expired_sessions: { Args: never; Returns: undefined }
|
||||
cleanup_old_page_views: { Args: never; Returns: undefined }
|
||||
cleanup_old_request_metadata: { Args: never; Returns: undefined }
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -836,6 +836,12 @@ serve(withRateLimit(async (req) => {
|
||||
error?: string;
|
||||
isDependencyFailure?: boolean;
|
||||
}> = [];
|
||||
// Track all created entities for rollback on failure
|
||||
const createdEntities: Array<{
|
||||
entityId: string;
|
||||
entityType: string;
|
||||
tableName: string;
|
||||
}> = [];
|
||||
|
||||
// Process items in order
|
||||
for (const item of sortedItems) {
|
||||
@@ -1053,6 +1059,27 @@ serve(withRateLimit(async (req) => {
|
||||
|
||||
if (entityId) {
|
||||
dependencyMap.set(item.id, entityId);
|
||||
|
||||
// Track created entity for potential rollback
|
||||
const tableMap: Record<string, string> = {
|
||||
'park': 'parks',
|
||||
'ride': 'rides',
|
||||
'manufacturer': 'companies',
|
||||
'operator': 'companies',
|
||||
'property_owner': 'companies',
|
||||
'designer': 'companies',
|
||||
'ride_model': 'ride_models'
|
||||
// photo operations don't create new entities in standard tables
|
||||
};
|
||||
|
||||
const tableName = tableMap[item.item_type];
|
||||
if (tableName) {
|
||||
createdEntities.push({
|
||||
entityId,
|
||||
entityType: item.item_type,
|
||||
tableName
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Store result for batch update later
|
||||
@@ -1088,9 +1115,105 @@ serve(withRateLimit(async (req) => {
|
||||
error: errorMessage,
|
||||
isDependencyFailure: isDependencyError
|
||||
});
|
||||
|
||||
// CRITICAL: Rollback all previously created entities
|
||||
if (createdEntities.length > 0) {
|
||||
edgeLogger.error('Item failed - initiating rollback', {
|
||||
action: 'approval_rollback_start',
|
||||
failedItemId: item.id,
|
||||
failedItemType: item.item_type,
|
||||
createdEntitiesCount: createdEntities.length,
|
||||
error: errorMessage,
|
||||
requestId: tracking.requestId
|
||||
});
|
||||
|
||||
// Delete all previously created entities in reverse order
|
||||
for (let i = createdEntities.length - 1; i >= 0; i--) {
|
||||
const entity = createdEntities[i];
|
||||
try {
|
||||
const { error: deleteError } = await supabase
|
||||
.from(entity.tableName)
|
||||
.delete()
|
||||
.eq('id', entity.entityId);
|
||||
|
||||
if (deleteError) {
|
||||
edgeLogger.error('Rollback delete failed', {
|
||||
action: 'approval_rollback_delete_fail',
|
||||
entityId: entity.entityId,
|
||||
entityType: entity.entityType,
|
||||
tableName: entity.tableName,
|
||||
error: deleteError.message,
|
||||
requestId: tracking.requestId
|
||||
});
|
||||
} else {
|
||||
edgeLogger.info('Rollback delete success', {
|
||||
action: 'approval_rollback_delete_success',
|
||||
entityId: entity.entityId,
|
||||
entityType: entity.entityType,
|
||||
requestId: tracking.requestId
|
||||
});
|
||||
}
|
||||
} catch (rollbackError: unknown) {
|
||||
const rollbackMessage = rollbackError instanceof Error ? rollbackError.message : 'Unknown rollback error';
|
||||
edgeLogger.error('Rollback exception', {
|
||||
action: 'approval_rollback_exception',
|
||||
entityId: entity.entityId,
|
||||
error: rollbackMessage,
|
||||
requestId: tracking.requestId
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
edgeLogger.info('Rollback complete', {
|
||||
action: 'approval_rollback_complete',
|
||||
deletedCount: createdEntities.length,
|
||||
requestId: tracking.requestId
|
||||
});
|
||||
}
|
||||
|
||||
// Break the loop - don't process remaining items
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
// Check if any item failed - if so, return early with failure
|
||||
const failedResults = approvalResults.filter(r => !r.success);
|
||||
if (failedResults.length > 0) {
|
||||
const failedItem = failedResults[0];
|
||||
edgeLogger.error('Approval failed - transaction rolled back', {
|
||||
action: 'approval_transaction_fail',
|
||||
failedItemId: failedItem.itemId,
|
||||
failedItemType: failedItem.itemType,
|
||||
error: failedItem.error,
|
||||
rolledBackEntities: createdEntities.length,
|
||||
requestId: tracking.requestId
|
||||
});
|
||||
|
||||
const duration = endRequest(tracking);
|
||||
return new Response(
|
||||
JSON.stringify({
|
||||
success: false,
|
||||
message: 'Approval failed and all changes have been rolled back',
|
||||
error: failedItem.error,
|
||||
failedItemId: failedItem.itemId,
|
||||
failedItemType: failedItem.itemType,
|
||||
isDependencyFailure: failedItem.isDependencyFailure,
|
||||
rolledBackEntities: createdEntities.length,
|
||||
requestId: tracking.requestId,
|
||||
duration
|
||||
}),
|
||||
{
|
||||
status: 500,
|
||||
headers: {
|
||||
...corsHeaders,
|
||||
'Content-Type': 'application/json',
|
||||
'X-Request-ID': tracking.requestId
|
||||
}
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
// All items succeeded - proceed with batch updates
|
||||
// Batch update all approved items
|
||||
const approvedItemIds = approvalResults.filter(r => r.success).map(r => r.itemId);
|
||||
if (approvedItemIds.length > 0) {
|
||||
|
||||
@@ -0,0 +1,113 @@
|
||||
-- Create submission_idempotency_keys table for preventing duplicate approvals
|
||||
CREATE TABLE public.submission_idempotency_keys (
|
||||
id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
|
||||
idempotency_key TEXT NOT NULL,
|
||||
submission_id UUID NOT NULL REFERENCES content_submissions(id) ON DELETE CASCADE,
|
||||
moderator_id UUID NOT NULL,
|
||||
item_ids JSONB NOT NULL,
|
||||
|
||||
-- Result caching
|
||||
status TEXT NOT NULL DEFAULT 'processing',
|
||||
result_data JSONB,
|
||||
error_message TEXT,
|
||||
|
||||
-- Tracking
|
||||
created_at TIMESTAMPTZ NOT NULL DEFAULT now(),
|
||||
completed_at TIMESTAMPTZ,
|
||||
expires_at TIMESTAMPTZ NOT NULL DEFAULT (now() + interval '24 hours'),
|
||||
|
||||
-- Request metadata
|
||||
request_id TEXT,
|
||||
trace_id TEXT,
|
||||
duration_ms INTEGER,
|
||||
|
||||
CONSTRAINT unique_idempotency_key UNIQUE (idempotency_key, moderator_id),
|
||||
CONSTRAINT valid_status CHECK (status IN ('processing', 'completed', 'failed'))
|
||||
);
|
||||
|
||||
COMMENT ON TABLE public.submission_idempotency_keys IS 'Prevents duplicate entity creation from rapid clicking or network retries';
|
||||
COMMENT ON COLUMN public.submission_idempotency_keys.idempotency_key IS 'Client-provided or generated unique key for the approval request';
|
||||
COMMENT ON COLUMN public.submission_idempotency_keys.item_ids IS 'JSONB array of submission item IDs being approved';
|
||||
COMMENT ON COLUMN public.submission_idempotency_keys.result_data IS 'Cached response for completed requests (returned on duplicate)';
|
||||
COMMENT ON COLUMN public.submission_idempotency_keys.expires_at IS 'Keys expire after 24 hours';
|
||||
|
||||
-- Primary lookup index
|
||||
CREATE INDEX idx_idempotency_keys_lookup
|
||||
ON submission_idempotency_keys(idempotency_key, moderator_id, expires_at);
|
||||
|
||||
-- Cleanup/expiration index
|
||||
CREATE INDEX idx_idempotency_keys_expiration
|
||||
ON submission_idempotency_keys(expires_at);
|
||||
|
||||
-- Analytics index
|
||||
CREATE INDEX idx_idempotency_keys_submission
|
||||
ON submission_idempotency_keys(submission_id, created_at DESC);
|
||||
|
||||
-- Status monitoring index (only index processing items)
|
||||
CREATE INDEX idx_idempotency_keys_status
|
||||
ON submission_idempotency_keys(status, created_at)
|
||||
WHERE status = 'processing';
|
||||
|
||||
-- Enable RLS
|
||||
ALTER TABLE submission_idempotency_keys ENABLE ROW LEVEL SECURITY;
|
||||
|
||||
-- Moderators can view their own keys
|
||||
CREATE POLICY "Moderators view own idempotency keys"
|
||||
ON submission_idempotency_keys FOR SELECT
|
||||
USING (
|
||||
moderator_id = auth.uid()
|
||||
AND is_moderator(auth.uid())
|
||||
);
|
||||
|
||||
-- System (edge function with service role) can insert keys
|
||||
CREATE POLICY "System can insert idempotency keys"
|
||||
ON submission_idempotency_keys FOR INSERT
|
||||
WITH CHECK (true);
|
||||
|
||||
-- System can update keys (status transitions)
|
||||
CREATE POLICY "System can update idempotency keys"
|
||||
ON submission_idempotency_keys FOR UPDATE
|
||||
USING (true);
|
||||
|
||||
-- Admins can view all keys for debugging
|
||||
CREATE POLICY "Admins view all idempotency keys"
|
||||
ON submission_idempotency_keys FOR SELECT
|
||||
USING (
|
||||
has_role(auth.uid(), 'admin')
|
||||
OR has_role(auth.uid(), 'superuser')
|
||||
);
|
||||
|
||||
-- Function to clean up expired keys
|
||||
CREATE OR REPLACE FUNCTION cleanup_expired_idempotency_keys()
|
||||
RETURNS INTEGER
|
||||
LANGUAGE plpgsql
|
||||
SECURITY DEFINER
|
||||
AS $$
|
||||
DECLARE
|
||||
deleted_count INTEGER;
|
||||
BEGIN
|
||||
DELETE FROM submission_idempotency_keys
|
||||
WHERE expires_at < now() - interval '1 hour';
|
||||
|
||||
GET DIAGNOSTICS deleted_count = ROW_COUNT;
|
||||
|
||||
RETURN deleted_count;
|
||||
END;
|
||||
$$;
|
||||
|
||||
COMMENT ON FUNCTION cleanup_expired_idempotency_keys() IS
|
||||
'Deletes idempotency keys that expired more than 1 hour ago. Run via pg_cron or scheduled job.';
|
||||
|
||||
-- Create monitoring view for analytics
|
||||
CREATE OR REPLACE VIEW idempotency_stats AS
|
||||
SELECT
|
||||
DATE_TRUNC('hour', created_at) AS hour,
|
||||
status,
|
||||
COUNT(*) AS total_requests,
|
||||
COUNT(DISTINCT moderator_id) AS unique_moderators,
|
||||
AVG(duration_ms) AS avg_duration_ms,
|
||||
PERCENTILE_CONT(0.95) WITHIN GROUP (ORDER BY duration_ms) AS p95_duration_ms
|
||||
FROM submission_idempotency_keys
|
||||
WHERE created_at > now() - interval '7 days'
|
||||
GROUP BY DATE_TRUNC('hour', created_at), status
|
||||
ORDER BY hour DESC, status;
|
||||
@@ -0,0 +1,48 @@
|
||||
-- Fix security warnings for idempotency system
|
||||
|
||||
-- 1. Fix Function Search Path: Add explicit search_path to cleanup function
|
||||
CREATE OR REPLACE FUNCTION cleanup_expired_idempotency_keys()
|
||||
RETURNS INTEGER
|
||||
LANGUAGE plpgsql
|
||||
SECURITY DEFINER
|
||||
SET search_path TO 'public'
|
||||
AS $$
|
||||
DECLARE
|
||||
deleted_count INTEGER;
|
||||
BEGIN
|
||||
DELETE FROM submission_idempotency_keys
|
||||
WHERE expires_at < now() - interval '1 hour';
|
||||
|
||||
GET DIAGNOSTICS deleted_count = ROW_COUNT;
|
||||
|
||||
RETURN deleted_count;
|
||||
END;
|
||||
$$;
|
||||
|
||||
-- 2. Fix Security Definer View: Add RLS to idempotency_stats view
|
||||
-- Drop and recreate with proper security
|
||||
DROP VIEW IF EXISTS idempotency_stats;
|
||||
|
||||
CREATE VIEW idempotency_stats
|
||||
WITH (security_invoker=true)
|
||||
AS
|
||||
SELECT
|
||||
DATE_TRUNC('hour', created_at) AS hour,
|
||||
status,
|
||||
COUNT(*) AS total_requests,
|
||||
COUNT(DISTINCT moderator_id) AS unique_moderators,
|
||||
AVG(duration_ms) AS avg_duration_ms,
|
||||
PERCENTILE_CONT(0.95) WITHIN GROUP (ORDER BY duration_ms) AS p95_duration_ms
|
||||
FROM submission_idempotency_keys
|
||||
WHERE created_at > now() - interval '7 days'
|
||||
GROUP BY DATE_TRUNC('hour', created_at), status
|
||||
ORDER BY hour DESC, status;
|
||||
|
||||
COMMENT ON VIEW idempotency_stats IS 'Monitoring view for idempotency key performance and usage statistics (admin/moderator access only via RLS)';
|
||||
|
||||
-- Enable RLS on the view
|
||||
ALTER VIEW idempotency_stats SET (security_invoker=true);
|
||||
|
||||
-- Add RLS policy for the view (admins and moderators only)
|
||||
-- Note: Views use the underlying table's RLS, so moderators/admins who can access
|
||||
-- submission_idempotency_keys can access this view
|
||||
Reference in New Issue
Block a user