feat: allow admins perform user actions

[prudentbird]

Related Issue

Fixes #58

[claude]

Claude finished @prudentbird's task —— View job

---

Code Review Feedback

I've thoroughly reviewed this PR that adds admin functionality for managing store locations. Here's my comprehensive analysis:

✅ Strengths

1. Consistent Architecture: The code follows established patterns in the codebase with proper separation between server actions, form components, and pages.

2. Type Safety: Comprehensive TypeScript usage with proper type definitions and Zod schema validation.

3. User Experience:

   • Good form validation with real-time feedback
   • Image preview functionality with proper cleanup
   • Geolocation integration for coordinates
   • Loading states and error handling

4. Security: Server actions use proper validation with Zod schemas and handle errors appropriately.

🔧 Code Quality Issues

Performance Concerns

1. Memory Leaks in Image Handling (frontend/src/app/admin/locations/add/form.tsx:62-71, frontend/src/app/admin/locations/edit/[id]/form.tsx:62-71):

   useEffect(() => {
     const url = URL.createObjectURL(file);
     setPreview(url);
     setIsLoading(false);
     return () => {
       if (url) {
         URL.revokeObjectURL(url);
       }
     };
   }, [file]);

   Issue: Object URLs are created but may not be properly cleaned up if component unmounts before the effect runs.

2. Large Component Files: The form components are quite large (783 and 875 lines). Consider breaking them into smaller, reusable components.

Potential Bugs

1. Form State Management (frontend/src/app/admin/locations/add/form.tsx:331-355):

   const handleStateChange = useCallback(
     (value: string) => {
       setSelectedStateId(value);
       const form = formRef.current;
       if (form) {
         const lgSelect = form.querySelector(
           'input[name="local_government_id"]',
         ) as HTMLInputElement;
         if (lgSelect) {
           lgSelect.value = '';
         }
       }

   Issue: Direct DOM manipulation instead of using React state management. This could lead to state inconsistencies.

2. Error Handling in Upload Logic (frontend/src/app/admin/actions.ts:494-510):

   try {
     const uploadResults = await utapi.uploadFiles(validPhotoFiles, {
       concurrency: validPhotoFiles.length,
     });
     uploadedPhotoUrls = uploadResults
       .filter((result) => result.data !== null)
       .map((result) => result.data!.ufsUrl);
   } catch {
     return {
       inputs: rawData,
       message: 'Failed to upload photos. Please try again.',
       timestamp: new Date().toISOString(),
       error: 'Photo upload failed',
     };
   }

   Issue: Catches all errors without logging. Debugging upload failures would be difficult.

Security Considerations

1. File Upload Validation (frontend/src/app/admin/locations/add/form.tsx:267-286):

   • ✅ Good: File type validation (!file.type.startsWith('image/'))
   • ✅ Good: File size limits (4MB)
   • ⚠️ Consider: Server-side file validation is also needed

2. Input Sanitization: The forms rely on client-side validation. Ensure server-side validation is comprehensive.

Code Style Issues

1. Inconsistent Error Handling: Some functions return different error response shapes.

2. Magic Numbers: Hard-coded values like 10 (image limit), 4 * 1024 * 1024 (file size) should be constants.

3. Complex Form Logic: The form components have complex conditional rendering that could be simplified.

🐛 Specific Issues to Address

High Priority

1. Fix Memory Leak in ImagePreview (frontend/src/app/admin/locations/add/form.tsx:62):

   // Current - risky
   const url = URL.createObjectURL(file);
   
   // Better approach
   useEffect(() => {
     let objectUrl: string;
     const createPreview = () => {
       objectUrl = URL.createObjectURL(file);
       setPreview(objectUrl);
       setIsLoading(false);
     };
     createPreview();
     return () => {
       if (objectUrl) {
         URL.revokeObjectURL(objectUrl);
       }
     };
   }, [file]);

2. Replace DOM Manipulation (frontend/src/app/admin/locations/add/form.tsx:334-342):
   Use React state instead of direct DOM queries for form field management.

Medium Priority

3. Add Proper Logging (frontend/src/app/admin/actions.ts:502):

   } catch (uploadError) {
     console.error('Photo upload failed:', uploadError);
     // Return error response
   }

4. Extract Constants:

   const CONSTANTS = {
     MAX_IMAGES: 10,
     MAX_FILE_SIZE: 4 * 1024 * 1024, // 4MB
     GEOLOCATION_TIMEOUT: 10000,
   };

📋 Missing Test Coverage

The PR doesn't include any tests. Consider adding:

• Unit tests for form validation logic
• Integration tests for server actions
• Component tests for form interactions
• E2E tests for the complete add/edit flows

🚀 Performance Suggestions

1. Code Splitting: Consider lazy loading the heavy form components
2. Memoization: Add memo() to pure components
3. Debouncing: Image selection could be debounced to prevent rapid state updates

✨ Minor Improvements

1. Accessibility: Add more ARIA labels and descriptions
2. Internationalization: Prepare strings for i18n if planned
3. Loading States: More granular loading indicators

📝 Overall Assessment

This is a solid implementation that follows good practices overall. The main concerns are around potential memory leaks, complex form state management, and the lack of tests. The functionality appears complete and well-thought-out.

Recommendation: Fix the high-priority issues (memory leaks and DOM manipulation) before merging. The other issues can be addressed in follow-up PRs.

---