Saving nodes outside Workbench Moderation leads to incorrect state transitions (e.g., "needs review" appearing as published)

It's possible this is happening through a publishing mechanism outside the control of Workbench Moderation.

On a test site I created a node, it was in the "draft" state.

I then went to "admin/content" and used the "update options" to make this node published.

The core status on the node was flipped from 0 to 1 and the workbench moderation state remained as draft.

I'm not sure if that's what happened in your case or if there is a way for Workbench Moderation to prevent it.

idiotzoo, The db log might record the event that is doing the publishing.

I've run into several issues related to this and it seems like the cause is clear: Workbench Moderation takes over node saving on the node form (and some other places), but when nodes are saved anywhere outside of that (could be admin/content, programmatic saves, Views Bulk Operations, etc.) the default state transitions it's assuming don't really make sense.

The attached patch is at least a start at fixing this, although maybe not complete. If Workbench Moderation doesn't receive a "preference" from the person saving the node as to whether its state should switch, then in general it seems to me like it should not try to change the state, right? That's most of what this patch accomplishes.

Perhaps it would make sense to go further in some cases (e.g., comparing $node->original->status to $node->status to see if Drupal's published status changed, and setting the moderation state based on that) but it's a tricky problem; however, that might also do a better job fixing the specific case that started this issue.

I'm going to mark these two issues as duplicates because they sound like they're essentially about the same problem:
#1558482: Allow VBO to change Workbench Moderation states
#1568616: Setting nodes as published in admin overview does not set workbench history values

The above patch would make setting the node published state a bit of a moot point, though, as it wouldn't actually change any workbench states. In terms of usability I think this would actually be a bit confusing. Perhaps an alternative solution would be to form_alter some of the node forms, and replace the node-module's actions (like publish / unpublish) with workbench actions (change state) similar to how this is done in the node form.

Comparing node->original with node in terms of status was also my first thought on resolving the issue, but may not solve all use cases, like 'publish on schedule'.

This patch removes the "Publish" and "Unpublish" actions from the node management form at admin/content. It is not intended to be the solution, but a temporary workaround. And perhaps a starting point for a more complete solution?

Subscribing

@_KASH_ Please stop subscribing and start following. :)

I was having the problem where workbench moderation was screwing up the state when programatically saving a node.

I found this flag and it fixed my problem, my code:

   ...
    // make workbench moderation not screw up the publication state
    $node->workbench_moderation['updating_live_revision'] = 1;
    node_save($node)
   ... 

This actually happens when you use the workbench moderation "Set moderation state" action and you go from Published to Draft, the node remains published but the state changes to Draft. Seems a little backwards to me...Maybe instead of having the custom code in the unpublish form submit handler for unpublishing a node, it should be in workbench_moderation_moderate?

This is a pretty bad. It means you can't seriously do anything to a node and have consistent data.

I think this query will allow you to assess the damage to nodes on your site. The moderation history for these nodes doesn't even show any published revisions yet the node revision is still published.

select * from workbench_moderation_node_history n where n.current = 1 having (select count(*) from workbench_moderation_node_history where nid = n.nid and published = 1) < 1;

For fixing current nodes, probably need to load up all node revisinos found here, and make a state transition to the published state. Does that sound about right? Given that the actual bug is fixed.

@acbramley - I see more than one problem in the conversation here. In order to clarify for my own mind, and possibly for the rest of the interested folks on this issue, here's what I see:

The original issue as identified by @idiotzoo is that scheduled publishing doesn't take workbench moderation into account - it publishes by updating the status field to 1 the node table and doesn't consider workbench moderation state at all. This results in a published node, while it's still in a draft moderation state.

#12 is a problem I've got as well - using the 'Set moderation state' action on a published node doesn't work as I'd expect, and sets to Draft instead.

It seems to me that workbench needs a configurable way for the site admin to pick what should be done with published content when someone attempts to change its state...That may be more closely related to #1147646: Hard coded 'Draft' status in .module

Ran into this (also processing nodes via node_save() in cron, hence the update by "Anonymous" in idiotzoo's initial report). For our setup, the workaround by Jared in #11 (set $node->workbench_moderation['updating_live_revision'] = 1; before node_save()) prevented Workbench Moderation pushing new 'review' revisions when nodes were modified by cron task. Hope this helps someone.

@dsayswhat I think the main issue is that calling node_save outside of workbench without the $node->workbench_moderation['updating_live_revision'] = 1; flag set prior to saving messes up live revisions. This is possibly what happens with the VBO action as well so maybe it's just getting other contrib to adhere to this, but that sounds like a lot of modules and a lot of work. So should it not be workbench_moderation that do these checks?

I got around the VBO issue by implementing custom actions for promoting to homepage and adding in the updating_live_revision line like so:

/**
 * Implements hook_action_info().
 */
function mymodule_action_info() {
  return array(
    'mymodule_promote_action' => array(
      'type' => 'mymodule',
      'label' => t('Promote content to front page'),
      'configurable' => FALSE,
      'behavior' => array('changes_property'),
      'triggers' => array('mymodule_presave', 'comment_insert', 'comment_update', 'comment_delete'),
    ),
    'mymodule_unpromote_action' => array(
      'type' => 'mymodule',
      'label' => t('Remove content from front page'),
      'configurable' => FALSE,
      'behavior' => array('changes_property'),
      'triggers' => array('mymodule_presave', 'comment_insert', 'comment_update', 'comment_delete'),
    ),
  );
}

/**
 * Sets the promote property of a node to 1.
 *
 * @param $node
 *   A node object.
 * @param $context
 *   (optional) Array of additional information about what triggered the action.
 *   Not used for this action.
 *
 * @ingroup actions
 */
function mymodule_promote_action($node, $context = array()) {
  $node->promote = NODE_PROMOTED;
  $node->workbench_moderation['updating_live_revision'] = 1;
  watchdog('action', 'Promoted @type %title to front page.', array('@type' => node_type_get_name($node), '%title' => $node->title));
}

/**
 * Sets the promote property of a node to 0.
 *
 * @param $node
 *   A node object.
 * @param $context
 *   (optional) Array of additional information about what triggered the action.
 *   Not used for this action.
 *
 * @ingroup actions
 */
function mymodule_unpromote_action($node, $context = array()) {
  $node->promote = NODE_NOT_PROMOTED;
  $node->workbench_moderation['updating_live_revision'] = 1;
  watchdog('action', 'Removed @type %title from front page.', array('@type' => node_type_get_name($node), '%title' => $node->title));
}

I think that this is the same issue I'm running into today. If I simply load and save a node that is published (with no newer draft), the state on the node is set back to Draft, though the node remains published:

$node = node_load(85);
node_save($node);

If you call node_save() twice, it will actually unpublish the node:

$node = node_load(85);
node_save($node);
node_save($node);

I tried what was suggested in some comments above, and set updating_live_revision:

$node = node_load(85);
$node->workbench_moderation['updating_live_revision'] = 1; 
node_save($node);

That does keep the state from being changed on me, but as others have noted, it's not really feasible to expect any node_save() call in contrib modules to be preceded by this set.

What does seem to be working, in some basic testing here anyway, is to check if $node->workbench_moderation_state_new is set before running most of the functionality in workbench_moderation_node_update(). There's already a condition to check if the node type is moderated, and to check the updating_live_revision flag mentioned above. When saving a node either from the node form, or by doing so programmatically and setting $node->workbench_moderation_state_new, this new condition will allow the rest of the function to run:

  // Don't proceed if moderation is not enabled on this content type, or if
  // we're replacing an already-published revision.
  if (!workbench_moderation_node_type_moderated($node->type) ||
      !empty($node->workbench_moderation['updating_live_revision']) ||
      !isset($node->workbench_moderation_state_new)) {
    return;
  }

Like I said, this seems to be working OK on my local test site, but I feel like I might be missing something.

Note that I had also tried changing workbench_moderation_node_data() to set $node->workbench_moderation_state_current and $node->workbench_moderation_state_new, so that they would not be set back to defaults in workbench_moderation_node_update(), but this didn't work right. I only mention it here in case anyone else is flailing down that path.

I agree with Dave Rothstein here:

If Workbench Moderation doesn't receive a "preference" from the person saving the node as to whether its state should switch, then in general it seems to me like it should not try to change the state, right?

And to acbramely's question:

This is possibly what happens with the VBO action as well so maybe it's just getting other contrib to adhere to this, but that sounds like a lot of modules and a lot of work. So should it not be workbench_moderation that do these checks?

IMO, It is not the rest of contrib's responsibility to make sure this Workbench Moderation behaves itself. :) Workbench moderation should do nothing to a node unless specifically asked to.

BrockBoland's fix seems to make sense to me. If the node comes into hook_node_update without $node->workbench_moderation_state_new set, then there is nothing to do. I don't know enough about Workbench Moderation to know if there are other implications of this.

Here is a patch based on BrockBoland's suggestion.

We also have many of the issues listed above. However, it was other use cases that brought us to this issue. Indirect updates to a node via webform, changes to node terms via term merge (taxonomy manager), updates to fields via rules all led to this problem.

The patch from #20 fixes our issues. We are using version 7.x-1.3.

Thanks Brockboland for finding the solution, and thanks q0rban for the patch!

Awesome, patch #20 worked for me with the VBO actions, node was correctly promoted to the homepage with no moderation changes.

The patch in #20 seems to work for me as well, effectively decoupling the "publishing" of a node within workbench moderation (i.e. setting moderation state to "published") from publishing in any other way (i.e. setting node state to "published").

BTW: I don't think 1558482 is a duplicate, since a bulk operation "Publish using Workbench Moderation" can still be useful.

Patch #20 works for me in case unpublish node through Rules Schedule

I had a problem with workbench setting content back to draft after it's edited in a cron process.

You can easily reproduce this:
1) Create a node (any nodetype with moderation enabled), set state to "published"
2) Execute this command:
drush php-eval "\$node = node_load(3306); node_save(\$node);"
(replace node id with node just created)
3) Check node/%/moderation

