Fix error propagation in forms

docs/FORM_SUBMISSION_PATTERNS.md

@@ -0,0 +1,281 @@
+# Form Submission Patterns
+
+## Overview
+This document defines the standard patterns for handling form submissions, toast notifications, and modal behavior across ThrillWiki.
+
+## Core Principles
+
+### Separation of Concerns
+- **Forms** handle UI, validation, and data collection
+- **Parent Pages** handle submission logic and user feedback
+- **Submission Helpers** handle database operations
+
+### Single Source of Truth
+- Only parent pages show success toasts
+- Forms should not assume submission outcomes
+- Modal closing is controlled by parent after successful submission
+
+## Toast Notification Rules
+
+### ✅ DO
+
+**Parent Pages Show Toasts**
+```typescript
+const handleParkSubmit = async (data: FormData) => {
+  try {
+    await submitParkCreation(data, user.id);
+    
+    toast({
+      title: "Park Submitted",
+      description: "Your submission has been sent for review."
+    });
+    
+    setIsModalOpen(false); // Close modal after success
+  } catch (error) {
+    // Error already handled by form via handleError utility
+  }
+};
+```
+
+**Use Correct Terminology**
+- ✅ "Submitted for review" (for new entities)
+- ✅ "Edit submitted" (for updates)
+- ❌ "Created" or "Updated" (implies immediate approval)
+
+**Conditional Toast in Forms (Only for standalone usage)**
+```typescript
+// Only show toast if NOT being called from a parent handler
+if (!initialData?.id) {
+  toast.success('Designer submitted for review');
+  onCancel();
+}
+```
+
+### ❌ DON'T
+
+**Forms Should NOT Show Success Toasts for Main Submissions**
+```typescript
+// ❌ WRONG - Form doesn't know if submission succeeded
+const handleFormSubmit = async (data: FormData) => {
+  await onSubmit(data);
+  
+  toast({
+    title: "Park Created", // ❌ Misleading terminology
+    description: "The new park has been created successfully."
+  });
+};
+```
+
+**Duplicate Toasts**
+```typescript
+// ❌ WRONG - Both form and parent showing toasts
+// Form:
+toast({ title: "Park Created" });
+
+// Parent:
+toast({ title: "Park Submitted" });
+```
+
+## Modal Behavior
+
+### Expected Flow
+1. User fills form and clicks submit
+2. Form validates and calls `onSubmit` prop
+3. Parent page handles submission
+4. Parent shows appropriate toast
+5. Parent closes modal via `setIsModalOpen(false)`
+
+### Common Issues
+
+**Issue**: Modal doesn't close after submission
+**Cause**: Form is showing a toast that interferes with normal flow
+**Solution**: Remove form-level success toasts
+
+**Issue**: User sees "Created" but item isn't visible
+**Cause**: Using wrong terminology - submissions go to moderation
+**Solution**: Use "Submitted for review" instead of "Created"
+
+## Form Component Template
+
+```typescript
+export function EntityForm({ onSubmit, onCancel, initialData }: EntityFormProps) {
+  const { user } = useAuth();
+  
+  const { register, handleSubmit, /* ... */ } = useForm({
+    // ... form config
+  });
+
+  return (
+    <form onSubmit={handleSubmit(async (data) => {
+      if (!user) {
+        toast.error('You must be logged in to submit');
+        return;
+      }
+      
+      try {
+        await onSubmit(data);
+        
+        // ⚠️ NO SUCCESS TOAST HERE - parent handles it
+        // Exception: Standalone forms not in modals can show toast
+      } catch (error: unknown) {
+        handleError(error, {
+          action: initialData?.id ? 'Update Entity' : 'Create Entity',
+          metadata: { entityName: data.name }
+        });
+        
+        // ⚠️ CRITICAL: Re-throw so parent can handle modal state
+        throw error;
+      }
+    })}>
+      {/* Form fields */}
+    </form>
+  );
+}
+```
+
+## Parent Page Template
+
+```typescript
+export function EntityListPage() {
+  const [isModalOpen, setIsModalOpen] = useState(false);
+
+  const handleEntitySubmit = async (data: FormData) => {
+    try {
+      const result = await submitEntityCreation(data, user.id);
+      
+      // ✅ Parent shows success feedback
+      toast({
+        title: "Entity Submitted",
+        description: "Your submission has been sent for review."
+      });
+      
+      // ✅ Parent closes modal
+      setIsModalOpen(false);
+      
+      // ✅ Parent refreshes data
+      queryClient.invalidateQueries(['entities']);
+    } catch (error) {
+      // Form already showed error via handleError
+      // Parent can optionally add additional handling
+      console.error('Submission failed:', error);
+    }
+  };
+
+  return (
+    <>
+      <Button onClick={() => setIsModalOpen(true)}>
+        Add Entity
+      </Button>
+      
+      <Dialog open={isModalOpen} onOpenChange={setIsModalOpen}>
+        <EntityForm 
+          onSubmit={handleEntitySubmit}
+          onCancel={() => setIsModalOpen(false)}
+        />
+      </Dialog>
+    </>
+  );
+}
+```
+
+## Error Handling
+
+### ⚠️ CRITICAL: Error Propagation Pattern
+
+Forms MUST re-throw errors after logging them so parent components can respond appropriately (keep modals open, show additional context, etc.).
+
+**Forms MUST re-throw errors:**
+```typescript
+} catch (error: unknown) {
+  // Log error for debugging and show toast to user
+  handleError(error, {
+    action: 'Submit Park',
+    userId: user?.id,
+    metadata: { parkName: data.name }
+  });
+  
+  // ⚠️ CRITICAL: Re-throw so parent can handle modal state
+  throw error;
+}
+```
+
+**Why Re-throw?**
+- Parent needs to know submission failed
+- Modal should stay open so user can retry
+- User can fix validation issues and resubmit
+- Prevents "success" behavior on failures
+- Maintains proper error flow through the app
+
+### Parent-Level Error Handling
+
+```typescript
+const handleParkSubmit = async (data: FormData) => {
+  try {
+    await submitParkCreation(data, user.id);
+    toast.success('Park submitted for review');
+    setIsModalOpen(false); // Only close on success
+  } catch (error) {
+    // Error already toasted by form via handleError()
+    // Modal stays open automatically because we don't close it
+    // User can fix issues and retry
+    console.error('Submission failed:', error);
+  }
+};
+```
+
+**Expected Error Flow:**
+1. User submits form → `onSubmit()` called
+2. Submission fails → Form catches error
+3. Form shows error toast via `handleError()`
+4. Form re-throws error to parent
+5. Parent's catch block executes
+6. Modal stays open (no `setIsModalOpen(false)`)
+7. User fixes issue and tries again
+
+**Common Mistake:**
+```typescript
+// ❌ WRONG - Error not re-thrown, parent never knows
+} catch (error: unknown) {
+  handleError(error, { action: 'Submit' });
+  // Missing: throw error;
+}
+```
+
+## Current Implementation Status
+
+### ✅ Correct Implementation
+- `DesignerForm.tsx` - Shows "Designer submitted for review" only when `!initialData?.id`
+- `OperatorForm.tsx` - Shows "Operator submitted for review" only when `!initialData?.id`
+- `PropertyOwnerForm.tsx` - Shows "Property owner submitted for review" only when `!initialData?.id`
+- `ManufacturerForm.tsx` - Shows "Manufacturer submitted for review" only when `!initialData?.id`
+- `RideModelForm.tsx` - No toasts, parent handles everything
+- `RideForm.tsx` - Shows "Submission Sent" with conditional description
+- `ParkForm.tsx` - Fixed to remove premature success toast
+
+### Parent Pages
+- `Parks.tsx` - Shows "Park Submitted" ✅
+- `Operators.tsx` - Shows "Operator Submitted" ✅
+- `Designers.tsx` - Shows "Designer Submitted" ✅
+- `Manufacturers.tsx` - Shows "Manufacturer Submitted" ✅
+- `ParkDetail.tsx` - Shows "Submission Sent" ✅
+
+## Testing Checklist
+
+When implementing or updating a form:
+
+- [ ] Form validates input correctly
+- [ ] Form calls `onSubmit` prop with clean data
+- [ ] Form only shows error toasts, not success toasts (unless standalone)
+- [ ] Parent page shows appropriate success toast
+- [ ] Success toast uses correct terminology ("submitted" not "created")
+- [ ] Modal closes after successful submission
+- [ ] User sees single toast, not duplicates
+- [ ] Error handling provides actionable feedback
+- [ ] Form can be used both in modals and standalone
+
+## Related Files
+
+- `src/lib/errorHandler.ts` - Error handling utilities
+- `src/lib/entitySubmissionHelpers.ts` - Submission logic
+- `src/hooks/use-toast.ts` - Toast notification hook
+- `tests/e2e/submission/park-creation.spec.ts` - E2E tests for submission flow

