Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis pull request introduces score adjustments functionality across tournament competitions and registrations. The changes include backend mutations updated to accept optional scoreAdjustments arrays, new trigger handlers that refresh tournament results when registrations change, and database schema extensions. Frontend components are refactored with ScoreAdjustmentFormItem renamed to ScoreAdjustmentFields and given a new subjectDisplayName prop. PageWrapper gains contextMenu support, header layouts transition to CSS grid, and new tournament registration forms integrate score adjustment fields with organizer-only visibility. Utility functions are reorganized for score adjustment formatting, and context menus are integrated into tournament detail pages. 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/PageWrapper/PageWrapper.tsx (1)
63-75:⚠️ Potential issue | 🟡 Minor
contextMenuis silently dropped when the header guard is false.If a caller passes
contextMenualongsidehideTitle={true}(and noshowBackButton), the prop is silently ignored because the entire header<div>is gated byshowBackButton || (title && !hideTitle). At minimum,contextMenushould be included in the guard condition:🛡️ Proposed guard fix
- {(showBackButton || (title && !hideTitle)) && ( + {(showBackButton || (title && !hideTitle) || contextMenu) && (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/PageWrapper/PageWrapper.tsx` around lines 63 - 75, The header currently renders only when showBackButton || (title && !hideTitle), which drops contextMenu if hideTitle is true and showBackButton is false; update the render guard to also check for contextMenu so the header <div className={styles.PageWrapper_Header}> is shown when contextMenu is provided (e.g., showBackButton || (title && !hideTitle) || contextMenu), then keep the existing cloneElement(contextMenu, { className: clsx(contextMenu.props.className, styles.PageWrapper_ContextMenu) }) logic intact so contextMenu is not silently ignored; adjust any related variable names (contextMenu, hideTitle, showBackButton, PageWrapper_Header) accordingly.convex/_model/tournamentRegistrations/mutations/updateTournamentRegistration.ts (1)
14-37:⚠️ Potential issue | 🟠 MajorAny user with Edit permission can set
scoreAdjustments, not just organizers.The UI correctly gates score adjustment editing behind
isOrganizer, but the backend mutation applies{ ...updated }(which includesscoreAdjustments) for anyone who has theEditaction. PergetAvailableActions, captains and the registrant themselves also haveEditon published tournaments. A non-organizer could craft a direct API call to setscoreAdjustmentson their own registration.Consider adding a server-side check that strips or rejects
scoreAdjustmentswhen the caller is not an organizer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/_model/tournamentRegistrations/mutations/updateTournamentRegistration.ts` around lines 14 - 37, The mutation updateTournamentRegistration currently applies all fields from updated (from updateTournamentRegistrationArgs) including scoreAdjustments to anyone with TournamentRegistrationActionKey.Edit; add a server-side guard after computing availableActions to prevent non-organizers from changing scoreAdjustments: detect organizer status (e.g., call an isOrganizer helper or check availableActions for the organizer action key) and if the caller is not an organizer either strip scoreAdjustments from the updated payload or throw a ConvexError; update the logic in updateTournamentRegistration to only allow scoreAdjustments to be applied when the caller is confirmed organizer before calling ctx.db.patch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@convex/_model/tournamentCompetitors/triggers.ts`:
- Around line 17-31: The current early return on deleted competitors (if
(!newDoc) return) prevents refreshing tournamentResults on deletions; change the
logic to use tournamentId = newDoc?.tournamentId ?? oldDoc?.tournamentId so you
still resolve the tournament via getDocStrict(ctx, tournamentId) when newDoc is
null, and perform the refresh of tournamentResults even for deletions; keep the
scoreAdjustments equality check only for update cases (i.e., only skip when
newDoc exists and scoreAdjustments didn't change), and still preserve the
archived check (tournament.status === 'archived') to avoid making changes for
archived tournaments.
In `@convex/_model/tournamentResults/mutations/refreshTournamentResult.ts`:
- Around line 33-62: The current adjustedRegistrations and adjustedCompetitors
mappers perform repeated linear scans (using find/filter) over
tournamentRegistrations and tournamentCompetitors, producing O(n²) behavior; fix
this by precomputing lookup maps before the maps: build a regById map from
tournamentRegistrations (keyed by _id), a compById map from
tournamentCompetitors (keyed by _id), and a regsByCompetitorId map grouping
registrations by tournamentCompetitorId, then rewrite the logic in
adjustedRegistrations to use regById and compById lookups and in
adjustedCompetitors to use regsByCompetitorId and compById so all operations
become linear; keep using applyScoreAdjustments and preserve the same ordering
of applying registration then competitor adjustments.
In `@src/components/TournamentRegistrationForm/TournamentRegistrationForm.tsx`:
- Around line 119-127: The handler handleAddScoreAdjustment uses
[...tournament.rankingFactors].pop()! which can produce undefined for an empty
rankingFactors array; change it to safely derive the last ranking factor before
calling append: compute const last = tournament.rankingFactors &&
tournament.rankingFactors.length > 0 ?
tournament.rankingFactors[tournament.rankingFactors.length - 1] : <saneDefault>;
then call append({ amount: 1, round: tournament.currentRound ?? 0,
rankingFactor: last, reason: '' }); pick a saneDefault that matches the
rankingFactor type (e.g. 0 or 1) and ensure types reflect the fallback.
In
`@src/pages/TournamentCompetitorDetailPage/components/Header/Header.module.scss`:
- Around line 75-79: The media query block that re-declares --avatar-size for
.Header is redundant because .Header already sets --avatar-size: 8rem; remove
the entire `@media` (width >= 688px) { .Header { --avatar-size: 8rem; } } block
from Header.module.scss so there is no duplicate declaration; ensure .Header
still defines --avatar-size in its base rule and run the style build to confirm
no visual changes.
In `@src/pages/TournamentCompetitorDetailPage/TournamentCompetitorDetailPage.tsx`:
- Around line 98-108: When tournament.useTeams is false you currently pass
tournamentCompetitor.registrations[0] into TournamentRegistrationContextMenu
which can be undefined and crash useActions; update the conditional to check
tournamentCompetitor.registrations?.length > 0 (or registrations?.[0]) before
rendering TournamentRegistrationContextMenu and otherwise render null or a safe
fallback (e.g. no contextMenu or a disabled menu). Locate this in
TournamentCompetitorDetailPage where contextMenu is set and ensure
TournamentRegistrationContextMenu only receives a defined tournamentRegistration
prop.
---
Outside diff comments:
In
`@convex/_model/tournamentRegistrations/mutations/updateTournamentRegistration.ts`:
- Around line 14-37: The mutation updateTournamentRegistration currently applies
all fields from updated (from updateTournamentRegistrationArgs) including
scoreAdjustments to anyone with TournamentRegistrationActionKey.Edit; add a
server-side guard after computing availableActions to prevent non-organizers
from changing scoreAdjustments: detect organizer status (e.g., call an
isOrganizer helper or check availableActions for the organizer action key) and
if the caller is not an organizer either strip scoreAdjustments from the updated
payload or throw a ConvexError; update the logic in updateTournamentRegistration
to only allow scoreAdjustments to be applied when the caller is confirmed
organizer before calling ctx.db.patch.
In `@src/components/PageWrapper/PageWrapper.tsx`:
- Around line 63-75: The header currently renders only when showBackButton ||
(title && !hideTitle), which drops contextMenu if hideTitle is true and
showBackButton is false; update the render guard to also check for contextMenu
so the header <div className={styles.PageWrapper_Header}> is shown when
contextMenu is provided (e.g., showBackButton || (title && !hideTitle) ||
contextMenu), then keep the existing cloneElement(contextMenu, { className:
clsx(contextMenu.props.className, styles.PageWrapper_ContextMenu) }) logic
intact so contextMenu is not silently ignored; adjust any related variable names
(contextMenu, hideTitle, showBackButton, PageWrapper_Header) accordingly.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
convex/_generated/api.d.tsis excluded by!**/_generated/**
📒 Files selected for processing (27)
convex/_model/tournamentCompetitors/triggers.tsconvex/_model/tournamentRegistrations/index.tsconvex/_model/tournamentRegistrations/mutations/updateTournamentRegistration.tsconvex/_model/tournamentRegistrations/table.tsconvex/_model/tournamentRegistrations/triggers.tsconvex/_model/tournamentResults/mutations/refreshTournamentResult.tsconvex/functions.tsconvex/tournamentRegistrations.tssrc/components/PageWrapper/PageWrapper.module.scsssrc/components/PageWrapper/PageWrapper.tsxsrc/components/ScoreAdjustmentFields/ScoreAdjustmentFields.module.scsssrc/components/ScoreAdjustmentFields/ScoreAdjustmentFields.schema.tssrc/components/ScoreAdjustmentFields/ScoreAdjustmentFields.tsxsrc/components/ScoreAdjustmentFields/ScoreAdjustmentFields.types.tssrc/components/ScoreAdjustmentFields/ScoreAdjustmentFields.utils.tssrc/components/ScoreAdjustmentFields/index.tssrc/components/TournamentCompetitorForm/TournamentCompetitorForm.schema.tssrc/components/TournamentCompetitorForm/TournamentCompetitorForm.tsxsrc/components/TournamentCompetitorForm/components/ScoreAdjustmentFormItem/ScoreAdjustmentFormItem.utils.tssrc/components/TournamentCompetitorForm/components/ScoreAdjustmentFormItem/index.tssrc/components/TournamentRegistrationForm/TournamentRegistrationForm.module.scsssrc/components/TournamentRegistrationForm/TournamentRegistrationForm.schema.tssrc/components/TournamentRegistrationForm/TournamentRegistrationForm.tsxsrc/components/TournamentRegistrationProvider/TournamentRegistrationContextMenu.tsxsrc/pages/TournamentCompetitorDetailPage/TournamentCompetitorDetailPage.tsxsrc/pages/TournamentCompetitorDetailPage/components/Header/Header.module.scsssrc/pages/TournamentCompetitorDetailPage/components/Header/Header.tsx
💤 Files with no reviewable changes (2)
- src/components/TournamentCompetitorForm/components/ScoreAdjustmentFormItem/index.ts
- src/components/TournamentCompetitorForm/components/ScoreAdjustmentFormItem/ScoreAdjustmentFormItem.utils.ts
convex/_model/tournamentResults/mutations/refreshTournamentResult.ts
Outdated
Show resolved
Hide resolved
src/components/TournamentRegistrationForm/TournamentRegistrationForm.tsx
Show resolved
Hide resolved
src/pages/TournamentCompetitorDetailPage/components/Header/Header.module.scss
Outdated
Show resolved
Hide resolved
src/pages/TournamentCompetitorDetailPage/TournamentCompetitorDetailPage.tsx
Show resolved
Hide resolved
…der.module.scss Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
Resolves: #286
Summary by CodeRabbit
Release Notes
New Features
Chores