Without the patch the state is changed to draft in step 2. No changes are made when the patch from #20 is applied.

I'm the 5th person saying that the patch works fine. Setting to rtbc.

With this patch, VBO's publish and unpublish actions aren't working correctly at all. Executing unpublish on a published node does nothing, and executing publish on an unpublished node publishes it but does not set it's state.

Note: This may be due to entity api changes.

The attached patch allows a node to be published and unpublished (from anywhere, including programatically) and have the change stored in workbench's history. Other changes will maintain the node's current workbench state.

+++ b/workbench_moderation.module
@@ -675,13 +675,21 @@ function workbench_moderation_node_update($node) {
+    if ($node->status == 0 && $node->original->status == 1) {
+      // moving from published to unpublished
+      $node->workbench_moderation_state_new = variable_get('workbench_moderation_default_state_' . $node->type, workbench_moderation_state_none());
+    } elseif ($node->status == 1 && $node->original->status == 0) {
+      // moving from unpublished to published
+      $node->workbench_moderation_state_new = workbench_moderation_state_published();
+    } else {
+      $node->workbench_moderation_state_new = $node->original->workbench_moderation['published']->state ? $node->original->workbench_moderation['published']->state : variable_get('workbench_moderation_default_state_' . $node->type, workbench_moderation_state_none());
+    }
+  }

The elseif and else blocks should begin on a new line per Drupal standard.

I think this approach makes sense.

Hurray! After applying #27 my nodes are correctly published/unpublished using VBO Publish and Unpublish actions and the states are maintained. I will now test scheduler functionality

Rerolled patch with formatting fixes

acbramley, looks like from Allow VBO to change Workbench Moderation states, #27 wasn't the answer, because it "messes up saving drafts completely"? Or was that unrelated?

