Hi,
My requirement is that in a workflow when the state changes from one state to another,mail has to be sent.I have set a rule for this.The event as "workflow state has changed",condition as "check workflow transition" and the action as "send mail to all users of a role". But this rule is not getting triggered. Please help me out.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mwallenberg’s picture

Title: Workflow rules » Workflow rule "Check workflow transition" not working
Category: task » bug

I have also set up a rule that uses "Check workflow transition". The rule works when "Old workflow state" is set to ANY state, but if I select a specific state, then the condition is always evaluated to FALSE.

The condition is used in a component rule, which takes a node as argument. The component rule is used by a triggered rule in answer to the event "Workflow state has changed".

I am using 7.x-1.0 with rules 7.x-2.0.

Patribus’s picture

Hello! Same problem here!

Any solutions to this problem?

markgp’s picture

It appears to me that workflow_node_previous_state is returning the current sid rather than old_sid as it should be. The following fix worked for me (apologies, I don't know how to make patch files).

// workflow.module line 810
function workflow_node_previous_state($node) {
  $sid = FALSE;
  // There is no nid when creating a node.
  if (!empty($node->nid)) {
    $sids = array();
    $sid = -1;
    $last_history = workflow_get_workflow_node_history_by_nid($node->nid, 1);
    // $sid = $last_history->sid;  // <------ old line
    $sid = $last_history->old_sid; // <------ new line
  }
  if (!$sid && !empty($node->type)) {
    // No current state. Use creation state.
    $sid = FALSE;
    if ($workflow = workflow_get_workflow_type_map_by_type($node->type)) {
      $sid = workflow_get_creation_state_by_wid($workflow->wid);
    }
  }
  return $sid;
}

I should note that my rule evaluates on "After updating existing content". Not sure if that makes any difference.

Patribus’s picture

Hi!

Thanks for the code. I'll edit my file and check if it works.
Pat

Bastlynn’s picture

Status: Active » Needs review

Please let me know if this worked for you. If so I'll test it on my end as well and get it into the dev branch.

rhayun’s picture

Look like #3 fix the problem..

what about release new version that fix this?

jramby’s picture

Hi,
Same problem here even in dev version... I didn't try this patch in 7.x-1.0.. In dev version the function seem to already have this "new line" but the same problem persists.

Bastlynn’s picture

Try this patch, let me know if it works.

chris.smith’s picture

We just upgraded to Workflow 7.x-1.0 and this issue still exists. We have Rules 7.x-2.1. The patch in #8 does not work for us.

Patribus’s picture

I used the code in #3 and it works just fine.

chris.smith’s picture

Could you confirm what version of Workflow and Rules you are using? The most recent version of Workflow includes the changes in #3, but the problem still exists.

n8handler’s picture

Version: 7.x-1.0 » 7.x-1.x-dev
Status: Needs review » Reviewed & tested by the community

I can confirm that the patch in #8 fixes the issue. I am using 7.x-1.0+18-dev, the code is not already present in this version and 18-dev is the latest version at the the of this writing. Also the patch in #8 changes more than just the line in #3 (not sure if that's relevant).

Patribus’s picture

I'm using workflow 7.x-1.0+9-dev

chris.smith’s picture

Just validated to be working on dev-19. Thanks for your help.

nevets’s picture

Bug still exists in latest D7 dev release, changing $last_history->sid to $last_history->old_sid fixed the problem.

dmadruga’s picture

#3 works beautifully!
Thks, markgp! You saved my Friday :-)

jorgecsilva’s picture

Thank you markgp!! #3 worked for me also...

berenddeboer’s picture

This is a reroll of #8 against version 1.0 for those not using the dev version.

jaarong’s picture

I take it that this patch is not in dev? Is there a chance it will make it in soon?

Finn Lewis’s picture

I can confirm that the patch does NOT appear to be in the latest dev.

The dev version just downloaded (7.x-1.0+19-dev) has this on line 781 of workflow.module:

$sid = !$last_history ? false : $last_history->sid;

Changing this to:

$sid = !$last_history ? false : $last_history->old_sid;

and rules kick in as expected when workflow state changes from one state to another.

mwallenberg’s picture

I can just confirm that the change described in #3 works for my setup (I am using 7.x-1.0 with rules 7.x-2.0.)

cswan’s picture

#3 worked for me also. Thank you! ... I'm using Workflow 7.x-1.0 with Rules 7.x-2.2.

pumpkinkid’s picture

I too was able to correct this thanks to #3... however, I had updated to the Dev version and as described in #20, noticed the line in question is different...

Looks like the line in dev should read:

$sid = !$last_history ? false : $old_history->sid;

Anyone think that first "!$last_history" also needs to be changed?

shenzhuxi’s picture

#18 works for 1.0

shenzhuxi’s picture

#8 works.
#18 can't be used with drush.

Sebastien VR’s picture

#20 works for dev-version (v +20)

nubeli’s picture

Component: Miscellaneous » Code
Priority: Major » Critical

Patch in #8 works for me with dev version +20.

Aron Novak’s picture

Another attempt, for 1.0

nagy.balint’s picture

#28 works perfectly for 1.0

Before this the rule always got fired on each node update, even if the status didnt change. But now it works fine with this patch.

shenzhuxi’s picture

#28 works with Drush 5.8

NancyDru’s picture

Priority: Critical » Major

Which version should we commit?

NancyDru’s picture

Status: Reviewed & tested by the community » Needs review

With at least two different patches following the RTBC, we need it reviewed again.

haggan’s picture

#28 Patch did not solve the rules problem on dev version 25

/Jonas

nagy.balint’s picture

#28 Patch has to be modified to work on the latest dev. It was originally made for 1.0 and there are some code changes.

NancyDru’s picture

Looks like BastLynn's patch in #8 is the winner!

nagy.balint’s picture

I think that with #8 if you set the rule up to react on node update, and to have the transition condition, then that rule will always fire (meaning even if you dont change the status it will always fire).

NancyDru’s picture

Well, please test it after I commit.

NancyDru’s picture

Status: Needs review » Fixed

Committed to -dev.

darko.ljubic’s picture

7.x-1.0+35-dev
"Check workflow transition" still not working (for me)!
The event is "workflow state has changed".

NancyDru’s picture

I don't see where you're getting "Check workflow transition." The correct rule should be "Workflow state has changed."

darko.ljubic’s picture

What I meant was that the rule is not doing what it is supposed to do when I set EVENT to "workflow state has changed", CONDITION to "check workflow transition" and action to whatever .

I tested the code and found out that the problem is caused by workflow history fetching method...

In dev release workflow_get_recent_node_history was used, and for that particular function the return value is being cached (statically inside the function), and by the time we call it, it has "stale" workflow history data because workflow state has indeed changed (our EVENT occurred). So $last_history->old_sid contains the old state id from previous state change, not the last one.

I highly recommend that we use workflow_get_workflow_node_history_by_nid function that is NOT CACHING the return value of last workflow history state. This function was used in recommended release (7.x-1.0).
I tested it, and it works. Can you please confirm?

/*
version: 7.x-1.0+35-dev
file: workflow.module 
line change: 767
*/
if (!empty($node->nid)) {
  $sids = array();
  $sid = -1;
  // $last_history = workflow_get_recent_node_history($node->nid);     // <------ OLD line (cached history)
  $last_history = workflow_get_workflow_node_history_by_nid($node->nid, 1);// <------ NEW line (not cached)
  $sid = !$last_history ? FALSE : $last_history->old_sid;
}

I'm sorry, I don't know how to make patch files. I promise I will learn it as soon as possible :)

berenddeboer’s picture

Making a patch is very simple if you got installed the dev version by cloning with git. Then you just say "git diff".

NancyDru’s picture

Yes, it would be, if that's the right patch. Removing the caching might be better.

NancyDru’s picture

Status: Fixed » Closed (fixed)

Included in 7.x-1.1-beta2.

rickmanelius’s picture

Status: Closed (fixed) » Active

I'm re-opening this thread because the latest beta (7.x-1.1-beta3) needs the adjustments in #41 to work properly. Previously, the patches noted in #44 fixed the situation where a new node was being created, but it was not working for "Workflow state has changed" event. I have a client with a very involved set of rules with several state transitions that I test with Selenium, and adding #41 brought me back to a green board.

rickmanelius’s picture

Status: Active » Needs review
FileSize
506 bytes

And here's a patch...

NancyDru’s picture

Status: Needs review » Fixed

I removed the static caching. Hopefully the database will have it cached if it is needed again.

shenzhuxi’s picture

The lastest dev still needs patch #46 to work.

NancyDru’s picture

Then the description in #41 is wrong?

rickmanelius’s picture

I'm wondering if the downloadable version was not already in place at the time of #48.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.