Skip to content

Conversation

@zacjones93
Copy link
Contributor

@zacjones93 zacjones93 commented Jan 19, 2026

Create comprehensive PRD with 16 user stories for building a general
volunteer scheduler for non-judge roles. The system will allow:

  • Time-based shift scheduling (vs heat-based for judges)
  • Admin UI for creating/managing shifts
  • Volunteer assignment to shifts
  • My Schedule page display for volunteers

Out of scope: equipment team (special scheduling needs)


Summary by cubic

Implements time-based volunteer shift scheduling for non-judge roles. Adds DB, server APIs, organizer UI for shift management and assignments, and shows shifts on My Schedule.

  • New Features

    • Database: volunteer_shifts and volunteer_shift_assignments tables with indexes and relations.
    • Server: shift CRUD; assign/unassign (with capacity checks); competition/volunteer queries.
    • UI: Shifts tab on Organizer > Volunteers with list, create/edit dialog, and assignment panel; “My Shifts” visible on My Schedule.
    • Tests & Docs: unit tests for shift server functions; PRD with 16 stories and Ralph scripts; equipment team remains out of scope.
  • Migration

    • Run DB migrations 0074 and 0075.

Written for commit 79f3cad. Summary will update on new commits.

Summary by CodeRabbit

  • New Features
    • Adds volunteer shift scheduling: viewable "My Shifts", shift cards, organizer shift management (create/edit/delete), and assignment panels for adding/removing volunteers.
  • UI
    • Tabs for Volunteers vs Shifts, schedule view updated to include shifts alongside events.
  • Tests
    • Comprehensive server-side tests for shift CRUD and assignment flows.
  • Chores / Documentation
    • Product spec, progress logs, agent workflow docs, and automation scripts for iterative development.

✏️ Tip: You can customize this high-level summary in your review settings.

Create comprehensive PRD with 16 user stories for building a general
volunteer scheduler for non-judge roles. The system will allow:
- Time-based shift scheduling (vs heat-based for judges)
- Admin UI for creating/managing shifts
- Volunteer assignment to shifts
- My Schedule page display for volunteers

Out of scope: equipment team (special scheduling needs)
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 19, 2026

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds a volunteer shift scheduling feature end-to-end (DB migrations, schema types, server functions, tests, UI components and organizer tools) plus a Ralph agent toolkit (controller/worker docs and scripts) and project PRD/progress/prompt files for iterative implementation.

Changes

Cohort / File(s) Summary
Ralph tooling & docs
scripts/ralph/prompt.md, scripts/ralph/prd.json, scripts/ralph/progress.txt, scripts/ralph/ralph-controller.md, scripts/ralph/ralph-worker.md, scripts/ralph/ralph.sh, scripts/ralph/ralph-once.sh
New agent framework files: PRD, progress log, agent prompts, controller/worker docs, and AFK/HITL iteration scripts. Defines workflow, file formats, and run/exit semantics.
Database migrations & journal
apps/wodsmith-start/src/db/migrations/0074_add-volunteer-shifts.sql, apps/wodsmith-start/src/db/migrations/0075_add-volunteer-shift-assignments.sql, apps/wodsmith-start/src/db/migrations/meta/_journal.json
Add volunteer_shifts and volunteer_shift_assignments tables, indexes, FKs, and migration journal entries.
DB schemas / ID helpers
apps/wodsmith-start/src/db/schemas/common.ts, apps/wodsmith-start/src/db/schemas/volunteers.ts
New ID generators and full volunteer shift/assignment table schemas, types, relations, and reverse-relations integrated into existing volunteer/judge graphs.
Server functions (scheduling)
apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts, apps/wodsmith-start/src/server-fns/volunteer-schedule-fns.ts
Admin-only CRUD and assignment APIs for shifts (create/read/update/delete, assign/unassign, bulk assign, listing). Schedule data fn extended to return events + shifts. Authorization and validation added.
Tests
apps/wodsmith-start/test/server-fns/volunteer-shifts.test.ts
Comprehensive unit tests covering shift CRUD, assignment flows, bulk assign, validations, and error cases.
My-schedule & Schedule view
apps/wodsmith-start/src/routes/compete/$slug/my-schedule.tsx, apps/wodsmith-start/src/routes/compete/$slug/-components/schedule-view.tsx
Loader now returns shifts; ScheduleView updated to accept and render volunteer shifts alongside events.
Shift UI components
apps/wodsmith-start/src/routes/compete/$slug/-components/shift-card.tsx
New ShiftCard component for displaying shift info (role badge, time range, location, notes).
Organizer UI: shifts
apps/wodsmith-start/src/routes/compete/organizer/$competitionId/-components/shifts/shift-list.tsx, .../shift-form-dialog.tsx, .../shift-assignment-panel.tsx, apps/wodsmith-start/src/routes/compete/organizer/$competitionId/volunteers.tsx
Admin-facing shift management: list, create/edit dialog, assignment panel (assign/unassign, capacity checks), and volunteers route wired to include Shifts tab and validated search params.

Sequence Diagram(s)

sequenceDiagram
  participant Organizer UI as Organizer (Browser)
  participant Server as API (server-fns)
  participant DB as Database
  participant Notifications as Toast/UI

  Organizer ->> Server: assignVolunteerToShift(shiftId, membershipId)
  Server ->> DB: validate shift exists, capacity, membership lookup
  DB -->> Server: shift/membership data
  Server ->> DB: insert volunteer_shift_assignment
  DB -->> Server: inserted assignment
  Server ->> Organizer: return assignment data
  Organizer ->> Notifications: show success toast
Loading

(Colored rectangles not required for this simple flow.)

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

Suggested reviewers

  • theianjones

Poem

🐰 I hopped in with a PRD in paw,

Schedules, shifts — I fixed the draw.
Shifts and panels, tests that sing,
Agents humming, scripts that spring.
A carrot for each volunteer — huzzah! 🍃

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: add volunteer scheduler PRD for Ralph' clearly and directly describes the main change: adding a Product Requirements Document for a volunteer scheduler system for the Ralph tool.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 7 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="scripts/ralph/ralph-once.sh">

<violation number="1" location="scripts/ralph/ralph-once.sh:47">
P2: The `|| true` plus pipeline status behavior will hide failures from the `claude` command, so the script can report a completed iteration even when the tool never ran. Add an explicit status check for the `claude` command (or use `PIPESTATUS`) so failures are surfaced.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.


# Run Claude Code interactively (no --dangerously-skip-permissions)
# This allows you to approve/deny actions as needed
OUTPUT=$(cat "${SCRIPT_DIR}/prompt.md" | claude 2>&1 | tee /dev/stderr) || true
Copy link

@cubic-dev-ai cubic-dev-ai bot Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The || true plus pipeline status behavior will hide failures from the claude command, so the script can report a completed iteration even when the tool never ran. Add an explicit status check for the claude command (or use PIPESTATUS) so failures are surfaced.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/ralph/ralph-once.sh, line 47:

<comment>The `|| true` plus pipeline status behavior will hide failures from the `claude` command, so the script can report a completed iteration even when the tool never ran. Add an explicit status check for the `claude` command (or use `PIPESTATUS`) so failures are surfaced.</comment>

<file context>
@@ -0,0 +1,60 @@
+
+# Run Claude Code interactively (no --dangerously-skip-permissions)
+# This allows you to approve/deny actions as needed
+OUTPUT=$(cat "${SCRIPT_DIR}/prompt.md" | claude 2>&1 | tee /dev/stderr) || true
+
+# Check for completion signal
</file context>
Fix with Cubic

@github-actions
Copy link

github-actions bot commented Jan 19, 2026

🚀 Preview Deployed

URL: https://wodsmith-app-pr-191.zacjones93.workers.dev

Detail Value
Commit 3ec8a17
Stage pr-191
Deployed 2026-01-19T02:31:29.079Z

This comment is automatically updated on each push to this PR.

claude added 16 commits January 19, 2026 01:15
Add database schema for time-based volunteer shifts:
- Add volunteer_shifts table with columns: id, competitionId, name,
  roleType, startTime, endTime, location, capacity, notes
- Add createVolunteerShiftId() function to common.ts
- Add VolunteerShift type export
- Define relations: shift belongs to competition
- Add indexes on competitionId and startTime
- Foreign key to competitions with cascade delete
Add junction table for assigning volunteers to shifts:
- volunteerShiftAssignmentsTable with shiftId, membershipId, notes columns
- Foreign keys to volunteer_shifts and team_membership with cascade delete
- Unique constraint on (shiftId, membershipId) to prevent duplicate assignments
- Indexes on shiftId and membershipId for efficient lookups
- Relations defined: assignment belongs to shift and membership
- Reverse relations added to volunteerShiftsTable and teamMembershipTable
Mark both VS-001 and VS-002 as passing in prd.json.
Add progress log entries with learnings for both stories.
- Applied migrations 0074 and 0075 to local D1 database
- Both volunteer_shifts and volunteer_shift_assignments tables created
- Created local wrangler config for db:migrate:local support
- All feedback loops pass (type-check, lint)
Add server functions for managing volunteer shifts:
- createShiftFn - creates a new shift with validation
- getCompetitionShiftsFn - gets all shifts ordered by startTime
- updateShiftFn - updates a shift by id
- deleteShiftFn - deletes a shift (cascade deletes assignments)

All functions validate competitionId and check organizer permission
using requireTeamPermission with MANAGE_COMPETITIONS.
- Mark VS-004 as passes=true in prd.json
- Add session notes and learnings to progress.txt
Add server functions for volunteer shift assignments:
- assignVolunteerToShiftFn: assigns volunteer to shift with capacity validation
- unassignVolunteerFromShiftFn: removes volunteer from shift
- getShiftAssignmentsFn: gets assignments with volunteer details (name, email, roleTypes)
- getVolunteerShiftsFn: gets all shifts for a specific volunteer
- bulkAssignVolunteersToShiftFn: bulk assign with capacity validation and D1 batching

All functions validate organizer permission and capacity constraints.
- Updated getVolunteerScheduleDataFn to fetch volunteer shift assignments
- Added VolunteerShiftData interface with id, name, roleType, startTime, endTime, location, notes
- Added VolunteerScheduleData interface combining events and shifts arrays
- Shifts are queried from volunteerShiftAssignmentsTable joined with volunteerShiftsTable
- Shifts are ordered by startTime
- Backward compatible: events array structure unchanged
Add ShiftCard component to display volunteer shift assignments on the
my-schedule page. Features include:
- Shift name with role type badge (colored by role)
- User-friendly time display (e.g., 'Sat 8:00 AM - 12:00 PM')
- Location display with MapPin icon when set
- Notes section with distinct styling when set
- Border-left accent style matching rotation-card.tsx
- Upcoming/past shift visual distinction
- Add shifts prop to ScheduleView component
- Add "My Shifts" section with ShiftCard components after profile card
- Show empty state when no shifts: "No shifts assigned yet"
- Update page title logic: show "My Volunteer Schedule" when shifts exist
- Update my-schedule.tsx loader to fetch and pass shifts data
- Include shifts: [] in early return cases for consistency
Add ShiftList component for organizer interface to view and manage
volunteer shifts. Features include:
- Shifts grouped by date with responsive table layout
- Each shift shows name, role type, time, location, capacity
- Delete functionality with confirmation dialog
- Empty state and add/edit button placeholders for VS-011

Files:
- apps/wodsmith-start/src/.../shifts/shift-list.tsx (new)
- scripts/ralph/prd.json (VS-010 passes=true)
- scripts/ralph/progress.txt (session learnings)
Add a dialog component for creating and editing volunteer shifts with
React Hook Form + Zod validation. The dialog supports both create and
edit modes, with proper form pre-filling for edits.

Features:
- Form fields: name, roleType, date, startTime, endTime, location, capacity, notes
- Excludes equipment role from roleType options
- Validates name required, startTime before endTime, capacity >= 1
- Combines date + time picker inputs into timestamps
- Shows loading state during submission
- Closes and refreshes list on success

Updated shift-list.tsx to use the new dialog instead of placeholder toasts.
Add shift assignment panel component for managing volunteer assignments:

- New ShiftAssignmentPanel component (Sheet-based side panel)
  - Shows shift details (name, role, time, location, capacity)
  - Lists assigned volunteers with remove button
  - Lists available volunteers filtered by roleType
  - Add/remove volunteer assignment functionality
  - Capacity indicator and disabled state when full

- Update ShiftList to integrate assignment panel
  - Add competitionTeamId prop for volunteer fetching
  - Make rows clickable to open assignment panel
  - Handle optimistic updates on assign/unassign
  - Stop propagation on edit/delete action buttons

Both pnpm type-check and pnpm lint pass.
- Add Tabs UI with Volunteers and Shifts tabs
- Integrate ShiftList component in Shifts tab
- Persist tab state in URL search params (?tab=shifts)
- Load shifts data in route loader
Add comprehensive test coverage for volunteer shift server functions:
- createShiftFn: validates input, time constraints, creates shifts correctly
- assignVolunteerToShiftFn: validates capacity, prevents duplicates, creates assignments
- unassignVolunteerFromShiftFn: removes assignments correctly with error handling
- getVolunteerShiftsFn: returns correct shifts filtered by competition
- updateShiftFn: updates shifts with time validation
- deleteShiftFn: deletes shifts with proper authorization
- getCompetitionShiftsFn: returns all shifts for a competition
- bulkAssignVolunteersToShiftFn: handles bulk assignment with capacity/membership validation