src/components/admin/DesignerForm.tsx

@@ -94,6 +94,9 @@ export function DesignerForm({ onSubmit, onCancel, initialData }: DesignerFormPr
               action: initialData?.id ? 'Update Designer' : 'Create Designer',
               metadata: { companyName: data.name }
             });
+            
+            // Re-throw so parent can handle modal closing
+            throw error;
           }
         })} className="space-y-6">
           {/* Basic Information */}

src/components/admin/ManufacturerForm.tsx

@@ -98,6 +98,9 @@ export function ManufacturerForm({ onSubmit, onCancel, initialData }: Manufactur
               action: initialData?.id ? 'Update Manufacturer' : 'Create Manufacturer',
               metadata: { companyName: data.name }
             });
+            
+            // Re-throw so parent can handle modal closing
+            throw error;
           }
         })} className="space-y-6">
           {/* Basic Information */}

src/components/admin/OperatorForm.tsx

@@ -94,6 +94,9 @@ export function OperatorForm({ onSubmit, onCancel, initialData }: OperatorFormPr
               action: initialData?.id ? 'Update Operator' : 'Create Operator',
               metadata: { companyName: data.name }
             });
+            
+            // Re-throw so parent can handle modal closing
+            throw error;
           }
         })} className="space-y-6">
           {/* Basic Information */}

