When you set at Publish Options in a content type in default options to default status = not published, save_draft module is enabled and configured to allow all authenticated users to save draft, the button publish appears to all users who have the permission to create a new content type, even when you want this new content needs to be approved before publication.
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | interdiff.txt | 3.48 KB | dsnopek |
| #10 | save_draft-access-1778836-10.patch | 6.97 KB | dsnopek |
Comments
Comment #1
beeradb commentedThe issue here is that the access check for the save button and the draft button were too tightly coupled. The reasoning behind this seems to be to save users from improper configuration of user permissions, potentially allowing a user to save a draft which they do not have access to view.
I would argue that we can just strip that out, as we can't really save a site builder from improperly configuring permissions. Here's a patch which strips that check out, and as a result respects default node options.
Comment #1.0
beeradb commentedJust to complete the issue
Comment #2
rooby commentedWe can't just pull out all this functionality.
We need to make this configurable and the default settings for existing users of the module should be the same as current functionality.
I'm still not 100% clear on what is being asked for.
Currently if a user has the save draft permission and the permission to edit the given node, they get all the save draft related buttons, including publish.
Is what you want, to force users to be able to save a draft and then that user cannot publish the content until it has been approved by someone else first?
If so that is not really what this module is for, you should use the workflow module or the workbench module, which allow you to have content approval workflows before content is published.
This module is effectively just a more user friendly way of using the published checkbox.
Marking as postponed until we have clarified what the request is.
Comment #3
markdorisonIf a node's content type is defaulted to be not published, and a user does not have the administer content permission, they should not be able to save a published node. In the current state of the module, a user is able to bypass this and publish the node.
Comment #4
jcisio commentedI was going to file a new issue "More granular access check for save_draft" and I found this one.
Users without "administer nodes" normally could not publish a node unless it is configured to be published by default. Another case is modules like https://www.drupal.org/project/override_node_options allow users without that "God" permission to publish nodes. So save_draft would have to manage these cases.
Because otherwise user_access('save draft') is not flexible and we don't have any xxx_alter mechanism for that.
PS: However I checked the latest dev release and it seems that it was fixed (but not in the latest stable in April 2011). Mark as duplicate of #1120164: is a separate permission needed?. So I think there is nothing left to do (other than making a new stable release). If you use hook_form_alter to modify the node options, then it is your responsibility to do that *before* save_draft with something like the following:
Comment #5
jcisio commentedSorry I read #1 again and it is a separate bug. #4 is for integration with override_node_options.
In this case I think the patch #1 can be considered to be in the right direction. It is not that an user can edit unpublished node that he can publish it. Now it depends on what we want when a user edit a node that he can't publish:
- Don't change anything, as if he doesn't have "save draft" permission.
- Change the text from "Save" to "Save as draft" to keep the UI consistent.
In no way there is a button to publish that node.
Comment #6
jcisio commentedHere is a patch. I don't include a permission for more granular access check because I think the "save draft" permission should not override the current more granular access control (using a contrib module, or hook_form_alter to add/remove the node status checkbox etc.). It should respect the presence of that checkbox. It does one thing, and does it correctly. Otherwise it can't be considered as a drop-in replacement for the old "Publish" button.
Because it is a behavioral change, we can add to the release note that in some rare cases, to restore the original behavior, a module like override_node_options is required. In that case, all roles with "save draft" permission should be attributed with the "override NODE_TYPE published option" permission.
For reference, in D8 the same thing happens. There is no "save draft" permission, only the text that changes based on current node status and node type options: (in NodeForm.php)
Comment #7
rooby commentedThat is not all this module does. Saving a draft also allows you to save incomplete data, even for required fields.
I understand the problem properly now however I don't think that either of the patches sufficiently solve the issue since they stop users from being able to save a new node as a draft if they don't have access to the status checkbox.
The problem is complex because you have 3 factors:
These patches are good in that they stop a user without access to the status field from saving a new draft on published content, which means they would be changing the status, however it breaks the draft saving functionality in some cases, for example where nodes are unpublished by default and a user wants to be able to save a draft.
I think we also need some changes to the form for which buttons the user can see, instead of the all or nothing approach of the current patches.
Maybe in the case where the user doesn't have access to status?:
I don't think this will be such a rare case. It definitely needs to be in the release notes and probably also a note on the project page and/or a message in an update function.
Comment #8
dsnopekHere's a new patch that (effectively) implements these cases from comment #7:
... except it does it in a simplified way to prevent the patch from getting too complex. Here's how that works:
When
save_draft_access()returns FALSE, there is just a "Save" button on the form which does Drupal's default thing. That means for a content type that defaults to unpublished, on new content, if the user can't change the status, it will be unpublished (case nr 2 above). And for existing content, if the user can't change the status, then saving will keep it with the same status (cases nr 3 and 4 above).So, really, all we need to do is have
save_draft_access()return TRUE for new nodes defaulting to published (case nr 1 above), and FALSE for all the other cases. This doesn't change the button labels as described above, but I'm not sure it really needs to.I think this all or nothing approach actually works fine, because "nothing" (Drupal's default behavior) actually matches the effective result that we want!
Of course, this still needs tests, to validate all of the above.
Comment #9
dsnopekHere's changes to the existing tests to make them work with this patch, and some new tests that validate the behavior of the patch.
One thing to note: with the set of cases laid out in comment #7, we have to give the
$this->save_draft_userthe "administer nodes" permission, because otherwise they don't have access to the status element on the form, and so trigger the new cases.I think this points to something that might have been overlooked in those set of cases. We have case nr 1:
... but, they can't actually "iterate on drafts then save as published as intended" because of case nr 3:
Once they've saved their new node as a draft (per case nr 1), when they edit it again, they won't be able to publish it (per case nr 3).
So, maybe this logic needs to be amended slightly? What if we say that if a node type is publish by default, and the current user is the owner (so, likely the one who created it), and it's unpublished, that they'll be able to publish it?
Comment #10
dsnopekHere's a new patch that implements my idea at the end of comment #9, so that a user who doesn't have access to status can save a node as draft, edit it a bunch and then publish it later.
Please let me know what you think!
Comment #11
cboyden commentedWe've been using this patch for a while and it's working as expected.