-
Notifications
You must be signed in to change notification settings - Fork 25
chore: Add test for editor note plugin #281
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
Conversation
Reviewer's GuideThis PR adds dedicated tests for the EditorNotePlugin (covering both standard and django CMS 4 inline editing scenarios), refines a frontend templatetag implementation, and updates test fixtures to defer publishing and improve placeholder retrieval. File-Level Changes
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 - here's some feedback:
- Remove the debug
print(response.content)
call fromtest_editor_note_with_cms4
, since it’s not needed in committed tests. - Fix the stray extra quote in the
update_component_properties
docstring (it currently reads" "Adds or appends…
). - In
fixtures.get_placeholders
, use thepage
parameter instead of hardcodingself.page
when fetchingPageContent
, to avoid confusion and improve reusability.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 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.
@@ -39,7 +39,7 @@ def is_inline_editing_active(context: template.Context) -> bool: | |||
|
|||
|
|||
def update_component_properties(context: template.Context, key: str, value: typing.Any, append: bool = False) -> None: | |||
""""Adds or appends the value to the property "key" of a component during delcaration""" |
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.
nitpick (typo): Docstring has an extra quote and a typo
Please remove the extra quote at the start and correct the spelling of 'declaration' in the docstring.
""""Adds or appends the value to the property "key" of a component during delcaration""" | |
"""Adds or appends the value to the property "key" of a component during declaration""" |
Summary by Sourcery
Add tests for the EditorNotePlugin and adjust test fixtures and template tags to support proper placeholder retrieval and inline editing behavior.
Enhancements:
Tests: