Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMoves client-side callback invocation to a new server-side connection handler, centralizes form creation via a new createForm utility, auto-creates event feedback forms when missing, and URL-encodes form slugs across UI links and QR generation. Also removes the standalone eventFeedback router. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Form Responder (browser)
participant Server as Next.js Server
participant ConnHandler as Connection Handler
participant AppRouter as appRouter / Procedures
participant Logger as Logger
Client->>Server: onSuccess(responseData)
Server->>ConnHandler: handleCallbacks(formName, formId, responseData)
ConnHandler->>AppRouter: authenticate & fetch connections for form
AppRouter-->>ConnHandler: connections list
loop per connection
ConnHandler->>AppRouter: resolve procedure from connection.route
alt procedure resolved
ConnHandler->>AppRouter: call procedure(payload)
alt success
AppRouter-->>ConnHandler: result
ConnHandler->>Logger: log success (form, user, proc)
else error
AppRouter-->>ConnHandler: error
ConnHandler->>Logger: log error (payload, error)
end
else not resolved
ConnHandler->>Logger: skip connection (missing proc)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
DGoel1602
added
Feature
DGoel1602
force-pushed
the
blade/embeddable-forms
branch
from
c8cd569 to
5b437dc
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@apps/blade/src/app/forms/`[formName]/_components/connection-handler.ts:
- Around line 49-61: The log currently includes raw response payload via
stringify(data) in connection-handler.ts (inside the catch that logs failures
for con.proc triggered after submission name and session.user.name), which risks
PII exposure; change the logging to emit only field names or a redacted payload
instead of full values — e.g., replace stringify(data) usage with a sanitized
representation (Object.keys(data) or map of keys to "[REDACTED]") and include
that sanitized string in the message passed to log({ ... }) so the rest of the
context (title, con.proc, name, session.user.discordUserId) remains unchanged.
- Around line 12-23: The handleCallbacks server action (function
handleCallbacks) can throw unhandled rejections from calls to auth(),
api.forms.getConnections(), or extractProcedures(appRouter); wrap the entire
body of handleCallbacks in a top-level try/catch that catches any error, logs or
records it (use your existing logger or processLogger) and returns/early-exits
gracefully so the client call does not receive an unhandled rejection. Ensure
the catch covers all awaited calls (auth, api.forms.getConnections,
extractProcedures) and reuses the same return shape the caller expects when
failing.
In `@packages/api/src/routers/forms.ts`:
- Around line 137-138: getForm currently calls decodeURIComponent on
input.slug_name but deleteForm, updateFormSection, and checkFormEditAccess do
not, causing inconsistent slug lookups; update each slug-based lookup (the where
clauses in deleteForm, updateFormSection, and checkFormEditAccess) to apply
decodeURIComponent(input.slug_name) (or decode the slug once at start of each
procedure) so all procedures use the same decoded slug value—alternatively, if
your API guarantees pre-decoded slugs, remove decodeURIComponent from getForm
instead, but ensure the same approach is applied across getForm, deleteForm,
updateFormSection, and checkFormEditAccess.
In `@packages/api/src/utils.ts`:
- Around line 523-531: The code silently falls back to "General" when
input.section is provided but not found; update the block that queries
FormSections (the sectionName, sectionId variables and the
db.query.FormSections.findFirst call) to detect when input.section is explicitly
provided yet section is null and then emit a clear warning (using the project's
logger such as processLogger.warn or a similar logger) including the missing
sectionName and context (e.g., caller like member.ts), and optionally fail fast
or surface the error depending on project policy; ensure sectionId remains null
only after logging so maintain current behavior if you choose to just warn.
- Around line 533-552: The onConflictDoUpdate in the FormsSchemas insert is dead
because id is autogenerated (defaultRandom()) so primary-key conflict won't
occur; either make upsert-by-slug work by changing the conflict target to
slugName and ensuring the DB has a unique constraint on FormsSchemas.slugName
(and keep set to the same fields), or if you only intend create-only behavior
remove the onConflictDoUpdate and use a plain insert; locate the insert(...)
call using FormsSchemas and update the .onConflictDoUpdate target or delete that
clause accordingly.
🧹 Nitpick comments (2)
packages/api/src/routers/member.ts (2)
500-576: Consider wrapping form creation in try/catch to prevent check-in failures.If
createFormthrows (e.g., DB error, section lookup failure), the entire check-in fails. Since the feedback form is supplementary to the check-in, you may want to catch and log errors rather than block the member from checking in.🛡️ Suggested improvement
if (form === undefined) { + try { await createForm({ formData: { name: formName, // ... rest of form data }, allowEdit: false, allowResubmission: false, duesOnly: false, section: "Feedback", }); + } catch (error) { + console.error("Failed to create feedback form:", error); + // Continue with check-in even if form creation fails + } }
519-568: Consider extracting default feedback questions to a shared constant.These hardcoded questions could be defined in
@forge/constsalongside other form-related constants. This would make future modifications easier and keep the procedure focused on orchestration.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/blade/src/app/forms/`[formName]/_components/form-responder-client.tsx:
- Around line 121-124: The Link rendering in form-responder-client.tsx uses
existing.formSlug directly and can produce a broken path when formSlug is
null/undefined; update the conditional to only render the <Link> when existing
&& existing.formSlug (truthy) and use encodeURIComponent(existing.formSlug)
inside the href, otherwise render a safe fallback (e.g., no link or plain text).
Locate the block around the existing check in the component (the existing
variable and the Link element) and change the guard to require existing.formSlug
before constructing `/forms/${encodeURIComponent(...)}/${existing.id}` so you
never build `/forms//...`.
In `@packages/api/src/utils.ts`:
- Line 10: Replace the default Zod import with the codebase-standard named
import: change the current default import of z to a named import (use "import {
z } from 'zod'") so usages of the z identifier (e.g., any calls like z.object,
z.string) remain unchanged and consistent with other files.
🧹 Nitpick comments (1)
packages/api/src/routers/member.ts (1)
507-575: Consider extracting the feedback-form definition to a helper.This inline definition is large and (if used in multiple flows, e.g., createMember) it’s easy for the question set or slug logic to drift. A small helper improves reuse and keeps updates consistent.
♻️ Example refactor (block-level change)
- const formName = event.name + " Feedback Form"; - const formSlugName = formName.toLowerCase().replaceAll(" ", "-"); + const { formName, formSlugName, formData } = buildFeedbackForm(event); const form = await db.query.FormsSchemas.findFirst({ where: (t, { eq }) => eq(t.slugName, formSlugName), }); - if (form === undefined) { - await createForm({ - formData: { - name: formName, - description: `Provide feedback for ${event.name} to help us make events better in the future!`, - questions: [ /* ... */ ], - }, - allowEdit: false, - allowResubmission: false, - duesOnly: false, - section: "Feedback", - }); - } + if (form === undefined) { + await createForm({ + formData, + allowEdit: false, + allowResubmission: false, + duesOnly: false, + section: "Feedback", + }); + }Example helper (placed near the router helpers):
function buildFeedbackForm(event: Event) { const formName = `${event.name} Feedback Form`; const formSlugName = formName.toLowerCase().replaceAll(" ", "-"); const formData: FORMS.FormType = { name: formName, description: `Provide feedback for ${event.name} to help us make events better in the future!`, questions: [/* existing questions */], }; return { formName, formSlugName, formData }; }
apps/blade/src/app/forms/[formName]/_components/form-responder-client.tsx
Outdated
Show resolved
Hide resolved
DGoel1602
force-pushed
the
blade/embeddable-forms
branch
from
ea6ca4c to
42eb5f6
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@apps/blade/src/app/dashboard/_components/member-dashboard/member-dashboard.tsx`:
- Around line 105-107: The current events.value.forEach(async ...) in
member-dashboard.tsx fires ensureForm calls without awaiting them so the server
component renders before forms are ready and unhandled rejections can occur;
replace the forEach with awaiting a batched call like Promise.allSettled over
events.value.map(...) so all api.event.ensureForm({ eventId: e.id }) promises
are awaited before returning JSX, and handle results/errors as needed
(referencing events.value and api.event.ensureForm).
In `@packages/api/src/routers/event.ts`:
- Around line 518-605: The current ensureForm procedure (ensureForm) uses a
check-then-insert flow against FormsSchemas.slugName which lacks a UNIQUE
constraint, causing race-condition duplicates and it runs under
protectedProcedure allowing any authenticated user to create forms; fix by
adding .unique() to FormsSchemas.slugName in the DB schema (knight-hacks.ts),
restrict the router to the proper permission scope (replace protectedProcedure
with permProcedure or add an explicit authorization check that the caller can
manage the event), and make ensureForm resilient by wrapping the createForm call
in a try/catch that on unique-constraint failure (or duplicate key DB error)
re-queries FormsSchemas for the slug and proceeds without throwing so concurrent
requests converge to a single created form.
apps/blade/src/app/dashboard/_components/member-dashboard/member-dashboard.tsx
Outdated
Show resolved
Hide resolved
DGoel1602
force-pushed
the
blade/embeddable-forms
branch
from
44a5cb2 to
7ff53c7
Compare
DVidal1205
requested changes
Feb 13, 2026
Contributor
DVidal1205
left a comment
DVidal1205
left a comment
There was a problem hiding this comment.
small change rest looks tuff
DGoel1602
force-pushed
the
blade/embeddable-forms
branch
from
0f8495e to
4b2cd0c
Compare
Contributor
|
Why did we force push? 🤔 |
…another abstract layer
…o ensure consistency
DGoel1602
force-pushed
the
blade/embeddable-forms
branch
from
4b2cd0c to
e3d244c
Compare
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
3 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.
Why
Feedback was unviewable but more importantly we weren't using the full extent of blade forms by embedding them in blade.
What
Issue(s): #341
Test Plan
Lmk know if we want it to not scroll I just didn't wanna do the CSS for the most part to compress it
Checklist
db:pushbefore mergingSummary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores