-
Notifications
You must be signed in to change notification settings - Fork 1
Implement notifications for wodsmith-start #190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Migrated the full notification system from the legacy WODsmith app: Email Templates (react-email): - Competition team invite (compete/team-invite.tsx) - Volunteer signup confirmation (compete/volunteer-signup-confirmation.tsx) - Volunteer approved (compete/volunteer-approved.tsx) - Organizer request submitted (organizer/request-submitted.tsx) Notification Functions (server/notifications): - notifyCompetitionTeamInvite - sends team invite emails - notifyVolunteerSignupReceived - confirms volunteer signup - notifyVolunteerApproved - notifies approved volunteers - Extracted helpers.ts for better testability - Updated index.ts with all 6 notification exports UI Components: - NotificationBell component for in-app notifications - Shows pending team invitations and profile alerts - Uses TanStack Router's Link component Toast notifications already integrated via sonner. All type-check, lint, and tests pass (1186 tests).
WalkthroughThe PR introduces a comprehensive notification system for WODsmith-start, including a client-side NotificationBell dropdown component, server-side notification functions for team competitions and volunteer operations, multiple React Email templates for invite and approval emails, reusable notification helpers, and Ralph automation scripts documenting a feature migration workflow. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Server as Server (compete.ts)
participant DB as Database
participant Email as Email Service
participant User as Recipient
App->>Server: notifyCompetitionTeamInvite(params)
Server->>DB: Fetch competition details
DB-->>Server: Competition data
Server->>DB: Fetch team & captain info
DB-->>Server: Team, captain, roster data
Server->>Server: Build invite link & format data
Server->>Email: Send email (CompetitionTeamInviteEmail)
Email-->>Server: Success
Server-->>App: Promise resolved
Email->>User: Team invite with join link
App->>Server: notifyVolunteerApproved(params)
Server->>DB: Fetch competition & team info
DB-->>Server: Competition, role types
Server->>Server: Format volunteer data
Server->>Email: Send email (VolunteerApprovedEmail)
Email-->>Server: Success
Server-->>App: Promise resolved
Email->>User: Volunteer approval confirmation
sequenceDiagram
participant Browser as Browser
participant UI as NotificationBell
participant Server as Server
participant DB as Database
Browser->>UI: Component mounts
UI->>Server: Fetch invitations & profile
Server->>DB: Query pending invitations
DB-->>Server: Invitation list
Server->>DB: Query missing profile fields
DB-->>Server: Missing fields
Server-->>UI: Invitations + missing fields
UI->>UI: Format missing fields text
UI->>UI: Determine notification count
UI->>Browser: Render bell with dropdown
Browser->>UI: User clicks dropdown
UI->>Browser: Display invitation items & completion prompt
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this 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 13 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="apps/wodsmith-start/src/react-email/compete/volunteer-signup-confirmation.tsx">
<violation number="1" location="apps/wodsmith-start/src/react-email/compete/volunteer-signup-confirmation.tsx:111">
P3: The `marginTop` style is overridden by the later `margin` declaration, so the intended top spacing never applies. Combine the margin values to avoid the override.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| border: "1px solid #f0f0f0", | ||
| borderRadius: "5px", | ||
| boxShadow: "0 5px 10px rgba(20,50,70,.2)", | ||
| marginTop: "20px", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3: The marginTop style is overridden by the later margin declaration, so the intended top spacing never applies. Combine the margin values to avoid the override.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/react-email/compete/volunteer-signup-confirmation.tsx, line 111:
<comment>The `marginTop` style is overridden by the later `margin` declaration, so the intended top spacing never applies. Combine the margin values to avoid the override.</comment>
<file context>
@@ -0,0 +1,202 @@
+ border: "1px solid #f0f0f0",
+ borderRadius: "5px",
+ boxShadow: "0 5px 10px rgba(20,50,70,.2)",
+ marginTop: "20px",
+ maxWidth: "600px",
+ margin: "0 auto",
</file context>
🚀 Preview DeployedURL: https://wodsmith-app-pr-190.zacjones93.workers.dev
This comment is automatically updated on each push to this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/wodsmith-start/src/server/notifications/index.ts (1)
1-6: Addimport "server-only"for this server module.This file is under
src/server/and should be marked server-only to prevent client bundling.Based on learnings, this is required for server-only files in this codebase.✅ Suggested fix
+import "server-only" + /** * Notifications Server Module for TanStack Start * Re-exports notification functions from compete module */apps/wodsmith-start/src/server/notifications/compete.ts (1)
9-35: Addimport "server-only"at the top of this server-only module.This file contains server-only dependencies (database access, logging, email utilities) that should not be bundled into client code. Add the import before other imports.
Fix
+import "server-only" import { eq } from "drizzle-orm" import { getDb } from "@/db"
🤖 Fix all issues with AI agents
In `@apps/wodsmith-start/src/components/nav/notification-bell.tsx`:
- Around line 63-71: The bell icon button in notification-bell.tsx (the <button>
that renders <Bell /> and uses hasNotifications) lacks an accessible name; add
an aria-label (e.g., aria-label="Notifications") or include visually-hidden text
inside the button so screen readers can announce it, and ensure the label
remains descriptive when hasNotifications changes (optionally include the count
or "new notifications" wording).
In `@apps/wodsmith-start/src/server/notifications/compete.ts`:
- Around line 64-121: The competition lookup using competitionsTable.findFirst
must also verify the team context to avoid cross-team access: update the query
that uses competitionId so its where clause also requires
competitionsTable.competitionTeamId (or the correct column name) to equal
competitionTeamId, and keep the existing not-found handling (logError, return)
unchanged; make the change where competitionsTable.findFirst is called and
ensure any variable names (competitionId, competitionTeamId, competitionsTable,
logError) are used consistently.
In `@apps/wodsmith-start/src/server/notifications/helpers.ts`:
- Around line 6-8: In helpers.ts add the directive import "server-only" as the
very first statement in the file (before the existing imports of
formatDateStringWithWeekday and getAppUrl) so the module is explicitly marked
server-only and enforces server-only boundaries.
In `@scripts/ralph/prompt.md`:
- Around line 37-45: Add a language identifier (e.g., text) to the three fenced
code blocks in the markdown: the block beginning with "## [Date] - [Story ID]",
the block beginning with "## Codebase Patterns", and the block containing
"<promise>COMPLETE</promise>" — replace the opening fences "```" with "```text"
for each to satisfy markdownlint MD040 and keep the rest of the block content
unchanged.
🧹 Nitpick comments (2)
apps/wodsmith-start/src/react-email/organizer/request-submitted.tsx (1)
21-27: Prefer a SITE_DOMAIN-derivedreviewLinkdefault.
Avoids hard-coding production URLs whenreviewLinkisn’t supplied.♻️ Suggested update
- reviewLink = "https://wodsmith.com/admin/organizer-requests", + reviewLink = `https://${SITE_DOMAIN}/admin/organizer-requests`,apps/wodsmith-start/src/server/notifications/helpers.ts (1)
97-102: Use a named object parameter (and interface) forisTeamComplete.This function has multiple params; guidelines require named object parameters and prefer interfaces. As per coding guidelines, consider:
♻️ Suggested refactor
+interface TeamCompleteParams { + currentRosterSize: number + maxRosterSize: number +} + -export function isTeamComplete( - currentRosterSize: number, - maxRosterSize: number, -): boolean { - return currentRosterSize >= maxRosterSize -} +export function isTeamComplete({ + currentRosterSize, + maxRosterSize, +}: TeamCompleteParams): boolean { + return currentRosterSize >= maxRosterSize +}
| <button | ||
| type="button" | ||
| className="relative font-bold text-foreground dark:text-dark-foreground" | ||
| > | ||
| <Bell className="h-5 w-5" /> | ||
| {hasNotifications && ( | ||
| <span className="absolute -top-1 -right-1 h-2.5 w-2.5 rounded-full bg-red-500" /> | ||
| )} | ||
| </button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add an accessible name to the bell button.
The icon-only button has no accessible label, so screen readers announce an unlabeled control. Add an aria-label or SR-only text.
♿ Suggested fix
- <button
+ <button
type="button"
className="relative font-bold text-foreground dark:text-dark-foreground"
+ aria-label="Notifications"
>
<Bell className="h-5 w-5" />
+ <span className="sr-only">Notifications</span>
{hasNotifications && (
<span className="absolute -top-1 -right-1 h-2.5 w-2.5 rounded-full bg-red-500" />
)}
</button>🤖 Prompt for AI Agents
In `@apps/wodsmith-start/src/components/nav/notification-bell.tsx` around lines 63
- 71, The bell icon button in notification-bell.tsx (the <button> that renders
<Bell /> and uses hasNotifications) lacks an accessible name; add an aria-label
(e.g., aria-label="Notifications") or include visually-hidden text inside the
button so screen readers can announce it, and ensure the label remains
descriptive when hasNotifications changes (optionally include the count or "new
notifications" wording).
| const competition = await db.query.competitionsTable.findFirst({ | ||
| where: eq(competitionsTable.id, competitionId), | ||
| }) | ||
|
|
||
| // Handle Date object (legacy) | ||
| const weekdays = [ | ||
| "Sunday", | ||
| "Monday", | ||
| "Tuesday", | ||
| "Wednesday", | ||
| "Thursday", | ||
| "Friday", | ||
| "Saturday", | ||
| ] | ||
| const months = [ | ||
| "January", | ||
| "February", | ||
| "March", | ||
| "April", | ||
| "May", | ||
| "June", | ||
| "July", | ||
| "August", | ||
| "September", | ||
| "October", | ||
| "November", | ||
| "December", | ||
| ] | ||
| const weekday = weekdays[date.getUTCDay()] | ||
| const month = months[date.getUTCMonth()] | ||
| const day = date.getUTCDate() | ||
| const year = date.getUTCFullYear() | ||
|
|
||
| return `${weekday}, ${month} ${day}, ${year}` | ||
| } | ||
| if (!competition) { | ||
| logError({ | ||
| message: "[Email] Cannot send team invite - no competition", | ||
| attributes: { competitionId, competitionTeamId }, | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| /** | ||
| * Derive athlete display name from user data | ||
| */ | ||
| function getAthleteName(user: { | ||
| firstName?: string | null | ||
| email?: string | null | ||
| }): string { | ||
| if (user.firstName) return user.firstName | ||
| if (user.email) return user.email.split("@")[0] || "Athlete" | ||
| return "Athlete" | ||
| } | ||
| // Fetch captain (inviter) | ||
| const captain = await db.query.userTable.findFirst({ | ||
| where: eq(userTable.id, invitedByUserId), | ||
| }) | ||
|
|
||
| /** | ||
| * Parse pending teammates count from JSON string | ||
| */ | ||
| function parsePendingTeammateCount( | ||
| pendingTeammatesJson: string | null | undefined, | ||
| ): number { | ||
| if (!pendingTeammatesJson) return 0 | ||
| const captainName = captain ? getAthleteName(captain) : "Team Captain" | ||
|
|
||
| try { | ||
| const parsed = JSON.parse(pendingTeammatesJson) as unknown | ||
| if (Array.isArray(parsed)) { | ||
| return parsed.length | ||
| // Fetch athlete team | ||
| const athleteTeam = await db.query.teamTable.findFirst({ | ||
| where: eq(teamTable.id, competitionTeamId), | ||
| }) | ||
|
|
||
| const teamName = athleteTeam?.name || "Team" | ||
|
|
||
| // Fetch registration to get division | ||
| const registration = await db.query.competitionRegistrationsTable.findFirst( | ||
| { | ||
| where: eq( | ||
| competitionRegistrationsTable.athleteTeamId, | ||
| competitionTeamId, | ||
| ), | ||
| }, | ||
| ) | ||
|
|
||
| let divisionName = "Open" | ||
| let currentRosterSize = 1 | ||
| let maxRosterSize = 3 | ||
|
|
||
| if (registration?.divisionId) { | ||
| const division = await db.query.scalingLevelsTable.findFirst({ | ||
| where: eq(scalingLevelsTable.id, registration.divisionId), | ||
| }) | ||
| if (division) { | ||
| divisionName = division.label | ||
| maxRosterSize = division.teamSize | ||
| } | ||
| } | ||
| return 0 | ||
| } catch { | ||
| return 0 | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Check if team is complete | ||
| */ | ||
| function isTeamComplete(currentSize: number, maxSize: number): boolean { | ||
| return currentSize >= maxSize | ||
| } | ||
| // Count current team members | ||
| if (competitionTeamId) { | ||
| const members = await db | ||
| .select() | ||
| .from(teamMembershipTable) | ||
| .where(eq(teamMembershipTable.teamId, competitionTeamId)) | ||
| currentRosterSize = members.length | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the full file to understand the function context and parameters
cat -n apps/wodsmith-start/src/server/notifications/compete.tsRepository: wodsmith/thewodapp
Length of output: 20898
🏁 Script executed:
# Let's also check the database schema to understand the relationship between tables and teams
fd -e ts -e tsx schema apps/wodsmith-start/src/db/Repository: wodsmith/thewodapp
Length of output: 98
🏁 Script executed:
# Search for teamId usage patterns in similar files to understand the convention
rg "teamId" apps/wodsmith-start/src/server --type ts --type tsx -B 2 -A 2 | head -100Repository: wodsmith/thewodapp
Length of output: 89
🏁 Script executed:
# Let's examine the database schema to understand multi-tenant structure and relationships
cat -n apps/wodsmith-start/src/db/schema.ts | head -200Repository: wodsmith/thewodapp
Length of output: 1332
🏁 Script executed:
# Search for competitionsTable schema definition specifically
rg "competitionsTable|competitionTeamId|athleteTeamId" apps/wodsmith-start/src/db/schema.ts -B 3 -A 3 | head -150Repository: wodsmith/thewodapp
Length of output: 44
🏁 Script executed:
# Check how these notification functions are called to understand the context
rg "notifyCompetitionTeamInvite|notifyVolunteerSignupReceived|notifyVolunteerApproved" apps/wodsmith-start/src --type ts -B 2 -A 2 | head -100Repository: wodsmith/thewodapp
Length of output: 2083
🏁 Script executed:
# Look at the competitions schema to understand the table structure
cat -n apps/wodsmith-start/src/db/schemas/competitions.tsRepository: wodsmith/thewodapp
Length of output: 18718
🏁 Script executed:
# Check the teams schema to understand team/teamId relationships
cat -n apps/wodsmith-start/src/db/schemas/teams.ts | head -150Repository: wodsmith/thewodapp
Length of output: 6076
🏁 Script executed:
# Check if competitionTeamId is a team or if it's a foreign key
rg "competitionTeamId" apps/wodsmith-start/src/db/schemas/ -B 2 -A 2Repository: wodsmith/thewodapp
Length of output: 1602
🏁 Script executed:
# Check the scalingLevelsTable schema to understand if divisionId is team-scoped
cat -n apps/wodsmith-start/src/db/schemas/scaling.tsRepository: wodsmith/thewodapp
Length of output: 5012
🏁 Script executed:
# Let's understand how these notification functions are called - look for the callers
rg "notifyCompetitionTeamInvite|notifyVolunteerSignupReceived|notifyVolunteerApproved" apps/wodsmith-start/src --type ts -B 3 -A 1 | grep -v "^--$" | head -150Repository: wodsmith/thewodapp
Length of output: 2257
🏁 Script executed:
# Check if there are examples in the codebase of properly team-scoped scalingLevels queries
rg "scalingLevelsTable" apps/wodsmith-start/src --type ts -B 3 -A 3 | head -200Repository: wodsmith/thewodapp
Length of output: 15164
🏁 Script executed:
# Look for how competitionsTable is typically queried with team context
rg "competitionsTable.findFirst|competitionsTable.find" apps/wodsmith-start/src --type ts -B 2 -A 2 | head -150Repository: wodsmith/thewodapp
Length of output: 11584
🏁 Script executed:
# Check where these notification functions are called from to see if teamId is available
rg "notifyCompetitionTeamInvite|notifyVolunteerSignupReceived|notifyVolunteerApproved" apps/wodsmith-start/src/server-fns --type ts -B 5 -A 2 | head -200Repository: wodsmith/thewodapp
Length of output: 44
🏁 Script executed:
# Let me check the exact lines 528-530 and 586-588 to verify they're already scoped correctly
sed -n '528,530p; 586,588p' apps/wodsmith-start/src/server/notifications/compete.tsRepository: wodsmith/thewodapp
Length of output: 345
🏁 Script executed:
# Also check the context around line 64-66 to understand competitionsTable query
sed -n '60,75p' apps/wodsmith-start/src/server/notifications/compete.tsRepository: wodsmith/thewodapp
Length of output: 413
🏁 Script executed:
# Check if there's a FK relationship enforcing that divisionId belongs to the competition
rg "competitionRegistrationsTable" apps/wodsmith-start/src/db/schemas/competitions.ts -B 5 -A 25 | head -60Repository: wodsmith/thewodapp
Length of output: 1964
Lines 528-530 and 586-588 are already correctly scoped by competitionTeamId. The review comment flags these unnecessarily—they already filter using the team context.
For lines 64-121: The initial competitionsTable.findFirst query at line 64 should validate that competitionId belongs to the competitionTeamId context (e.g., by joining on competitionsTable.competitionTeamId). This prevents accidental or malicious cross-team competition access. While competitionTeamId is correctly used downstream (athleteTeam fetch, registration queries), the competition lookup itself bypasses team validation.
🤖 Prompt for AI Agents
In `@apps/wodsmith-start/src/server/notifications/compete.ts` around lines 64 -
121, The competition lookup using competitionsTable.findFirst must also verify
the team context to avoid cross-team access: update the query that uses
competitionId so its where clause also requires
competitionsTable.competitionTeamId (or the correct column name) to equal
competitionTeamId, and keep the existing not-found handling (logError, return)
unchanged; make the change where competitionsTable.findFirst is called and
ensure any variable names (competitionId, competitionTeamId, competitionsTable,
logError) are used consistently.
| import { formatDateStringWithWeekday } from "@/utils/date-utils" | ||
| import { getAppUrl } from "@/lib/env" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add import "server-only" to enforce server-only module boundaries.
This file is under src/server/** and should be explicitly marked server-only.
Based on learnings, please add the directive at the top.
✅ Suggested fix
+import "server-only"
import { formatDateStringWithWeekday } from "@/utils/date-utils"
import { getAppUrl } from "@/lib/env"📝 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.
| import { formatDateStringWithWeekday } from "@/utils/date-utils" | |
| import { getAppUrl } from "@/lib/env" | |
| import "server-only" | |
| import { formatDateStringWithWeekday } from "@/utils/date-utils" | |
| import { getAppUrl } from "@/lib/env" | |
🤖 Prompt for AI Agents
In `@apps/wodsmith-start/src/server/notifications/helpers.ts` around lines 6 - 8,
In helpers.ts add the directive import "server-only" as the very first statement
in the file (before the existing imports of formatDateStringWithWeekday and
getAppUrl) so the module is explicitly marked server-only and enforces
server-only boundaries.
| ``` | ||
| ## [Date] - [Story ID] | ||
| - What was implemented | ||
| - Files changed | ||
| - **Learnings:** | ||
| - Patterns discovered | ||
| - Gotchas encountered | ||
| --- | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add language identifiers to fenced code blocks.
markdownlint flags MD040 here; please add a language (e.g., text) to the fenced blocks at Line 37, Line 49, and Line 67.
📝 Suggested update
-```
+```text
## [Date] - [Story ID]
- What was implemented
- Files changed
- **Learnings:**
- Patterns discovered
- Gotchas encountered
---@@
- +text
Codebase Patterns
- Pattern: description
- Gotcha: description
@@
-```
+```text
<promise>COMPLETE</promise>
</details>
Also applies to: 49-53, 67-69
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>
37-37: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In @scripts/ralph/prompt.md around lines 37 - 45, Add a language identifier
(e.g., text) to the three fenced code blocks in the markdown: the block
beginning with "## [Date] - [Story ID]", the block beginning with "## Codebase
Patterns", and the block containing "COMPLETE" — replace the
opening fences "" with "text" for each to satisfy markdownlint MD040 and
keep the rest of the block content unchanged.
</details>
<!-- fingerprinting:phantom:poseidon:eagle -->
<!-- This is an auto-generated comment by CodeRabbit -->
Migrated the full notification system from the legacy WODsmith app:
Email Templates (react-email):
Notification Functions (server/notifications):
UI Components:
Toast notifications already integrated via sonner.
All type-check, lint, and tests pass (1186 tests).
Summary by cubic
Implemented notifications in wodsmith-start, migrating key emails and in-app alerts from the legacy app. Users now get team invites and volunteer updates, with a notification bell showing pending actions.
New Features
Refactors
Written for commit bdd31d2. Summary will update on new commits.
Summary by CodeRabbit
New Features
✏️ Tip: You can customize this high-level summary in your review settings.