All tests mock the database using FakeDrizzleDb and auth functions appropriately.
- Update prd.json to mark VS-016 verification as passed
- Add final verification summary to progress.txt
- All 16 user stories now pass
- All feedback loops verified (type-check, lint, test)
Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 19 files (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="apps/wodsmith-start/src/routes/compete/organizer/$competitionId/-components/shifts/shift-form-dialog.tsx">

<violation number="1" location="apps/wodsmith-start/src/routes/compete/organizer/$competitionId/-components/shifts/shift-form-dialog.tsx:54">
P2: This scheduler is scoped to non-judge roles, but the role list still allows Judge and Head Judge. Remove judge roles from the enum and dropdown so users can’t create judge shifts in this feature.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.


// Shift role type enum values (excluding equipment per VS-011 requirements)
const shiftRoleTypeEnum = z.enum([
"judge",
Copy link

@cubic-dev-ai cubic-dev-ai bot Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This scheduler is scoped to non-judge roles, but the role list still allows Judge and Head Judge. Remove judge roles from the enum and dropdown so users can’t create judge shifts in this feature.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/routes/compete/organizer/$competitionId/-components/shifts/shift-form-dialog.tsx, line 54:

<comment>This scheduler is scoped to non-judge roles, but the role list still allows Judge and Head Judge. Remove judge roles from the enum and dropdown so users can’t create judge shifts in this feature.</comment>

<file context>
@@ -0,0 +1,507 @@
+
+// Shift role type enum values (excluding equipment per VS-011 requirements)
+const shiftRoleTypeEnum = z.enum([
+	"judge",
+	"head_judge",
+	"medical",
</file context>
Fix with Cubic

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🤖 Fix all issues with AI agents
In
`@apps/wodsmith-start/src/routes/compete/organizer/`$competitionId/-components/shifts/shift-form-dialog.tsx:
- Around line 217-229: When updating an existing shift in the isEditing branch,
emptying location/notes currently sends undefined (treated as no-op) so prior
values persist; change the payload construction in updateShift to send null for
cleared fields by mapping values.location and values.notes to (values.location
=== "" ? null : values.location) and (values.notes === "" ? null : values.notes)
respectively (refer to updateShift call and values.location/values.notes) and
ensure the server update handler accepts null to remove the stored values.
- Around line 52-80: The shift role enum and dropdown still include
judge/head_judge which must be excluded for time-based scheduling; update the
z.enum definition shiftRoleTypeEnum to remove "judge" and "head_judge" and
remove their entries from the SHIFT_ROLE_TYPES array (types derived via
ShiftRoleType will update automatically), and verify any place that expects
those values (e.g., form validation or value mapping) is adjusted or gated
accordingly so time-based shift flows never expose judge/head_judge options.

In
`@apps/wodsmith-start/src/routes/compete/organizer/`$competitionId/-components/shifts/shift-list.tsx:
- Around line 108-119: The ShiftList component initializes local state with
useState(initialShifts) but never syncs when props change—add a useEffect in
ShiftList that watches initialShifts and calls setShifts(initialShifts) so the
list updates after router.invalidate(); also refactor the helper
getCapacityBadgeVariant to accept a single object parameter (e.g., { capacity,
filled }) instead of positional args and update all call sites in this file to
pass a named object to match the project guideline.

In `@apps/wodsmith-start/src/server-fns/volunteer-schedule-fns.ts`:
- Around line 270-293: The query fetching shiftAssignments (using db.select on
volunteerShiftAssignmentsTable joined to volunteerShiftsTable) is only filtered
by membershipId and competitionId and must be scoped by team context; update the
query to join the table that links competitions to teams (e.g., competitionTeams
or competition_team table) or the membership->team table and add a predicate
that ensures the shift's competitionTeam/teamId matches the caller's team
context (use the same team identifier available in data or derive it via
membership) so multi-tenant filtering is enforced alongside the existing
eq(volunteerShiftAssignmentsTable.membershipId, data.membershipId) and
eq(volunteerShiftsTable.competitionId, data.competitionId) conditions.

In `@apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts`:
- Around line 1-10: Add a top-level bare import "server-only" to this module
(before other imports) to mark it as server-only and prevent client bundling;
update the file containing createServerFn and getDb (the volunteer shift
server-fns module) by inserting import "server-only" at the top of the file as a
standalone import statement.
- Around line 208-216: The update block currently converts explicit nulls to
undefined using the nullish coalescing operator, preventing clearing nullable DB
columns; inside the conditional guards (e.g., the checks `if (data.location !==
undefined)` and `if (data.notes !== undefined)`), assign the raw input values to
updateValues (e.g., `updateValues.location = data.location` and
`updateValues.notes = data.notes`) instead of using `?? undefined` so that
explicit nulls are preserved and Drizzle can write NULL to the database.
- Around line 32-35: The shift ID schema (shiftIdSchema) currently validates IDs
starting with "vshf_" but createVolunteerShiftId() emits "vshft_", causing real
IDs to fail validation in updateShiftFn, deleteShiftFn, assignShiftFn,
unassignShiftFn, getShiftAssignmentsFn, and reassignShiftFn; update
shiftIdSchema to accept both "vshf_" and "vshft_" prefixes (e.g., use a regex or
z.union/z.preprocess to allow either prefix) so existing code paths validate
correctly, and update any test fixtures that use the old "vshf_" prefix to
"vshft_" to match createVolunteerShiftId().
- Around line 283-301: The membership existence checks for volunteer shift
assignments are only filtering by membership id and not by the competition's
organizing team, which permits cross-tenant assignments; in functions handling
shifts (e.g., getShiftWithAuthCheck and the code that loads shift.assignments)
add a filter to membership lookups to include the competition.organizingTeamId
(obtain via getCompetitionWithAuthCheck(shift.competitionId)) so queries check
both id and teamId, and update any bulk membership lookup call’s autochunk
parameter to match the new chunking/count expectations used elsewhere.

In `@scripts/ralph/progress.txt`:
- Around line 176-181: The progress log incorrectly documents volunteer shift ID
prefixes as "vshf_" and "vsha_" but the actual constants used are defined in
common.ts (see createVolunteerShiftId and related ID helpers) and use "vshft_"
and "vshfta_"; update all occurrences in scripts/ralph/progress.txt (including
lines ~201-206 and the current 176-181 section) to match the real prefixes
("vshft_" and "vshfta_") so readers aren't misled and reference the
createVolunteerShiftId symbol for verification.
🧹 Nitpick comments (4)
apps/wodsmith-start/src/routes/compete/organizer/$competitionId/-components/shifts/shift-list.tsx (1)

57-65: Use named object params for multi-arg helpers.

Align helper signatures with the repo guideline for functions with more than one parameter.

♻️ Suggested refactor
-function getCapacityBadgeVariant(
-	assigned: number,
-	capacity: number,
-): "default" | "secondary" | "destructive" | "outline" {
+function getCapacityBadgeVariant({
+	assigned,
+	capacity,
+}: {
+	assigned: number
+	capacity: number
+}): "default" | "secondary" | "destructive" | "outline" {
 	if (assigned >= capacity) return "default" // Full
 	if (assigned > 0) return "secondary" // Partially filled
 	return "outline" // Empty
 }
@@
-									const capacityVariant = getCapacityBadgeVariant(
-										assignedCount,
-										shift.capacity,
-									)
+									const capacityVariant = getCapacityBadgeVariant({
+										assigned: assignedCount,
+										capacity: shift.capacity,
+									})

As per coding guidelines, use named object parameters for functions with >1 parameter.

Also applies to: 268-271

apps/wodsmith-start/src/routes/compete/$slug/-components/shift-card.tsx (1)

64-90: Use named object params for formatShiftTimeRange.

This helper takes two positional parameters; prefer a named-parameter object to match the project guideline.

♻️ Suggested refactor
-function formatShiftTimeRange(startTime: Date, endTime: Date): string {
+function formatShiftTimeRange({
+	startTime,
+	endTime,
+}: {
+	startTime: Date
+	endTime: Date
+}): string {
 	const dayFormatter = new Intl.DateTimeFormat("en-US", {
 		weekday: "short",
 	})
@@
-	const timeRangeStr = formatShiftTimeRange(shift.startTime, shift.endTime)
+	const timeRangeStr = formatShiftTimeRange({
+		startTime: shift.startTime,
+		endTime: shift.endTime,
+	})

As per coding guidelines, prefer named object parameters for functions with more than one parameter.

Also applies to: 103-106

apps/wodsmith-start/src/routes/compete/organizer/$competitionId/-components/shifts/shift-form-dialog.tsx (1)

136-144: Use named object params for combineDateAndTime.

This helper takes two positional parameters; please align with the project guideline.

♻️ Suggested refactor
-function combineDateAndTime(date: Date, timeString: string): Date {
-	const [hours, minutes] = timeString.split(":").map(Number)
-	const result = new Date(date)
+function combineDateAndTime({
+	date,
+	timeString,
+}: {
+	date: Date
+	timeString: string
+}): Date {
+	const [hours, minutes] = timeString.split(":").map(Number)
+	const result = new Date(date)
 	result.setHours(hours, minutes, 0, 0)
 	return result
 }
@@
-			const startTime = combineDateAndTime(values.date, values.startTime)
-			const endTime = combineDateAndTime(values.date, values.endTime)
+			const startTime = combineDateAndTime({
+				date: values.date,
+				timeString: values.startTime,
+			})
+			const endTime = combineDateAndTime({
+				date: values.date,
+				timeString: values.endTime,
+			})

As per coding guidelines, prefer named object parameters for functions with more than one parameter.

Also applies to: 214-215

apps/wodsmith-start/src/routes/compete/organizer/$competitionId/-components/shifts/shift-assignment-panel.tsx (1)

61-70: Use named parameters for formatDateRange.

Project guidance prefers named object params for functions with >1 argument; it also makes the call site clearer. As per coding guidelines, use named object parameters for functions with >1 parameter.

♻️ Proposed refactor
-function formatDateRange(startTime: Date, endTime: Date): string {
+function formatDateRange({
+	startTime,
+	endTime,
+}: {
+	startTime: Date
+	endTime: Date
+}): string {
 	const startDate = toDate(startTime)
 	const endDate = toDate(endTime)
 	const dateStr = startDate.toLocaleDateString("en-US", {
 		weekday: "short",
 		month: "short",
 		day: "numeric",
 	})
 	return `${dateStr}, ${formatTime(startDate)} - ${formatTime(endDate)}`
 }
-						{formatDateRange(startTime, endTime)}
+						{formatDateRange({ startTime, endTime })}

Also applies to: 308-311

Comment on lines +52 to +80
// Shift role type enum values (excluding equipment per VS-011 requirements)
const shiftRoleTypeEnum = z.enum([
"judge",
"head_judge",
"medical",
"check_in",
"staff",
"scorekeeper",
"emcee",
"floor_manager",
"media",
"general",
])

type ShiftRoleType = z.infer<typeof shiftRoleTypeEnum>

// Role type options for the select dropdown
const SHIFT_ROLE_TYPES: { value: ShiftRoleType; label: string }[] = [
{ value: "judge", label: "Judge" },
{ value: "head_judge", label: "Head Judge" },
{ value: "medical", label: "Medical" },
{ value: "check_in", label: "Check-In" },
{ value: "staff", label: "Staff" },
{ value: "scorekeeper", label: "Scorekeeper" },
{ value: "emcee", label: "Emcee" },
{ value: "floor_manager", label: "Floor Manager" },
{ value: "media", label: "Media" },
{ value: "general", label: "General" },
]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Time-based shift roles still include judge/head_judge.

The PRD describes time-based scheduling for non-judge roles, but the enum and options still expose judge roles, which can conflict with heat-based judging assignments. Consider removing or gating these roles.

🛠️ Suggested change
 const shiftRoleTypeEnum = z.enum([
-	"judge",
-	"head_judge",
 	"medical",
 	"check_in",
 	"staff",
 	"scorekeeper",
 	"emcee",
 	"floor_manager",
 	"media",
 	"general",
 ])
@@
 const SHIFT_ROLE_TYPES: { value: ShiftRoleType; label: string }[] = [
-	{ value: "judge", label: "Judge" },
-	{ value: "head_judge", label: "Head Judge" },
 	{ value: "medical", label: "Medical" },
 	{ value: "check_in", label: "Check-In" },
 	{ value: "staff", label: "Staff" },
 	{ value: "scorekeeper", label: "Scorekeeper" },
 	{ value: "emcee", label: "Emcee" },
 	{ value: "floor_manager", label: "Floor Manager" },
 	{ value: "media", label: "Media" },
 	{ value: "general", label: "General" },
 ]

Based on PR objectives, time-based shifts should exclude judge roles.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Shift role type enum values (excluding equipment per VS-011 requirements)
const shiftRoleTypeEnum = z.enum([
"judge",
"head_judge",
"medical",
"check_in",
"staff",
"scorekeeper",
"emcee",
"floor_manager",
"media",
"general",
])
type ShiftRoleType = z.infer<typeof shiftRoleTypeEnum>
// Role type options for the select dropdown
const SHIFT_ROLE_TYPES: { value: ShiftRoleType; label: string }[] = [
{ value: "judge", label: "Judge" },
{ value: "head_judge", label: "Head Judge" },
{ value: "medical", label: "Medical" },
{ value: "check_in", label: "Check-In" },
{ value: "staff", label: "Staff" },
{ value: "scorekeeper", label: "Scorekeeper" },
{ value: "emcee", label: "Emcee" },
{ value: "floor_manager", label: "Floor Manager" },
{ value: "media", label: "Media" },
{ value: "general", label: "General" },
]
// Shift role type enum values (excluding equipment per VS-011 requirements)
const shiftRoleTypeEnum = z.enum([
"medical",
"check_in",
"staff",
"scorekeeper",
"emcee",
"floor_manager",
"media",
"general",
])
type ShiftRoleType = z.infer<typeof shiftRoleTypeEnum>
// Role type options for the select dropdown
const SHIFT_ROLE_TYPES: { value: ShiftRoleType; label: string }[] = [
{ value: "medical", label: "Medical" },
{ value: "check_in", label: "Check-In" },
{ value: "staff", label: "Staff" },
{ value: "scorekeeper", label: "Scorekeeper" },
{ value: "emcee", label: "Emcee" },
{ value: "floor_manager", label: "Floor Manager" },
{ value: "media", label: "Media" },
{ value: "general", label: "General" },
]
🤖 Prompt for AI Agents
In
`@apps/wodsmith-start/src/routes/compete/organizer/`$competitionId/-components/shifts/shift-form-dialog.tsx
around lines 52 - 80, The shift role enum and dropdown still include
judge/head_judge which must be excluded for time-based scheduling; update the
z.enum definition shiftRoleTypeEnum to remove "judge" and "head_judge" and
remove their entries from the SHIFT_ROLE_TYPES array (types derived via
ShiftRoleType will update automatically), and verify any place that expects
those values (e.g., form validation or value mapping) is adjusted or gated
accordingly so time-based shift flows never expose judge/head_judge options.

Comment on lines +217 to +229
if (isEditing && shift) {
// Update existing shift
await updateShift({
data: {
shiftId: shift.id,
name: values.name,
roleType: values.roleType,
startTime,
endTime,
location: values.location || undefined,
capacity: values.capacity,
notes: values.notes || undefined,
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Editing can’t clear location/notes.

Emptying the fields sends undefined, which the update handler treats as “no change,” so prior values persist. Send null (and ensure the server update path preserves null) to allow clearing.

🛠️ Suggested fix
-						location: values.location || undefined,
+						location: values.location?.trim() ? values.location : null,
 						capacity: values.capacity,
-						notes: values.notes || undefined,
+						notes: values.notes?.trim() ? values.notes : null,
🤖 Prompt for AI Agents
In
`@apps/wodsmith-start/src/routes/compete/organizer/`$competitionId/-components/shifts/shift-form-dialog.tsx
around lines 217 - 229, When updating an existing shift in the isEditing branch,
emptying location/notes currently sends undefined (treated as no-op) so prior
values persist; change the payload construction in updateShift to send null for
cleared fields by mapping values.location and values.notes to (values.location
=== "" ? null : values.location) and (values.notes === "" ? null : values.notes)
respectively (refer to updateShift call and values.location/values.notes) and
ensure the server update handler accepts null to remove the stored values.

Comment on lines +108 to +119
export function ShiftList({
competitionId,
competitionTeamId,
shifts: initialShifts,
}: ShiftListProps) {
const [shifts, setShifts] = useState(initialShifts)
const [deletingShiftId, setDeletingShiftId] = useState<string | null>(null)
const [isDeleting, setIsDeleting] = useState(false)
const [formDialogOpen, setFormDialogOpen] = useState(false)
const [editingShift, setEditingShift] = useState<VolunteerShift | undefined>(undefined)
const [assignmentPanelOpen, setAssignmentPanelOpen] = useState(false)
const [selectedShift, setSelectedShift] = useState<ShiftWithAssignments | null>(null)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n apps/wodsmith-start/src/routes/compete/organizer/'$competitionId'/-components/shifts/shift-list.tsx | head -150

Repository: wodsmith/thewodapp

Length of output: 5424


🏁 Script executed:

cat -n apps/wodsmith-start/src/routes/compete/organizer/'$competitionId'/-components/shifts/shift-list.tsx | tail -200

Repository: wodsmith/thewodapp

Length of output: 7720


🏁 Script executed:

cat -n apps/wodsmith-start/src/routes/compete/organizer/'$competitionId'/-components/shifts/shift-list.tsx | sed -n '144,195p'

Repository: wodsmith/thewodapp

Length of output: 1800


🏁 Script executed:

cat -n apps/wodsmith-start/src/routes/compete/organizer/'$competitionId'/index.tsx | head -100

Repository: wodsmith/thewodapp

Length of output: 3994


🏁 Script executed:

find apps/wodsmith-start/src/routes/compete/organizer -name "*shift*" -type f

Repository: wodsmith/thewodapp

Length of output: 374


🏁 Script executed:

rg "ShiftList" apps/wodsmith-start/src/routes/compete/organizer --type tsx

Repository: wodsmith/thewodapp

Length of output: 89


🏁 Script executed:

rg "ShiftList" apps/wodsmith-start/src/routes/compete/organizer

Repository: wodsmith/thewodapp

Length of output: 665


🏁 Script executed:

cat -n apps/wodsmith-start/src/routes/compete/organizer/'$competitionId'/volunteers.tsx

Repository: wodsmith/thewodapp

Length of output: 11298


🏁 Script executed:

cat -n apps/wodsmith-start/src/routes/compete/organizer/'$competitionId'/-components/shifts/shift-form-dialog.tsx | head -80

Repository: wodsmith/thewodapp

Length of output: 2699


🏁 Script executed:

cat -n apps/wodsmith-start/src/routes/compete/organizer/'$competitionId'/-components/shifts/shift-form-dialog.tsx | grep -A 30 "handleSubmit\|createShiftFn\|updateShiftFn" | head -60

Repository: wodsmith/thewodapp

Length of output: 2052


🏁 Script executed:

cat -n apps/wodsmith-start/src/routes/compete/organizer/'$competitionId'/-components/shifts/shift-form-dialog.tsx | sed -n '176,250p'

Repository: wodsmith/thewodapp

Length of output: 2508


🏁 Script executed:

cat -n apps/wodsmith-start/src/routes/compete/organizer/'$competitionId'/-components/shifts/shift-form-dialog.tsx | sed -n '248,255p'

Repository: wodsmith/thewodapp

Length of output: 265


🏁 Script executed:

rg "getCapacityBadgeVariant" apps/wodsmith-start/src/routes/compete/organizer/'$competitionId'/-components/shifts/shift-list.tsx

Repository: wodsmith/thewodapp

Length of output: 153


🏁 Script executed:

cat -n apps/wodsmith-start/src/routes/compete/organizer/'$competitionId'/-components/shifts/shift-list.tsx | sed -n '268,272p'

Repository: wodsmith/thewodapp

Length of output: 217


🏁 Script executed:

rg "router.invalidate()" apps/wodsmith-start/src/routes/compete/organizer/'$competitionId'/-components/shifts/

Repository: wodsmith/thewodapp

Length of output: 189


Add useEffect to sync shifts state when prop changes, and refactor helper function to use named parameters.

After create/edit, router.invalidate() refetches shifts in the loader, but useState(initialShifts) only initializes once—the component won't reflect new data without manually reloading the page. Sync state when initialShifts changes by adding a useEffect dependency.

Additionally, getCapacityBadgeVariant uses positional parameters but should accept a named object per the coding guideline for functions with >1 parameter.

🛠️ Proposed fixes
-import { useCallback, useMemo, useState } from "react"
+import { useCallback, useEffect, useMemo, useState } from "react"

 function getCapacityBadgeVariant(
-	assigned: number,
-	capacity: number,
+	{ assigned, capacity }: { assigned: number; capacity: number },
 ): "default" | "secondary" | "destructive" | "outline" {
 	if (assigned >= capacity) return "default"
 	if (assigned > 0) return "secondary"
 	return "outline"
 }

 export function ShiftList({
 	competitionId,
 	competitionTeamId,
 	shifts: initialShifts,
 }: ShiftListProps) {
 	const [shifts, setShifts] = useState(initialShifts)
+	useEffect(() => {
+		setShifts(initialShifts)
+	}, [initialShifts])

-					const capacityVariant = getCapacityBadgeVariant(
-						assignedCount,
-						shift.capacity,
-					)
+					const capacityVariant = getCapacityBadgeVariant({
+						assigned: assignedCount,
+						capacity: shift.capacity,
+					})
🤖 Prompt for AI Agents
In
`@apps/wodsmith-start/src/routes/compete/organizer/`$competitionId/-components/shifts/shift-list.tsx
around lines 108 - 119, The ShiftList component initializes local state with
useState(initialShifts) but never syncs when props change—add a useEffect in
ShiftList that watches initialShifts and calls setShifts(initialShifts) so the
list updates after router.invalidate(); also refactor the helper
getCapacityBadgeVariant to accept a single object parameter (e.g., { capacity,
filled }) instead of positional args and update all call sites in this file to
pass a named object to match the project guideline.

Comment on lines +270 to +293
// Query volunteer's shift assignments with shift details
const shiftAssignments = await db
.select({
shift: {
id: volunteerShiftsTable.id,
name: volunteerShiftsTable.name,
roleType: volunteerShiftsTable.roleType,
startTime: volunteerShiftsTable.startTime,
endTime: volunteerShiftsTable.endTime,
location: volunteerShiftsTable.location,
notes: volunteerShiftsTable.notes,
},
})
.from(volunteerShiftAssignmentsTable)
.innerJoin(
volunteerShiftsTable,
eq(volunteerShiftAssignmentsTable.shiftId, volunteerShiftsTable.id),
)
.where(
and(
eq(volunteerShiftAssignmentsTable.membershipId, data.membershipId),
eq(volunteerShiftsTable.competitionId, data.competitionId),
),
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Scope shift assignment query by team context.

The new query filters by membershipId and competitionId only. To meet multi-tenant requirements, add a team/competition-team guard (e.g., join through team membership/competition team ID) rather than relying solely on competitionId. As per coding guidelines, always filter multi-tenant queries by team context.

🤖 Prompt for AI Agents
In `@apps/wodsmith-start/src/server-fns/volunteer-schedule-fns.ts` around lines
270 - 293, The query fetching shiftAssignments (using db.select on
volunteerShiftAssignmentsTable joined to volunteerShiftsTable) is only filtered
by membershipId and competitionId and must be scoped by team context; update the
query to join the table that links competitions to teams (e.g., competitionTeams
or competition_team table) or the membership->team table and add a predicate
that ensures the shift's competitionTeam/teamId matches the caller's team
context (use the same team identifier available in data or derive it via
membership) so multi-tenant filtering is enforced alongside the existing
eq(volunteerShiftAssignmentsTable.membershipId, data.membershipId) and
eq(volunteerShiftsTable.competitionId, data.competitionId) conditions.

Comment on lines +1 to +10
/**
* Volunteer Shift CRUD Server Functions for TanStack Start
* Functions for managing volunteer shifts (time-based shifts for non-judge roles)
* These are admin-only operations requiring MANAGE_COMPETITIONS permission.
*/

import { createServerFn } from "@tanstack/react-start"
import { and, asc, eq, inArray } from "drizzle-orm"
import { z } from "zod"
import { getDb } from "@/db"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add server-only marker for this module.

Server functions should be explicitly marked to avoid accidental client bundling. Based on learnings, server-fns files must include import "server-only".

🔧 Proposed fix
+import "server-only"
+
 import { createServerFn } from "@tanstack/react-start"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Volunteer Shift CRUD Server Functions for TanStack Start
* Functions for managing volunteer shifts (time-based shifts for non-judge roles)
* These are admin-only operations requiring MANAGE_COMPETITIONS permission.
*/
import { createServerFn } from "@tanstack/react-start"
import { and, asc, eq, inArray } from "drizzle-orm"
import { z } from "zod"
import { getDb } from "@/db"
/**
* Volunteer Shift CRUD Server Functions for TanStack Start
* Functions for managing volunteer shifts (time-based shifts for non-judge roles)
* These are admin-only operations requiring MANAGE_COMPETITIONS permission.
*/
import "server-only"
import { createServerFn } from "@tanstack/react-start"
import { and, asc, eq, inArray } from "drizzle-orm"
import { z } from "zod"
import { getDb } from "@/db"
🤖 Prompt for AI Agents
In `@apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts` around lines 1 -
10, Add a top-level bare import "server-only" to this module (before other
imports) to mark it as server-only and prevent client bundling; update the file
containing createServerFn and getDb (the volunteer shift server-fns module) by
inserting import "server-only" at the top of the file as a standalone import
statement.

Comment on lines +32 to +35
const shiftIdSchema = z
.string()
.startsWith("vshf_", "Invalid volunteer shift ID")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

rg -n "createVolunteerShiftId" --type ts --type tsx

Repository: wodsmith/thewodapp

Length of output: 89


🏁 Script executed:

rg -n "vshf_|vshft_" --type ts --type tsx

Repository: wodsmith/thewodapp

Length of output: 89


🏁 Script executed:

find . -name "*volunteer-shift*" -type f | head -20

Repository: wodsmith/thewodapp

Length of output: 335


🏁 Script executed:

rg -n "createVolunteerShiftId" --max-count 20

Repository: wodsmith/thewodapp

Length of output: 693


🏁 Script executed:

rg -n "vshf_|vshft_" --max-count 20

Repository: wodsmith/thewodapp

Length of output: 2753


🏁 Script executed:

cat -n apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts | head -60

Repository: wodsmith/thewodapp

Length of output: 2397


🏁 Script executed:

rg -n "shiftIdSchema|shiftId" apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts | head -40

Repository: wodsmith/thewodapp

Length of output: 1238


🏁 Script executed:

rg -n "fixture|mock" apps/wodsmith-start/test/server-fns/volunteer-shifts.test.ts | head -20

Repository: wodsmith/thewodapp

Length of output: 1029


🏁 Script executed:

rg -n "createVolunteerShiftId\(\)" apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts

Repository: wodsmith/thewodapp

Length of output: 44


🏁 Script executed:

sed -n '170,190p' apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts

Repository: wodsmith/thewodapp

Length of output: 676


🏁 Script executed:

sed -n '240,260p' apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts

Repository: wodsmith/thewodapp

Length of output: 598


Fix shift ID prefix mismatch: createVolunteerShiftId() generates vshft_ but schema validates vshf_.

Real volunteer shifts created with createVolunteerShiftId() will have the vshft_ prefix and fail validation during update, delete, and assign operations. This affects updateShiftFn, deleteShiftFn, assignShiftFn, unassignShiftFn, getShiftAssignmentsFn, and reassignShiftFn. Update the schema to accept both prefixes while you standardize:

Proposed fix
const shiftIdSchema = z
	.string()
-	.startsWith("vshf_", "Invalid volunteer shift ID")
+	.regex(/^vshft?_/, "Invalid volunteer shift ID")

Also update test fixtures from vshf_ to vshft_ to match actual IDs.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const shiftIdSchema = z
.string()
.startsWith("vshf_", "Invalid volunteer shift ID")
const shiftIdSchema = z
.string()
.regex(/^vshft?_/, "Invalid volunteer shift ID")
🤖 Prompt for AI Agents
In `@apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts` around lines 32 -
35, The shift ID schema (shiftIdSchema) currently validates IDs starting with
"vshf_" but createVolunteerShiftId() emits "vshft_", causing real IDs to fail
validation in updateShiftFn, deleteShiftFn, assignShiftFn, unassignShiftFn,
getShiftAssignmentsFn, and reassignShiftFn; update shiftIdSchema to accept both
"vshf_" and "vshft_" prefixes (e.g., use a regex or z.union/z.preprocess to
allow either prefix) so existing code paths validate correctly, and update any
test fixtures that use the old "vshf_" prefix to "vshft_" to match
createVolunteerShiftId().

Comment on lines +208 to +216
if (data.name !== undefined) updateValues.name = data.name
if (data.roleType !== undefined) updateValues.roleType = data.roleType
if (data.startTime !== undefined) updateValues.startTime = data.startTime
if (data.endTime !== undefined) updateValues.endTime = data.endTime
if (data.location !== undefined)
updateValues.location = data.location ?? undefined
if (data.capacity !== undefined) updateValues.capacity = data.capacity
if (data.notes !== undefined) updateValues.notes = data.notes ?? undefined

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts | sed -n '200,220p'

Repository: wodsmith/thewodapp

Length of output: 1134


🏁 Script executed:

fd volunteer-shift-fns.ts

Repository: wodsmith/thewodapp

Length of output: 119


🏁 Script executed:

find apps/wodsmith-start/src -type f -name "*volunteer*" | head -20

Repository: wodsmith/thewodapp

Length of output: 1478


🏁 Script executed:

cat -n apps/wodsmith-start/src/db/schemas/volunteers.ts

Repository: wodsmith/thewodapp

Length of output: 18342


🏁 Script executed:

rg "volunteerShiftsTable" apps/wodsmith-start/src/db/schemas/ -A 30

Repository: wodsmith/thewodapp

Length of output: 10522


🏁 Script executed:

cat -n apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts | sed -n '220,240p'

Repository: wodsmith/thewodapp

Length of output: 636


🏁 Script executed:

rg "updateVolunteerShift\|\.update\(" apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts -A 3 -B 3

Repository: wodsmith/thewodapp

Length of output: 44


🏁 Script executed:

rg "updateVolunteerShift" apps/wodsmith-start/src -B 3 -A 3

Repository: wodsmith/thewodapp

Length of output: 44


🏁 Script executed:

cat -n apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts | sed -n '150,180p'

Repository: wodsmith/thewodapp

Length of output: 907


🏁 Script executed:

cat -n apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts | sed -n '175,210p'

Repository: wodsmith/thewodapp

Length of output: 1590


Preserve null values to allow clearing nullable fields on update.

The input validator accepts .nullable() for location and notes, allowing callers to explicitly pass null to clear these fields. However, the nullish coalescing operator (??) converts null to undefined, preventing Drizzle from setting these columns to NULL. Remove the nullish coalescing to preserve explicit null values.

🔧 Proposed fix
-	if (data.location !== undefined)
-		updateValues.location = data.location ?? undefined
+	if (data.location !== undefined) updateValues.location = data.location
 ...
-	if (data.notes !== undefined) updateValues.notes = data.notes ?? undefined
+	if (data.notes !== undefined) updateValues.notes = data.notes
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (data.name !== undefined) updateValues.name = data.name
if (data.roleType !== undefined) updateValues.roleType = data.roleType
if (data.startTime !== undefined) updateValues.startTime = data.startTime
if (data.endTime !== undefined) updateValues.endTime = data.endTime
if (data.location !== undefined)
updateValues.location = data.location ?? undefined
if (data.capacity !== undefined) updateValues.capacity = data.capacity
if (data.notes !== undefined) updateValues.notes = data.notes ?? undefined
if (data.name !== undefined) updateValues.name = data.name
if (data.roleType !== undefined) updateValues.roleType = data.roleType
if (data.startTime !== undefined) updateValues.startTime = data.startTime
if (data.endTime !== undefined) updateValues.endTime = data.endTime
if (data.location !== undefined)
updateValues.location = data.location
if (data.capacity !== undefined) updateValues.capacity = data.capacity
if (data.notes !== undefined) updateValues.notes = data.notes
🤖 Prompt for AI Agents
In `@apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts` around lines 208 -
216, The update block currently converts explicit nulls to undefined using the
nullish coalescing operator, preventing clearing nullable DB columns; inside the
conditional guards (e.g., the checks `if (data.location !== undefined)` and `if
(data.notes !== undefined)`), assign the raw input values to updateValues (e.g.,
`updateValues.location = data.location` and `updateValues.notes = data.notes`)
instead of using `?? undefined` so that explicit nulls are preserved and Drizzle
can write NULL to the database.

Comment on lines +283 to +301
async function getShiftWithAuthCheck(shiftId: string) {
const db = getDb()

const shift = await db.query.volunteerShiftsTable.findFirst({
where: eq(volunteerShiftsTable.id, shiftId),
with: {
assignments: true,
},
})

if (!shift) {
throw new Error("NOT_FOUND: Volunteer shift not found")
}

// Validate permission for this competition
await getCompetitionWithAuthCheck(shift.competitionId)

return shift
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts | head -320 | tail -50

Repository: wodsmith/thewodapp

Length of output: 1654


🏁 Script executed:

wc -l apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts

Repository: wodsmith/thewodapp

Length of output: 123


🏁 Script executed:

cat -n apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts | sed -n '283,310p'

Repository: wodsmith/thewodapp

Length of output: 893


🏁 Script executed:

cat -n apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts | sed -n '322,375p'

Repository: wodsmith/thewodapp

Length of output: 1899


🏁 Script executed:

cat -n apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts | sed -n '571,595p'

Repository: wodsmith/thewodapp

Length of output: 1052


🏁 Script executed:

cat -n apps/wodsmith-start/src/db/schemas/teams.ts | head -100

Repository: wodsmith/thewodapp

Length of output: 3318


🏁 Script executed:

rg "getCompetitionWithAuthCheck" apps/wodsmith-start/src/server-fns/ -A 10 -B 2

Repository: wodsmith/thewodapp

Length of output: 8167


🏁 Script executed:

rg "organizingTeamId" apps/wodsmith-start/src/db/schemas/competitions.ts -A 2 -B 2

Repository: wodsmith/thewodapp

Length of output: 1232


🏁 Script executed:

rg "teamId" apps/wodsmith-start/src/db/schemas/teams.ts -A 2 -B 2 | head -40

Repository: wodsmith/thewodapp

Length of output: 1184


🏁 Script executed:

cat -n apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts | sed -n '1,50p'

Repository: wodsmith/thewodapp

Length of output: 1997


🏁 Script executed:

rg "async function getCompetitionWithAuthCheck" apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts -A 15

Repository: wodsmith/thewodapp

Length of output: 509


🏁 Script executed:

cat -n apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts | sed -n '540,610p'

Repository: wodsmith/thewodapp

Length of output: 2937


🏁 Script executed:

cat -n apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts | sed -n '243,270p'

Repository: wodsmith/thewodapp

Length of output: 1024


🏁 Script executed:

cat -n apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts | sed -n '500,560p'

Repository: wodsmith/thewodapp

Length of output: 2233


🏁 Script executed:

rg "removeVolunteerFromShiftFn|getVolunteerAssignmentsFn|bulkAssignVolunteersFn" apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts -A 30 | head -100

Repository: wodsmith/thewodapp

Length of output: 44


🏁 Script executed:

cat -n apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts | sed -n '379,430p'

Repository: wodsmith/thewodapp

Length of output: 1645


🏁 Script executed:

cat -n apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts | sed -n '420,480p'

Repository: wodsmith/thewodapp

Length of output: 2140


🏁 Script executed:

rg "async function getCompetitionWithAuthCheck" apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts -A 25

Repository: wodsmith/thewodapp

Length of output: 748


🏁 Script executed:

rg "getShiftWithAuthCheck" apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts

Repository: wodsmith/thewodapp

Length of output: 321


🏁 Script executed:

cat -n apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts | sed -n '260,280p'

Repository: wodsmith/thewodapp

Length of output: 868


🏁 Script executed:

rg "competitionsTable" apps/wodsmith-start/src/db/schema.ts -A 5 | head -30

Repository: wodsmith/thewodapp

Length of output: 44


🏁 Script executed:

cat -n apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts | sed -n '198,230p'

Repository: wodsmith/thewodapp

Length of output: 1597


🏁 Script executed:

rg "teamMembershipTable.findFirst\|teamMembershipTable.findMany" apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts -B 2 -A 5

Repository: wodsmith/thewodapp

Length of output: 44


🏁 Script executed:

rg "teamMembershipTable" apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts -B 3 -A 3

Repository: wodsmith/thewodapp

Length of output: 1204


🏁 Script executed:

cat -n apps/wodsmith-start/src/db/schemas/competitions.ts | grep -A 30 "export const competitionsTable"

Repository: wodsmith/thewodapp

Length of output: 1473


Filter membership lookups by teamId to prevent cross-tenant assignment.

Membership existence checks only verify id without filtering by teamId, allowing assignments for memberships outside the competition's organizing team. Filter all membership lookups by the competition's organizingTeamId and update the bulk lookup's autochunk parameter count.

🔧 Proposed fix
-async function getShiftWithAuthCheck(shiftId: string) {
+async function getShiftWithAuthCheck(shiftId: string) {
 	const db = getDb()
 	const shift = await db.query.volunteerShiftsTable.findFirst({
 		where: eq(volunteerShiftsTable.id, shiftId),
 		with: { assignments: true },
 	})

 	if (!shift) {
 		throw new Error("NOT_FOUND: Volunteer shift not found")
 	}

-	await getCompetitionWithAuthCheck(shift.competitionId)
-	return shift
+	const competition = await getCompetitionWithAuthCheck(shift.competitionId)
+	return { shift, competition }
 }
 ...
-	const shift = await getShiftWithAuthCheck(data.shiftId)
+	const { shift, competition } = await getShiftWithAuthCheck(data.shiftId)
 ...
-	const membership = await db.query.teamMembershipTable.findFirst({
-		where: eq(teamMembershipTable.id, data.membershipId),
-	})
+	const membership = await db.query.teamMembershipTable.findFirst({
+		where: and(
+			eq(teamMembershipTable.id, data.membershipId),
+			eq(teamMembershipTable.teamId, competition.organizingTeamId),
+		),
+	})
 ...
-	const shift = await getShiftWithAuthCheck(data.shiftId)
+	const { shift, competition } = await getShiftWithAuthCheck(data.shiftId)
 ...
-	const memberships = await autochunk(
-		{ items: newMembershipIds, otherParametersCount: 0 },
+	const memberships = await autochunk(
+		{ items: newMembershipIds, otherParametersCount: 1 },
 		async (chunk) =>
 			db
 				.select({ id: teamMembershipTable.id })
 				.from(teamMembershipTable)
-				.where(inArray(teamMembershipTable.id, chunk)),
+				.where(
+					and(
+						eq(teamMembershipTable.teamId, competition.organizingTeamId),
+						inArray(teamMembershipTable.id, chunk),
+					),
+				),
 	)

Also applies to: 322-372, 571-590

🤖 Prompt for AI Agents
In `@apps/wodsmith-start/src/server-fns/volunteer-shift-fns.ts` around lines 283 -
301, The membership existence checks for volunteer shift assignments are only
filtering by membership id and not by the competition's organizing team, which
permits cross-tenant assignments; in functions handling shifts (e.g.,
getShiftWithAuthCheck and the code that loads shift.assignments) add a filter to
membership lookups to include the competition.organizingTeamId (obtain via
getCompetitionWithAuthCheck(shift.competitionId)) so queries check both id and
teamId, and update any bulk membership lookup call’s autochunk parameter to
match the new chunking/count expectations used elsewhere.

Comment on lines +176 to +181
**Learnings:**
- Use `.inputValidator((data: unknown) => z.object({...}).parse(data))` not `.validator()` - the pattern differs from the newer TanStack Start docs
- The `asc` ordering function is imported from `drizzle-orm` for orderBy clauses
- For nullable optional fields in update operations, use pattern: `data.field !== undefined ? (data.field ?? undefined) : undefined` to distinguish between "not provided" and "set to null"
- Competition authorization checks should go through the competition's `organizingTeamId` - that's the team with MANAGE_COMPETITIONS permission
- The shift ID prefix is `vshf_` (defined in common.ts as createVolunteerShiftId)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix volunteer shift ID prefix references.

The progress log lists vshf_ / vsha_, but the actual prefixes in apps/wodsmith-start/src/db/schemas/common.ts are vshft_ and vshfta_. This can mislead future readers.

📝 Suggested fix
- - The shift ID prefix is `vshf_` (defined in common.ts as createVolunteerShiftId)
+ - The shift ID prefix is `vshft_` (defined in common.ts as createVolunteerShiftId)
...
- - The shift assignment ID prefix is `vsha_` (defined in common.ts as createVolunteerShiftAssignmentId)
+ - The shift assignment ID prefix is `vshfta_` (defined in common.ts as createVolunteerShiftAssignmentId)

Also applies to: 201-206

🤖 Prompt for AI Agents
In `@scripts/ralph/progress.txt` around lines 176 - 181, The progress log
incorrectly documents volunteer shift ID prefixes as "vshf_" and "vsha_" but the
actual constants used are defined in common.ts (see createVolunteerShiftId and
related ID helpers) and use "vshft_" and "vshfta_"; update all occurrences in
scripts/ralph/progress.txt (including lines ~201-206 and the current 176-181
section) to match the real prefixes ("vshft_" and "vshfta_") so readers aren't
misled and reference the createVolunteerShiftId symbol for verification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants