Implement orbis forms #25
Conversation
…owser details from db schema
|
Caution Review failedFailed to post review comments WalkthroughIntroduces a comprehensive forms feature to the Orbis application spanning database schema, backend API controllers/routes, and frontend React components for form creation, submission, editing, and response management with multi-user permissions and field-level configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant Backend
participant Database
User->>Frontend: Navigate to Create Form
Frontend->>Frontend: Load FormBuilder
User->>Frontend: Configure form fields & metadata
User->>Frontend: Click Save
Frontend->>Backend: POST /api/forms/createForm
Backend->>Database: Create Form + FormFields (transaction)
Database-->>Backend: Success
Backend-->>Frontend: Return form with id & formUrl
Frontend->>Frontend: Navigate to /forms
User->>Frontend: Share form URL
Note over User,Frontend: Public Form Submission
User->>Frontend: Visit /form/:formUrl
Frontend->>Backend: GET /api/forms/getFormForDisplay
Backend->>Database: Fetch Form + FormFields
Database-->>Backend: Return form data
Backend-->>Frontend: Return form structure
Frontend->>Frontend: Render FormPreview
User->>Frontend: Fill & Submit form
Frontend->>Backend: POST /api/forms/submitFormResponse
Backend->>Database: Create FormResponse + FieldAnswers (transaction)
Database-->>Backend: Success
Backend-->>Frontend: Return submissionResult (editCode if editable)
Frontend->>Frontend: Show success & edit code
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Rationale: Substantial heterogeneous changes spanning database schema (4 migration files with structural table modifications), comprehensive backend controller with 11 functions handling multi-entity operations with transaction logic, and 8+ frontend components with state management and validation. Requires careful review of permission logic, transaction safety, API contract consistency, and field-type validation handling. Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
frontend/src/components/Navbar.jsx (2)
21-32: Outside-click handler never triggers; overlay covers entire viewport.Because the
.mobile-menu-containerspans the full screen,event.target.closest('.mobile-menu-container')is always truthy when the menu is open—so the document click handler can’t close the menu. Either remove the listener or close on overlay clicks while excluding the panel.Suggested fix: remove the document listener and close via overlay onClick; mark the inner panel and stop propagation.
- useEffect(() => { - const handleClickOutside = (event) => { - if (event.target.closest('.mobile-menu-button')) return; - - if (mobileMenuOpen && !event.target.closest('.mobile-menu-container')) { - setMobileMenuOpen(false); - } - }; - - document.addEventListener('click', handleClickOutside); - return () => document.removeEventListener('click', handleClickOutside); - }, [mobileMenuOpen]); + // Close when clicking the translucent overlay (outside the panel) + // No document-level listener needed.- <div - className={`fixed inset-0 bg-white/95 backdrop-blur-md shadow-xl z-10 transition-transform duration-300 md:hidden mobile-menu-container ${ - mobileMenuOpen ? 'translate-x-0' : 'translate-x-full' - }`} - > - <div className="flex flex-col pt-24 px-6 space-y-6 bg-white min-h-screen"> + <div + className={`fixed inset-0 bg-white/95 backdrop-blur-md shadow-xl z-10 transition-transform duration-300 md:hidden mobile-menu-container ${ + mobileMenuOpen ? 'translate-x-0' : 'translate-x-full' + }`} + onClick={() => setMobileMenuOpen(false)} + > + <div + className="flex flex-col pt-24 px-6 space-y-6 bg-white min-h-screen mobile-menu-panel" + onClick={(e) => e.stopPropagation()} + >Also applies to: 108-115
34-36: Also close on querystring changes (not just path/hash).Include
location.searchto catch navigation that only changes query params.- }, [location.pathname, location.hash]); + }, [location.pathname, location.search, location.hash]);backend/server.js (1)
50-69: Await server.close() before disconnecting Prisma and exiting.Current flow calls server.close() but doesn't wait; in‑flight requests may be cut short.
- // Close server - server.close(() => { - console.log('Express server closed'); - }); - - try { + try { + // Close server and wait + await new Promise((resolve, reject) => { + server.close(err => { + if (err) return reject(err); + console.log('Express server closed'); + resolve(); + }); + }); // Disconnect Prisma await prisma.$disconnect(); console.log('Database connection closed'); process.exit(0); } catch (err) {The route mounting concern is incorrect—
backend/src/routes/index.jsis unused; onlybackend/server.js(the actual entry point per package.json) handles routing.
🧹 Nitpick comments (22)
frontend/src/components/Navbar.jsx (1)
61-67: Improve accessibility on mobile menu toggle and dialog.Add ARIA to communicate state and modal semantics.
- <button - onClick={toggleMobileMenu} - className="md:hidden z-20 p-2 mobile-menu-button" - > + <button + onClick={toggleMobileMenu} + className="md:hidden z-20 p-2 mobile-menu-button" + aria-label="Toggle navigation menu" + aria-expanded={mobileMenuOpen} + aria-controls="mobile-menu" + >- <div + <div className={`fixed inset-0 bg-white/95 backdrop-blur-md shadow-xl z-10 transition-transform duration-300 md:hidden mobile-menu-container ${ mobileMenuOpen ? 'translate-x-0' : 'translate-x-full' }`} + id="mobile-menu" + role="dialog" + aria-modal="true" >Also applies to: 108-113
frontend/src/components/FormFieldEditor.jsx (2)
5-19: Small polish: replace all underscores in fallback type name.Ensures nicer display for multi-word types.
- return names[fieldType] || fieldType.replace('_', ' '); + return names[fieldType] || fieldType.replace(/_/g, ' ');
343-355: Add ARIA to the “Required Field” toggle.Communicate state to assistive tech.
- <button - onClick={() => handleFieldChange('isRequired', !field.isRequired)} - className={`relative inline-flex h-6 w-11 items-center rounded-full transition-colors ${ + <button + onClick={() => handleFieldChange('isRequired', !field.isRequired)} + aria-pressed={!!field.isRequired} + aria-label="Toggle required" + className={`relative inline-flex h-6 w-11 items-center rounded-full transition-colors ${ field.isRequired ? 'bg-blue-600' : 'bg-gray-200' }`} >backend/prisma/migrations/20251019021225_form_response_model_changes/migration.sql (1)
13-16: Index for anonymous submission lookups recommended; no code references to dropped columns found.Verification confirms the codebase contains no references to the dropped columns (
metadata,responderIp,userAgent). The Prisma schema correctly reflects the migration changes, with these fields absent andanonymousIdpresent. Adding an index on(formId, anonymousId)remains recommended for query performance on anonymous submissions:-- AlterTable ALTER TABLE "FormResponse" DROP COLUMN "metadata", DROP COLUMN "responderIp", DROP COLUMN "userAgent", ADD COLUMN "anonymousId" TEXT; + +-- Recommended for performance on anonymous submission lookups +CREATE INDEX "FormResponse_formId_anonymousId_idx" ON "FormResponse"("formId","anonymousId");backend/src/routes/form.js (1)
27-31: Add basic rate limiting on public submit/edit/read endpoints.POST/PUT submit and POST responses are public and can be spammed. Add per-IP throttling.
Example (express-rate-limit):
+import rateLimit from 'express-rate-limit'; + +const submissionLimiter = rateLimit({ + windowMs: 10 * 60 * 1000, + max: 60, + standardHeaders: true, + legacyHeaders: false +}); -router.get('/display/:formUrl', getFormForDisplay); -router.post('/submit/:formUrl', createFormResponse); -router.put('/submit/:formUrl', updateFormResponse); -router.post('/responses/:formUrl', getFormResponses); +router.get('/display/:formUrl', getFormForDisplay); +router.post('/submit/:formUrl', submissionLimiter, createFormResponse); +router.put('/submit/:formUrl', submissionLimiter, updateFormResponse); +router.post('/responses/:formUrl', submissionLimiter, getFormResponses);frontend/src/pages/CreateForm.jsx (1)
8-13: Avoid double navigation after save.FormBuilder already navigates to /forms on create; this onSave handler triggers a second navigate.
Apply one of:
- Remove onSave here and rely on FormBuilder’s internal navigate, or
- Keep onSave but remove navigate call.
Minimal change:
- const handleSave = (formData) => { - console.log('Form saved:', formData); - // After successful save, redirect to forms list - navigate('/forms'); - }; + const handleSave = (formData) => { + console.log('Form saved:', formData); + // Navigation handled by FormBuilder on create + };Also applies to: 19-22
frontend/src/App.jsx (1)
20-26: Consider lazy‑loading the forms pages to shrink initial bundle.These screens aren’t needed on first paint; load them with React.lazy and wrap their Route elements in <React.Suspense fallback={...}>.
Example:
// replace static imports const FormsPage = React.lazy(() => import('./pages/FormsPage')); const CreateForm = React.lazy(() => import('./pages/CreateForm')); const EditForm = React.lazy(() => import('./pages/EditForm')); const FormSubmission = React.lazy(() => import('./components/FormSubmission')); const FormEdit = React.lazy(() => import('./components/FormEdit')); const FormResponses = React.lazy(() => import('./components/FormResponses')); // usage <Route path="/forms" element={<React.Suspense fallback={<div>Loading...</div>}><ProtectedRoute><FormsPage/></ProtectedRoute></React.Suspense>} /> // ...repeat for other new routesAlso applies to: 110-146
frontend/src/api/api.js (1)
265-321: Normalize response shapes from forms API.getUserForms conditionally returns response.data.data or response.data; this leaks backend shape differences into callers.
- Standardize server to always return
{ data: ... }, then here always returnresponse.data.data.- Or centralize normalization:
- getUserForms: async () => { - const response = await api.get('/api/forms/my-forms'); - // Handle both old and new response format - return response.data.data || response.data; - }, + getUserForms: async () => { + const { data } = await api.get('/api/forms/my-forms'); + return Array.isArray(data?.data) ? data.data : data; // temporary shim + },Additionally, consider accepting an optional AbortSignal on read methods to cancel in‑flight requests when navigating away (e.g., getFormForDisplay, getFormResponsesById).
frontend/src/components/FormsList.jsx (2)
74-82: Use toLocaleString for date+time; handle invalid datestoLocaleDateString ignores hour/minute. Also guard against invalid dateString.
- const formatDate = (dateString) => { - return new Date(dateString).toLocaleDateString('en-US', { + const formatDate = (dateString) => { + const d = new Date(dateString); + if (Number.isNaN(d.getTime())) return '—'; + return d.toLocaleString('en-US', { year: 'numeric', month: 'short', day: 'numeric', hour: '2-digit', minute: '2-digit' }); };
22-41: Abort fetch on unmount to prevent state updates after navigationCancel in-flight request if user navigates away to avoid setState on unmounted component.
- useEffect(() => { - const loadForms = async () => { + useEffect(() => { + const controller = new AbortController(); + const loadForms = async () => { try { const formsData = await formsAPI.getUserForms(); ... } catch (error) { console.error('Error loading forms:', error); ... } finally { setLoading(false); } }; - - if (isAuthenticated) { - loadForms(); - } - }, [isAuthenticated]); + if (isAuthenticated) loadForms(); + return () => controller.abort(); + }, [isAuthenticated]);frontend/src/pages/EditForm.jsx (1)
12-21: Simplify response shape handling; drop noisy logsformsAPI.getFormForEdit already returns response.data; you can setForm(response) directly and remove logs.
- try { - console.log('Loading form for editing with ID:', id); - const response = await formsAPI.getFormForEdit(id); - console.log('Form loaded successfully:', response); - - // Handle both old and new response format - const formData = response.success ? response : response.data || response; - setForm(formData); + try { + const formData = await formsAPI.getFormForEdit(id); + setForm(formData);frontend/src/components/FormEdit.jsx (2)
249-257: Remove unsupported propFormPreview doesn’t accept isEditing; it’s ignored. Avoid passing extra props.
- <FormPreview + <FormPreview form={form} isPublic={true} - isEditing={true} existingResponse={existingResponse} onSubmit={handleUpdate} submitButtonText={isUpdating ? 'Updating...' : 'Update Response'} disabled={isUpdating} />
178-198: Clipboard fallback (non-HTTPS/denied permissions)navigator.clipboard can fail off-HTTPS. Add a prompt fallback.
- navigator.clipboard.writeText(submissionResult.anonymousId); - alert('Edit code copied to clipboard!'); + navigator.clipboard?.writeText(submissionResult.anonymousId) + .then(() => alert('Edit code copied to clipboard!')) + .catch(() => { + window.prompt('Copy your edit code:', submissionResult.anonymousId); + });backend/src/controllers/form.js (1)
44-60: Fragile conditions referencing transient field IDsvalidateFieldConditions checks dependsOn against in-memory field ids from the request. On create, these IDs change when persisted. Consider storing dependsOn by stable identifiers (persisted field id) and performing a second-pass resolution after create, or disallowing conditions on new fields lacking an id.
frontend/src/components/FormBuilder.jsx (3)
51-66: Defaults: set allowMultiple for CHECKBOX; improve label formatting.Better UX and consistent semantics for multi-select checkboxes; nicer default label.
const addField = (fieldType) => { const newField = { id: `temp_${Date.now()}`, - label: `New ${fieldType.replace('_', ' ')} Field`, + label: `New ${fieldType.replaceAll('_', ' ')} Field`, fieldType, position: formData.fields.length + 1, isRequired: false, + allowMultiple: fieldType === 'CHECKBOX', placeholder: '', helpText: '', options: ['MULTIPLE_CHOICE', 'DROPDOWN', 'CHECKBOX', 'SINGLE_CHOICE'].includes(fieldType) ? [{ label: 'Option 1', value: 'option1' }, { label: 'Option 2', value: 'option2' }] : null, validation: null, conditions: null };
203-214: Field type descriptions are inconsistent; clarify SINGLE_CHOICE vs MULTIPLE_CHOICE.Labels/descriptions conflict and can confuse users.
const fieldTypes = [ - { type: 'MULTIPLE_CHOICE', label: 'Multiple Choice', description: 'Single selection from options' }, - { type: 'CHECKBOX', label: 'Checkboxes', description: 'Multiple selections' }, - { type: 'SINGLE_CHOICE', label: 'Single Choice', description: 'Radio button selection' }, + { type: 'SINGLE_CHOICE', label: 'Single Choice', description: 'Single selection (radio)' }, + { type: 'MULTIPLE_CHOICE', label: 'Multiple Choice', description: 'Multiple selections (checkboxes)' }, + { type: 'CHECKBOX', label: 'Checkboxes', description: 'Multiple selections' },Optionally remove one of MULTIPLE_CHOICE/CHECKBOX to avoid duplication in UI.
14-19: Auth redirect UX: avoid premature redirects and preserve return path.Guard until auth state is resolved and remember where to return.
- useEffect(() => { - if (!isAuthenticated) { - navigate('/'); - return; - } - }, [isAuthenticated, navigate]); + useEffect(() => { + if (isAuthenticated === false) { + navigate('/', { replace: true, state: { from: '/forms/builder' } }); + } + }, [isAuthenticated, navigate]); - if (!isAuthenticated) { + if (isAuthenticated === false) { return null; }Also applies to: 46-49
backend/prisma/schema.prisma (5)
327-341: Strengthen relations with explicit onDelete behavior.Prevent orphaned rows and define lifecycle clearly.
model Form { id String @id @default(cuid()) title String description String? createdBy String formUrl String @unique createdAt DateTime @default(now()) updatedAt DateTime @updatedAt isActive Boolean @default(true) - isEditable Boolean @default(false) - creator User @relation("UserForms", fields: [createdBy], references: [id]) + isEditable Boolean @default(false) + creator User @relation("UserForms", fields: [createdBy], references: [id], onDelete: Restrict) contributors FormContributor[] fields FormField[] responses FormResponse[] }Rationale: Do not allow deleting a user who owns forms without handoff.
359-374: Add cascade and indexes; enforce unique ordering per form.Improves integrity and query performance.
model FormField { id Int @id @default(autoincrement()) formId String label String fieldType FieldType isRequired Boolean @default(false) allowMultiple Boolean @default(false) options Json? position Int placeholder String? helpText String? validation Json? conditions Json? - form Form @relation(fields: [formId], references: [id]) + form Form @relation(fields: [formId], references: [id], onDelete: Cascade) answers FieldAnswer[] + @@index([formId]) + @@unique([formId, position]) }
390-399: Set cascade for responses and set-null for user; add index.Keeps answers tidy on form deletion; preserves anonymous responses if user is removed.
model FormResponse { id Int @id @default(autoincrement()) formId String submittedAt DateTime @default(now()) submittedBy String? anonymousId String? - form Form @relation(fields: [formId], references: [id]) - user User? @relation("UserFormResponses", fields: [submittedBy], references: [id]) + form Form @relation(fields: [formId], references: [id], onDelete: Cascade) + user User? @relation("UserFormResponses", fields: [submittedBy], references: [id], onDelete: SetNull) answers FieldAnswer[] + @@index([formId]) }
401-409: Cascade deletes for FieldAnswer and add indexes.Avoids manual cleanup; speeds analytics queries.
model FieldAnswer { id Int @id @default(autoincrement()) responseId Int fieldId Int answerValue String? answerJson Json? - response FormResponse @relation(fields: [responseId], references: [id]) - field FormField @relation(fields: [fieldId], references: [id]) + response FormResponse @relation(fields: [responseId], references: [id], onDelete: Cascade) + field FormField @relation(fields: [fieldId], references: [id], onDelete: Cascade) + @@index([responseId]) + @@index([fieldId]) }
376-387: ClarifyFieldTypesemantics; consider removing redundancy.Having both SINGLE_CHOICE and MULTIPLE_CHOICE plus CHECKBOX is confusing. Either:
- Keep SINGLE_CHOICE (radio) and CHECKBOX (multi), remove MULTIPLE_CHOICE; or
- Keep MULTIPLE_CHOICE (checkbox) and RADIO (single).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
backend/prisma/migrations/20251018092621_add_forms_to_orbis/migration.sql(1 hunks)backend/prisma/migrations/20251018123047_add_form_url/migration.sql(1 hunks)backend/prisma/migrations/20251018152641_add_permissions_to_forms/migration.sql(1 hunks)backend/prisma/migrations/20251019021225_form_response_model_changes/migration.sql(1 hunks)backend/prisma/schema.prisma(2 hunks)backend/server.js(2 hunks)backend/src/controllers/form.js(1 hunks)backend/src/routes/form.js(1 hunks)backend/src/routes/index.js(1 hunks)frontend/src/App.jsx(2 hunks)frontend/src/api/api.js(1 hunks)frontend/src/components/FormBuilder.jsx(1 hunks)frontend/src/components/FormEdit.jsx(1 hunks)frontend/src/components/FormFieldEditor.jsx(1 hunks)frontend/src/components/FormPreview.jsx(1 hunks)frontend/src/components/FormResponses.jsx(1 hunks)frontend/src/components/FormSubmission.jsx(1 hunks)frontend/src/components/FormsList.jsx(1 hunks)frontend/src/components/Navbar.jsx(5 hunks)frontend/src/pages/CreateForm.jsx(1 hunks)frontend/src/pages/EditForm.jsx(1 hunks)frontend/src/pages/FormsPage.jsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (16)
backend/src/routes/form.js (1)
backend/src/controllers/form.js (22)
createForm(62-194)createForm(62-194)updateForm(196-351)updateForm(196-351)deleteForm(353-408)deleteForm(353-408)getAllForms(410-466)getAllForms(410-466)getFormByUrl(468-558)getFormByUrl(468-558)getFormById(560-627)getFormById(560-627)getFormResponsesById(1049-1130)getFormResponsesById(1049-1130)getFormForDisplay(629-711)getFormForDisplay(629-711)createFormResponse(713-861)createFormResponse(713-861)updateFormResponse(863-983)updateFormResponse(863-983)getFormResponses(985-1047)getFormResponses(985-1047)
frontend/src/components/FormResponses.jsx (2)
backend/src/controllers/form.js (11)
form(358-364)form(473-500)form(569-583)form(635-656)form(731-734)form(868-871)form(995-1009)form(1057-1065)responses(1078-1095)response(762-768)response(885-893)frontend/src/api/api.js (16)
formsAPI(265-321)formsAPI(265-321)response(37-37)response(42-42)response(47-47)response(68-68)response(73-73)response(78-78)response(83-83)response(88-88)response(100-100)response(110-110)response(127-130)response(151-154)response(164-164)response(175-175)
frontend/src/components/FormsList.jsx (3)
backend/src/controllers/form.js (10)
forms(413-445)form(358-364)form(473-500)form(569-583)form(635-656)form(731-734)form(868-871)form(995-1009)form(1057-1065)formUrl(124-124)frontend/src/api/api.js (2)
formsAPI(265-321)formsAPI(265-321)frontend/src/components/Button.jsx (1)
Button(4-25)
frontend/src/pages/CreateForm.jsx (2)
frontend/src/components/FormBuilder.jsx (4)
navigate(11-11)handleSave(151-201)formData(21-27)FormBuilder(9-487)frontend/src/pages/EditForm.jsx (3)
navigate(8-8)handleSave(39-43)handleCancel(45-47)
backend/src/routes/index.js (1)
backend/src/middleware/auth.js (2)
checkJwt(20-24)checkJwt(20-24)
frontend/src/App.jsx (6)
frontend/src/pages/FormsPage.jsx (1)
FormsPage(4-6)frontend/src/pages/CreateForm.jsx (1)
CreateForm(5-24)frontend/src/pages/EditForm.jsx (1)
EditForm(6-67)frontend/src/components/FormResponses.jsx (1)
FormResponses(5-155)frontend/src/components/FormSubmission.jsx (1)
FormSubmission(6-255)frontend/src/components/FormEdit.jsx (1)
FormEdit(6-284)
frontend/src/components/Navbar.jsx (2)
frontend/src/contexts/AuthContext.jsx (2)
useAuth(69-75)useAuth(69-75)frontend/src/components/Button.jsx (1)
Button(4-25)
frontend/src/components/FormSubmission.jsx (3)
backend/src/controllers/form.js (14)
form(358-364)form(473-500)form(569-583)form(635-656)form(731-734)form(868-871)form(995-1009)form(1057-1065)error(6-6)formUrl(124-124)response(762-768)response(885-893)fieldId(775-775)fieldId(906-906)frontend/src/api/api.js (16)
response(37-37)response(42-42)response(47-47)response(68-68)response(73-73)response(78-78)response(83-83)response(88-88)response(100-100)response(110-110)response(127-130)response(151-154)response(164-164)response(175-175)formsAPI(265-321)formsAPI(265-321)frontend/src/components/FormPreview.jsx (2)
handleSubmit(61-83)FormPreview(4-373)
frontend/src/components/FormPreview.jsx (3)
backend/src/controllers/form.js (16)
responses(1078-1095)fieldId(775-775)fieldId(906-906)form(358-364)form(473-500)form(569-583)form(635-656)form(731-734)form(868-871)form(995-1009)form(1057-1065)field(14-14)field(45-45)field(776-776)field(907-907)error(6-6)frontend/src/components/FormSubmission.jsx (3)
form(8-8)handleSubmit(36-63)error(10-10)frontend/src/components/Button.jsx (1)
Button(4-25)
frontend/src/components/FormEdit.jsx (2)
frontend/src/api/api.js (16)
response(37-37)response(42-42)response(47-47)response(68-68)response(73-73)response(78-78)response(83-83)response(88-88)response(100-100)response(110-110)response(127-130)response(151-154)response(164-164)response(175-175)formsAPI(265-321)formsAPI(265-321)frontend/src/components/FormPreview.jsx (2)
responses(12-12)FormPreview(4-373)
backend/src/controllers/form.js (2)
backend/src/config/database.js (1)
prisma(4-12)frontend/src/api/api.js (16)
response(37-37)response(42-42)response(47-47)response(68-68)response(73-73)response(78-78)response(83-83)response(88-88)response(100-100)response(110-110)response(127-130)response(151-154)response(164-164)response(175-175)response(267-267)response(272-272)
backend/server.js (2)
backend/src/middleware/auth.js (2)
checkJwt(20-24)checkJwt(20-24)backend/src/middleware/error.js (1)
errorHandler(1-22)
frontend/src/pages/EditForm.jsx (2)
frontend/src/components/FormBuilder.jsx (4)
navigate(11-11)formData(21-27)handleSave(151-201)FormBuilder(9-487)frontend/src/api/api.js (16)
response(37-37)response(42-42)response(47-47)response(68-68)response(73-73)response(78-78)response(83-83)response(88-88)response(100-100)response(110-110)response(127-130)response(151-154)response(164-164)response(175-175)formsAPI(265-321)formsAPI(265-321)
frontend/src/api/api.js (2)
backend/src/controllers/form.js (5)
formData(126-167)response(762-768)response(885-893)formUrl(124-124)responseData(839-843)frontend/src/components/FormBuilder.jsx (1)
formData(21-27)
frontend/src/pages/FormsPage.jsx (1)
frontend/src/components/FormsList.jsx (1)
FormsList(7-294)
frontend/src/components/FormBuilder.jsx (5)
backend/src/controllers/form.js (22)
formData(126-167)form(358-364)form(473-500)form(569-583)form(635-656)form(731-734)form(868-871)form(995-1009)form(1057-1065)fieldId(775-775)fieldId(906-906)field(14-14)field(45-45)field(776-776)field(907-907)error(6-6)user(64-64)user(198-198)user(355-355)user(412-412)user(471-471)user(563-563)frontend/src/api/api.js (2)
formsAPI(265-321)formsAPI(265-321)frontend/src/components/Button.jsx (1)
Button(4-25)frontend/src/components/FormPreview.jsx (1)
FormPreview(4-373)frontend/src/components/FormFieldEditor.jsx (1)
FormFieldEditor(21-389)
🔇 Additional comments (6)
frontend/src/components/Navbar.jsx (1)
78-82: LGTM: Forms entry points are integrated cleanly.Conditional “Forms” link and “Create Form” actions for authenticated users look correct in both desktop and mobile.
Please confirm the routes
/formsand/forms/createare guarded as intended inApp.jsxand exist (FormsList/CreateForm).Also applies to: 96-97, 156-163
frontend/src/pages/FormsPage.jsx (1)
4-6: LGTM: Thin page wrapper is fine.
FormsPagecleanly composesFormsList. No issues.backend/src/routes/form.js (1)
19-26: Protected routes wiring looks correct.Good use of checkJwt → ensureUser ordering on owner/contributor-only endpoints. No issues spotted.
backend/prisma/migrations/20251018092621_add_forms_to_orbis/migration.sql (1)
1-98: No issues found—follow-up migrations properly add required columnsVerification confirms that all referenced columns (formUrl, isEditable, allowMultiple, anonymousId) are added by subsequent migrations timestamped after the original migration, and all are present in the current schema. Migrations will apply in the correct order. No action needed.
backend/prisma/migrations/20251018152641_add_permissions_to_forms/migration.sql (2)
28-28: Unique index onForm.formUrlexists and is established before droppingslugunique index.The migration that creates the UNIQUE INDEX on
formUrl(timestamp20251018123047) is applied before the migration that drops theslugunique index (timestamp20251018152641). The schema confirmsformUrlis marked@unique. No gap in unique constraint coverage exists; shareable URLs remain protected against duplicates.
46-51: No issues found — unique constraint will persist post-migration.The unique index
FormContributor_formId_userId_keyon columns(formId, userId)was created in an earlier migration and is not dropped by the current migration. Since those columns are not being modified (only theidPK column is recreated), PostgreSQL will automatically preserve the index through the operation. The constraint defined inschema.prismaremains intact.
| -- AlterTable | ||
| ALTER TABLE "Form" ADD COLUMN "formUrl" TEXT NOT NULL; | ||
|
|
||
| -- CreateIndex | ||
| CREATE UNIQUE INDEX "Form_formUrl_key" ON "Form"("formUrl"); |
There was a problem hiding this comment.
🧩 Analysis chain
Risky NOT NULL add on non-empty table; backfill first, then enforce NOT NULL + UNIQUE.
As written, this fails if Form has rows or duplicates. Prefer a safe, two-step migration that backfills from an existing unique field (e.g., slug) before adding constraints.
--- BEFORE
ALTER TABLE "Form" ADD COLUMN "formUrl" TEXT NOT NULL;
CREATE UNIQUE INDEX "Form_formUrl_key" ON "Form"("formUrl");
+++ AFTER (safer)
+-- 1) Add as nullable
+ALTER TABLE "Form" ADD COLUMN "formUrl" TEXT;
+
+-- 2) Backfill (assuming "slug" exists and is unique enough)
+UPDATE "Form" SET "formUrl" = "slug" WHERE "formUrl" IS NULL;
+
+-- 3) Validate uniqueness (manually check/clean duplicates if any), then:
+CREATE UNIQUE INDEX "Form_formUrl_key" ON "Form"("formUrl");
+
+-- 4) Enforce NOT NULL
+ALTER TABLE "Form" ALTER COLUMN "formUrl" SET NOT NULL;Please confirm Form.slug exists and is unique; if not, we should generate a stable code (e.g., shortid/uuid) and backfill instead. Also check for case-insensitive uniqueness needs; if required, consider a functional index on lower("formUrl").
I need to verify the safety of this migration by examining the codebase context. Let me gather information about the Form table structure and the migration context.
🏁 Script executed:
#!/bin/bash
# Find and examine the Prisma schema to understand Form model structure
find . -name "schema.prisma" -type f | head -5Length of output: 30
🏁 Script executed:
#!/bin/bash
# Examine the migration file and surrounding context
find . -path "*prisma/migrations/20251018123047*" -type fLength of output: 69
🏁 Script executed:
#!/bin/bash
# Look for Prisma schema definition
find . -name "schema.prisma" -exec cat {} \;Length of output: 10878
🏁 Script executed:
#!/bin/bash
# Search for Form model definition in the codebase
rg -n "model Form" --type ts --type js --type prisma -A 20Length of output: 30
🏁 Script executed:
#!/bin/bash
# Check for any migration files that might have populated data before this one
find . -path "*/prisma/migrations/*" -name "*.sql" | sort | tail -10Length of output: 797
I need to examine the earlier migration to understand the Form table evolution and determine if the backfill suggestion is valid.
---
🏁 Script executed:
```shell
#!/bin/bash
# Read the Form table creation migration
cat ./backend/prisma/migrations/20251018092621_add_forms_to_orbis/migration.sql
Length of output: 3384
🏁 Script executed:
#!/bin/bash
# Read the migration under review in full
cat ./backend/prisma/migrations/20251018123047_add_form_url/migration.sqlLength of output: 450
🏁 Script executed:
#!/bin/bash
# Check for any seed files or data population scripts
find . -name "seed*" -o -name "*seed*" -type f | grep -E "\.(ts|js|sql)$"Length of output: 0
Migration adds NOT NULL constraint without safe backfill; use two-step approach.
The migration's risk is real: the Prisma warning confirms "This is not possible if the table is not empty". The Form table was created with a slug field that is unique, so backfilling from slug is a sound strategy. The two-step migration approach is appropriate: add nullable column, backfill from slug, create unique index, then enforce NOT NULL.
--- BEFORE
ALTER TABLE "Form" ADD COLUMN "formUrl" TEXT NOT NULL;
CREATE UNIQUE INDEX "Form_formUrl_key" ON "Form"("formUrl");
+++ AFTER (safer)
+-- 1) Add as nullable
+ALTER TABLE "Form" ADD COLUMN "formUrl" TEXT;
+
+-- 2) Backfill from slug (which is guaranteed unique)
+UPDATE "Form" SET "formUrl" = "slug" WHERE "formUrl" IS NULL;
+
+-- 3) Create unique index
+CREATE UNIQUE INDEX "Form_formUrl_key" ON "Form"("formUrl");
+
+-- 4) Enforce NOT NULL
+ALTER TABLE "Form" ALTER COLUMN "formUrl" SET NOT NULL;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| -- AlterTable | |
| ALTER TABLE "Form" ADD COLUMN "formUrl" TEXT NOT NULL; | |
| -- CreateIndex | |
| CREATE UNIQUE INDEX "Form_formUrl_key" ON "Form"("formUrl"); | |
| -- AlterTable | |
| -- 1) Add as nullable | |
| ALTER TABLE "Form" ADD COLUMN "formUrl" TEXT; | |
| -- 2) Backfill from slug (which is guaranteed unique) | |
| UPDATE "Form" SET "formUrl" = "slug" WHERE "formUrl" IS NULL; | |
| -- 3) Create unique index | |
| CREATE UNIQUE INDEX "Form_formUrl_key" ON "Form"("formUrl"); | |
| -- 4) Enforce NOT NULL | |
| ALTER TABLE "Form" ALTER COLUMN "formUrl" SET NOT NULL; |
🤖 Prompt for AI Agents
In backend/prisma/migrations/20251018123047_add_form_url/migration.sql around
lines 8-12 the migration adds "formUrl" as NOT NULL directly which will fail for
non-empty tables; change to a two-step safe migration: 1) ADD COLUMN "formUrl"
TEXT NULL; 2) UPDATE "Form" SET "formUrl" = "slug" WHERE "formUrl" IS NULL; 3)
CREATE UNIQUE INDEX "Form_formUrl_key" ON "Form"("formUrl"); 4) ALTER TABLE
"Form" ALTER COLUMN "formUrl" SET NOT NULL. Ensure the backfill covers all
existing rows before setting NOT NULL and create the unique index after backfill
to avoid constraint violations.
| ALTER TABLE "FieldAnswer" DROP CONSTRAINT "FieldAnswer_pkey", | ||
| DROP COLUMN "id", | ||
| ADD COLUMN "id" SERIAL NOT NULL, | ||
| DROP COLUMN "responseId", | ||
| ADD COLUMN "responseId" INTEGER NOT NULL, | ||
| DROP COLUMN "fieldId", | ||
| ADD COLUMN "fieldId" INTEGER NOT NULL, | ||
| ADD CONSTRAINT "FieldAnswer_pkey" PRIMARY KEY ("id"); |
There was a problem hiding this comment.
High risk of data loss by drop/recreate; prefer in-place type change or staged backfill.
Dropping responseId/fieldId and id can orphan or lose data in populated DBs.
Safer pattern:
-ALTER TABLE "FieldAnswer" DROP CONSTRAINT "FieldAnswer_pkey",
-DROP COLUMN "id",
-ADD COLUMN "id" SERIAL NOT NULL,
-DROP COLUMN "responseId",
-ADD COLUMN "responseId" INTEGER NOT NULL,
-DROP COLUMN "fieldId",
-ADD COLUMN "fieldId" INTEGER NOT NULL,
-ADD CONSTRAINT "FieldAnswer_pkey" PRIMARY KEY ("id");
+-- Option A: in-place cast if prior types are castable
+ALTER TABLE "FieldAnswer" ALTER COLUMN "responseId" TYPE INTEGER USING ("responseId"::INTEGER);
+ALTER TABLE "FieldAnswer" ALTER COLUMN "fieldId" TYPE INTEGER USING ("fieldId"::INTEGER);
+-- If changing PK strategy, add new column, backfill, then swap in a transaction with minimal lock.
+-- Example:
+-- ALTER TABLE "FieldAnswer" ADD COLUMN "id2" SERIAL;
+-- UPDATE "FieldAnswer" SET "id2" = nextval(pg_get_serial_sequence('"FieldAnswer"','id2')) WHERE "id2" IS NULL;
+-- ALTER TABLE "FieldAnswer" DROP CONSTRAINT "FieldAnswer_pkey";
+-- ALTER TABLE "FieldAnswer" RENAME COLUMN "id" TO "id_old";
+-- ALTER TABLE "FieldAnswer" RENAME COLUMN "id2" TO "id";
+-- ALTER TABLE "FieldAnswer" ADD CONSTRAINT "FieldAnswer_pkey" PRIMARY KEY ("id");Schedule a maintenance window and take backups before applying.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
backend/prisma/migrations/20251018152641_add_permissions_to_forms/migration.sql
around lines 31 to 38, do not DROP and re-ADD the id, responseId, and fieldId
columns as that risks data loss; instead perform in-place changes or a staged
backfill: add new temporary columns of the desired type (e.g., new_id SERIAL,
new_responseId INTEGER, new_fieldId INTEGER), backfill them from the existing
columns or compute values as needed, verify integrity, set new columns NOT NULL,
create constraints/indexes on the new columns, then swap names (rename old to
backup_, rename new to original), and finally drop the old backup_ columns;
ensure this migration runs inside a maintenance window with a backup and add
transactional safety where supported.
| -- AddForeignKey | ||
| ALTER TABLE "FieldAnswer" ADD CONSTRAINT "FieldAnswer_responseId_fkey" FOREIGN KEY ("responseId") REFERENCES "FormResponse"("id") ON DELETE RESTRICT ON UPDATE CASCADE; | ||
|
|
||
| -- AddForeignKey | ||
| ALTER TABLE "FieldAnswer" ADD CONSTRAINT "FieldAnswer_fieldId_fkey" FOREIGN KEY ("fieldId") REFERENCES "FormField"("id") ON DELETE RESTRICT ON UPDATE CASCADE; |
There was a problem hiding this comment.
Use cascading deletes for child rows to prevent referential dead-ends.
RESTRICT will block deleting responses/fields during edits. Prefer ON DELETE CASCADE for FieldAnswer FKs.
-ALTER TABLE "FieldAnswer" ADD CONSTRAINT "FieldAnswer_responseId_fkey" FOREIGN KEY ("responseId") REFERENCES "FormResponse"("id") ON DELETE RESTRICT ON UPDATE CASCADE;
+ALTER TABLE "FieldAnswer" ADD CONSTRAINT "FieldAnswer_responseId_fkey" FOREIGN KEY ("responseId") REFERENCES "FormResponse"("id") ON DELETE CASCADE ON UPDATE CASCADE;
-ALTER TABLE "FieldAnswer" ADD CONSTRAINT "FieldAnswer_fieldId_fkey" FOREIGN KEY ("fieldId") REFERENCES "FormField"("id") ON DELETE RESTRICT ON UPDATE CASCADE;
+ALTER TABLE "FieldAnswer" ADD CONSTRAINT "FieldAnswer_fieldId_fkey" FOREIGN KEY ("fieldId") REFERENCES "FormField"("id") ON DELETE CASCADE ON UPDATE CASCADE;| const existingFieldsMap = new Map(existingForm.fields.map(f => [f.label, f])); | ||
|
|
||
| const fieldsToCreate = []; | ||
| const fieldsToUpdate = []; | ||
| const fieldIdsToDelete = []; | ||
|
|
||
| for (const field of fields) { | ||
| const existingField = existingFieldsMap.get(field.label); | ||
| if (existingField) { | ||
| const updateData = {}; | ||
| if (existingField.position !== field.position) updateData.position = field.position; | ||
| if (existingField.placeholder !== field.placeholder) updateData.placeholder = field.placeholder || null; | ||
| if (existingField.isRequired !== field.isRequired) updateData.isRequired = field.isRequired || false; | ||
| if (existingField.allowMultiple !== field.allowMultiple) updateData.allowMultiple = field.allowMultiple || false; | ||
| if (existingField.helpText !== field.helpText) updateData.helpText = field.helpText || null; | ||
| if (JSON.stringify(existingField.options) !== JSON.stringify(field.options)) | ||
| updateData.options = typeof field.options === "string" ? JSON.parse(field.options) : field.options; | ||
| if (JSON.stringify(existingField.validation) !== JSON.stringify(field.validation)) | ||
| updateData.validation = typeof field.validation === "string" ? JSON.parse(field.validation) : field.validation; | ||
| if (JSON.stringify(existingField.conditions) !== JSON.stringify(field.conditions)) | ||
| updateData.conditions = typeof field.conditions === "string" ? JSON.parse(field.conditions) : field.conditions; | ||
|
|
||
| if (Object.keys(updateData).length > 0) { | ||
| fieldsToUpdate.push({ id: existingField.id, data: updateData }); | ||
| } | ||
|
|
||
| existingFieldsMap.delete(field.label); | ||
| } else { | ||
| fieldsToCreate.push({ | ||
| formId: id, | ||
| label: field.label, | ||
| fieldType: field.fieldType, | ||
| position: field.position, | ||
| placeholder: field.placeholder || null, | ||
| isRequired: field.isRequired || false, | ||
| allowMultiple: field.allowMultiple || false, | ||
| helpText: field.helpText || null, | ||
| options: field.options ? (typeof field.options === "string" ? JSON.parse(field.options) : field.options) : null, | ||
| validation: field.validation ? (typeof field.validation === "string" ? JSON.parse(field.validation) : field.validation) : null, | ||
| conditions: field.conditions ? (typeof field.conditions === "string" ? JSON.parse(field.conditions) : field.conditions) : null, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| for (const [_, fieldToDelete] of existingFieldsMap) { | ||
| fieldIdsToDelete.push(fieldToDelete.id); | ||
| } | ||
|
|
||
| if (fieldsToCreate.length > 0) await tx.formField.createMany({ data: fieldsToCreate }); |
There was a problem hiding this comment.
Do not match/update fields by label; use stable IDs to avoid data loss
Keying by label breaks on rename and can delete/recreate fields (and answers). Match by field.id when present; only create when no id. Prevents accidental loss.
- const existingFieldsMap = new Map(existingForm.fields.map(f => [f.label, f]));
+ const existingFieldsById = new Map(existingForm.fields.map(f => [String(f.id), f]));
...
- for (const field of fields) {
- const existingField = existingFieldsMap.get(field.label);
+ for (const field of fields) {
+ const fieldIdKey = field.id ? String(field.id) : null;
+ const existingField = fieldIdKey ? existingFieldsById.get(fieldIdKey) : null;
if (existingField) {
const updateData = {};
if (existingField.position !== field.position) updateData.position = field.position;
if (existingField.placeholder !== field.placeholder) updateData.placeholder = field.placeholder || null;
if (existingField.isRequired !== field.isRequired) updateData.isRequired = field.isRequired || false;
if (existingField.allowMultiple !== field.allowMultiple) updateData.allowMultiple = field.allowMultiple || false;
if (existingField.helpText !== field.helpText) updateData.helpText = field.helpText || null;
if (JSON.stringify(existingField.options) !== JSON.stringify(field.options))
updateData.options = typeof field.options === "string" ? JSON.parse(field.options) : field.options;
if (JSON.stringify(existingField.validation) !== JSON.stringify(field.validation))
updateData.validation = typeof field.validation === "string" ? JSON.parse(field.validation) : field.validation;
if (JSON.stringify(existingField.conditions) !== JSON.stringify(field.conditions))
updateData.conditions = typeof field.conditions === "string" ? JSON.parse(field.conditions) : field.conditions;
if (Object.keys(updateData).length > 0) {
fieldsToUpdate.push({ id: existingField.id, data: updateData });
}
- existingFieldsMap.delete(field.label);
+ existingFieldsById.delete(fieldIdKey);
} else {
fieldsToCreate.push({
formId: id,
label: field.label,
fieldType: field.fieldType,
position: field.position,
placeholder: field.placeholder || null,
isRequired: field.isRequired || false,
allowMultiple: field.allowMultiple || false,
helpText: field.helpText || null,
options: field.options ? (typeof field.options === "string" ? JSON.parse(field.options) : field.options) : null,
validation: field.validation ? (typeof field.validation === "string" ? JSON.parse(field.validation) : field.validation) : null,
conditions: field.conditions ? (typeof field.conditions === "string" ? JSON.parse(field.conditions) : field.conditions) : null,
});
}
}
- for (const [_, fieldToDelete] of existingFieldsMap) {
+ for (const [_, fieldToDelete] of existingFieldsById) {
fieldIdsToDelete.push(fieldToDelete.id);
}| export const getFormByUrl = async (req, res) => { | ||
| try { | ||
| const { formUrl } = req.params; | ||
| const user = ensureAuth(req); | ||
|
|
||
| const form = await prisma.form.findUnique({ | ||
| where: { formUrl }, | ||
| include: { | ||
| fields: { | ||
| include: { | ||
| answers: { | ||
| include: { | ||
| response: { | ||
| select: { | ||
| id: true, | ||
| submittedAt: true, | ||
| user: { select: { id: true, username: true, email: true } }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| contributors: { | ||
| select: { | ||
| userId: true, | ||
| permission: true, | ||
| user: { select: { id: true, username: true, email: true } }, | ||
| }, | ||
| }, | ||
| creator: { select: { id: true, username: true, email: true } }, | ||
| }, | ||
| }); | ||
|
|
||
|
|
||
| if (!form) return res.status(404).json({ error: 'Form not found' }); | ||
|
|
||
| const isOwner = form.createdBy === user.id; | ||
| const isContributor = form.contributors.some(c => c.userId === user.id); | ||
| if (!isOwner && !isContributor) { | ||
| return res.status(403).json({ error: 'Permission denied' }); | ||
| } | ||
|
|
||
| const responseWise = form.responses.map(response => ({ | ||
| responseId: response.id, | ||
| submittedAt: response.submittedAt, | ||
| submittedBy: response.user ? { id: response.user.id, username: response.user.username, email: response.user.email } : null, | ||
| answers: response.answers.map(ans => ({ | ||
| fieldId: ans.fieldId, | ||
| fieldLabel: ans.field.label, | ||
| fieldType: ans.field.fieldType, | ||
| answerValue: ans.answerValue, | ||
| answerJson: ans.answerJson | ||
| })) | ||
| })); | ||
|
|
||
| const questionWise = form.fields.map(field => ({ | ||
| id: field.id, | ||
| label: field.label, | ||
| fieldType: field.fieldType, | ||
| answers: field.answers.map(ans => ({ | ||
| responseId: ans.response.id, | ||
| submittedAt: ans.response.submittedAt, | ||
| submittedBy: ans.response.user | ||
| ? { id: ans.response.user.id, username: ans.response.user.username, email: ans.response.user.email } | ||
| : null, | ||
| answerValue: ans.answerValue, | ||
| answerJson: ans.answerJson, | ||
| })), | ||
| })); | ||
|
|
||
| res.status(200).json({ | ||
| id: form.id, | ||
| title: form.title, | ||
| formUrl: form.formUrl, | ||
| description: form.description, | ||
| isActive: form.isActive, | ||
| isTemplate: form.isTemplate, | ||
| isEditable: form.isEditable, | ||
| fields: form.fields, | ||
| contributors: form.contributors, | ||
| creator: form.creator, | ||
| responsesCount: form.responses.length, | ||
| responseWise, | ||
| questionWise | ||
| }); | ||
|
|
||
| } catch (error) { | ||
| res.status(500).json({ error: 'Failed to fetch form: ' + error }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Include responses in getFormByUrl or derive responseWise differently
responseWise uses form.responses but responses aren’t included in the query, causing undefined access at runtime. Include responses with their answers/users.
- const form = await prisma.form.findUnique({
+ const form = await prisma.form.findUnique({
where: { formUrl },
include: {
fields: {
include: {
answers: {
include: {
response: {
select: {
id: true,
submittedAt: true,
user: { select: { id: true, username: true, email: true } },
},
},
},
},
},
},
+ responses: {
+ include: {
+ answers: {
+ include: { field: { select: { id: true, label: true, fieldType: true } } }
+ },
+ user: { select: { id: true, username: true, email: true } }
+ }
+ },
contributors: {
select: {
userId: true,
permission: true,
user: { select: { id: true, username: true, email: true } },
},
},
creator: { select: { id: true, username: true, email: true } },
},
});🤖 Prompt for AI Agents
In backend/src/controllers/form.js around lines 468 to 558, the code builds
responseWise from form.responses but the Prisma query does not include
responses, causing runtime undefined access; update the prisma.form.findUnique
include to add responses (including their answers and user/select fields) so
response objects are available, or alternatively change the responseWise
construction to derive data from included fields (e.g., gather responses from
answers) — ensure the responses include id, submittedAt, user (id, username,
email) and answers with field/answer data so responseWise and responsesCount
work without errors.
| const validateForm = () => { | ||
| const newErrors = {}; | ||
|
|
||
| form.fields.forEach(field => { | ||
| if (field.isRequired && !responses[field.id]) { | ||
| newErrors[field.id] = 'This field is required'; | ||
| } | ||
| }); | ||
|
|
||
| setErrors(newErrors); | ||
| return Object.keys(newErrors).length === 0; | ||
| }; |
There was a problem hiding this comment.
Required-field validation: treat 0 as valid and handle arrays properly
Current check fails for numeric 0 and doesn’t distinguish arrays/strings.
- form.fields.forEach(field => {
- if (field.isRequired && !responses[field.id]) {
- newErrors[field.id] = 'This field is required';
- }
- });
+ form.fields.forEach(field => {
+ if (!field.isRequired) return;
+ const val = responses[field.id];
+ const isMissing =
+ val === undefined ||
+ val === null ||
+ (typeof val === 'string' && val.trim() === '') ||
+ (Array.isArray(val) && val.length === 0);
+ if (isMissing) newErrors[field.id] = 'This field is required';
+ });🤖 Prompt for AI Agents
In frontend/src/components/FormPreview.jsx around lines 48 to 59, the
required-field check treats numeric 0 as empty and doesn't properly handle
arrays/strings; change the validation to consider a field missing only when the
response is null/undefined, or when it's a string that's empty/whitespace, or
when it's an array with length 0. Update the loop to read the response value
once, then use a predicate like: value === null || value === undefined ||
(typeof value === 'string' && value.trim() === '') || (Array.isArray(value) &&
value.length === 0) to decide if the required error should be set (allow numbers
including 0 and non-empty arrays/strings).
| const handleSubmit = async (e) => { | ||
| e.preventDefault(); | ||
|
|
||
| if (!validateForm()) { | ||
| return; | ||
| } | ||
|
|
||
| setIsSubmitting(true); | ||
| try { | ||
| const formattedResponses = form.fields.map(field => ({ | ||
| fieldId: field.id, | ||
| value: responses[field.id] || null | ||
| })); | ||
|
|
||
| console.log('Submitting responses:', formattedResponses); | ||
| onSubmit && onSubmit(formattedResponses); | ||
| } catch (error) { | ||
| console.error('Error submitting form:', error); | ||
| alert('Failed to submit form. Please try again.'); | ||
| } finally { | ||
| setIsSubmitting(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Await onSubmit to catch errors and prevent race conditions
Without await, errors are unhandled and isSubmitting resets too early.
- onSubmit && onSubmit(formattedResponses);
+ if (onSubmit) {
+ await onSubmit(formattedResponses);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleSubmit = async (e) => { | |
| e.preventDefault(); | |
| if (!validateForm()) { | |
| return; | |
| } | |
| setIsSubmitting(true); | |
| try { | |
| const formattedResponses = form.fields.map(field => ({ | |
| fieldId: field.id, | |
| value: responses[field.id] || null | |
| })); | |
| console.log('Submitting responses:', formattedResponses); | |
| onSubmit && onSubmit(formattedResponses); | |
| } catch (error) { | |
| console.error('Error submitting form:', error); | |
| alert('Failed to submit form. Please try again.'); | |
| } finally { | |
| setIsSubmitting(false); | |
| } | |
| }; | |
| const handleSubmit = async (e) => { | |
| e.preventDefault(); | |
| if (!validateForm()) { | |
| return; | |
| } | |
| setIsSubmitting(true); | |
| try { | |
| const formattedResponses = form.fields.map(field => ({ | |
| fieldId: field.id, | |
| value: responses[field.id] || null | |
| })); | |
| console.log('Submitting responses:', formattedResponses); | |
| if (onSubmit) { | |
| await onSubmit(formattedResponses); | |
| } | |
| } catch (error) { | |
| console.error('Error submitting form:', error); | |
| alert('Failed to submit form. Please try again.'); | |
| } finally { | |
| setIsSubmitting(false); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In frontend/src/components/FormPreview.jsx around lines 61 to 83, the onSubmit
callback is invoked without awaiting it so thrown errors are unhandled and
setIsSubmitting is cleared too early; change the call to await the result (e.g.
if (onSubmit) await Promise.resolve(onSubmit(formattedResponses))) so the
try/catch will capture async errors and isSubmitting remains true until
submission completes, preserving proper error handling and preventing race
conditions.
| const getAnswerDisplay = (answer) => { | ||
| if (answer.answerJson) { | ||
| try { | ||
| const jsonData = JSON.parse(answer.answerJson); | ||
| if (Array.isArray(jsonData)) { | ||
| return jsonData.join(', '); | ||
| } | ||
| return JSON.stringify(jsonData); | ||
| } catch { | ||
| return answer.answerJson; | ||
| } | ||
| } | ||
| return answer.answerValue || 'No answer'; | ||
| }; |
There was a problem hiding this comment.
Fix JSONB answer rendering (arrays/objects)
answer.answerJson from backend is JSON (object/array), not a JSON string. Parsing fails and UI shows “[object Object]”.
- const getAnswerDisplay = (answer) => {
- if (answer.answerJson) {
- try {
- const jsonData = JSON.parse(answer.answerJson);
- if (Array.isArray(jsonData)) {
- return jsonData.join(', ');
- }
- return JSON.stringify(jsonData);
- } catch {
- return answer.answerJson;
- }
- }
- return answer.answerValue || 'No answer';
- };
+ const getAnswerDisplay = (answer) => {
+ const j = answer.answerJson;
+ if (Array.isArray(j)) return j.join(', ');
+ if (j && typeof j === 'object') return JSON.stringify(j);
+ if (typeof j === 'string') {
+ try {
+ const parsed = JSON.parse(j);
+ return Array.isArray(parsed) ? parsed.join(', ') : JSON.stringify(parsed);
+ } catch {}
+ }
+ return answer.answerValue ?? 'No answer';
+ };🤖 Prompt for AI Agents
In frontend/src/components/FormResponses.jsx around lines 36 to 49, the code
assumes answer.answerJson is a JSON string and calls JSON.parse, causing parsing
to fail and objects to render as “[object Object]”; update the function to first
check the type of answer.answerJson and only parse when it's a string, otherwise
treat it as already-parsed data — if it is an array join with ', ', if it is an
object return JSON.stringify or a suitable formatted string, keep the existing
fallback to answer.answerValue or 'No answer', and retain a try/catch around
parse to fall back to the raw value on error.
| const filteredForms = forms.filter(form => { | ||
| const matchesSearch = form.title.toLowerCase().includes(searchTerm.toLowerCase()) || | ||
| form.description.toLowerCase().includes(searchTerm.toLowerCase()); | ||
|
|
||
| const matchesFilter = filter === 'all' || | ||
| (filter === 'active' && form.isActive) || | ||
| (filter === 'inactive' && !form.isActive); | ||
|
|
||
| return matchesSearch && matchesFilter; | ||
| }); |
There was a problem hiding this comment.
Null-safe search to avoid crashes on missing description
form.description may be null/undefined; calling toLowerCase() would throw. Default to empty strings.
- const filteredForms = forms.filter(form => {
- const matchesSearch = form.title.toLowerCase().includes(searchTerm.toLowerCase()) ||
- form.description.toLowerCase().includes(searchTerm.toLowerCase());
+ const filteredForms = forms.filter(form => {
+ const title = (form.title || '').toLowerCase();
+ const desc = (form.description || '').toLowerCase();
+ const q = searchTerm.toLowerCase();
+ const matchesSearch = title.includes(q) || desc.includes(q);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const filteredForms = forms.filter(form => { | |
| const matchesSearch = form.title.toLowerCase().includes(searchTerm.toLowerCase()) || | |
| form.description.toLowerCase().includes(searchTerm.toLowerCase()); | |
| const matchesFilter = filter === 'all' || | |
| (filter === 'active' && form.isActive) || | |
| (filter === 'inactive' && !form.isActive); | |
| return matchesSearch && matchesFilter; | |
| }); | |
| const filteredForms = forms.filter(form => { | |
| const title = (form.title || '').toLowerCase(); | |
| const desc = (form.description || '').toLowerCase(); | |
| const q = searchTerm.toLowerCase(); | |
| const matchesSearch = title.includes(q) || desc.includes(q); | |
| const matchesFilter = filter === 'all' || | |
| (filter === 'active' && form.isActive) || | |
| (filter === 'inactive' && !form.isActive); | |
| return matchesSearch && matchesFilter; | |
| }); |
🤖 Prompt for AI Agents
In frontend/src/components/FormsList.jsx around lines 47 to 56, the filter logic
calls toLowerCase() on form.description which may be null/undefined and crash;
make the search null-safe by defaulting title and description to empty strings
(e.g. const title = (form.title || '').toLowerCase(); const description =
(form.description || '').toLowerCase()) and then use those variables for
includes(searchTerm.toLowerCase()); keep the existing matchesFilter logic
intact.
| const handleSubmit = async (responsesArray) => { | ||
| try { | ||
| const answers = responsesArray.map(({ fieldId, value }) => { | ||
| if (Array.isArray(value)) { | ||
| return { | ||
| fieldId: fieldId.toString(), | ||
| answerJson: value, | ||
| answerValue: null | ||
| }; | ||
| } | ||
| return { | ||
| fieldId: fieldId.toString(), | ||
| answerValue: value, | ||
| answerJson: null | ||
| }; | ||
| }); | ||
|
|
||
| console.log('Sending to backend:', { answers }); | ||
| const result = await formsAPI.submitFormResponse(formUrl, { answers }); | ||
| console.log('Submission result:', result); |
There was a problem hiding this comment.
🧩 Analysis chain
Handle FILE inputs or disable until supported
Current payload sends File objects in answerValue for FILE fields, which won’t serialize via JSON and backend doesn’t process files. Either implement multipart upload or gate FILE type in FormPreview/FormBuilder until backend support exists.
🌐 Web query:
What’s the recommended way to upload files with axios (multipart/form-data) and Prisma-backed APIs?
💡 Result:
Short answer — recommended pattern:
-
Client: send files in a FormData (append files + fields). In browsers let axios set Content-Type (don’t set multipart Content-Type yourself). In Node clients use the form-data package and pass form.getHeaders() to axios. [1][2][5]
-
Server: avoid routing large file bytes through Prisma. Two options:
- Preferred — issue short-lived presigned upload URLs (S3/GCS/etc.) from your API, let the client upload directly, then use Prisma to save the file metadata (key/URL, size, mime, owner, timestamps). This reduces server bandwidth and simplifies scaling. [3]
- If you must accept multipart uploads on the server, parse streams with a multipart parser (multer, busboy or similar), validate and stream to object storage (not into Prisma). After successful store, record metadata in Prisma. Validate fields before/after upload as needed. [4]
Minimal example pointers
- Browser client:
- const fd = new FormData(); fd.append('file', file); axios.post('/api/sign', fd) — do not set Content-Type; axios/browser will set the boundary. [1][2]
- Direct-to-cloud flow:
- POST /api/create-upload -> server returns presigned PUT/POST URL + key
- client PUTs file to presigned URL
- client notifies server (or server verifies) -> server writes record in Prisma (url/key/userId/size). [3]
Why this is recommended
- Letting client upload directly to object storage saves server bandwidth and avoids storing blobs in the DB; Prisma is ideal for structured file metadata, not binary streams. If you accept multipart on the server, use streaming parsers and stream to storage, then persist metadata with Prisma. [3][4]
Sources: Axios multipart docs & guidance (FormData, headers) [1]; advice to not set Content-Type manually (boundary) [2]; AWS presigned-URL guidance (direct uploads) [3]; multer/multipart parsing best-practices (server-side) [4]; example client/server FormData usage tutorial [5].
I'll verify the FILE handling implementation and backend integration to confirm whether this is a valid concern.
🏁 Script executed:
#!/bin/bash
# Check for FILE field type handling
rg -n 'FILE' frontend/src --type=jsx --type=js -C3 | head -40Length of output: 27
🏁 Script executed:
#!/bin/bash
# Inspect formsAPI.submitFormResponse implementation
fd -e js -e jsx -path '*api*' -o -path '*API*' | xargs grep -l 'submitFormResponse' 2>/dev/null | head -5Length of output: 168
🏁 Script executed:
#!/bin/bash
# Check FormPreview for error handling and field type gating
fd -name 'FormPreview.jsx' -o -name 'FormBuilder.jsx' | head -5Length of output: 230
🏁 Script executed:
#!/bin/bash
# Search for multipart/FormData usage indicating file upload support
rg -n 'FormData|multipart|upload' frontend/src --type=jsx --type=js -C2 | head -50Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check backend/API route for form submission
fd -name '*.js' -o -name '*.ts' | xargs grep -l 'submitFormResponse\|form.*response\|/api.*form' 2>/dev/null | grep -v node_modules | head -10Length of output: 230
🏁 Script executed:
#!/bin/bash
# Look for Prisma schema to understand data model
fd -name 'schema.prisma' -o -name '*.prisma'Length of output: 230
🏁 Script executed:
#!/bin/bash
# Check for presigned URL or upload handlers
rg -n 'presigned|upload.*URL|s3\|gcs|formData' --type=js --type=ts --type=jsx -C2 | head -40Length of output: 27
🏁 Script executed:
#!/bin/bash
# Get file structure around FormSubmission to understand context
ls -la frontend/src/components/ | grep -i formLength of output: 529
🏁 Script executed:
#!/bin/bash
# Check FormSubmission.jsx for complete context (error handling, FILE field detection)
fd -name 'FormSubmission.jsx' -exec cat {} \;Length of output: 230
🏁 Script executed:
#!/bin/bash
# Check FormPreview.jsx
fd -name 'FormPreview.jsx' -exec cat {} \;Length of output: 230
🏁 Script executed:
#!/bin/bash
# Search for field type definitions/constants
rg -n "type.*=|const.*FIELD|fieldType" frontend/src --type=jsx --type=js | grep -i file | head -20Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check formsAPI module
fd -name '*formsAPI*' -o -name '*forms*api*' | head -5Length of output: 230
🏁 Script executed:
#!/bin/bash
# Inspect backend form submission endpoint
fd -name 'form*.js' -o -name 'form*.ts' | xargs grep -l 'POST\|put\|router' 2>/dev/null | grep -v node_modules | head -5Length of output: 230
🏁 Script executed:
#!/bin/bash
# Look for catch/error handling in FormSubmission around submission
rg -A20 'const handleSubmit' frontend/src/components/FormSubmission.jsxLength of output: 633
🏁 Script executed:
#!/bin/bash
# Check if there's any FILE field validation or filtering
rg -n 'fieldType.*FILE|FILE.*field|\.type.*FILE' frontend/src --type=jsx --type=js -C3Length of output: 27
🏁 Script executed:
#!/bin/bash
# Search for form schema definition
rg -n 'schema|field.*definition|fields.*type' frontend/src --type=jsx --type=js | grep -i 'form\|field' | head -30Length of output: 27
🏁 Script executed:
#!/bin/bash
# Look at directory structure to find API implementation
find . -type f -name '*.js' -o -name '*.ts' | grep -i 'api\|route\|backend' | grep -v node_modules | head -20Length of output: 647
🏁 Script executed:
#!/bin/bash
# Check next.js API routes
ls -la pages/api 2>/dev/null || ls -la app/api 2>/dev/null || echo "No API routes found in standard locations"Length of output: 41
🏁 Script executed:
#!/bin/bash
# Find where formsAPI is defined
rg -n 'export.*formsAPI|formsAPI\s*=' frontend/src --type=jsx --type=js -B2 -A10 | head -40Length of output: 27
🏁 Script executed:
#!/bin/bash
# Look for form response endpoint in backend
rg -n 'submitFormResponse|form.*response' . --type=js --type=ts | grep -v node_modules | head -20Length of output: 2206
🏁 Script executed:
#!/bin/bash
# Check FormPreview for error handling specifics
rg -n 'onSubmit|catch|error' frontend/src/components/FormPreview.jsx -A3 -B3 2>/dev/null | head -50Length of output: 1420
🏁 Script executed:
#!/bin/bash
# List all jsx/js files in components directory
find frontend/src/components -name '*.jsx' -o -name '*.js'Length of output: 815
🏁 Script executed:
#!/bin/bash
# Check for any existing file upload implementation
rg -n 'new FormData|\.append\(' frontend/src --type=jsx --type=js -B2 -A2 | head -40Length of output: 27
🏁 Script executed:
#!/bin/bash
# Search for any FILE type field in the entire codebase
rg -rn '\bFILE\b' . --type=js --type=ts --type=jsx | grep -v node_modules | head -30Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check if backend has multipart handling middleware
rg -n 'multer|busboy|multipart' . --type=js --type=ts | grep -v node_modules | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Get the root structure
find . -maxdepth 2 -type d -not -path '*/\.*' | head -20Length of output: 105
🏁 Script executed:
#!/bin/bash
# Check package.json for relevant dependencies
cat package.json | jq '.dependencies | keys[]' 2>/dev/null | grep -i 'form\|upload\|multer\|busboy' || cat package.json | grep -i 'form\|upload'Length of output: 89
🏁 Script executed:
#!/bin/bash
# Look for any test or documentation mentioning FILE fields
rg -n 'FILE' . --type=md | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Final check: get complete FormSubmission.jsx if not yet retrieved
cat frontend/src/components/FormSubmission.jsx 2>/dev/null || echo "File not found in expected location"Length of output: 9409
🏁 Script executed:
#!/bin/bash
# Check the actual form submission API endpoint
rg -rn 'formsAPI|/forms.*response' backend . --type=js --type=ts 2>/dev/null | head -20Length of output: 1667
🏁 Script executed:
#!/bin/bash
# Search for prisma file operations
rg -n 'prisma.*File|File.*prisma|upload' prisma/ --type=prisma 2>/dev/null || echo "Checking schema..."Length of output: 18
🏁 Script executed:
#!/bin/bash
# Check backend form controller for file handling
cat backend/src/controllers/form.jsLength of output: 43216
🏁 Script executed:
#!/bin/bash
# Check backend form routes
cat backend/src/routes/form.jsLength of output: 1056
🏁 Script executed:
#!/bin/bash
# Check frontend API definition
cat frontend/src/api/api.js | head -350Length of output: 8951
🏁 Script executed:
#!/bin/bash
# Check FormBuilder for field types it supports
cat frontend/src/components/FormBuilder.jsx | grep -i 'type\|FILE\|upload' -A2 -B2 | head -80Length of output: 3599
🏁 Script executed:
#!/bin/bash
# Search for field type constants
grep -rn 'const.*FIELD\|type.*:.*\[' frontend/src --include="*.jsx" --include="*.js" | grep -v node_modules | head -40Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check Prisma schema
find . -name 'schema.prisma'Length of output: 30
🏁 Script executed:
#!/bin/bash
# Look at complete FormPreview to understand onSubmit handling
cat frontend/src/components/FormPreview.jsxLength of output: 12291
Implement file upload support or gate FILE field type
FILE field inputs are captured in FormPreview, but the submission pipeline attempts JSON serialization of File objects—which fails silently. Backend validates FILE field rules but has no multipart parsing, streaming, or storage implementation.
Per best practices, implement presigned upload URLs (preferred) or server-side multipart parsing with streaming to object storage. Either:
- Presigned URLs: API issues short-lived signed URLs; client uploads directly; server records metadata in Prisma.
- Multipart upload: Add multipart parser (multer/busboy) on server; stream to storage; record metadata.
Alternatively, disable FILE fields in FormBuilder until backend support exists.
Secondary: FormPreview doesn't await onSubmit callback (line 76), so FormSubmission errors aren't caught. Use await onSubmit(...) to propagate errors.
🤖 Prompt for AI Agents
In frontend/src/components/FormSubmission.jsx around lines 36 to 55, the
submission currently attempts to JSON-serialize File objects (which fails) and
does not await onSubmit in FormPreview; fix by removing File objects from the
answers payload and implementing a presigned-upload flow: for each FILE field,
call a new backend presign endpoint to get a short-lived upload URL, PUT the
File directly to that URL from the client, then include only the file metadata
(storage URL/key, filename, size, mimeType) in the answers array so the backend
records metadata via Prisma; as an alternative you may temporarily gate/omit
FILE fields from submission until backend support exists; also ensure
FormPreview awaits the onSubmit callback (use await onSubmit(...)) so submission
errors propagate and can be handled.
1 participant
Orbis Forms
A complete implementation of Orbis Forms (backend + frontend) — a Google Forms–like module for the Orbis project.
Video demo: https://youtu.be/BowmN-xVW2k
Project Overview
Orbis Forms lets admins create shareable forms and collect responses.
Key features:
isEditable)Summary by CodeRabbit
Release Notes