Rules condition - current moderation state - is not Evaluating correctly

I experienced the same problem and basically had to switch my rules backwards to get them to work.

It appears that the condition "Content's current moderation state" is reporting the last moderation state.

For example, say I have a notification email that gets sent to a different role at each state change. In 7.x-1.4, this worked similar to this:

1. A new revision of a published node is created and saved as Draft; an email is sent that the node has a new draft for editing
2. The Draft is sent to Needs Review; an email is sent that the node needs review
3. The node is published; an email is sent that the latest version of the node is published

In 7.x-3.0, this is what is happening now:

1. A new revision of a published node is created and saved as Draft; an email is sent that the node is published
2. The Draft is sent to Needs Review; an email is sent that the node has a new draft for editing
3. The node is published; an email is sent that the node needs review

In reality there are many more states and if, for example, a node went from state 2 to state 7 to state 8, the email associated with moving from 2 to 7 will be sent when moving from 7 to 8. In summary, it reports last state, not (current state - 1).

I am also now experiencing this issue.

When the event "after moderation transition" fires the moderation state that it thinks the node is now in is actually the state that it was previously in.

So if I save a node and update it from Draft -> Published it thinks I just set it to Draft.
If I then set it from Published->Draft it thinks it is in Published.

As noted above, this is highly problematic if there are more than 2 states, which in my use case there are.

EDIT - If I set up rules like this:

Event: create new content, update existing content
Condition: workflow state = foo

Then those rules are also not triggered.

Thanks Anskelt, that patch works for me.

Yep this is where we should set the current state, so the node object will carry the real current state. Working for me now.

#5 was a life saving quick fix. Big Thanks Anskelt.

#5 worked for me as well. Thank you.

#5 worked for me too, thanks!

#5 works for me

Thanks Anskelt, Given patch worked for me as well

Is this related to #1568272: Rules conditions for moderation states should compare my_revision? The patch in #5 works for me too.

I'm testing patch #5 and added a message to output the moderation state tokens in the Rules action. Going from "Draft" to "Needs review" states, before applying the patch the tokens are:

[previous-state:value]: draft
[new-state:value]: needs_review

After the patch:

[previous-state:value]: needs_review
[new-state:value]: needs_review

Oddly the tokens don't seem to agree with the Rules conditions. As others have pointed out above the "Content's current moderation state" condition shows the previous state (in 3.x) even though the [new-state:value] token shows the actual current state. It looks like the patch then messes up the [previous-state:value] -- likely inherits the value from this patch and shows the new state rather than the previous.

#5 did the trick for us as well.

Yep, patch in #5 seems to work well.

Hi,

I was having the same problem with rules not evaluating the current moderation state properly and the patch in #5 solved the problem. However I noticed after applying this patch, workbench emails (provided by workbench_email module) are no longer being sent. I haven't dug into exactly why but wanted to note it here in case anyone else runs into the same issue with this patch.

TL;DR the line $node->workbench_moderation['my_revision'] = $new_revision; in patch #5 seems to break workbench email without doing anything to fix the rules integration and can be removed based on initial debugging.

-----------------

Even though patch in #5 fixes the rules integration for me I'm also seeing the workbench email issue (#24).

The email issues seems to be caused by the old and new states being the same (#20). Since the email reacts on transitions from different states, it never fires. Taking a look at the patched code:

// workbench_moderation.module line ~828

  // set current revision
  $node->workbench_moderation['my_revision'] = $new_revision;
  if ($new_revision->is_current) {
    $node->workbench_moderation['current'] = $new_revision;
  }
  // On a moderation loop, inform other modules of the change.
  if (!empty($node->is_current)) {
    // Clear the node's cache.
    entity_get_controller('node')->resetCache(array($node->nid));

    // Notify other modules that the state was changed.
    module_invoke_all('workbench_moderation_transition', $node, $node->workbench_moderation['my_revision']->state, $node->workbench_moderation_state_new);
  }
}

$node->workbench_moderation['my_revision'] and $node->workbench_moderation_state_new are effectively set to the same value (the new revision). The problem is that those two variables are passed into the moderation transition hook as the old and new state respectively. This is incorrect according to the hook defintion (http://cgit.drupalcode.org/workbench_moderation/tree/workbench_moderatio...) and also the workbench email code:

// workbench_email.module line ~116
// http://cgit.drupalcode.org/workbench_email/tree/workbench_email.module#n116

/**
 * Implements hook_workbench_moderation_transition().
 */
function workbench_email_workbench_moderation_transition($node, $previous_state, $new_state) {

On the other hand, the patch fixes the Rules current moderation state because Rules only cares about $node->workbench_moderation['current']:

// workbench_moderation.rules.inc line ~249
// http://cgit.drupalcode.org/workbench_moderation/tree/workbench_moderation.rules.inc?h=7.x-3.x#n249

function workbench_moderation_rules_condition_contents_current_state($node, $moderation_state) {
  if (!is_object($node)) {
    return FALSE;
  }

  $state = (!empty($node->workbench_moderation)) ? $node->workbench_moderation['current']->state : $node->workbench_moderation_state_current;

By removing the line $node->workbench_moderation['my_revision'] = $new_revision;, Rules integration seems to be fixed without breaking workbench email. New patch and diff attached.

The analysis from matt.mendonca makes sense to me.
Here's an updated patch with minors coding standards fixes and whitespace fix.

Switching to needs review: let's see the simpletest results.

Just tested the patch from #26 on Drupal core 7.56 with Workbench Moderation 7.x-3.0. Patch applies cleanly and appears to fix the problem.

#26 tested also with Drupal 7.56 and Workbench Moderation 7.x-3.0 and works ok as well.

The patch in #26 worked for me! Finally, rules that make sense again!

Drupal: 7.56
Workbench Moderation: 7.x-3.0
Rules: 7.x-2.10

In case anyone is interested, this patch also fixes https://www.drupal.org/project/workbench_notifier which (without this) only notifies users or the second-most recent transition.

hi @dshields
Many thanks for this infos

Patch #26 also worked for me. THANK YOU!

I have been using this patch in production for many months. Would be great to have this committed.

Instead of pushing the revision content into the current like done in the patches suggested, think it makes more sense to change the rule and test the new revision state, just like it's done in the module invoke:

module_invoke_all('workbench_moderation_transition', $node, $node->workbench_moderation['my_revision']->state, $node->workbench_moderation_state_new);

Therefore i would test the new current state on workbench_moderation_state_new instead, this way you can still access the information from the current-before-save version, which i do need in my case.

Thanks Delphine Lepers,

I tested patch #34 for my sites via following cases, it did work for me.

1. Create a new content (state draft)
2. Create a new content (state published)
3. Edit an existing content (state need review)
4. Edit an existing content (state published)
5. Update a content state via moderation page (node/%/moderation)
6. Unpublished a content

patch #34 makes more sense to me.

Good work.

To be complete, the previous state should be fixed as well.
I am attaching a new version

There seem to be two equally viable approaches in this thread. If for some reason the first method of addressing the issue within workbench_moderation_save() is not acceptable, I think that patch #36 needs a tweak in order to not return true when you're not looking at the current version ($node->is_current). Patch attached.

Without this additional logic, a user would see unexpected triggering of a rule when checking a specific state (on existing content) since the previous state matched and was also evaluated in the workbench_moderation_rules_condition_contents_current_state() function.

My last file upload failed validation due to no line at end of file. Fixed patch attached.

Indeed, we have 2 approaches to fix this issue.

At first sight, I am in favour of the "workbench_moderation_save()" approach because it is more central and respects the chronology of the events as called in implementations of the "hook_node_insert" and "hook_node_update" called in "node_save"; I.E.:

1. The node is saved
2. workbench_moderation_save is called via one the 2 hooks, as the node is already saved, then $new_revision is virtually the new current revision.

"Rules" triggers also node related rules between the node save and the node save transaction commit; I.E. but through implementations of hook_entity_update or hook_entity_insert instead of implementations of hook_node_update or hook_node_insert.

I added "virtually" in the 2nd point because the transaction where the node is saved is not committed yet; which means the newly saved data are not available for other queries.
It is when all hook_node_update and hook_entity_update, or hook_node_insert and hook_entity_insert + the cache reset, that the transaction will be committed and the node data will be effectively persisted (see doc of hook_node_update, hook_node_insert and node_save).

But if we choose this approach, should we be consistent and passing the state set in the "current" item in the "workbench_moderation_transition" invocations like in the example below?:
module_invoke_all('workbench_moderation_transition', $node, $node->workbench_moderation['my_revision']->state, $node->workbench_moderation['current']->state);

The same kind of fix as #36 was suggested on issue 1568272 long ago https://www.drupal.org/project/workbench_moderation/issues/1568272

I agree with the comment that if patch #26 is chosen, they the invoke_all function should be adapted
@jlancaster #39 did not pass the tests.

After further tests, I change my mind about the way to fix this ticket.

On the paper, the fix in "workbench_moderation_save" is great because it allows to fix multiple issues at once but my last tests show that it does not fix the issue when we define a rule on the "Before saving content" event.

Indeed, this kind of rules is triggered in a hook_node_presave while the "workbench_moderation_save" function is called after the node saving.

Then, the fix has no effect on the rule behaviour in this case while the patch based on an modification in the functions "workbench_moderation_rules_condition_contents_current_state" and "workbench_moderation_rules_condition_contents_previous_state", works.

I provide a new patch because the #38 patch raised a PHP notices with a rule on a rule on the "Before saving content" event:

• When the rule is triggered before the node creation, $node->workbench_moderation is not set yet.
  Then, the "workbench_moderation_rules_condition_contents_previous_state" function does not work in this case.
• The "is_current" flag is not set yet, then "if" that use it raise PHP notice and the test does not work.
  Then, I propose a new "workbench_moderation_node_is_current_revision" function that check if the tested node review is the current one whatever the case.
  This function is different from "workbench_moderation_node_is_current" because this one is focused on the live node revision which it is not what we want here.

Hi all,

I tested the patch #42, it's working in my configuration.
@Anskelt, did you had time to review it?
K.R

Tested #42

All working as expected

During further tests, my team discovered that the patch #42 generates the following PHP notice when we define a rules applied to any content type; even those that are not moderated:

Notice: Undefined property: stdClass::$workbench_moderation_state_current in workbench_moderation_rules_condition_contents_current_state() (line 258 of ../workbench_moderation/workbench_moderation.rules.inc).

I corrected my patch in consequence. I posted also the interdiff.

I have tested the #46 patch and it works.

Could we consider it as RTBC?

+1 for patch in #46

+1 for patch in #46

+1 for patch in #46

Created a patch against 7.x-3.0

Re-roll of #53 for 7.x-3.0