core: store problem content in seperate collection #896
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Co-authored-by: panda <panda_dtdyy@outlook.com>
| const problemContent = path.join(problemPath, 'problem.md'); | ||
| await fs.writeFile(problemContent, pdoc.content); | ||
| for (const i of pdoc.content) { | ||
| await fs.writeFile(path.join(problemPath, `problem_${i.name}_${i.lang}.md`), [ |
Check failure
Code scanning / CodeQL
Insecure temporary file High
WalkthroughThe changes introduce a new structured type for problem content, enabling support for multiple localized content entries. The Estimated code review effort4 (60–120 minutes) 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/model/document.tsOops! Something went wrong! :( ESLint: 9.31.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@hydrooj/eslint-config' imported from /eslint.config.mjs packages/hydrooj/src/service/bus.tsOops! Something went wrong! :( ESLint: 9.31.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@hydrooj/eslint-config' imported from /eslint.config.mjs packages/hydrooj/src/interface.tsOops! Something went wrong! :( ESLint: 9.31.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: 1
🧹 Nitpick comments (1)
packages/hydrooj/src/model/problem.ts (1)
564-574: Consider consistent content handling between edit and add operations.When editing a problem, the content is always wrapped in a localized array (lines 564-566), but when adding a new problem, the raw content is passed (line 572). While this works due to the
addmethod's normalization, consider making the handling consistent.: await ProblemModel.add( - domainId, pid, title.trim(), overrideContent || pdoc.content?.toString() || 'No content', + domainId, pid, title.trim(), overrideContent || [{ + name: 'default', content: pdoc.content?.toString() || 'No content', lang: 'en', + }], operator || pdoc.owner, tag, { hidden: options.hidden || pdoc.hidden, difficulty: pdoc.difficulty }, );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/hydrooj/src/interface.ts(2 hunks)packages/hydrooj/src/model/document.ts(1 hunks)packages/hydrooj/src/model/problem.ts(7 hunks)packages/hydrooj/src/service/bus.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/hydrooj/src/service/bus.ts (4)
packages/ui-default/pages/problem_edit.page.js (1)
content(245-245)packages/hydrooj/src/interface.ts (1)
ProblemDoc(190-190)packages/hydrooj/src/model/problem.ts (1)
ProblemDoc(35-35)framework/framework/interface.ts (1)
VoidReturn(21-21)
packages/hydrooj/src/model/problem.ts (1)
packages/hydrooj/src/interface.ts (2)
LocalizedContent(152-152)ProblemDoc(190-190)
⏰ 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 (8)
packages/hydrooj/src/interface.ts (1)
152-153: LGTM! Well-structured type definitions for localized content.The new
LocalizedContenttype and updatedContentunion type provide a clean way to support multiple localized content entries while maintaining backward compatibility with string content.packages/hydrooj/src/model/document.ts (1)
52-62: LGTM! Improved type safety for document content.Using
DocType[T]['content']instead of the genericContenttype ensures that each document type uses its specific content structure, improving type safety throughout the codebase.packages/hydrooj/src/service/bus.ts (1)
69-69: LGTM! Event signature properly updated for new content structure.The event signature correctly uses
ProblemDoc['content']to match the new localized content structure.packages/hydrooj/src/model/problem.ts (5)
45-62: LGTM! Well-implemented localized content discovery.The function properly handles multiple language variants and file types (MD/PDF). The regex pattern correctly matches files with language suffixes.
Note: There's a TODO comment about parsing markdown headers to extract metadata, which could be tracked as a future enhancement.
111-111: LGTM! Correct default values for localized content.The empty array
[]is the appropriate default value for the newLocalizedContenttype.Also applies to: 132-132
145-160: LGTM! Excellent backward compatibility handling.The method properly accepts both string and array formats, normalizing strings to the new localized content structure with sensible defaults (lang: 'en', name: 'default').
664-674: LGTM! Well-structured export of localized content.The export logic properly handles multiple content variants, creating separate files with clear naming (
problem_${name}_${lang}.md) and including metadata in YAML front matter.
1-728: PR title doesn't match the implementation.The PR title mentions "store problem content in separate collection", but the changes don't create or use a separate MongoDB collection. Instead, they restructure the content field within the existing problem documents to support localized content arrays.
Likely an incorrect or invalid review comment.
| pid: string; | ||
| title: string; | ||
| content: string; | ||
| content: LocalizedContent; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using the Content type instead of LocalizedContent for backward compatibility.
The ProblemDoc interface uses LocalizedContent directly, but the Content type definition on line 153 suggests that content can be either a string or LocalizedContent. This might break backward compatibility if existing code expects string content.
- content: LocalizedContent;
+ content: Content;📝 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.
| content: LocalizedContent; | |
| content: Content; |
🤖 Prompt for AI Agents
In packages/hydrooj/src/interface.ts at line 170, replace the type of the
`content` property in the `ProblemDoc` interface from `LocalizedContent` to the
`Content` type. This change ensures backward compatibility by allowing `content`
to be either a string or `LocalizedContent`, matching the existing `Content`
type definition on line 153.
Summary by CodeRabbit
New Features
Refactor