core&ui: add contest language limit (#1007) - #1014
Conversation
WalkthroughThis set of changes introduces support for configuring and selecting allowed programming languages across contest, homework, and problem configuration interfaces. Backend handler methods for contest and homework editing ( 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/ui-default/components/autocomplete/DomainSelectAutoComplete.tsxOops! Something went wrong! :( ESLint: 9.31.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@hydrooj/eslint-config' imported from /eslint.config.mjs packages/ui-default/components/autocomplete/UserSelectAutoComplete.tsxOops! Something went wrong! :( ESLint: 9.31.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@hydrooj/eslint-config' imported from /eslint.config.mjs packages/ui-default/pages/home_domain.page.tsxOops! Something went wrong! :( ESLint: 9.31.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@hydrooj/eslint-config' imported from /eslint.config.mjs 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✨ 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: 4
🧹 Nitpick comments (1)
packages/ui-default/templates/homework_edit.html (1)
100-107: LGTM! Form field implementation is correct.The new submission language limit form field is properly implemented using the form helper macro. The field configuration, positioning, and default value handling are all appropriate.
Consider using a more specific placeholder like
_("Language codes separated by commas, e.g., cpp,java,python")to provide better user guidance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
packages/components/frontend/autocomplete/AutoComplete.tsx(1 hunks)packages/hydrooj/src/handler/contest.ts(2 hunks)packages/hydrooj/src/handler/homework.ts(2 hunks)packages/hydrooj/src/handler/problem.ts(1 hunks)packages/hydrooj/src/interface.ts(1 hunks)packages/ui-default/components/autocomplete/LanguageSelectAutoComplete.tsx(1 hunks)packages/ui-default/components/autocomplete/components/LanguageSelectAutoComplete.tsx(1 hunks)packages/ui-default/components/problemconfig/BasicForm.tsx(2 hunks)packages/ui-default/components/problemconfig/ProblemConfigForm.tsx(2 hunks)packages/ui-default/locales/zh.yaml(2 hunks)packages/ui-default/pages/contest_edit.page.ts(2 hunks)packages/ui-default/pages/domain_edit.page.ts(0 hunks)packages/ui-default/pages/domain_edit.page.tsx(1 hunks)packages/ui-default/templates/contest_edit.html(1 hunks)packages/ui-default/templates/homework_edit.html(1 hunks)
💤 Files with no reviewable changes (1)
- packages/ui-default/pages/domain_edit.page.ts
🧰 Additional context used
🧬 Code Graph Analysis (3)
packages/ui-default/pages/contest_edit.page.ts (1)
packages/ui-default/components/autocomplete/LanguageSelectAutoComplete.tsx (1)
LanguageSelectAutoComplete(27-54)
packages/ui-default/pages/domain_edit.page.tsx (2)
packages/ui-default/misc/Page.ts (1)
NamedPage(37-37)packages/ui-default/components/autocomplete/LanguageSelectAutoComplete.tsx (1)
LanguageSelectAutoComplete(27-54)
packages/hydrooj/src/handler/contest.ts (2)
framework/framework/decorators.ts (1)
param(111-111)packages/hydrooj/src/model/setting.ts (1)
langs(358-358)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (javascript)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (20)
packages/hydrooj/src/handler/problem.ts (1)
307-307: LGTM! Correct integration of contest language limits.The addition of
this.tdoc?.langsto the language filtering array follows the existing pattern and correctly integrates contest-level language restrictions with domain and problem-level restrictions. The optional chaining ensures safe access.packages/hydrooj/src/interface.ts (1)
272-272: LGTM! Appropriate interface extension.The addition of the optional
langs?: string[]property to theTdocinterface correctly supports the new language limit feature while maintaining backward compatibility.packages/ui-default/pages/contest_edit.page.ts (2)
3-3: LGTM! Correct import addition.The import of
LanguageSelectAutoCompletefollows the existing import pattern in the file.
13-13: LGTM! Consistent autocomplete initialization.The initialization of
LanguageSelectAutoCompletefollows the same pattern as other autocomplete components in the file and correctly enables multi-selection for language limits.packages/ui-default/templates/contest_edit.html (1)
135-142: LGTM! Well-integrated form field addition.The new "Submission language limit" field is properly integrated into the Contest Settings section. The field name
langscorrectly matches the JavaScript initialization, and the value binding withtdoc.langs|default('')handles both new and existing contests appropriately.packages/ui-default/pages/domain_edit.page.tsx (1)
1-8: LGTM! Excellent refactoring to use centralized component.This new implementation is much cleaner and simpler than the previous manual language selection logic. Using the
LanguageSelectAutoCompletecomponent reduces code duplication and centralizes language selection functionality, which is a good architectural improvement.packages/ui-default/locales/zh.yaml (2)
824-826: LGTM! Translation additions look correct.The Chinese translations for "Select All" and "Select None" are accurate and appropriately placed in the locale file.
889-889: LGTM! Submission language limit translation is appropriate.The Chinese translation "提交语言限制" for "Submission language limit" is accurate and follows the existing translation patterns in the file.
packages/hydrooj/src/handler/contest.ts (2)
313-313: LGTM! Parameter implementation is correct.The
langsparameter is properly integrated into the function signature with an appropriate default value and correctly passed to thecontest.editcall. The implementation follows existing patterns in the codebase.Also applies to: 354-354
308-308:langsParameter Integration Approved; Typo Consistent Across CodebaseThe
langsparameter is declared, typed, and passed correctly, following existing patterns. The spelling “CommaSeperatedArray” appears consistently in multiple handlers and in the framework’s Types definition and tests, so this PR doesn’t introduce the typo—no changes required here.packages/components/frontend/autocomplete/AutoComplete.tsx (1)
313-326: LGTM! Excellent defensive programming improvement.The updated rendering logic properly handles cases where
renderItemreturns falsy values by skipping those items instead of rendering empty list elements. This prevents potential UI issues and makes the component more robust.The addition of the wrapping
<div>around the inner content maintains consistency with the existing structure while providing the necessary container for the rendered item.packages/ui-default/components/problemconfig/ProblemConfigForm.tsx (2)
76-81: LGTM! Component usage is simplified and cleaner.The update to use
LanguageSelectAutoCompletesimplifies the component by removing manual data processing and filtering logic. The direct use of thelangsarray from state and simplified onChange handler is cleaner and more maintainable.The placeholder logic is also improved, providing clear feedback about the current state (unlimited vs. specific languages selected).
9-9: Import path validated. TheLanguageSelectAutoCompletecomponent is present at
- packages/ui-default/components/autocomplete/components/LanguageSelectAutoComplete.tsx
No further changes needed.
packages/ui-default/components/problemconfig/BasicForm.tsx (1)
5-5: LGTM: Import change aligns with new component architecture.The import change to the new
LanguageSelectAutoCompletecomponent is consistent with the centralized language selection implementation.packages/ui-default/components/autocomplete/components/LanguageSelectAutoComplete.tsx (1)
5-8: Language processing logic looks robust.The prefix extraction and data transformation logic correctly separates prefixed languages from their base forms and creates display names with hierarchical structure.
packages/hydrooj/src/handler/homework.ts (3)
194-194: Parameter decoration follows established pattern.The
@paramdecorator forlangscorrectly usesTypes.CommaSeperatedArraywhich is consistent with theassignparameter handling.
199-199: Method signature update is consistent.The parameter addition with default empty array follows the same pattern as other optional array parameters in the method.
230-230: contest.edit already supports the langs field
TheTdocinterface in packages/hydrooj/src/interface.ts defineslangs?: string[];and
contest.editin packages/hydrooj/src/model/contest.ts (around line 813) accepts aPartial<Tdoc>, so anylangsproperty passed in$setwill be persisted viadocument.set. No further changes are needed.packages/ui-default/components/autocomplete/LanguageSelectAutoComplete.tsx (2)
10-25: State management in Component wrapper looks good.The React component properly manages internal state and handles both array and string values for selectedKeys conversion.
42-53: Component initialization and ref handling is well implemented.The attach method properly initializes the React component with the DOM value and sets up the ref callback for value access.
| onChange={(val) => { | ||
| let value = val.split(','); | ||
| const active = new Set(value.filter((i) => i.includes('.')).map((i) => i.split('.')[0])); | ||
| value = value.filter((i) => !prefixes.has(i) || active.has(i)); | ||
| for (const i of active) if (!value.includes(i)) value.push(i); | ||
| props.onChange(value); | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Complex onChange logic may have edge cases.
The logic handles prefixed language relationships but has potential issues:
- Line 17:
val.split(',')assumesvalis always a string, but it could be an array in some cases - Line 20: The loop adds base languages but doesn't validate if they exist in the original value
Consider adding type guards and validation:
onChange={(val) => {
- let value = val.split(',');
+ let value = Array.isArray(val) ? val : val.split(',');
const active = new Set(value.filter((i) => i.includes('.')).map((i) => i.split('.')[0]));
value = value.filter((i) => !prefixes.has(i) || active.has(i));
- for (const i of active) if (!value.includes(i)) value.push(i);
+ for (const i of active) {
+ if (!value.includes(i) && Object.prototype.hasOwnProperty.call(window.LANGS, i)) {
+ value.push(i);
+ }
+ }
props.onChange(value);
}}🤖 Prompt for AI Agents
In
packages/ui-default/components/autocomplete/components/LanguageSelectAutoComplete.tsx
around lines 16 to 22, the onChange handler assumes val is always a string and
splits it, which can cause errors if val is an array. Add a type check to handle
both string and array inputs properly. Also, before adding base languages in the
loop, validate that they exist in the original input to avoid adding unintended
values. Adjust the logic to safely process val and ensure only valid base
languages are included before calling props.onChange.
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
packages/ui-default/components/autocomplete/UserSelectAutoComplete.tsx (1)
19-22: Action Required: Implement or ForwardgetSelectedItemKeysandgetSelectedItemson the AutoComplete refThe base
AutoCompleteclass correctly assigns your component’s ref (ref = null;andref={(ref) => { this.ref = ref; }}), but I didn’t findgetSelectedItemKeysorgetSelectedItemsdefined or forwarded inUserSelectAutoComplete.tsx. Without those methods onthis.ref,value()will throw at runtime.Please update
UserSelectAutoCompleteFCto expose those instance methods—either by:
- Defining them directly on the class or functional component via
useImperativeHandle(forforwardRef).- Forwarding the ref to an underlying component that already implements them.
For example, if you’re wrapping a library
Selectthat has these methods, you can:- const UserSelectAutoCompleteFC = (props) => { … } + const UserSelectAutoCompleteFC = forwardRef<YourRefType, Props>((props, ref) => { + const innerRef = useRef<YourRefType>(null); + useImperativeHandle(ref, () => ({ + getSelectedItemKeys: () => innerRef.current!.getSelectedItemKeys(), + getSelectedItems: () => innerRef.current!.getSelectedItems(), + })); + return <Select ref={innerRef} {...props} />; + });• File: packages/ui-default/components/autocomplete/components/UserSelectAutoComplete.tsx
• Location: around the export/definition of your componentOnce those methods are available on
this.ref, thevalue()implementation will work as intended.
🧹 Nitpick comments (1)
packages/ui-default/components/autocomplete/index.tsx (1)
22-22: Consider usingRecord<string, any>instead of{}The static analysis correctly flags the use of
{}as a type. For better clarity and type safety, consider usingRecord<string, any>as the default type parameter.-export default class AutoComplete<Options extends Record<string, any> = {}, Multi extends boolean = boolean> extends DOMAttachedObject { +export default class AutoComplete<Options extends Record<string, any> = Record<string, any>, Multi extends boolean = boolean> extends DOMAttachedObject {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/ui-default/components/autocomplete/CustomSelectAutoComplete.tsx(1 hunks)packages/ui-default/components/autocomplete/DomainSelectAutoComplete.tsx(1 hunks)packages/ui-default/components/autocomplete/FileSelectAutoComplete.tsx(1 hunks)packages/ui-default/components/autocomplete/LanguageSelectAutoComplete.tsx(1 hunks)packages/ui-default/components/autocomplete/ProblemSelectAutoComplete.tsx(1 hunks)packages/ui-default/components/autocomplete/UserSelectAutoComplete.tsx(1 hunks)packages/ui-default/components/autocomplete/index.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ui-default/components/autocomplete/LanguageSelectAutoComplete.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/ui-default/components/autocomplete/index.tsx (2)
packages/ui-default/components/DOMAttachedObject.ts (1)
DOMAttachedObject(26-129)packages/ui-default/components/autocomplete/UserSelectAutoComplete.tsx (1)
value(19-22)
🪛 Biome (1.9.4)
packages/ui-default/components/autocomplete/index.tsx
[error] 22-22: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build
- GitHub Check: Analyze (javascript)
- GitHub Check: build
🔇 Additional comments (5)
packages/ui-default/components/autocomplete/ProblemSelectAutoComplete.tsx (1)
10-14: LGTM! Consistent with the refactoring pattern.The changes properly delegate component rendering to the base
AutoCompleteclass by passing the functional component and its props.packages/ui-default/components/autocomplete/DomainSelectAutoComplete.tsx (1)
10-14: LGTM! Follows the established refactoring pattern.The changes correctly pass the component and props to the base class. The fixed height of '34px' appears appropriate for domain selection UI consistency.
packages/ui-default/components/autocomplete/CustomSelectAutoComplete.tsx (1)
14-19: LGTM! Proper implementation with type safety.The changes correctly implement the refactoring pattern while maintaining type safety through the generic
Multiparameter. Thedataprop is properly passed to support custom selection options.packages/ui-default/components/autocomplete/FileSelectAutoComplete.tsx (1)
17-22: Component and props configuration looks good.The refactoring correctly passes the component and required props including the typed
dataprop.packages/ui-default/components/autocomplete/UserSelectAutoComplete.tsx (1)
10-14: LGTM! Consistent refactoring implementation.The changes properly delegate rendering to the base class following the established pattern.
| render: () => '', | ||
| text: () => null, | ||
| multi: false, | ||
| multi: false as Multi, |
There was a problem hiding this comment.
Remove type assertion for the multi option
The type assertion false as Multi can cause type inconsistencies. Let TypeScript infer the correct type from the passed options.
- multi: false as Multi,
+ multi: false,📝 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.
| multi: false as Multi, | |
| multi: false, |
🤖 Prompt for AI Agents
In packages/ui-default/components/autocomplete/index.tsx at line 38, remove the
type assertion "false as Multi" from the "multi" option. Instead, just assign
the value false directly and let TypeScript infer the type automatically to
avoid type inconsistencies.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/ui-default/components/autocomplete/index.tsx (2)
38-38: Remove type assertion for themultioption.The type assertion
false as Multican cause type inconsistencies. Let TypeScript infer the correct type from the passed options.- multi: false as Multi, + multi: false,
94-97: Fix return type consistency in thevaluemethod.The multi-select case should return an array of numbers to match the type signature and be consistent with other implementations like
UserSelectAutoComplete.value(): Multi extends true ? (string | number)[] : string { - if (this.options.multi) return this.ref?.getSelectedItemKeys() ?? this.$dom.val(); + if (this.options.multi) return this.ref?.getSelectedItemKeys()?.map((i) => +i) ?? []; return this.ref?.getSelectedItems()[0] ?? null; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
build/prepare.js(1 hunks)packages/ui-default/components/autocomplete/FileSelectAutoComplete.tsx(1 hunks)packages/ui-default/components/autocomplete/components/LanguageSelectAutoComplete.tsx(1 hunks)packages/ui-default/components/autocomplete/index.tsx(4 hunks)packages/ui-default/components/problemconfig/BasicForm.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/ui-default/components/problemconfig/BasicForm.tsx
- packages/ui-default/components/autocomplete/components/LanguageSelectAutoComplete.tsx
- packages/ui-default/components/autocomplete/FileSelectAutoComplete.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
packages/ui-default/components/autocomplete/index.tsx
[error] 22-22: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build
- GitHub Check: Analyze (javascript)
- GitHub Check: build
🔇 Additional comments (3)
build/prepare.js (1)
63-63: LGTM! Necessary change to support TSX files.This change correctly expands the build configuration to include TypeScript JSX files (
.tsx) alongside TypeScript files (.ts), which is essential for the new React components being added to the codebase.packages/ui-default/components/autocomplete/index.tsx (2)
2-2: LGTM: React import correctly added.The explicit React import is necessary for the JSX syntax used in the refactored
attachmethod.
67-81: LGTM: React wrapper pattern is well-implemented.The refactored
attachmethod effectively creates a React wrapper that:
- Manages internal state with
useState- Handles component flexibility via the
componentoption- Correctly processes
selectedKeyswith proper string splitting and trimming- Maintains backward compatibility while enabling new features
This pattern centralizes React rendering logic and eliminates the need for subclasses to implement their own React wrappers.
| } | ||
|
|
||
| export default class AutoComplete<Options extends Record<string, any> = {}> extends DOMAttachedObject { | ||
| export default class AutoComplete<Options extends Record<string, any> = {}, Multi extends boolean = boolean> extends DOMAttachedObject { |
There was a problem hiding this comment.
Replace {} type with explicit object type.
The static analysis tool correctly flags the use of {} as a type. This type means "any non-nullable value" which is overly broad and can lead to type safety issues.
-export default class AutoComplete<Options extends Record<string, any> = {}, Multi extends boolean = boolean> extends DOMAttachedObject {
+export default class AutoComplete<Options extends Record<string, any> = Record<string, any>, Multi extends boolean = boolean> extends DOMAttachedObject {🧰 Tools
🪛 Biome (1.9.4)
[error] 22-22: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
🤖 Prompt for AI Agents
In packages/ui-default/components/autocomplete/index.tsx at line 22, replace the
generic type parameter default from {} to a more explicit object type such as
Record<string, unknown> or a defined interface to improve type safety and avoid
the overly broad {} type. Update the declaration of Options to use this explicit
object type instead of {}.
| if (this.pdoc.config.langs) t.push(this.pdoc.config.langs); | ||
| if (ddoc.langs) t.push(ddoc.langs.split(',').map((i) => i.trim()).filter((i) => i)); | ||
| if (this.domain.langs) t.push(this.domain.langs.split(',').map((i) => i.trim()).filter((i) => i)); | ||
| if (this.tdoc?.langs) t.push(this.tdoc.langs); |
There was a problem hiding this comment.
Also need check if tdoc.langs.length > 0, or none language can be submited.
3 participants
Summary by CodeRabbit
New Features
Improvements
Localization