core: contest team#1183
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds team participation to contests, including team creation, invitations, membership limits, team attendance, and contest configuration. Contest status, scoreboard rendering, submissions, judging, hacking, clarifications, file access, and record visibility now account for team members. New team-management and attendance UI flows, responsive styles, and Simplified and Traditional Chinese localization strings are included. Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/hydrooj/src/handler/contest.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/hydrooj/src/model/contest.ts (1)
953-964: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winParallelize per-contest status resolution.
This loop issues up to three sequential round-trips per
tid(getStatus, thengetTeamVid+getStatuson the non-attended path). On the contest list page with many contests this serializes into a long chain. Consider resolving alltidsconcurrently withPromise.all.♻️ Suggested parallelization
- for (const tid of tids) { - // eslint-disable-next-line no-await-in-loop - const tsdoc = await getStatus(domainId, tid, uid); - if (tsdoc?.attend) { - r[tid.toHexString()] = tsdoc; - continue; - } - // eslint-disable-next-line no-await-in-loop - const teamVid = await getTeamVid(domainId, tid, uid); - // eslint-disable-next-line no-await-in-loop - r[tid.toHexString()] = teamVid ? await getStatus(domainId, tid, teamVid) : tsdoc; - } + await Promise.all(tids.map(async (tid) => { + const tsdoc = await getStatus(domainId, tid, uid); + if (tsdoc?.attend) { + r[tid.toHexString()] = tsdoc; + return; + } + const teamVid = await getTeamVid(domainId, tid, uid); + r[tid.toHexString()] = teamVid ? await getStatus(domainId, tid, teamVid) : tsdoc; + }));🤖 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 `@packages/hydrooj/src/model/contest.ts` around lines 953 - 964, Parallelize contest status resolution by replacing the sequential loop with a Promise.all over tids, preserving the existing attended-status and team-status fallback behavior in each mapped task, then assign each resolved result into r using the contest ID key.
🤖 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 `@packages/hydrooj/locales/zh_TW.yaml`:
- Around line 489-521: Traditional Chinese is missing the Reject translation
used by contest team UI. Add the key “Reject” to the zh_TW locale entries,
translating it as “拒絕”, alongside the related team invitation translations.
In `@packages/hydrooj/src/handler/contest.ts`:
- Line 476: Update the allowTeam assignment in the contest edit handler so an
explicit form value of false is not overridden by this.tdoc?.allowTeam. Preserve
the existing setting only when disabling teams is disallowed or existing teams
are in use; otherwise pass the submitted value through. Use the surrounding
contest update logic and tdoc team-usage checks to implement the appropriate
guard.
In `@packages/hydrooj/src/handler/judge.ts`:
- Around line 125-128: Add an index on collStatus covering the fields queried by
contest.getTeamVid, preferably { domainId, docType, docId, members }, and ensure
it is created during collection initialization or migration. Alternatively,
update getTeamVid to bypass the members lookup for non-team contests.
In `@packages/hydrooj/src/model/user.ts`:
- Around line 382-384: Update ensureVuser to normalize the username consistently
with createVuser by trimming it before lowercasing in the collV.findOne
unameLower query, ensuring whitespace-padded names match existing vusers.
---
Nitpick comments:
In `@packages/hydrooj/src/model/contest.ts`:
- Around line 953-964: Parallelize contest status resolution by replacing the
sequential loop with a Promise.all over tids, preserving the existing
attended-status and team-status fallback behavior in each mapped task, then
assign each resolved result into r using the contest ID key.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 18d2a5e0-dd96-4483-8ff7-2069819b3cd5
📒 Files selected for processing (21)
packages/hydrooj/locales/zh.yamlpackages/hydrooj/locales/zh_TW.yamlpackages/hydrooj/src/error.tspackages/hydrooj/src/handler/contest.tspackages/hydrooj/src/handler/judge.tspackages/hydrooj/src/handler/problem.tspackages/hydrooj/src/handler/record.tspackages/hydrooj/src/interface.tspackages/hydrooj/src/model/contest.tspackages/hydrooj/src/model/setting.tspackages/hydrooj/src/model/user.tspackages/onsite-toolkit/submit.tspackages/ui-default/components/contest/contest.page.tspackages/ui-default/components/contest/contest_sidebar.page.stylpackages/ui-default/pages/contest_team.page.stylpackages/ui-default/templates/contest_edit.htmlpackages/ui-default/templates/contest_main.htmlpackages/ui-default/templates/contest_team.htmlpackages/ui-default/templates/partials/contest_sidebar.htmlpackages/ui-default/templates/partials/contest_user.htmlpackages/ui-default/templates/partials/scoreboard.html
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9dee68e87c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } else { | ||
| await this.mustMember(vuid); | ||
| } | ||
| await user.updateVuserById(vuid, { $pull: { members: target, invite: target } }); |
There was a problem hiding this comment.
If a teammate is removed after the team has already attended a contest, this only updates the virtual user document; existing contest status docs still keep the old members array. Since contest access is resolved through contest.getTeamVid() by searching those stored status members, the removed user will still be treated as part of the team for already-attended contests and can continue accessing team submissions/private contest state. Either update the corresponding contest status memberships or block membership changes once the team has attended contests.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@undefined-moe Should we prevent team member changes while a contest is active, or simply keep a snapshot for current contest and allow change team member for possible further contest?
There was a problem hiding this comment.
In my view, preserving snapshots during the contest is expected behavior. However, I hadn't fully considered whether to prohibit changes in order to reduce ambiguity.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/ui-default/pages/contest_team.page.styl (2)
5-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate
&__row/&__actionspatterns between.team-invites-bannerand.team-card.The row (Lines 29-38, 99-108) and actions (Lines 39-45, 109-119) blocks are nearly identical between the two components. Consider extracting a shared mixin/class (e.g.
list-row()/list-actions()) to avoid drift as one gets tweaked independently later.Also applies to: 46-119
🤖 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 `@packages/ui-default/pages/contest_team.page.styl` around lines 5 - 45, Extract the duplicated row and actions styling from .team-invites-banner and .team-card into shared Stylus mixins or reusable classes, then apply them to both components. Preserve the existing flex, spacing, form-margin, and last-row border behavior while ensuring future changes remain centralized.
36-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInconsistent border color sourcing.
Lines 37 and 107 use hardcoded
rgba(#000, 0.06)/rgba(#000, 0.05)for row separators, while line 115 uses the theme variable$table-border-colorfor the same visual purpose a few lines away. If$table-border-coloris theme-aware (e.g. dark mode), the hardcoded black-alpha borders on Lines 37/107 won't adapt consistently.♻️ Suggested fix
&__row display: flex align-items: center justify-content: space-between gap: rem(10px) padding: rem(5px) 0 &:not(:last-child) - border-bottom: 1px solid rgba(`#000`, 0.06) + border-bottom: 1px solid $table-border-colorApply the same change at Line 107.
Also applies to: 106-108, 109-115
🤖 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 `@packages/ui-default/pages/contest_team.page.styl` around lines 36 - 38, Replace the hardcoded rgba(`#000`, 0.06) and rgba(`#000`, 0.05) row-separator borders in the relevant contest team styles with the existing $table-border-color theme variable, including the separator near the &:not(:last-child) rule and the corresponding rule around the later row styles. Preserve the current border placement and width.
🤖 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 `@packages/hydrooj/src/handler/contest.ts`:
- Around line 1000-1006: Update postRename to trim the incoming name, validate
that the trimmed value is non-empty using the same guard and error behavior as
postCreate, and only then call user.updateVuserById with the validated display
name.
- Around line 1026-1058: Update postAccept and postLeave to synchronize the
corresponding ContestStatusDoc.members snapshot whenever the vuser roster
changes. Add the necessary contest-status update after each successful
membership addition or removal, preserving existing validation and vuser
updates, so getTeamVid and contest record/clarification access immediately
reflect late joins and departures.
---
Nitpick comments:
In `@packages/ui-default/pages/contest_team.page.styl`:
- Around line 5-45: Extract the duplicated row and actions styling from
.team-invites-banner and .team-card into shared Stylus mixins or reusable
classes, then apply them to both components. Preserve the existing flex,
spacing, form-margin, and last-row border behavior while ensuring future changes
remain centralized.
- Around line 36-38: Replace the hardcoded rgba(`#000`, 0.06) and rgba(`#000`, 0.05)
row-separator borders in the relevant contest team styles with the existing
$table-border-color theme variable, including the separator near the
&:not(:last-child) rule and the corresponding rule around the later row styles.
Preserve the current border placement and width.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 890003fb-b908-423d-9174-8db99d0ad01d
📒 Files selected for processing (22)
packages/hydrooj/locales/zh.yamlpackages/hydrooj/locales/zh_TW.yamlpackages/hydrooj/src/error.tspackages/hydrooj/src/handler/contest.tspackages/hydrooj/src/handler/judge.tspackages/hydrooj/src/handler/problem.tspackages/hydrooj/src/handler/record.tspackages/hydrooj/src/interface.tspackages/hydrooj/src/model/contest.tspackages/hydrooj/src/model/document.tspackages/hydrooj/src/model/setting.tspackages/hydrooj/src/model/user.tspackages/onsite-toolkit/submit.tspackages/ui-default/components/contest/contest.page.tspackages/ui-default/components/contest/contest_sidebar.page.stylpackages/ui-default/pages/contest_team.page.stylpackages/ui-default/templates/contest_edit.htmlpackages/ui-default/templates/contest_main.htmlpackages/ui-default/templates/contest_team.htmlpackages/ui-default/templates/partials/contest_sidebar.htmlpackages/ui-default/templates/partials/contest_user.htmlpackages/ui-default/templates/partials/scoreboard.html
🚧 Files skipped from review as they are similar to previous changes (18)
- packages/onsite-toolkit/submit.ts
- packages/ui-default/components/contest/contest_sidebar.page.styl
- packages/hydrooj/src/model/setting.ts
- packages/ui-default/templates/partials/scoreboard.html
- packages/ui-default/templates/contest_edit.html
- packages/hydrooj/src/interface.ts
- packages/hydrooj/locales/zh.yaml
- packages/ui-default/templates/contest_main.html
- packages/ui-default/templates/partials/contest_user.html
- packages/hydrooj/src/model/document.ts
- packages/hydrooj/src/error.ts
- packages/hydrooj/src/handler/judge.ts
- packages/hydrooj/locales/zh_TW.yaml
- packages/ui-default/components/contest/contest.page.ts
- packages/hydrooj/src/model/user.ts
- packages/hydrooj/src/handler/problem.ts
- packages/hydrooj/src/model/contest.ts
- packages/hydrooj/src/handler/record.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/hydrooj/locales/zh.yaml (1)
496-496: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the form of address consistent.
These new strings use informal “你”, while the surrounding Simplified Chinese UI consistently uses formal “您”. Please update these entries unless the team flow intentionally uses a different tone.
Proposed wording
-Choose how you want to participate in this contest.: 选择你参加本次比赛的方式。 +Choose how you want to participate in this contest.: 选择您参加本次比赛的方式。 -Leave this team? You may lose access to team contests.: 确定退出该队伍吗？退出后你可能无法访问该队伍参加的比赛。 +Leave this team? You may lose access to team contests.: 确定退出该队伍吗？退出后您可能无法访问该队伍参加的比赛。 -You are not in any team.: 你还没有加入任何队伍。 -You are not in any team yet. Create one from the panel on the right, or ask a teammate to invite you by your username.: 你还没有加入任何队伍。在右侧面板创建一个，或请队友按你的用户名邀请你。 -You have {0} pending team invite(s): 你有 {0} 个待处理的队伍邀请 +You are not in any team.: 您还没有加入任何队伍。 +You are not in any team yet. Create one from the panel on the right, or ask a teammate to invite you by your username.: 您还没有加入任何队伍。在右侧面板创建一个，或请队友按您的用户名邀请您。 +You have {0} pending team invite(s): 您有 {0} 个待处理的队伍邀请Also applies to: 505-505, 522-524
🤖 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 `@packages/hydrooj/locales/zh.yaml` at line 496, Update the newly added Simplified Chinese contest-related entries at the referenced locale keys, including “Choose how you want to participate in this contest.” and the entries around the other noted locations, replacing informal “你” with formal “您” while preserving the existing translations and meaning.
🤖 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.
Nitpick comments:
In `@packages/hydrooj/locales/zh.yaml`:
- Line 496: Update the newly added Simplified Chinese contest-related entries at
the referenced locale keys, including “Choose how you want to participate in
this contest.” and the entries around the other noted locations, replacing
informal “你” with formal “您” while preserving the existing translations and
meaning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e7562ad1-cdac-4a46-81f5-56620c111705
📒 Files selected for processing (3)
packages/hydrooj/locales/zh.yamlpackages/hydrooj/locales/zh_TW.yamlpackages/ui-default/templates/contest_edit.html
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/ui-default/templates/contest_edit.html
- packages/hydrooj/locales/zh_TW.yaml
2 participants
Summary by CodeRabbit