@mausolos, sorry that comment was in reference to my comment above (https://drupal.org/node/1558482#comment-7843441) where I tried to implement a rules configuration to fix the broken states.

Eh, I never changed the status...

Found I was getting some undefined keys for 'published'.

Re-roll of the patch in #30 with changes:

• extra isset()
• Using Drupal core constants for the status checks
• Misc comment changes for readability

#34 seems to be working for me after a small tweak. The following checks always return false because there is a difference in type:

$node->original->status === NODE_PUBLISHED
$node->original->status === NODE_NOT_PUBLISHED

$node->original->status is getting saved as a string while NODE_PUBLISHED and NODE_NOT_PUBLISHED return integers so the '===' operator causes a FALSE comparison which skips over these two checks and always goes to the else.

Changing operator to '==' fixes this for me.

Another tweak I made that is a bit more specific to my use case, but might also be relevant to some, is that I set the default state for all of my content types to "Published" and use the patch in #1408858: Default Moderation state of Published creates Drafts instead to make it work correctly. With the patch in #34, the state gets set to the default state of the content type which, in my case, is "published". This results in the state getting set to "Published" when I unpublish the node. The fix that worked for me here was to adjust the line (which occurs twice in this patch) :

$node->workbench_moderation_state_new = variable_get('workbench_moderation_default_state_' . $node->type, workbench_moderation_state_none());

to instead take "Draft" as the default unpublished state by making it:

$node->workbench_moderation_state_new = workbench_moderation_state_none();

Re-roll of patch #34 based on the first note in comment #35.

$node->original does not exist for new new nodes -- node_save() sets this property conditionally:

    // Load the stored entity, if any.
    if (!empty($node->nid) && !isset($node->original)) {
      $node->original = entity_load_unchanged('node', $node->nid);
    }

Updated #36 to account for absent $node->original

If there is no original node, then we will end up with:

$node->workbench_moderation_state_current = workbench_moderation_state_none();

$node->workbench_moderation_state_new = variable_get('workbench_moderation_default_state_' . $node->type, workbench_moderation_state_none());

It might be preferable to split the "no original node object" case out into a separate block of code, but I went with the most straightforward modifications to the existing patch.

I was able to unpublish a node on the moderation page after applying this patch. Hurray!

Patch in #38 works well when non-workbench-moderation means are used to publish/unpublish a node.

Not sure what happened but at some point clicking "unpublish" didn't work anymore. The only thing I can think of is that we updated to Drupal 7.24 right after I tested this patch and it was working. Maybe the core update broke something. Otherwise, we haven't updated any other contrib modules.

Another method that works is as follows. This is some place where you are programmatically saving the node:

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

And this is a drupal hook:

function my_module_node_presave($node) {
  if (isset($node->my_flag) && $node->my_flag) {
    // This ensures that our revision retains the current state
    $node->workbench_moderation_state_new = $node->workbench_moderation['current']->state;
    $node->workbench_moderation_state_current = $node->workbench_moderation['current']->state;
    $node->workbench_moderation['my_revision'] = $node->workbench_moderation['current'];
  }
  
}

Re: #41, clicking the "Unpublish this revision" link in the workbench moderation block still works for me, as does clicking on "unpublish" on the Moderation tab. I'm wondering if that might be another issue?

I've done a bunch of click testing with creating nodes and modifying them through the workbench and the admin/content page. So far I have not encountered any breakage. Concur with RTBC.

Patch #38 needs to be committed, it solves a lot of issues with other modules (another use case is the drag-n-drop functionality in the Full Calendar module, without this patch it changes the state to Draft after changing an event's date through dragging the event on the calendar). Also publishing/unpublishing worked fine, tested through the moderate tab and core's admin content page.

I had a problem where a rule was updating a field in a published node and it was sending it to draft, just like the picture in #1. I applied this patch and that problem went away. I agree, this fixed a major problem for me. Thanks so much!

This has been open for awhile and has been community tested and reviewed. Is it time to discuss working it into an official release?

Reviewed, fixed issue, thanks.

Re-reviewed, custom states are being turned back to draft

Don't see why it's using published instead of current here

      if (!empty($node->original->workbench_moderation['published']->state)) {
        $node->workbench_moderation_state_new = $node->original->workbench_moderation['published']->state;
      }

So changing that to current.

State remains same but actually node is getting published.

Steps to reproduce:

1. Create a node and publish it with views action
2. Go to new node page again, create a new draft of same node
3. Do VBO and change any entity(field) value in it
4. State remains same but when you see moderate page of node it is getting published

I applied patch attached in comment #49

@tajinder.minhas: I am unable to reproduce this issue.

Re-rolling patch with additional comparison operator changes (type juggling).

I was just looking at workbench_moderation_node_unpublish_form_submit and it appears there is some additional work being done on Unpublish that sync's the live and current revision. This is missed when Unpublishing outside of Workbench Moderation.

@pattersonc, are you going to address your remark? I am also looking into this issue at the moment. Would be a shame if we were doing overlapping work. I will start with writing a test for this issue.

@pfrenssen: It appears workbench_moderation_moderate handles this when going Unpublished -> Published. However, when going in the opposite direction, the code to sync live with current is tucked away in the Unpublish form submit. I don't like the idea of duplicating the code in hook_node_update but it appears to fix the issue.

Then we should maybe also integrate it in workbench_moderation_moderate(), then that function also works as expected.

Edit: This is out of scope for this issue I guess.

@pattersonc, it's probably not a good idea to call node_save() inside hook_node_update(), as this is invoked in node_save() itself. Just a bit higher up workbench_moderation_moderate() is called which registers its own node_save() in a shutdown function. It might be better to let workbench_moderation_moderate() handle this.

Looking at workbench_moderation_node_unpublish_form_submit(), it seems that the logic to unpublish the node that's in there should be moved to workbench_moderation_moderate(), as it is also called in that function.

Rerolling patch with unpublishing logic moved to shutdown function. This will avoid calling node_save() from within hook_node_update(). Also, it provides a DRYer implementation for unpublishing.

Is there some reason why tests are not running on this patch (relatively new to drupal.org).

Thanks! I will review the updated patch today.

The tests are triggered if the issue status is set to "Needs review".

Going to work on this now. Will review the work so far, see to the failing tests and start writing a test for this issue.

1. +++ b/workbench_moderation.module
   @@ -675,13 +675,28 @@ function workbench_moderation_node_update($node) {
   +    // Moving from published to unpublished.
   +    if ($node->status == NODE_NOT_PUBLISHED && isset($node->original->status) && $node->original->status == NODE_PUBLISHED) {
   +      $node->workbench_moderation_state_new = variable_get('workbench_moderation_default_state_' . $node->type, workbench_moderation_state_none());
   +    }
   

   This sets the node to the default status when it is unpublished, but what happens if the default status is "published"?
   I looked into this briefly but Workbench Moderation currently does not support a default "unpublished state". Adding this would be out of scope for this issue, so I guess this is OK for now.

2. +++ b/workbench_moderation.module
   @@ -1580,6 +1597,44 @@ function workbench_moderation_store($node) {
   +  // Make sure the 'current' revision is the 'live' revision -- ie, the revision
   +  // in {node}.
   +  $live_revision = workbench_moderation_node_current_load($node);
   +  $live_revision->status = 0;
   +  $live_revision->revision = 0;
   +  $live_revision->workbench_moderation['updating_live_revision'] = TRUE;
   +  node_save($live_revision);
   

   This will do a node_save(), while the workbench_moderation_store() shutdown function which runs just after this also does one. It would be worthwhile to see if we can solve this with a single save.

3. +++ b/workbench_moderation.node.inc
   @@ -284,24 +284,9 @@ function workbench_moderation_node_unpublish_form_submit($form, &$form_state) {
   +  // Moderate the revision. This will do the heavy lifting
      workbench_moderation_moderate($node, $form_state['values']['state']);
   

   Missing period at the end of the sentence.

1.

+++ b/workbench_moderation.module
@@ -675,13 +675,28 @@ function workbench_moderation_node_update($node) {
+    // Moving from published to unpublished.
+    if ($node->status == NODE_NOT_PUBLISHED && isset($node->original->status) && $node->original->status == NODE_PUBLISHED) {
+      $node->workbench_moderation_state_new = variable_get('workbench_moderation_default_state_' . $node->type, workbench_moderation_state_none());
+    }

This appears to be a relic of #35. Maybe we should remove and address in #1408858: Default Moderation state of Published creates Drafts instead.

2.

+++ b/workbench_moderation.module
@@ -1580,6 +1597,44 @@ function workbench_moderation_store($node) {
+  // Make sure the 'current' revision is the 'live' revision -- ie, the revision
+  // in {node}.
+  $live_revision = workbench_moderation_node_current_load($node);
+  $live_revision->status = 0;
+  $live_revision->revision = 0;
+  $live_revision->workbench_moderation['updating_live_revision'] = TRUE;
+  node_save($live_revision);

This doesn't look right. No modifications to workbench_moderation_store() should be needed. I'll double check patch.

Edit: This is correct (part of patch is not reflected above). I added a new shutdown function to handle unpublishing. Very similar to existing workbench_moderation_store() function.

3. Right

@pattersonc, don't start working on it yet. I have already addressed all the remarks, am now working on a test. I will post my progress and unassign myself in a few hours.

This appears to be a relic of #35. Maybe we should remove and address in #1408858: Default Moderation state of Published creates Drafts instead.

Thanks for pointing out that issue, I've added a link to it in the documentation of the affected code.

This doesn't look right. No modifications to workbench_moderation_store() should be needed. I'll double check patch.

Edit: This is correct (part of patch is not reflected above). I added a new shutdown function to handle unpublishing. Very similar to existing workbench_moderation_store() function.

Dreditor seems to have messed up the reference to the function that hunk originated from, it was indeed from the new shutdown function. Both shutdown functions were largely identical, so I merged them into one in the current patch.

Attached patch addressed my review remarks + added a test.

Changes look good and working as expected.

Found a bug during QA. If the $node->workbench_moderation['current'] array is removed in a hook_workbench_moderation_transition() hook after it has been passed to the shutdown function this strict notice is thrown:

Creating default object from empty value on workbench_moderation.module:1607

The solution is to clone the node before passing it on.

I had a look and generally it looks OK except the following:

+++ b/tests/external_node_update.test
@@ -0,0 +1,197 @@
+  /**
+   * Checks that the tested node doesn't change after it is resaved.
+   *
+   * @return
+   *   TRUE if the assertion succeeded, FALSE otherwise.
+   */
+  public function assertNodeUnchangedAfterResave() {

This method a bit puzzling - resave and assert in the same method? It should not do 2 things - split it or do the assertion directly in the test.

You're right, that method is doing too much. I simplified it a bit.

Thanks for the edit pfrenssen - it's a bit more code but looks much clearer.

Fixed some coding standards violations found by PHP Codesniffer. Leaving this to RTBC since these are not changing the patch itself.

Here is a query which will give you a list of nodes where their status does not match Workbench Moderation's current published state.

SELECT 
node.nid,
node.title,
node.status as node_status,
workbench_moderation_node_history.published as wb_published,
workbench_moderation_node_history.state as wb_state
FROM 
workbench_moderation_node_history 
INNER JOIN node ON node.vid = workbench_moderation_node_history.vid 
INNER JOIN node_revision ON node_revision.vid = workbench_moderation_node_history.vid 
WHERE
workbench_moderation_node_history.published <> node_revision.status
AND
workbench_moderation_node_history.current = 1

I've found this can also happen in this scenario here #2314907: Node revision status can easily become out of sync with moderation history status.

You can use Rules with VBO in order to circumvent the issue.

1. 1. In Rules - Create a new rule component for each workflow state (Draft, Needs Review, etc...). Use "force transition" to ensure the change in workflow state.
2. 2. In Views - Assign VBO to an action.
3. 3. In Views - VBO - Select the rules component you created.
4. 4. Save and Test the view with the VBO.

Respectfully,

Newtonsgroove

Can this patch be committed? This is a fairly large usability issue.

Please can we commit this? it has caused a security implication on one of my sites.

Raising priority. Depending on what a site is doing outside of Workbench Moderation this can have quite far reaching implications. I wouldn't be comfortable using Workbench Moderation on a site without this patch. This is at least major.

@fubarhouse, what was the security problem you experienced? Information disclosure?

Yeah... the result was the accidental early release of sensitive information...

I'm rolling this patch out over the latest stable build of the module on a few of the sites I maintain, I just want to pass on my thanks - it works a charm.

+1 for a release.

This is working well for me, thanks very much.

This one was a bit tricky, but I think I got it. Had to do a bit of a three-way re-roll based on the latest patch here and #2389507: Reverting a revision publishes it (workbench_moderation).

Committed to dev. Please test that branch, and if there are any problems with this functionality, report them here. I'm trying to get the branch up-to-date quickly to prevent more uncommitted patches that conflict with each other, but also to give us something reasonably stable in dev. If there are no issues after a while, will cut a release.

Forgot to set this up for forward porting.

Wooohoo!!! Thanks!!

Does not apply to the 2.x branch, it uses State Machine / State Flow for those operations.
I don't know of such an issue in State Machine, however it could be. But marking as fixed here.

Had another comment here about v 1.4, but I see now it looks like this patch has already been committed to it (can't delete my comment, so just updating it)

I must apologize!
The problem happened during a workbench hook but was not caused by workbench, it was another module that was misbehaving and changing the status of the nodes.

therefore....DISREGARD THE COMMENT BELOW

-------------------------------------------------------------
Unfortunately this is not fixed for me: I still have exactly the same issue.
I end up with published nodes that I cannot unpublish. The scary thing is that nothing seems to work to put the nodes back to a stable state, not even direct SQL manipulation! I tried to manually set "status" in node table and "state" + "published" in workbench_moderation_node_history table to the same value (first both unpublished, then both published) but no luck: the node is fine as long as I don't touch it, but as soon as I end up with a published revision that I want do unpublish, I'm stuck in the loop.

I'm on 7.x-1.4+12-dev (2016-05-20).
I attach a screenshot. As you can see from the message, this is the result of clicking on "unpublish". Drupal says the node has been unpublished, the current revision is marked as "ready" (my label for Needs Review ) but the node is published.

Looking at the db, I can see that workbench moderation as the right data, as expected, but the node table has the node marked as published. So it's still a problem of keeping the two things in sync. HOWEVER, I did nothing "outside" workbench moderation this time. Simply publishing a revision is enough for not being able, later on, to unpublish the live revision. So it seems that the bug is in the "normal" user journey of editing a node: something, somewhere, does not act on the node table to keep the status in sync with workbench.

This will be fixed more completely by #2376839: Rewrite to use the new Drafty module.