Node options behaviour on editing by non-admin user

Thanks for providing the patch for this problem and the related issue @ http://drupal.org/node/7940.

It would be great if this would make it into 4.7!

- I think the form needs work; the title does not describe the options, and the options are not balanced. It's not clear what it is about until you read 'when users edit a post'.

- I think there is a 'to' missing in the form description.

- We call the 'post options', 'publishing options', I think.

drumm,
Now that the two dependencies have been committed to HEAD, do you think this one might make it, too?

+1 to this.

At very least when people get mad because "its not remembering my settings" we can ask if they set thier options properly. I can also see several ways you would want this to work depending on the makeup of your site's users and content so it will be good to be able to set this depending on the specifics.

typo: unpriviled

After a short discussion on IRC, I concluded that the new setting should behave more like the existing settings, rather than slightly different. That is, there should be a per node type default, but it should be able to overwrite this on a per node basis. It looks like there are use cases for that.

Any chance of someone able to updating this so we can test again? This is a very important usability issue that currently drives people nuts.

Some of us want to force remoderation for new versions of things. It would be nice if both options worked: delegating a node to a user so that they can edit unsupervised, and insisting on supervision of each new revision.

sepeck marked http://drupal.org/node/50630 as a duplicate of this, but 50630 asks for the opposite behavior of this request: if the current version has been marked unpublished due to an edit, we are looking for the node module to use the revision system to display previous version that was marked published, not to automatically publish new revisions. No objection to that feature, just not what we need. This would require, in 4.7, the revision system to store the moderation/publication status for back revisions.

As a new member of the community here, I didn't think it appropriate to engage in a dispute about the definition of 'duplicate' status, so I'm adding this note. If the person assigned to this issue thinks that this is an unreasonable request atop the original goal I apologize, but I don't see what else to do.

bump?

drumm - are you planning to work on this? groups.drupal.org is getting its groups unpublished and moderated on every edit because of this behavior. thats not desired. we just want to approve new groups ... i will probably just publish everything immediately soon rather than re-moderate after minor edits.

updated drumm's patch for HEAD. as dries suggested, this patch lets each content type decide whether its options should reset upon non admin edit

Just curious... will this be backported into 4.7?

I don't think that this qualifies for getting backported to 4.7, since it's a new feature and it affects existing node workflow behaviour. However, that's up to killes as the 4.7 maintainer.

I am also reluctant for this setting to even go into HEAD, as it seems like a hack to me. I would rather see http://drupal.org/node/48731 get fixed up and committed ('save revision as draft feature'), as that is a solution that will work without any extra UI cruft being needed. The better approach would be to recommend that site admins enable revisions for content that gets moderated and approved, and to then allow users to edit published and promoted content as much as they like - just that their changes get saved in a draft revision, and the old content remains published until the new revision gets approved.

hey, i'm OK if some other patch solves the need. does that patch actually let an admin do initial moderation and then allow subsequent edits to be immediately published (i.e. no reset of options)? it seems to focus on other use case concerning drafts, which is the opposite of what is wanted by groups site and many others.

also, that other patch is likely to linger in patch queue for a long while because not that many people care about it and it is a big change. it should go through, but this provides some relief from the bug now.

no features will ever get backported to 4.7.

There is an exception to every rule. This feature was backported into 4.7: http://drupal.org/node/60453

@moshe: I think I figured out how to avoid having the moderation bit reset by using hook_nodeapi to restore the value. You could do the same in the 4.7 version of og.module if it's a problem now. See hook_nodeapi in:

http://drupal.org/project/content_moderator

i have a hack like that in place too ... lets fix core instead :)

I agree with Jaza (#22).
setting as "Patch Needs Work" until http://drupal.org/node/48731 is fixed.
(in any case, the current patch is probably out of sync.).

the 'save as draft' ferature is not a replacement for this fix. lets not create dependencies where they aren't needed.

I've spent my entire afternoon wondering why I can't get Drupal 4.7 to simply allow an authenticated user to edit their post and have it stay on the front page with no fancy drafts, revisions, etc - and I wonder why some people are sOoooo darn adament that no one should EVER commit anything to core which would let an administrator do this by simply checking a box. (no one says if the checkbox is there that you have to use it right?)

User's not being able to edit posts without having them go places ='s not really a community site by the standards of many community sites out there. It's fine if one doesn't want to run their community site this way, but I don't understand why the functionality I want can't be included in core either.

Saving a revision as draft does not address the simple functionality that many seem to be asking for (which doesn't mean the both could exist side-by-side, right).

< /rant>

Rant followup for Drupal 4.7:

Obviously, it would be better to have the patch submitted by Drumm/moshe, but since 4.7 is apparently an unsupported Drupal version I came up with my own solution. ;-)

Here it is for anyone searching:

With my workflow I had an issue that when users update their stuff things would get unpromoted/demoted.

After pulling my hair out for hours I finally figured out that by changing 10 characters I could get things working right (e.g., user COULD edit their posts without it getting unpromoted):

Changed this line in node.module:

foreach (array('status', 'moderate', 'promote', 'sticky', 'revision') as $key)

to:

foreach (array('status', 'moderate', 'sticky', 'revision') as $key)

Now it worky. Imagine that one could take other things out of that line and get a desired effect, too. (though I haven't tried it)

What's core for if not for hacking, right!? :-)

=====
Bloggyland.com
Hosting for pre-tuned/pre-configured Drupal installations
No set-up time, instant fully-functioning site with many extra modules
Drupal specific support, SSH/FTP/cPanel

Always remember: If you're hacking Drupal core, you're doing something wrong. :P

Probably you want a hook_form_alter in a custom module which will disable that checkbox for you.

Obviously, it would be better to have the patch submitted by Drumm/moshe, but since 4.7 is apparently an unsupported Drupal version I came up with my own solution. ;-)

Just for reference: there is a big difference between 'unsupported' and 'not the place where new features are added.' ;)

Thanks to moshe for pointing me here from my original posting:

http://drupal.org/node/81314

Seems like this important issue got lost in complexity.

I really think that my simple patch (re-appended) is a better default behaviour - it seems to me that the above discussion involving system settings and forms to alter the default is kind of marginal and obscure from the point of view of most sites (yeah, call me selfish). I'd like to see my patch implemented and if anyone feels strongly about the current default behaviour, I'm willing to work on the code to allow an override (but node specific? I think it might be time to step back and think about other ways of dealing with the corresponding use case ..). I'll bet that most of the current site implementors would actually be surprised at the current behaviour.

[A small note - this patch differs from the previous one in that it still has the security that non-admin posters can't modify the options values]

[also - i think this is really a bugfix, not a feature, but I'll leave the original, though i've updated the title]

I guess this is going to get lost again because of the code freeze, so I'm marking it as a bug, since that's what I think it is ...

+1 in general for this change and marking it a bug. I'm not sure I like the implementation, however - I need to look a little more at the surrounding context in the node module.

Also, I think the patch file isn't going to work, since it attempts to patch the file "node.module.patched". Most people prefer if you make your diffs from the Drupal root directory:
using cvs: "cvs diff -up modules/node/node.module"
using diff: " diff -up modules/node/node.module.orig modules/node/node.module"
where node.module is your modified file.

Before we can review any patch, we need to reach a consensus on the following questions:

Question 1: what are we trying to achieve?
a) Add an option for the admin to force a node status to stay the same even after an edit by a user.
b) Do not revert the status upon user edit.

Question 2: what is the status of this issue?
c) it's a feature request
d) it's a bug.

I doubt that b) will be committed in this (pre-5.0) release cycle, and a) is definitely a feature.

Postpone?

Just wanted to mention that the UNdesired behavior, IMO, of having updates to groups on groups.drupal.org be moderated is mentioned here.

Specifically, when a group is edited this message is displayed at the top of the homepage: "The post has been submitted for moderation and won't be accessible until it has been approved."

IMO, if the web site administrator CAN choose whether or not updates to groups ARE moderated, I see this as a feature.

IMO, if the web site administrator can NOT choose whether or not updates to groups ARE moderated, I see this as a bug or, at the least, as an UNdesired feature.

By the way, I didn't mean for the "Version" to be changed from "x.y.z" (what it was before my post) to "4.7.4" (what it is after my post).

I don't see "x.y.z" on the drop-down menu under "Version". For future reference, should I have chosen "none" as the version and would this have kept it as "x.y.z"? Thank you!

No, "x.y.z" is just the name that was given to what used to be "cvs" in the old project module. There've been a number of updates around the release system and one of them was that kind of confusing one. ;)

The right thing to do is to move issues from x.y.z to a "real" version, like 4.7.4, so you're good. ;)

However, I think this probably belongs @ 6.x-dev since it smells feature-ish. So moving there.

i think this is a real bug, that should be fixed in D5, not just put off until D6. in fact, i might even try to roll a patch for 4.7.x, but i don't think i want to do battle with killes over that.

this is causing real problems right here on d.o: http://drupal.org/node/107951

i'd propose the following simple approach:

if a user doesn't have permissions to modify the workflow options on a node, their edits should never modify those options.

basic justification: if a node's current settings don't match the defaults, it's because something/someone set it that way on purpose; either an admin, a module, etc. if the user doesn't have permissions to modify these settings, editing the node should respect whatever thing(s) caused the settings to differ from the defaults in the first place.

to show how this solves the problems mentioned above:

a) groups: groups default to unpublished (in the moderation queue, whatever). when a g.d.o admin publishes, it's now published. at no point can regular group owners publish/unpublish their own groups, only admins can do that (as expected).

b) release nodes: default to unpublished. packaging script publishes them once the tarballs are generated. again, release node owners can't publish their own releases prematurely, but they're free to edit all they want without unpublishing. only admins (and the packaging script) can publish nodes (as expected).

no extra UI cruft, no fancy settings, nothing. just make the default behavior make sense. if people need something more fancy regarding drafts, revisions, moderation, blah, blah, put it in a fancy contrib that manages fancy workflow stuff. however, core should be simple, clean, and intuitive. the current behavior fails on all 3 counts. what's there now is confusing, totally unexpected [1], and needlessly complicated.

i'll try to roll a patch against HEAD for (hopeful) inclusion in 5.0-RC2. stay tuned.

thanks,
-derek

[1] it's bad enough that i was fooled by the current behavior regarding release nodes, but i'm still a relative a novice by some counts. however, the fact that user/23 (moshe) got this wrong, too, should be ample evidence that the current behavior is hopelessly non-intuative.

ok, i guess the new patch isn't significantly simpler in the implementation, as i was hoping. ;) it just moves some logic around so that the defaults only get stuffed in for new nodes. tested locally and works fine.

FYI, I just created super small module at http://drupal.org/project/override_node_options which allows users to override the default publishing options for nodes they can edit without giving them the 'administer nodes' permission. Maybe this could help out in the meantime. For D6 maybe we could flush out the perms a bit more like this.

Patch works as advertised. +1 to getting this into core asap.

@RobRoy, I think that module is addressing some other need than the simple fix that dww is talking about -- though it may be appropriate to the confusing mass of comments above that which have muddled this poor issue so completely.

Let's stay focused on the simple problem outlined in #41, which _is_ a bug and _might_ be able to make it into D5.

Will try and test this at my next available opportunity.

dww - I agree totally with the goal, but I'm wondering about the implementation for a couple reasons. With FAPI and the '#access' element, is it actually possible for users to modify the form (per the existing code comment)?

Also, I really think these should be set at the same place in the code where they are currently- before the calls hook_submit and hook_nodeapi. Otherwise, you've just taken away the ability of any module to use hook_submit or hook_nodeapi() to change the node options for new nodes. For example, I think this would negate part of the code currently in book_submit

both are excellent points. here's a new patch that still accomplishes my goals and addresses both of pwolanin's concerns. while i was at it, i added some better comments (adhereing to the new standard). ;) tested heavily on a local site.

in case there's still any doubt this is a bug, consider this scenario:

a) default for forum nodes is to be published
b) non-admin user foo creates a questionable forum post
c) admin unpublishes node, and warns the user
d) user foo can edit their own forum node, and as soon as they do (even if they don't fix anything, or even make it worse), it's now published again until an admin notices and re-unpublishes.

i just tested this whole scenario before and after my patch. current core re-publishes the content at step (d), against the wishes of the node admin. with my patch, at step (d) the node remains unpublished until something else (the admin, a digg-style voting module, whatever) decides to republish the node again. i'd almost consider this a critical bug, since regular users should never be able to subvert the decisions of node administrators...

This may be critical, yes. I have other scenarious though -- let's imagine a site where content is unpublished by default and node admin publishes them. If I do not give 'edit own' permissions then noone can edit the simplest typo. If I give them 'edit won' permissions and this patch goes through then they can edit when the article is published which definitely does not fit the goals outlined for this fictional site.

My followup obviously is hard to understand. Let me try again. We have a site where every content needs to go through a node administrator. For the sake of simplicity, I will call 'users' everyone who is not a node administrator. First of all, I would like to prove that users need 'edit own' permission: if they do not have that, then they can not fix the simplest mistakes. So, we need to give users 'edit own' permission. In Drupal's current state, if a user edits her own content then it reverts to unpublished state. But if this patch goes through then it will remain published which is not desired in this scenario.

I am convinced now: this is a corner case, to be handled by contrib. Let this fly.

so, to clarify, the key goal for this fictional site is: "once published, no one regular user can edit a node anymore". however, you want to let them edit before it goes "live". makes sense for a small subset of drupal sites... i'd suggest one of 2 approaches if this (IMHO) rare use case is what you need:

1. when the admin publishes the node, they also change the node author so the original author's "edit own [foo]" doesn't give them an edit tab anymore.
2. use a fancy contrib like workflow or actions or whatever.

in IRC chx said "i don't even know how promoted and sticky should behave...". i can imagine scenarios where you want things to revert those values on edits, and scenarios where you don't. since it's not clear what you want, the default should be the simple thing: if you're not an admin, your edits don't change the existing values. if your site requires functionality where the act of editing needs to impact the workflow, use a contrib specifically for that. but don't make the default behavior complicated and counter-intuative to accomodate the edge cases, and force everyone in the common case to install the contribs and work-arounds.

This is indeed the most sensible behaviour. Other behaviour can be accomplished using workflows and such. Core should have the simplest solution.

Committed to HEAD.

UnConeD: whoops, you seem to have applied an earlier patch of mine. the commit message nor your follow-up suggest you did so on purpose, so i'm assuming you just didn't grab the right patch. here's a new patch that reverts the bad commit and applies the right patch.

Oops.