-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Proper registration of third-party RTE (CKEditor4 or 5, or other) #47
Conversation
Reviewer's Guide by SourceryThis pull request fixes an issue with third-party rich text editors not being registered correctly. The fix involves ensuring that the editor name is correctly extracted and used for registration, and that the editor name is truncated to a maximum length when saved to the database. Sequence diagram for third-party RTE registration and usagesequenceDiagram
participant App as Application
participant Config as Editor Config
participant Registry as RTE Registry
participant Model as Text Model
App->>Config: get_editor_config(editor)
Config->>Config: Extract config_name
alt Third-party editor not registered
Config->>Config: Import module
Config->>Registry: register(rte_config_instance)
end
Config->>App: Return configuration
App->>Model: Save text with RTE
Note over Model: Truncate RTE name to
Note over Model: max length (16 chars)
Class diagram showing Text model changesclassDiagram
class AbstractText {
+CharField rte
}
note for AbstractText "rte field max_length=16"
class RTEConfig {
+String name
+register()
}
AbstractText --> RTEConfig: uses
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @fsbraun - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #47 +/- ##
==========================================
+ Coverage 81.16% 81.20% +0.04%
==========================================
Files 17 17
Lines 929 931 +2
Branches 104 104
==========================================
+ Hits 754 756 +2
Misses 132 132
Partials 43 43 ☔ View full report in Codecov by Sentry. |
Fixes #46
Summary by Sourcery
Fix the registration of third-party rich text editors by correcting the configuration loading logic and caching. Enhance code maintainability by introducing a constant for maximum RTE length and ensure RTE names are truncated appropriately. Add an integration test to verify editor save functionality in Django CMS 4.
Bug Fixes:
Enhancements:
Tests: