core&ui: support bulk rejudge task #998
Conversation
WalkthroughThis change introduces a comprehensive bulk rejudge management feature to the system. It adds new backend handlers, data models, and MongoDB collections to support creating, previewing, and tracking rejudge tasks. New interfaces and document types are defined for storing rejudge task metadata and results. The UI is extended with a new management page and template, offering filters for user, problem, contest, language, status, and time range, as well as options for high priority and apply mode. Localization entries for English, Simplified Chinese, and Traditional Chinese are added to support the new feature in the interface. Routing and UI injection points are updated to integrate the new management functionality. 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/interface.tsOops! Something went wrong! :( ESLint: 9.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@hydrooj/eslint-config' imported from /eslint.config.mjs packages/hydrooj/src/handler/manage.tsOops! Something went wrong! :( ESLint: 9.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@hydrooj/eslint-config' imported from /eslint.config.mjs packages/hydrooj/src/model/document.tsOops! Something went wrong! :( ESLint: 9.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@hydrooj/eslint-config' imported from /eslint.config.mjs
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
packages/ui-default/templates/manage_rejudge.html (1)
112-112: Remove trailing space in class attribute- <div class="section__body no-padding"> + <div class="section__body no-padding">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/hydrooj/locales/en.yaml(1 hunks)packages/hydrooj/locales/zh.yaml(1 hunks)packages/hydrooj/locales/zh_TW.yaml(1 hunks)packages/hydrooj/src/handler/manage.ts(3 hunks)packages/hydrooj/src/interface.ts(2 hunks)packages/hydrooj/src/lib/ui.ts(1 hunks)packages/hydrooj/src/model/document.ts(4 hunks)packages/hydrooj/src/model/record.ts(4 hunks)packages/ui-default/pages/manage_rejudge.page.ts(1 hunks)packages/ui-default/templates/manage_rejudge.html(1 hunks)
🧰 Additional context used
🪛 HTMLHint (1.5.0)
packages/ui-default/templates/manage_rejudge.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
🔇 Additional comments (11)
packages/hydrooj/locales/zh.yaml (1)
468-468: LGTM! Accurate Simplified Chinese translation added.The translation "重测管理" (Rejudge Management) is accurate and properly positioned within the locale file structure.
packages/hydrooj/locales/en.yaml (1)
48-48: LGTM! Clear English translation added.The translation "Rejudge Management" is clear and follows standard naming conventions for management interfaces.
packages/hydrooj/locales/zh_TW.yaml (1)
255-255: LGTM! Accurate Traditional Chinese translation added.The translation "重測管理" correctly uses Traditional Chinese characters and is consistent with the Simplified Chinese version.
packages/hydrooj/src/lib/ui.ts (1)
66-66: LGTM! UI injection follows established patterns.The ControlPanel injection for 'manage_rejudge' correctly follows the existing pattern and will inherit appropriate permissions from the parent manage_dashboard context.
packages/hydrooj/src/model/document.ts (4)
32-32: LGTM! Document type constant follows logical numbering.The
TYPE_REJUDGE = 50constant follows the established numbering pattern and provides appropriate separation from other document types.
10-10: LGTM! Import addition is necessary and well-integrated.The
RecordRejudgeDocimport is properly positioned within the existing interface imports and supports the new document type functionality.
44-44: LGTM! Type mapping correctly associates constant with interface.The mapping
[TYPE_REJUDGE]: RecordRejudgeDocproperly integrates the new document type into the TypeScript type system.
492-492: LGTM! Global export properly exposes the new document type.The
TYPE_REJUDGEexport is correctly positioned alphabetically and makes the constant available throughout the system as needed for the rejudge functionality.packages/hydrooj/src/interface.ts (1)
231-249: Well-structured interface definitionsThe new interfaces
RecordRejudgeDocandRecordRejudgeResultDocare properly defined with clear type annotations and extend the appropriate base interfaces.packages/hydrooj/src/model/record.ts (1)
303-303: Verify that newRev field exists in RecordRejudgeResultDoc interfaceThe code sets a
newRevfield, but this field is not defined in theRecordRejudgeResultDocinterface.#!/bin/bash # Description: Check if newRev field is defined in RecordRejudgeResultDoc interface # Search for RecordRejudgeResultDoc interface definition ast-grep --pattern 'interface RecordRejudgeResultDoc { $$$ }'packages/hydrooj/src/handler/manage.ts (1)
478-478: ```shell
#!/bin/bashSearch for RecordRejudgeDoc definition across the repo
rg -n "RecordRejudgeDoc" -A5 -B5 .
Inspect getRejudgeTask implementation
rg -n "getRejudgeTask" -A10 -B5 packages/hydrooj/src/model/record.ts
Inspect getMultiRejudgeTask implementation
rg -n "getMultiRejudgeTask" -A10 -B5 packages/hydrooj/src/model/record.ts
</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| const prefixes = new Set(Object.keys(window.LANGS).filter((i) => i.includes('.')).map((i) => i.split('.')[0])); | ||
| const langs = Object.keys(window.LANGS).filter((i) => !prefixes.has(i)).map((i) => ( | ||
| { name: `${i.includes('.') ? `${window.LANGS[i.split('.')[0]].display}/` : ''}${window.LANGS[i].display}`, _id: i } | ||
| )); |
There was a problem hiding this comment.
Review the language filtering logic
The current logic filters out base languages (like 'cpp') if any variant (like 'cpp.11') exists. This could lead to missing language options in the dropdown.
Consider this alternative approach to include all languages properly:
- const prefixes = new Set(Object.keys(window.LANGS).filter((i) => i.includes('.')).map((i) => i.split('.')[0]));
- const langs = Object.keys(window.LANGS).filter((i) => !prefixes.has(i)).map((i) => (
- { name: `${i.includes('.') ? `${window.LANGS[i.split('.')[0]].display}/` : ''}${window.LANGS[i].display}`, _id: i }
- ));
+ const langs = Object.keys(window.LANGS).map((i) => {
+ const parts = i.split('.');
+ const displayName = parts.length > 1
+ ? `${window.LANGS[parts[0]].display}/${window.LANGS[i].display}`
+ : window.LANGS[i].display;
+ return { name: displayName, _id: 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.
| const prefixes = new Set(Object.keys(window.LANGS).filter((i) => i.includes('.')).map((i) => i.split('.')[0])); | |
| const langs = Object.keys(window.LANGS).filter((i) => !prefixes.has(i)).map((i) => ( | |
| { name: `${i.includes('.') ? `${window.LANGS[i.split('.')[0]].display}/` : ''}${window.LANGS[i].display}`, _id: i } | |
| )); | |
| const langs = Object.keys(window.LANGS).map((i) => { | |
| const parts = i.split('.'); | |
| const displayName = parts.length > 1 | |
| ? `${window.LANGS[parts[0]].display}/${window.LANGS[i].display}` | |
| : window.LANGS[i].display; | |
| return { name: displayName, _id: i }; | |
| }); |
🤖 Prompt for AI Agents
In packages/ui-default/pages/manage_rejudge.page.ts around lines 15 to 18, the
current filtering logic excludes base languages if any variant exists, causing
some languages to be missing from the dropdown. Modify the logic to include all
languages regardless of variants by removing the filtering that excludes base
languages when variants are present. Adjust the mapping to correctly display
language names with their variant prefixes where applicable, ensuring all
languages appear in the dropdown.
| <label class="checkbox"> | ||
| {{ _('High priority') }} | ||
| <input name="high_priority" type="checkbox" class="checkbox" {% if high_priority %}checked{% endif %}> | ||
| </label> | ||
| <br> | ||
| <label class="checkbox"> | ||
| {{ _('Apply result after rejudge') }} | ||
| <input name="apply" type="checkbox" class="checkbox" {% if apply %}checked{% endif %}> | ||
| </label> | ||
| </label> |
There was a problem hiding this comment.
Fix invalid nested label elements
The HTML structure has nested <label> elements which is invalid according to HTML specifications.
Apply this fix to correct the HTML structure:
- <div class="medium-5 columns">
- <label>
- <label class="checkbox">
- {{ _('High priority') }}
- <input name="high_priority" type="checkbox" class="checkbox" {% if high_priority %}checked{% endif %}>
- </label>
- <br>
- <label class="checkbox">
- {{ _('Apply result after rejudge') }}
- <input name="apply" type="checkbox" class="checkbox" {% if apply %}checked{% endif %}>
- </label>
- </label>
- </div>
+ <div class="medium-5 columns">
+ <label class="checkbox">
+ {{ _('High priority') }}
+ <input name="high_priority" type="checkbox" class="checkbox" {% if high_priority %}checked{% endif %}>
+ </label>
+ <br>
+ <label class="checkbox">
+ {{ _('Apply result after rejudge') }}
+ <input name="apply" type="checkbox" class="checkbox" {% if apply %}checked{% endif %}>
+ </label>
+ </div>📝 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.
| <label class="checkbox"> | |
| {{ _('High priority') }} | |
| <input name="high_priority" type="checkbox" class="checkbox" {% if high_priority %}checked{% endif %}> | |
| </label> | |
| <br> | |
| <label class="checkbox"> | |
| {{ _('Apply result after rejudge') }} | |
| <input name="apply" type="checkbox" class="checkbox" {% if apply %}checked{% endif %}> | |
| </label> | |
| </label> | |
| <div class="medium-5 columns"> | |
| <label class="checkbox"> | |
| {{ _('High priority') }} | |
| <input name="high_priority" type="checkbox" class="checkbox" {% if high_priority %}checked{% endif %}> | |
| </label> | |
| <br> | |
| <label class="checkbox"> | |
| {{ _('Apply result after rejudge') }} | |
| <input name="apply" type="checkbox" class="checkbox" {% if apply %}checked{% endif %}> | |
| </label> | |
| </div> |
🤖 Prompt for AI Agents
In packages/ui-default/templates/manage_rejudge.html around lines 83 to 92,
there are nested <label> elements which is invalid HTML. Remove the extra
closing </label> tag at the end to ensure each checkbox has its own properly
closed <label> without nesting. This will fix the invalid HTML structure.
| return await document.add(domainId, '', doc.owner, document.TYPE_REJUDGE, null, null, null, doc); | ||
| } | ||
|
|
||
| static async pushRejudgeResult(rrid: ObjectId, newStatus: number, newScore: number, newRev) { |
There was a problem hiding this comment.
Add type annotation for the newRev parameter
- static async pushRejudgeResult(rrid: ObjectId, newStatus: number, newScore: number, newRev) {
+ static async pushRejudgeResult(rrid: ObjectId, newStatus: number, newScore: number, newRev: ObjectId) {📝 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.
| static async pushRejudgeResult(rrid: ObjectId, newStatus: number, newScore: number, newRev) { | |
| static async pushRejudgeResult(rrid: ObjectId, newStatus: number, newScore: number, newRev: ObjectId) { |
🤖 Prompt for AI Agents
In packages/hydrooj/src/model/record.ts at line 298, the method
pushRejudgeResult has a parameter newRev without a type annotation. Add an
appropriate type annotation to the newRev parameter to ensure type safety and
clarity in the method signature.
| static async pushRejudgeResult(rrid: ObjectId, newStatus: number, newScore: number, newRev) { | ||
| await RecordModel.collRejudge.updateOne({ _id: rrid }, { | ||
| $set: { | ||
| newStatus, | ||
| newScore, | ||
| newRev, | ||
| }, | ||
| }, { | ||
| upsert: true, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Method name doesn't reflect its actual behavior
The method name pushRejudgeResult suggests appending to an array, but it's actually performing an upsert operation.
Consider renaming the method to better reflect its behavior:
- static async pushRejudgeResult(rrid: ObjectId, newStatus: number, newScore: number, newRev: ObjectId) {
+ static async upsertRejudgeResult(rrid: ObjectId, newStatus: number, newScore: number, newRev: ObjectId) {📝 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.
| static async pushRejudgeResult(rrid: ObjectId, newStatus: number, newScore: number, newRev) { | |
| await RecordModel.collRejudge.updateOne({ _id: rrid }, { | |
| $set: { | |
| newStatus, | |
| newScore, | |
| newRev, | |
| }, | |
| }, { | |
| upsert: true, | |
| }); | |
| } | |
| static async upsertRejudgeResult( | |
| rrid: ObjectId, | |
| newStatus: number, | |
| newScore: number, | |
| newRev: ObjectId | |
| ) { | |
| await RecordModel.collRejudge.updateOne( | |
| { _id: rrid }, | |
| { | |
| $set: { | |
| newStatus, | |
| newScore, | |
| newRev, | |
| }, | |
| }, | |
| { | |
| upsert: true, | |
| } | |
| ); | |
| } |
🤖 Prompt for AI Agents
In packages/hydrooj/src/model/record.ts around lines 298 to 308, the method name
pushRejudgeResult is misleading because it suggests appending data, but the
method actually performs an upsert operation on the database. Rename the method
to something more descriptive like upsertRejudgeResult or
updateOrInsertRejudgeResult to accurately reflect its behavior.
| highPriority, | ||
| apply: _apply, | ||
| recordLength: rdocs.length, | ||
| rrdocs: await record.getMultiRejudgeTask(undefined, {}), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use consistent variable naming
Same issue as above - maintain consistent naming convention.
- rrdocs: await record.getMultiRejudgeTask(undefined, {}),
+ tasks: await record.getMultiRejudgeTask(undefined, {}),📝 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.
| rrdocs: await record.getMultiRejudgeTask(undefined, {}), | |
| tasks: await record.getMultiRejudgeTask(undefined, {}), |
🤖 Prompt for AI Agents
In packages/hydrooj/src/handler/manage.ts at line 443, the variable name used in
the call to record.getMultiRejudgeTask is inconsistent with the naming
convention used elsewhere. Rename the variable from undefined to a properly
named variable that matches the established naming pattern in the surrounding
code to maintain consistency.
| class SystemRejudgeHandler extends SystemHandler { | ||
| async get() { | ||
| this.response.body = { | ||
| rrdocs: await record.getMultiRejudgeTask(undefined, {}), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use consistent variable naming for rejudge task documents
The variable name rrdocs suggests "rejudge result documents" but it's actually storing rejudge task documents.
- rrdocs: await record.getMultiRejudgeTask(undefined, {}),
+ tasks: await record.getMultiRejudgeTask(undefined, {}),📝 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.
| rrdocs: await record.getMultiRejudgeTask(undefined, {}), | |
| tasks: await record.getMultiRejudgeTask(undefined, {}), |
🤖 Prompt for AI Agents
In packages/hydrooj/src/handler/manage.ts at line 370, rename the variable
`rrdocs` to a name that clearly reflects it stores rejudge task documents, such
as `rejudgeTaskDocs` or `rejudgeTasks`, to maintain consistent and descriptive
variable naming.
| const rid = await record.addRejudgeTask(domainId, { | ||
| owner: this.user._id, | ||
| apply: _apply, | ||
| }); |
There was a problem hiding this comment.
Add count field when creating rejudge task
The rejudge task should include a count of records to be rejudged.
const rid = await record.addRejudgeTask(domainId, {
owner: this.user._id,
apply: _apply,
+ count: rdocs.length,
});📝 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.
| const rid = await record.addRejudgeTask(domainId, { | |
| owner: this.user._id, | |
| apply: _apply, | |
| }); | |
| const rid = await record.addRejudgeTask(domainId, { | |
| owner: this.user._id, | |
| apply: _apply, | |
| count: rdocs.length, | |
| }); |
🤖 Prompt for AI Agents
In packages/hydrooj/src/handler/manage.ts around lines 448 to 451, the rejudge
task creation is missing a count field that indicates how many records are to be
rejudged. Modify the object passed to addRejudgeTask to include a count property
with the appropriate number of records to be rejudged, ensuring this count
accurately reflects the intended scope of the rejudge task.
| if (_apply) await record.reset(domainId, rdocs.map((rdoc) => rdoc._id), true); | ||
| else { | ||
| await record.collHistory.insertMany(rdocs.map((rdoc) => ({ | ||
| ...pick(rdoc, [ | ||
| 'compilerTexts', 'judgeTexts', 'testCases', 'subtasks', | ||
| 'score', 'time', 'memory', 'status', 'judgeAt', 'judger', | ||
| ]), | ||
| rid: rdoc._id, | ||
| _id: new ObjectId(), | ||
| }))); | ||
| } |
There was a problem hiding this comment.
Fix conditional logic for history insertion
The current logic creates history records when NOT applying (!_apply), but based on the context, history should be created before the actual rejudge operation regardless of the apply flag.
The history insertion should happen before rejudging to preserve the original state:
- if (_apply) await record.reset(domainId, rdocs.map((rdoc) => rdoc._id), true);
- else {
- await record.collHistory.insertMany(rdocs.map((rdoc) => ({
- ...pick(rdoc, [
- 'compilerTexts', 'judgeTexts', 'testCases', 'subtasks',
- 'score', 'time', 'memory', 'status', 'judgeAt', 'judger',
- ]),
- rid: rdoc._id,
- _id: new ObjectId(),
- })));
- }
+ // Always preserve history before rejudging
+ await record.collHistory.insertMany(rdocs.map((rdoc) => ({
+ ...pick(rdoc, [
+ 'compilerTexts', 'judgeTexts', 'testCases', 'subtasks',
+ 'score', 'time', 'memory', 'status', 'judgeAt', 'judger',
+ ]),
+ rid: rdoc._id,
+ _id: new ObjectId(),
+ })));
+
+ if (_apply) await record.reset(domainId, rdocs.map((rdoc) => rdoc._id), true);📝 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.
| if (_apply) await record.reset(domainId, rdocs.map((rdoc) => rdoc._id), true); | |
| else { | |
| await record.collHistory.insertMany(rdocs.map((rdoc) => ({ | |
| ...pick(rdoc, [ | |
| 'compilerTexts', 'judgeTexts', 'testCases', 'subtasks', | |
| 'score', 'time', 'memory', 'status', 'judgeAt', 'judger', | |
| ]), | |
| rid: rdoc._id, | |
| _id: new ObjectId(), | |
| }))); | |
| } | |
| // Always preserve history before rejudging | |
| await record.collHistory.insertMany(rdocs.map((rdoc) => ({ | |
| ...pick(rdoc, [ | |
| 'compilerTexts', 'judgeTexts', 'testCases', 'subtasks', | |
| 'score', 'time', 'memory', 'status', 'judgeAt', 'judger', | |
| ]), | |
| rid: rdoc._id, | |
| _id: new ObjectId(), | |
| }))); | |
| if (_apply) await record.reset(domainId, rdocs.map((rdoc) => rdoc._id), true); |
🤖 Prompt for AI Agents
In packages/hydrooj/src/handler/manage.ts around lines 453 to 463, the history
records are only inserted when _apply is false, but history should be recorded
before rejudging regardless of the _apply flag. Refactor the code to always
insert history records before the rejudge operation, then conditionally call
record.reset only if _apply is true.
Summary by CodeRabbit
New Features
Localization
User Interface