Publishing/Unpublishing a revision changes the author of that revision.

Sounds like we're running into the same problem as drupal.org: #1217286: Posting a comment changes too many issue node revision properties.

Subscribing

On many sites it is actually very important to avoid this.

Where would I change the code for node save?

The following change works in other modules:

global $user;
$saved_user = $user;
$user = user_load($node->uid);
node_save($node);
$user = $saved_user;

Here is my patch for 7.x-1.2 tested at one site with moderation by several people from draft to published. The node kept the original author.

Found out how to make Netbeans change line endings.

The patch preserves the user id of the node creator in the node so the creator can get the right access. The workbench moderation node history continues to record the id of the person making the state change so you can trace who did it. This fits our requirement with individual authors and one level of moderation.

If I'm understanding correctly, this patch would change the revision uid of subsequent drafts to match that of the author of the first revision, regardless of the actual creator of the draft. This doesn't seem like it really solves the problem, it just replaces it with a slightly different one.

"// @TODO: do we trust node_save() here?"

It appears not: #1308924: node_save assumes revision is owned by user 1

May be easier to go with below, like the patch in #1307256: Timestamp bug when saving a new draft

Untested:

  // Preserve the changed timestamp of the revision when updating live revision.
  if (!empty($node->workbench_moderation['updating_live_revision'])) {
    $node->uid = $node->workbench_moderation['my_revision']->uid;
  }

Ok, looks like we really need to impersonate. Patch in #1450672: Cron task does not impersonate to admin user on automatic content updates has impersonate and revert functions that can be re re-used here and creates a clean abstraction layer, until we have #287292: Add functionality to impersonate a user in core. I may role a patch for this soon.

Patch attached.

Patch in #17 works for me for single WBM-state changes. Won't work for VBO.

This is a "must fix" imho, because state-changes by another user let revisions disappear from WB views where filter "revision owner = current user" is applied.

Thanks :-)

Subscribe

Is there any progress in this issue? Is the supplied patch added to the dev version for instance?

This patch is not on the latest dev ...
And it doesn't work,
when user X edits a node , created by user Y, he becomes the owner of the node's revision, so changing the state to Draft , after saving the node, does not actually send the Draft back to its author.

this solution looks promising

http://drupal.stackexchange.com/questions/45114/assigning-revision-owner...

a possible work-arround is to alter the Views , for My Drafts , and not to use the relationship revision user:Current , but use the author of the node . And the filter criteria should also change to Workbench Moderation: State (= Draft).

I created a new View (see attachment), for this, and now a user sees the nodes he created , and were revised by an editor, so they were send back to him as drafts to correct/improve...

This View could act as a communication channel between nodes going from "Needs Review" state back to the author of the node, as a Draft , until the module is patched to overcome this issue.

The patch in #17 is not made to solve this workflow. It was made to assure that a forward revision does NOT change the author of the life content. This was destroyed without the patch.

What you guys are trying is different and I'm not sure how we can solve this as you expect that the author will fix a node content. This is not realistic and not a must or often not wished. E.g. User A created a node, user B reviews and publishes. Now user A leaves your site and user B or C rewrites the content from now. In this case user D will review the next revission.

Could we focus on this patch review here and get it committed, please?

OT: The solution you are looking for is not related to this issue here and may require a new issue, but from the theoretical side it looks not solvable how you may expect it. The view should list these revissions to all users ever authored a node... All other ideas sound very compelling and not solvable without a strict rules workflow, but I may missed something.

Our main thoughts were focused on the states Needs Review -> Draft , were, theoretically speaking, the editor/moderator wants to send the node back to it's creator, so the Draft should be visible again to its creator. This could be a workflow state/iposibility in an organization...

It is clear that I did not understand your point of view , and what exactly the patch is about

#17: workbench_moderation_1240850+Publishing+Unpublishing+a+revision+changes+the+author+of+that+revision.patch queued for re-testing.

Patch #17 does not solve the problem It preserves the original author instead of changing it to the publishing user.
But, see the following problem:

Moderation states are Draft, Review, Live.
User A creates a node and moves its state to Review. Node autor: A, revision author: A
User B changes the node a bit, hereby creating a new revision; Node author: A, revision author: B.
User C now publishes the node.

Original Drupal author states: Node author: A; Revision author: C (WRONG!!)
With patch #17: Node author: A; Revision author: A (WRONG!!)

Expected solution: Node author: A; Revision author: B;
C does not change the node, and should NOT be mentioned in any changed state, besides "changed moderation state from review to published - but did not change the node at all!

Is there any progress?
Currently it is completely impossible in Drupal to create e.g. a token with the autor that has edited the node last, when you have a moderation workflow. Every time a redakteur sets a moderation state to e.g. live (and publishes the node using a rule), he changes the author to himself. So ALL published revisions have a redakteur as "last author" instead of the real ones.

There are at least two different uses for author and it might be better to move one or more out of the node system to simplify the conflicting requirements for this issue.

Author is used, on many systems, to identify the author to the public. The name does not change across multiple edits and revisions.

Author is not displayed to the public on some systems and is used internally to identify the person making a change.

There is a conflict between the two. The simplest change is to add the original author as a field and make that field public as the author of the work. The Drupal user id in the node is then free for use to record the person making a change.

Drupal 8 does something different but still tries to handle all the different cases with one or two user ids. Moving the identification of the "author" to a separate field will simplify some of the cases mentioned here. The change also lets the author use a formal name different to the user name.

@droetker: this is not in the scope of this issue. we cannot fix these issue in D7. This patch is the best we can do for now and it fixes the most important stuff that is broken. Otherwise we need to patch core.

Can we commit this patch now, please?

This issue seems to be partly fixed in current 7.x-1.x-dev, the unpublish function now relies on "workbench_moderation_moderate()" instead of "node_save". This seems to solve change of owner.
For state changes the issue still exists. I will attach a new, simpeler patch.

Looks good. Couple of minor changes.

+++ b/workbench_moderation.module
@@ -1809,10 +1809,19 @@ function workbench_moderation_store($node) {
+  // See http://drupal.org/node/1240850

This line can be removed; it's not necessary. (It's missing punctuation anyway.)

+++ b/workbench_moderation.module
@@ -1809,10 +1809,19 @@ function workbench_moderation_store($node) {
+  ¶

Extra whitespace here.

Hi, missing your point on the whitespace? I added linebreaks that might be what you mean by that. I'm probably overlooking.

@keesje: Lines 1797 & 1824 both contain 2 spaces, and then a new-line character. They should only contain the new-line character with no spaces before it.

If you install Dreditor and cick on the Review button near the patch, you'll see what I mean.

Is this a duplicate of: https://www.drupal.org/node/1345616 ?

Thanks, for the hints!
Without the spaces then.

Marked #1345616: Author cannot edit node after it was rejected by moderator as a duplicate of this issue. Fixing status.

You should use my patch #17 as base. It is very clean. These small patch does not work properly.

re-rolled against latest dev

This is no proper patch.

@hass, please provide feedback. What goes wrong, error messages, steps to reproduce.

See my patch in #17, please. This one was correct. The others are not saving the session and so on. You cannot impersonate a user properly without this. My patch was an easy and reusable piece of code. No idea why others appeared on the case.

Hiding invalid patches

#17 needs review not the later patches.

Note I'm running this patch for about 2 years in production without any issues.

I've been running #17 in production for a few months and it has been fine.

I haven't reviewed the code in any detail though.

The same patch is also in use in linkchecker D6/D7 for 4 years or more. All are based on a previous D7 core patch that found it's way into D8 first. The impersonation part has been reviewed by several people.

Agreed. I just reviewed the code; it looks good. This is the same approach as in core, and was successfully tested in #48. I'll commit this soon.

Does what is stated in #30 make any sense then? IMHO that part of the patch in #17 becomes overkill because of that ( "workbench_moderation_moderate()" instead of "node_save")? At least that is what I could make up and that is the only reason for me to make a simpeler patch.
Just my 2 cts

@keesje: It's true that your patch in #31 is simpler, but as it stands now, it's not handling the session properly. As per Safely Impersonating Another User, what's there is unsafe. We could add the missing session handling to it, however.

On the other hand, I'm willing to add a little bit of complexity to emulate what's happening in core in D8, as that was a widely agreed upon approach. In a perfect world, we'd get that functionality backported to D7 core, and then use that. But it probably won't get in any time soon.

I could be convinced of either approach.

@hass: What do you think about @keesje's simpler approach (with the safer session handling added)? It's not using a stack, but maybe we don't need to bother with all of that here.

Anyone else have thoughts on this?

After reading some more on this topic (thanks for the link colan) agree that the safer session handling should be in place.

I wonder though if someone has the same problems as I encountered with the #17 patch approach with Views Bulk Operations? That did not work, thats why I took the #9 approach.

Can we get the VBO problem fixed here as well? If it's not directly related, let's handle it in another ticket, and commit #17. Some analysis would be helpful here.

I'm strongly against the simpler patch as it is not re-usable.

We should go with #17 as noted. It is quite simple to use and good documented. Impersonate user, switch back user, forget it. Maybe the impersonation code get's backported to D7 core, too. I hate re-inventing the wheel every week again.

No idea what this views issue has to do with the issue here.

Okay, if it works for you then it's not directly related.

@keesje: Please open a separate issue for that if there isn't one already.

I couldn't get #17 to apply, even with "git am --3way", so updating status. Looks like the tests failed so it's not just me.

sorry, dont meant to irritate people

Patch #17, rerolled against current dev

This reroll is missing some code from #17

I don't think so : the last section in #17 don't show in #61, because unpublish form submit now delegates to moderate function :

function workbench_moderation_node_unpublish_form_submit($form, &$form_state) {
  global $user;
  $node = $form['node']['#value'];

  // Moderate the revision. This will do the heavy lifting.
  workbench_moderation_moderate($node, $form_state['values']['state']);

So there is nothing to patch (directly) there...

This might be the underlying cause of #2382215: Transition from Needs Review to Draft does not go back to author from edit screen but I was able to fix that with a slight update to the "My Drafts" view.

If you update the view to include nodes where the current revision author = current user OR the original node author = current user, the view works as expected. That addresses the concern in #18.

I added an export of the updated view in the thread there: https://www.drupal.org/files/issues/workbench_moderation_view-2382215-9.txt

There's also a possible filter to display any content where the node contains a revision (at any point) that was authored by the current user, but I'm not sure if that is too open permission to be the default for the view.

Hello,

I have actual dev version (2015-Aug-27) and I have always this issue.

I have not seen this patch in the code of this version.

May be this patch has to change if dev version has been modified ?

Thank you

Hello,

I have actual dev version (2015-Aug-27) and I have always this issue.

I have not seen this patch in the code of this version.

May be this patch has to change if dev version has been modified ?

Thank you

@altbzh1

This issue is still in progress so a patch has not been added to the dev version yet.

Reroll

We have this bug reported by the client too. In our case I can't work around with the original author of the node as the client has figured out that you can remove the 'author' field, and has done so for all their content, but moderation still tracks who does what as there's still an author on the revisions.

User impersonation is always a big security risk, so avoiding that if at all possible would be great.

Without this patch applied, can someone confirm something for me? Because I think that we can solve this problem by updating the views referenced in another issue, rather than fundamentally changing how this module works.

For instance, we have a similar problem where a moderated piece of content does not appear for the user in their workbench view if it's been edited by another user. However, the original user maintains the ability to further edit the node if they are linked to it directly.

Can someone confirm without this patch:
1) That the username does change on the content
2) But that the original author can still edit the content as expected (give them the direct link)?

Impersonisation is not a security risk. The node will be safed again as the user who have authored the text. This does not provide more or less permissions to the safe process. There is no other solution for this case.

Why is this still not committed?

#68 is an incomplete re-role.

@hass: what's incomplete about it? it's a full reroll of #61.

Please compare the patch. 20-30% is missing

@hass: I think I see what you mean:
#68 is a full reroll of #61 but #61 is an incomplete reroll of #17, hence #68 is an incomplete reroll of #17.

Patches need to be re-rolled against 3.x as there has been a refactor on how this is handled. This issue is causing: https://www.drupal.org/node/2382215

Something like this?

This keeps the revision owner the same when changing moderation state but still tracks the current moderator in the moderation history.

hi @Eric_A
i just apply your patch which applied correctly..

The only thing i notice and that on the my edit block will the author will no more have node in list as soon as an editor edit it or create a revision

The patch apply correctly but it works only if the node is sent back to draft without open the draft, otherwise does not appear under My Draft for the author, so it does not fix the issue.