#266 Implement TournamentCompetitor details page #269
Conversation
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis pull request implements a comprehensive tournament competitor details page with supporting infrastructure. It introduces new Convex query/action/mutation operations for file and list management, adds action-driven permission systems for tournament competitors and registrations, refactors the generic Table component, updates multiple UI component APIs (Button, ScrollArea), and exposes new provider patterns for managing competitor and registration state across the application. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ianpaschal
force-pushed
the
266-implement-tournamentcompetitor-details-page
branch
from
6f5b14c to
4cdc78d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 57
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/generic/Tag/Tag.scss (1)
5-5: Remove unused import.The
bordersmodule is imported but not used anywhere in the file. According to the AI summary, theborders.normalmixin usage was removed, making this import unnecessary.🔎 Proposed fix
-@use "/src/style/borders";convex/_model/tournamentRegistrations/mutations/deleteTournamentRegistration.ts (1)
48-54: Fix critical authorization bug on line 49.The expression
...tournamentOrganizers.map((r) => r.userId === userId)spreads an array of booleans intoauthorizedUserIds, breaking the authorization check. This should be.map((r) => r.userId)to extract user IDs:const authorizedUserIds = [ - ...tournamentOrganizers.map((r) => r.userId === userId), + ...tournamentOrganizers.map((r) => r.userId), registration.userId, ];This is a critical security vulnerability that allows the authorization check to fail unexpectedly.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
convex/_generated/api.d.tsis excluded by!**/_generated/**package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (107)
convex/_model/common/_helpers/gameSystem/aggregateTournamentData.ts(2 hunks)convex/_model/files/queries/getFileMetadata.ts(1 hunks)convex/_model/lists/actions/extractListData.ts(1 hunks)convex/_model/lists/actions/importList.ts(1 hunks)convex/_model/lists/index.ts(1 hunks)convex/_model/lists/mutations/createList.ts(1 hunks)convex/_model/lists/mutations/importListData.ts(0 hunks)convex/_model/lists/table.ts(2 hunks)convex/_model/matchResults/queries/getMatchResults.ts(1 hunks)convex/_model/tournamentCompetitors/_helpers/deepenTournamentCompetitor.ts(3 hunks)convex/_model/tournamentCompetitors/_helpers/getAvailableActions.ts(1 hunks)convex/_model/tournamentCompetitors/index.ts(1 hunks)convex/_model/tournamentCompetitors/queries/getTournamentCompetitor.ts(2 hunks)convex/_model/tournamentRegistrations/_helpers/deepenTournamentRegistration.ts(1 hunks)convex/_model/tournamentRegistrations/_helpers/getAvailableActions.ts(1 hunks)convex/_model/tournamentRegistrations/index.ts(2 hunks)convex/_model/tournamentRegistrations/mutations/deleteTournamentRegistration.ts(2 hunks)convex/_model/tournamentRegistrations/mutations/toggleActive.ts(3 hunks)convex/_model/tournaments/table.ts(1 hunks)convex/lists.ts(1 hunks)package.json(2 hunks)src/api.ts(3 hunks)src/components/App/App.hooks.ts(2 hunks)src/components/App/App.tsx(2 hunks)src/components/AvatarEditable/AvatarEditable.tsx(1 hunks)src/components/ContextMenu/ContextMenu.tsx(1 hunks)src/components/ContextMenu/ContextMenu.types.ts(1 hunks)src/components/ContextMenu/index.ts(1 hunks)src/components/FloatingActionButton/FloatingActionButton.tsx(1 hunks)src/components/HeartToggle/HeartToggle.tsx(1 hunks)src/components/IdentityBadge/IdentityBadge.hooks.tsx(2 hunks)src/components/IdentityBadge/TournamentCompetitorAvatar.tsx(1 hunks)src/components/IdentityBadge/index.ts(1 hunks)src/components/ListCreateDialog/ListCreateDialog.hooks.tsx(1 hunks)src/components/ListCreateDialog/ListCreateDialog.module.scss(1 hunks)src/components/ListCreateDialog/ListCreateDialog.tsx(1 hunks)src/components/ListCreateDialog/index.ts(1 hunks)src/components/MatchResultCard/MatchResultCard.tsx(3 hunks)src/components/MatchResultCreateDialog/MatchResultCreateDialog.tsx(2 hunks)src/components/MatchResultEditDialog/MatchResultEditDialog.tsx(1 hunks)src/components/MatchResultPhotos/MatchResultPhotos.tsx(1 hunks)src/components/PaginatedList/PaginatedList.module.scss(1 hunks)src/components/PaginatedList/PaginatedList.tsx(1 hunks)src/components/PaginatedList/index.ts(1 hunks)src/components/TournamentActionsProvider/index.ts(1 hunks)src/components/TournamentCard/TournamentCard.tsx(1 hunks)src/components/TournamentCompetitorEditDialog/TournamentCompetitorEditDialog.tsx(4 hunks)src/components/TournamentCompetitorProvider/TournamentCompetitorContextMenu.tsx(1 hunks)src/components/TournamentCompetitorProvider/TournamentCompetitorProvider.context.ts(1 hunks)src/components/TournamentCompetitorProvider/TournamentCompetitorProvider.hooks.tsx(1 hunks)src/components/TournamentCompetitorProvider/TournamentCompetitorProvider.tsx(1 hunks)src/components/TournamentCompetitorProvider/index.ts(1 hunks)src/components/TournamentContextMenu/index.ts(1 hunks)src/components/TournamentRegistrationProvider/TournamentRegistrationActiveToggle.tsx(1 hunks)src/components/TournamentRegistrationProvider/TournamentRegistrationContextMenu.tsx(1 hunks)src/components/TournamentRegistrationProvider/TournamentRegistrationProvider.context.ts(1 hunks)src/components/TournamentRegistrationProvider/TournamentRegistrationProvider.hooks.tsx(1 hunks)src/components/TournamentRegistrationProvider/TournamentRegistrationProvider.tsx(1 hunks)src/components/TournamentRegistrationProvider/index.ts(1 hunks)src/components/TournamentRoster/TournamentRoster.tsx(3 hunks)src/components/TournamentRoster/components/CompetitorActions/CompetitorActions.module.scss(1 hunks)src/components/TournamentRoster/components/CompetitorActions/CompetitorActions.tsx(1 hunks)src/components/generic/Avatar/Avatar.tsx(5 hunks)src/components/generic/Button/Button.module.scss(4 hunks)src/components/generic/Button/Button.tsx(2 hunks)src/components/generic/Carousel/CarouselNextButton.tsx(1 hunks)src/components/generic/Carousel/CarouselPreviousButton.tsx(1 hunks)src/components/generic/ScrollArea/ScrollArea.hooks.tsx(5 hunks)src/components/generic/ScrollArea/ScrollArea.tsx(1 hunks)src/components/generic/Table/Table.hooks.tsx(1 hunks)src/components/generic/Table/Table.module.scss(1 hunks)src/components/generic/Table/Table.tsx(1 hunks)src/components/generic/Table/Table.types.ts(1 hunks)src/components/generic/Table/Table.utils.ts(1 hunks)src/components/generic/Table/TableCell.tsx(1 hunks)src/components/generic/Table/TableRow.tsx(0 hunks)src/components/generic/Table/index.ts(1 hunks)src/components/generic/Tabs/TabsList.module.scss(1 hunks)src/components/generic/Tabs/TabsList.tsx(3 hunks)src/components/generic/Tabs/TabsTrigger.scss(1 hunks)src/components/generic/Tag/Tag.scss(1 hunks)src/pages/DashboardPage/components/ActiveTournament/ActiveTournament.module.scss(1 hunks)src/pages/DashboardPage/components/ActiveTournament/ActiveTournament.tsx(1 hunks)src/pages/DashboardPage/components/ActiveTournament/ActiveTournament.utils.tsx(2 hunks)src/pages/TournamentCompetitorDetailPage/TournamentCompetitorDetailPage.module.scss(1 hunks)src/pages/TournamentCompetitorDetailPage/TournamentCompetitorDetailPage.tsx(1 hunks)src/pages/TournamentCompetitorDetailPage/components/Header/Header.module.scss(1 hunks)src/pages/TournamentCompetitorDetailPage/components/Header/Header.tsx(1 hunks)src/pages/TournamentCompetitorDetailPage/components/Header/index.ts(1 hunks)src/pages/TournamentCompetitorDetailPage/components/MatchResultsList/MatchResultsList.module.scss(1 hunks)src/pages/TournamentCompetitorDetailPage/components/MatchResultsList/MatchResultsList.tsx(1 hunks)src/pages/TournamentCompetitorDetailPage/components/MatchResultsList/index.ts(1 hunks)src/pages/TournamentCompetitorDetailPage/components/TournamentRegistrationListButton/TournamentRegistrationListButton.module.scss(1 hunks)src/pages/TournamentCompetitorDetailPage/components/TournamentRegistrationListButton/TournamentRegistrationListButton.tsx(1 hunks)src/pages/TournamentCompetitorDetailPage/components/TournamentRegistrationListButton/index.ts(1 hunks)src/pages/TournamentCompetitorDetailPage/components/TournamentRegistrationsTable/TournamentRegistrationsTable.module.scss(1 hunks)src/pages/TournamentCompetitorDetailPage/components/TournamentRegistrationsTable/TournamentRegistrationsTable.tsx(1 hunks)src/pages/TournamentCompetitorDetailPage/components/TournamentRegistrationsTable/index.ts(1 hunks)src/pages/TournamentCompetitorDetailPage/index.ts(1 hunks)src/pages/TournamentDetailPage/TournamentDetailPage.tsx(1 hunks)src/pages/TournamentDetailPage/components/TournamentPairingsCard/TournamentPairingsCard.module.scss(1 hunks)src/pages/TournamentDetailPage/components/TournamentPairingsCard/TournamentPairingsCard.tsx(1 hunks)src/pages/TournamentDetailPage/components/TournamentPairingsCard/TournamentPairingsCard.utils.tsx(3 hunks)src/pages/TournamentDetailPage/components/TournamentRankingsCard/TournamentRankingsCard.module.scss(1 hunks)src/pages/TournamentDetailPage/components/TournamentRankingsCard/TournamentRankingsCard.tsx(1 hunks)src/pages/TournamentDetailPage/components/TournamentRankingsCard/TournamentRankingsCard.utils.tsx(2 hunks)src/pages/TournamentPairingsPage/components/ConfirmPairingsDialog/ConfirmPairingsDialog.module.scss(1 hunks)
⛔ Files not processed due to max files limit (13)
- src/pages/TournamentPairingsPage/components/ConfirmPairingsDialog/ConfirmPairingsDialog.tsx
- src/pages/TournamentPairingsPage/components/ConfirmPairingsDialog/ConfirmPairingsDialog.utils.tsx
- src/routes.tsx
- src/services/files.ts
- src/services/lists.ts
- src/services/utils/createPaginatedQueryHook.ts
- src/settings.ts
- src/style/_borders.scss
- src/style/_text.scss
- src/style/_variants.scss
- src/utils/common/getPathWithQuery.ts
- src/utils/common/getTournamentCompetitorDisplayName.ts
- src/utils/common/isUserTournamentCompetitorCaptain.ts
💤 Files with no reviewable changes (2)
- convex/_model/lists/mutations/importListData.ts
- src/components/generic/Table/TableRow.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-08T05:51:08.211Z
Learnt from: ianpaschal
Repo: ianpaschal/combat-command PR: 111
File: src/components/generic/Table/Table.types.ts:10-10
Timestamp: 2025-07-08T05:51:08.211Z
Learning: In TypeScript, adding optional parameters to function signatures is backward compatible. When a function type signature specifies more parameters than an implementation uses (e.g., changing `(row: T) => ReactNode` to `(row: T, index: number) => ReactNode`), it doesn't cause TypeScript errors. Functions can accept fewer parameters than allowed by the type, making such changes additive rather than breaking.
Applied to files:
src/components/generic/Table/Table.types.ts
🧬 Code graph analysis (41)
src/components/ContextMenu/ContextMenu.types.ts (1)
src/api.ts (1)
TournamentActionKey(85-85)
src/components/TournamentRegistrationProvider/TournamentRegistrationActiveToggle.tsx (1)
src/components/TournamentRegistrationProvider/TournamentRegistrationProvider.hooks.tsx (1)
useTournamentRegistration(18-24)
src/components/TournamentRegistrationProvider/TournamentRegistrationContextMenu.tsx (1)
src/components/TournamentRegistrationProvider/TournamentRegistrationProvider.hooks.tsx (1)
useTournamentRegistration(18-24)
src/components/TournamentCompetitorProvider/TournamentCompetitorContextMenu.tsx (3)
src/types/componentLib.ts (1)
ElementSize(5-5)src/components/TournamentCompetitorProvider/TournamentCompetitorProvider.hooks.tsx (1)
useTournamentCompetitor(20-26)src/components/TournamentContextMenu/TournamentContextMenu.tsx (1)
TournamentContextMenuProps(14-32)
src/components/TournamentRegistrationProvider/TournamentRegistrationProvider.hooks.tsx (5)
src/components/TournamentRegistrationProvider/TournamentRegistrationProvider.context.ts (2)
TournamentRegistrationContext(17-17)TournamentRegistrationActions(10-10)src/components/TournamentCompetitorProvider/TournamentCompetitorProvider.hooks.tsx (1)
useTournamentCompetitor(20-26)src/utils/common/getTournamentDisplayName.ts (1)
getTournamentDisplayName(3-8)src/utils/common/getTournamentCompetitorDisplayName.ts (1)
getTournamentCompetitorDisplayName(10-35)src/services/tournamentRegistrations.ts (2)
useDeleteTournamentRegistration(11-11)useToggleTournamentRegistrationActive(14-14)
src/components/PaginatedList/PaginatedList.tsx (2)
src/services/utils/createPaginatedQueryHook.ts (2)
QueryFn(12-12)PaginatedQueryHookResult(16-21)src/settings.ts (1)
DEFAULT_PAGE_SIZE(10-10)
src/components/generic/Tabs/TabsList.tsx (2)
src/components/generic/Tabs/index.ts (2)
Tabs(5-5)TabsTrigger(3-3)src/components/generic/Tabs/TabsTrigger.tsx (1)
TabsTrigger(17-23)
src/pages/TournamentCompetitorDetailPage/components/Header/Header.tsx (5)
src/utils/common/getTournamentDisplayName.ts (1)
getTournamentDisplayName(3-8)src/components/TournamentCompetitorProvider/TournamentCompetitorProvider.hooks.tsx (1)
useTournamentCompetitor(20-26)src/utils/common/getTournamentCompetitorDisplayName.ts (1)
getTournamentCompetitorDisplayName(10-35)src/utils/common/getPathWithQuery.ts (1)
getPathWithQuery(3-8)src/settings.ts (1)
PATHS(12-37)
src/components/TournamentCompetitorProvider/TournamentCompetitorProvider.hooks.tsx (6)
src/components/TournamentCompetitorProvider/TournamentCompetitorProvider.context.ts (1)
TournamentCompetitorContext(21-24)src/utils/common/getTournamentCompetitorDisplayName.ts (1)
getTournamentCompetitorDisplayName(10-35)src/components/TournamentCompetitorEditDialog/TournamentCompetitorEditDialog.hooks.ts (1)
useTournamentCompetitorEditDialog(12-21)src/services/tournamentCompetitors.ts (1)
useDeleteTournamentCompetitor(14-14)src/settings.ts (1)
PATHS(12-37)src/services/tournamentRegistrations.ts (2)
useCreateTournamentRegistration(10-10)useDeleteTournamentRegistration(11-11)
src/components/generic/Button/Button.tsx (1)
src/types/componentLib.ts (2)
ElementSize(5-5)ElementVariant(1-1)
src/components/ListCreateDialog/ListCreateDialog.hooks.tsx (1)
src/modals.ts (1)
useModal(28-37)
src/components/TournamentCompetitorEditDialog/TournamentCompetitorEditDialog.tsx (2)
src/services/tournamentRegistrations.ts (1)
useGetTournamentRegistrationsByCompetitor(5-5)src/components/generic/Button/Button.tsx (1)
Button(30-64)
src/pages/TournamentDetailPage/components/TournamentPairingsCard/TournamentPairingsCard.tsx (1)
src/components/generic/Table/Table.tsx (1)
Table(17-58)
src/components/ListCreateDialog/ListCreateDialog.tsx (5)
src/components/ListCreateDialog/ListCreateDialog.hooks.tsx (1)
useListCreateDialog(4-4)src/api.ts (1)
StorageId(44-44)src/services/files.ts (1)
useUploadFile(15-56)src/services/lists.ts (3)
useImportList(15-15)useExtractListData(16-16)useCreateList(12-12)src/components/generic/Tabs/index.ts (1)
Tabs(5-5)
convex/_model/lists/actions/extractListData.ts (2)
convex/_model/common/_helpers/getStaticEnumConvexValidator.ts (1)
getStaticEnumConvexValidator(7-13)convex/_generated/server.d.ts (1)
ActionCtx(129-129)
convex/_model/common/_helpers/gameSystem/aggregateTournamentData.ts (1)
convex/_model/common/types.ts (1)
AnyBaseStats(11-11)
convex/_model/tournamentCompetitors/_helpers/getAvailableActions.ts (3)
convex/_generated/server.d.ts (1)
QueryCtx(113-113)convex/_generated/dataModel.d.ts (1)
Doc(30-33)convex/_model/tournamentRegistrations/_helpers/checkUserIsRegistered.ts (1)
checkUserIsRegistered(4-16)
src/components/TournamentCompetitorProvider/TournamentCompetitorProvider.tsx (2)
src/components/TournamentCompetitorProvider/TournamentCompetitorProvider.context.ts (1)
TournamentCompetitorContext(21-24)src/components/TournamentCompetitorEditDialog/TournamentCompetitorEditDialog.tsx (1)
TournamentCompetitorEditDialog(29-130)
src/components/IdentityBadge/TournamentCompetitorAvatar.tsx (1)
src/utils/common/getCountryName.ts (1)
getCountryName(3-20)
convex/_model/tournamentRegistrations/_helpers/getAvailableActions.ts (2)
convex/_generated/server.d.ts (1)
QueryCtx(113-113)convex/_generated/dataModel.d.ts (1)
Doc(30-33)
src/components/ContextMenu/ContextMenu.tsx (2)
src/components/ContextMenu/ContextMenu.types.ts (1)
ActionKey(14-14)src/types/componentLib.ts (1)
ElementSize(5-5)
convex/lists.ts (1)
convex/_model/lists/index.ts (4)
importList(10-10)extractListData(6-6)createList(21-21)getList(25-25)
convex/_model/files/queries/getFileMetadata.ts (2)
convex/_generated/server.d.ts (1)
QueryCtx(113-113)convex/_model/common/errors.ts (1)
getErrorMessage(113-125)
convex/_model/tournamentRegistrations/mutations/toggleActive.ts (3)
convex/_model/tournamentRegistrations/index.ts (1)
TournamentRegistrationActionKey(9-9)src/api.ts (1)
TournamentRegistrationActionKey(70-70)convex/_model/common/errors.ts (1)
getErrorMessage(113-125)
src/components/TournamentRegistrationProvider/TournamentRegistrationProvider.tsx (1)
src/components/TournamentRegistrationProvider/TournamentRegistrationProvider.context.ts (1)
TournamentRegistrationContext(17-17)
src/components/generic/Table/Table.tsx (4)
src/components/generic/Table/Table.types.ts (2)
RowData(3-3)ColumnDef(5-14)src/components/generic/Table/Table.hooks.tsx (2)
useScrollIndicator(25-55)useGridStyle(57-75)src/components/generic/Table/TableCell.tsx (1)
TableCell(19-63)src/components/generic/Table/Table.utils.ts (1)
getPosition(3-14)
src/components/App/App.tsx (2)
src/settings.ts (2)
MAX_WIDTH(7-7)MIN_WIDTH_TABLET(2-2)src/components/AccountMenu/AccountMenu.tsx (1)
AccountMenu(18-88)
convex/_model/lists/actions/importList.ts (1)
convex/_generated/server.d.ts (1)
ActionCtx(129-129)
convex/_model/tournamentCompetitors/_helpers/deepenTournamentCompetitor.ts (1)
convex/_generated/dataModel.d.ts (1)
Doc(30-33)
src/components/MatchResultCard/MatchResultCard.tsx (7)
src/settings.ts (1)
PATHS(12-37)src/components/MatchResultProvider/MatchResultProvider.tsx (1)
MatchResultProvider(11-18)src/components/MatchResultPhotos/MatchResultPhotos.tsx (1)
MatchResultPhotos(23-61)src/components/MatchResultPlayers/MatchResultPlayers.tsx (1)
MatchResultPlayers(13-70)src/components/MatchResultSocials/MatchResultSocials.tsx (1)
MatchResultSocials(24-88)src/components/generic/Timestamp/Timestamp.tsx (1)
Timestamp(12-21)src/components/MatchResultContextMenu/MatchResultContextMenu.tsx (1)
MatchResultContextMenu(16-50)
src/components/App/App.hooks.ts (2)
src/utils/common/getPathWithQuery.ts (1)
getPathWithQuery(3-8)src/settings.ts (1)
PATHS(12-37)
convex/_model/tournamentRegistrations/_helpers/deepenTournamentRegistration.ts (2)
convex/_generated/server.d.ts (1)
QueryCtx(113-113)convex/_generated/dataModel.d.ts (1)
Doc(30-33)
src/pages/TournamentCompetitorDetailPage/components/MatchResultsList/MatchResultsList.tsx (1)
src/services/matchResults.ts (1)
useGetMatchResults(10-10)
convex/_model/lists/mutations/createList.ts (3)
convex/_generated/server.d.ts (1)
MutationCtx(121-121)convex/_generated/dataModel.d.ts (1)
Id(48-49)convex/_model/common/errors.ts (1)
getErrorMessage(113-125)
src/components/generic/Table/TableCell.tsx (1)
src/components/generic/Table/Table.types.ts (3)
RowData(3-3)ColumnDef(5-14)CellPosition(18-21)
src/pages/TournamentCompetitorDetailPage/TournamentCompetitorDetailPage.tsx (5)
src/services/tournaments.ts (1)
useGetTournament(6-6)src/services/tournamentCompetitors.ts (1)
useGetTournamentCompetitor(5-5)src/utils/common/getLastVisibleTournamentRound.ts (1)
getLastVisibleTournamentRound(13-28)src/hooks/useDeviceSize.ts (1)
useDeviceSize(17-34)src/components/generic/Tabs/index.ts (1)
Tabs(5-5)
src/components/TournamentRoster/TournamentRoster.tsx (6)
src/utils/common/isUserTournamentOrganizer.ts (1)
isUserTournamentOrganizer(3-11)src/services/tournamentRegistrations.ts (1)
useToggleTournamentRegistrationActive(14-14)convex/_model/tournamentCompetitors/index.ts (1)
TournamentCompetitorId(3-3)src/api.ts (1)
TournamentCompetitorId(50-50)src/settings.ts (1)
PATHS(12-37)src/components/generic/Accordion/Accordion.tsx (1)
Accordion(13-28)
src/components/generic/Table/Table.hooks.tsx (1)
src/components/generic/Table/Table.types.ts (1)
RowData(3-3)
src/components/generic/ScrollArea/ScrollArea.tsx (1)
src/components/generic/ScrollArea/ScrollArea.hooks.tsx (3)
IndicatorSide(15-15)IndicatorState(16-19)useScrollIndicators(94-136)
src/components/generic/Table/Table.utils.ts (1)
src/components/generic/Table/Table.types.ts (1)
PositionFlag(16-16)
src/components/IdentityBadge/IdentityBadge.hooks.tsx (1)
src/utils/common/getTournamentCompetitorDisplayName.ts (1)
getTournamentCompetitorDisplayName(10-35)
🪛 Biome (2.1.2)
src/components/IdentityBadge/IdentityBadge.hooks.tsx
[error] 32-32: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 33-33: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
Comment on lines
+111
to
114
| const opponentCompetitorId = competitorIds.filter((id) => id !== tournamentCompetitorId).pop(); | ||
| if (opponentCompetitorId) { | ||
| competitorStats[tournamentCompetitorId].opponentIds.push(opponentCompetitorId); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using find() instead of filter().pop().
The current approach works but find() is more idiomatic and slightly more efficient for getting a single element.
🔎 Proposed refactor
- const opponentCompetitorId = competitorIds.filter((id) => id !== tournamentCompetitorId).pop();
+ const opponentCompetitorId = competitorIds.find((id) => id !== tournamentCompetitorId);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const opponentCompetitorId = competitorIds.filter((id) => id !== tournamentCompetitorId).pop(); | |
| if (opponentCompetitorId) { | |
| competitorStats[tournamentCompetitorId].opponentIds.push(opponentCompetitorId); | |
| } | |
| const opponentCompetitorId = competitorIds.find((id) => id !== tournamentCompetitorId); | |
| if (opponentCompetitorId) { | |
| competitorStats[tournamentCompetitorId].opponentIds.push(opponentCompetitorId); | |
| } |
🤖 Prompt for AI Agents
In convex/_model/common/_helpers/gameSystem/aggregateTournamentData.ts around
lines 111 to 114, replace the use of competitorIds.filter(...).pop() with
competitorIds.find(...) to get the first id that is not equal to
tournamentCompetitorId; update the variable name if helpful (e.g.,
opponentCompetitorId) and keep the existing conditional and push into
competitorStats[tournamentCompetitorId].opponentIds when the found id is truthy.
This removes the unnecessary array allocation from filter and uses the more
idiomatic, slightly more efficient find() for a single-element lookup.
Comment on lines
+16
to
+25
| export const getFileMetadata = async ( | ||
| ctx: QueryCtx, | ||
| args: Infer<typeof getFileMetadataArgs>, | ||
| ): Promise<GenericDoc<SystemDataModel,'_storage'> | null> => { | ||
| const fileMetadata = await ctx.db.system.get(args.id); | ||
| if (!fileMetadata) { | ||
| throw new ConvexError(getErrorMessage('FILE_NOT_FOUND')); | ||
| } | ||
| return fileMetadata; | ||
| }; |
There was a problem hiding this comment.
Return type includes | null but the function never returns null.
The return type declares Promise<GenericDoc<SystemDataModel,'_storage'> | null>, but the implementation always either throws a ConvexError (line 22) or returns a non-null value (line 24). This creates an inconsistency between the declared contract and actual behavior, potentially leading consumers to write unnecessary null checks.
🔎 Proposed fix: Remove `| null` from return type
export const getFileMetadata = async (
ctx: QueryCtx,
args: Infer<typeof getFileMetadataArgs>,
-): Promise<GenericDoc<SystemDataModel,'_storage'> | null> => {
+): Promise<GenericDoc<SystemDataModel,'_storage'>> => {
const fileMetadata = await ctx.db.system.get(args.id);
if (!fileMetadata) {
throw new ConvexError(getErrorMessage('FILE_NOT_FOUND'));
}
return fileMetadata;
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const getFileMetadata = async ( | |
| ctx: QueryCtx, | |
| args: Infer<typeof getFileMetadataArgs>, | |
| ): Promise<GenericDoc<SystemDataModel,'_storage'> | null> => { | |
| const fileMetadata = await ctx.db.system.get(args.id); | |
| if (!fileMetadata) { | |
| throw new ConvexError(getErrorMessage('FILE_NOT_FOUND')); | |
| } | |
| return fileMetadata; | |
| }; | |
| export const getFileMetadata = async ( | |
| ctx: QueryCtx, | |
| args: Infer<typeof getFileMetadataArgs>, | |
| ): Promise<GenericDoc<SystemDataModel,'_storage'>> => { | |
| const fileMetadata = await ctx.db.system.get(args.id); | |
| if (!fileMetadata) { | |
| throw new ConvexError(getErrorMessage('FILE_NOT_FOUND')); | |
| } | |
| return fileMetadata; | |
| }; |
🤖 Prompt for AI Agents
In convex/_model/files/queries/getFileMetadata.ts around lines 16 to 25 the
function return type is declared as
Promise<GenericDoc<SystemDataModel,'_storage'> | null> but the implementation
never returns null (it throws on missing data and otherwise returns a non-null
document); update the declared return type to
Promise<GenericDoc<SystemDataModel,'_storage'>> to match actual behavior and
remove the unnecessary nullable union so callers won’t expect a null case.
Comment on lines
+1
to
+36
| /* eslint-disable @typescript-eslint/no-unused-vars */ | ||
| import { Infer, v } from 'convex/values'; | ||
|
|
||
| import { ActionCtx } from '../../../_generated/server'; | ||
|
|
||
| export const importListArgs = v.object({ | ||
| url: v.string(), | ||
| gameSystem: v.string(), | ||
| }); | ||
|
|
||
| export const importList = async ( | ||
| ctx: ActionCtx, | ||
| args: Infer<typeof importListArgs>, | ||
| ): Promise<void> => { | ||
|
|
||
| const url = new URL(args.url); | ||
|
|
||
| // Extract basic components | ||
| const domain = url.hostname; // "example.com" | ||
| const protocol = url.protocol; // "https:" | ||
| const pathname = url.pathname; // "/users/550e8400-e29b-41d4-a801-146655440000" | ||
| const searchParams = url.searchParams; // URLSearchParams object | ||
|
|
||
| // Extract path segments | ||
| const pathSegments = url.pathname.split('/').filter(Boolean); | ||
| // ["users", "550e8400-e29b-41d4-a801-146655440000"] | ||
|
|
||
| // Extract UUID from path (if you know its position) | ||
| const uuid = pathSegments[1]; | ||
|
|
||
| // Or use regex to find UUID anywhere in the path | ||
| const uuidRegex = /[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/i; | ||
| const match = url.pathname.match(uuidRegex); | ||
| const extractedUuid = match ? match[0] : null; | ||
|
|
||
| }; |
There was a problem hiding this comment.
Complete the stub implementation before merging.
This action is incomplete and non-functional:
- All extracted URL components are unused
- No actual import logic is implemented
- Function returns void without side effects
- The
eslint-disable @typescript-eslint/no-unused-varsconfirms the incomplete state
This stub should not be merged to main until the import logic is implemented.
Do you want me to help implement the list import logic, or should this file be removed from the PR until the implementation is complete?
🤖 Prompt for AI Agents
In convex/_model/lists/actions/importList.ts lines 1–36, the function is a stub:
it parses the URL but never uses any values, has eslint-disable for unused vars,
and performs no import side effects; implement the import logic by removing the
eslint-disable and unused variables, validate args.url and args.gameSystem,
fetch the remote resource at args.url (handle network errors and non-200
responses), parse the payload according to the expected source format (JSON or
HTML), extract list metadata and entries (use the UUID or extractedUuid as the
canonical external id if present), map/normalize entries to our schema and
upsert the list and its items into Convex using ctx.db (or provided server APIs)
within a transaction, add logging and error handling, and return/resolve only
after successful DB writes; include unit/integration tests or error responses
for invalid inputs and ensure no unused variables remain.
Comment on lines
+12
to
+15
| // const forceDiagram = getStaticEnumConvexValidator(ForceDiagram); | ||
| // const formation = getStaticEnumConvexValidator(Unit); | ||
|
|
||
| // const nanoid = customAlphabet('1234567890abcdefghijklmnopqrstuvwxyz', 6); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove commented-out code.
These commented-out imports and code should be removed. If they're needed for future features, they can be restored from version control when required.
🔎 Proposed cleanup
-// const forceDiagram = getStaticEnumConvexValidator(ForceDiagram);
-// const formation = getStaticEnumConvexValidator(Unit);
-
-// const nanoid = customAlphabet('1234567890abcdefghijklmnopqrstuvwxyz', 6);
-📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // const forceDiagram = getStaticEnumConvexValidator(ForceDiagram); | |
| // const formation = getStaticEnumConvexValidator(Unit); | |
| // const nanoid = customAlphabet('1234567890abcdefghijklmnopqrstuvwxyz', 6); |
🤖 Prompt for AI Agents
In convex/_model/lists/mutations/createList.ts around lines 12 to 15, remove the
three commented-out lines (the two getStaticEnumConvexValidator calls and the
nanoid customAlphabet declaration); delete these commented blocks to clean up
unused code—if needed later they can be restored from VCS.
|
|
||
| export const createListArgs = v.object(editableFields); | ||
|
|
||
| // TODO: Auto lock if after deadline |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify or remove the TODO comment.
The auto-locking logic appears to already be implemented on line 40, where locked is set based on comparing Date.now() with listSubmissionClosesAt. Either remove this TODO or clarify if additional auto-locking behavior is needed.
🤖 Prompt for AI Agents
In convex/_model/lists/mutations/createList.ts around line 19, the TODO comment
“Auto lock if after deadline” is misleading because auto-lock logic is already
implemented at line 40 by setting `locked` based on comparing `Date.now()` with
`listSubmissionClosesAt`; either remove this TODO comment or update it to
specify what additional behavior (if any) is still required (for example:
scheduled background job, webhook, or notification when auto-locked), and if no
extra behavior is needed, delete the TODO to avoid confusion.
| value: i, | ||
| })); | ||
|
|
||
| const query = useGetMatchResults({}); |
There was a problem hiding this comment.
Major: Match results query needs filter parameter.
The query is called with an empty object, which will fetch ALL match results instead of filtering by tournamentCompetitorId. This is related to the critical issue in convex/_model/matchResults/queries/getMatchResults.ts where the filter parameter is defined but not implemented.
Once the filter is implemented in the query, update this to:
- const query = useGetMatchResults({});
+ const query = useGetMatchResults({
+ filter: {
+ tournamentCompetitorId,
+ },
+ });This component depends on the filter implementation in getMatchResults. Should this be completed in the current PR, or is this intentionally incomplete?
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/pages/TournamentCompetitorDetailPage/components/MatchResultsList/MatchResultsList.tsx
around line 34, the useGetMatchResults hook is being called with an empty object
which causes fetching all match results; update the call to pass the
tournamentCompetitorId filter (e.g., { filter: { tournamentCompetitorId:
props.tournamentCompetitorId } }) once the getMatchResults query implements the
filter parameter, and add a conditional guard or loading state if the id is not
yet available; confirm whether getMatchResults will be completed in this PR or
coordinate so the prop-based filter here matches the query's expected shape.
Comment on lines
+26
to
+63
| columns={[ | ||
| { | ||
| key: 'active', | ||
| label: 'Active', | ||
| renderCell: (r) => ( | ||
| <TournamentRegistrationProvider tournamentRegistration={r}> | ||
| <TournamentRegistrationActiveToggle /> | ||
| </TournamentRegistrationProvider> | ||
| ), | ||
| }, | ||
| { | ||
| key: 'identity', | ||
| label: 'Player', | ||
| width: '1fr', | ||
| xAlign: 'left', | ||
| renderCell: ({ user }) => ( | ||
| <IdentityBadge user={user} /> | ||
| ), | ||
| }, | ||
| // { | ||
| // key: 'lists', | ||
| // label: 'List', | ||
| // renderCell: (row) => { | ||
| // const isCaptain = user && (tournamentCompetitors ?? []).find((c) => c._id === row.tournamentCompetitorId)?.registrations.find((r) => r.userId === user._id); | ||
| // return ( | ||
| // <TournamentRegistrationListButton tournamentRegistration={row} deadline={Date.now() - 3600} /> | ||
| // ); | ||
| // }, | ||
| // }, | ||
| { | ||
| key: 'actions', | ||
| renderCell: (r) => ( | ||
| <TournamentRegistrationProvider tournamentRegistration={r}> | ||
| <TournamentRegistrationContextMenu /> | ||
| </TournamentRegistrationProvider> | ||
| ), | ||
| }, | ||
| ]} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Duplicate TournamentRegistrationProvider instances per row.
Each row instantiates TournamentRegistrationProvider twice (lines 31-34 for the toggle, lines 58-60 for the context menu). This is inefficient and could lead to inconsistent state if the provider manages local state or triggers side effects.
Consider restructuring so each row is wrapped in a single provider, or use a custom row renderer that wraps all cells for a given registration.
🤖 Prompt for AI Agents
In
src/pages/TournamentCompetitorDetailPage/components/TournamentRegistrationsTable/TournamentRegistrationsTable.tsx
around lines 26 to 63, each row currently creates two
TournamentRegistrationProvider instances (one around the Active toggle and one
around the ContextMenu) which is inefficient and can cause inconsistent state;
refactor so a single TournamentRegistrationProvider wraps the entire row:
implement a custom row renderer (or wrap the cell renderers at the columns
level) that takes the registration row, returns <TournamentRegistrationProvider
tournamentRegistration={row}> around all cells for that row, then render
TournamentRegistrationActiveToggle and TournamentRegistrationContextMenu inside
that single provider (or alternatively remove the providers from the individual
cells and pass the row down as props to the components) to eliminate duplicate
providers and ensure consistent state.
Comment on lines
+45
to
+54
| // { | ||
| // key: 'lists', | ||
| // label: 'List', | ||
| // renderCell: (row) => { | ||
| // const isCaptain = user && (tournamentCompetitors ?? []).find((c) => c._id === row.tournamentCompetitorId)?.registrations.find((r) => r.userId === user._id); | ||
| // return ( | ||
| // <TournamentRegistrationListButton tournamentRegistration={row} deadline={Date.now() - 3600} /> | ||
| // ); | ||
| // }, | ||
| // }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove or properly implement commented-out code.
This commented code references user and tournamentCompetitors which are not defined in scope. If this feature is planned, consider creating an issue to track it and removing the dead code. If it's needed soon, implement it with proper variable definitions.
🤖 Prompt for AI Agents
In
src/pages/TournamentCompetitorDetailPage/components/TournamentRegistrationsTable/TournamentRegistrationsTable.tsx
around lines 45 to 54, remove the commented-out block that references undefined
variables `user` and `tournamentCompetitors` or replace it with a tracked TODO:
either delete the dead code entirely or create a proper implementation by
introducing the required props/state (provide `user` and `tournamentCompetitors`
from parent or context) and wire the renderCell to use those variables; if you
don’t implement it now, open an issue referencing this file/lines and add a
one-line comment pointing to that issue before removing the commented code.
Comment on lines
+85
to
+96
| {/* <div className={styles.TournamentCompetitorPage_Warnings}> | ||
| warnings | ||
| </div> */} | ||
| <TournamentRegistrationsTable | ||
| className={styles.TournamentCompetitorPage_Table} | ||
| registrations={tournamentCompetitor?.registrations ?? []} | ||
| /> | ||
| {/* {showJoinButton && ( | ||
| <div className={styles.TournamentCompetitorPage_Actions}> | ||
| <Button text="Join" icon={<UserPlus />} /> | ||
| </div> | ||
| )} */} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove or implement commented UI sections.
The warnings section (lines 85-87) and join button (lines 92-96) are commented out. Either implement these features or remove the commented code to reduce clutter.
🤖 Prompt for AI Agents
In src/pages/TournamentCompetitorDetailPage/TournamentCompetitorDetailPage.tsx
around lines 85 to 96, there are two UI blocks commented out (a warnings section
and a join button) creating clutter; either remove these commented blocks or
implement them: if removing, delete the commented JSX lines and any unused
styles/props related to them; if implementing, restore the JSX, wire the
warnings block to a warnings prop or compute from tournamentCompetitor data and
render meaningful messages, and enable the Join button by adding a
showJoinButton boolean (derived from permissions/registration state) and a click
handler that triggers the join flow (call existing registration action or
route), then remove leftover commented code and unused imports/styles
accordingly.
| xAlign: 'center', | ||
| renderCell: ({ matchResultsProgress: { submitted, required } }) => ( | ||
| <CircularProgress total={required} filled={submitted} size={matchIndicatorSize} color={submitted === required ? 'green' : undefined}> | ||
| <CircularProgress total={required} filled={submitted} size={40} color={submitted === required ? 'green' : undefined}> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider defining a constant for the CircularProgress size.
The value 40 is now hardcoded directly in the component. While the AI summary indicates the matchIndicatorSize constant was intentionally removed, hardcoding this value makes future maintenance harder if the size needs to change. Consider defining a local constant such as MATCH_INDICATOR_SIZE = 40 at the top of the file for better maintainability.
🔎 Proposed refactor
Add a constant at the top of the file:
import styles from './TournamentPairingsCard.module.scss';
+
+const MATCH_INDICATOR_SIZE = 40;
export const getTournamentPairingTableConfig = (navigate: NavigateFunction): ColumnDef<TournamentPairing>[] => [Then use the constant:
- <CircularProgress total={required} filled={submitted} size={40} color={submitted === required ? 'green' : undefined}>
+ <CircularProgress total={required} filled={submitted} size={MATCH_INDICATOR_SIZE} color={submitted === required ? 'green' : undefined}>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/pages/TournamentDetailPage/components/TournamentPairingsCard/TournamentPairingsCard.utils.tsx
around line 41, the CircularProgress size is hardcoded to 40; define a local
constant (e.g., MATCH_INDICATOR_SIZE = 40) near the top of the file and replace
the literal 40 with that constant in the component so future size changes
require updating a single, named value.
Comment on lines
+1
to
+14
| /* eslint-disable @typescript-eslint/no-unused-vars */ | ||
| import { GameSystem } from '@ianpaschal/combat-command-game-systems/common'; | ||
| import { ForceDiagram, Unit } from '@ianpaschal/combat-command-game-systems/flamesOfWarV4'; | ||
| import { Infer, v } from 'convex/values'; | ||
| import { customAlphabet } from 'nanoid'; | ||
|
|
||
| import { ActionCtx, MutationCtx } from '../../../_generated/server'; | ||
| import { getStaticEnumConvexValidator } from '../../common/_helpers/getStaticEnumConvexValidator'; | ||
|
|
||
| const forceDiagram = getStaticEnumConvexValidator(ForceDiagram); | ||
| const formation = getStaticEnumConvexValidator(Unit); | ||
|
|
||
| const nanoid = customAlphabet('1234567890abcdefghijklmnopqrstuvwxyz', 6); | ||
|
|
There was a problem hiding this comment.
Stub implementation with unused imports.
This file appears to be a placeholder with several unused imports (ForceDiagram, Unit, nanoid, validators). The eslint-disable directive suppresses warnings, but these unused dependencies add unnecessary overhead.
If this is intentional scaffolding for future work, consider opening a follow-up issue to track the complete implementation. Would you like me to help generate a proper implementation based on the commented code below?
🤖 Prompt for AI Agents
In convex/_model/lists/actions/extractListData.ts lines 1-14: the file is a stub
with multiple unused imports (ForceDiagram, Unit, nanoid, validators) and an
unnecessary eslint-disable; remove the unused imports and the eslint-disable
directive, and either implement the intended extractListData logic or add a
clear TODO and open a follow-up issue tracking the required implementation so
the file no longer contains dead code. Ensure exports/typing remain correct
after cleanup and run the linter to confirm no unused symbols remain.
Comment on lines
+20
to
+32
| export const extractListData = async ( | ||
| ctx: ActionCtx, | ||
| args: Infer<typeof extractListDataArgs>, | ||
| ): Promise<{ fileType: string; data: Record<string, string> }> => | ||
|
|
||
| // const fileMetadata = ctx.runQuery(); | ||
| ({ | ||
| fileType: 'foo', | ||
| data: { | ||
| foo: 'foo', | ||
| }, | ||
| }) | ||
| ; |
There was a problem hiding this comment.
Critical: Stub implementation returns placeholder data.
The extractListData action returns hardcoded placeholder values (fileType: 'foo', data: { foo: 'foo' }). This will break any functionality that depends on real list data extraction.
The commented line 25 suggests a query should be executed. Please implement the actual data extraction logic before merging.
🔍 Verify usage of this action
#!/bin/bash
# Description: Find all usages of extractListData to assess impact
# Search for extractListData imports and calls
rg -n --type=ts --type=tsx 'extractListData' -A 3 -B 1🤖 Prompt for AI Agents
In convex/_model/lists/actions/extractListData.ts around lines 20 to 32, the
function currently returns hardcoded placeholder values; replace the stub with
real extraction logic: call the appropriate ctx.runQuery or ctx.db query
(uncomment/use the suggested query) using the provided args to fetch the
file/list metadata and contents, parse and map the result to return an object
with the actual fileType string and a data record (Record<string,string>)
derived from the fetched fields, validate and sanitize values, and propagate or
log errors instead of returning placeholders so callers receive real extracted
data.
Comment on lines
+34
to
+49
| // gameSystem: GameSystem.FlamesOfWarV4, | ||
| // tournamentRegistrationId: | ||
| // // ownerUserId: row.playerUserId, | ||
| // // data: { | ||
| // // meta: { | ||
| // // forceDiagram: row.forceDiagram, | ||
| // // pointsLimit: args.pointsLimit, | ||
| // // }, | ||
| // // formations: row.formations.map((sourceId) => ({ | ||
| // // id: nanoid(), | ||
| // // sourceId, | ||
| // // })), | ||
| // // units: [], | ||
| // // commandCards: [], | ||
| // // }, | ||
| // // locked: args.locked, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove or implement commented code.
This large block of commented code suggests the intended implementation but reduces code maintainability.
Consider either:
- Implementing the actual logic if this is the correct approach
- Moving this to a TODO comment or separate documentation
- Removing it if it's no longer relevant
🔎 Suggested cleanup
-// gameSystem: GameSystem.FlamesOfWarV4,
-// tournamentRegistrationId:
-// // ownerUserId: row.playerUserId,
-// // data: {
-// // meta: {
-// // forceDiagram: row.forceDiagram,
-// // pointsLimit: args.pointsLimit,
-// // },
-// // formations: row.formations.map((sourceId) => ({
-// // id: nanoid(),
-// // sourceId,
-// // })),
-// // units: [],
-// // commandCards: [],
-// // },
-// // locked: args.locked,
+// TODO: Implement data extraction logic
+// - Query file metadata using ctx.runQuery()
+// - Parse list data based on gameSystem
+// - Return structured data with appropriate fileType📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // gameSystem: GameSystem.FlamesOfWarV4, | |
| // tournamentRegistrationId: | |
| // // ownerUserId: row.playerUserId, | |
| // // data: { | |
| // // meta: { | |
| // // forceDiagram: row.forceDiagram, | |
| // // pointsLimit: args.pointsLimit, | |
| // // }, | |
| // // formations: row.formations.map((sourceId) => ({ | |
| // // id: nanoid(), | |
| // // sourceId, | |
| // // })), | |
| // // units: [], | |
| // // commandCards: [], | |
| // // }, | |
| // // locked: args.locked, | |
| // TODO: Implement data extraction logic | |
| // - Query file metadata using ctx.runQuery() | |
| // - Parse list data based on gameSystem | |
| // - Return structured data with appropriate fileType |
🤖 Prompt for AI Agents
In convex/_model/lists/actions/extractListData.ts around lines 34 to 49, there
is a large block of commented-out code that either belongs as implemented logic
or should be removed/moved to a TODO; remove or restore it to keep the codebase
clean. Decide whether this block represents required behavior (e.g., populating
gameSystem, tournamentRegistrationId, ownerUserId, data.meta, data.formations,
data.units, data.commandCards, locked) — if so, implement that logic inline
using existing helpers and types and replace the comments with working code;
otherwise delete the commented block or convert it to a single TODO comment with
a short rationale and a ticket/PR reference. Ensure any newly implemented fields
conform to the surrounding function signature and tests, and run
linting/formatting after the change.
Comment on lines
+15
to
+20
| export type IndicatorSide = 'top' | 'right' | 'bottom' | 'left'; | ||
| export type IndicatorState = { | ||
| visible?: boolean; | ||
| border?: boolean; | ||
| }; | ||
| export type IndicatorConfig = Partial<Record<IndicatorSide, IndicatorState>>; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the file exists and read the relevant sections
echo "=== Checking ScrollArea.hooks.tsx file ==="
if [ -f "src/components/generic/ScrollArea/ScrollArea.hooks.tsx" ]; then
echo "File exists. Reading lines 1-60 to see type definitions and hook signature:"
head -60 "src/components/generic/ScrollArea/ScrollArea.hooks.tsx" | cat -n
else
echo "File not found at expected path"
echo "Searching for ScrollArea files:"
find . -name "*ScrollArea*" -type f 2>/dev/null | head -20
fiRepository: ianpaschal/combat-command
Length of output: 2269
🏁 Script executed:
#!/bin/bash
# Search for all useScrollIndicators calls throughout the codebase
echo "=== Finding all useScrollIndicators call sites ==="
rg -n 'useScrollIndicators' --type=ts --type=tsx -B 2 -A 3Repository: ianpaschal/combat-command
Length of output: 164
🏁 Script executed:
#!/bin/bash
# Search for all useScrollIndicators calls - simpler approach
echo "=== Finding all useScrollIndicators call sites ==="
rg -n 'useScrollIndicators' -A 3 -B 2Repository: ianpaschal/combat-command
Length of output: 2746
🏁 Script executed:
#!/bin/bash
# Read the rest of the file to see the hook definition
echo "=== Reading useScrollIndicators hook definition ==="
sed -n '70,140p' "src/components/generic/ScrollArea/ScrollArea.hooks.tsx" | cat -nRepository: ianpaschal/combat-command
Length of output: 2637
🏁 Script executed:
#!/bin/bash
# Read the JSDoc comment for the hook to check if it still references old param name
echo "=== Reading hook JSDoc (lines 88-95) ==="
sed -n '88,95p' "src/components/generic/ScrollArea/ScrollArea.hooks.tsx" | cat -nRepository: ianpaschal/combat-command
Length of output: 615
🏁 Script executed:
#!/bin/bash
# Check if there are any imports of useScrollIndicators to see all potential call sites
echo "=== Finding all imports of useScrollIndicators ==="
rg -n 'import.*useScrollIndicators|from.*ScrollArea' --type=ts --type=tsx -iRepository: ianpaschal/combat-command
Length of output: 164
🏁 Script executed:
#!/bin/bash
# Search for any remaining old-style string-based borders parameter usage
echo "=== Checking for old API patterns (string borders) ==="
rg -n "borders.*=.*['\"]" | grep -i scrollRepository: ianpaschal/combat-command
Length of output: 123
🏁 Script executed:
#!/bin/bash
# Try simpler import search without type filters
echo "=== Finding all useScrollIndicators imports ==="
rg 'useScrollIndicators' | grep -E 'import|from'Repository: ianpaschal/combat-command
Length of output: 253
🏁 Script executed:
#!/bin/bash
# Get a better count of where useScrollIndicators is used across the codebase
echo "=== Summary of useScrollIndicators usage ==="
rg 'useScrollIndicators' --count-matchesRepository: ianpaschal/combat-command
Length of output: 268
🏁 Script executed:
#!/bin/bash
# Check the ScrollArea component to understand how indicatorConfig is passed
echo "=== ScrollArea.tsx component signature to see indicatorConfig prop ==="
sed -n '1,35p' "src/components/generic/ScrollArea/ScrollArea.tsx" | cat -nRepository: ianpaschal/combat-command
Length of output: 1386
Update JSDoc to reflect current parameter name.
The useScrollIndicators hook API refactor from string-based borders to structured IndicatorConfig has been properly migrated. The single call site in ScrollArea.tsx (line 33) correctly uses the new config-based API.
However, the JSDoc comment at lines 88-93 still references the old parameter name @param borders when it should be @param config to match the actual function signature.
/**
* A hook which enables the rendering of gradient and bordered scroll indicators on the edges of a scroll viewport.
* @param config - Configuration object for indicator visibility and borders per side.
* @returns A object containing a viewport ref, onScroll handler, and four sides' indicator elements.
*/
🤖 Prompt for AI Agents
In src/components/generic/ScrollArea/ScrollArea.hooks.tsx around lines 88 to 93,
the JSDoc for useScrollIndicators still references the old parameter name
`borders`; update the doc to match the function signature by replacing `@param
borders` with `@param config` and adjust the description to mention the
IndicatorConfig structure (per-side visibility and border options) so the
comment accurately reflects the current API.
Comment on lines
+51
to
+54
| // TODO: Maybe someday turn these into form fields? | ||
| const [storageId, setStorageId] = useState<StorageId | null>(null); | ||
| const [fileType, setFileType] = useState<string | null>(null); | ||
| const [url, setUrl] = useState<string>(''); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Address the TODO comment or track it as a separate issue.
The TODO suggests converting these state variables into form fields. Consider whether this refactoring should be done now or tracked as a separate issue.
Do you want me to open a new issue to track this technical debt?
Comment on lines
+33
to
+40
| if (tournamentRegistration.lists.length > 1) { | ||
| return ( | ||
| <Button className={clsx(styles.TournamentRegistrationListButton, className)} text="View List" /> | ||
| ); | ||
| } | ||
| return ( | ||
| <Button className={clsx(styles.TournamentRegistrationListButton, className)} text="View List" /> | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove duplicate code branches.
Both the lists.length > 1 and the default branch render identical "View List" buttons. This suggests incomplete implementation or unnecessary branching.
🔎 Proposed fix
If the behavior should be the same for all non-zero list counts:
if (tournamentRegistration.lists.length === 0) {
return (
<div className={clsx(styles.TournamentRegistrationListButton, className)}>
<Badge value={isOverdue ? 'Late' : undefined} intent="danger">
<Button text="List" icon={<FilePlusCorner />} />
</Badge>
</div>
);
}
- if (tournamentRegistration.lists.length > 1) {
- return (
- <Button className={clsx(styles.TournamentRegistrationListButton, className)} text="View List" />
- );
- }
return (
<Button className={clsx(styles.TournamentRegistrationListButton, className)} text="View List" />
);Or if multiple lists should show different text:
- if (tournamentRegistration.lists.length > 1) {
- return (
- <Button className={clsx(styles.TournamentRegistrationListButton, className)} text="View List" />
- );
- }
return (
- <Button className={clsx(styles.TournamentRegistrationListButton, className)} text="View List" />
+ <Button
+ className={clsx(styles.TournamentRegistrationListButton, className)}
+ text={tournamentRegistration.lists.length > 1 ? "View Lists" : "View List"}
+ />
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (tournamentRegistration.lists.length > 1) { | |
| return ( | |
| <Button className={clsx(styles.TournamentRegistrationListButton, className)} text="View List" /> | |
| ); | |
| } | |
| return ( | |
| <Button className={clsx(styles.TournamentRegistrationListButton, className)} text="View List" /> | |
| ); | |
| if (tournamentRegistration.lists.length === 0) { | |
| return ( | |
| <div className={clsx(styles.TournamentRegistrationListButton, className)}> | |
| <Badge value={isOverdue ? 'Late' : undefined} intent="danger"> | |
| <Button text="List" icon={<FilePlusCorner />} /> | |
| </Badge> | |
| </div> | |
| ); | |
| } | |
| return ( | |
| <Button className={clsx(styles.TournamentRegistrationListButton, className)} text="View List" /> | |
| ); |
Suggested change
| if (tournamentRegistration.lists.length > 1) { | |
| return ( | |
| <Button className={clsx(styles.TournamentRegistrationListButton, className)} text="View List" /> | |
| ); | |
| } | |
| return ( | |
| <Button className={clsx(styles.TournamentRegistrationListButton, className)} text="View List" /> | |
| ); | |
| if (tournamentRegistration.lists.length === 0) { | |
| return ( | |
| <div className={clsx(styles.TournamentRegistrationListButton, className)}> | |
| <Badge value={isOverdue ? 'Late' : undefined} intent="danger"> | |
| <Button text="List" icon={<FilePlusCorner />} /> | |
| </Badge> | |
| </div> | |
| ); | |
| } | |
| return ( | |
| <Button | |
| className={clsx(styles.TournamentRegistrationListButton, className)} | |
| text={tournamentRegistration.lists.length > 1 ? "View Lists" : "View List"} | |
| /> | |
| ); |
🤖 Prompt for AI Agents
In
src/pages/TournamentCompetitorDetailPage/components/TournamentRegistrationListButton/TournamentRegistrationListButton.tsx
around lines 33 to 40, the two conditional branches both render the identical
"View List" button; remove the redundant conditional and consolidate to a single
return that renders the Button with
clsx(styles.TournamentRegistrationListButton, className) and text="View List".
If the original intent was to vary behavior for multiple lists, replace the
duplicated branch with the appropriate differing text/props for lists.length > 1
instead of duplicating the same JSX.
Comment on lines
+53
to
+57
| &_Actions { | ||
| grid-area: actions; | ||
| align-self: center; | ||
| justify-self: start; | ||
| } |
There was a problem hiding this comment.
Critical: Duplicate _Actions selector.
The _Actions selector is defined twice with different styles:
- Lines 53-57: Defines
grid-area: actionsfor header actions - Lines 131-137: Defines
flex.rowwith top border for page actions
The second definition will override the first, breaking the grid layout. These appear to serve different purposes and should have distinct names.
🔎 Proposed fix
&_Actions {
grid-area: actions;
align-self: center;
justify-self: start;
}
// ... other selectors ...
- &_Actions {
+ &_PageActions {
@include flex.row($xAlign: right);
@include borders.normal($side: top);
width: 100%;
padding: 1rem var(--container-padding-x);
}Also applies to: 131-137
Comment on lines
+140
to
+156
| @media (width >= 480px) { | ||
| .TournamentCompetitorPage { | ||
| --ranking-size: var(--avatar-size); | ||
|
|
||
| &_Name { | ||
| @include flex.row; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @media (width >= 688px) { | ||
| .TournamentCompetitorPage { | ||
| &_Name { | ||
| @include flex.row; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove redundant media query rule.
The _Name { @include flex.row; } rule appears in both the 480px and 688px media queries. Since devices ≥688px will match the 480px query, the second declaration is redundant.
🔎 Proposed cleanup
@media (width >= 480px) {
.TournamentCompetitorPage {
--ranking-size: var(--avatar-size);
&_Name {
@include flex.row;
}
}
}
@media (width >= 688px) {
.TournamentCompetitorPage {
- &_Name {
- @include flex.row;
- }
+ // Additional styles for larger viewports can go here
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @media (width >= 480px) { | |
| .TournamentCompetitorPage { | |
| --ranking-size: var(--avatar-size); | |
| &_Name { | |
| @include flex.row; | |
| } | |
| } | |
| } | |
| @media (width >= 688px) { | |
| .TournamentCompetitorPage { | |
| &_Name { | |
| @include flex.row; | |
| } | |
| } | |
| } | |
| @media (width >= 480px) { | |
| .TournamentCompetitorPage { | |
| --ranking-size: var(--avatar-size); | |
| &_Name { | |
| @include flex.row; | |
| } | |
| } | |
| } | |
| @media (width >= 688px) { | |
| .TournamentCompetitorPage { | |
| // Additional styles for larger viewports can go here | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/pages/TournamentCompetitorDetailPage/TournamentCompetitorDetailPage.module.scss
around lines 140 to 156, there is a duplicate rule ".TournamentCompetitorPage
&_Name { @include flex.row; }" inside both the 480px and 688px media queries;
remove the redundant declaration from the 688px media query (or consolidate
breakpoint logic by keeping a single rule in the smallest applicable media
query) so the styling is defined only once for devices ≥480px.
Comment on lines
+66
to
+68
| if (isLoading) { | ||
| return <div>Loading...</div>; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Replace simple loading div with a proper loading component.
The loading state returns a simple <div>Loading...</div> instead of a consistent loading component or skeleton. This creates an inconsistent user experience.
🔎 Suggested improvement
if (isLoading) {
- return <div>Loading...</div>;
+ return (
+ <PageWrapper maxWidth={640} showBackButton fitToWindow>
+ <div className={clsx(styles.TournamentCompetitorPage, className)}>
+ {/* Add skeleton or loading spinner component here */}
+ </div>
+ </PageWrapper>
+ );
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/pages/TournamentCompetitorDetailPage/TournamentCompetitorDetailPage.tsx
around lines 66 to 68, the component currently returns a bare
<div>Loading...</div>; replace this with the app's standard loading UI by
importing and using the shared Loading component or page skeleton (e.g.,
LoadingSpinner, FullPageSkeleton, or similar) so the UX is consistent. Update
the imports at the top to include the shared loader, replace the return with
that component (including any required props like size/centered or aria-label),
and ensure styling/placement matches surrounding pages.
| return <NotFoundView />; | ||
| } | ||
|
|
||
| // const showJoinButton = tournamentCompetitor.availableActions.includes(TournamentCompetitorActionKey.Join); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify the status of commented code.
The showJoinButton logic is commented out. If this feature is intentionally deferred, add a TODO comment explaining when/why it should be enabled. Otherwise, remove the commented code.
🤖 Prompt for AI Agents
In src/pages/TournamentCompetitorDetailPage/TournamentCompetitorDetailPage.tsx
around line 74, the showJoinButton calculation is commented out; either restore
it or clarify intent. If the join-button feature is planned later, replace the
commented line with a one-line TODO explaining when/why to re-enable (e.g.,
reference ticket/condition and expected behavior); otherwise delete the
commented code to remove dead code. Ensure the TODO uses standard format (TODO:
<ticket/condition> - brief reason) if keeping, or simply remove the line if the
logic is no longer needed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 #266
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.