Feature/tournament component - knockout stage#919
Open
SzBeni2003 wants to merge 106 commits into
Open
Conversation
NOT TESTED!
NOT TESTED!
…to feature/tournament-component # Conflicts: # backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentApiController.kt # backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentDetailedView.kt # backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentMatchEntity.kt # frontend/src/pages/tournament/components/Tournament.tsx # frontend/src/util/paths.ts # frontend/src/util/views/tournament.view.ts
# Conflicts: # backend/src/main/kotlin/hu/bme/sch/cmsch/config/ComponentLoadConfig.kt # backend/src/main/kotlin/hu/bme/sch/cmsch/controller/admin/OneDeepEntityPage.kt # frontend/src/App.tsx # frontend/src/api/contexts/config/types.ts # frontend/src/api/hooks/queryKeys.ts # frontend/src/util/paths.ts # helm/cmsch/templates/cmsch-config.yml # helm/cmsch/values.yaml
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis PR adds an end-to-end tournament feature with backend domain models, persistence, services, API/admin workflows, permissions, configuration, templates, seeded data, and a frontend tournament listing, detail view, registration flow, and bracket rendering. ChangesTournament Feature
Estimated code review effort: 4 (Complex) | ~75 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (13)
backend/src/main/resources/templates/matchAdmin.html-63-63 (1)
63-63: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMissing
</html>closing tag.The file ends with
</body>but never closes the<html>element opened on line 2.🔧 Proposed fix
</body> +</html>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/main/resources/templates/matchAdmin.html` at line 63, The template ends after closing the body but never closes the root html element. Update matchAdmin.html so the markup opened in the main template is properly terminated with a closing html tag after the existing body close, ensuring the document structure is complete.Source: Linters/SAST tools
backend/src/main/resources/templates/matchScore.html-33-33 (1)
33-33: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winInvalid CSS
box-shadowvalue — missingpxunit.
box-shadow: 5 4px 16px rgba(0,0,0,0.3)— the5lacks a length unit, making the entire declaration invalid. The popup renders without a shadow.🎨 Proposed fix
- box-shadow: 5 4px 16px rgba(0,0,0,0.3); + box-shadow: 5px 4px 16px rgba(0,0,0,0.3);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/main/resources/templates/matchScore.html` at line 33, The `box-shadow` declaration in the `matchScore.html` template is invalid because the first offset value is missing a length unit. Update the CSS in the popup styling so the `box-shadow` values are all valid lengths, using the existing shadow declaration near the popup styles in `matchScore.html`.backend/src/main/resources/templates/matchScore.html-116-136 (1)
116-136: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGuard the action buttons by match status
TournamentMatchController.scoreMatchPagesetsreadOnly = false, so a finished match still renders these buttons even though the popup is omitted; theonclickhandlers then hit missing elements. Apply the sameNOT_STARTED/IN_PROGRESScondition to the three<object>blocks.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/main/resources/templates/matchScore.html` around lines 116 - 136, The action buttons in matchScore.html are only hidden by readOnly, so finished matches still render the forfeit/save controls even though the score popup inputs are missing. Update the three <object> blocks around the away-forfeit, home-forfeit, and save-button elements to use the same match-status guard as the popup rendering, keyed off the scoreMatchPage flow and the NOT_STARTED/IN_PROGRESS states, so these buttons only appear when the score inputs exist.frontend/src/pages/tournament/components/Match.tsx-10-10 (1)
10-10: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTie scores render red for both sides.
When
score1 === score2,score1 > score2isfalse, so both participants get the losing color. A draw should use a neutral color.🐛 Proposed fix
- return score1 > score2 ? 'green.600' : 'red.600' + return score1 > score2 ? 'green.600' : score1 === score2 ? 'gray.600' : 'red.600'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/tournament/components/Match.tsx` at line 10, The score color logic in the Match component treats ties as losses because the current comparison only checks whether one score is greater than the other. Update the helper or inline ternary in Match.tsx so equal scores map to a neutral color instead of the losing color, while preserving the existing winning and losing colors for non-tie cases.frontend/src/pages/tournament/components/Tournament.tsx-112-112 (1)
112-112: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd
keyprop toCustomTabButtonin the stages map.Rendering
CustomTabButtoninside.map()without akeyprop triggers a React warning and can cause incorrect reconciliation if the stages list reorders.🐛 Proposed fix — add key prop
{tournament.stages.map((stage, index) => ( - <CustomTabButton value={`stage-${index}`}>{stage.name}</CustomTabButton> + <CustomTabButton key={stage.id} value={`stage-${index}`}>{stage.name}</CustomTabButton> ))}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/tournament/components/Tournament.tsx` at line 112, The stage tabs rendered in the stages .map() are missing a React key, which can cause reconciliation issues. Update the CustomTabButton usage in Tournament to pass a stable key prop alongside value, using the stage item’s unique identifier if available or the index only as a fallback. Keep the change localized to the mapped CustomTabButton element so React can track each stage tab correctly.backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageEntity.kt-117-124 (1)
117-124: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
equals()treats two unsaved entities (id=0) as equal.Unlike
TournamentEntity.equals()which guards withid != 0 && id == other.id, this implementation only checksid != other.id. Two distinct unsavedTournamentStageEntityinstances (both withid = 0) would be considered equal, which can cause subtle bugs inSet/Mapoperations before persistence.🐛 Proposed fix — add id != 0 guard consistent with TournamentEntity
override fun equals(other: Any?): Boolean { if (this === other) return true if (other !is TournamentStageEntity) return false - if (id != other.id) return false - - return true + return id != 0 && id == other.id }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageEntity.kt` around lines 117 - 124, `TournamentStageEntity.equals()` currently treats two new entities with `id = 0` as equal; update the equality check to match the guard used in `TournamentEntity.equals()` so only persisted entities can compare equal by id. In the `TournamentStageEntity` class, modify the `equals` implementation to require a non-zero id before comparing ids, keeping the existing `this === other` and type checks intact.backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentMatchEntity.kt-82-87 (1)
82-87: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
kickoffTimetype contradicts@Column(nullable = true).The field is
Long(non-nullable) but the column is declarednullable = true. If the database ever storesNULL, Hibernate will throw aNullPointerExceptionwhen loading the entity. Either make the fieldLong?or change the column tonullable = false.🔧 Proposed fix — make the field nullable to match the column
- var kickoffTime: Long = 0, + var kickoffTime: Long? = null,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentMatchEntity.kt` around lines 82 - 87, The kickoffTime mapping in TournamentMatchEntity is inconsistent because the JPA column allows NULL while the Kotlin property is non-nullable. Update the TournamentMatchEntity.kickoffTime field to match the database nullability by making it nullable, or alternatively make the column non-nullable if NULL should never be stored; keep the annotations and property type aligned so Hibernate can load the entity safely.backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageEntity.kt-109-109 (1)
109-109: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGuard
rounds()against non-positiveparticipantCount.
log2(0.0)returns-Infinityandlog2of a negative number returnsNaN;ceilof either yieldsNaN/-Infinity, and.toInt()coerces to0. While the UI enforcesmin = 1, there is no runtime or DB constraint preventingparticipantCount = 0, which would silently produce0rounds instead of an error.🛡️ Proposed fix — add a guard clause
- fun rounds() = ceil(log2(participantCount.toDouble())).toInt() + fun rounds(): Int { + require(participantCount > 0) { "participantCount must be positive, was $participantCount" } + return ceil(log2(participantCount.toDouble())).toInt() + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageEntity.kt` at line 109, Guard TournamentStageEntity.rounds() against non-positive participantCount by adding an explicit check before calling log2/ceil. If participantCount is 0 or negative, fail fast with a clear error or otherwise handle it intentionally instead of letting the calculation silently return 0. Keep the fix localized to rounds() and preserve the existing behavior for valid positive counts.backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageController.kt-298-300 (1)
298-300: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winError message is lost on redirect.
model.addAttribute("error", ...)followed by aredirect:discards the attribute (model does not survive a redirect). The seed page will render without the error. UseRedirectAttributes.addFlashAttributeinstead. The same pattern recurs at Lines 326-327 and 342-345.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageController.kt` around lines 298 - 300, The error message is being added to the regular model before a redirect in TournamentStageController, so it is lost after navigation. Update the affected redirect paths in the seed-related methods to use RedirectAttributes.addFlashAttribute instead of model.addAttribute for the "error" message, including the repeated cases in the same controller. Make sure the methods handling the seed edit/save redirects keep the flash attribute so the error is available on the redirected page.backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageController.kt-318-322 (1)
318-322: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
stageStatusparam can NPE on malformed requests.
allRequestParams["stageStatus"]!!throws aNullPointerException(500) if the parameter is absent. Handle the missing case gracefully like the other validated inputs.🛡️ Proposed fix
- val stageStatus: StageStatus = allRequestParams["stageStatus"]!!.let { when (it) { - "CREATED" -> StageStatus.CREATED - "SET" -> StageStatus.SET - else -> return "error" - }} + val stageStatus: StageStatus = when (allRequestParams["stageStatus"]) { + "CREATED" -> StageStatus.CREATED + "SET" -> StageStatus.SET + else -> return "error" + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageController.kt` around lines 318 - 322, The stageStatus parsing in TournamentStageController can throw a NullPointerException because allRequestParams["stageStatus"]!! assumes the parameter is always present. Update the validation flow around stageStatus to handle a missing value gracefully, similar to the other checked request params, and keep the existing mapping to StageStatus.CREATED and StageStatus.SET while returning the error path for absent or invalid input.backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageService.kt-244-248 (1)
244-248: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFiltered result is discarded; the time-window/status filter is a no-op.
matches.filter { ... }.filter { ... }returns a new list that is never assigned, so Line 248 returns the full unfilteredmatches. The status andcloseMatchesTimeWindowfiltering has no effect. (Function is currently unused per the comment, but the logic is still broken.)🐛 Proposed fix
- matches.filter { it.status in listOf(MatchStatus.IN_PROGRESS, MatchStatus.NOT_STARTED) } - .filter { (it.kickoffTime - clock.getTime()) in -timeFrame..timeFrame } - - return matches.sortedBy { it.kickoffTime } + return matches + .filter { it.status in listOf(MatchStatus.IN_PROGRESS, MatchStatus.NOT_STARTED) } + .filter { (it.kickoffTime - clock.getTime()) in -timeFrame..timeFrame } + .sortedBy { it.kickoffTime }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageService.kt` around lines 244 - 248, The time-window and status filtering in TournamentStageService’s match selection is currently a no-op because the chained filter calls discard their result before the return. Update the logic in the match-sorting/filtering method so the filtered list is actually used, applying both the MatchStatus and closeMatchesTimeWindow conditions before sorting and returning. Keep the existing TournamentStageService method structure, but ensure the final returned collection reflects the filtered matches rather than the original matches list.backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentService.kt-81-86 (1)
81-86: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign group joinability with
teamRegisterPrivileged users still satisfyjoinChangableforOwnershipType.GROUP, butteamRegisterreturnsINSUFFICIENT_PERMISSIONS, so the detail view can expose a join action that always fails.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentService.kt` around lines 81 - 86, Align the joinability check in TournamentService’s joinable logic with the teamRegister permission rules so the detail view never offers a group join action that will fail. Update the `joinChangable` condition to match `teamRegister` for `OwnershipType.GROUP`, ensuring only users who can actually register a group are counted as joinable; keep the existing participant limit, deadline, and joinable checks intact.backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentApiController.kt-46-64 (1)
46-64: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winAdd the tournament access guard to registration endpoints.
register/unregisteronly require an authenticated user; they bypass theminRole/showTournamentsAtAllchecks used by the read endpoints, so restricted or hidden tournaments can still be joined directly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentApiController.kt` around lines 46 - 64, The registerTeam and unregisterTeam endpoints in TournamentApiController bypass the same tournament access checks used by the read endpoints, so restricted or hidden tournaments can still be joined. Update these endpoints to validate the tournament with the existing access guard logic (the minRole/showTournamentsAtAll checks used for tournament reads) before calling tournamentService.register or tournamentService.unregister, and return an appropriate forbidden/insufficient-permissions status when access is denied.
🧹 Nitpick comments (9)
backend/src/main/resources/templates/matchScore.html (1)
67-70: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove commented-out code.
The commented-out
<object>block for tournament title display should be removed or uncommented if needed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/main/resources/templates/matchScore.html` around lines 67 - 70, The commented-out tournament title block in the matchScore template should be removed unless it is meant to be used. Clean up the template by deleting the unused <object> section around the tournament title display, or uncommenting it if the UI still needs it; use the surrounding label and th:text binding to locate the block.frontend/src/util/paths.ts (1)
29-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNaming inconsistency:
Paths.TOURNAMENT(singular) vsAbsolutePaths.TOURNAMENTS/ApiPaths.TOURNAMENTS(plural).Other entries in this file maintain consistent plurality between
PathsandAbsolutePaths(e.g.,TEAMS/TEAMS,TOKEN/TOKEN). The singularPaths.TOURNAMENTpaired with pluralAbsolutePaths.TOURNAMENTSandApiPaths.TOURNAMENTSbreaks this convention and could cause confusion.Also applies to: 63-63, 112-112
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/util/paths.ts` at line 29, `Paths.TOURNAMENT` is inconsistent with the plural naming used in `AbsolutePaths.TOURNAMENTS` and `ApiPaths.TOURNAMENTS`; align the `Paths` enum/member name with the other route constants. Update the related route symbols in `paths.ts` so the same plurality is used across `Paths`, `AbsolutePaths`, and `ApiPaths`, and make sure any references to the tournament path constant are renamed consistently.frontend/src/pages/tournament/tournamentList.page.tsx (1)
22-32: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse
<Link>for tournament items
frontend/src/pages/tournament/tournamentList.page.tsx: replace the raw<a href>withLinkso tournament navigation stays in-app and avoids a full reload. This page is inconsistent with the other list pages.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/tournament/tournamentList.page.tsx` around lines 22 - 32, The tournament list item navigation is using a raw anchor tag, which causes a full page reload instead of in-app routing. Update the tournament item link in tournamentList.page.tsx to use the existing Link component used elsewhere in the app, keeping the same destination built from AbsolutePaths.TOURNAMENTS and tournament.id. Make sure the tournament title still renders as the clickable label and that the surrounding list/map structure remains unchanged.frontend/src/pages/tournament/components/KnockoutStage.tsx (1)
7-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer standard
import typeoverimport()type expressions.Using
import('...').Typeis valid but non-idiomatic and harder to read. Standardimport typeis clearer and consistent with the rest of the codebase.♻️ Proposed refactor
-type TournamentStageView = import('../../../util/views/tournament.view').TournamentStageView -type MatchView = import('../../../util/views/tournament.view').MatchView +import type { TournamentStageView, MatchView } from '../../../util/views/tournament.view'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/tournament/components/KnockoutStage.tsx` around lines 7 - 8, Replace the `import('...').Type` aliases in `KnockoutStage` with standard `import type` imports. Update the `TournamentStageView` and `MatchView` references to use a proper type-only import from `tournament.view` so the declarations remain clear, consistent, and idiomatic with the rest of the codebase.frontend/src/pages/tournament/components/Tournament.tsx (2)
50-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove redundant double
refetch()calls.Both
joinActionResponseCallback(line 36) andcancelActionResponseCallback(line 44) already callrefetch()when the response isOK. TheonSuccesscallbacks injoinTournament(lines 55-57) andcancelJoinTournament(lines 70-72) callrefetch()again for the same condition, resulting in duplicate invocations.♻️ Proposed fix — remove redundant refetch from onSuccess callbacks
const joinTournament = () => { if (tournament.tournament.joinEnabled) { joinMutation.mutate(tournament.tournament.id, { onSuccess: (response: TournamentJoinResponses) => { joinActionResponseCallback(response) - if (response === TournamentJoinResponses.OK) { - refetch() - } }, onError: () => { toast({ variant: 'destructive', title: 'Hiba történt a versenyre való jelentkezés során.' }) } }) } } const cancelJoinTournament = () => { if (tournament.tournament.joinCancellable) { cancelMutation.mutate(tournament.tournament.id, { onSuccess: (response: TournamentCancelResponses) => { cancelActionResponseCallback(response) - if (response === TournamentCancelResponses.OK) { - refetch() - } }, onError: () => { toast({ variant: 'destructive', title: 'Hiba történt a versenyre való jelentkezés visszavonása során.' }) } }) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/tournament/components/Tournament.tsx` around lines 50 - 79, The `joinTournament` and `cancelJoinTournament` handlers in `Tournament.tsx` are triggering duplicate refetches because `joinActionResponseCallback` and `cancelActionResponseCallback` already refetch on `TournamentJoinResponses.OK` and `TournamentCancelResponses.OK`. Remove the extra `refetch()` calls from the `onSuccess` branches inside `joinMutation.mutate` and `cancelMutation.mutate`, and let the existing callback functions own the refresh behavior.
128-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
StageTypeconstants instead of string literals.The
StageTypeenum is already imported-adjacent intournament.view.tsbut not used here. Comparing against raw strings'KNOCKOUT'and'STAGE'bypasses TypeScript's type safety — if the enum values ever change, these comparisons silently break.♻️ Proposed fix — use StageType constants
+import { + StageType, + TournamentCancelResponseMessages, + TournamentCancelResponses, + TournamentJoinResponseMessages, + TournamentJoinResponses +} from '`@/util/views/tournament.view.ts`'- {stage.type === 'KNOCKOUT' && <KnockoutStage key={stage.id} stage={stage} />} - {stage.type === 'STAGE' && <GroupStage key={stage.id} stage={stage} />} + {stage.type === StageType.KNOCKOUT && <KnockoutStage key={stage.id} stage={stage} />} + {stage.type === StageType.STAGE && <GroupStage key={stage.id} stage={stage} />}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/tournament/components/Tournament.tsx` around lines 128 - 129, The stage rendering in Tournament.tsx uses raw string comparisons for stage.type, which should be replaced with StageType enum constants for type safety. Update the conditional checks in the Tournament component to compare against the corresponding StageType members instead of 'KNOCKOUT' and 'STAGE', using the existing StageType import so the logic stays aligned with the enum definitions.backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentComponentController.kt (1)
19-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the redundant
MenuServiceinjection.
MenuServiceis injected twice:menuService(used in the super call) andservice(Line 23), which is unused. Drop theserviceparameter.♻️ Proposed fix
private val tournamentService: TournamentService, private val auditLogService: AuditLogService, - private val storageService: StorageService, - service: MenuService + private val storageService: StorageService ) : ComponentApiBase(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentComponentController.kt` around lines 19 - 23, The constructor in TournamentComponentController injects MenuService twice, but only the menuService parameter is used in the super call. Remove the redundant unused service: MenuService parameter from the class constructor and keep the existing menuService injection that is already passed to the parent, ensuring the remaining constructor parameters still match the controller’s dependencies.backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageService.kt (1)
262-274: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCollapse the duplicated BYE branches.
Both the home-winner and away-winner branches do exactly the same thing (set
MatchStatus.BYEand save); only the else needs to remain distinct.♻️ Proposed refactor
- if (winner.teamId == match.homeTeamId) { - match.status = MatchStatus.BYE - matchRepository.save(match) - } else if (winner.teamId == match.awayTeamId) { - match.status = MatchStatus.BYE - matchRepository.save(match) - } else { - // Handle case where winner is not one of the teams in the match - throw IllegalStateException("Winner team ID ${winner.teamId} does not match any team in match ${match.id}.") - } + if (winner.teamId == match.homeTeamId || winner.teamId == match.awayTeamId) { + match.status = MatchStatus.BYE + matchRepository.save(match) + } else { + throw IllegalStateException("Winner team ID ${winner.teamId} does not match any team in match ${match.id}.") + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageService.kt` around lines 262 - 274, The winner-handling logic in TournamentStageService is duplicating the same BYE update for both home and away winners. Refactor the conditional inside the matches loop so that the `winner()` result only checks whether `winner.teamId` matches either `match.homeTeamId` or `match.awayTeamId`, then performs the shared `match.status = MatchStatus.BYE` and `matchRepository.save(match)` once; keep the `IllegalStateException` branch only for the invalid-team case.backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageController.kt (1)
211-219: 🩺 Stability & Availability | 🔵 Trivial | 🏗️ Heavy liftDeriving stage type and tournament id from the
Refererheader is fragile.Both
stageTypeandtournamentIdare parsed out of theReferer, and the mapping requiresheaders = ["Referer"]. Referrer can be stripped by browserReferrer-Policy, proxies, or privacy settings, in which case stage creation silently 404s or redirects. Prefer passingtournamentId/typeas an explicit hidden form field or path variable so creation does not depend on the referer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageController.kt` around lines 211 - 219, The stage creation flow in TournamentStageController currently depends on parsing stageType and tournamentId from the Referer header, which is fragile and can fail when the header is missing or stripped. Update the create handler to accept these values explicitly instead of deriving them from Referer, ideally by using hidden form fields or path/query parameters in the stage creation form and controller method. Keep the StageType mapping logic in TournamentStageController, but read the incoming type and tournament id from the explicit request data so creation no longer depends on header presence.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/StageResultDto.kt`:
- Around line 15-27: StageResultDto.compareTo is comparing different fields for
the null-detailedStats case, which breaks Comparable consistency. Update the
comparison in compareTo so it uses the same property on both sides of the
comparison, likely comparing this.highestSeed with other.highestSeed if that is
the intended ordering. Keep the rest of the ordering logic in
StageResultDto.compareTo and compareValuesBy unchanged, but ensure all branches
compare matching fields to preserve transitivity.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentMatchEntity.kt`:
- Around line 140-152: `TournamentMatchEntity.winner()` contains dead null
checks on `homeSeed` and `awaySeed` even though those fields are non-nullable,
so update the guard to match the real “unset” sentinel used by this model (for
example, check for 0 instead of null) or change the seed properties to nullable
if null is intended. Keep the rest of the winner selection logic unchanged, and
ensure the early-return validation in `winner()` consistently reflects how seeds
and team assignment are represented in `TournamentMatchEntity`.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentService.kt`:
- Around line 152-158: The getResultsInStage method in TournamentService
currently parses every split line without guarding against an empty participants
field, so an empty string can still reach objectMapper.readValue and throw.
Update getResultsInStage to mirror the empty-input handling used by
getParticipants: check stage.get().participants for blank/empty content before
splitting, and filter out empty lines from the split result before mapping to
StageResultDto so only real entries are parsed.
- Around line 285-288: Delete tournament stage matches before removing the
stages, because deleteAllByTournamentId in
TournamentService.deleteStagesForTournament bypasses
TournamentStageController.onEntityDeleted and leaves orphaned match stageId
references. Update deleteStagesForTournament to either reuse the existing stage
deletion path that already clears matches or explicitly remove the associated
matches first, so getAggregatedMatchesByTournamentId no longer encounters
missing stage entries.
- Around line 160-176: The tournament aggregation in
getAggregatedMatchesByTournamentId assumes every aggregated stage still exists,
but deleted stages can leave stale stageId values and cause a null crash. Update
the loop over matchRepository.findAllAggregated() to safely handle missing
entries from stages[aggregatedStage.stageId] instead of forcing a non-null
access, and either skip those aggregates or otherwise account for them before
building the MatchGroupDto list.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageService.kt`:
- Around line 59-76: `transferTeamsForStage` can throw
`IndexOutOfBoundsException` when `getResultsForStage(stage)` returns fewer
entries than `stage.participantCount`, because the `participants[i]` loop always
iterates up to `participantCount`. Fix this by ensuring the list is padded or
bounded before assigning `initialSeed` in
`TournamentStageService.transferTeamsForStage`, so every slot up to
`stage.participantCount` has a `StageResultDto` (including bye placeholders)
before the seeding loop runs.
- Around line 133-149: The bounds check in createRoundMatchesForGroupStageGroup
is inverted, so valid group numbers are rejected while out-of-range values may
pass. Update the require condition to fail only when the requested group exceeds
the stage’s groupCount, and keep the existing error message consistent with that
validation. Use the createRoundMatchesForGroupStageGroup function and the
stage.groupCount/group parameters to locate and correct the check.
In `@backend/src/main/resources/templates/matchScore.html`:
- Line 100: The score fields in matchScore.html are using an unsupported
Thymeleaf attribute, so the values are not rendered. Update the home and away
score elements in the matchScore template to use th:text instead of th:number,
keeping the existing match.homeTeamScore and match.awayTeamScore bindings so the
actual numbers display correctly.
In `@backend/src/main/resources/templates/seedSettings.html`:
- Line 80: The seed settings template currently renders a user-visible
placeholder in the stageLevel>1 branch of the table cell, so update the
th:if/th:text logic in seedSettings.html to remove the literal placeholder from
the “Előző eredmény” column. Either wire this cell to the real previous-result
value in the relevant template binding or hide the column/cell until the feature
is implemented, using the existing stageLevel condition as the locator.
In `@frontend/src/pages/tournament/components/Match.tsx`:
- Around line 8-11: getScoreColor in Match.tsx is returning Chakra semantic
tokens that are then passed into inline style color values, so the browser never
resolves them. Update the score rendering in Match to use Chakra’s color prop
(or another Chakra-aware styling path) instead of style={{ color: ... }}, or
change getScoreColor to return real CSS color values so the completed score text
is actually colored.
---
Minor comments:
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentApiController.kt`:
- Around line 46-64: The registerTeam and unregisterTeam endpoints in
TournamentApiController bypass the same tournament access checks used by the
read endpoints, so restricted or hidden tournaments can still be joined. Update
these endpoints to validate the tournament with the existing access guard logic
(the minRole/showTournamentsAtAll checks used for tournament reads) before
calling tournamentService.register or tournamentService.unregister, and return
an appropriate forbidden/insufficient-permissions status when access is denied.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentMatchEntity.kt`:
- Around line 82-87: The kickoffTime mapping in TournamentMatchEntity is
inconsistent because the JPA column allows NULL while the Kotlin property is
non-nullable. Update the TournamentMatchEntity.kickoffTime field to match the
database nullability by making it nullable, or alternatively make the column
non-nullable if NULL should never be stored; keep the annotations and property
type aligned so Hibernate can load the entity safely.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentService.kt`:
- Around line 81-86: Align the joinability check in TournamentService’s joinable
logic with the teamRegister permission rules so the detail view never offers a
group join action that will fail. Update the `joinChangable` condition to match
`teamRegister` for `OwnershipType.GROUP`, ensuring only users who can actually
register a group are counted as joinable; keep the existing participant limit,
deadline, and joinable checks intact.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageController.kt`:
- Around line 298-300: The error message is being added to the regular model
before a redirect in TournamentStageController, so it is lost after navigation.
Update the affected redirect paths in the seed-related methods to use
RedirectAttributes.addFlashAttribute instead of model.addAttribute for the
"error" message, including the repeated cases in the same controller. Make sure
the methods handling the seed edit/save redirects keep the flash attribute so
the error is available on the redirected page.
- Around line 318-322: The stageStatus parsing in TournamentStageController can
throw a NullPointerException because allRequestParams["stageStatus"]!! assumes
the parameter is always present. Update the validation flow around stageStatus
to handle a missing value gracefully, similar to the other checked request
params, and keep the existing mapping to StageStatus.CREATED and StageStatus.SET
while returning the error path for absent or invalid input.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageEntity.kt`:
- Around line 117-124: `TournamentStageEntity.equals()` currently treats two new
entities with `id = 0` as equal; update the equality check to match the guard
used in `TournamentEntity.equals()` so only persisted entities can compare equal
by id. In the `TournamentStageEntity` class, modify the `equals` implementation
to require a non-zero id before comparing ids, keeping the existing `this ===
other` and type checks intact.
- Line 109: Guard TournamentStageEntity.rounds() against non-positive
participantCount by adding an explicit check before calling log2/ceil. If
participantCount is 0 or negative, fail fast with a clear error or otherwise
handle it intentionally instead of letting the calculation silently return 0.
Keep the fix localized to rounds() and preserve the existing behavior for valid
positive counts.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageService.kt`:
- Around line 244-248: The time-window and status filtering in
TournamentStageService’s match selection is currently a no-op because the
chained filter calls discard their result before the return. Update the logic in
the match-sorting/filtering method so the filtered list is actually used,
applying both the MatchStatus and closeMatchesTimeWindow conditions before
sorting and returning. Keep the existing TournamentStageService method
structure, but ensure the final returned collection reflects the filtered
matches rather than the original matches list.
In `@backend/src/main/resources/templates/matchAdmin.html`:
- Line 63: The template ends after closing the body but never closes the root
html element. Update matchAdmin.html so the markup opened in the main template
is properly terminated with a closing html tag after the existing body close,
ensuring the document structure is complete.
In `@backend/src/main/resources/templates/matchScore.html`:
- Line 33: The `box-shadow` declaration in the `matchScore.html` template is
invalid because the first offset value is missing a length unit. Update the CSS
in the popup styling so the `box-shadow` values are all valid lengths, using the
existing shadow declaration near the popup styles in `matchScore.html`.
- Around line 116-136: The action buttons in matchScore.html are only hidden by
readOnly, so finished matches still render the forfeit/save controls even though
the score popup inputs are missing. Update the three <object> blocks around the
away-forfeit, home-forfeit, and save-button elements to use the same
match-status guard as the popup rendering, keyed off the scoreMatchPage flow and
the NOT_STARTED/IN_PROGRESS states, so these buttons only appear when the score
inputs exist.
In `@frontend/src/pages/tournament/components/Match.tsx`:
- Line 10: The score color logic in the Match component treats ties as losses
because the current comparison only checks whether one score is greater than the
other. Update the helper or inline ternary in Match.tsx so equal scores map to a
neutral color instead of the losing color, while preserving the existing winning
and losing colors for non-tie cases.
In `@frontend/src/pages/tournament/components/Tournament.tsx`:
- Line 112: The stage tabs rendered in the stages .map() are missing a React
key, which can cause reconciliation issues. Update the CustomTabButton usage in
Tournament to pass a stable key prop alongside value, using the stage item’s
unique identifier if available or the index only as a fallback. Keep the change
localized to the mapped CustomTabButton element so React can track each stage
tab correctly.
---
Nitpick comments:
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentComponentController.kt`:
- Around line 19-23: The constructor in TournamentComponentController injects
MenuService twice, but only the menuService parameter is used in the super call.
Remove the redundant unused service: MenuService parameter from the class
constructor and keep the existing menuService injection that is already passed
to the parent, ensuring the remaining constructor parameters still match the
controller’s dependencies.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageController.kt`:
- Around line 211-219: The stage creation flow in TournamentStageController
currently depends on parsing stageType and tournamentId from the Referer header,
which is fragile and can fail when the header is missing or stripped. Update the
create handler to accept these values explicitly instead of deriving them from
Referer, ideally by using hidden form fields or path/query parameters in the
stage creation form and controller method. Keep the StageType mapping logic in
TournamentStageController, but read the incoming type and tournament id from the
explicit request data so creation no longer depends on header presence.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageService.kt`:
- Around line 262-274: The winner-handling logic in TournamentStageService is
duplicating the same BYE update for both home and away winners. Refactor the
conditional inside the matches loop so that the `winner()` result only checks
whether `winner.teamId` matches either `match.homeTeamId` or `match.awayTeamId`,
then performs the shared `match.status = MatchStatus.BYE` and
`matchRepository.save(match)` once; keep the `IllegalStateException` branch only
for the invalid-team case.
In `@backend/src/main/resources/templates/matchScore.html`:
- Around line 67-70: The commented-out tournament title block in the matchScore
template should be removed unless it is meant to be used. Clean up the template
by deleting the unused <object> section around the tournament title display, or
uncommenting it if the UI still needs it; use the surrounding label and th:text
binding to locate the block.
In `@frontend/src/pages/tournament/components/KnockoutStage.tsx`:
- Around line 7-8: Replace the `import('...').Type` aliases in `KnockoutStage`
with standard `import type` imports. Update the `TournamentStageView` and
`MatchView` references to use a proper type-only import from `tournament.view`
so the declarations remain clear, consistent, and idiomatic with the rest of the
codebase.
In `@frontend/src/pages/tournament/components/Tournament.tsx`:
- Around line 50-79: The `joinTournament` and `cancelJoinTournament` handlers in
`Tournament.tsx` are triggering duplicate refetches because
`joinActionResponseCallback` and `cancelActionResponseCallback` already refetch
on `TournamentJoinResponses.OK` and `TournamentCancelResponses.OK`. Remove the
extra `refetch()` calls from the `onSuccess` branches inside
`joinMutation.mutate` and `cancelMutation.mutate`, and let the existing callback
functions own the refresh behavior.
- Around line 128-129: The stage rendering in Tournament.tsx uses raw string
comparisons for stage.type, which should be replaced with StageType enum
constants for type safety. Update the conditional checks in the Tournament
component to compare against the corresponding StageType members instead of
'KNOCKOUT' and 'STAGE', using the existing StageType import so the logic stays
aligned with the enum definitions.
In `@frontend/src/pages/tournament/tournamentList.page.tsx`:
- Around line 22-32: The tournament list item navigation is using a raw anchor
tag, which causes a full page reload instead of in-app routing. Update the
tournament item link in tournamentList.page.tsx to use the existing Link
component used elsewhere in the app, keeping the same destination built from
AbsolutePaths.TOURNAMENTS and tournament.id. Make sure the tournament title
still renders as the clickable label and that the surrounding list/map structure
remains unchanged.
In `@frontend/src/util/paths.ts`:
- Line 29: `Paths.TOURNAMENT` is inconsistent with the plural naming used in
`AbsolutePaths.TOURNAMENTS` and `ApiPaths.TOURNAMENTS`; align the `Paths`
enum/member name with the other route constants. Update the related route
symbols in `paths.ts` so the same plurality is used across `Paths`,
`AbsolutePaths`, and `ApiPaths`, and make sure any references to the tournament
path constant are renamed consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cf5e269b-5b85-4625-90dd-fb98d6a63d3d
📒 Files selected for processing (53)
backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/MatchGroupDto.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/ParticipantDto.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/StageGroupDto.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/StageResultDto.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentApiController.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentCancelStatus.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentComponent.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentComponentController.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentComponentEntityConfiguration.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentController.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentDetailedView.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentEntity.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentJoinDto.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentJoinStatus.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentMatchController.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentMatchEntity.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentMatchRepository.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentPreviewView.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentRepository.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentService.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageController.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageEntity.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageRepository.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageService.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/tournament-features.mdbackend/src/main/kotlin/hu/bme/sch/cmsch/config/ComponentLoadConfig.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/config/TestConfig.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/service/PermissionsService.ktbackend/src/main/resources/config/application-env.propertiesbackend/src/main/resources/config/application.propertiesbackend/src/main/resources/static/style4.cssbackend/src/main/resources/templates/matchAdmin.htmlbackend/src/main/resources/templates/matchScore.htmlbackend/src/main/resources/templates/seedSettings.htmlfrontend/src/App.tsxfrontend/src/api/contexts/config/types.tsfrontend/src/api/hooks/queryKeys.tsfrontend/src/api/hooks/tournament/actions/useTournamentCancelMutation.tsfrontend/src/api/hooks/tournament/actions/useTournamentJoinMutation.tsfrontend/src/api/hooks/tournament/queries/useTournamentListQuery.tsfrontend/src/api/hooks/tournament/queries/useTournamentQuery.tsfrontend/src/pages/tournament/components/GroupStage.tsxfrontend/src/pages/tournament/components/KnockoutBracket.tsxfrontend/src/pages/tournament/components/KnockoutStage.tsxfrontend/src/pages/tournament/components/Match.tsxfrontend/src/pages/tournament/components/Tournament.tsxfrontend/src/pages/tournament/tournament.page.tsxfrontend/src/pages/tournament/tournamentList.page.tsxfrontend/src/pages/tournament/util/matchTree.tsfrontend/src/util/paths.tsfrontend/src/util/views/tournament.view.tshelm/cmsch/templates/cmsch-config.ymlhelm/cmsch/values.yaml
Comment on lines
+15
to
+27
| override fun compareTo(other: StageResultDto): Int { | ||
| if (this.stageId != other.stageId) { | ||
| return this.stageId.compareTo(other.stageId) | ||
| } | ||
| if (this.detailedStats == null && other.detailedStats == null) { | ||
| return -this.initialSeed.compareTo(other.highestSeed) | ||
| } | ||
| if (this.detailedStats == null) { | ||
| return -1 // null detailed stats are considered better (knockout stages are later) | ||
| } | ||
| return compareValuesBy(this, other, | ||
| { it.detailedStats?:GroupStageResults()}, {it.initialSeed}, {it.highlighted}) | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
compareTo compares mismatched fields, violating the Comparable contract.
Line 20 compares this.initialSeed against other.highestSeed — two different fields. This breaks transitivity. For example, with three teams all having detailedStats == null:
- A (initialSeed=1, highestSeed=10), B (initialSeed=5, highestSeed=5), C (initialSeed=10, highestSeed=1)
- A vs B →
-1.compareTo(5)→ negative → A < B - B vs C →
-5.compareTo(1)→ negative → B < C - A vs C →
-1.compareTo(1)→ zero → A == C
A < B and B < C but A == C violates the transitivity requirement of Comparable.
🐛 Proposed fix — compare the same field on both sides
if (this.detailedStats == null && other.detailedStats == null) {
- return -this.initialSeed.compareTo(other.highestSeed)
+ return -this.initialSeed.compareTo(other.initialSeed)
}If the business intent is actually to compare highestSeed, use this.highestSeed.compareTo(other.highestSeed) instead. The key requirement is comparing the same field on both sides.
📝 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
| override fun compareTo(other: StageResultDto): Int { | |
| if (this.stageId != other.stageId) { | |
| return this.stageId.compareTo(other.stageId) | |
| } | |
| if (this.detailedStats == null && other.detailedStats == null) { | |
| return -this.initialSeed.compareTo(other.highestSeed) | |
| } | |
| if (this.detailedStats == null) { | |
| return -1 // null detailed stats are considered better (knockout stages are later) | |
| } | |
| return compareValuesBy(this, other, | |
| { it.detailedStats?:GroupStageResults()}, {it.initialSeed}, {it.highlighted}) | |
| } | |
| override fun compareTo(other: StageResultDto): Int { | |
| if (this.stageId != other.stageId) { | |
| return this.stageId.compareTo(other.stageId) | |
| } | |
| if (this.detailedStats == null && other.detailedStats == null) { | |
| return -this.initialSeed.compareTo(other.initialSeed) | |
| } | |
| if (this.detailedStats == null) { | |
| return -1 // null detailed stats are considered better (knockout stages are later) | |
| } | |
| return compareValuesBy(this, other, | |
| { it.detailedStats?:GroupStageResults()}, {it.initialSeed}, {it.highlighted}) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/StageResultDto.kt`
around lines 15 - 27, StageResultDto.compareTo is comparing different fields for
the null-detailedStats case, which breaks Comparable consistency. Update the
comparison in compareTo so it uses the same property on both sides of the
comparison, likely comparing this.highestSeed with other.highestSeed if that is
the intended ordering. Keep the rest of the ordering logic in
StageResultDto.compareTo and compareValuesBy unchanged, but ensure all branches
compare matching fields to preserve transitivity.
Comment on lines
+140
to
+152
| fun winner(): ParticipantDto? { | ||
| return when { | ||
| homeSeed == null || awaySeed == null -> null // No teams set | ||
| homeTeamId == null || awayTeamId == null -> null // No teams set | ||
| homeTeamId == 0 -> ParticipantDto(awayTeamId!!, awayTeamName) | ||
| awayTeamId == 0 -> ParticipantDto(homeTeamId!!, homeTeamName) | ||
| status != MatchStatus.FINISHED -> null | ||
| homeTeamScore == null || awayTeamScore == null -> null | ||
| homeTeamScore!! > awayTeamScore!! -> ParticipantDto(homeTeamId ?: 0, homeTeamName) | ||
| awayTeamScore!! > homeTeamScore!! -> ParticipantDto(awayTeamId ?: 0, awayTeamName) | ||
| else -> null // Draw or no winner | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix dead null checks on non-nullable seed fields in winner().
homeSeed and awaySeed are declared as non-nullable Int (lines 55–56, 69–70), so the checks homeSeed == null || awaySeed == null on line 142 are always false — this is dead code that gives a false sense of validation. The intent was likely to check whether seeds are unset (i.e., 0), or the fields should be Int? if null is a valid "no team assigned" sentinel.
🐛 Proposed fix — check for unset seeds (value 0) instead of null
return when {
- homeSeed == null || awaySeed == null -> null // No teams set
+ homeSeed == 0 || awaySeed == 0 -> null // No seeds set
homeTeamId == null || awayTeamId == null -> null // No teams set📝 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
| fun winner(): ParticipantDto? { | |
| return when { | |
| homeSeed == null || awaySeed == null -> null // No teams set | |
| homeTeamId == null || awayTeamId == null -> null // No teams set | |
| homeTeamId == 0 -> ParticipantDto(awayTeamId!!, awayTeamName) | |
| awayTeamId == 0 -> ParticipantDto(homeTeamId!!, homeTeamName) | |
| status != MatchStatus.FINISHED -> null | |
| homeTeamScore == null || awayTeamScore == null -> null | |
| homeTeamScore!! > awayTeamScore!! -> ParticipantDto(homeTeamId ?: 0, homeTeamName) | |
| awayTeamScore!! > homeTeamScore!! -> ParticipantDto(awayTeamId ?: 0, awayTeamName) | |
| else -> null // Draw or no winner | |
| } | |
| } | |
| fun winner(): ParticipantDto? { | |
| return when { | |
| homeSeed == 0 || awaySeed == 0 -> null // No seeds set | |
| homeTeamId == null || awayTeamId == null -> null // No teams set | |
| homeTeamId == 0 -> ParticipantDto(awayTeamId!!, awayTeamName) | |
| awayTeamId == 0 -> ParticipantDto(homeTeamId!!, homeTeamName) | |
| status != MatchStatus.FINISHED -> null | |
| homeTeamScore == null || awayTeamScore == null -> null | |
| homeTeamScore!! > awayTeamScore!! -> ParticipantDto(homeTeamId ?: 0, homeTeamName) | |
| awayTeamScore!! > homeTeamScore!! -> ParticipantDto(awayTeamId ?: 0, awayTeamName) | |
| else -> null // Draw or no winner | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentMatchEntity.kt`
around lines 140 - 152, `TournamentMatchEntity.winner()` contains dead null
checks on `homeSeed` and `awaySeed` even though those fields are non-nullable,
so update the guard to match the real “unset” sentinel used by this model (for
example, check for 0 instead of null) or change the seed properties to nullable
if null is intended. Keep the rest of the winner selection logic unchanged, and
ensure the early-return validation in `winner()` consistently reflects how seeds
and team assignment are represented in `TournamentMatchEntity`.
Comment on lines
+152
to
+158
| fun getResultsInStage(tournamentId: Int, stageId: Int): List<StageResultDto> { | ||
| val stage = stageRepository.findById(stageId) | ||
| if (stage.isEmpty || stage.get().tournamentId != tournamentId) { | ||
| return emptyList() | ||
| } | ||
| return stage.get().participants.split("\n").map { objectMapper.readValue(it, StageResultDto::class.java) } | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard against empty stage participants before parsing.
Unlike getParticipants (Lines 134, 143), this path never checks for an empty participants string nor filters empty lines. When participants == "", split("\n") yields [""] and objectMapper.readValue("", ...) throws, failing the request.
🛡️ Proposed fix
- val stage = stageRepository.findById(stageId)
- if (stage.isEmpty || stage.get().tournamentId != tournamentId) {
- return emptyList()
- }
- return stage.get().participants.split("\n").map { objectMapper.readValue(it, StageResultDto::class.java) }
+ val stage = stageRepository.findById(stageId)
+ if (stage.isEmpty || stage.get().tournamentId != tournamentId || stage.get().participants.isEmpty()) {
+ return emptyList()
+ }
+ return stage.get().participants.split("\n")
+ .filter { it.isNotEmpty() }
+ .map { objectMapper.readValue(it, StageResultDto::class.java) }📝 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
| fun getResultsInStage(tournamentId: Int, stageId: Int): List<StageResultDto> { | |
| val stage = stageRepository.findById(stageId) | |
| if (stage.isEmpty || stage.get().tournamentId != tournamentId) { | |
| return emptyList() | |
| } | |
| return stage.get().participants.split("\n").map { objectMapper.readValue(it, StageResultDto::class.java) } | |
| } | |
| fun getResultsInStage(tournamentId: Int, stageId: Int): List<StageResultDto> { | |
| val stage = stageRepository.findById(stageId) | |
| if (stage.isEmpty || stage.get().tournamentId != tournamentId || stage.get().participants.isEmpty()) { | |
| return emptyList() | |
| } | |
| return stage.get().participants.split("\n") | |
| .filter { it.isNotEmpty() } | |
| .map { objectMapper.readValue(it, StageResultDto::class.java) } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentService.kt`
around lines 152 - 158, The getResultsInStage method in TournamentService
currently parses every split line without guarding against an empty participants
field, so an empty string can still reach objectMapper.readValue and throw.
Update getResultsInStage to mirror the empty-input handling used by
getParticipants: check stage.get().participants for blank/empty content before
splitting, and filter out empty lines from the split result before mapping to
StageResultDto so only real entries are parsed.
Comment on lines
+160
to
+176
| fun getAggregatedMatchesByTournamentId(): List<MatchGroupDto> { | ||
| val tournaments = findAll().associateBy { it.id } | ||
| val stages = stageRepository.findAll().associateBy { it.id } | ||
| val aggregatedByStageId = matchRepository.findAllAggregated() | ||
| val aggregated = mutableMapOf<Int, Long>() | ||
| for(aggregatedStage in aggregatedByStageId) { | ||
| aggregated[stages[aggregatedStage.stageId]!!.tournamentId] = aggregated.getOrDefault(stages[aggregatedStage.stageId]!!.tournamentId, 0) + aggregatedStage.matchCount | ||
| } | ||
| return aggregated.map { | ||
| MatchGroupDto( | ||
| it.key, | ||
| tournaments[it.key]?.title ?:"", | ||
| tournaments[it.key]?.location ?:"", | ||
| it.value.toInt() | ||
| ) | ||
| }.sortedByDescending { it.matchCount } | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the target service and the referenced delete path
git ls-files | rg 'TournamentService\.kt$|StageRepository|MatchRepository|MatchGroupDto'
# Show the relevant file structure and line ranges around the cited methods
ast-grep outline backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentService.kt --view expanded
# Read the relevant section around getAggregatedMatchesByTournamentId
sed -n '140,210p' backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentService.kt
# Read the deleteStagesForTournament section
sed -n '260,320p' backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentService.ktRepository: kir-dev/cmsch
Length of output: 5798
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect repository methods involved in the aggregation and deletion paths
ast-grep outline backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageRepository.kt --view expanded
ast-grep outline backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentMatchRepository.kt --view expanded
sed -n '1,220p' backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageRepository.kt
printf '\n---\n'
sed -n '1,240p' backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentMatchRepository.ktRepository: kir-dev/cmsch
Length of output: 3551
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect stage/match entity mappings and any cascading or foreign-key behavior
ast-grep outline backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageEntity.kt --view expanded
ast-grep outline backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentMatchEntity.kt --view expanded
sed -n '1,260p' backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageEntity.kt
printf '\n---\n'
sed -n '1,260p' backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentMatchEntity.ktRepository: kir-dev/cmsch
Length of output: 12420
Guard the stage lookup in tournament aggregation.
deleteStagesForTournament() removes stage rows without touching matches, so matchRepository.findAllAggregated() can still return stageIds that no longer exist. stages[aggregatedStage.stageId]!! then NPEs and breaks the whole admin aggregation; skip missing stages or clean up the related matches first.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentService.kt`
around lines 160 - 176, The tournament aggregation in
getAggregatedMatchesByTournamentId assumes every aggregated stage still exists,
but deleted stages can leave stale stageId values and cause a null crash. Update
the loop over matchRepository.findAllAggregated() to safely handle missing
entries from stages[aggregatedStage.stageId] instead of forcing a non-null
access, and either skip those aggregates or otherwise account for them before
building the MatchGroupDto list.
Comment on lines
+285
to
+288
| @Transactional | ||
| fun deleteStagesForTournament(tournament: TournamentEntity) { | ||
| stageRepository.deleteAllByTournamentId(tournament.id) | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files first
git ls-files | rg 'backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/(TournamentService.kt|TournamentStageController.kt|.*Repository.kt|.*Entity.kt|.*Match.*\.kt)$|backend/src/main/kotlin/hu/bme/sch/cmsch/component/.*Match.*\.kt$'
printf '\n--- TournamentService outline ---\n'
ast-grep outline backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentService.kt --view expanded
printf '\n--- TournamentStageController outline ---\n'
ast-grep outline backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageController.kt --view expanded
printf '\n--- Search for deleteAllByTournamentId and onEntityDeleted ---\n'
rg -n "deleteAllByTournamentId|onEntityDeleted|getAggregatedMatchesByTournamentId|stageId|matchRepository|stageRepository" backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament backend/src/main/kotlin/hu/bme/sch/cmsch/component -SRepository: kir-dev/cmsch
Length of output: 22991
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant slices once the files are located
sed -n '1,260p' backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentService.kt
printf '\n--- controller ---\n'
sed -n '1,260p' backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageController.kt
printf '\n--- repositories / entities / match aggregation ---\n'
rg -n "interface .*Repository|class .*Entity|data class .*Entity|getAggregatedMatchesByTournamentId|!!" backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament backend/src/main/kotlin/hu/bme/sch/cmsch/component -SRepository: kir-dev/cmsch
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- TournamentService around aggregation and deleteStagesForTournament ---'
sed -n '155,295p' backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentService.kt
printf '\n%s\n' '--- TournamentStageController onEntityDeleted ---'
sed -n '345,370p' backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageController.kt
printf '\n%s\n' '--- TournamentController onEntityDeleted ---'
sed -n '50,75p' backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentController.kt
printf '\n%s\n' '--- TournamentStageEntity ---'
sed -n '1,220p' backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageEntity.kt
printf '\n%s\n' '--- TournamentMatchEntity ---'
sed -n '1,240p' backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentMatchEntity.kt
printf '\n%s\n' '--- TournamentStageRepository and TournamentMatchRepository ---'
sed -n '1,120p' backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageRepository.kt
printf '\n---\n'
sed -n '1,120p' backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentMatchRepository.ktRepository: kir-dev/cmsch
Length of output: 21410
Delete stage matches before removing tournament stages. deleteAllByTournamentId bypasses TournamentStageController.onEntityDeleted, so matches keep dangling stageIds. Those orphaned rows make getAggregatedMatchesByTournamentId() hit stages[aggregatedStage.stageId]!!. Delete the matches first, or reuse the stage deletion path that already clears them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentService.kt`
around lines 285 - 288, Delete tournament stage matches before removing the
stages, because deleteAllByTournamentId in
TournamentService.deleteStagesForTournament bypasses
TournamentStageController.onEntityDeleted and leaves orphaned match stageId
references. Update deleteStagesForTournament to either reuse the existing stage
deletion path that already clears matches or explicitly remove the associated
matches first, so getAggregatedMatchesByTournamentId no longer encounters
missing stage entries.
Comment on lines
+59
to
+76
| fun transferTeamsForStage(stage: TournamentStageEntity): String { | ||
| val teamSeeds = (1..stage.participantCount).asIterable().toList() | ||
| var participants = getResultsForStage(stage) | ||
| if (participants.size >= stage.participantCount) { | ||
| participants = | ||
| participants.subList(0, stage.participantCount).map { StageResultDto(it.teamId, it.teamName) } | ||
| } | ||
| for (i in 0 until stage.participantCount) { | ||
| participants[i].initialSeed = teamSeeds[i] | ||
| } | ||
| val parts = mutableListOf<StageResultDto>() | ||
| parts.addAll(participants) | ||
| for (i in stage.participantCount + 1 until 2.0.pow(stage.rounds()).toInt() + 1) { | ||
| parts.add(StageResultDto(teamId = 0, teamName = "ByeGame", initialSeed = i)) | ||
| } | ||
| stage.participants = parts.joinToString("\n") { objectMapper.writeValueAsString(it) } | ||
| return stage.participants | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
IndexOutOfBoundsException when fewer teams than participantCount.
The truncation only runs when participants.size >= stage.participantCount. If fewer teams advanced/joined, participants keeps its smaller size, yet the loop iterates 0 until stage.participantCount and reads participants[i], throwing out of bounds during stage creation (onEntityChanged).
🐛 Proposed fix
- for (i in 0 until stage.participantCount) {
+ for (i in participants.indices) {
participants[i].initialSeed = teamSeeds[i]
}📝 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
| fun transferTeamsForStage(stage: TournamentStageEntity): String { | |
| val teamSeeds = (1..stage.participantCount).asIterable().toList() | |
| var participants = getResultsForStage(stage) | |
| if (participants.size >= stage.participantCount) { | |
| participants = | |
| participants.subList(0, stage.participantCount).map { StageResultDto(it.teamId, it.teamName) } | |
| } | |
| for (i in 0 until stage.participantCount) { | |
| participants[i].initialSeed = teamSeeds[i] | |
| } | |
| val parts = mutableListOf<StageResultDto>() | |
| parts.addAll(participants) | |
| for (i in stage.participantCount + 1 until 2.0.pow(stage.rounds()).toInt() + 1) { | |
| parts.add(StageResultDto(teamId = 0, teamName = "ByeGame", initialSeed = i)) | |
| } | |
| stage.participants = parts.joinToString("\n") { objectMapper.writeValueAsString(it) } | |
| return stage.participants | |
| } | |
| fun transferTeamsForStage(stage: TournamentStageEntity): String { | |
| val teamSeeds = (1..stage.participantCount).asIterable().toList() | |
| var participants = getResultsForStage(stage) | |
| if (participants.size >= stage.participantCount) { | |
| participants = | |
| participants.subList(0, stage.participantCount).map { StageResultDto(it.teamId, it.teamName) } | |
| } | |
| for (i in participants.indices) { | |
| participants[i].initialSeed = teamSeeds[i] | |
| } | |
| val parts = mutableListOf<StageResultDto>() | |
| parts.addAll(participants) | |
| for (i in stage.participantCount + 1 until 2.0.pow(stage.rounds()).toInt() + 1) { | |
| parts.add(StageResultDto(teamId = 0, teamName = "ByeGame", initialSeed = i)) | |
| } | |
| stage.participants = parts.joinToString("\n") { objectMapper.writeValueAsString(it) } | |
| return stage.participants | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageService.kt`
around lines 59 - 76, `transferTeamsForStage` can throw
`IndexOutOfBoundsException` when `getResultsForStage(stage)` returns fewer
entries than `stage.participantCount`, because the `participants[i]` loop always
iterates up to `participantCount`. Fix this by ensuring the list is padded or
bounded before assigning `initialSeed` in
`TournamentStageService.transferTeamsForStage`, so every slot up to
`stage.participantCount` has a `StageResultDto` (including bye placeholders)
before the seeding loop runs.
Comment on lines
+133
to
+149
| @Transactional | ||
| fun createRoundMatchesForGroupStageGroup(stage: TournamentStageEntity, group: Int) { | ||
| require(stage.type == StageType.GROUP) { | ||
| "Stage type must be GROUP to create round matches." | ||
| } | ||
| require(stage.groupCount < group) { | ||
| "Group number $group is out of bounds for stage with ${stage.groupCount} groups." | ||
| } | ||
| val matches = mutableListOf<TournamentMatchEntity>() | ||
| val groupSize = let { | ||
| val min = floor(stage.participantCount.toDouble() / stage.groupCount).toInt() | ||
| if (stage.participantCount % stage.groupCount < group) { | ||
| min + 1 | ||
| } else { | ||
| min | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Inverted bounds check rejects valid group numbers.
require(stage.groupCount < group) throws whenever group <= groupCount — i.e. exactly for the valid group numbers — which contradicts the "out of bounds" message. The intent is to fail when group exceeds the group count.
🐛 Proposed fix
- require(stage.groupCount < group) {
+ require(group in 1..stage.groupCount) {
"Group number $group is out of bounds for stage with ${stage.groupCount} groups."
}📝 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
| @Transactional | |
| fun createRoundMatchesForGroupStageGroup(stage: TournamentStageEntity, group: Int) { | |
| require(stage.type == StageType.GROUP) { | |
| "Stage type must be GROUP to create round matches." | |
| } | |
| require(stage.groupCount < group) { | |
| "Group number $group is out of bounds for stage with ${stage.groupCount} groups." | |
| } | |
| val matches = mutableListOf<TournamentMatchEntity>() | |
| val groupSize = let { | |
| val min = floor(stage.participantCount.toDouble() / stage.groupCount).toInt() | |
| if (stage.participantCount % stage.groupCount < group) { | |
| min + 1 | |
| } else { | |
| min | |
| } | |
| } | |
| `@Transactional` | |
| fun createRoundMatchesForGroupStageGroup(stage: TournamentStageEntity, group: Int) { | |
| require(stage.type == StageType.GROUP) { | |
| "Stage type must be GROUP to create round matches." | |
| } | |
| require(group in 1..stage.groupCount) { | |
| "Group number $group is out of bounds for stage with ${stage.groupCount} groups." | |
| } | |
| val matches = mutableListOf<TournamentMatchEntity>() | |
| val groupSize = let { | |
| val min = floor(stage.participantCount.toDouble() / stage.groupCount).toInt() | |
| if (stage.participantCount % stage.groupCount < group) { | |
| min + 1 | |
| } else { | |
| min | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/tournament/TournamentStageService.kt`
around lines 133 - 149, The bounds check in createRoundMatchesForGroupStageGroup
is inverted, so valid group numbers are rejected while out-of-range values may
pass. Update the require condition to fail only when the requested group exceeds
the stage’s groupCount, and keep the existing error message consistent with that
validation. Use the createRoundMatchesForGroupStageGroup function and the
stage.groupCount/group parameters to locate and correct the check.
| </object> | ||
| <div th:if="${match.homeTeamScore!=null && match.awayTeamScore!=null}" th:remove="tag"> | ||
| <label class="card-label">Gólok</label> | ||
| <p th:number="${match.homeTeamScore}" class="highlight">Home team goals</p> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Relevant template lines =="
sed -n '90,120p' backend/src/main/resources/templates/matchScore.html
echo
echo "== Search for th:number usage =="
rg -n --hidden --no-ignore-vcs 'th:number|number="' backend/src/main/resources || true
echo
echo "== Search for custom Thymeleaf dialect/processor references =="
rg -n --hidden --no-ignore-vcs 'Thymeleaf|IDialect|IProcessor|AttributeTagProcessor|number' backend/src/main java . || trueRepository: kir-dev/cmsch
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Template excerpt =="
sed -n '96,114p' backend/src/main/resources/templates/matchScore.html
echo
echo "== th:number occurrences in backend =="
rg -n --hidden --no-ignore-vcs 'th:number' backend || true
echo
echo "== Thymeleaf dependency/config in backend build files =="
rg -n --hidden --no-ignore-vcs 'thymeleaf|Thymeleaf|spring-boot-starter-thymeleaf|IDialect|IProcessor|AttributeTagProcessor' backend/build.gradle backend/pom.xml backend/settings.gradle backend/src/main || trueRepository: kir-dev/cmsch
Length of output: 8073
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Build files at repo root/backends =="
fd -a 'build.gradle*|pom.xml|settings.gradle*' . || true
echo
echo "== Thymeleaf-related classes in backend source =="
rg -n --hidden --no-ignore-vcs --glob 'backend/src/main/**' \
'class .*Dialect|interface .*Dialect|extends .*Dialect|implements .*Dialect|AbstractAttributeTagProcessor|AttributeTagProcessor|IProcessor|IDialect|SpringTemplateEngine|TemplateEngine|thymeleaf' \
backend/src/main || true
echo
echo "== matchScore template context =="
sed -n '1,140p' backend/src/main/resources/templates/matchScore.htmlRepository: kir-dev/cmsch
Length of output: 14362
Replace th:number with th:text for the score values backend/src/main/resources/templates/matchScore.html:100,110
th:number isn’t handled here, so the home/away score values stay as placeholder text instead of rendering the actual numbers.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/main/resources/templates/matchScore.html` at line 100, The score
fields in matchScore.html are using an unsupported Thymeleaf attribute, so the
values are not rendered. Update the home and away score elements in the
matchScore template to use th:text instead of th:number, keeping the existing
match.homeTeamScore and match.awayTeamScore bindings so the actual numbers
display correctly.
| <td th:text="${team.teamName}"> | ||
| csapatnév | ||
| </td> | ||
| <td th:if="${stageLevel>1}">todo</td> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
"todo" placeholder is visible to end users.
The "Előző eredmény" (Previous result) column shows literal todo text for all stages with stageLevel > 1. This is user-facing incomplete functionality that should either be implemented or hidden until ready.
💡 Suggested interim fix
- <td th:if="${stageLevel>1}">todo</td>
+ <td th:if="${stageLevel>1}" th:text="${team.previousResult ?: '—'}">todo</td>Or hide the column entirely until the feature is implemented:
- <th th:if="${stageLevel>1}">Előző eredmény</th>
+ <!-- TODO: implement previous result display for stages > level 1 -->🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/main/resources/templates/seedSettings.html` at line 80, The seed
settings template currently renders a user-visible placeholder in the
stageLevel>1 branch of the table cell, so update the th:if/th:text logic in
seedSettings.html to remove the literal placeholder from the “Előző eredmény”
column. Either wire this cell to the real previous-result value in the relevant
template binding or hide the column/cell until the feature is implemented, using
the existing stageLevel condition as the locator.
Comment on lines
+8
to
+11
| const getScoreColor = (match: MatchView, score1?: number, score2?: number) => { | ||
| if (match.status !== 'COMPLETED' || score1 === undefined || score2 === undefined) return 'gray.600' | ||
| return score1 > score2 ? 'green.600' : 'red.600' | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Chakra color tokens are invalid in inline styles — scores won't be colored.
getScoreColor returns Chakra UI semantic tokens ('gray.600', 'green.600', 'red.600'), but these are used in inline style={{ color: ... }} objects (lines 46, 59). Chakra tokens are only resolved when used via Chakra's color prop or useColorModeValue/theme hooks — not in raw inline styles. The browser will treat them as invalid CSS color values, so score colors won't render.
Use either Chakra's <Text color={...}> component or replace with actual CSS color values.
🐛 Proposed fix using CSS hex values
const getScoreColor = (match: MatchView, score1?: number, score2?: number) => {
if (match.status !== 'COMPLETED' || score1 === undefined || score2 === undefined) return '`#4a5568`'
- return score1 > score2 ? 'green.600' : 'red.600'
+ return score1 > score2 ? '`#2f855a`' : '`#c53030`'
}Also applies to: 46-46, 59-59
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/pages/tournament/components/Match.tsx` around lines 8 - 11,
getScoreColor in Match.tsx is returning Chakra semantic tokens that are then
passed into inline style color values, so the browser never resolves them.
Update the score rendering in Match to use Chakra’s color prop (or another
Chakra-aware styling path) instead of style={{ color: ... }}, or change
getScoreColor to return real CSS color values so the completed score text is
actually colored.
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
3 participants
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.
Sportversenyek vezetésére szolgáló komponens.
Summary by CodeRabbit