Avoid execute_transition twice (when using Rules, Actions)

subscribing

Coming from #370111: Rules integration for Workflow.

Here is a shorter version of the patch.
* comment unsetting is done *after* the hook is called (comment is still available from the node in hook implementations)
* the workflow variable of the node is set to the new state after a transition. This should avoid double execution of a state transition.

It works fine for me, I tested some variations.

The Rules integration from #370111: Rules integration for Workflow does not make use of hook_workflow() and the comment is unset *before* the Rules integration is called. I decided to leave the comment in the node to not break existing hook implementations that rely on it.

Could you do some testing and describe an example where you get double runs?

New patch: considering comment #7, unsetting the comment is now done *before* the hook invocation. Thanks for the explanation!

In my patch it works the same as in your patch: "empty" subsequent runs stop at the same place as in your patch, because a written comment is cleared immediately after it was written to the history.

The problem that I see at the moment: we cannot detect whether the hook_nodeapi('update') call is a subsequent run or a completely different node update call. In my opinion, adding more complex mechanisms would blow up the code and make it even more difficult to manage. I suggest to stick with the simple unset of the comment, it works and fulfills our needs.

It also makes sense to provide the comment within the node in the first hook invocation ("transition pre") and to remove/unset it afterwards in hook "transition post".

I think a simple solution like that from #9 would suffice as it's a quite common approach to just unset the variable once it has been processed.

So I gave the patch from #9 a test together with #370111: Rules integration for Workflow - it's working fine, no subsequent runs appeared in my tests.

Tested the patch for a certain time and it works great (together with #370111 ).
However - if you make a state transition (e.g. using the workflow tab) which triggers a rules' action to modify the node again (where transition without state change occur) a new transition is added with identical comment.

If you decide to unset the comment in case ($old_sid == $sid) we should think about unsetting the comment after state transition (_workflow_node_to_state), before trigger (module_invoke_all('workflow'.. ), too. This solves my problem. However the rules trigger will then lack the comment (attached to node). But i think this would be ok. If not we need to introduce a different subsequent runs control strategy (such as attaching a value to the node).

Any inputs appreciated. Will provide patch then.

Hm. I got here from #309974: Duplicate nid creation when making forum topics and adding to organic groups. I'm using OG Actions with Workflow and was experiencing "Duplicate key" errors during creation, and saw a pointer to this patch.

Before applying the patch, what happens is all of my assigned actions to fire on (creation) => State 1 transition (add/remove the post to some groups, etc.) fire, but I get a duplicate key error.

With the patch in #9, I still get the duplicate key error, and none of my actions show up as fired in dblog as they normally do.

I can't be sure this isn't just something in my environment, so I won't set this down to "needs work," but definitely test with OG and see what happens.

It might be an issue of weight.
I remember some issue addressing this ... telling to increase weight -- but i can't find it anymore.
This could happen if actions that trigger a node_save (not being executed at the end of save) will result in some modules receiving update before save...

Hm. Yes, that makes sense. It sounds like I might be running into #27007: Add 'insert/update post' op to hook_nodeapi.

SWEET MERCIFUL CRAP. Thank you. That was it. Updating workflow to weight 1 solved my issue.

I'll now stay out of this since it's totally unrelated. :)

Applied #9 to 6.x-1.x-dev except for the setting of $node->workflow, which is a data property coming from a form.

As a note, could this issue be related to the following one? #722432: Duplicate emails when Content Notifications used with Workflow module

I tried the latest dev release mentioned in #18 to see if that solved it, but no luck.

Issue is still in the latest dev and stable release that I downloaded from the homepage

I still see duplications in 6.x-1.4 as well. See my post at http://drupal.org/node/309974#comment-2914150 Changing Workflow module's weight in the system table seems to work as a temporary solution.

I get duplicate workflow history entries when changing states, but only when accompanying that state change with a comment. This exists on 6.x-1.x-dev and 6.x-2.x-dev. Raising the module weight in the system table doesn't work in my case.

If I disable my rule (Rules module) that performs an action on the node after the state has changed (unpublishing the node in my case), the duplicate history doesn't happen.

Removing the hunk that came from workflow-370111.patch from 6.x-1.x-dev and 6.x-2.x-dev, and then adapting crea's patch workflow_subsequent.patch, works!

I know this ^^ is very hackish and violating, but if someone wants this fix for 6.x-2.x-dev, patch for workflow.inc is attached.

I am using 6.x-3.15 and am still seeing issues with this, see http://drupal.org/node/1611874

Marked the following as duplicate:
#1212614: State is changed twice - only when adding a comment during state change

Moving all remaining D6 issues to D7.

IMO this is resolved/implemented in some other issue in version 7.x-2.x.
I wish there was a nicer way to clear the issue queue from 'D6-issues that are fixed in D7' then a "won't fix for D6."

I think patch #24 would work, but I didn't see it. Here is my patch that works too (and easier to understand).