How to recreate:

1. As admin, navigate to: administer » content » settings, and uncheck 'publish' for stories.
2. As admin, create a story and assign it to a user that can create and maintain their own stories, say this user's name is Mark.
3. Login as Mark and edit that story.
4. Instead of staying published, it reverts back to it's unpublished state.

I think this behavior is a bug as these content settings are default settings and should only be applied during node creation (not editing). I know Drupal-contrib project owners get bitten with this everytime they edit their project pages.

I'll work on a patch within the next day.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

matt westgate’s picture

FileSize
1.05 KB

Proposed solution:

If a node is being updated: respect the following existing settings:

1. node status (published/unpublished)
2. node promotion (promoted on front page)
3. node static (static on front page)

For any node, force the defaults for:

1. moderation
2. revisions

With our current situation, say the admin makes 'promote to front page' and 'static on front page' the default settings for stories. Another user posts a story and bam! it's of course immediately visible. The admin decides he/she doesn't like that story and demotes it. Well as soon as that user edits his story, the default settings are re-assigned and his post is right back on the front page.

matt westgate’s picture

FileSize
1.19 KB

When editing nodes, the following node properties should not be overwritten

- node status (published/unpublished)
- node promotion (promoted on front page)
- node sticky (top of the list)

Currently, if the author of a node doesn't have 'administer node' privileges, than the above settings will always revert to the default behavior of their node type group. This makes nodes disappear and node authors very very confused.

ccourtne’s picture

Don't forget to take moderation into account. If a node is edited should it go back through moderation? Some sites may want the behavior of going back some sites may not. This is probably best as a toggle option on workflow default config screen.

matt westgate’s picture

I don't think my patch interferes with the moderation features. If the default workflow for a type of nodes is set to 'moderate', than the moderation flag will always reset itself to 'moderation' upon editing. I did not alter this functionality.

matt westgate’s picture

FileSize
1.64 KB

I was recently bitten by this bug again, so I'll try to describe the exact nature of the problem.

If an user of a node doesn't have 'administer nodes' permission, than whenever they edit their own node the status, promotion, static, moderation and revision settings revert to their default state. This can cause:

- A demoted post to once again be promoted.
- A promoted post to be demoted.
- A static post to disappear from the front page.

All of these scenarios have happened to me simply by authors editing their own content. This patch allows all options except moderation and revision to stay with a node once it's created. Note: along a very similar line, the one-line 'node_validate does not respect group editing patch' has been rolled into this as well. I'll take it out if need be.

matt westgate’s picture

I was recently bitten by this bug again, so I'll try to describe the exact nature of the problem.

If an user of a node doesn't have 'administer nodes' permission, than whenever they edit their own node the status, promotion, static, moderation and revision settings revert to their default state. This can cause:

- A demoted post to once again be promoted.
- A promoted post to be demoted.
- A static post to disappear from the front page.

All of these scenarios have happened to me simply by authors editing their own content. This patch allows all options except moderation and revision to stay with a node once it's created. Note: along a very similar line, the one-line 'node_validate does not respect group editing patch' has been rolled into this as well. I'll take it out if need be.

Dries’s picture

When someone edits a node that has been promoted, I do want to demote it from the main page. If not, the system is open for abuse. IMO, this is 'by design'.

Bèr Kessels’s picture

Both Dries and Mathias have valid points here.

If I add a post with loads of ugly stuff inside it, but an adminstrator decides not to delete it (because the post is still not too bad, or because of policy), but demote it, I can promote my spam next minute again by simply editing my content. This is bad.

If I add a post with lots of good information, so that an administratr promotes it, next thing i can do is open it up and add loads of ugly spam to it.

Buth are unwanted scenarios. The first is what we have now. The second is when this patch is committed.

I think we need to seriously rething the workflow logic, maybe splitting workflow up into "add" and "edit".
But maybe there are even better options?

Bèr Kessels’s picture

Both Dries and Mathias have valid points here.

If I add a post with loads of ugly stuff inside it, but an adminstrator decides not to delete it (because the post is still not too bad, or because of policy), but demote it, I can promote my spam next minute again by simply editing my content. This is bad.

If I add a post with lots of good information, so that an administrator promotes it, next thing i can do is open it up and add loads of ugly spam to it. and then i have a promoted post with spam on that site.

Both are unwanted scenarios. The first is what we have now. The second is when this patch is committed.

I think we need to seriously rething the workflow logic, maybe splitting workflow up into "add" and "edit".
But maybe there are even better options?

Bèr Kessels’s picture

Both Dries and Mathias have valid points here.

If I add a post with loads of ugly stuff inside it, but an adminstrator decides not to delete it (because the post is still not too bad, or because of policy), but demote it, I can promote my spam next minute again by simply editing my content. This is bad.

If I add a post with lots of good information, so that an administrator promotes it, next thing i can do is open it up and add loads of ugly spam to it. and then i have a promoted post with spam on that site.

Both are unwanted scenarios. The first is what we have now. The second is when this patch is committed.

I think we need to seriously rething the workflow logic, maybe splitting workflow up into "add" and "edit".
But maybe there are even better options?

Dries’s picture

The first problem can be solved by blocking the user account, or by revoking the user's permissions.

The second problem can probably be solved using the node-level permissions. Whenever I promote a post to the front page, I could revoke the user's edit rights. It would only work, if the modules don't by-pass the access rights, of course (I believe some do).

matt westgate’s picture

FileSize
4.22 KB

Dries is right about using the power of node-level permissions for community editing. I think this patch would assist that goal. As it stands, node-level permissions don't work with community editing, at least in the sense we're used to. For one node_validate transfers ownership of the node to the current editing user which may not always be the author. This causes many problems since original authorship usually implies special privileges (e.g. 'edit own foo', delete).

Second, sites have different trust levels for users. On some sites there's just a handful of trusted users that edit and maintain all the content. The current approach for community editing is to create a new role and give it the 'administer nodes' permission, which in my opinion makes the node form interface much more cluttered and gives users in that role full access to node properties. Full access is not always desired. Using a combination of node-level permissions and allowing a node to retain some or all of its options makes a clean, easy to use approach to maintainance.

This new patch lets the admin control how node_validate handles node options during editing. It splits the default workflow screen into two parts: Creation settings, and Editing options. The creation settings is the default workflow we're all used to. The editing options allow the admin to say what happens to those node options when they are edited.

I've created a node-level permissions module based on JonBob's work that allows a user to choose which roles can view and/or edit their post. Unfortunately it needs this patch to allow node_validate to be a little less controlling. I'd like to contribute this module if it didn't need a patch.

matt westgate’s picture

Screenshot of the new default 'node editing options' interface.

Bèr Kessels’s picture

Mathias,

First: using radio's for a Boolean is bad UI design IMO. It would be much better if you use checkboxes.
Second: If you use radio's you can make the same matrix as above, for each node-type a reset foo flag.

This will not only make the UI more consistent, but also improve usability by allowing per-node-type reset flag.

Ber

Dries’s picture

The settings GUI's help text is rather hard-core and will scare many user away. It will even frighten most Drupal developers. Also, I'm not sure I understood the screenshot correctly.

The functionality introduced is much needed but the GUI and configuration parts needs work IMO. Keep working on it.

matt westgate’s picture

FileSize
4.74 KB

As per Ber's feedback, I changed the interface to use checkboxes and allowed the reset flags to be set for each node-type. The result is a much more familiar and consistent experience.

Dries: I revamped the help text, removing the scary bits and sticking to the bare essentials.

The dialog concerning this patch has been excellent, and the result of which I feel is a sweet slice of code ready for core :)

matt westgate’s picture

Latest screenshot of patch.

matt westgate’s picture

FileSize
4.81 KB

Updated the help text to be slightly more descriptive, informing the reader that: Users with "administer nodes" permission can override the editing options.

Steven’s picture

I'm not sure I like that admin interface. My brain tries to correlate the two tables, but the meanings are different (what is the value, keep the value?). Maybe it would be better just to have one checkbox per node type to allow you to either revert the published/frontpage/moderate/sticky fields on editing, or not. This should cover 99% of all use cases with an admin interface that is much easier to understand.

matt westgate’s picture

FileSize
5.06 KB

It provides a consistant look and feel if the creation settings are the same as the editing settings since they both manipulate the same components of a node and you're just toggling states based on different actions.

I think it is a bad idea to only give site admins an 'all or none' approach to override node options upon editing. Here are some examples why:

- You want nodes to remember their 'sticky' setting, but unpublish themselves after editing. Once the node is approved, it goes right back to where it was.

- You want nodes to remember their 'promote to front page' setting, but unpublish themselves after editing. Once the node is approved, it goes right back to where it was.

- You want nodes to remember all their settings except moderation. The node is still published, but it goes into the queue to be rated again since the content has changed.

- By default nodes are unpublished. Once it's been tagged as published it stays that way. Perhaps the moderation tag is reset.

I've updated the code to hopefully make things even more clearer. Here's the latest screenshot.

moshe weitzman’s picture

this patch fixes a fairly undesirable problem with our workflow. lets consider it along with other proposed changes to default workflow.

killes@www.drop.org’s picture

Does not apply anymore.

rkerr’s picture

I'm still experiencing this problem with latest CVS (head and drupal-4-6) and in addition to the publishing options (Published, Promoted to front page, etc), it also affects the settings for comments.

When a user edits the page, the settings revert to the default value for that content type, instead of what is currently set for the specific node.

Example of this problem with respect to Comments:
( Create a user with permissions to edit own pages, but not administer comments. )

1. user creates page (comments default to read/write)
2. admin edits page and turns off comments
3. user edits page and saves
4. comments are reset to read/write

Help :)
(Am willing to assist in fixing this if necessary)

killes@www.drop.org’s picture

Ok, just some remarks from irc:

21:23 < killes> r11r: After looking a bit more at the code I think it is more of a feature request than a bug.
21:23 < killes> Not that I would mind some more flexibility, of course.
21:24 < r11r> killes: well, node settings magically resetting themselves could be considered a bug.. hehe
21:25 < killes> Well, it is not magically, it is "by design". ;)
21:25 < killes> i don't think we will get that into 4.6. But I woul dwork with you to get it into 4.7.
21:27 < killes> This needs to be part of a more general improved workflow thingie.
21:27 < killes> Did you try clousseau's workflow module? it is amazing.

rkerr’s picture

Haven't yet looked at workflow, but this issue can be fixed by simply unsetting the node properties for which the user doing the edit does not have permissions.

i.e. in node.module (around line 1255)
if (!$node->nid):
-- load defaults from database settings
else:
-- unset node->status, moderate, promote, sticky, revision, created

also in comment.module (around line 247, the nodeapi hooks)
if (!user_access('administer nodes')):
-- unset node->comment

Because any properties that are not set in the node object do not get passed into the update sql statement, the node will retain its previous values after being edited by a user instead of reverting to the default values for the content-type.

Can anyone think of problems with this approach for the time being?

rkerr’s picture

Title: Failure to respect node status upon editing (reverts to default) » Update for comment.module

Change in comment.module, line 257 for head and 4.6, line 254 for 4.5.

    case 'validate':
      if (!user_access('administer nodes')) {
        if (!$node->nid) {
          // Force default for normal users:
          $node->comment = variable_get("comment_$node->type", 2);
        }
        else {
          // If the node is being updated, respect its previous settings
          unset($node->comment);
        }
      }
      break;
rkerr’s picture

Title: Update for comment.module » Failure to respect node status upon editing (reverts to default)

Didn't really mean to change the title!

rkerr’s picture

Change in node.module, for head and 4.6, around line 1258

    $node_options = variable_get('node_options_'. $node->type, array('status', 'promote'));

    if (!$node->nid) {
      // Force defaults in case people modify the form:
      $node->status = in_array('status', $node_options);
      $node->moderate = in_array('moderate', $node_options);
      $node->promote = in_array('promote', $node_options);
      $node->sticky = in_array('sticky', $node_options);
    }
    else {
      // If the node is being updated, respect is previous settings
      unset($node->status);
      unset($node->moderate);
      unset($node->promote);
      unset($node->sticky);
    }

    $node->revision = in_array('revision', $node_options);
    unset($node->created);

For 4.5, the code is slightly different in node.module, around line 1095...

    if (!$node->nid) {
      // Force defaults in case people modify the form:
      $node->status = variable_get("node_status_$node->type", 1);
      $node->promote = variable_get("node_promote_$node->type", 1);
      $node->moderate = variable_get("node_moderate_$node->type", 0);
      $node->sticky = variable_get("node_sticky_$node->type", 0);
    }
    else {
      // If the node is being updated, respect is previous settings
      unset($node->status);
      unset($node->moderate);
      unset($node->promote);
      unset($node->sticky);
    }
rkerr’s picture

FileSize
1.38 KB

Patch for node.module in HEAD

rkerr’s picture

FileSize
76.29 KB

Patch for comment.module in HEAD

rkerr’s picture

FileSize
976 bytes

Err, sorry too many files to pick from in the directory . :)

Here's the patch again. (for comment.module, in HEAD)

rkerr’s picture

Patch for node.module in DRUPAL-4-6

rkerr’s picture

Patch for comment.module in DRUPAL-4-6

rkerr’s picture

Patch for node.module in DRUPAL-4-5

rkerr’s picture

FileSize
1010 bytes

Patch for comment.module in DRUPAL-4-5

rkerr’s picture

killes@www.drop.org’s picture

Some people think, that the node settings _should_ be reset after editing. There are good reasons for some of the settings to be reset, for exampel in case of the "promote" bit. I cannot see such reasons in case of the "comment" bit and would like to see that part of the patch included.

rkerr’s picture

FileSize
1.98 KB

Instead of having to patch, I've made a module that I think accomplishes the same thing as the pathes using nodeapi.

The "fixoptions" module.

I suppose settings could be added to this module to optionally reset all options or only the comments option as per killes' comment.

shouchen’s picture

In Drupal 4.6.3, the publishing options do not revert back to site defaults when editing a node. In the current HEAD, they do. This is particularly bad in HEAD because the publishing options are in a collapsed section. So, it is very easy to miss the fact that the settings have been changed back to defaults.

I think that when a node is edited, nothing should be changed unless the user changes it when editing the node. From my perspective, this is a regression between 4.6.3 and HEAD. I don't understand why this would be intentional.

-Steve

shouchen’s picture

In Drupal 4.6.3, the publishing options do not revert back to site defaults when editing a node. In the current HEAD, they do. Will this continue to be the behavior in 4.7?

rbrooks00’s picture

+1 for putting this patch into core - "by design" or not it is highly annoying if you are running a community site to have posts revert back to unpublished (assuming that is the default for good reason) when the author edits them. To avoid this there is no other way than to give the user "administer node" which for most sites is completely unacceptable.

I can see the argument for resetting attributes for certain sites but I think that it should be up to the site administrator as Matt has designed rather than enforced for all sites based on the assumptions for one type of site. The defaults for this module could be set to work the way Drupal does now if that isn't the way it works already.

shouchen’s picture

+1 for this patch in core from me, also

Thomas Sewell’s picture

I agree with shouchen on this one.

Editing a node should follow the Principle of least astonishment and not change options in a collapsed section when the user has not changed those options himself. This is especially true for an administrator who would have every expectation that unless they decide to de-publish something, it's not going to invisibly do it to itself when edited.

If options are going to revert to the default when the node is edited, at the very least good usability would require that those new option modifications be visible in the default edit screen view and highlighted somehow to call a user's attention to the new values. This should be core behavior.

shouchen’s picture

Would somebody who makes decisions about Drupal 4.7 please comment on this issue??

Bèr Kessels’s picture

I thnk killes is correct to state that some options shuold be kept while other s should revert.

But I think we need to find a very good workflow in this. For examlpe, what happens when an admin 'demotes' something that is, by default promoted? In that case it should not revert, for an admin did that change on purpose.

There are much more of such cases posible. None of these patches fix the behavoir in a solid way.
I would be for a:
* very transparant, and safe, yet extremely basic default, in drupal core. Taht would be nearly (if not completely) similar to what we have now.
* A module where one can change all the workflow, which exist as workflow.module.
* A good practice page written in the handbooks.

Oh, and I do no volunteer for this ;)

mikevk’s picture

My new site got hit by this issue last night. My pages are unpublished by default. Authors don't have node administration privileges, but they can edit their own published pages, some of which are linked from the navigation menus. As soon as an author tried editing, the page reverted to unpublished and visitors started getting "access denied" errors.

Fortunately it's fixable through hook_nodeapi as described earlier. But this behavior at the very least needs to be better documented. It's very easy to miss it if you're usually logged in with node administration privileges.

kbahey’s picture

Let me play the devil's advocate for a second ...

On a site where the nodes are only published after approval by a moderator, the behavior outlined in the previous comments can be desirable. Imagine this scenario ...

1. User signs up, and adds a node

2. Moderator/admin reviews it and finds it acceptable and publishes it

3. If user has edit own node permission, they can edit the node, and add inappropriate content

4. If the node does not revert to unpublished, the inappropriate content will be seen by all, which is not what we want.

This is a case where the current behavior is desirable.

moshe weitzman’s picture

kbahey - thats what dries posted earlier in thbis thread - but he did so in about 20% of the lines. lets try to avoid repeating ourselves here.

it is obvious that admins need to be able to pick their desired behavior upon node editing. we need a patch which gives them that flexibility. my .02

shouchen’s picture

I agree, moshe. This issue has two very valid sides... so the option should be left to the administrator. They need to make it an option, so people on both sides of the issue can be satisfied with the implementation.

Bèr Kessels’s picture

* the current behaviour is as it should be. (dries and kbahey proved it)
* we dont want options for options. Or at least I don't. Options to set the workflow options are going to be a usability nightmare.
* The current functionality could be enhanced with a drupal_set_message() and some documentation.
* For all your other needs use workflow.module. It is made for this.

I'm all +1 for closing this issue, unless someone makes a patch to make the behaviour more transparant with docs and a message here and there.

shouchen’s picture

OK, Bèr, how about this...

PROPOSAL
When editing a node, begin with the section containing these options EXPANDED, not COLLAPSED, to draw attention to them.
Indicate the CURRENT SETTINGS somehow (red * beside each one, for example) so the editor can see what the settings were before editing. The way it is now, the only way to SEE the settings is to go into Edit... but that reverts the settings to default, so there is no way to know the current state. Right?

If the above suggestions are implemented, an editor will know what (pending) changes have been made by Drupal, and can change some of them back to the original settings if desired (assuming they have rights to do so).

If at all possible, this needs to make it into 4.7, along with the documentation and messages you proposed. If I could code it, I would supply a patch. It sounds simple... but I haven't learned to modify Drupal modules yet.

What do you think of these ideas?

-Steve

chx’s picture

Title: Failure to respect node status upon editing (reverts to default) » node option status reverts to default upon edit -- messages and documentation needed
Category: bug » task
Status: Needs review » Needs work

shouchen, only admins see node options and for them it does not revert to default. I changed the title so that it indicates what's needed.

chx’s picture

Title: node option status reverts to default upon edit -- messages and documentation needed » node options reverts to default upon edit -- messages and documentation needed
Status: Needs work » Active

Oh, and current code does not address this at all.

shouchen’s picture

chx, on my HEAD site, I just edited a node (as admin) to make it 'sticky'. Then, I clicked edit again, and the check mark was removed from the 'sticky' option. When I submitted the change, the node was indeed no longer 'sticky'. Please test the latest CVS and let me know if you see the same behavior. Thank you!

shouchen’s picture

chx, have you tried to reproduce the problem (as admin) in HEAD?

thanks...

shouchen’s picture

Category: task » bug
Priority: Normal » Critical

According to chx, the node options should not revert to site defaults for admins. My experience is different from this. Could someone please verify this? I'm marking this as a critical bug since the code doesn't behave as described by chx, and this seems rather urgent to me. Hope you all agree...
-Steve

beginner’s picture

Just to confirm what shouchen says: editing a node as ADMIN will revert publication settings to default ones.

shouchen’s picture

bump

drumm’s picture

Assigned: matt westgate » drumm
Status: Active » Needs review
FileSize
3.88 KB

Here is a patch which makes things work as desired. If a user has the 'administer nodes' permission the checkboxes are shown with the currently set values filled in (that was the broken part). If a user does not have that permission the values are saved with the form but are reset in the execute section. I will write a second patch which uses these.

drumm’s picture

http://drupal.org/node/38451 builds on this patch by making the reseting behavior configurable.

Dries’s picture

Can we add some code comments describing the desired behavior (and explaining the code)?

drumm’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
7.41 KB

Added comments for this section of code.

drumm’s picture

FileSize
4.03 KB

Wrong patch.

asimmonds’s picture

Have tested patch #63, works for users with 'administer nodes' as Neil has described in #59.

Small spelling typo in the comments:
s/becsause/because/

drumm’s picture

FileSize
4.03 KB

Spelling fixed

crunchywelch’s picture

+1 to this.

This bug adds four or five clicks to a node edit, depending on what your defaults are, and if the current node has different settings than those defaults. At very least I find myself expanding both comment and publishing options every time to double check they are what I want them to be.

With admin privs, this is the current behaviour, and is infuriating. Saving the options and later having them revert and the changes be hidden is a UI disaster. A user has no idea why this occurs, only that posts that were promoted are now promoted, or not, depending on you know, what combination of invisible site defaults there are and the variation of options that have been changed from such defaults. Or not.

Behaviour may be programatically consistent, but gives the appearance of drupal not saving changes properly.

The only exception I can see is the example for moderation given above, and only then if the user does not have admin permissions.

What needs to be done before this is committed?

crunchywelch’s picture

And actually, on my head install, default options are not obeyed at all. All types have comments enabled, are published and promoted, regardless of the content type's settings.

Steven’s picture

Status: Reviewed & tested by the community » Fixed

Works as advertised. Committing to fix the serious bug in HEAD.

Continue at the follow-up patch:
http://drupal.org/node/38451

Anonymous’s picture

Status: Fixed » Closed (fixed)
calebgilbert’s picture

Version: » 4.7.3

Drupal 4.7:

With my workflow I didn't have an issue with things being unpublished when users update their stuff, my issue was with things getting unpromoted when users edited things.

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? :-)

webchick’s picture

subscribing