Skip to content

feat: add division cutoff rank with leaderboard line #384

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.

Sign up for GitHub

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

Open
zacjones93 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat: add division cutoff rank with leaderboard line #384

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
02d50e4
feat: add competition division cutoff rank with leaderboard line
zacjones93 Apr 11, 2026
ce1c49a
fix: remove aria role=separator from cutoff divs to pass lint
zacjones93 Apr 11, 2026
0634494
fix: remove aria-hidden from cutoff tr elements to pass lint
zacjones93 Apr 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 53 additions & 20 deletions apps/wodsmith-start/src/components/competition-leaderboard-table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ interface CompetitionLeaderboardTableProps {
}>
selectedEventId: string | null // null = overall view
scoringAlgorithm: ScoringAlgorithm
cutoffRank: number | null
}

function getRankIcon(rank: number) {
Expand Down Expand Up @@ -426,6 +427,7 @@ export function CompetitionLeaderboardTable({
events,
selectedEventId,
scoringAlgorithm,
cutoffRank,
}: CompetitionLeaderboardTableProps) {
// Compute the correct default sort column based on view mode
const defaultSortColumn = selectedEventId ? "eventRank" : "overallRank"
Expand Down Expand Up @@ -825,14 +827,26 @@ export function CompetitionLeaderboardTable({
</div>
) : (
<div>
{tableData.map((entry) => (
<MobileLeaderboardRow
key={entry.registrationId}
entry={entry}
events={events}
scoringAlgorithm={scoringAlgorithm}
/>
))}
{tableData.map((entry, idx) => {
const nextEntry = tableData[idx + 1]
const showCutoff =
cutoffRank != null &&
!selectedEventId &&
entry.overallRank <= cutoffRank &&

@cubic-dev-ai cubic-dev-ai Bot Apr 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Cutoff line placement is incorrect when the leaderboard is sorted by anything other than overall-rank ascending.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/components/competition-leaderboard-table.tsx, line 835:

<comment>Cutoff line placement is incorrect when the leaderboard is sorted by anything other than overall-rank ascending.</comment>

<file context>
@@ -825,14 +827,26 @@ export function CompetitionLeaderboardTable({
+              const showCutoff =
+                cutoffRank != null &&
+                !selectedEventId &&
+                entry.overallRank <= cutoffRank &&
+                (!nextEntry || nextEntry.overallRank > cutoffRank)
+              return (
</file context>
Fix with Cubic

(!nextEntry || nextEntry.overallRank > cutoffRank)
return (
<Fragment key={entry.registrationId}>
<MobileLeaderboardRow
entry={entry}
events={events}
scoringAlgorithm={scoringAlgorithm}
/>
{showCutoff && (
<div className="h-[3px] bg-orange-500" />
)}
</Fragment>
)
})}
</div>
)}
</div>
Expand Down Expand Up @@ -885,18 +899,37 @@ export function CompetitionLeaderboardTable({
</TableCell>
</TableRow>
) : (
table.getRowModel().rows.map((row) => (
<TableRow key={row.id} className="table-row">
{row.getVisibleCells().map((cell) => (
<TableCell key={cell.id} className="table-cell">
{flexRender(
cell.column.columnDef.cell,
cell.getContext(),
)}
</TableCell>
))}
</TableRow>
))
table.getRowModel().rows.map((row, rowIdx) => {
const entry = row.original
const rows = table.getRowModel().rows
const nextRow = rows[rowIdx + 1]
const showCutoff =
cutoffRank != null &&
!selectedEventId &&
entry.overallRank <= cutoffRank &&
(!nextRow || nextRow.original.overallRank > cutoffRank)
return (
<Fragment key={row.id}>
<TableRow className="table-row">
{row.getVisibleCells().map((cell) => (
<TableCell key={cell.id} className="table-cell">
{flexRender(
cell.column.columnDef.cell,
cell.getContext(),
)}
</TableCell>
))}
</TableRow>
{showCutoff && (
<tr>
<td colSpan={columns.length} className="p-0">
<div className="h-[3px] bg-orange-500" />
</td>
</tr>
)}
Comment on lines +902 to +929

@coderabbitai coderabbitai Bot Apr 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Hide or recompute the cutoff when the table is re-sorted.

This boundary check runs against the current sorted row model, so as soon as the user sorts overall view by athlete/event or flips rank descending, the orange line can land in the middle of the table or disappear entirely. The cutoff marker should only render in ascending overallRank order, or be computed from a separately rank-sorted view.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/wodsmith-start/src/components/competition-leaderboard-table.tsx` around
lines 902 - 929, The cutoff marker logic using table.getRowModel().rows and
showCutoff (checking cutoffRank, selectedEventId, entry.overallRank, and
nextRow.original.overallRank) can display incorrectly after user resorting;
update the render condition so the cutoff only appears when the table is
currently sorted by overallRank in ascending order (or recompute cutoff
positions from a dedicated rank-sorted row model). Concretely, detect the active
sort state via the table instance (e.g., table.getState().sorting) and only
evaluate the existing showCutoff condition when the first sort column is
"overallRank" and direction is "asc"; alternatively build a separate rows array
sorted by overallRank and use that to compute the cutoff instead of
table.getRowModel().rows, ensuring the orange divider is stable when sorting
changes.

</Fragment>
)
})
)}
</TableBody>
</Table>
Expand Down
53 changes: 53 additions & 0 deletions apps/wodsmith-start/src/components/divisions/organizer-division-item.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,16 @@ export interface OrganizerDivisionItemProps {
description: string | null
maxSpots: number | null
defaultMaxSpots: number | null
cutoffRank: number | null
defaultCutoffRank: number | null
index: number
registrationCount: number
isOnly: boolean
instanceId: symbol
onLabelSave: (value: string) => void
onDescriptionSave: (value: string | null) => void
onMaxSpotsSave: (value: number | null) => void
onCutoffRankSave: (value: number | null) => void
onRemove: () => void
onDrop: (sourceIndex: number, targetIndex: number) => void
}
Expand All @@ -55,13 +58,16 @@ export function OrganizerDivisionItem({
description,
maxSpots,
defaultMaxSpots,
cutoffRank,
defaultCutoffRank,
index,
registrationCount,
isOnly,
instanceId,
onLabelSave,
onDescriptionSave,
onMaxSpotsSave,
onCutoffRankSave,
onRemove,
onDrop,
}: OrganizerDivisionItemProps) {
Expand All @@ -74,6 +80,7 @@ export function OrganizerDivisionItem({
const labelRef = useRef(label)
const [localDescription, setLocalDescription] = useState(description ?? "")
const [localMaxSpots, setLocalMaxSpots] = useState(maxSpots?.toString() ?? "")
const [localCutoffRank, setLocalCutoffRank] = useState(cutoffRank?.toString() ?? "")
const [isExpanded, setIsExpanded] = useState(false)

// Sync local state when prop changes (e.g., after server update)
Expand All @@ -90,6 +97,10 @@ export function OrganizerDivisionItem({
setLocalMaxSpots(maxSpots?.toString() ?? "")
}, [maxSpots])

useEffect(() => {
setLocalCutoffRank(cutoffRank?.toString() ?? "")
}, [cutoffRank])

const canDelete = registrationCount === 0 && !isOnly

useEffect(() => {
Expand Down Expand Up @@ -318,6 +329,48 @@ export function OrganizerDivisionItem({
: "Leave blank for unlimited"}
</span>
</div>
<div className="flex items-center gap-3">
<label
htmlFor={`cutoffRank-${id}`}
className="text-sm text-muted-foreground whitespace-nowrap"
>
Cutoff rank:
</label>
<Input
id={`cutoffRank-${id}`}
type="number"
min={1}
value={localCutoffRank}
onChange={(e) => setLocalCutoffRank(e.target.value)}
onBlur={() => {
const newVal =
localCutoffRank.trim() === ""
? null
: parseInt(localCutoffRank, 10)
if (newVal !== cutoffRank) {
if (
newVal !== null &&
(Number.isNaN(newVal) || newVal < 1)
) {
setLocalCutoffRank(cutoffRank?.toString() ?? "")
return
}
onCutoffRankSave(newVal)
}
Comment on lines +345 to +359

@coderabbitai coderabbitai Bot Apr 11, 2026

Copy link
Copy Markdown
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:

#!/bin/bash
node - <<'NODE'
for (const raw of ["1.5", "2.9", "1e2"]) {
  console.log({
    raw,
    parsedInt: parseInt(raw, 10),
    numeric: Number(raw),
    isInteger: Number.isInteger(Number(raw)),
  })
}
NODE

Repository: wodsmith/thewodapp

Length of output: 243

🏁 Script executed:

cat -n apps/wodsmith-start/src/components/divisions/organizer-division-item.tsx | sed -n '330,370p'

Repository: wodsmith/thewodapp

Length of output: 1949

🏁 Script executed:

rg -B 20 "localCutoffRank.trim" apps/wodsmith-start/src/components/divisions/organizer-division-item.tsx

Repository: wodsmith/thewodapp

Length of output: 890

🏁 Script executed:

rg "cutoffRank" apps/wodsmith-start/src/components/divisions/organizer-division-item.tsx -A 2 -B 2

Repository: wodsmith/thewodapp

Length of output: 1533

This blur handler silently truncates decimal input from the number field.

The <Input type="number"> field allows users to type decimal values (e.g., 1.5), but the blur handler uses parseInt(), which truncates to an integer. If a user types 1.5 and the current cutoffRank is 1, the parsed value equals the current value, so nothing is saved—but the UI still shows 1.5, making it appear the input was ignored. When cutoffRank differs, the truncated value gets saved silently. Use Number() and validate with Number.isInteger() instead:

const newVal =
  localCutoffRank.trim() === ""
    ? null
    : Number(localCutoffRank)
if (newVal !== cutoffRank) {
  if (
    newVal !== null &&
    (Number.isNaN(newVal) || !Number.isInteger(newVal) || newVal < 1)
  ) {
    setLocalCutoffRank(cutoffRank?.toString() ?? "")
    return
  }
  onCutoffRankSave(newVal)
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/wodsmith-start/src/components/divisions/organizer-division-item.tsx`
around lines 345 - 359, The blur handler for the number input truncates decimals
by using parseInt, causing mismatches between the displayed value and saved
value; replace parseInt(localCutoffRank, 10) with Number(localCutoffRank) and
validate using Number.isInteger so decimals are rejected (or reset) rather than
silently truncated; update the logic in the onBlur block that computes newVal
and the subsequent validation checks around localCutoffRank, cutoffRank,
onCutoffRankSave, and setLocalCutoffRank to use Number(...) +
Number.isInteger(...) and the same newVal < 1 and NaN checks before calling
onCutoffRankSave.

}}
placeholder={
defaultCutoffRank
? `${defaultCutoffRank} (default)`
: "No cutoff"
}
className="w-32 text-sm"
/>
<span className="text-xs text-muted-foreground">
{defaultCutoffRank
? "Leave blank to use competition default"
: "Leave blank for no cutoff line"}
</span>
</div>
<Textarea
value={localDescription}
onChange={(e) => setLocalDescription(e.target.value)}
Expand Down
46 changes: 46 additions & 0 deletions apps/wodsmith-start/src/components/divisions/organizer-division-manager.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
switchCompetitionScalingGroupFn,
updateCompetitionDivisionFn,
updateDivisionCapacityFn,
updateDivisionCutoffFn,
updateDivisionDescriptionFn,
} from "@/server-fns/competition-divisions-fns"
import { OrganizerDivisionItem } from "./organizer-division-item"
Expand All @@ -51,6 +52,7 @@ interface Division {
description: string | null
feeCents: number | null
maxSpots: number | null
cutoffRank: number | null
}

interface ScalingGroupWithLevels {
Expand All @@ -74,6 +76,7 @@ interface OrganizerDivisionManagerProps {
scalingGroupTitle: string | null
scalingGroups: ScalingGroupWithLevels[]
defaultMaxSpotsPerDivision: number | null
defaultCutoffRank: number | null
}

export function OrganizerDivisionManager({
Expand All @@ -84,6 +87,7 @@ export function OrganizerDivisionManager({
scalingGroupTitle,
scalingGroups,
defaultMaxSpotsPerDivision,
defaultCutoffRank,
}: OrganizerDivisionManagerProps) {
const router = useRouter()
const [divisions, setDivisions] = useState(initialDivisions)
Expand Down Expand Up @@ -255,6 +259,43 @@ export function OrganizerDivisionManager({
}
}

const handleCutoffRankSave = async (
divisionId: string,
newCutoffRank: number | null,
) => {
const original = initialDivisions.find((d) => d.id === divisionId)
if (original && original.cutoffRank === newCutoffRank) return

setDivisions((prev) =>
prev.map((d) =>
d.id === divisionId ? { ...d, cutoffRank: newCutoffRank } : d,
),
)

try {
await updateDivisionCutoffFn({
data: {
teamId,
competitionId,
divisionId,
cutoffRank: newCutoffRank,
},
})
toast.success("Division cutoff updated")
} catch (error) {
toast.error(
error instanceof Error ? error.message : "Failed to update cutoff",
)
setDivisions((prev) =>
prev.map((d) =>
d.id === divisionId
? { ...d, cutoffRank: original?.cutoffRank ?? newCutoffRank }

@cubic-dev-ai cubic-dev-ai Bot Apr 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Rollback uses ?? with a nullable original value, so failed saves do not revert when the previous cutoff was null.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/components/divisions/organizer-division-manager.tsx, line 292:

<comment>Rollback uses `??` with a nullable original value, so failed saves do not revert when the previous cutoff was `null`.</comment>

<file context>
@@ -255,6 +259,43 @@ export function OrganizerDivisionManager({
+      setDivisions((prev) =>
+        prev.map((d) =>
+          d.id === divisionId
+            ? { ...d, cutoffRank: original?.cutoffRank ?? newCutoffRank }
+            : d,
+        ),
</file context>
Suggested change
? { ...d, cutoffRank: original?.cutoffRank ?? newCutoffRank }
? { ...d, cutoffRank: original ? original.cutoffRank : newCutoffRank }
Fix with Cubic

: d,
),
)
}
Comment on lines +262 to +296

@coderabbitai coderabbitai Bot Apr 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Rollback uses a stale/null-hostile source value.

This handler captures original from initialDivisions, then reverts with original?.cutoffRank ?? newCutoffRank. If the prior value was null, the catch path keeps the optimistic value instead of rolling back, and after one successful save initialDivisions is also stale for subsequent failures. Capture the pre-edit value from current state before mutating and restore that exact value on error.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/wodsmith-start/src/components/divisions/organizer-division-manager.tsx`
around lines 262 - 296, The rollback uses a stale/null-hostile value from
initialDivisions; modify handleCutoffRankSave to read and store the pre-edit
cutoff from the live divisions state (use the current divisions lookup before
calling setDivisions) into a local variable (e.g., previousCutoff) and use that
exact value in the catch rollback instead of original?.cutoffRank ??
newCutoffRank; ensure updateDivisionCutoffFn is still awaited and that after a
successful save you also update whatever source-of-truth (if any) you use for
initialDivisions so future errors can rollback correctly.

}

const handleRemove = async (divisionId: string) => {
try {
await deleteCompetitionDivisionFn({
Expand Down Expand Up @@ -421,6 +462,8 @@ export function OrganizerDivisionManager({
description={division.description}
maxSpots={division.maxSpots}
defaultMaxSpots={defaultMaxSpotsPerDivision}
cutoffRank={division.cutoffRank}
defaultCutoffRank={defaultCutoffRank}
index={index}
registrationCount={division.registrationCount}
isOnly={divisions.length === 1}
Expand All @@ -432,6 +475,9 @@ export function OrganizerDivisionManager({
onMaxSpotsSave={(spots) =>
handleMaxSpotsSave(division.id, spots)
}
onCutoffRankSave={(rank) =>
handleCutoffRankSave(division.id, rank)
}
onRemove={() => handleRemove(division.id)}
onDrop={handleDrop}
/>
Expand Down
5 changes: 5 additions & 0 deletions apps/wodsmith-start/src/components/leaderboard-page-content.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ export function LeaderboardPageContent({
const selectedEventId = searchParams.event ?? null
const selectedAffiliate = searchParams.affiliate ?? "all"

// Get cutoff rank for the selected division
const cutoffRank = divisions?.find((d) => d.id === selectedDivision)?.cutoffRank ?? null

const [leaderboard, setLeaderboard] = useState<CompetitionLeaderboardEntry[]>(
[],
)
Expand Down Expand Up @@ -569,13 +572,15 @@ export function LeaderboardPageContent({
events={events}
selectedEventId={effectiveEventId}
scoringAlgorithm={scoringAlgorithm}
cutoffRank={cutoffRank}
/>
) : (
<CompetitionLeaderboardTable
leaderboard={filteredLeaderboard}
events={events}
selectedEventId={effectiveEventId}
scoringAlgorithm={scoringAlgorithm}
cutoffRank={cutoffRank}
/>
)}
</div>
Expand Down
Loading
Loading