Improve image handling, optimize hooks, and add rate limiting

This commit introduces several improvements:
- Enhances `RideModelCard` by safely accessing and displaying ride count and image data, preventing potential errors.
- Refactors `useEntityVersions` and `useSearch` hooks to use `useCallback` and improve performance and prevent race conditions.
- Introduces a `MAX_MAP_SIZE` and cleanup mechanism for the rate limiting map in `detect-location` Supabase function to prevent memory leaks.
- Adds robust error handling and cleanup for image uploads in `uploadPendingImages`.
- Modifies `ManufacturerModels` to correctly map and display ride counts.
- Includes error handling for topological sort in `process-selective-approval` Supabase function.

Replit-Commit-Author: Agent
Replit-Commit-Session-Id: 39bb006b-d046-477f-a1f9-b7821836f3a1
Replit-Commit-Checkpoint-Type: intermediate_checkpoint

src/components/rides/RideModelCard.tsx

@@ -25,16 +25,25 @@ export function RideModelCard({ model, manufacturerSlug }: RideModelCardProps) {
     ).join(' ');
   };
 
-  const rideCount = (model as any).rides?.[0]?.count || 0;
+  // Safely extract ride count and image data
+  const extendedModel = model as RideModel & { 
+    ride_count?: number;
+    card_image_url?: string;
+    card_image_id?: string;
+  };
+  
+  const rideCount = extendedModel.ride_count || 0;
+  const cardImageUrl = extendedModel.card_image_url;
+  const cardImageId = extendedModel.card_image_id;
 
   return (
     <Card className="overflow-hidden hover:shadow-lg transition-shadow cursor-pointer group">
       <div 
         className="aspect-video bg-gradient-to-br from-primary/10 via-secondary/10 to-accent/10 relative overflow-hidden"
       >
-        {((model as any).card_image_url || (model as any).card_image_id) ? (
+        {(cardImageUrl || cardImageId) ? (
           <img 
-            src={(model as any).card_image_url || `https://imagedelivery.net/${import.meta.env.VITE_CLOUDFLARE_ACCOUNT_HASH}/${(model as any).card_image_id}/public`}
+            src={cardImageUrl || `https://imagedelivery.net/${import.meta.env.VITE_CLOUDFLARE_ACCOUNT_HASH}/${cardImageId}/public`}
             alt={model.name}
             className="w-full h-full object-cover group-hover:scale-105 transition-transform duration-300"
           />

src/hooks/useEntityVersions.ts

@@ -1,4 +1,4 @@
-import { useState, useEffect, useRef } from 'react';
+import { useState, useEffect, useRef, useCallback } from 'react';
 import { supabase } from '@/integrations/supabase/client';
 import { toast } from 'sonner';
 
@@ -43,9 +43,14 @@ export function useEntityVersions(entityType: string, entityId: string) {
   // Track the current channel to prevent duplicate subscriptions
   const channelRef = useRef<ReturnType<typeof supabase.channel> | null>(null);
   
-  const fetchVersions = async () => {
+  // Track if a fetch is in progress to prevent race conditions
+  const fetchInProgressRef = useRef(false);
+
+  const fetchVersions = useCallback(async () => {
     try {
-      if (!isMountedRef.current) return;
+      if (!isMountedRef.current || fetchInProgressRef.current) return;
+      
+      fetchInProgressRef.current = true;
       
       setLoading(true);
       
@@ -87,11 +92,12 @@ export function useEntityVersions(entityType: string, entityId: string) {
         toast.error('Failed to load version history');
       }
     } finally {
+      fetchInProgressRef.current = false;
       if (isMountedRef.current) {
         setLoading(false);
       }
     }
-  };
+  }, [entityType, entityId]);
 
   const fetchFieldHistory = async (versionId: string) => {
     try {
@@ -195,7 +201,7 @@ export function useEntityVersions(entityType: string, entityId: string) {
     if (entityType && entityId) {
       fetchVersions();
     }
-  }, [entityType, entityId]);
+  }, [entityType, entityId, fetchVersions]);
 
   // Set up realtime subscription for version changes
   useEffect(() => {

src/hooks/useSearch.tsx

@@ -1,4 +1,4 @@
-import { useState, useEffect, useMemo } from 'react';
+import { useState, useEffect, useMemo, useCallback } from 'react';
 import { supabase } from '@/integrations/supabase/client';
 import { Park, Ride, Company } from '@/types/database';
 
@@ -20,13 +20,23 @@ interface UseSearchOptions {
   debounceMs?: number;
 }
 
+// Hoist default values to prevent recreating on every render
+const DEFAULT_TYPES: ('park' | 'ride' | 'company')[] = ['park', 'ride', 'company'];
+const DEFAULT_LIMIT = 10;
+const DEFAULT_MIN_QUERY = 2;
+const DEFAULT_DEBOUNCE_MS = 300;
+
 export function useSearch(options: UseSearchOptions = {}) {
-  const {
+  // Stabilize options using JSON stringify to prevent infinite loops from array recreation
-    types = ['park', 'ride', 'company'],
+  const optionsKey = JSON.stringify({
-    limit = 10,
+    types: options.types || DEFAULT_TYPES,
-    minQuery = 2,
+    limit: options.limit || DEFAULT_LIMIT,
-    debounceMs = 300
+    minQuery: options.minQuery || DEFAULT_MIN_QUERY,
-  } = options;
+    debounceMs: options.debounceMs || DEFAULT_DEBOUNCE_MS
+  });
+
+  const stableOptions = useMemo(() => JSON.parse(optionsKey), [optionsKey]);
+  const { types, limit, minQuery, debounceMs } = stableOptions;
 
   const [query, setQuery] = useState('');
   const [results, setResults] = useState<SearchResult[]>([]);
@@ -61,7 +71,7 @@ export function useSearch(options: UseSearchOptions = {}) {
   }, []);
 
   // Search function
-  const search = async (searchQuery: string) => {
+  const search = useCallback(async (searchQuery: string) => {
     if (searchQuery.length < minQuery) {
       setResults([]);
       return;
@@ -162,7 +172,7 @@ export function useSearch(options: UseSearchOptions = {}) {
     } finally {
       setLoading(false);
     }
-  };
+  }, [types, limit, minQuery]);
 
   // Effect for debounced search
   useEffect(() => {
@@ -171,7 +181,7 @@ export function useSearch(options: UseSearchOptions = {}) {
     } else {
       setResults([]);
     }
-  }, [debouncedQuery]);
+  }, [debouncedQuery, search]);
 
   // Save search to recent searches
   const saveSearch = (searchQuery: string) => {

src/hooks/useUsernameValidation.ts

@@ -1,4 +1,4 @@
-import { useState, useEffect } from 'react';
+import { useState, useEffect, useCallback } from 'react';
 import { supabase } from '@/integrations/supabase/client';
 import { usernameSchema } from '@/lib/validation';
 import { useDebounce } from './useDebounce';
@@ -20,6 +20,33 @@ export function useUsernameValidation(username: string, currentUsername?: string
 
   const debouncedUsername = useDebounce(username, 500);
 
+  const checkUsernameAvailability = useCallback(async (normalizedUsername: string) => {
+    try {
+      const { data, error } = await supabase
+        .from('profiles')
+        .select('username')
+        .eq('username', normalizedUsername)
+        .maybeSingle();
+
+      if (error) throw error;
+
+      const isAvailable = !data;
+      setState({
+        isValid: isAvailable,
+        isAvailable,
+        isChecking: false,
+        error: isAvailable ? null : 'Username is already taken',
+      });
+    } catch (error) {
+      setState({
+        isValid: false,
+        isAvailable: null,
+        isChecking: false,
+        error: 'Error checking username availability',
+      });
+    }
+  }, []);
+
   useEffect(() => {
     if (!debouncedUsername || debouncedUsername === currentUsername) {
       setState({
@@ -47,34 +74,7 @@ export function useUsernameValidation(username: string, currentUsername?: string
     setState(prev => ({ ...prev, isChecking: true, error: null }));
     
     checkUsernameAvailability(validation.data);
-  }, [debouncedUsername, currentUsername]);
+  }, [debouncedUsername, currentUsername, checkUsernameAvailability]);
-
-  const checkUsernameAvailability = async (normalizedUsername: string) => {
-    try {
-      const { data, error } = await supabase
-        .from('profiles')
-        .select('username')
-        .eq('username', normalizedUsername)
-        .maybeSingle();
-
-      if (error) throw error;
-
-      const isAvailable = !data;
-      setState({
-        isValid: isAvailable,
-        isAvailable,
-        isChecking: false,
-        error: isAvailable ? null : 'Username is already taken',
-      });
-    } catch (error) {
-      setState({
-        isValid: false,
-        isAvailable: null,
-        isChecking: false,
-        error: 'Error checking username availability',
-      });
-    }
-  };
 
   return state;
 }

src/lib/imageUploadHelper.ts

@@ -16,18 +16,19 @@ export interface CloudflareUploadResponse {
  */
 export async function uploadPendingImages(images: UploadedImage[]): Promise<UploadedImage[]> {
   const uploadedImages: UploadedImage[] = [];
+  const newlyUploadedIds: string[] = []; // Track newly uploaded IDs for cleanup on error
+  let currentImageIndex = 0;
 
+  try {
     for (const image of images) {
       if (image.isLocal && image.file) {
-      try {
         // Step 1: Get upload URL from our Supabase Edge Function
         const { data: uploadUrlData, error: urlError } = await supabase.functions.invoke('upload-image', {
           body: { action: 'get-upload-url' }
         });
 
         if (urlError || !uploadUrlData?.uploadURL) {
-          console.error('Error getting upload URL:', urlError);
+          throw new Error(`Failed to get upload URL: ${urlError?.message || 'Unknown error'}`);
-          throw new Error('Failed to get upload URL from Cloudflare');
         }
 
         // Step 2: Upload file directly to Cloudflare
@@ -40,7 +41,8 @@ export async function uploadPendingImages(images: UploadedImage[]): Promise<Uplo
         });
 
         if (!uploadResponse.ok) {
-          throw new Error(`Upload failed with status: ${uploadResponse.status}`);
+          const errorText = await uploadResponse.text();
+          throw new Error(`Upload failed (status ${uploadResponse.status}): ${errorText}`);
         }
 
         const result: CloudflareUploadResponse = await uploadResponse.json();
@@ -49,6 +51,9 @@ export async function uploadPendingImages(images: UploadedImage[]): Promise<Uplo
           throw new Error('Cloudflare upload returned unsuccessful response');
         }
 
+        // Track this newly uploaded image
+        newlyUploadedIds.push(result.result.id);
+
         // Step 3: Return uploaded image metadata
         uploadedImages.push({
           url: result.result.variants[0], // Use first variant (usually the original)
@@ -59,10 +64,6 @@ export async function uploadPendingImages(images: UploadedImage[]): Promise<Uplo
 
         // Clean up object URL
         URL.revokeObjectURL(image.url);
-      } catch (error) {
-        console.error('Error uploading image:', error);
-        throw new Error(`Failed to upload image: ${error instanceof Error ? error.message : 'Unknown error'}`);
-      }
       } else {
         // Already uploaded, keep as is
         uploadedImages.push({
@@ -72,7 +73,28 @@ export async function uploadPendingImages(images: UploadedImage[]): Promise<Uplo
           isLocal: false,
         });
       }
+      currentImageIndex++;
     }
 
     return uploadedImages;
+  } catch (error) {
+    // Cleanup: Attempt to delete newly uploaded images on error
+    if (newlyUploadedIds.length > 0) {
+      console.error(`Upload failed at image ${currentImageIndex + 1}. Cleaning up ${newlyUploadedIds.length} uploaded images...`);
+      
+      // Attempt cleanup but don't throw if it fails
+      for (const imageId of newlyUploadedIds) {
+        try {
+          await supabase.functions.invoke('upload-image', {
+            body: { action: 'delete', imageId }
+          });
+        } catch (cleanupError) {
+          console.error(`Failed to cleanup image ${imageId}:`, cleanupError);
+        }
+      }
+    }
+
+    const errorMessage = error instanceof Error ? error.message : 'Unknown error';
+    throw new Error(`Failed to upload image ${currentImageIndex + 1} of ${images.length}: ${errorMessage}`);
+  }
 }

src/pages/ManufacturerModels.tsx

@@ -57,7 +57,14 @@ export default function ManufacturerModels() {
 
         const { data: modelsData, error: modelsError } = await query;
         if (modelsError) throw modelsError;
-        setModels((modelsData || []) as any);
+        
+        // Transform data to include ride count
+        const modelsWithCounts = (modelsData || []).map(model => ({
+          ...model,
+          ride_count: Array.isArray(model.rides) ? model.rides[0]?.count || 0 : 0
+        }));
+        
+        setModels(modelsWithCounts as RideModel[]);
       }
     } catch (error) {
       console.error('Error fetching data:', error);

supabase/functions/detect-location/index.ts

@@ -15,12 +15,39 @@ interface IPLocationResponse {
 const rateLimitMap = new Map<string, { count: number; resetAt: number }>();
 const RATE_LIMIT_WINDOW = 60000; // 1 minute in milliseconds
 const MAX_REQUESTS = 10; // 10 requests per minute per IP
+const MAX_MAP_SIZE = 10000; // Maximum number of IPs to track
+
+function cleanupExpiredEntries() {
+  const now = Date.now();
+  for (const [ip, data] of rateLimitMap.entries()) {
+    if (now > data.resetAt) {
+      rateLimitMap.delete(ip);
+    }
+  }
+}
 
 function checkRateLimit(ip: string): { allowed: boolean; retryAfter?: number } {
   const now = Date.now();
   const existing = rateLimitMap.get(ip);
 
   if (!existing || now > existing.resetAt) {
+    // If map is too large, clean up expired entries first
+    if (rateLimitMap.size >= MAX_MAP_SIZE) {
+      cleanupExpiredEntries();
+      
+      // If still too large after cleanup, clear oldest entries
+      if (rateLimitMap.size >= MAX_MAP_SIZE) {
+        const toDelete = Math.floor(MAX_MAP_SIZE * 0.2); // Remove 20% of entries
+        let deleted = 0;
+        for (const key of rateLimitMap.keys()) {
+          if (deleted >= toDelete) break;
+          rateLimitMap.delete(key);
+          deleted++;
+        }
+        console.warn(`Rate limit map reached size limit. Cleared ${deleted} entries.`);
+      }
+    }
+    
     // Create new entry or reset expired entry
     rateLimitMap.set(ip, { count: 1, resetAt: now + RATE_LIMIT_WINDOW });
     return { allowed: true };
@@ -36,14 +63,7 @@ function checkRateLimit(ip: string): { allowed: boolean; retryAfter?: number } {
 }
 
 // Clean up old entries periodically to prevent memory leak
-setInterval(() => {
+setInterval(cleanupExpiredEntries, RATE_LIMIT_WINDOW);
-  const now = Date.now();
-  for (const [ip, data] of rateLimitMap.entries()) {
-    if (now > data.resetAt) {
-      rateLimitMap.delete(ip);
-    }
-  }
-}, RATE_LIMIT_WINDOW);
 
 serve(async (req) => {
   // Handle CORS preflight requests

supabase/functions/process-selective-approval/index.ts

@@ -170,7 +170,22 @@ serve(async (req) => {
     const submitterId = submission.user_id;
 
     // Topologically sort items by dependencies
-    const sortedItems = topologicalSort(items);
+    let sortedItems;
+    try {
+      sortedItems = topologicalSort(items);
+    } catch (sortError) {
+      const errorMessage = sortError instanceof Error ? sortError.message : 'Failed to sort items';
+      console.error('Topological sort failed:', errorMessage);
+      return new Response(
+        JSON.stringify({ 
+          error: 'Invalid submission structure',
+          message: errorMessage,
+          details: 'The submission contains circular dependencies or missing required items'
+        }),
+        { status: 400, headers: { ...corsHeaders, 'Content-Type': 'application/json' } }
+      );
+    }
+    
     const dependencyMap = new Map<string, string>();
     const approvalResults: Array<{
   itemId: string;