Update page locking rules #462
Conversation
tl;dr - Wagtail now provides a better panel API - https://docs.wagtail.org/en/latest/extending/client_side_panels.html via wagtail/wagtail#13248 and follow ups. This means that InlinePanel/MultipleChooserPanel now respects the read-only attribute, so we don't need a whole separate template
Annotations are now lazy by default, so the quoting can be removed. https://docs.python.org/3/whatsnew/3.14.html#whatsnew314-deferred-annotations However, if third-party depends on `.__annotations__` then you need `from __future__ import annotations` -- needed for instance of code that import Django Views
The `aria_label_format` attribute was removed ref: wagtail/wagtail@8c18b13
There was a problem hiding this comment.
Done and initial pass, mostly all seems as expected. 👍
Will re-review once we have a direction on #462 (comment)
|
|
||
| class PageInBundleReadyToBePublishedLock(BaseLock): | ||
| """A lock that is enabled when the page is in a bundle that is ready to be published.""" | ||
| class PageReadyToBePublishedLock(WorkflowLock): |
There was a problem hiding this comment.
Currently, there is a scenario where the locking mechanism can be bypassed, allowing content to be edited when it should not be.
If a page is locked by the workflow, a Publishing Admin cannot edit it through the normal edit view, as expected. However, the PA can still apply a manual user lock to the same page. While that user lock is active, they are able to make edits. When they then remove their own lock, those changes persist, even though the page remains locked by the workflow and is still in the Ready to publish state. The UI continues to indicate that the page is locked by the workflow and cannot be edited, but the content has already been modified.
This effectively bypasses the workflow lock, as content changes are saved without the page leaving the Ready to publish state.
We should disallow user-level locking for all users when a page is already locked by the workflow. Editors must unlock or cancel the workflow before they can apply a user lock and make changes.
There was a problem hiding this comment.
@MebinAbraham can you share the step to reproduce this? I feel like I am missing something
There was a problem hiding this comment.
My mistake, just had another look and I think we are actually okay, it seems like I was testing with a super user, so the scenario is only an issue with a super user account. As a Publishing Admin I can see that the page is locked when workflow locks it. Just for super admin, the page is not locked even if it is locked by workflow..
So when page is locked by workflow, PA sees:
But super admin sees:
It would be nice for the super admin to have the same behaviour (SA should be able to bypass workflow i.e. local dev but not work around it), but happy as-is, given that the elevated role doesn't exist in live.
There was a problem hiding this comment.
This is hard to do without monkey_patching because of https://github.com/wagtail/wagtail/blob/f7407b5319f3ff3935632ee7ca7e6d59918b8e94/wagtail/admin/views/generic/mixins.py#L334-L339 I take it back. PagePermissionTester.can_lock ftw
There was a problem hiding this comment.
Seems to be mostly there but now after unlocking it, for superusers, it still says locked by workflow even though it is unlocked. Works fine for PA/PO.
MebinAbraham
left a comment
MebinAbraham
left a comment
There was a problem hiding this comment.
LGTM 👍
It would be great if some of this was added back to core. I have some concerns about the divergence and it would be good to stay aligned with core as much as possible, especially when it comes to some core capabilities like workflows/publish.
| And the "This page is included in a bundle that is ready to be published. You must revert the bundle to Draft or In preview in order to make further changes." text is displayed | ||
|
|
||
|
|
||
| Scenario: When page is Approved (Ready to publish) and not in a bundle, a Publishing Admin can publish it |
There was a problem hiding this comment.
Suggestion: We should also validate the inverse, i.e. prove that when in a bundle, they can't publish it.
|
|
||
| class PageInBundleReadyToBePublishedLock(BaseLock): | ||
| """A lock that is enabled when the page is in a bundle that is ready to be published.""" | ||
| class PageReadyToBePublishedLock(WorkflowLock): |
There was a problem hiding this comment.
Seems to be mostly there but now after unlocking it, for superusers, it still says locked by workflow even though it is unlocked. Works fine for PA/PO.
|
@zerolab Due to the higher priority of help/ info pages work and other priorities. Will come back to this, Dan and the other PR branched of this. |
|
Need to validate some odd behaviour I have seen, will double check and report back. |
False alarm, user error 🙈 |
|
@MebinAbraham I've addressed #462 (comment) in ac3dbf1 |
What is the context of this PR?
This PR updates the page locking rules:
Screencast
How to review
Follow-up Actions
List any follow-up actions (if applicable), like needed documentation updates or additional testing.