If a user without the 'Schedule content publication' permission edits a node that has been scheduled, when they save the node the scheduling data is lost.
Example:
User A creates an unpublished node, scheduled to publish in the future.
User B (without schedule permission) edits the unpublished node, makes a change & saves. The publish_on value has now been removed from the node.
Expected behaviour is that the scheduled date would remain, as User B doesn't have permission to change the scheduling settings.
This also happens with published content & the unpublish on setting.
I'm looking into this.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | 2989100-7.scheduled-time-lost.patch | 4.98 KB | jonathan1055 |
Comments
Comment #2
smazComment #3
smazI think this has two related issues:
#2651448: Publish and Unpublish fields are shown for users who do not have the permission
#2326533: The structure of values received in WidgetInterface::massageFormValues() varies depending on #access TRUE/FALSE
When a user doesn't have access to the scheduling fields, TimestampDatetimeNoDefaultWidget::massageFormValues() is given NULL for the values.
I'm guessing that because the user has access to $form['publish_on'] & $form['unpublish_on'] (it's only $form['scheduler_settings'] that has no access), the default value for the field isn't being applied because the user may have chosen to leave/set the fields blank.
Adding the following fixes the issue for me:
$form['publish_on']['#access'] = FALSE;
$form['unpublish_on']['#access'] = FALSE;
The default value (which is the current value) is then passed through to massageFormValues().
Patch attached, but I'm not sure if there are other cases where it won't work due to the comment at https://www.drupal.org/project/scheduler/issues/2651448#comment-10756814.
Cheers
Comment #4
jonathan1055 commentedHi Smaz,
Thanks for debuging this and providing a patch. I am surprised that this fault has not been noticed before. Maybe Core behaviour has changed? Or maybe we just never tried this scenario. Anyway we need test coverage for it but if you say it is fixed for you then that's a good start.
Jonathan
Comment #5
jonathan1055 commentedI have added a test for this bug. This patch just has the extra test, not the code fix, so should fail.
Comment #7
jonathan1055 commentedGood, the new test failed. Here's the test plus the code fix from @smaz
Comment #9
jonathan1055 commentedFixed and committed.
Thanks Smaz for providing the initial patch.
Comment #10
smazThank you!
Sorry, I never got around to adding a test - I've been struggling to get tests running locally for a while now & have been busy with the Umami/Out of the box initiative since.
Comment #11
jonathan1055 commentedNo apology needed! You found the bug and provided the patch to fix it.