Repository navigation
Private Forms - #147
Merged
Merged
Private Forms#147
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughPrivate application forms now support invitee-based access across Firestore, backend response routes, and frontend navigation. Super-reviewers can create private forms and manage invitees, while applicants can view and continue eligible invited applications. ChangesPrivate form access and invitations
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant SuperReviewer
participant CreatePrivateFormDialog
participant useUploadApplicationForm
participant applicationFormsService
participant Firestore
SuperReviewer->>CreatePrivateFormDialog: Enter private form details
CreatePrivateFormDialog->>useUploadApplicationForm: Upload with createOnly
useUploadApplicationForm->>applicationFormsService: Create application form
applicationFormsService->>Firestore: Create inactive private form
Firestore-->>CreatePrivateFormDialog: Creation result
CreatePrivateFormDialog-->>SuperReviewer: Show success and close dialog
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
Visit the preview URL for this PR (updated for commit 7842396): https://h4i-applications--pr147-private-forms-4ypwqc2l.web.app (expires Sat, 01 Aug 2026 21:14:56 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 22093378deab11a21f59ccd5f5596d569dc5fc0d |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/App.tsx (1)
168-181: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winGate the application-filling route with
RequireFormAccess.
FormProvidercreates/loads an in-progress response byuserId+formIdwithout checking form eligibility, so any authenticated, email-verified user can navigate to/apply/f/:formId/:sectionIdfor another user’s private form. WrapFormProviderlike therevisit/submitroutes.🔧 Proposed fix
<Route element={ <div className="w-full h-full bg-muted"> - <RequireAuth> - <FormProvider /> - </RequireAuth> + <RequireAuth> + <RequireFormAccess> + <FormProvider /> + </RequireFormAccess> + </RequireAuth> </div> } >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/App.tsx` around lines 168 - 181, Update the application-filling route in App.tsx to wrap FormProvider with RequireFormAccess, matching the protection used by the revisit and submit routes. Preserve the existing RequireAuth wrapper and route structure while ensuring access validation occurs before FormProvider loads the response.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/functions/src/middleware/authentication.ts`:
- Line 1: The public-form access checks must not treat a missing form as public.
In the backend authentication flow, update the Applicant branch around the form
access condition to require a truthy form before checking !form.isPrivate; in
frontend RequireFormAccess, require form to exist before computing
isApplicantOnPublicForm and surface the query error state instead of allowing
access after a failed fetch.
- Around line 158-170: Update canRespondToForm so the Applicant public-form
authorization branch only calls next() when form exists and is not private;
preserve the invitedUsers authorization path separately, ensuring a missing form
is denied rather than treated as public.
- Around line 117-160: Wrap the Firestore reads in canRespondToForm, including
the existingResponse lookup and form lookup, in try/catch handling so rejected
promises do not escape the Express 4 middleware. On unexpected errors, follow
the established isAuthenticated/hasRoles error-response behavior, while
preserving the existing unauthorized checks and normal authorization flow.
In `@firebase/firestore.rules`:
- Around line 81-83: Update the update rule to validate the incoming
request.resource.data.applicationFormId, preventing callers from changing the
response’s form association or otherwise bypassing eligibility; preserve the
existing checks for ownership, in-progress status, and open responses.
In `@frontend/src/components/auth/RequireFormAccess.tsx`:
- Around line 14-47: Update RequireFormAccess to handle useApplicationForm
failures and missing forms before evaluating access: inspect the hook’s error
state and require a defined form, so isApplicantOnPublicForm is only true for an
actually loaded public form. Preserve the existing invited-user and
authenticated navigation behavior, and ensure errored or non-existent forms
cannot be treated as publicly accessible.
In
`@frontend/src/components/dor/CreatePrivateFormDialog/CreatePrivateFormDialog.tsx`:
- Around line 1-198: Reformat the affected files with the project’s configured
oxfmt command and commit the formatting-only changes:
frontend/src/components/dor/CreatePrivateFormDialog/CreatePrivateFormDialog.tsx
(lines 1-198),
frontend/src/components/dor/InviteUsersDialog/InviteUsersDialog.tsx (lines
1-192), frontend/src/pages/super-reviewer/AdminHome.tsx (lines 1-331), and
frontend/src/components/ui/card.tsx (lines 1-93). Ensure the resulting files
pass the CI format check without changing behavior.
- Around line 54-108: Update the private-form submission flow around
handleSubmit and the /forms backend write to use an atomic create-or-fail guard
keyed by form.id, replacing the overwrite-prone
formsCollection.doc(form.id).set(form) behavior. Keep the existing getAllForms
duplicate check as an early client-side optimization, but handle the backend
conflict/error response in uploadForm’s onError and only treat success as
creation after the atomic guard confirms no existing form was overwritten.
- Around line 54-58: Update handleSubmit to validate formId against the allowed
alphanumeric, underscore, and hyphen pattern, requiring an alphanumeric first
character before creating the form. Show the existing error toast and return
when validation fails, while preserving the current required-field validation
and successful submission flow.
In `@frontend/src/components/dor/InviteUsersDialog/InviteUsersDialog.tsx`:
- Around line 42-46: Update the useEffect that initializes selected so it
resynchronizes only on the closed-to-open transition, rather than whenever
form.invitedUsers changes while the dialog remains open. Preserve initialization
from the current form.invitedUsers when opening, and avoid overwriting unsaved
checkbox selections during background refetches.
In `@frontend/src/pages/applicant/AppOverview.tsx`:
- Line 24: Update the useInvitedForms() destructuring in AppOverview to capture
its error and surface it alongside the existing form/application errors.
Preserve the current invitedForms data flow while ensuring a failed invitation
query is displayed as an error instead of being treated as an empty list.
---
Outside diff comments:
In `@frontend/src/App.tsx`:
- Around line 168-181: Update the application-filling route in App.tsx to wrap
FormProvider with RequireFormAccess, matching the protection used by the revisit
and submit routes. Preserve the existing RequireAuth wrapper and route structure
while ensuring access validation occurs before FormProvider loads the response.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7f0870a5-0c3c-4c36-9d18-b56a9c81a701
📒 Files selected for processing (20)
.gitignorebackend/functions/src/middleware/authentication.tsbackend/functions/src/routes/application.tsfirebase/firestore.rulesfrontend/src/App.tsxfrontend/src/components/Navbar.tsxfrontend/src/components/applicant/PrivateFormAlert.tsxfrontend/src/components/auth/RequireFormAccess.tsxfrontend/src/components/dor/CreatePrivateFormDialog/CreatePrivateFormDialog.tsxfrontend/src/components/dor/InviteUsersDialog/InviteUsersDialog.tsxfrontend/src/components/form/AssessmentSubmit.tsxfrontend/src/components/ui/card.tsxfrontend/src/components/ui/checkbox.tsxfrontend/src/hooks/useApplicationForm.tsfrontend/src/hooks/useUsers.tsfrontend/src/pages/applicant/AppOverview.tsxfrontend/src/pages/super-reviewer/AdminHome.tsxfrontend/src/services/applicationFormsService.tsfrontend/src/utils/form.tsshared/src/types/form.ts
Comment on lines
+1
to
+198
| import { Timestamp } from "firebase/firestore"; | ||
| import { ChevronDownIcon } from "lucide-react"; | ||
| import { useState } from "react"; | ||
|
|
||
| import { throwErrorToast } from "@/components/toasts/ErrorToast"; | ||
| import { throwSuccessToast } from "@/components/toasts/SuccessToast"; | ||
| import { Button } from "@/components/ui/button"; | ||
| import { Calendar } from "@/components/ui/calendar"; | ||
| import { | ||
| Dialog, | ||
| DialogClose, | ||
| DialogContent, | ||
| DialogDescription, | ||
| DialogFooter, | ||
| DialogHeader, | ||
| DialogTitle, | ||
| } from "@/components/ui/dialog"; | ||
| import { Input } from "@/components/ui/input"; | ||
| import { Label } from "@/components/ui/label"; | ||
| import { | ||
| Popover, | ||
| PopoverContent, | ||
| PopoverTrigger, | ||
| } from "@/components/ui/popover"; | ||
| import { h4iApplicationForm } from "@/data/h4i-application-form"; | ||
| import { useUploadApplicationForm } from "@/hooks/useApplicationForm"; | ||
| import { useAuth } from "@/hooks/useAuth"; | ||
| import { getAllForms } from "@/services/applicationFormsService"; | ||
| import type { ApplicationForm } from "@/types/types"; | ||
|
|
||
| // Two weeks out, at the same time of day — just a sensible starting point | ||
| // admins can change afterward via "Change due date". | ||
| const defaultDueDate = () => { | ||
| const date = new Date(); | ||
| date.setDate(date.getDate() + 14); | ||
| return date; | ||
| }; | ||
|
|
||
| export default function CreatePrivateFormDialog({ | ||
| open, | ||
| onOpenChange, | ||
| }: { | ||
| open: boolean; | ||
| onOpenChange: (open: boolean) => void; | ||
| }) { | ||
| const [formId, setFormId] = useState(""); | ||
| const [semester, setSemester] = useState(""); | ||
| const [dueDate, setDueDate] = useState<Date>(defaultDueDate()); | ||
| const [popoverOpen, setPopoverOpen] = useState(false); | ||
|
|
||
| const { token } = useAuth(); | ||
| const { mutate: uploadForm, isPending } = useUploadApplicationForm(); | ||
|
|
||
| const handleSubmit = async () => { | ||
| if (!formId.trim() || !semester.trim()) { | ||
| throwErrorToast("Please fill out all fields."); | ||
| return; | ||
| } | ||
|
|
||
| const tok = await token(); | ||
| if (!tok) { | ||
| throwErrorToast("Not authenticated."); | ||
| return; | ||
| } | ||
|
|
||
| let existingForms; | ||
| try { | ||
| existingForms = await getAllForms(); | ||
| } catch (error) { | ||
| console.error(error); | ||
| throwErrorToast("Failed to check existing form IDs. Please try again."); | ||
| return; | ||
| } | ||
| if (existingForms.some((form) => form.id === formId.trim())) { | ||
| throwErrorToast("Form ID already exists"); | ||
| return; | ||
| } | ||
|
|
||
| const newForm: ApplicationForm = { | ||
| ...h4iApplicationForm, | ||
| id: formId.trim(), | ||
| semester: semester.trim(), | ||
| dueDate: Timestamp.fromDate(dueDate), | ||
| isActive: false, | ||
| isPrivate: true, | ||
| invitedUsers: [], | ||
| decisionsReleased: false, | ||
| }; | ||
|
|
||
| uploadForm( | ||
| { form: newForm, token: tok }, | ||
| { | ||
| onSuccess: () => { | ||
| throwSuccessToast( | ||
| "Private form created. Edit its content in the form builder, then invite users.", | ||
| ); | ||
| setFormId(""); | ||
| setSemester(""); | ||
| setDueDate(defaultDueDate()); | ||
| onOpenChange(false); | ||
| }, | ||
| onError: (error) => { | ||
| throwErrorToast("An error occurred while creating the form."); | ||
| console.error(error); | ||
| }, | ||
| }, | ||
| ); | ||
| }; | ||
|
|
||
| return ( | ||
| <Dialog open={open} onOpenChange={onOpenChange}> | ||
| <DialogContent | ||
| onCloseAutoFocus={(event) => { | ||
| event.preventDefault(); | ||
| document.body.style.pointerEvents = ""; | ||
| }} | ||
| > | ||
| <DialogHeader> | ||
| <DialogTitle>Create Private Form</DialogTitle> | ||
| <DialogDescription> | ||
| A private form starts inactive and hidden from all applicants. Once | ||
| created, invite the users who should see it from the form dropdown | ||
| menu. | ||
| </DialogDescription> | ||
| </DialogHeader> | ||
| <div className="grid gap-4"> | ||
| <div className="grid gap-3"> | ||
| <Label htmlFor="private-form-id">Form ID</Label> | ||
| <Input | ||
| id="private-form-id" | ||
| placeholder="unique-form-id" | ||
| value={formId} | ||
| onChange={(e) => setFormId(e.target.value)} | ||
| /> | ||
| </div> | ||
| <div className="grid gap-3"> | ||
| <Label htmlFor="private-form-semester">Semester</Label> | ||
| <Input | ||
| id="private-form-semester" | ||
| placeholder="Fall 2025" | ||
| value={semester} | ||
| onChange={(e) => setSemester(e.target.value)} | ||
| /> | ||
| </div> | ||
| <div className="grid gap-3"> | ||
| <Label htmlFor="private-form-due-date" className="px-1"> | ||
| Due Date | ||
| </Label> | ||
| <Popover open={popoverOpen} onOpenChange={setPopoverOpen}> | ||
| <PopoverTrigger asChild> | ||
| <Button | ||
| variant="outline" | ||
| id="private-form-due-date" | ||
| className="w-full justify-between font-normal" | ||
| > | ||
| {dueDate.toLocaleString()} | ||
| <ChevronDownIcon /> | ||
| </Button> | ||
| </PopoverTrigger> | ||
| <PopoverContent | ||
| className="w-auto overflow-hidden p-0" | ||
| align="start" | ||
| > | ||
| <Calendar | ||
| mode="single" | ||
| selected={dueDate} | ||
| defaultMonth={dueDate} | ||
| hidden={{ before: new Date() }} | ||
| onSelect={(newDate) => { | ||
| if (newDate) { | ||
| const merged = new Date(newDate); | ||
| merged.setHours( | ||
| dueDate.getHours(), | ||
| dueDate.getMinutes(), | ||
| dueDate.getSeconds(), | ||
| 0, | ||
| ); | ||
| setDueDate(merged); | ||
| } | ||
| setPopoverOpen(false); | ||
| }} | ||
| /> | ||
| </PopoverContent> | ||
| </Popover> | ||
| </div> | ||
| </div> | ||
| <DialogFooter> | ||
| <DialogClose asChild> | ||
| <Button variant="outline">Cancel</Button> | ||
| </DialogClose> | ||
| <Button type="submit" onClick={handleSubmit} disabled={isPending}> | ||
| {isPending ? "Creating..." : "Create"} | ||
| </Button> | ||
| </DialogFooter> | ||
| </DialogContent> | ||
| </Dialog> | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
CI format check (oxfmt --check .) fails for this PR. All four files in this layer report the same "Checks / Format check" and "Checks / 3_Format check.txt" failures, indicating the PR branch isn't formatted per the project's oxfmt config.
frontend/src/components/dor/CreatePrivateFormDialog/CreatePrivateFormDialog.tsx#L1-L198: run the project's format command (e.g.oxfmt --write .ornpm run format) and commit the result.frontend/src/components/dor/InviteUsersDialog/InviteUsersDialog.tsx#L1-L192: same — reformat and commit.frontend/src/pages/super-reviewer/AdminHome.tsx#L1-L331: same — reformat and commit.frontend/src/components/ui/card.tsx#L1-L93: same — reformat and commit.
🧰 Tools
🪛 GitHub Actions: Checks / 3_Format check.txt
[error] 1-1: Command failed: oxfmt --check . (exit status 1). Step: frontend format:check
🪛 GitHub Actions: Checks / Format check
[error] 1-1: frontend@0.0.0 format:check failed: oxfmt --check . (exit status 1).
📍 Affects 4 files
frontend/src/components/dor/CreatePrivateFormDialog/CreatePrivateFormDialog.tsx#L1-L198(this comment)frontend/src/components/dor/InviteUsersDialog/InviteUsersDialog.tsx#L1-L192frontend/src/pages/super-reviewer/AdminHome.tsx#L1-L331frontend/src/components/ui/card.tsx#L1-L93
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@frontend/src/components/dor/CreatePrivateFormDialog/CreatePrivateFormDialog.tsx`
around lines 1 - 198, Reformat the affected files with the project’s configured
oxfmt command and commit the formatting-only changes:
frontend/src/components/dor/CreatePrivateFormDialog/CreatePrivateFormDialog.tsx
(lines 1-198),
frontend/src/components/dor/InviteUsersDialog/InviteUsersDialog.tsx (lines
1-192), frontend/src/pages/super-reviewer/AdminHome.tsx (lines 1-331), and
frontend/src/components/ui/card.tsx (lines 1-93). Ensure the resulting files
pass the CI format check without changing behavior.
Source: Pipeline failures
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/functions/src/routes/grading.ts (1)
155-165: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRestore runtime validation before casting Firestore form data.
ApplicationFormSchema.safeParse(...)is commented out whileformDoc.data()is cast withas ApplicationForm; this bypasses runtime validation for malformed Firestore documents and lets invalid form data flow into the grading task payload.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/functions/src/routes/grading.ts` around lines 155 - 165, Restore the ApplicationFormSchema.safeParse validation before the form assignment in the grading route. Validate formDoc.data(), log formResult.error and return the existing 500 response for invalid data, and only cast or use the parsed form after validation succeeds.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/dor/DuplicateFormDialog/DuplicateFormDialog.tsx`:
- Around line 58-63: Add the same form-ID validation used by
CreatePrivateFormDialog to DuplicateFormDialog before invoking duplicateForm,
including its targeted toast for invalid or duplicate IDs. Reuse the existing
validation logic and ensure submission is skipped when the ID fails validation;
retain the current onError handling for other duplication failures.
---
Outside diff comments:
In `@backend/functions/src/routes/grading.ts`:
- Around line 155-165: Restore the ApplicationFormSchema.safeParse validation
before the form assignment in the grading route. Validate formDoc.data(), log
formResult.error and return the existing 500 response for invalid data, and only
cast or use the parsed form after validation succeeds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f6abc6b8-4f04-4bbe-9c28-c2700be2d726
📒 Files selected for processing (14)
backend/functions/src/middleware/authentication.tsbackend/functions/src/routes/application.tsbackend/functions/src/routes/grading.tsfirebase/firestore.rulesfrontend/src/App.tsxfrontend/src/components/auth/RequireFormAccess.tsxfrontend/src/components/dor/CreatePrivateFormDialog/CreatePrivateFormDialog.tsxfrontend/src/components/dor/DuplicateFormDialog/DuplicateFormDialog.tsxfrontend/src/components/dor/InviteUsersDialog/InviteUsersDialog.tsxfrontend/src/hooks/useApplicationForm.tsfrontend/src/pages/applicant/AppOverview.tsxfrontend/src/pages/super-reviewer/AdminHome.tsxfrontend/src/services/applicationFormsService.tsfrontend/src/utils/form.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- frontend/src/components/auth/RequireFormAccess.tsx
- frontend/src/pages/applicant/AppOverview.tsx
- frontend/src/pages/super-reviewer/AdminHome.tsx
- frontend/src/services/applicationFormsService.ts
- firebase/firestore.rules
- frontend/src/components/dor/InviteUsersDialog/InviteUsersDialog.tsx
- backend/functions/src/middleware/authentication.ts
- frontend/src/App.tsx
Member
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit