QoL: Settings page remembers tab#154
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds seven Prisma models for a Forms feature ( ChangesOrbit Forms Feature and Settings Refactor
Sequence DiagramsequenceDiagram
participant User
participant SettingsPage
participant Router
participant FormsComponent
participant RecoilState
User->>SettingsPage: clicks "Features" sidebar tab
SettingsPage->>Router: router.replace({ query: { t: "features" } }, shallow)
Router-->>SettingsPage: query.t = "features"
SettingsPage->>SettingsPage: decodeTab("features") -> setActiveSection
SettingsPage->>FormsComponent: render with triggerToast prop
FormsComponent->>RecoilState: useRecoilValue(workspacestate)
RecoilState-->>FormsComponent: workspace.settings?.policiesEnabled
FormsComponent-->>User: renders Forms toggle row
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pages/api/forms/main.ts (1)
1-22:⚠️ Potential issue | 🔴 CriticalMissing default handler export in API route
Both
pages/api/forms/main.tsandpages/api/forms/specific.tslack default handler exports. In Next.js Pages Router, every API route must export a default handler function. These files currently contain only JSDoc headers and imports, making the/api/formsand/api/forms/:formIdroutes invalid.🤖 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 `@pages/api/forms/main.ts` around lines 1 - 22, The file pages/api/forms/main.ts is missing a default handler export that Next.js Pages Router requires for API routes. After the imports and before or at the end of the file, add a default export function that accepts NextApiRequest and NextApiResponse parameters. This handler function should implement the logic for the GET and POST routes mentioned in the JSDoc header (for listing and creating forms), and should include appropriate permission checks using the Forms.View and Forms.Create permissions referenced in the comments. Similarly, apply the same fix to pages/api/forms/specific.ts to handle the specific form routes.
🤖 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 `@components/settings/general/form.tsx`:
- Around line 29-33: The SwitchComponenet is bound to the wrong setting and
lacks an update mechanism. Change the checked prop from
workspace.settings?.policiesEnabled to the correct Forms-related setting (likely
workspace.settings?.formsEnabled or similar), and add an onChange handler that
updates the Forms setting when the user toggles the switch. This ensures the
component reflects the actual Forms state and allows users to change it from the
settings panel.
In `@pages/api/forms/specific.ts`:
- Around line 1-20: The file pages/api/forms/specific.ts contains only
documentation but lacks the required default export handler function for the
Next.js Pages Router API route. Add a default export async function that accepts
the standard Next.js request and response objects (req and res parameters), and
implement handlers for the three HTTP methods mentioned in the documentation:
GET, PATCH, and DELETE for the /api/forms/:formId endpoint. The function should
check the req.method property to route to the appropriate handler logic for each
HTTP verb.
In `@pages/workspace/`[id]/settings.tsx:
- Around line 311-318: The URL tab decoding validation in the useEffect hook
only checks against SECTIONS but not availableSections, allowing users to
deep-link to sections they lack access to. Modify the condition in the useEffect
hook (where router.query.t is decoded) to validate the decoded tab not only
exists in SECTIONS but also exists in availableSections before setting it as the
active section. Additionally, the fallback guard logic that prevents invalid
activeSection values currently has an empty dependency array and runs only once,
so it won't catch later invalid changes to activeSection. Add activeSection to
the dependency array of that fallback guard useEffect hook to ensure it
validates whenever activeSection changes.
In `@prisma/schema.prisma`:
- Around line 816-842: The reviewerId field in the formReview model and the
authorId field in the formComment model are stored as unconstrained scalar
fields without foreign key relations to the user model, which allows orphaned
records. Add proper `@relation` fields that reference the user model with
appropriate foreign key constraints for both the formReview and formComment
models. For formReview, add a relation field for the user referenced by
reviewerId with `@relation` decorator pointing to the user model and establish the
field constraint. Similarly, for formComment, add a relation field for the user
referenced by authorId. This will enforce referential integrity at the database
level and prevent users from being deleted if they have associated reviews or
comments.
- Around line 784-800: The formSubmission model stores workspaceGroupId as a
denormalized field but lacks a foreign key constraint to ensure referential
integrity. Add a relation constraint for workspaceGroupId that references the
workspace table (similar to how formId is constrained to the form table using
the `@relation` decorator and references), ensuring that the workspaceGroupId on
each formSubmission matches the workspaceGroupId of its related form and
preventing data corruption in workspace-scoped queries.
- Around line 774-775: The field name `updatedat` on line 774 uses an
inconsistent naming convention with a lowercase 'a' instead of camelCase. Change
the field name from `updatedat` to `updatedAt` to match the naming conventions
used throughout the rest of the Prisma schema and maintain consistency across
all model consumers.
---
Outside diff comments:
In `@pages/api/forms/main.ts`:
- Around line 1-22: The file pages/api/forms/main.ts is missing a default
handler export that Next.js Pages Router requires for API routes. After the
imports and before or at the end of the file, add a default export function that
accepts NextApiRequest and NextApiResponse parameters. This handler function
should implement the logic for the GET and POST routes mentioned in the JSDoc
header (for listing and creating forms), and should include appropriate
permission checks using the Forms.View and Forms.Create permissions referenced
in the comments. Similarly, apply the same fix to pages/api/forms/specific.ts to
handle the specific form routes.
🪄 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
Run ID: d706552c-bc94-42b5-b722-018d3747d4cf
📒 Files selected for processing (7)
components/settings/general/form.tsxcomponents/settings/general/index.tspages/api/forms/helpers.tspages/api/forms/main.tspages/api/forms/specific.tspages/workspace/[id]/settings.tsxprisma/schema.prisma
Comment on lines
+29
to
+33
| <SwitchComponenet | ||
| checked={workspace.settings?.policiesEnabled} | ||
| label="" | ||
| classoverride="mt-0" | ||
| /> |
Contributor
There was a problem hiding this comment.
Forms toggle is wired to the wrong setting and has no update path
Line 30 binds to workspace.settings?.policiesEnabled, so this switch reflects Policies state, not Forms state. Also, no onChange is passed, so users cannot actually toggle Forms behavior from this panel.
🤖 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 `@components/settings/general/form.tsx` around lines 29 - 33, The
SwitchComponenet is bound to the wrong setting and lacks an update mechanism.
Change the checked prop from workspace.settings?.policiesEnabled to the correct
Forms-related setting (likely workspace.settings?.formsEnabled or similar), and
add an onChange handler that updates the Forms setting when the user toggles the
switch. This ensures the component reflects the actual Forms state and allows
users to change it from the settings panel.
Comment on lines
+1
to
+20
| /** | ||
| * Orbit Forms | ||
| * Licensed under GPL-3.0 (see LICENSE for details) | ||
| * | ||
| * Form collection endpoint. | ||
| * Used for listing existing forms and creating new forms. | ||
| * | ||
| * Routes: | ||
| * GET /api/forms/:formId | ||
| * PATCH /api/forms/:formId | ||
| * DELETE /api/forms/:formId | ||
| * | ||
| * Permissions: | ||
| * - Forms.View | ||
| * - Forms.Create | ||
| * | ||
| * @module api/forms | ||
| * @author BuddyWinte | ||
| * @since 2.1.10-beta20 | ||
| */ No newline at end of file |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "specific.ts" -path "*/api/forms/*" -type fRepository: PlanetaryOrbit/orbit
Length of output: 94
🏁 Script executed:
cat pages/api/forms/specific.tsRepository: PlanetaryOrbit/orbit
Length of output: 450
Add default handler export to complete the API route
File contains only module documentation with no handler function. Next.js Pages Router API routes require a default export; without it, the /api/forms/:formId endpoint is non-functional.
🤖 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 `@pages/api/forms/specific.ts` around lines 1 - 20, The file
pages/api/forms/specific.ts contains only documentation but lacks the required
default export handler function for the Next.js Pages Router API route. Add a
default export async function that accepts the standard Next.js request and
response objects (req and res parameters), and implement handlers for the three
HTTP methods mentioned in the documentation: GET, PATCH, and DELETE for the
/api/forms/:formId endpoint. The function should check the req.method property
to route to the appropriate handler logic for each HTTP verb.
Comment on lines
+311
to
+318
| useEffect(() => { | ||
| const t = router.query.t as string | undefined; | ||
| const decoded = decodeTab(t ?? null); | ||
|
|
||
| if (decoded && SECTIONS[decoded as keyof typeof SECTIONS]) { | ||
| setActiveSection(decoded); | ||
| } | ||
| }, [router.query.t]); |
Contributor
There was a problem hiding this comment.
URL tab decoding bypasses section access checks
Line 315 validates decoded tabs against SECTIONS only, not availableSections. A user can deep-link t to sections they shouldn’t access in the sidebar. Also, the fallback guard at Lines 344-351 runs only once ([]) and won’t correct later invalid activeSection changes.
Suggested fix
useEffect(() => {
const t = router.query.t as string | undefined;
const decoded = decodeTab(t ?? null);
+ const allowed = new Set(availableSections.map(([key]) => key));
- if (decoded && SECTIONS[decoded as keyof typeof SECTIONS]) {
+ if (decoded && allowed.has(decoded)) {
setActiveSection(decoded);
}
- }, [router.query.t]);
+ }, [router.query.t, availableSections]);
useEffect(() => {
if (
availableSections.length > 0 &&
!availableSections.find(([key]) => key === activeSection)
) {
setActiveSection(availableSections[0][0]);
}
- }, []);
+ }, [availableSections, activeSection]);Also applies to: 331-342, 344-351
🤖 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 `@pages/workspace/`[id]/settings.tsx around lines 311 - 318, The URL tab
decoding validation in the useEffect hook only checks against SECTIONS but not
availableSections, allowing users to deep-link to sections they lack access to.
Modify the condition in the useEffect hook (where router.query.t is decoded) to
validate the decoded tab not only exists in SECTIONS but also exists in
availableSections before setting it as the active section. Additionally, the
fallback guard logic that prevents invalid activeSection values currently has an
empty dependency array and runs only once, so it won't catch later invalid
changes to activeSection. Add activeSection to the dependency array of that
fallback guard useEffect hook to ensure it validates whenever activeSection
changes.
Comment on lines
+774
to
+775
| updatedat DateTime @updatedAt | ||
| form form @relation(fields: [formId], references: [id], onDelete: Cascade) |
Contributor
There was a problem hiding this comment.
updatedat creates a mismatched timestamp field
Line 774 uses updatedat (lowercase a) while the rest of the schema consistently uses updatedAt. This creates a separate column and breaks expected field naming across model consumers.
Suggested fix
- updatedat DateTime `@updatedAt`
+ updatedAt DateTime `@updatedAt`🤖 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 `@prisma/schema.prisma` around lines 774 - 775, The field name `updatedat` on
line 774 uses an inconsistent naming convention with a lowercase 'a' instead of
camelCase. Change the field name from `updatedat` to `updatedAt` to match the
naming conventions used throughout the rest of the Prisma schema and maintain
consistency across all model consumers.
Comment on lines
+784
to
+800
| workspaceGroupId Int | ||
| userId BigInt? | ||
| robloxUserId BigInt? | ||
| discordUserId String? | ||
| status String @default("SUBMITTED") | ||
| metadata Json? | ||
| submittedAt DateTime @default(now()) | ||
| updatedAt DateTime @updatedAt | ||
| form form @relation(fields: [formId], references: [id], onDelete: Cascade) | ||
| answers formAnswer[] | ||
| reviews formReview[] | ||
| comments formComment[] | ||
| auditLogs formAuditLog[] | ||
|
|
||
| @@index([formId]) | ||
| @@index([workspaceGroupId]) | ||
| @@index([status]) |
Contributor
There was a problem hiding this comment.
formSubmission.workspaceGroupId is denormalized without a relation constraint
Line 784 stores workspaceGroupId, but the model only enforces formId (Line 792) and has no FK to workspace. This allows mismatches between form.workspaceGroupId and formSubmission.workspaceGroupId, which can corrupt workspace-scoped queries.
🤖 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 `@prisma/schema.prisma` around lines 784 - 800, The formSubmission model stores
workspaceGroupId as a denormalized field but lacks a foreign key constraint to
ensure referential integrity. Add a relation constraint for workspaceGroupId
that references the workspace table (similar to how formId is constrained to the
form table using the `@relation` decorator and references), ensuring that the
workspaceGroupId on each formSubmission matches the workspaceGroupId of its
related form and preventing data corruption in workspace-scoped queries.
Comment on lines
+816
to
+842
| model formReview { | ||
| id String @id @default(uuid()) @db.Uuid | ||
| submissionId String @db.Uuid | ||
| reviewerId BigInt | ||
| vote String? | ||
| score Int? | ||
| notes String? | ||
| createdAt DateTime @default(now()) | ||
| updatedAt DateTime @updatedAt | ||
| submission formSubmission @relation(fields: [submissionId], references: [id], onDelete: Cascade) | ||
|
|
||
| @@index([submissionId]) | ||
| @@index([reviewerId]) | ||
| } | ||
|
|
||
| model formComment { | ||
| id String @id @default(uuid()) @db.Uuid | ||
| submissionId String @db.Uuid | ||
| authorId BigInt | ||
| content String | ||
| internal Boolean @default(false) | ||
| createdAt DateTime @default(now()) | ||
| updatedAt DateTime @updatedAt | ||
| submission formSubmission @relation(fields: [submissionId], references: [id], onDelete: Cascade) | ||
|
|
||
| @@index([submissionId]) | ||
| } |
Contributor
There was a problem hiding this comment.
Reviewer/author IDs are unconstrained scalar fields
Lines 819 and 834 store user IDs without @relation to user. That removes FK enforcement and allows orphaned review/comment authors, weakening auditability and consistency.
🤖 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 `@prisma/schema.prisma` around lines 816 - 842, The reviewerId field in the
formReview model and the authorId field in the formComment model are stored as
unconstrained scalar fields without foreign key relations to the user model,
which allows orphaned records. Add proper `@relation` fields that reference the
user model with appropriate foreign key constraints for both the formReview and
formComment models. For formReview, add a relation field for the user referenced
by reviewerId with `@relation` decorator pointing to the user model and establish
the field constraint. Similarly, for formComment, add a relation field for the
user referenced by authorId. This will enforce referential integrity at the
database level and prevent users from being deleted if they have associated
reviews or comments.
breadddevv
approved these changes
Jun 19, 2026
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
2 participants
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.
The settings page now stores the current tab as base-64 via a
turl param. This is to allow refreshing, cache clearing and sharing to be a lot easier.Also added the WIP Forms tab with a..slight rework of how it works
Whoever originally made this, I hope both sides of your pillow are warm tonight.
IGNORE THE FORM API ROUTES, THEY ARE COMING SOON
Summary by CodeRabbit