Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. WalkthroughAdds alignment/faction support across backend and frontend: new common types and Convex schemas, persisted registration details, visibility flags and toggle mutation, alignment-aware pairing changes, UI components (AlignmentGraph, FactionIndicator, DetailsFields), form/schema updates, style tweaks, and related hooks/actions. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 inconclusive)
✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 33
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/components/TournamentForm/TournamentForm.schema.ts (1)
88-91: Add validation preventing AdjacentAlignment pairing with team tournaments.Per PR requirements, the schema must validate that
pairingMethodcannot beTournamentPairingMethod.AdjacentAlignmentwhencompetitorSize > 1. This cross-field validation is currently missing from the schema's.refine()block.Proposed fix
}).refine(validateGameSystemConfig, { message: 'Invalid config for the selected game system.', path: ['gameSystemConfig'], // Highlight the game_system_config field in case of error +}).refine((data) => { + if (data.pairingMethod === TournamentPairingMethod.AdjacentAlignment && data.competitorSize > 1) { + return false; + } + return true; +}, { + message: 'Red vs. Blue pairing is not supported for team tournaments.', + path: ['pairingMethod'], });convex/_fixtures/createMockTournamentCompetitor.ts (1)
22-29: Thedetailsfield is placed after the spread and cannot be overridden.Like
activeRegistrationCountandavailableActions, thedetailsfield is placed after...overrides, which means tests cannot customize alignment/faction data through theoverridesparameter. If this is intentional for consistency with other fixed fields, this is fine. Otherwise, consider movingdetailsinto the default object before the spread, or adding it to the defaults with a fallback pattern.♻️ Optional: Allow details to be overridden
displayName: 'Test Tournament', tournamentId: 'T0' as Id<'tournaments'>, + details: { + alignments: [], + factions: [], + }, ...overrides, _id: overrides.id as Id<'tournamentCompetitors'>, activeRegistrationCount: 0, availableActions: [], - details: { - alignments: [], - factions: [], - }, });convex/_model/tournamentRegistrations/mutations/createTournamentRegistration.ts (1)
100-108: Validate alignment requirement when registering.When
tournament.registrationDetails?.alignment === 'required', the mutation should validate thatargs.details?.alignmentis provided before allowing registration. This follows the same validation pattern astournament.requireRealNames(lines 79-81).convex/_model/tournamentPairings/_helpers/generateDraftPairings.test.ts (1)
66-90: Add test coverage for alignment-based pairing behavior.The
generateDraftPairingsfunction implements alignment-based constraints (allowSameAlignmentoption, defaulting totrue), but there are no tests verifying this behavior whenallowSameAlignment: false. Add tests that:
- Verify competitors with different alignments are paired when
allowSameAlignment: false- Verify an error is thrown when no valid cross-alignment pairings exist
- Verify fallback behavior when alignment data is missing or incomplete
convex/_model/tournamentPairings/_helpers/generateDraftPairings.ts (1)
24-26: Outdated JSDoc parameter description.The
@param allowRepeatsdocumentation still references the old boolean parameter. Update it to reflect the newoptionsobject.📝 Proposed fix
* `@param` orderedCompetitors - Ordered array of competitors - * `@param` allowRepeats - Allow repeat match-ups - * `@returns` An array of + * `@param` options - Pairing constraints (allowRepeats, allowSameAlignment) + * `@returns` An array of competitor pairs
🤖 Fix all issues with AI agents
In `@convex/_model/common/faction.ts`:
- Around line 1-14: Both alignment.ts and faction.ts duplicate the same
enum-to-validator pattern; extract a helper (e.g., createCrossSystemUnion) that
accepts multiple enum objects and returns a de-duped list and a v.union
validator, then replace the logic in faction.ts (and alignment.ts) to call
createCrossSystemUnion(...enums) and use the returned validator for the exported
faction constant; ensure the helper preserves string literal types for the union
and that you reference the existing exported symbol faction (and alignment) to
swap its current values.map(...) v.union construction for the helper's
validator.
- Around line 7-10: The values array defined in faction.ts currently
concatenates ...Object.values(flamesOfWarV4) and ...Object.values(teamYankeeV2)
which can produce duplicate Faction entries; update the construction of values
to deduplicate entries (e.g., by using a Set or Map keyed on a unique Faction
identifier such as id or name) so that the final values array contains only
unique factions. Locate the values declaration and replace the simple spread
concat with a deduplication step that preserves one canonical Faction per unique
id/name.
In `@convex/_model/tournamentCompetitors/_helpers/getDisplayName.ts`:
- Around line 30-32: Remove the redundant optional chaining on
tournament?.competitorSize in getDisplayName (tournament is guaranteed non-null
by the earlier throw), i.e., use tournament.competitorSize while retaining the
defensive activeRegistrations[0]?.user check; update the conditional that
returns activeRegistrations[0].user.displayName accordingly so it reads against
tournament.competitorSize without the ?.
In `@convex/_model/tournamentPairings/_helpers/generateDraftPairings.ts`:
- Around line 80-88: The function checkIfRepeat can be simplified to a single
return statement: replace the current block in generateDraftPairings.ts (the
checkIfRepeat function that takes a: DeepTournamentCompetitor and b:
DeepTournamentCompetitor) with a single-line return that directly returns the
boolean result of a.opponentIds.includes(b._id), preserving types and parameter
names.
In
`@convex/_model/tournamentRegistrations/_helpers/deepenTournamentRegistration.ts`:
- Around line 35-36: The current construction using Array.from(new Set(...)) for
alignments and factions is unnecessary for single-element branches; replace
those expressions with simple conditional arrays to return either a one-item
array or an empty array (e.g. set alignments to alignmentsVisible &&
details?.alignment ? [details.alignment] : [] and similarly for factions) in
deepenTournamentRegistration so behavior remains identical until the TODO
introduces multi-item lists.
- Around line 30-32: The visibility logic in deepenTournament.ts is using the
nullish coalescing operator (??) incorrectly for alignmentsVisible and
factionsVisible; update those expressions to use logical OR (||) so that when
checkUserIsTournamentOrganizer (which returns a boolean) is false the code falls
back to tournament.alignmentsRevealed and tournament.factionsRevealed
respectively; locate the alignmentsVisible and factionsVisible properties in
deepenTournament.ts and replace the "isOrganizer ??
tournament.alignmentsRevealed" and "isOrganizer ?? tournament.factionsRevealed"
usages with "isOrganizer || tournament.alignmentsRevealed" and "isOrganizer ||
tournament.factionsRevealed" to match deepenTournamentRegistration.ts and ensure
consistent behavior.
In `@convex/_model/tournamentRegistrations/table.ts`:
- Around line 11-14: Extract the duplicated inline validator into a shared
exported validator named details that composes the existing alignment and
faction validators (i.e., export const details = v.optional(v.object({
alignment: v.optional(alignment), faction: v.optional(faction) }))), then
replace the inline details definitions in both table schemas with an import of
this shared details and update the imports in the files that used the inline
object to import the new details symbol (removing any now-unnecessary local
alignment/faction imports if they become redundant).
In `@convex/_model/tournaments/_helpers/deepenTournament.ts`:
- Around line 48-53: The visibility logic is using the nullish coalescing
operator so a false isOrganizer hides reveals; change the expressions that set
alignmentsVisible and factionsVisible to use logical OR instead of ?? so they
evaluate to true when either isOrganizer is true or the tournament flags are
true (update the assignments for alignmentsVisible and factionsVisible to use
isOrganizer || tournament.alignmentsRevealed and isOrganizer ||
tournament.factionsRevealed respectively).
In `@convex/_model/tournaments/table.ts`:
- Around line 65-67: The two fields alignmentsRevealed and factionsRevealed are
visibility flags and should be grouped with other visibility-related fields
(e.g., listsRevealed) rather than mixed with registration data; refactor the
table schema by creating a visibility sub-object (e.g., visibility:
v.type({...})) containing alignmentsRevealed, factionsRevealed and any
listsRevealed equivalent, update any code that reads/writes those fields to use
the new visibility object, and remove the standalone
alignmentsRevealed/factionsRevealed entries so all display-related flags live
under visibility while leaving registrationDetails unchanged.
In `@src/components/AlignmentGraph/AlignmentGraph.tsx`:
- Around line 39-41: The tournament registrations query result from
useGetTournamentRegistrationsByTournament is not handling loading or error
states; update the AlignmentGraph component to read and use the hook's status
flags (e.g., isLoading, isError, error) alongside data (tournamentRegistrations)
and render appropriate conditional UI: show a loading indicator/skeleton while
isLoading or data is undefined, show an error message or fallback UI when
isError (including error.message), and only render the normal graph logic once
tournamentRegistrations is available; reference
useGetTournamentRegistrationsByTournament and the tournamentRegistrations
variable when making these changes.
- Around line 84-86: Replace the hard-coded "Allies" and "Axis" headers in
AlignmentGraph.tsx with dynamic labels derived from tournament.gameSystem:
implement or use a small mapping (e.g., getSideLabels(gameSystem) or a
SIDE_LABELS constant) that returns the left and right strings for known game
systems (Flames of War -> "Allies"/"Axis", Team Yankee -> "NATO"/"Warsaw Pact",
etc.), then render those values where you currently use
styles.AlignmentGraph_AnchorLeft and styles.AlignmentGraph_AnchorRight; ensure a
sensible default fallback for unknown game systems.
- Around line 56-74: The component currently only builds sections for
GameSystem.FlamesOfWarV4 using hard-coded labels; update the section-building
logic in the AlignmentGraph component to handle GameSystem.TeamYankeeV2 (map
alignmentGroups[TeamYankeeV2.NATO] and [TeamYankeeV2.WarsawPact] with
appropriate theme colors) and otherwise fallback gracefully by deriving section
labels/values from the keys of alignmentGroups (excluding Unknown) so labels are
not hard-coded; modify the code around tournament.gameSystem, alignmentGroups
and sections to use a per-gameSystem mapping (e.g., a small switch or lookup for
FlamesOfWarV4 and TeamYankeeV2) and a default path that iterates alignmentGroups
to build sections dynamically.
In `@src/components/AlignmentIndicator/AlignmentIndicator.module.scss`:
- Around line 97-98: Remove the two commented-out outline rules in
AlignmentIndicator.module.scss (the lines containing "outline:
var(--border-width) solid var(--border-color-default);" and "outline-offset:
calc(-1 * var(--border-width));") — either delete them entirely or, if you
intend to keep them for a reason, restore them and add a brief explanatory
comment above the rules describing why they are preserved (e.g., browser/QA
workaround or accessibility toggle) so future readers understand their purpose.
- Line 52: AlignmentIndicator.module.scss contains a hard-coded background-color
"#eee"; replace that literal with the theme CSS variable (e.g. background-color:
var(--gray-3);) to respect the design system and dark mode—update the
background-color declaration in AlignmentIndicator.module.scss (where "#eee"
appears) to use the chosen CSS variable name consistent with the project tokens.
In `@src/components/AlignmentIndicator/AlignmentIndicator.tsx`:
- Around line 9-13: The AlignmentIndicatorProps interface declares an unused
factions prop; remove factions from the AlignmentIndicatorProps definition (and
any related unused imports/types) or, if you intend to use it soon, re-add a
brief TODO comment in the AlignmentIndicator component noting planned usage;
adjust the AlignmentIndicator component’s destructuring (the commented-out
factions) and any consumers to match the updated props shape so there are no
unused prop declarations or commented-out code.
- Around line 34-36: The JSX in the AlignmentIndicator component uses
styles.FactionIndicator, styles.FactionIndicator_Ring and
styles.FactionIndicator_Center which mismatches the component name; either
rename those class selectors in the SCSS module to AlignmentIndicator,
AlignmentIndicator_Ring and AlignmentIndicator_Center to match the component, or
if the Faction* classes are intentionally shared, add a clear comment in
AlignmentIndicator.tsx explaining the shared usage and (optionally) create small
local aliases (e.g., map styles.FactionIndicator to styles.AlignmentIndicator)
so the component’s code and class names are consistent and maintainable; update
references in the AlignmentIndicator component accordingly.
- Line 22: primaryAlignment is read as alignments[0] without guarding for an
empty array, which can yield undefined and break
getAlignmentDisplayName(primaryAlignment) and subsequent color checks; update
the AlignmentIndicator component to handle an empty or missing alignments array
by computing a safePrimary (e.g., null or a default alignment) before calling
getAlignmentDisplayName and before any color comparisons, and use that
safePrimary in place of primaryAlignment so functions like
getAlignmentDisplayName(safePrimary) and boolean checks for colors (e.g.,
safePrimary === 'good') never receive undefined.
In `@src/components/AlignmentIndicator/AlignmentIndicator.utils.ts`:
- Around line 6-22: Replace the if-chain in getAlignmentColor with a lookup map
keyed by alignment constants to improve maintainability: create a mapping object
(e.g., const alignmentColorMap) that maps FlamesOfWarV4.Allies,
FlamesOfWarV4.Axis, TeamYankeeV2.Nato, TeamYankeeV2.WarsawPact to 'blue'/'red'
and then return alignmentColorMap[alignment] ?? 'mixed' inside
getAlignmentColor; keep the function signature and Alignment type usage intact
so adding future game systems only requires adding new entries to the map.
In `@src/components/IdentityBadge/TournamentCompetitorAvatar.tsx`:
- Around line 28-31: The code checks activeRegistrations[0]?.user but then
renders Avatar using tournamentCompetitor.registrations[0]?.user?.avatarUrl,
causing inconsistent array access; change the Avatar prop to use the same
filtered/active array (activeRegistrations[0]?.user?.avatarUrl) so the displayed
avatar matches the checked registration. Update the return in the non-team
branch (the component rendering Avatar with ref and {...avatarProps}) to
reference activeRegistrations instead of tournamentCompetitor.registrations,
keeping isTeam, ref, Avatar, and avatarProps unchanged.
In `@src/components/ThemeProvider/ThemeProvider.hooks.ts`:
- Around line 12-34: The useResolvedTheme hook currently uses
mediaQuery.addEventListener/removeEventListener which fails on older Safari;
update the effect in useResolvedTheme to detect whether
mediaQuery.addEventListener exists and if not fall back to the legacy
mediaQuery.addListener, and likewise use mediaQuery.removeEventListener or
mediaQuery.removeListener for cleanup; ensure the same updateTheme callback (the
updateTheme function and mediaQuery variable) is used for both registration and
teardown so the fallback works correctly.
In `@src/components/TournamentForm/components/CompetitorFields.tsx`:
- Around line 135-137: The description string for the FormField with name
"factionsRevealed" is grammatically incorrect; update the description prop on
the FormField (the one with label "Factions Visible to Players" inside
CompetitorFields) from "You always can change this later." to "You can always
change this later." so the text reads properly in the UI.
- Line 84: Add a validation rule to reject submissions when pairingMethod is
TournamentPairingMethod.AdjacentAlignment while competitorSize > 1: update the
tournament form schema in TournamentForm.schema.ts to include a .refine(...)
that checks !(pairingMethod === TournamentPairingMethod.AdjacentAlignment &&
competitorSize > 1) and returns a descriptive error message (e.g., "Adjacent
alignment is not allowed for team competitions"); ensure the error path maps to
the pairingMethod/competitorSize fields so the form displays the message, or
alternatively implement equivalent component-level validation in
CompetitorFields.tsx that prevents submit when pairingMethod/competitorSize
violate this rule.
In `@src/components/TournamentForm/components/FormatFields.tsx`:
- Around line 28-32: Add schema-level validation to tournamentFormSchema using
.refine() or .superRefine() to reject the combination where pairingMethod ===
TournamentPairingMethod.AdjacentAlignment and competitorSize > 1, returning a
clear validation message on the registrationDetails.alignment or competitorSize
field; locate tournamentFormSchema and add the check referencing pairingMethod
and competitorSize. Also update the useEffect in FormatFields.tsx (the effect
that calls setValue('registrationDetails.alignment', 'required') when
pairingMethod === TournamentPairingMethod.AdjacentAlignment) to either reset
alignment when pairingMethod changes away from AdjacentAlignment (e.g.,
setValue('registrationDetails.alignment', undefined) or appropriate default) or
add a concise comment explaining that leaving alignment unchanged when switching
away is intentional UX behavior.
In `@src/components/TournamentForm/TournamentForm.schema.ts`:
- Line 123: The schema currently hardcodes editionYear as 2025; update the
default to be dynamic by replacing the literal with new Date().getFullYear() so
the TournamentForm schema (the editionYear field in TournamentForm.schema.ts)
always uses the current year; ensure any type expectations accept a number and
run tests/compile to confirm no type conflicts with the editionYear property.
- Around line 67-70: Replace the z.union([...z.literal..., z.null()]) usage with
z.enum for the string literals and then .nullable() for the existing null
allowance; specifically update the registrationDetails object's alignment and
faction validators (in TournamentForm.schema.ts) to use
z.enum(['optional','required']).nullable() instead of
z.union([z.literal('optional'), z.literal('required'), z.null()]) to improve
readability and type inference.
In
`@src/components/TournamentProvider/actions/useToggleAlignmentsRevealedAction.ts`:
- Around line 11-18: The success toast and action label need to reflect the
toggle direction using the current subject.alignmentsRevealed state: update the
label (in the returned action where KEY is used) to be conditional — e.g., when
subject.alignmentsRevealed is true show 'Make Inactive' else show 'Reveal
Alignments' — and change the onSuccess toast passed to
useToggleTournamentAlignmentsRevealed so it reports the resulting state (compute
newState = !subject.alignmentsRevealed and call
toast.success(`${subject.displayName} is now ${newState ? 'active' :
'inactive'}!`) or equivalent). Ensure you reference subject.alignmentsRevealed,
mutation, KEY, and the onSuccess handler so the label and toast consistently
reflect the toggle direction.
In `@src/components/TournamentProvider/TournamentProvider.hooks.tsx`:
- Line 17: The action hook useToggleAlignmentsRevealedAction contains copy-paste
mistakes: update the toast message to reflect alignments being revealed or
hidden (use subject.displayName in a message like "<name> alignments revealed"
or "<name> alignments hidden" depending on the new state) and replace confusing
button/label text ('Make Inactive' / 'Make AlignmentsRevealed') with clear terms
such as 'Reveal Alignments' and 'Hide Alignments'; adjust the logic in
useToggleAlignmentsRevealedAction so it chooses the correct message and label
based on the new alignmentsRevealed boolean state and uses subject.displayName
consistently.
In
`@src/components/TournamentRegistrationForm/components/DetailsFields/DetailsFields.tsx`:
- Around line 30-34: The effect handling declaredFaction only sets
details.alignment when declaredFaction is truthy, leaving a stale value when the
faction is cleared; update the useEffect that references declaredFaction,
getFactionAlignment and setValue so that when declaredFaction is falsy it
explicitly calls setValue('details.alignment', null) (and otherwise sets the
alignment to getFactionAlignment(declaredFaction) ?? null), ensuring
details.alignment is cleared when the user removes the faction.
In
`@src/components/TournamentRegistrationForm/TournamentRegistrationForm.schema.ts`:
- Around line 15-28: getDetailsSchema currently throws an Error for unsupported
tournament.gameSystem which will crash the form during initialization; instead
modify getDetailsSchema to avoid throwing—either return a safe fallback schema
(e.g., a minimal/empty validation schema or one that marks all fields optional)
or return null/undefined and let the caller handle rendering an error state.
Update the function branches that call flamesOfWarV4.createSchema and
teamYankeeV2.createSchema to return their schemas as before, and replace the
default throw with a non-throwing fallback (or a sentinel value) so callers of
getDetailsSchema (the form component) can detect unsupported game systems and
render a user-facing message rather than crashing.
In
`@src/pages/TournamentDetailPage/components/TournamentRosterCard/TournamentRosterCard.module.scss`:
- Line 4: The SCSS file imports the borders module via the `@use`
"/src/style/borders"; statement but never uses any mixins or variables from it;
remove the unused import line from TournamentRosterCard.module.scss (delete the
`@use` "/src/style/borders"; statement) to eliminate the dead dependency and keep
the stylesheet clean.
In
`@src/pages/TournamentDetailPage/components/TournamentRosterCard/TournamentRosterCard.tsx`:
- Around line 112-118: The wrapper div is rendered when
tournament.factionsVisible is true even though no factions graph exists,
producing empty space; in TournamentRosterCard update the conditional so the
graphs container is only rendered when tournament.alignmentsVisible is true
(i.e., change the outer condition from (tournament.alignmentsVisible ||
tournament.factionsVisible) to tournament.alignmentsVisible) or alternatively
implement and render a FactionsGraph component when tournament.factionsVisible
is true; adjust JSX around AlignmentGraph and styles.TournamentRosterCard_Graphs
accordingly.
In
`@src/pages/TournamentDetailPage/components/TournamentRosterCard/TournamentRosterCard.utils.tsx`:
- Around line 24-31: The renderCell currently spreads r.details into
<AlignmentIndicator {...r.details} />, which hides which props are passed and
can mask type issues; update the renderCell in TournamentRosterCard.utils.tsx
(the ColumnDef<TournamentCompetitor> entry for key 'alignments') to explicitly
extract the expected props from r.details (e.g., const { propA, propB } =
r.details) and pass them as named props to <AlignmentIndicator propA={propA}
propB={propB} /> so types are explicit and readability improved.
- Around line 28-30: The renderCell handler currently types the row as
TournamentCompetitor but spreads r.details into <AlignmentIndicator>, which
requires DeepTournamentCompetitor (with alignments/factions produced by
getDetails); update the column typing so the row is DeepTournamentCompetitor (or
call getDetails(r) and pass its result) to ensure alignments and factions are
present—locate renderCell in TournamentRosterCard.utils.tsx and change the row
type to DeepTournamentCompetitor or replace {...r.details} with
{...getDetails(r).details} accordingly, keeping AlignmentIndicator props
correctly populated.
Comment on lines
+7
to
+10
| const values: Faction[] = [ | ||
| ...Object.values(flamesOfWarV4), | ||
| ...Object.values(teamYankeeV2), | ||
| ]; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Same duplicate values concern as alignment.ts.
Apply the same deduplication to prevent potential issues with overlapping faction values between game systems.
♻️ Proposed fix
-const values: Faction[] = [
- ...Object.values(flamesOfWarV4),
- ...Object.values(teamYankeeV2),
-];
+const values: Faction[] = [
+ ...new Set([
+ ...Object.values(flamesOfWarV4),
+ ...Object.values(teamYankeeV2),
+ ]),
+];🤖 Prompt for AI Agents
In `@convex/_model/common/faction.ts` around lines 7 - 10, The values array
defined in faction.ts currently concatenates ...Object.values(flamesOfWarV4) and
...Object.values(teamYankeeV2) which can produce duplicate Faction entries;
update the construction of values to deduplicate entries (e.g., by using a Set
or Map keyed on a unique Faction identifier such as id or name) so that the
final values array contains only unique factions. Locate the values declaration
and replace the simple spread concat with a deduplication step that preserves
one canonical Faction per unique id/name.
convex/_model/tournamentRegistrations/_helpers/deepenTournamentRegistration.ts
Show resolved
Hide resolved
src/components/TournamentRegistrationForm/TournamentRegistrationForm.schema.ts
Show resolved
Hide resolved
src/pages/TournamentDetailPage/components/TournamentRosterCard/TournamentRosterCard.module.scss
Outdated
Show resolved
Hide resolved
src/pages/TournamentDetailPage/components/TournamentRosterCard/TournamentRosterCard.tsx
Show resolved
Hide resolved
src/pages/TournamentDetailPage/components/TournamentRosterCard/TournamentRosterCard.utils.tsx
Show resolved
Hide resolved
src/pages/TournamentDetailPage/components/TournamentRosterCard/TournamentRosterCard.utils.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
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)
src/components/TournamentForm/components/CompetitorFields.tsx (1)
84-124: Don’t disable alignment without forcing it to “required” for AdjacentAlignment.
Currently the field is disabled, leaving the value at its previous/default state (oftenoptional/null), which can yield no alignment data for red‑vs‑blue pairing. Consider forcing alignment to'required'when AdjacentAlignment is selected and make the control read‑only instead of disabled.🐛 Suggested fix (force required when AdjacentAlignment)
-import { useFormContext } from 'react-hook-form'; +import { useEffect } from 'react'; +import { useFormContext } from 'react-hook-form'; @@ - const { reset, watch } = useFormContext<TournamentFormData>(); + const { reset, watch, setValue } = useFormContext<TournamentFormData>(); @@ - const disableAlignmentField = pairingMethod === TournamentPairingMethod.AdjacentAlignment; + const disableAlignmentField = pairingMethod === TournamentPairingMethod.AdjacentAlignment; + + useEffect(() => { + if (pairingMethod === TournamentPairingMethod.AdjacentAlignment) { + setValue('registrationDetails.alignment', 'required', { shouldValidate: true }); + } + }, [pairingMethod, setValue]);convex/_model/tournamentRegistrations/mutations/createTournamentRegistration.ts (1)
97-105: Validate required alignment/faction before persistingdetails.
detailsis now saved, but there’s no enforcement whentournament.registrationDetails.alignment/factionisrequired, so registrations can bypass the required questions. Add validation in the “VALIDATE” section and return a clear error.🐛 Suggested validation guard
@@ if (tournament.requireRealNames && currentUser.nameVisibility < VisibilityLevel.Tournaments && !args.nameVisibilityConsent) { throw new ConvexError(getErrorMessage('CANNOT_CREATE_REGISTRATION_WITHOUT_REAL_NAME')); } + if (tournament.registrationDetails?.alignment === 'required' && !args.details?.alignment) { + throw new ConvexError(getErrorMessage('ALIGNMENT_REQUIRED')); + } + if (tournament.registrationDetails?.faction === 'required' && !args.details?.faction) { + throw new ConvexError(getErrorMessage('FACTION_REQUIRED')); + }
🤖 Fix all issues with AI agents
In `@convex/_model/tournaments/mutations/toggleTournamentAlignmentsRevealed.ts`:
- Around line 33-41: The current manual authorization block in
toggleTournamentAlignmentsRevealed.ts duplicates logic and should be replaced
with the shared helper: import and call checkUserIsTournamentOrganizer (the same
helper used in deepenTournament.ts and getAvailableActions.ts) instead of
fetching tournamentOrganizers and building authorizedUserIds; remove the manual
includes-check and rely on the helper (or rethrow/translate its error to
ConvexError with getErrorMessage('USER_DOES_NOT_HAVE_PERMISSION') if the helper
returns a boolean), ensuring the function name checkUserIsTournamentOrganizer is
referenced and imported appropriately so authorization behavior is consistent
across mutations.
In `@src/components/FactionIndicator/FactionIndicator.tsx`:
- Around line 24-30: Replace the hardcoded alignment-to-color logic in the
FactionIndicator component (the primaryAlignmentColor assignment block) with a
call to the shared getAlignmentColor utility: import getAlignmentColor and set
primaryAlignmentColor = getAlignmentColor(primaryAlignment) || 'mixed' so the
component uses the centralized mapping; remove the array checks for 'allies',
'nato', 'axis', 'warsaw_pact' and ensure the import for getAlignmentColor is
added at the top of the file.
In
`@src/components/TournamentCompetitorProvider/actions/useToggleActiveAction.tsx`:
- Around line 11-13: The onSuccess callback for
useToggleTournamentCompetitorActive currently uses an implicitly typed parameter
"active"; change it to explicitly type the parameter as boolean (e.g.,
onSuccess: (active: boolean): void => ...) so the mutation’s return type is
clear and avoids implicit any—update the callback passed to
useToggleTournamentCompetitorActive (the mutation.onSuccess handler) where
"active" is referenced.
In `@src/components/TournamentForm/TournamentForm.schema.ts`:
- Around line 67-73: The schema currently declares alignmentsRevealed and
factionsRevealed as required booleans which will fail for legacy rows; update
both symbols (alignmentsRevealed and factionsRevealed) to accept missing values
by using z.boolean().optional().default(false) (or
z.boolean().optional().catch(false) if you prefer catch semantics) so the schema
matches the backend's optional fields and preserves backward compatibility;
ensure you apply the same change wherever these two fields are validated
alongside registrationDetails.
♻️ Duplicate comments (5)
src/components/TournamentProvider/actions/useToggleAlignmentsRevealedAction.ts (1)
11-13: Toast message wording is inconsistent with the action label.The label correctly says "Hide Alignments" / "Reveal Alignments", but the toast uses "active" / "inactive" which doesn't clearly communicate what changed. Align the toast wording with the action's purpose.
♻️ Suggested fix
const { mutation } = useToggleTournamentAlignmentsRevealed({ - onSuccess: (revealed): void => toast.success(`${subject.displayName} is now ${revealed ? 'active' : 'inactive'}.`), + onSuccess: (revealed): void => toast.success(`${subject.displayName} alignments are now ${revealed ? 'revealed' : 'hidden'}.`), });src/pages/TournamentDetailPage/components/TournamentRosterCard/TournamentRosterCard.utils.tsx (1)
24-31: Ensure roster rows include computeddetailsfor alignment rendering.
renderCellspreadsr.details, but the table is typed asTournamentCompetitor. Ifdetailsonly exists on the deep/expanded competitor shape, this will passundefinedintoFactionIndicatorand hide alignments. Consider typing the column rows as the deep type or mapping rows through the details helper before rendering.src/components/FactionIndicator/FactionIndicator.module.scss (1)
51-52: Remove or justify the commented-out outline rules.Either delete these or document why they’re intentionally preserved.
♻️ Suggested cleanup
- // outline: var(--border-width) solid var(--border-color-default); - // outline-offset: calc(-1 * var(--border-width));src/components/FactionIndicator/FactionIndicator.tsx (1)
21-34: Guard emptyalignmentsbefore callinggetAlignmentDisplayName.If
alignmentscan be empty,getAlignmentDisplayName(undefined)may throw or return unexpected results. Use a safe fallback label.🐛 Proposed fix
- const primaryAlignment = alignments[0]; + const primaryAlignment = alignments[0]; + const alignmentLabel = primaryAlignment + ? getAlignmentDisplayName(primaryAlignment) + : 'Unknown Alignment'; @@ - <InfoPopover content={getAlignmentDisplayName(primaryAlignment) ?? 'Unknown Alignment'}> + <InfoPopover content={alignmentLabel}>src/components/AlignmentGraph/AlignmentGraph.tsx (1)
40-55: Handle query errors explicitly (currently treated as loading).If the registrations query fails,
tournamentRegistrationsstaysundefinedand the graph renders a perpetual loading bar with empty anchors. Consider surfacing the hook’s error state with an error UI/fallback.
convex/_model/tournaments/mutations/toggleTournamentAlignmentsRevealed.ts
Outdated
Show resolved
Hide resolved
src/components/TournamentCompetitorProvider/actions/useToggleActiveAction.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/TournamentForm/TournamentForm.schema.ts`:
- Around line 72-73: The schema's alignmentsRevealed default in
TournamentForm.schema (alignmentsRevealed:
z.boolean().optional().default(false)) conflicts with the form's defaultValues
which set alignmentsRevealed: true; either make the two sources consistent or
document the intent. Fix by updating the zod schema's alignmentsRevealed default
to true to match defaultValues (or change defaultValues to false), or if the
difference is intentional add a clear comment next to the schema entry and the
defaultValues block explaining the legacy vs new-tournament behavior; reference
alignmentsRevealed, factionsRevealed, TournamentForm.schema, and the
defaultValues object when making the change.
♻️ Duplicate comments (3)
src/components/TournamentForm/TournamentForm.schema.ts (1)
67-70: Consider usingz.enumfor cleaner union of string literals.The union of literals can be simplified using
z.enum(['optional', 'required']).nullable()for better readability and type inference.src/components/FactionIndicator/FactionIndicator.utils.ts (1)
6-22: Consider using a lookup map for maintainability.A map-based approach scales better as more game systems are added.
src/components/FactionIndicator/FactionIndicator.tsx (1)
10-20: Unusedfactionsprop in interface and component.The
factionsprop is declared as required inFactionIndicatorPropsbut is commented out in the destructuring and never used. This creates a misleading API where callers must provide a value that has no effect.Either implement the faction display logic or remove the prop until it's needed to keep the interface honest.
♻️ Option A: Remove unused prop
export interface FactionIndicatorProps { alignments: Alignment[]; - factions: Faction[]; className?: string; } export const FactionIndicator = ({ alignments, - // factions, className, }: FactionIndicatorProps): JSX.Element => {♻️ Option B: Make prop optional if planned for future
export interface FactionIndicatorProps { alignments: Alignment[]; - factions: Faction[]; + factions?: Faction[]; className?: string; }
This was referenced Feb 8, 2026
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 #279
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.