src/components/admin/ParkForm.tsx

@@ -261,12 +261,7 @@ export function ParkForm({ onSubmit, onCancel, initialData, isEditing = false }:
         _compositeSubmission: (tempNewOperator || tempNewPropertyOwner) ? submissionContent : undefined
       });
       
-      toast({
-        title: isEditing ? "Park Updated" : "Park Created",
-        description: isEditing 
-          ? "The park information has been updated successfully."
-          : "The new park has been created successfully."
-      });
+      // Parent component handles success feedback
     } catch (error: unknown) {
       const errorMessage = getErrorMessage(error);
       handleError(error, {
@@ -279,6 +274,9 @@ export function ParkForm({ onSubmit, onCancel, initialData, isEditing = false }:
           hasNewOwner: !!tempNewPropertyOwner
         }
       });
+      
+      // Re-throw so parent can handle modal closing
+      throw error;
     }
   };
 

src/components/admin/PropertyOwnerForm.tsx

@@ -94,6 +94,9 @@ export function PropertyOwnerForm({ onSubmit, onCancel, initialData }: PropertyO
               action: initialData?.id ? 'Update Property Owner' : 'Create Property Owner',
               metadata: { companyName: data.name }
             });
+            
+            // Re-throw so parent can handle modal closing
+            throw error;
           }
         })} className="space-y-6">
           {/* Basic Information */}

src/components/admin/RideForm.tsx

@@ -352,6 +352,9 @@ export function RideForm({ onSubmit, onCancel, initialData, isEditing = false }:
           hasNewModel: !!tempNewRideModel
         }
       });
+      
+      // Re-throw so parent can handle modal closing
+      throw error;
     }
   };
 

src/components/admin/RideModelForm.tsx

@@ -113,6 +113,9 @@ export function RideModelForm({
       handleError(error, {
         action: initialData?.id ? 'Update Ride Model' : 'Create Ride Model'
       });
+      
+      // Re-throw so parent can handle modal closing
+      throw error;
     }
   };