-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix: Respect individual placeholder checks if they can be changed #8317
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 enforces placeholder-specific editability in the frontend by introducing a placeholder_is_editable filter tied to the backend’s check_source method and wiring it into toolbar templates and styles to disable drag, drop, and plugin operations in read-only placeholders, alongside refactoring Sass spacing. Class diagram for Placeholder and placeholder_is_editable filterclassDiagram
class Placeholder {
+check_source(user)
}
class placeholder_is_editable {
<<filter>>
+__call__(placeholder, user)
}
placeholder_is_editable --> Placeholder: uses check_source(user)
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 there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `cms/templates/cms/toolbar/dragitem.html` </location>
<code_context>
+ {% if not allow_children or object_is_immutable or not placeholder|placeholder_is_editable:request.user %} cms-btn-disabled{% endif %}
</code_context>
<issue_to_address>
Tooltip message may not fully reflect all disabled states.
The tooltip should account for cases where the button is disabled due to permissions or immutability, not just the lack of allowed children.
</issue_to_address>
### Comment 2
<location> `cms/static/cms/sass/components/_structureboard.scss:453` </location>
<code_context>
display: none !important;
}
}
+ .cms-drag-disabled.cms-draggable .cms-dragitem {
+ // Remove drag handle from read-only dragables
+ background-image: none;
</code_context>
<issue_to_address>
Selector specificity may cause unintended style overrides.
Please verify that this selector does not unintentionally affect styles for non-disabled drag items, particularly in nested contexts.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8317 +/- ##
=======================================
Coverage 89.60% 89.61%
=======================================
Files 129 129
Lines 12710 12721 +11
=======================================
+ Hits 11389 11400 +11
Misses 1321 1321 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Looks good to me. Let's get this merged and released 🚢 🚀 |
) * fix: Respect placeholder checks of different PlaceholderRelationFields * fix: tests * fix: More specific css rule * fix: Performance optimization * Update cms/templates/cms/toolbar/dragbar.html * feat: Add preview option * feat: Add edit button icon * tests: Add test for get_edit_url template tag * fix: Avoid static pin to be selectable * tests: add test for plcaeholder_is_immutable filter * fix: ruff issues * Fix: formatting (missing tab)
) (#8318) * fix: Respect placeholder checks of different PlaceholderRelationFields * fix: tests * fix: More specific css rule * fix: Performance optimization * Update cms/templates/cms/toolbar/dragbar.html * feat: Add preview option * feat: Add edit button icon * tests: Add test for get_edit_url template tag * fix: Avoid static pin to be selectable * tests: add test for plcaeholder_is_immutable filter * fix: ruff issues * Fix: formatting (missing tab)
Description
In Django CMS 4+ placeholder changeability depends on the PlaceholderRelationField's check mechanism. In Django CMS 5 the structure board works for immutable models (such as published ones).
However, since the checks base on the PlaceholderRelationField, it means that placeholders belonging to different PlaceholderRelationFields might have different rules if they can be changed or not. This possibility has not been considered in the implementation.
While rare, this can happen within a model (with multiple RelaceholderRelationFields) or when displaying placeholders from a different model (e.g., static alias placeholders).
This PR fixes some inconsistencies in the frontend for such situations:
While rare, these events otherwise lead to inconsistent states between the backend and the frontend editor.
The solution introduces a small template filter for
cms_admintags that runs theplaceholder.check_source(user)method to see if a placeholder is editable. This is the exact check that the backend uses.Related resources
Checklist
mainSummary by Sourcery
Enforce backend placeholder changeability rules in the frontend by introducing a templating filter and context updates, disabling plugin interactions and updating styles for read-only placeholders to maintain consistent editor behavior
Bug Fixes:
Enhancements: