When a user submits the workflow tab form, the uid of the current node revision is set to that user's and the revision's timestamp is updated, even though the user didn't actually change the revision by the operation. The result of this is that historical data about the node is destroyed and changes may be wrongly re-attributed to uninvolved users. Take this scenario as an example:

  1. User jack creates a story node November 11, 2011 and then creates a new revision with changes. The revisions tab correctly records the history.

    workflow-jack.gif

  2. Months later user jill saves the workflow tab form (no changes to it necessary). The current revision, created last November, is updated to show her as the author today. The changes jack made months ago are now attributed to her today. The log no longer accurately reflects the actual history of changes.

    workflow-jill.gif

This happens because workflow_tab_form_submit() in workflow.pages.inc performs a node_save() "to make sure any handlers that use the new workflow values will see them." Unfortunately, this also updates the revision, changing its uid and timestamp. That's fine when changing workflow state from the node edit form, because then the revision is actually being changed and the user is presented with options for controlling how that's recorded. How can handlers be alerted to new workflow values from the tab form without forcing a node_save() and destroying historical data?

CommentFileSizeAuthor
#1 workflow-1471014-1.patch1.12 KBtraviscarden
workflow-jill.gif4.37 KBtraviscarden
workflow-jack.gif4.35 KBtraviscarden

Comments

traviscarden’s picture

traviscarden commented
Version: 6.x-2.x-dev » 6.x-1.x-dev
Status: Active » Needs review
StatusFileSize
new workflow-1471014-1.patch1.12 KB

This patch replaces node_save() with workflow_transition(). It gets the job done for me, because I'm not depending on the former behavior. Might other people be?

Bastlynn’s picture

Bastlynn commented
Status: Needs review » Closed (fixed)

Thanks :) This has been put to dev on the 6.x and 7.x branches after I took a long look at how node_save and the Rules triggers are handled in the module. All in all, we should be ok with the change. Thanks!

kswan’s picture

kswan commented

I was just bit by this issue. Another option that keeps the node_save() would be:

$node->revision = TRUE;
node_save($node);

I don't know if the node_save is required, but dropping it is a major change.