Conversation
Contributor
📝 WalkthroughWalkthroughThis PR introduces a strongly-typed ParagraphSuggestionAction enum to replace string-based action parameters across the CMS and headless-LMS services. It removes the "generate" action group, updates the suggestion dialog UI to single-column per-suggestion rendering, replaces hard-coded colors with theme-driven values, and removes runtime action validation in favor of compile-time type safety. Localization files are updated across multiple languages. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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.
🧹 Nitpick comments (4)
services/headless-lms/models/src/cms_ai.rs (1)
76-116: Consider usingstrumto eliminate string duplication.The
as_strmethod duplicates the API strings already defined in#[serde(rename = "...")]attributes. This creates a maintenance burden where changes must be made in two places, risking drift.Consider using
strum_macros::AsRefStrorstrum_macros::Displaywith matching rename attributes, or implementingas_strvia serialization:♻️ Alternative approach using serde serialization
impl ParagraphSuggestionAction { /// Returns the API string value for this action. pub fn as_str(&self) -> &'static str { // Use a static map to avoid allocation on each call match self { // Keep the current implementation, but consider generating this // from a build script or macro to ensure sync with serde renames ... } } }Or with
strum:use strum_macros::AsRefStr; #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, AsRefStr)] #[cfg_attr(feature = "ts_rs", derive(TS))] pub enum ParagraphSuggestionAction { #[serde(rename = "moocfi/ai/generate-draft-from-notes")] #[strum(serialize = "moocfi/ai/generate-draft-from-notes")] GenerateDraftFromNotes, // ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/headless-lms/models/src/cms_ai.rs` around lines 76 - 116, The as_str method duplicates serde rename strings for ParagraphSuggestionAction leading to drift; replace the manual match with a derived string-representation using strum (or wire serde value) instead. Add strum_macros::AsRefStr (or Display) to the enum derive and annotate each variant with #[strum(serialize = "...")] matching the existing #[serde(rename = "...")] values, then change as_str to take &self and return self.as_ref() (or call the strum-provided method); alternatively derive both serde and strum from the same attributes so the API string is defined once for ParagraphSuggestionAction and remove the long manual match in impl as_str.services/headless-lms/server/src/ts_binding_generator.rs (1)
58-58: Movecms_ai::ParagraphSuggestionActionto maintain module grouping.The new export is placed between
chapters::ChapterWithStatusandchapters::DatabaseChapter, breaking the module-based grouping pattern. It should be placed after allchapters::*exports and beforechatbot_configurations::*exports to maintain alphabetical module ordering.♻️ Suggested placement
chapters::ChapterWithStatus, + cms_ai::ParagraphSuggestionAction, - cms_ai::ParagraphSuggestionAction, chapters::DatabaseChapter,Should become:
chapters::CourseUserInfo, chapters::ChapterLockPreview, chapters::UnreturnedExercise, + cms_ai::ParagraphSuggestionAction, chatbot_configurations::ChatbotConfiguration,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/headless-lms/server/src/ts_binding_generator.rs` at line 58, The export cms_ai::ParagraphSuggestionAction is currently inserted between chapters::ChapterWithStatus and chapters::DatabaseChapter, breaking the module grouping; move the cms_ai::ParagraphSuggestionAction line so it sits after all chapters::* exports (i.e., after chapters::DatabaseChapter and any other chapters::... entries) and before the chatbot_configurations::* exports to restore alphabetical/module grouping.services/cms/src/utils/Gutenberg/withParagraphAiToolbarAction.tsx (2)
386-390: Type mismatch:ai-dialog-label-suggestion-nis not inAiActionLabelKey.The translation key
"ai-dialog-label-suggestion-n"exists in cms.json but is not included in theAiActionLabelKeytype union (see menu.ts lines 40-61). While TypeScript won't catch this at compile time since you're not casting this particular call, consider either:
- Creating a separate type for dialog-related translation keys
- Extending
AiActionLabelKeyto include dialog keysThis would improve type consistency with the pattern used elsewhere in this file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/cms/src/utils/Gutenberg/withParagraphAiToolbarAction.tsx` around lines 386 - 390, The translation key "ai-dialog-label-suggestion-n" is used in withParagraphAiToolbarAction.tsx but isn't included in the AiActionLabelKey union; update types so the call is covered: either add the dialog key(s) (e.g., "ai-dialog-label-suggestion-n") to the AiActionLabelKey union in menu.ts or create a new dialog-specific union and use that type for dialog translation calls in withParagraphAiToolbarAction.tsx; ensure the translation call and any related functions reference the updated type (AiActionLabelKey or the new DialogLabelKey) to keep typings consistent.
325-326: Consider removing unused parameters or documenting why they're retained.The parameters
originalHtmlandallowedHtmlTagNamesare renamed with underscore prefixes indicating they're unused. If they're not needed in the current implementation, consider removing them from the props interface entirely rather than keeping dead parameters.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/cms/src/utils/Gutenberg/withParagraphAiToolbarAction.tsx` around lines 325 - 326, The props for withParagraphAiToolbarAction (in withParagraphAiToolbarAction.tsx) currently include originalHtml and allowedHtmlTagNames but they are renamed to _originalHtml and _allowedHtmlTagNames indicating they are unused; remove these dead props from the relevant props/interface definition and any places that pass them through (or if they must be kept for API compatibility, add a comment documenting why they are retained) — update the Props interface/type and the withParagraphAiToolbarAction parameter list (and any callers) to drop originalHtml and allowedHtmlTagNames, or add a clear TODO/comment above the props explaining why they are intentionally unused.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@services/cms/src/utils/Gutenberg/withParagraphAiToolbarAction.tsx`:
- Around line 386-390: The translation key "ai-dialog-label-suggestion-n" is
used in withParagraphAiToolbarAction.tsx but isn't included in the
AiActionLabelKey union; update types so the call is covered: either add the
dialog key(s) (e.g., "ai-dialog-label-suggestion-n") to the AiActionLabelKey
union in menu.ts or create a new dialog-specific union and use that type for
dialog translation calls in withParagraphAiToolbarAction.tsx; ensure the
translation call and any related functions reference the updated type
(AiActionLabelKey or the new DialogLabelKey) to keep typings consistent.
- Around line 325-326: The props for withParagraphAiToolbarAction (in
withParagraphAiToolbarAction.tsx) currently include originalHtml and
allowedHtmlTagNames but they are renamed to _originalHtml and
_allowedHtmlTagNames indicating they are unused; remove these dead props from
the relevant props/interface definition and any places that pass them through
(or if they must be kept for API compatibility, add a comment documenting why
they are retained) — update the Props interface/type and the
withParagraphAiToolbarAction parameter list (and any callers) to drop
originalHtml and allowedHtmlTagNames, or add a clear TODO/comment above the
props explaining why they are intentionally unused.
In `@services/headless-lms/models/src/cms_ai.rs`:
- Around line 76-116: The as_str method duplicates serde rename strings for
ParagraphSuggestionAction leading to drift; replace the manual match with a
derived string-representation using strum (or wire serde value) instead. Add
strum_macros::AsRefStr (or Display) to the enum derive and annotate each variant
with #[strum(serialize = "...")] matching the existing #[serde(rename = "...")]
values, then change as_str to take &self and return self.as_ref() (or call the
strum-provided method); alternatively derive both serde and strum from the same
attributes so the API string is defined once for ParagraphSuggestionAction and
remove the long manual match in impl as_str.
In `@services/headless-lms/server/src/ts_binding_generator.rs`:
- Line 58: The export cms_ai::ParagraphSuggestionAction is currently inserted
between chapters::ChapterWithStatus and chapters::DatabaseChapter, breaking the
module grouping; move the cms_ai::ParagraphSuggestionAction line so it sits
after all chapters::* exports (i.e., after chapters::DatabaseChapter and any
other chapters::... entries) and before the chatbot_configurations::* exports to
restore alphabetical/module grouping.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 04086ff6-e5e8-49e9-9006-278b311d34e3
📒 Files selected for processing (15)
services/cms/src/utils/Gutenberg/ai/abilities.tsservices/cms/src/utils/Gutenberg/ai/menu.tsservices/cms/src/utils/Gutenberg/withParagraphAiToolbarAction.tsxservices/headless-lms/chatbot/src/cms_ai_suggestion.rsservices/headless-lms/models/src/cms_ai.rsservices/headless-lms/models/src/lib.rsservices/headless-lms/server/src/controllers/cms/ai_suggestions.rsservices/headless-lms/server/src/ts_binding_generator.rsshared-module/packages/common/src/bindings.guard.tsshared-module/packages/common/src/bindings.tsshared-module/packages/common/src/locales/ar/cms.jsonshared-module/packages/common/src/locales/en/cms.jsonshared-module/packages/common/src/locales/fi/cms.jsonshared-module/packages/common/src/locales/sv/cms.jsonshared-module/packages/common/src/locales/uk/cms.json
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
1 participant
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.
Summary by CodeRabbit
Release Notes
New Features
Refactor