[codex] Add crew volunteer broadcast tools - #612
Open
zacjones93 wants to merge 6 commits into
Open
Conversation
Entire-Checkpoint: 553e86f4a2ae
Contributor
WalkthroughAdds a crew volunteer messaging composer with template storage, recipient preview and selection, server-side queueing for assignment and broadcast messages, a tabbed Messages page, and supporting schema, email, and documentation updates. ChangesCrew Messaging Composer
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ComposeTab as MessagesComposeTab
participant ServerFn as queueCrewMessagesFn
participant Server as crew-messages.server.ts
participant Confirmation as queueCrewTemplatedAssignmentEmails
participant Queue as Broadcast Queue
User->>ComposeTab: choose recipients and send
ComposeTab->>ServerFn: queueCrewMessagesFn(eventId, mode, recipientKeys, template)
ServerFn->>Server: queueCrewMessages(data)
Server->>Confirmation: queueCrewTemplatedAssignmentEmails(recipientConfirmationIds, render)
Confirmation->>Queue: send email message
Queue-->>Confirmation: queued / failed
Confirmation-->>Server: counts and skips
Server-->>ServerFn: QueueCrewMessagesResult
ServerFn-->>ComposeTab: success or error result
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
theianjones
marked this pull request as ready for review
July 4, 2026 05:15
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
9 issues found across 20 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/crew/src/server/crew-confirmation.server.ts">
<violation number="1" location="apps/crew/src/server/crew-confirmation.server.ts:1101">
P3: The assignment-email queue workflow now exists in two near-identical functions, which makes lock/idempotency/skipped-counter fixes easy to apply in one path and miss in the other. A shared helper for the common queue/claim/finalize loop with an injected render step would reduce that maintenance risk.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| * {@link queueCrewAssignmentConfirmationEmails}. Confirmation ids that are not | ||
| * eligible (already sent, responded, past shift, etc.) are ignored. | ||
| */ | ||
| export async function queueCrewTemplatedAssignmentEmails(params: { |
Contributor
There was a problem hiding this comment.
P3: The assignment-email queue workflow now exists in two near-identical functions, which makes lock/idempotency/skipped-counter fixes easy to apply in one path and miss in the other. A shared helper for the common queue/claim/finalize loop with an injected render step would reduce that maintenance risk.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/crew/src/server/crew-confirmation.server.ts, line 1101:
<comment>The assignment-email queue workflow now exists in two near-identical functions, which makes lock/idempotency/skipped-counter fixes easy to apply in one path and miss in the other. A shared helper for the common queue/claim/finalize loop with an injected render step would reduce that maintenance risk.</comment>
<file context>
@@ -1064,6 +1065,185 @@ export async function queueCrewAssignmentConfirmationEmails(
+ * {@link queueCrewAssignmentConfirmationEmails}. Confirmation ids that are not
+ * eligible (already sent, responded, past shift, etc.) are ignored.
+ */
+export async function queueCrewTemplatedAssignmentEmails(params: {
+ eventId: string
+ mode: CrewAssignmentConfirmationEmailOperationMode
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/wodsmith-db/src/schemas/crew-message-templates.ts (1)
1-73: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMissing
@latcode reference.This new schema file has no
//@lat: [[section-id]]comment anywhere, unlike sibling crew schemas that are tied tolat.md/crew.mdsections (e.g.,crew-billing-events,crew-self-serve-presets). Given this PR's stack explicitly addslat.md/crew.mddocumentation for this feature and the PR description listslat checkas required validation, this file is likely to fail that check.📝 Suggested fix
+// `@lat`: [[crew#Volunteer Messaging Composer#Crew message templates schema]] +export const crewMessageTemplatesTable = mysqlTable( -export const crewMessageTemplatesTable = mysqlTable(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/wodsmith-db/src/schemas/crew-message-templates.ts` around lines 1 - 73, Add the missing `@lat` reference comment to crewMessageTemplatesTable so this schema is linked to the appropriate lat.md/crew.md section like the sibling crew schema files. Place the comment near the top-level schema definition in crew-message-templates.ts and use the same section-id convention already used by crew-billing-events and crew-self-serve-presets so lat check can associate this table with the new documentation.Source: Coding guidelines
🧹 Nitpick comments (4)
apps/crew/src/routes/events/$eventId/messages.tsx (2)
76-97: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTab list is missing full ARIA wiring.
role="tab"/aria-selectedis set, but there's noaria-controls/matchingidlinking each tab button to its panel, and the panels below aren't markedrole="tabpanel". Click/keyboard-Tab still works, but this isn't a complete ARIA tabs pattern for screen readers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/crew/src/routes/events/`$eventId/messages.tsx around lines 76 - 97, The tab UI in messages.tsx is only partially wired for accessibility; update the MESSAGE_TABS button rendering and the associated panel markup so each tab button in the tablist has a stable id plus aria-controls pointing to its matching panel, and each panel has the corresponding id and role="tabpanel" (with appropriate hidden/selected state tied to activeTab). Use the existing activeTab, handleTabChange, and MESSAGE_TABS mapping to keep the tab and panel linkage consistent.
56-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate timezone fallback logic.
composer.event.timezone ?? dashboard.event.timezone ?? "America/Denver"here vs.dashboard.event.timezone ?? "America/Denver"independently inMessagesResponsesTab(apps/crew/src/routes/events/$eventId/-components/messages-responses-tab.tsx, line 74). Consider extracting a singleresolveCrewMessagingTimezone(...)helper (or just pass this component's computedtimezonedown toMessagesResponsesTabtoo) so the fallback and precedence stay in one place.♻️ Proposed fix
{activeTab === "responses" ? ( - <MessagesResponsesTab eventId={eventId} dashboard={dashboard} /> + <MessagesResponsesTab + eventId={eventId} + dashboard={dashboard} + timezone={timezone} + /> ) : null}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/crew/src/routes/events/`$eventId/messages.tsx around lines 56 - 57, The timezone fallback logic is duplicated between Messages and MessagesResponsesTab, which can drift over time. Extract the precedence into a single resolveCrewMessagingTimezone helper or pass the already computed timezone from Messages into MessagesResponsesTab, and update the MessagesResponsesTab usage so it reuses that shared value instead of recalculating dashboard.event.timezone ?? "America/Denver" independently.apps/crew/src/routes/events/$eventId/-components/messages-responses-tab.tsx (1)
309-316: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: list key includes array
index.Combining
indexinto the key is unnecessary ifvolunteerName:shiftName:startsAtis already unique per row; keeping it doesn't cause a bug today but slightly weakens key stability if rows are ever reordered.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/crew/src/routes/events/`$eventId/-components/messages-responses-tab.tsx around lines 309 - 316, The list key in AssignmentResponseRow currently mixes in the array index, which weakens key stability for no gain if volunteerName, shiftName, and startsAt already uniquely identify each row. Update the rows.map rendering in messages-responses-tab.tsx to use a stable unique key built only from the row fields already present, and keep the AssignmentResponseRow props unchanged.apps/crew/src/lib/crew/message-recipients.ts (1)
107-130: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUntested edge case:
reminderCountjumps from 0 straight to 2.When
reminderCountis 0 and the shift falls within the 24-hour window,getReminderDuereturnsreminderCount: 2directly, skipping the "48-hour" reminder stage entirely. This may be intentional (send the more urgent reminder when none has gone out), but it isn't covered by the test suite. Worth adding a test to lock in the intended behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/crew/src/lib/crew/message-recipients.ts` around lines 107 - 130, The reminder progression in getReminderDue can jump from 0 directly to 2 when the shift is within the 24-hour window, and this behavior is currently untested. Add a test around getReminderDue in message-recipients.ts that covers reminderCount = 0 with hoursUntilShift inside the 24-hour threshold and asserts the returned reminderCount is 2, so the intended escalation path is locked in.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/crew/src/db/schemas/crew-message-templates.ts`:
- Line 1: The crew-message-templates module is missing the required `@lat` code
reference tag, which may cause the lat check to fail. Update the
crew-message-templates export file to include a matching // `@lat`: [[section-id]]
comment consistent with the sibling message-templates.ts and
templated-message.tsx files, keeping the export intact.
In `@apps/crew/src/routes/events/`$eventId/-components/messages-compose-tab.tsx:
- Around line 143-162: The preview fetch in the messages compose tab silently
treats errors as empty results, so surface failures instead of only clearing
recipients. Update the useEffect that calls previewCrewMessageRecipientsFn in
messages-compose-tab to capture the error in the .catch() path and show a toast
or error state, while keeping the existing loading and cancellation behavior
intact. Make sure the user can distinguish a failed preview request from a valid
“no recipients” result.
In
`@apps/crew/src/routes/events/`$eventId/-components/messages-template-editor.tsx:
- Around line 136-163: The message template editor currently binds the
subject/body fields directly to draft state, so Save can persist empty or
invalid content. Update the form in messages-template-editor.tsx to use React
Hook Form with Zod validation for the subject and body fields, and wire
validation into the save flow so Save template is blocked until both fields
satisfy the required/length rules. Keep the existing field UI, but replace the
direct draft-only handling around the Input/Textarea and the save path with
validated form state in the relevant editor component.
In `@apps/crew/src/server/crew-confirmation.server.ts`:
- Around line 1180-1183: The preview logging in crew-confirmation.server.ts
exposes PII by printing recipient email and message subject in the queue-missing
branch. Update the existing console.log in the crew confirmation flow to remove
or redact those fields, keeping only non-sensitive identifiers if needed; use
the surrounding queue check and message properties in the same block to locate
it.
- Around line 1202-1212: The crew confirmation flow in finalize-and-send logic
is enqueueing before the database row is safely finalized, which can leave the
confirmation eligible for retry if finalization fails after queueing. Update the
sequence around queue.send(message) and finalizeCrewAssignmentEmailQueued so the
state transition and send are atomic: either finalize the queued state before
enqueuing, or move the send into an outbox/consumer flow tied to the existing
operation, db, tokenHash, and queuedAt handling. Use the existing
finalizeCrewAssignmentEmailQueued path and the surrounding queue.send(message)
flow to keep sentAt/lastReminderAt from remaining unset after a successful send.
In `@apps/crew/src/server/crew-messages.server.ts`:
- Around line 641-644: The CrewBroadcast preview log is exposing recipient PII
by printing volunteerEmail and user-authored subject values. Update the logging
in the preview loop inside the crew-messages.server.ts broadcast preparation
flow to use internal identifiers or redacted placeholders instead of recipient
emails and subjects, while keeping the broadcastId for traceability.
In `@apps/crew/test/lib/crew-message-recipients.test.ts`:
- Around line 37-254: The test file is missing the required single `// `@lat`:`
spec reference comments for each `it(...)` case, which will fail the test-file
guideline validation. Add exactly one `// `@lat`:` comment adjacent to every test
in the describe blocks for classifyCrewMessageRecipient,
filterCrewMessageCandidates, buildCrewAssignmentMessageRecipients, and
buildCrewBroadcastMessageRecipients, using the relevant spec identifier for each
case and keeping the comments next to the corresponding test names.
In `@apps/crew/test/lib/crew-message-templates.test.ts`:
- Around line 10-130: Add exactly one `@lat` spec comment next to each it(...) in
renderCrewMessageTemplate, getDefaultCrewMessageTemplate, and
getCrewMessageTemplateVariables, using the appropriate spec reference for that
test and not placing comments at the file top. Ensure every test block in
crew-message-templates.test.ts has a single nearby `@lat` annotation with no
duplicates so the lat check passes.
In `@packages/wodsmith-db/src/schema.ts`:
- Line 17: The new crew-message-templates export is missing the required `@lat`
tag, unlike the other crew-prefixed schema exports in schema.ts. Add the same
preceding // `@lat`: comment for the crew-message-templates schema and ensure the
source module referenced by export * from "./schemas/crew-message-templates"
includes the tag so the export site stays consistent with crew-billing-events,
crew-self-serve-presets, and crew-volunteer-intelligence.
---
Outside diff comments:
In `@packages/wodsmith-db/src/schemas/crew-message-templates.ts`:
- Around line 1-73: Add the missing `@lat` reference comment to
crewMessageTemplatesTable so this schema is linked to the appropriate
lat.md/crew.md section like the sibling crew schema files. Place the comment
near the top-level schema definition in crew-message-templates.ts and use the
same section-id convention already used by crew-billing-events and
crew-self-serve-presets so lat check can associate this table with the new
documentation.
---
Nitpick comments:
In `@apps/crew/src/lib/crew/message-recipients.ts`:
- Around line 107-130: The reminder progression in getReminderDue can jump from
0 directly to 2 when the shift is within the 24-hour window, and this behavior
is currently untested. Add a test around getReminderDue in message-recipients.ts
that covers reminderCount = 0 with hoursUntilShift inside the 24-hour threshold
and asserts the returned reminderCount is 2, so the intended escalation path is
locked in.
In `@apps/crew/src/routes/events/`$eventId/-components/messages-responses-tab.tsx:
- Around line 309-316: The list key in AssignmentResponseRow currently mixes in
the array index, which weakens key stability for no gain if volunteerName,
shiftName, and startsAt already uniquely identify each row. Update the rows.map
rendering in messages-responses-tab.tsx to use a stable unique key built only
from the row fields already present, and keep the AssignmentResponseRow props
unchanged.
In `@apps/crew/src/routes/events/`$eventId/messages.tsx:
- Around line 76-97: The tab UI in messages.tsx is only partially wired for
accessibility; update the MESSAGE_TABS button rendering and the associated panel
markup so each tab button in the tablist has a stable id plus aria-controls
pointing to its matching panel, and each panel has the corresponding id and
role="tabpanel" (with appropriate hidden/selected state tied to activeTab). Use
the existing activeTab, handleTabChange, and MESSAGE_TABS mapping to keep the
tab and panel linkage consistent.
- Around line 56-57: The timezone fallback logic is duplicated between Messages
and MessagesResponsesTab, which can drift over time. Extract the precedence into
a single resolveCrewMessagingTimezone helper or pass the already computed
timezone from Messages into MessagesResponsesTab, and update the
MessagesResponsesTab usage so it reuses that shared value instead of
recalculating dashboard.event.timezone ?? "America/Denver" independently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b6f6b37d-36e7-4a4a-b7ae-b6849520c7d6
📒 Files selected for processing (20)
apps/crew/src/db/schemas/crew-message-templates.tsapps/crew/src/lib/crew/message-recipients.tsapps/crew/src/lib/crew/message-templates.tsapps/crew/src/react-email/crew/templated-message.tsxapps/crew/src/routes/events/$eventId/-components/messages-compose-tab.tsxapps/crew/src/routes/events/$eventId/-components/messages-history-tab.tsxapps/crew/src/routes/events/$eventId/-components/messages-recipients-panel.tsxapps/crew/src/routes/events/$eventId/-components/messages-responses-tab.tsxapps/crew/src/routes/events/$eventId/-components/messages-template-editor.tsxapps/crew/src/routes/events/$eventId/messages.tsxapps/crew/src/server-fns/crew-message-fns.tsapps/crew/src/server/broadcast-queue-consumer.tsapps/crew/src/server/crew-confirmation.server.tsapps/crew/src/server/crew-messages.server.tsapps/crew/test/lib/crew-message-recipients.test.tsapps/crew/test/lib/crew-message-templates.test.tslat.md/crew.mdpackages/wodsmith-db/src/schema.tspackages/wodsmith-db/src/schemas/common.tspackages/wodsmith-db/src/schemas/crew-message-templates.ts
| @@ -0,0 +1 @@ | |||
| export * from "@repo/wodsmith-db/schemas/crew-message-templates" | |||
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Missing @lat code ref.
This .ts file has no // @lat: [[section-id]] tag, unlike the sibling message-templates.ts and templated-message.tsx files in this cohort. Since the PR description lists lat check as a validation step, this omission may fail that check.
📝 Suggested fix
+// `@lat`: [[crew#Volunteer Messaging Composer#Message templates]]
export * from "`@repo/wodsmith-db/schemas/crew-message-templates`"As per coding guidelines, "Tie source code to concepts using code refs: // @lat: [[section-id]] (JS/TS/Rust/Go/C) or # @lat: [[section-id]] (Python)".
📝 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
| export * from "@repo/wodsmith-db/schemas/crew-message-templates" | |
| // `@lat`: [[crew#Volunteer Messaging Composer#Message templates]] | |
| export * from "`@repo/wodsmith-db/schemas/crew-message-templates`" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/crew/src/db/schemas/crew-message-templates.ts` at line 1, The
crew-message-templates module is missing the required `@lat` code reference tag,
which may cause the lat check to fail. Update the crew-message-templates export
file to include a matching // `@lat`: [[section-id]] comment consistent with the
sibling message-templates.ts and templated-message.tsx files, keeping the export
intact.
Source: Coding guidelines
Comment on lines
+136
to
+163
| <div className="space-y-2"> | ||
| <Label htmlFor="message-subject">Subject</Label> | ||
| <Input | ||
| id="message-subject" | ||
| ref={subjectRef} | ||
| value={draft.subject} | ||
| onFocus={() => { | ||
| activeFieldRef.current = "subject" | ||
| }} | ||
| onChange={(e) => onDraftChange({ ...draft, subject: e.target.value })} | ||
| placeholder="Email subject line" | ||
| /> | ||
| </div> | ||
|
|
||
| <div className="space-y-2"> | ||
| <Label htmlFor="message-body">Body</Label> | ||
| <Textarea | ||
| id="message-body" | ||
| ref={bodyRef} | ||
| rows={8} | ||
| value={draft.body} | ||
| onFocus={() => { | ||
| activeFieldRef.current = "body" | ||
| }} | ||
| onChange={(e) => onDraftChange({ ...draft, body: e.target.value })} | ||
| placeholder="Write your message. Use variables to personalize it." | ||
| /> | ||
| </div> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Subject/body form has no validation before save.
The subject Input and body Textarea are bound directly to draft state with no schema/validation; Save template is only gated on isDirty, so an empty subject or body can be persisted. As per coding guidelines, **/*.tsx forms should use React Hook Form with Zod validation, which would also give you built-in required/length checks here.
Also applies to: 186-198
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/crew/src/routes/events/`$eventId/-components/messages-template-editor.tsx
around lines 136 - 163, The message template editor currently binds the
subject/body fields directly to draft state, so Save can persist empty or
invalid content. Update the form in messages-template-editor.tsx to use React
Hook Form with Zod validation for the subject and body fields, and wire
validation into the save flow so Save template is blocked until both fields
satisfy the required/length rules. Keep the existing field UI, but replace the
direct draft-only handling around the Input/Textarea and the save path with
validated form state in the relevant editor component.
Source: Coding guidelines
Comment on lines
+1202
to
+1212
| await queue.send(message) | ||
|
|
||
| const finalized = await finalizeCrewAssignmentEmailQueued({ | ||
| db, | ||
| operation, | ||
| tokenHash, | ||
| queuedAt: now, | ||
| }) | ||
| if (!finalized) { | ||
| throw new Error("Queued assignment email could not be finalized") | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect queue/finalize ordering and related recovery behavior.
rg -n -C6 'claimCrewAssignmentEmailToken|queue\.send\(message\)|finalizeCrewAssignmentEmailQueued|queueCrewAssignmentConfirmationEmails|queueCrewTemplatedAssignmentEmails' apps/crew/src/server/crew-confirmation.server.tsRepository: wodsmith/thewodapp
Length of output: 3719
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the surrounding flow and the finalize/claim helpers.
sed -n '1160,1765p' apps/crew/src/server/crew-confirmation.server.ts | cat -nRepository: wodsmith/thewodapp
Length of output: 23034
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the rest of finalizeCrewAssignmentEmailQueued and any state checks that gate requeueing.
sed -n '576,760p' apps/crew/src/server/crew-confirmation.server.ts | cat -n
printf '\n---\n'
rg -n -C4 'sentAt|lastReminderAt|reminderCount|status.*PENDING|alreadySent|notDue|tokenHash' apps/crew/src/server/crew-confirmation.server.tsRepository: wodsmith/thewodapp
Length of output: 25547
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the email plan builder to understand retry eligibility after a failed finalize.
rg -n -C8 'function buildCrewAssignmentConfirmationEmailPlan|buildCrewAssignmentConfirmationEmailPlan\(' apps/crew/src/server/crew-confirmation.server.tsRepository: wodsmith/thewodapp
Length of output: 2198
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the queue contract and whether send() is documented as idempotent/deduplicating.
rg -n -C4 'idempotencyKey|queue\.send\(|interface .*Queue|type .*Queue|send\(message' apps/crew src packagesRepository: wodsmith/thewodapp
Length of output: 50374
Avoid enqueueing before the row is finalized.
If queue.send(message) succeeds and finalizeCrewAssignmentEmailQueued then fails, the confirmation stays eligible for the next run: tokenHash is already advanced, but sentAt/lastReminderAt is still unset, so the same email can be queued again. An outbox-style write or moving the state transition into the consumer would make the send/state change atomic.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/crew/src/server/crew-confirmation.server.ts` around lines 1202 - 1212,
The crew confirmation flow in finalize-and-send logic is enqueueing before the
database row is safely finalized, which can leave the confirmation eligible for
retry if finalization fails after queueing. Update the sequence around
queue.send(message) and finalizeCrewAssignmentEmailQueued so the state
transition and send are atomic: either finalize the queued state before
enqueuing, or move the send into an outbox/consumer flow tied to the existing
operation, db, tokenHash, and queuedAt handling. Use the existing
finalizeCrewAssignmentEmailQueued path and the surrounding queue.send(message)
flow to keep sentAt/lastReminderAt from remaining unset after a successful send.
Comment on lines
+37
to
+254
| describe("classifyCrewMessageRecipient — confirmation mode", () => { | ||
| it("marks an unsent pending assignment with email as eligible", () => { | ||
| const result = classifyCrewMessageRecipient({ | ||
| candidate: candidate(), | ||
| mode: "assignment_confirmation", | ||
| now: NOW, | ||
| }) | ||
| expect(result).toEqual({ | ||
| eligible: true, | ||
| skipReason: null, | ||
| reminderCount: 0, | ||
| }) | ||
| }) | ||
|
|
||
| it("skips a candidate with no email", () => { | ||
| const result = classifyCrewMessageRecipient({ | ||
| candidate: candidate({ volunteerEmail: null }), | ||
| mode: "assignment_confirmation", | ||
| now: NOW, | ||
| }) | ||
| expect(result.eligible).toBe(false) | ||
| expect(result.skipReason).toBe("missing_email") | ||
| }) | ||
|
|
||
| it("skips an already-sent confirmation", () => { | ||
| const result = classifyCrewMessageRecipient({ | ||
| candidate: candidate({ sentAt: new Date(NOW.getTime() - HOUR) }), | ||
| mode: "assignment_confirmation", | ||
| now: NOW, | ||
| }) | ||
| expect(result.skipReason).toBe("already_sent") | ||
| }) | ||
|
|
||
| it("skips a responded (non-pending) assignment", () => { | ||
| const result = classifyCrewMessageRecipient({ | ||
| candidate: candidate({ | ||
| status: CREW_ASSIGNMENT_CONFIRMATION_STATUS.CONFIRMED, | ||
| }), | ||
| mode: "assignment_confirmation", | ||
| now: NOW, | ||
| }) | ||
| expect(result.skipReason).toBe("responded") | ||
| }) | ||
|
|
||
| it("skips a shift that has already started", () => { | ||
| const result = classifyCrewMessageRecipient({ | ||
| candidate: candidate({ startsAt: new Date(NOW.getTime() - HOUR) }), | ||
| mode: "assignment_confirmation", | ||
| now: NOW, | ||
| }) | ||
| expect(result.skipReason).toBe("past_shift") | ||
| }) | ||
| }) | ||
|
|
||
| describe("classifyCrewMessageRecipient — reminder mode", () => { | ||
| it("is not due when the initial confirmation was never sent", () => { | ||
| const result = classifyCrewMessageRecipient({ | ||
| candidate: candidate({ | ||
| sentAt: null, | ||
| startsAt: new Date(NOW.getTime() + 20 * HOUR), | ||
| }), | ||
| mode: "reminder", | ||
| now: NOW, | ||
| }) | ||
| expect(result.eligible).toBe(false) | ||
| expect(result.skipReason).toBe("not_due") | ||
| }) | ||
|
|
||
| it("is due at reminderCount 1 inside the 48-hour window", () => { | ||
| const result = classifyCrewMessageRecipient({ | ||
| candidate: candidate({ | ||
| sentAt: new Date(NOW.getTime() - 24 * HOUR), | ||
| reminderCount: 0, | ||
| startsAt: new Date(NOW.getTime() + 40 * HOUR), | ||
| }), | ||
| mode: "reminder", | ||
| now: NOW, | ||
| }) | ||
| expect(result.eligible).toBe(true) | ||
| expect(result.reminderCount).toBe(1) | ||
| }) | ||
|
|
||
| it("is due at reminderCount 2 inside the 24-hour window", () => { | ||
| const result = classifyCrewMessageRecipient({ | ||
| candidate: candidate({ | ||
| sentAt: new Date(NOW.getTime() - 24 * HOUR), | ||
| reminderCount: 1, | ||
| startsAt: new Date(NOW.getTime() + 12 * HOUR), | ||
| }), | ||
| mode: "reminder", | ||
| now: NOW, | ||
| }) | ||
| expect(result.eligible).toBe(true) | ||
| expect(result.reminderCount).toBe(2) | ||
| }) | ||
|
|
||
| it("is not due when the shift is too far out", () => { | ||
| const result = classifyCrewMessageRecipient({ | ||
| candidate: candidate({ | ||
| sentAt: new Date(NOW.getTime() - HOUR), | ||
| reminderCount: 0, | ||
| startsAt: new Date(NOW.getTime() + 96 * HOUR), | ||
| }), | ||
| mode: "reminder", | ||
| now: NOW, | ||
| }) | ||
| expect(result.skipReason).toBe("not_due") | ||
| }) | ||
| }) | ||
|
|
||
| describe("filterCrewMessageCandidates", () => { | ||
| const pool = [ | ||
| candidate({ assignmentId: "a1", roleType: "judge", shiftId: "s1", state: "pending", volunteerName: "Ada" }), | ||
| candidate({ assignmentId: "a2", roleType: "medical", shiftId: "s2", state: "sent", volunteerName: "Bo", volunteerEmail: "bo@example.com" }), | ||
| candidate({ assignmentId: "a3", roleType: "judge", shiftId: "s2", state: "confirmed", volunteerName: "Cy" }), | ||
| ] | ||
|
|
||
| it("filters by state", () => { | ||
| const result = filterCrewMessageCandidates(pool, { states: ["sent"] }) | ||
| expect(result.map((c) => c.assignmentId)).toEqual(["a2"]) | ||
| }) | ||
|
|
||
| it("filters by role", () => { | ||
| const result = filterCrewMessageCandidates(pool, { roles: ["judge"] }) | ||
| expect(result.map((c) => c.assignmentId)).toEqual(["a1", "a3"]) | ||
| }) | ||
|
|
||
| it("filters by shift", () => { | ||
| const result = filterCrewMessageCandidates(pool, { shiftIds: ["s2"] }) | ||
| expect(result.map((c) => c.assignmentId)).toEqual(["a2", "a3"]) | ||
| }) | ||
|
|
||
| it("filters by search across name and email", () => { | ||
| expect( | ||
| filterCrewMessageCandidates(pool, { search: "bo@example" }).map( | ||
| (c) => c.assignmentId, | ||
| ), | ||
| ).toEqual(["a2"]) | ||
| expect( | ||
| filterCrewMessageCandidates(pool, { search: "cy" }).map( | ||
| (c) => c.assignmentId, | ||
| ), | ||
| ).toEqual(["a3"]) | ||
| }) | ||
|
|
||
| it("returns everything when no filters are set", () => { | ||
| expect(filterCrewMessageCandidates(pool, {})).toHaveLength(3) | ||
| }) | ||
| }) | ||
|
|
||
| describe("buildCrewAssignmentMessageRecipients", () => { | ||
| it("hides already-sent rows by default and shows them when included", () => { | ||
| const candidates = [ | ||
| candidate({ confirmationId: "c1" }), | ||
| candidate({ | ||
| confirmationId: "c2", | ||
| assignmentId: "a2", | ||
| sentAt: new Date(NOW.getTime() - HOUR), | ||
| }), | ||
| ] | ||
|
|
||
| const hidden = buildCrewAssignmentMessageRecipients({ | ||
| candidates, | ||
| mode: "assignment_confirmation", | ||
| includeAlreadySent: false, | ||
| now: NOW, | ||
| }) | ||
| expect(hidden.recipients.map((r) => r.key)).toEqual(["c1"]) | ||
| expect(hidden.skipped.alreadySent).toBe(0) | ||
|
|
||
| const shown = buildCrewAssignmentMessageRecipients({ | ||
| candidates, | ||
| mode: "assignment_confirmation", | ||
| includeAlreadySent: true, | ||
| now: NOW, | ||
| }) | ||
| expect(shown.recipients.map((r) => r.key)).toEqual(["c1", "c2"]) | ||
| expect(shown.skipped.alreadySent).toBe(1) | ||
| const alreadySent = shown.recipients.find((r) => r.key === "c2") | ||
| expect(alreadySent?.eligible).toBe(false) | ||
| }) | ||
| }) | ||
|
|
||
| describe("buildCrewBroadcastMessageRecipients", () => { | ||
| it("dedupes volunteers by email and keeps the earliest shift", () => { | ||
| const candidates = [ | ||
| candidate({ | ||
| assignmentId: "a1", | ||
| confirmationId: "c1", | ||
| volunteerEmail: "ada@example.com", | ||
| shiftName: "Later shift", | ||
| startsAt: new Date(NOW.getTime() + 48 * HOUR), | ||
| }), | ||
| candidate({ | ||
| assignmentId: "a2", | ||
| confirmationId: "c2", | ||
| volunteerEmail: "ADA@example.com", | ||
| shiftName: "Earlier shift", | ||
| startsAt: new Date(NOW.getTime() + 12 * HOUR), | ||
| }), | ||
| ] | ||
|
|
||
| const result = buildCrewBroadcastMessageRecipients(candidates) | ||
| expect(result.recipients).toHaveLength(1) | ||
| expect(result.recipients[0].key).toBe("email:ada@example.com") | ||
| expect(result.recipients[0].eligible).toBe(true) | ||
| expect(result.recipients[0].shiftName).toBe("Earlier shift") | ||
| }) | ||
|
|
||
| it("marks volunteers with no email ineligible", () => { | ||
| const result = buildCrewBroadcastMessageRecipients([ | ||
| candidate({ volunteerEmail: null }), | ||
| ]) | ||
| expect(result.recipients[0].eligible).toBe(false) | ||
| expect(result.recipients[0].skipReason).toBe("missing_email") | ||
| expect(result.skipped.missingEmail).toBe(1) | ||
| }) | ||
| }) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
No @lat refs anywhere in this test file.
Per the test-file guideline, each test should reference its spec with exactly one // @lat: comment placed next to the relevant test. None of the it(...) blocks here have one. Since the PR description lists lat check as required validation, this file is likely to fail it.
📝 Example fix pattern (repeat per test group)
describe("classifyCrewMessageRecipient — confirmation mode", () => {
+ // `@lat`: [[crew#Volunteer Messaging Composer#Filtered recipient selection]]
it("marks an unsent pending assignment with email as eligible", () => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/crew/test/lib/crew-message-recipients.test.ts` around lines 37 - 254,
The test file is missing the required single `// `@lat`:` spec reference comments
for each `it(...)` case, which will fail the test-file guideline validation. Add
exactly one `// `@lat`:` comment adjacent to every test in the describe blocks for
classifyCrewMessageRecipient, filterCrewMessageCandidates,
buildCrewAssignmentMessageRecipients, and buildCrewBroadcastMessageRecipients,
using the relevant spec identifier for each case and keeping the comments next
to the corresponding test names.
Source: Coding guidelines
Comment on lines
+10
to
+130
| describe("renderCrewMessageTemplate", () => { | ||
| it("substitutes known variables in subject and body", () => { | ||
| const result = renderCrewMessageTemplate({ | ||
| subject: "{{eventName}}: confirm {{shiftName}}", | ||
| body: "Hi {{volunteerName}}, you are on {{shiftName}}.", | ||
| variables: { | ||
| eventName: "Summer Throwdown", | ||
| shiftName: "Lane judging", | ||
| volunteerName: "Ada", | ||
| }, | ||
| }) | ||
|
|
||
| expect(result.subject).toBe("Summer Throwdown: confirm Lane judging") | ||
| expect(result.paragraphs).toEqual([ | ||
| "Hi Ada, you are on Lane judging.", | ||
| ]) | ||
| }) | ||
|
|
||
| it("leaves unknown variables literal", () => { | ||
| const result = renderCrewMessageTemplate({ | ||
| subject: "Hello {{unknownVar}}", | ||
| body: "Body {{alsoUnknown}} text", | ||
| variables: { volunteerName: "Ada" }, | ||
| }) | ||
|
|
||
| expect(result.subject).toBe("Hello {{unknownVar}}") | ||
| expect(result.paragraphs).toEqual(["Body {{alsoUnknown}} text"]) | ||
| }) | ||
|
|
||
| it("leaves empty-valued variables literal so blanks don't collapse", () => { | ||
| const result = renderCrewMessageTemplate({ | ||
| subject: "s", | ||
| body: "Location: {{location}}", | ||
| variables: { location: "" }, | ||
| }) | ||
|
|
||
| expect(result.paragraphs).toEqual(["Location: {{location}}"]) | ||
| }) | ||
|
|
||
| it("splits the body into paragraphs on blank lines", () => { | ||
| const result = renderCrewMessageTemplate({ | ||
| subject: "s", | ||
| body: "First para.\n\nSecond para.\n\n\nThird para.", | ||
| variables: {}, | ||
| }) | ||
|
|
||
| expect(result.paragraphs).toEqual([ | ||
| "First para.", | ||
| "Second para.", | ||
| "Third para.", | ||
| ]) | ||
| }) | ||
|
|
||
| it("keeps single newlines inside a paragraph", () => { | ||
| const result = renderCrewMessageTemplate({ | ||
| subject: "s", | ||
| body: "Shift: A\nRole: B\n\nNext", | ||
| variables: {}, | ||
| }) | ||
|
|
||
| expect(result.paragraphs).toEqual(["Shift: A\nRole: B", "Next"]) | ||
| }) | ||
| }) | ||
|
|
||
| describe("getDefaultCrewMessageTemplate", () => { | ||
| it("returns default copy for every template type", () => { | ||
| for (const type of CREW_MESSAGE_TEMPLATE_TYPES) { | ||
| const template = getDefaultCrewMessageTemplate(type) | ||
| expect(template.subject.length).toBeGreaterThan(0) | ||
| expect(template.body.length).toBeGreaterThan(0) | ||
| } | ||
| }) | ||
|
|
||
| it("matches the assignment confirmation subject wording", () => { | ||
| const template = getDefaultCrewMessageTemplate( | ||
| CREW_MESSAGE_TEMPLATE_TYPE.ASSIGNMENT_CONFIRMATION, | ||
| ) | ||
| expect(template.subject).toBe("{{eventName}}: confirm {{shiftName}}") | ||
| expect(template.body).toContain( | ||
| "You are assigned to help with {{eventName}}", | ||
| ) | ||
| }) | ||
|
|
||
| it("uses the reminder wording for the 24-hour reminder", () => { | ||
| const template = getDefaultCrewMessageTemplate( | ||
| CREW_MESSAGE_TEMPLATE_TYPE.REMINDER_24_HOUR, | ||
| ) | ||
| expect(template.body).toContain("within 24 hours") | ||
| }) | ||
|
|
||
| it("returns a fresh copy that callers cannot mutate", () => { | ||
| const first = getDefaultCrewMessageTemplate( | ||
| CREW_MESSAGE_TEMPLATE_TYPE.CUSTOM_BROADCAST, | ||
| ) | ||
| first.subject = "mutated" | ||
| const second = getDefaultCrewMessageTemplate( | ||
| CREW_MESSAGE_TEMPLATE_TYPE.CUSTOM_BROADCAST, | ||
| ) | ||
| expect(second.subject).not.toBe("mutated") | ||
| }) | ||
| }) | ||
|
|
||
| describe("getCrewMessageTemplateVariables", () => { | ||
| it("exposes the full variable set for assignment confirmations", () => { | ||
| const keys = getCrewMessageTemplateVariables( | ||
| CREW_MESSAGE_TEMPLATE_TYPE.ASSIGNMENT_CONFIRMATION, | ||
| ).map((variable) => variable.key) | ||
| expect(keys).toContain("confirmUrl") | ||
| expect(keys).toContain("shiftName") | ||
| expect(keys).toContain("scheduleUrl") | ||
| }) | ||
|
|
||
| it("limits custom broadcast to volunteer/event/schedule variables", () => { | ||
| const keys = getCrewMessageTemplateVariables( | ||
| CREW_MESSAGE_TEMPLATE_TYPE.CUSTOM_BROADCAST, | ||
| ).map((variable) => variable.key) | ||
| expect(keys).toEqual(["volunteerName", "eventName", "scheduleUrl"]) | ||
| expect(keys).not.toContain("confirmUrl") | ||
| expect(keys).not.toContain("shiftName") | ||
| }) | ||
| }) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Test cases are missing required @lat spec references.
None of the it(...) blocks in this file carry a // @lat: comment. Per the coding guidelines for test files, each test must reference its spec with exactly one @lat: comment placed next to the test (not at the file top), with no duplicates. Given the PR explicitly lists lat check as a validation step, this will likely fail CI.
📝 Example fix pattern (repeat per test)
describe("renderCrewMessageTemplate", () => {
+ // `@lat`: [[crew#Volunteer Messaging Composer#Message templates]]
it("substitutes known variables in subject and body", () => {As per coding guidelines, "Each test in code should reference its spec with exactly one // @lat: or # @lat: comment placed next to the relevant test, not at the top of the file, with no duplicate refs".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/crew/test/lib/crew-message-templates.test.ts` around lines 10 - 130, Add
exactly one `@lat` spec comment next to each it(...) in renderCrewMessageTemplate,
getDefaultCrewMessageTemplate, and getCrewMessageTemplateVariables, using the
appropriate spec reference for that test and not placing comments at the file
top. Ensure every test block in crew-message-templates.test.ts has a single
nearby `@lat` annotation with no duplicates so the lat check passes.
Source: Coding guidelines
| export * from "./schemas/crew-event-settings" | ||
| // `@lat`: [[crew#Add Thin Crew Tables]] | ||
| export * from "./schemas/crew-imports" | ||
| export * from "./schemas/crew-message-templates" |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
New export lacks a @lat tag, inconsistent with sibling crew schema exports.
Every other crew-prefixed export in this file (crew-billing-events, crew-self-serve-presets, crew-volunteer-intelligence) has a preceding // @lat: comment; this one doesn't. Root cause is the missing tag in crew-message-templates.ts itself — flagging here as it's the same compliance gap surfaced at the export site.
📝 Suggested fix
+// `@lat`: [[crew#Volunteer Messaging Composer]]
export * from "./schemas/crew-message-templates"📝 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
| export * from "./schemas/crew-message-templates" | |
| // `@lat`: [[crew#Volunteer Messaging Composer]] | |
| export * from "./schemas/crew-message-templates" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/wodsmith-db/src/schema.ts` at line 17, The new
crew-message-templates export is missing the required `@lat` tag, unlike the other
crew-prefixed schema exports in schema.ts. Add the same preceding // `@lat`:
comment for the crew-message-templates schema and ensure the source module
referenced by export * from "./schemas/crew-message-templates" includes the tag
so the export site stays consistent with crew-billing-events,
crew-self-serve-presets, and crew-volunteer-intelligence.
Source: Coding guidelines
Rename crew_message_templates to competition_message_templates in the shared @repo/wodsmith-db schema so Start can adopt the same record, generalize the related identifiers (MESSAGE_TEMPLATE_TYPE, MessageTemplateType, msgtpl_ id prefix), and add a Start schema shim. Type the template render pipeline with the CrewTemplateVariableKey union end to end, and add schema-mapping and type-level tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 4135ce3bf2e2
Rename crew_message_templates to competition_message_templates in the shared @repo/wodsmith-db schema so Start can adopt the same record, generalize the related identifiers (MESSAGE_TEMPLATE_TYPE, MessageTemplateType, msgtpl_ id prefix), and add a Start schema shim. Type the template render pipeline with the CrewTemplateVariableKey union end to end, and add schema-mapping and type-level tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 4135ce3bf2e2
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/wodsmith-db/src/schemas/message-templates.ts (1)
1-73: 📐 Maintainability & Code Quality | 🟠 MajorMissing
@latconcept tag on the new schema file.Every other crew schema file in this package (
crew-billing-events.ts,crew-self-serve-presets.ts,crew-volunteer-intelligence.ts,crew-imports.ts) carries a//@lat: [[...]]tag tying the file to a documented concept. This new file introducingcompetitionMessageTemplatesTablehas none. A prior review on this same feature already identified this exact gap and traced the root cause to this schema file itself (then namedcrew-message-templates.ts), rather than theschema.tsexport line. The PR description also listslat checkas a required validation step, so this omission is likely to fail CI, not just a style nit.📝 Suggested fix
import type { InferSelectModel } from "drizzle-orm" import { relations } from "drizzle-orm" import { index, mysqlTable, text, uniqueIndex, varchar, } from "drizzle-orm/mysql-core" import { commonColumns, createMessageTemplateId } from "./common" import { competitionsTable } from "./competitions" +// `@lat`: [[crew#Volunteer Messaging Composer]] /** * Message template types an organizer can compose and send to competition * volunteers. The reminder + confirmation types seed their default copy from * app-defined defaults; `custom_broadcast` is a free-form message. */ export const MESSAGE_TEMPLATE_TYPE = {Confirm the correct section-id given this schema is now de-namespaced/shared with the Start app rather than crew-specific.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/wodsmith-db/src/schemas/message-templates.ts` around lines 1 - 73, The new schema file defining competitionMessageTemplatesTable is missing the required `@lat` concept tag used by the other schema files in this package. Add the appropriate // `@lat`: [[...]] annotation near the top of this file, and make sure the section-id matches the shared Start/Crew concept rather than the old crew-only name. Keep the tag aligned with the competitionMessageTemplatesTable schema so lat check passes.Source: Coding guidelines
packages/wodsmith-db/src/schema.ts (1)
17-17: 📐 Maintainability & Code Quality | 🟠 MajorStill missing
@lattag before the export, as previously flagged.This re-export lacks the
//@lat:comment that precedes every other crew-related schema export in this file (crew-billing-events,crew-imports,crew-self-serve-presets,crew-volunteer-intelligence). A previous review already surfaced this exact gap on the prior file name (crew-message-templates); it persists here under the renamed path (message-templates). Root-cause fix belongs in the schema file itself (see comment onpackages/wodsmith-db/src/schemas/message-templates.ts), but this export site remains inconsistent with its neighbors either way.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/wodsmith-db/src/schema.ts` at line 17, Add the missing `@lat` comment before the re-export in schema.ts so it matches the other crew-related schema exports in this module. Keep the export of message-templates, but precede it with the same // `@lat`: annotation pattern used for crew-billing-events, crew-imports, crew-self-serve-presets, and crew-volunteer-intelligence to keep the schema re-exports consistent.Source: Coding guidelines
🧹 Nitpick comments (2)
packages/wodsmith-db/src/schemas/message-templates.ts (1)
49-55: 🚀 Performance & Scalability | 🔵 TrivialRedundant single-column index alongside the composite unique index.
competition_message_templates_competition_idxoncompetitionIdalone is redundant: the composite unique index(competitionId, templateType)already serves as a usable index for lookups filtered oncompetitionIdalone (leftmost-prefix usage), which is standard InnoDB/MySQL behavior. The extra index adds write overhead and storage without a query benefit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/wodsmith-db/src/schemas/message-templates.ts` around lines 49 - 55, The `messageTemplates` index definition is creating a redundant single-column index on `competitionId` alongside the composite unique index. In the schema builder for `competition_message_templates_competition_type_unique_idx`, remove the separate `competition_message_templates_competition_idx` entry from the table index list and keep only the composite unique index on `competitionId` and `templateType`.apps/crew/src/server/crew-messages.server.ts (1)
657-696: 🩺 Stability & Availability | 🔵 Trivial | 🏗️ Heavy liftSequential per-recipient render+send loop can block the request thread on large broadcasts.
Each recipient's email is rendered and queued one at a time, awaited in series. For broadcasts with many eligible recipients, this will serialize potentially slow
render()andqueue.send()calls on the request path, increasing latency/timeout risk with no concurrency control.Consider batching with a bounded concurrency (e.g.,
Promise.allSettledover chunks) or moving personalization/queueing itself into the queue consumer to keep this handler fast.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/crew/src/server/crew-messages.server.ts` around lines 657 - 696, The per-recipient render-and-queue flow in the broadcast handler is fully sequential, so large broadcasts can block the request path. Update the loop in the crew broadcast sending logic to process recipients with bounded concurrency instead of awaiting each render() and queue.send() one by one, keeping the same per-recipient error handling and failed-status update behavior. Use the existing prepared entries handling in crew-messages.server.ts as the place to introduce chunked Promise.allSettled or move the personalization/queueing work off the request thread.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@packages/wodsmith-db/src/schema.ts`:
- Line 17: Add the missing `@lat` comment before the re-export in schema.ts so it
matches the other crew-related schema exports in this module. Keep the export of
message-templates, but precede it with the same // `@lat`: annotation pattern used
for crew-billing-events, crew-imports, crew-self-serve-presets, and
crew-volunteer-intelligence to keep the schema re-exports consistent.
In `@packages/wodsmith-db/src/schemas/message-templates.ts`:
- Around line 1-73: The new schema file defining
competitionMessageTemplatesTable is missing the required `@lat` concept tag used
by the other schema files in this package. Add the appropriate // `@lat`: [[...]]
annotation near the top of this file, and make sure the section-id matches the
shared Start/Crew concept rather than the old crew-only name. Keep the tag
aligned with the competitionMessageTemplatesTable schema so lat check passes.
---
Nitpick comments:
In `@apps/crew/src/server/crew-messages.server.ts`:
- Around line 657-696: The per-recipient render-and-queue flow in the broadcast
handler is fully sequential, so large broadcasts can block the request path.
Update the loop in the crew broadcast sending logic to process recipients with
bounded concurrency instead of awaiting each render() and queue.send() one by
one, keeping the same per-recipient error handling and failed-status update
behavior. Use the existing prepared entries handling in crew-messages.server.ts
as the place to introduce chunked Promise.allSettled or move the
personalization/queueing work off the request thread.
In `@packages/wodsmith-db/src/schemas/message-templates.ts`:
- Around line 49-55: The `messageTemplates` index definition is creating a
redundant single-column index on `competitionId` alongside the composite unique
index. In the schema builder for
`competition_message_templates_competition_type_unique_idx`, remove the separate
`competition_message_templates_competition_idx` entry from the table index list
and keep only the composite unique index on `competitionId` and `templateType`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7e94e152-ba20-4468-bcac-6c7463eac224
📒 Files selected for processing (13)
apps/crew/src/db/schemas/message-templates.tsapps/crew/src/lib/crew/message-templates.tsapps/crew/src/routes/events/$eventId/-components/messages-compose-tab.tsxapps/crew/src/routes/events/$eventId/-components/messages-template-editor.tsxapps/crew/src/server-fns/crew-message-fns.tsapps/crew/src/server/crew-messages.server.tsapps/crew/test/db/message-templates-schema.test.tsapps/crew/test/lib/crew-message-templates.test.tsapps/wodsmith-start/src/db/schemas/message-templates.tslat.md/crew.mdpackages/wodsmith-db/src/schema.tspackages/wodsmith-db/src/schemas/common.tspackages/wodsmith-db/src/schemas/message-templates.ts
✅ Files skipped from review due to trivial changes (1)
- lat.md/crew.md
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/crew/src/routes/events/$eventId/-components/messages-template-editor.tsx
- apps/crew/src/routes/events/$eventId/-components/messages-compose-tab.tsx
Entire-Checkpoint: b884325ae492
Contributor
There was a problem hiding this comment.
1 issue found across 14 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/crew/src/routes/events/$eventId/-components/messages-compose-tab.tsx">
<violation number="1" location="apps/crew/src/routes/events/$eventId/-components/messages-compose-tab.tsx:161">
P3: The catch fallback message "Could not load recipients" duplicates the hardcoded prefix in the recipients panel. If a non-Error value (e.g. a string `throw`) reaches this catch, the user sees "Could not load recipients. Could not load recipients" rendered at runtime. Change the fallback to a distinct message so it reads naturally with the panel's prefix.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| setPreviewError( | ||
| error instanceof Error | ||
| ? error.message | ||
| : "Could not load recipients", |
Contributor
There was a problem hiding this comment.
P3: The catch fallback message "Could not load recipients" duplicates the hardcoded prefix in the recipients panel. If a non-Error value (e.g. a string throw) reaches this catch, the user sees "Could not load recipients. Could not load recipients" rendered at runtime. Change the fallback to a distinct message so it reads naturally with the panel's prefix.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/crew/src/routes/events/$eventId/-components/messages-compose-tab.tsx, line 161:
<comment>The catch fallback message "Could not load recipients" duplicates the hardcoded prefix in the recipients panel. If a non-Error value (e.g. a string `throw`) reaches this catch, the user sees "Could not load recipients. Could not load recipients" rendered at runtime. Change the fallback to a distinct message so it reads naturally with the panel's prefix.</comment>
<file context>
@@ -142,13 +146,21 @@ export function MessagesComposeTab({
+ setPreviewError(
+ error instanceof Error
+ ? error.message
+ : "Could not load recipients",
+ )
+ }
</file context>
Suggested change
| : "Could not load recipients", | |
| : "Unknown error", |
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
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the crew volunteer broadcast workflow for composing, templating, sending, and reviewing event messages. The changes split the messages route into focused compose/history/responses/template components, add recipient resolution and template helpers, persist crew message templates, and wire broadcast delivery/confirmation handling through server helpers and React Email rendering.
Impact
Organizers can manage crew broadcast drafts and templates from the event messages surface, target volunteer recipients more explicitly, and review outbound message history and responses. The DB package now exposes the crew message template schema used by the crew app.
Validation
pnpm --filter crew test -- test/lib/crew-message-recipients.test.ts test/lib/crew-message-templates.test.tspassed: 28 testspnpm --filter crew type-checkpassedpnpm check:schema-ownershippassedlat checkpassedNote: pnpm reported the existing Node engine warning because this shell is running Node 22 while the repo requests Node >=24.
Summary by cubic
Adds a volunteer broadcast composer with saved templates, recipient filtering, and delivery history so organizers can draft, preview, and send targeted crew emails from the Messages page. The page now has Compose, Responses, and History tabs, and templates are stored in the shared
competition_message_templatesschema in@repo/wodsmith-db.New Features
competition_message_templates; reset to defaults; live preview via@react-email/componentswith a unifiedCrewTemplatedMessageEmaillayout; shared for@repo/wodsmith-start.Bug Fixes
Written for commit cdddff1. Summary will update on new commits.
Summary by CodeRabbit