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:
📝 WalkthroughWalkthroughAdds role-based visibility and an operations-calendar flag for events across DB, API, UI, and cron; updates create/update/delete flows to branch on internal vs external events (Discord/Google behavior); migration docs extended. No public APIs removed. Changes
Sequence DiagramsequenceDiagram
participant Admin as Admin User
participant Form as Blade Event Form
participant API as Event Router
participant DB as Database
participant Discord as Discord API
participant GCal as Google Calendar API
Admin->>Form: Fill event + select `roles` + toggle `isOperationsCalendar`
Form->>API: POST createEvent({ ..., roles, isOperationsCalendar })
API->>DB: Insert Event (roles, isOperationsCalendar, ...)
DB-->>API: Insert OK
API->>API: Determine internal vs external (isOperationsCalendar)
alt Internal (isOperationsCalendar = true)
API->>Discord: Create event using DISCORD_OPS_VOICE_CHANNEL_ID
API->>GCal: Create on operations calendar
else External (isOperationsCalendar = false)
API->>Discord: Create on standard channel
API->>GCal: Create on public events calendar
end
API-->>Form: Success
Form-->>Admin: Event created
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 7 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (7 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}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/api/src/routers/event.ts (1)
457-500:⚠️ Potential issue | 🟠 MajorUse the canonical external IDs from the database.
Both mutations load the event record first, but the outbound Discord/Google calls still use
input.discordIdandinput.googleId. A stale or tampered payload can update/delete the wrong external event and recreate the sync drift this PR is trying to eliminate. Read those IDs fromevent/eventRecordinstead.🔧 Suggested fix
- Routes.guildScheduledEvent(DISCORD.KNIGHTHACKS_GUILD, input.discordId) + Routes.guildScheduledEvent(DISCORD.KNIGHTHACKS_GUILD, event.discordId) - eventId: input.googleId, + eventId: event.googleId, - Routes.guildScheduledEvent(DISCORD.KNIGHTHACKS_GUILD, input.discordId) + Routes.guildScheduledEvent(DISCORD.KNIGHTHACKS_GUILD, eventRecord.discordId) - eventId: input.googleId, + eventId: eventRecord.googleId,Also applies to: 651-672
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routers/event.ts` around lines 457 - 500, The outbound Discord and Google API calls currently use the untrusted input.discordId and input.googleId; instead, read the canonical external IDs from the loaded event record (event or eventRecord) and use those when calling Routes.guildScheduledEvent(...) and google.calendar.events.update(...). Locate the code paths that call Routes.guildScheduledEvent and google.calendar.events.update and replace references to input.discordId and input.googleId with the corresponding fields from the previously fetched event/eventRecord so the mutation always targets the DB-authoritative external IDs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/blade/src/app/_components/admin/club/events/create-event.tsx`:
- Line 62: The form is rendering before the roles query finishes, allowing
partial options; update create-event.tsx to gate rendering on both queries'
success by checking the query states (e.g., rolesQuery =
api.roles.getAllLinks.useQuery() and hackathonsQuery) and only render the form
when both are successful; while either is loading render a single shared loading
UI, and if either errored render a unified error state (combine/propagate
rolesQuery.error || hackathonsQuery.error) so rolesData?.map(...) never runs on
missing data.
In `@packages/api/src/routers/event.ts`:
- Around line 269-273: The code is still using the dev calendar constant
(EVENTS.DEV_GOOGLE_CALENDAR_ID) for operations events (see
google.calendar.events.insert and related calls) and contains a placeholder
comment; replace that with the dedicated Ops calendar constant (e.g.,
EVENTS.OPERATIONS_GOOGLE_CALENDAR_ID) wherever input.isOperationsCalendar is
used so operations events target the Ops calendar, and update the other
occurrences in this file (the other insert/update/delete blocks referenced
around the same spots) to use the same Ops constant.
- Around line 171-178: The router is trusting client-supplied access-control
fields (InsertEventSchema.roles and isOperationsCalendar) and exposing full
event rows in getEvents; fix by validating/sanitizing client roles server-side
and enforcing authorization in read handlers: in the event creation/update
handlers (schemas derived from InsertEventSchema) replace blind acceptance of
input.roles with a server-side lookup/validation against the authoritative role
store (reject or strip unknown IDs) and only allow setting roles for callers
with appropriate privileges (e.g., admin/operator), and adjust getEvents to
enforce visibility rules (return only public events to unauthenticated callers
and require authentication/role-checks to return role-restricted events) — apply
the same validation to the other affected handlers referenced around the other
diff span (lines ~384-389) so all procedures perform role validation and
authorization checks before persisting or returning event data.
- Around line 218-245: The current isInternalEvent flag incorrectly becomes true
whenever input.roles?.length > 0, causing role-scoped (Blade) events to be
treated as Ops/internal; change the logic so internal behavior is only triggered
by input.isOperationsCalendar (e.g., set isInternalEvent =
Boolean(input.isOperationsCalendar) or require input.isOperationsCalendar &&
roles if you truly need both), then keep channel_id, entity_type and
finalDescription usage tied to that corrected isInternalEvent; apply the same
fix to the other occurrence of this logic (lines around the second block
mentioned).
- Around line 497-499: When handling Google Calendar sync for an event, detect
when input.isOperationsCalendar differs from event.isOperationsCalendar and
treat that as a calendar move: instead of calling events.update in the source
calendar, create the event in the destination calendar (use
EVENTS.DEV_GOOGLE_CALENDAR_ID or EVENTS.GOOGLE_CALENDAR_ID based on
input.isOperationsCalendar), persist the returned googleId to the event record,
then delete the old event from the source calendar using the previous googleId;
only call events.update in-place when the calendar stays the same. Update logic
that currently selects calendarId (using input.isOperationsCalendar,
event.isOperationsCalendar, and googleId) to branch to events.insert + DB
persist + events.delete on a flip, otherwise perform events.update and keep the
existing googleId.
---
Outside diff comments:
In `@packages/api/src/routers/event.ts`:
- Around line 457-500: The outbound Discord and Google API calls currently use
the untrusted input.discordId and input.googleId; instead, read the canonical
external IDs from the loaded event record (event or eventRecord) and use those
when calling Routes.guildScheduledEvent(...) and
google.calendar.events.update(...). Locate the code paths that call
Routes.guildScheduledEvent and google.calendar.events.update and replace
references to input.discordId and input.googleId with the corresponding fields
from the previously fetched event/eventRecord so the mutation always targets the
DB-authoritative external IDs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 671746a0-7504-4dc0-a1af-bb2598d4a6c9
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (6)
MIGRATION_STATUS.mdapps/blade/src/app/_components/admin/club/events/create-event.tsxapps/cron/src/crons/reminder.tspackages/api/src/env.tspackages/api/src/routers/event.tspackages/db/src/schemas/knight-hacks.ts
apps/blade/src/app/_components/admin/club/events/create-event.tsx
Outdated
Show resolved
Hide resolved
must108
reviewed
Mar 9, 2026
Member
must108
left a comment
must108
left a comment
There was a problem hiding this comment.
very well written PR 🙂
please resolve CI and AI review, a video depicting the intended behavior is also helpful (please do this)
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/api/src/routers/event.ts (1)
666-682:⚠️ Potential issue | 🟡 MinorReorder null check before DB query.
The
input.idnull check (line 677) occurs after it's already used in the DB query (line 667). Ifinput.idis undefined, the query runs with an empty string (?? ""), then the check throws.Move the check before the query for clarity and to avoid unnecessary DB calls:
🔧 Suggested fix
+ if (!input.id) { + throw new TRPCError({ + message: "Event ID is required to delete an Event.", + code: "BAD_REQUEST", + }); + } + const eventRecord = await db.query.Event.findFirst({ - where: (t, { eq }) => eq(t.id, input.id ?? ""), + where: (t, { eq }) => eq(t.id, input.id), }); if (!eventRecord) { throw new TRPCError({ message: "Event not found.", code: "BAD_REQUEST", }); } - if (!input.id) { - throw new TRPCError({ - message: "Event ID is required to delete an Event.", - code: "BAD_REQUEST", - }); - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routers/event.ts` around lines 666 - 682, The code performs db.query.Event.findFirst using input.id before validating input.id; move the input.id null/undefined check to before the call so you don't query with a fallback empty string. Specifically, in the handler containing db.query.Event.findFirst and the subsequent TRPCError throws, validate input.id first (throw the TRPCError with message "Event ID is required to delete an Event." and code "BAD_REQUEST" if missing), then call db.query.Event.findFirst({ where: (t, { eq }) => eq(t.id, input.id) }) and afterward handle the eventRecord not found case by throwing the existing "Event not found." TRPCError.
♻️ Duplicate comments (1)
packages/api/src/routers/event.ts (1)
171-178:⚠️ Potential issue | 🟠 MajorValidate
rolesagainst the authoritative roles source.The
rolesfield accepts arbitrary strings without server-side validation. Per the PR objectives, selectable roles should be restricted to those inroles_perms. Without validation, callers can persist dangling role IDs.Consider validating the input roles exist before persisting:
// Example validation approach const validRoles = await db.query.RolesPerms.findMany({ where: inArray(RolesPerms.id, input.roles), }); if (validRoles.length !== input.roles.length) { throw new TRPCError({ message: "One or more role IDs are invalid", code: "BAD_REQUEST", }); }As per coding guidelines: "
packages/api/**: Shared tRPC API layer used by all apps. Pay special attention to: Zod schema validation on all inputs".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routers/event.ts` around lines 171 - 178, The InsertEventSchema currently allows arbitrary strings for the roles array; update the event creation flow to validate input.roles against the authoritative RolesPerms source before persisting: fetch matching RolesPerms entries for the provided IDs (use db.query.RolesPerms.findMany / inArray on RolesPerms.id), compare the returned count to input.roles.length, and if they differ throw a TRPCError with code "BAD_REQUEST" (e.g., "One or more role IDs are invalid"); ensure this check runs where InsertEventSchema is consumed in the event create/update handler so only valid role IDs are saved.
🧹 Nitpick comments (2)
apps/blade/src/app/_components/admin/club/events/create-event.tsx (2)
682-684: Consider improving text contrast for accessibility.The
text-gray-400class may not meet WCAG AA contrast requirements (4.5:1 for normal text) depending on the background. Consider using a slightly darker shade liketext-gray-500ortext-muted-foregroundif available in your design system.♿ Proposed fix
- <span className="text-sm font-normal text-gray-400"> + <span className="text-sm font-normal text-muted-foreground"> Use Operations Calendar (Hide from public events) </span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blade/src/app/_components/admin/club/events/create-event.tsx` around lines 682 - 684, The span rendering the helper text "Use Operations Calendar (Hide from public events)" uses the class text-gray-400 which may fail WCAG contrast; update the class on that span (in the create-event component) to a darker utility such as text-gray-500 or your design-system token like text-muted-foreground to improve contrast while preserving the same sizing and font classes.
619-621: Remove stale development comment.This NOTE comment references replacing
rolesDatawith the tRPC query data, butrolesDatais already populated fromapi.roles.getAllLinks.useQuery()on line 61-64. This appears to be leftover guidance from development that should be cleaned up.🧹 Proposed cleanup
<div className="col-span-3 mt-1 grid grid-cols-2 gap-x-2 gap-y-3"> - {/* NOTE: Replace `rolesData` with your actual tRPC query data. - Example: const { data: rolesData } = api.roles.getAll.useQuery(); - */} {rolesData?.map((role) => (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blade/src/app/_components/admin/club/events/create-event.tsx` around lines 619 - 621, Remove the stale development NOTE comment that suggests replacing `rolesData` with a tRPC query result; `rolesData` is already obtained via api.roles.getAllLinks.useQuery() (see the variable `rolesData` declaration), so delete the commented block around the JSX (the NOTE comment) in the CreateEvent component to avoid misleading guidance.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/api/src/routers/event.ts`:
- Around line 452-458: The two ternary assignments for sourceCalendarId and
targetCalendarId have inconsistent/reduced indentation; align their indentation
to match the surrounding block so they follow the same indent level as
neighboring statements (adjust the lines with sourceCalendarId, targetCalendarId
that reference event.isOperationsCalendar, input.isOperationsCalendar and
EVENTS.DEV_GOOGLE_CALENDAR_ID / EVENTS.GOOGLE_CALENDAR_ID) to restore consistent
formatting.
- Around line 528-547: The current flow in the event update handler that moves
an event (using google.calendar.events.insert and google.calendar.events.delete)
can leave an orphaned created event if delete fails; change the move logic
inside the else-branch to perform a compensating operation: after calling
google.calendar.events.insert (created) immediately try the
google.calendar.events.delete for the source, and wrap delete in a try/catch; if
delete throws, attempt to roll back by calling google.calendar.events.delete on
created.data.id (the new event) and log both errors, only set newGoogleId and
proceed to update the DB when both insert and source delete have succeeded; if
rollback delete fails, include both errors in logs and rethrow so the caller can
surface the failure instead of leaving DB inconsistent (refer to
google.calendar.events.insert, google.calendar.events.delete, created.data.id,
and newGoogleId).
- Around line 474-476: Replace the magic literal 3 used for entity_type with the
existing constant DISCORD.EVENT_TYPE for consistency; in the block where
entity_type is set (currently "entity_type: isInternalEvent ? 2 : 3"), change
the external branch to use DISCORD.EVENT_TYPE (i.e., "isInternalEvent ? 2 :
DISCORD.EVENT_TYPE") so it matches the usage in createEvent and avoids hardcoded
values.
---
Outside diff comments:
In `@packages/api/src/routers/event.ts`:
- Around line 666-682: The code performs db.query.Event.findFirst using input.id
before validating input.id; move the input.id null/undefined check to before the
call so you don't query with a fallback empty string. Specifically, in the
handler containing db.query.Event.findFirst and the subsequent TRPCError throws,
validate input.id first (throw the TRPCError with message "Event ID is required
to delete an Event." and code "BAD_REQUEST" if missing), then call
db.query.Event.findFirst({ where: (t, { eq }) => eq(t.id, input.id) }) and
afterward handle the eventRecord not found case by throwing the existing "Event
not found." TRPCError.
---
Duplicate comments:
In `@packages/api/src/routers/event.ts`:
- Around line 171-178: The InsertEventSchema currently allows arbitrary strings
for the roles array; update the event creation flow to validate input.roles
against the authoritative RolesPerms source before persisting: fetch matching
RolesPerms entries for the provided IDs (use db.query.RolesPerms.findMany /
inArray on RolesPerms.id), compare the returned count to input.roles.length, and
if they differ throw a TRPCError with code "BAD_REQUEST" (e.g., "One or more
role IDs are invalid"); ensure this check runs where InsertEventSchema is
consumed in the event create/update handler so only valid role IDs are saved.
---
Nitpick comments:
In `@apps/blade/src/app/_components/admin/club/events/create-event.tsx`:
- Around line 682-684: The span rendering the helper text "Use Operations
Calendar (Hide from public events)" uses the class text-gray-400 which may fail
WCAG contrast; update the class on that span (in the create-event component) to
a darker utility such as text-gray-500 or your design-system token like
text-muted-foreground to improve contrast while preserving the same sizing and
font classes.
- Around line 619-621: Remove the stale development NOTE comment that suggests
replacing `rolesData` with a tRPC query result; `rolesData` is already obtained
via api.roles.getAllLinks.useQuery() (see the variable `rolesData` declaration),
so delete the commented block around the JSX (the NOTE comment) in the
CreateEvent component to avoid misleading guidance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 6d601a4f-69d6-4a18-ae0f-4737673eab29
📒 Files selected for processing (2)
apps/blade/src/app/_components/admin/club/events/create-event.tsxpackages/api/src/routers/event.ts
There was a problem hiding this comment.
♻️ Duplicate comments (3)
packages/api/src/routers/event.ts (3)
383-388: 🛠️ Refactor suggestion | 🟠 MajorSame role validation concern applies to updateEvent.
The
updateEventschema also acceptsroles: z.array(z.string())without server-side validation. Apply the same validation logic suggested forcreateEvent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routers/event.ts` around lines 383 - 388, The updateEvent route accepts roles via UpdateEventSchema without the server-side validation used in createEvent; mirror the same fix: extend UpdateEventSchema (or the schema used in the updateEvent handler) to validate roles (e.g., ensure each role string matches expected UUID format and/or run the same database existence check/refinement used by createEvent), then have the updateEvent handler apply that refined schema so invalid/missing role IDs are rejected before persisting; reference the updateEvent route and UpdateEventSchema/roles field when making this change.
269-272:⚠️ Potential issue | 🟠 MajorPlaceholder calendar ID needs replacement before production.
The inline comment
// NEED GOOGLE OPS IDconfirms this is still a placeholder. TheDEV_GOOGLE_CALENDAR_IDis explicitly defined as a development calendar constant. Using it for operations events in production will route internal events to the wrong calendar.💡 Recommended approach
- Add
OPS_GOOGLE_CALENDAR_IDtopackages/consts/src/events.ts- Replace the placeholder:
calendarId: input.isOperationsCalendar - ? EVENTS.DEV_GOOGLE_CALENDAR_ID + ? EVENTS.OPS_GOOGLE_CALENDAR_ID : EVENTS.GOOGLE_CALENDAR_ID,This affects lines 270-271, 359-360, 453-454, 457-458, and 725-727.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routers/event.ts` around lines 269 - 272, Replace the placeholder development calendar ID with a proper operations constant: add OPS_GOOGLE_CALENDAR_ID to the events constants (e.g., in packages/consts/src/events.ts) and update all places that branch on input.isOperationsCalendar to use EVENTS.OPS_GOOGLE_CALENDAR_ID instead of EVENTS.DEV_GOOGLE_CALENDAR_ID (e.g., the calendarId assignment in the event router and the other mentioned occurrences); ensure the new constant is exported from the events module and used wherever input.isOperationsCalendar is checked.
171-178: 🛠️ Refactor suggestion | 🟠 MajorServer-side role validation is still missing.
The
rolescolumn in the Event table is defined ast.varchar({ length: 255 }).array().notNull().default([]). This is a Postgres text array column. The API accepts any string array from the client without verifying that the role IDs actually exist in theRolestable. An admin could accidentally or intentionally persist invalid/stale role IDs, leading to confusing visibility behavior.Consider validating
input.rolesagainst the authoritative roles source before persisting:🛡️ Suggested validation approach
.mutation(async ({ input, ctx }) => { permissions.controlPerms.or(["EDIT_CLUB_EVENT", "EDIT_HACK_EVENT"], ctx); + // Validate that all provided role IDs exist + if (input.roles.length > 0) { + const validRoles = await db.query.Roles.findMany({ + where: (t, { inArray }) => inArray(t.id, input.roles), + }); + const validRoleIds = new Set(validRoles.map((r) => r.id)); + const invalidRoles = input.roles.filter((id) => !validRoleIds.has(id)); + if (invalidRoles.length > 0) { + throw new TRPCError({ + message: `Invalid role IDs: ${invalidRoles.join(", ")}`, + code: "BAD_REQUEST", + }); + } + } // Step 0: Convert provided start/end datetimes...As per coding guidelines "
packages/api/**: Zod schema validation on all inputs" — while the shape is validated, the referential integrity of role IDs is not.
🧹 Nitpick comments (1)
apps/blade/src/app/_components/admin/club/events/create-event.tsx (1)
610-666: Roles picker UI works correctly, minor note on the leftover comment.The nested
FormFieldpattern for multi-checkbox selection is appropriate. ThegetAllLinksquery returns complete role objects withid,name,discordRoleId, andpermissionsfields, so usingrole.idas the value androle.nameas the label is correct.One small cleanup — the comment on lines 621-623 is leftover guidance that can be removed:
🧹 Remove leftover comment
<div className="col-span-3 mt-1 grid grid-cols-2 gap-x-2 gap-y-3"> - {/* NOTE: Replace `rolesData` with your actual tRPC query data. - Example: const { data: rolesData } = api.roles.getAll.useQuery(); - */} {rolesData?.map((role) => (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blade/src/app/_components/admin/club/events/create-event.tsx` around lines 610 - 666, Remove the leftover guidance comment near the roles checkbox UI: delete the multi-line comment that references replacing `rolesData` with a tRPC query (the block above the {rolesData?.map(...)} loop). This cleanup affects the nested FormField rendering for the "roles" field (see `rolesData`, the outer `FormField` with name "roles", and the inner mapped `FormField` rendering each role) — simply remove the comment lines to keep the JSX clean.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/api/src/routers/event.ts`:
- Around line 383-388: The updateEvent route accepts roles via UpdateEventSchema
without the server-side validation used in createEvent; mirror the same fix:
extend UpdateEventSchema (or the schema used in the updateEvent handler) to
validate roles (e.g., ensure each role string matches expected UUID format
and/or run the same database existence check/refinement used by createEvent),
then have the updateEvent handler apply that refined schema so invalid/missing
role IDs are rejected before persisting; reference the updateEvent route and
UpdateEventSchema/roles field when making this change.
- Around line 269-272: Replace the placeholder development calendar ID with a
proper operations constant: add OPS_GOOGLE_CALENDAR_ID to the events constants
(e.g., in packages/consts/src/events.ts) and update all places that branch on
input.isOperationsCalendar to use EVENTS.OPS_GOOGLE_CALENDAR_ID instead of
EVENTS.DEV_GOOGLE_CALENDAR_ID (e.g., the calendarId assignment in the event
router and the other mentioned occurrences); ensure the new constant is exported
from the events module and used wherever input.isOperationsCalendar is checked.
---
Nitpick comments:
In `@apps/blade/src/app/_components/admin/club/events/create-event.tsx`:
- Around line 610-666: Remove the leftover guidance comment near the roles
checkbox UI: delete the multi-line comment that references replacing `rolesData`
with a tRPC query (the block above the {rolesData?.map(...)} loop). This cleanup
affects the nested FormField rendering for the "roles" field (see `rolesData`,
the outer `FormField` with name "roles", and the inner mapped `FormField`
rendering each role) — simply remove the comment lines to keep the JSX clean.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 7c396fc7-47c4-43f0-88ae-9eb195e2f69c
📒 Files selected for processing (2)
apps/blade/src/app/_components/admin/club/events/create-event.tsxpackages/api/src/routers/event.ts
must108
requested changes
Mar 9, 2026
Member
must108
left a comment
must108
left a comment
There was a problem hiding this comment.
code lgtm. mostly nits on comments
apps/blade/src/app/_components/admin/club/events/create-event.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/api/src/routers/event.ts (1)
435-536:⚠️ Potential issue | 🟠 MajorThe update path can still drift across Discord, Google Calendar, and the DB.
discord.api.patch(...)runs first, Google mutation runs second, anddb.update(...)runs last with no compensating rollback. If Google fails after the Discord patch, or if the final DB write fails, the three systems disagree about the same event. Since this PR is specifically fixing sync bugs, I’d strongly avoid another partial-success path here—either defer the Discord patch until the calendar work succeeds, or add compensation/reconciliation around the later steps similar to the create flow.As per coding guidelines "
packages/api/**: Shared tRPC API layer used by all apps. Pay special attention to: Error handling with user-facing messages,Database transaction handling, andIntegration points (Stripe, Discord, MinIO, PassKit)`."Also applies to: 619-629
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routers/event.ts` around lines 435 - 536, The current update flow patches Discord first via discord.api.patch(...) (using event.discordId) then updates Google Calendar (google.calendar.events.update/insert/delete) and finally writes the DB (db.update(...)), which can leave systems inconsistent if a later step fails; change the flow so external side-effects are atomic: either defer the Discord update until after Google calendar calls succeed and the DB transaction is ready to commit, or implement compensating rollbacks — capture the original Discord event state before calling discord.api.patch and if Google or DB fails, call discord.api.patch/delete to restore the original state (use event.discordId and the patched fields like description/name/channel_id), and ensure the DB write is performed inside a transaction that only commits after external services succeed (use newGoogleId/created.data.id to keep Google IDs in sync); make sure all error paths log the original error and perform the compensating Discord rollback before rethrowing the TRPCError.
♻️ Duplicate comments (1)
packages/api/src/routers/event.ts (1)
171-178:⚠️ Potential issue | 🔴 CriticalEnforce event visibility on the server, not just in the form.
These new fields are still trusted as raw client input:
rolesaccepts arbitrary strings here, whilegetEventsis still public and returns everyEventrow. That means callers can persist dangling role IDs and unauthenticated consumers can still fetch internal/restricted events unless the router validates role IDs against the authoritative roles source and filters read paths byroles/isOperationsCalendar. Please apply the same validation toupdateEventtoo.As per coding guidelines "
packages/api/**: Shared tRPC API layer used by all apps. Pay special attention to: Authorization checks in every procedure`."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routers/event.ts` around lines 171 - 178, InsertEventSchema currently accepts arbitrary client-supplied roles and isOperationsCalendar values which are stored and returned without server-side validation; update the server-side logic so InsertEventSchema is not the sole trust boundary: validate and canonicalize the roles field against the authoritative roles source (e.g., lookup allowed role IDs from your roles store) before persisting in the procedure that handles creation, and strip/replace any unknown IDs; enforce that only authorized users can set isOperationsCalendar (and override client input if unauthorized); apply the same server-side validation/authorization in updateEvent and ensure getEvents filters returned Event rows by the caller’s effective roles and by isOperationsCalendar visibility so unauthenticated/unauthorized consumers cannot read restricted events.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/blade/src/app/_components/admin/club/events/create-event.tsx`:
- Around line 621-683: The role checkboxes and the "Use Operations Calendar"
checkbox are not associated with their labels; update the rendering in the roles
map and the isOperationsCalendar FormField to give each Checkbox a stable id and
set the corresponding FormLabel's htmlFor to that id (e.g., for roles use an id
like `role-${role.id}` and for the operations checkbox use
`isOperationsCalendar`), ensuring you keep existing props
(checked/onCheckedChange) intact so screen readers and click targets are
properly linked (locate the map rendering of FormField name="roles" and the
FormField name="isOperationsCalendar" to apply this change).
In `@packages/api/src/routers/event.ts`:
- Around line 365-368: InsertEventSchema currently supplies defaults for roles
and isOperationsCalendar which causes omitted fields to overwrite stored values;
update the API to avoid silent clearing by either making these fields required
in InsertEventSchema and updating the callers
(admin/club/events/update-event.tsx and admin/hackathon/events/update-event.tsx)
to always submit them, or (preferred) change the update-handler that consumes
InsertEventSchema to treat missing fields as no-op by merging with existing
event values (e.g., use input.roles ?? event.roles and
input.isOperationsCalendar ?? event.isOperationsCalendar before spreading into
the DB update), referencing InsertEventSchema, roles, isOperationsCalendar, and
the update handling function to locate the fix.
---
Outside diff comments:
In `@packages/api/src/routers/event.ts`:
- Around line 435-536: The current update flow patches Discord first via
discord.api.patch(...) (using event.discordId) then updates Google Calendar
(google.calendar.events.update/insert/delete) and finally writes the DB
(db.update(...)), which can leave systems inconsistent if a later step fails;
change the flow so external side-effects are atomic: either defer the Discord
update until after Google calendar calls succeed and the DB transaction is ready
to commit, or implement compensating rollbacks — capture the original Discord
event state before calling discord.api.patch and if Google or DB fails, call
discord.api.patch/delete to restore the original state (use event.discordId and
the patched fields like description/name/channel_id), and ensure the DB write is
performed inside a transaction that only commits after external services succeed
(use newGoogleId/created.data.id to keep Google IDs in sync); make sure all
error paths log the original error and perform the compensating Discord rollback
before rethrowing the TRPCError.
---
Duplicate comments:
In `@packages/api/src/routers/event.ts`:
- Around line 171-178: InsertEventSchema currently accepts arbitrary
client-supplied roles and isOperationsCalendar values which are stored and
returned without server-side validation; update the server-side logic so
InsertEventSchema is not the sole trust boundary: validate and canonicalize the
roles field against the authoritative roles source (e.g., lookup allowed role
IDs from your roles store) before persisting in the procedure that handles
creation, and strip/replace any unknown IDs; enforce that only authorized users
can set isOperationsCalendar (and override client input if unauthorized); apply
the same server-side validation/authorization in updateEvent and ensure
getEvents filters returned Event rows by the caller’s effective roles and by
isOperationsCalendar visibility so unauthenticated/unauthorized consumers cannot
read restricted events.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 2216f332-174d-4094-a0ca-5d777dabcabf
📒 Files selected for processing (2)
apps/blade/src/app/_components/admin/club/events/create-event.tsxpackages/api/src/routers/event.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/api/src/routers/event.ts (2)
171-178:⚠️ Potential issue | 🔴 CriticalValidate
rolesserver-side before persisting visibility rules.This still trusts arbitrary
roles: string[]from the client, so callers can save dangling or unauthorized role scopes. Becauserolesis now access-control state, resolve it against the authoritative role source before create/update and reject or strip unknown IDs; the read path should enforce the same visibility rules.As per coding guidelines "
packages/api/**: Shared tRPC API layer used by all apps. Pay special attention to: Authorization checks in every procedure" and "packages/api/**: Shared tRPC API layer used by all apps. Pay special attention to: Zod schema validation on all inputs".Also applies to: 365-368
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routers/event.ts` around lines 171 - 178, The InsertEventSchema extension currently accepts arbitrary roles from clients; you must validate and canonicalize those role IDs against the authoritative role store before persisting visibility rules. In the event creation/update handlers (the procedures that consume InsertEventSchema and the corresponding update path in routers/event.ts), resolve each roles entry against the central role service/DB, strip or reject any unknown or unauthorized role IDs, and return a validation error if the caller tries to set roles they don't own; apply the same check on reads that enforce visibility rules so clients cannot bypass access control. Ensure you reference and update the code paths that consume InsertEventSchema and the create/update/read procedures so the server enforces canonical role IDs rather than trusting the client-supplied roles array.
257-259:⚠️ Potential issue | 🟠 MajorUse the dedicated Ops calendar here, not
DEV_GOOGLE_CALENDAR_ID.These branches still send ops-only events to the dev calendar. In production, create/update/delete for
isOperationsCalendarevents will target the wrong calendar unless this is switched to the real Ops calendar constant/env.Also applies to: 340-342, 427-433, 681-683
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/routers/event.ts` around lines 257 - 259, The code branches that decide calendarId using input.isOperationsCalendar currently use EVENTS.DEV_GOOGLE_CALENDAR_ID for ops events; update those branches to use the real Ops calendar constant/environment variable (e.g., EVENTS.OPERATIONS_GOOGLE_CALENDAR_ID or the project's defined ops calendar env) instead of EVENTS.DEV_GOOGLE_CALENDAR_ID. Locate occurrences that use the ternary with input.isOperationsCalendar (the snippet shown and the other spots flagged around the create/update/delete handlers) and replace EVENTS.DEV_GOOGLE_CALENDAR_ID with the correct Ops calendar constant so ops-only create/update/delete operations target the proper calendar.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/api/src/routers/event.ts`:
- Around line 365-368: updateEvent currently reuses InsertEventSchema (via
InsertEventSchema.extend(...)) which allows clients to send server-managed
external IDs like discordId; change the input schema for updateEvent to a
dedicated UpdateEventSchema that omits or strips server-managed fields (e.g.,
.omit({ discordId: true, externalId: true })) and/or explicitly picks only
updatable fields, then use that UpdateEventSchema in the updateEvent handler and
ensure the .set({ ...input, ... }) call only spreads the validated updatable
fields (and merges server-controlled ids separately on the server side); update
the related occurrences (the other spots referenced around the 621-625 region)
to use the new schema too.
---
Duplicate comments:
In `@packages/api/src/routers/event.ts`:
- Around line 171-178: The InsertEventSchema extension currently accepts
arbitrary roles from clients; you must validate and canonicalize those role IDs
against the authoritative role store before persisting visibility rules. In the
event creation/update handlers (the procedures that consume InsertEventSchema
and the corresponding update path in routers/event.ts), resolve each roles entry
against the central role service/DB, strip or reject any unknown or unauthorized
role IDs, and return a validation error if the caller tries to set roles they
don't own; apply the same check on reads that enforce visibility rules so
clients cannot bypass access control. Ensure you reference and update the code
paths that consume InsertEventSchema and the create/update/read procedures so
the server enforces canonical role IDs rather than trusting the client-supplied
roles array.
- Around line 257-259: The code branches that decide calendarId using
input.isOperationsCalendar currently use EVENTS.DEV_GOOGLE_CALENDAR_ID for ops
events; update those branches to use the real Ops calendar constant/environment
variable (e.g., EVENTS.OPERATIONS_GOOGLE_CALENDAR_ID or the project's defined
ops calendar env) instead of EVENTS.DEV_GOOGLE_CALENDAR_ID. Locate occurrences
that use the ternary with input.isOperationsCalendar (the snippet shown and the
other spots flagged around the create/update/delete handlers) and replace
EVENTS.DEV_GOOGLE_CALENDAR_ID with the correct Ops calendar constant so ops-only
create/update/delete operations target the proper calendar.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 4b213fdf-05f7-4bff-a24e-cbb8e883ad50
📒 Files selected for processing (1)
packages/api/src/routers/event.ts
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.
Screencast.from.2026-03-10.23-36-25.webm
Screencast.from.2026-03-10.17-35-50.webm
Screencast.from.2026-03-09.11-41-34.webm
Why
To allow the team to easily schedule and manage internal Operations events and role-specific events directly through the Blade dashboard without exposing them to the public. Additionally, this PR addresses a synchronization bug where updating or deleting an event on Blade was leaving "ghost" events behind on Discord and Google Calendar.
What
entity_typepayload, ensuring changes now perfectly sync with Discord and Google Calendar.isOperationsCalendar(boolean) androles(string array) columns to the database schema for the Event table.Closes: #353
Test Plan
I thoroughly tested the full event lifecycle (Create, Update, Delete) locally to ensure all three systems (Database, Discord API, Google Calendar) stay perfectly in sync:
entity_type: 3).entity_type: 2), successfully linking to the Ops Voice Channel ID.PATCHandDELETErequests (with the correctchannel_idandentity_metadatanullifications) so no ghost events are left behind.Checklist
Important Note for Reviewer:
DISCORD_OPS_VOICE_CHANNEL_IDto the.envlinked to the ops voice channel in the kh server. This is crucial for the Ops-only events to show specifically to them on Discord.isOperationsCalendar(boolean) androles(string array) columns to the database schema for the Event table. So running pnpm db:push is necessary.Summary by CodeRabbit
New Features
Bug Fixes
Database
Chores
API
Documentation