Workflow sometimes inserting WF History logs or executing state changes twice

Ever figure this out? As far as I can tell, it only happens when a comment is added during the state change.

Suscribing, same issue ...duplicates on views that show workflow comments.

Subscribing... Same issue.

It appears to happen during state changes that involve publishing and unpublishing but only when there is a comment involved. (I've tried this in a number of combinations and I'm pretty sure this is how it's working on our system.) So if you change a node from Draft to Needs approval (both unpublished in our system) the log works normally whether there's a comment or not. If you push it from Needs approval to Live (published), but with no comment, still fine.

But then if you move it from Live to Retired (unpublished) with a comment, you get an extra log entry. Could this be caused by the fact that when you choose Publish or Unpublish Node as an action for a state change, it also automatically adds a second action, Save Node?

The root of this problem is way how workflow plugs itself: its called via nodeapi update hook. But in the process of transition from one state to another nodeapi update event maybe fired more than once, so workflow module must implement some logic around this.
Currently workflow writes comment to workflow history on EVERY subsequent nodeapi update event in same request.
If your use case involves some additional modules that depend on workflow changes, they are probably firing up those subsequent nodeapi events.
I'll try to make patch for this, but for 5.x ( yes, this problem is THAT old ).

Please review this patch. Marking it as 5.x but you can apply it manually to 6.x, change is very simple.
This patch is very UGLY. It tries to fix with small workaround one big problem: workflow module does not have proper logic to control subsequent runs.
And for this I made more "general" issue #472966: Avoid execute_transition twice (when using Rules, Actions). Not marking this as duplicate because it contains simple, ugly, but working patch

Tried your patch on a 6.9 install, but does not work for all cases.

On actions triggered by the creation of a node it does not work.
I get two duplicate messages in my page, of which one does not correctly replace the %title and %node_type placeholders, but the other one does.
I get two mals, , of which one does not correctly replace the %title and %node_type placeholders, but the other one does.
Replacing the placeholders is works for the %username though, in all circumstances. So I think the first run is when the node is still in a void somewhere, but the user already exists (of course).

Some other state change actions now are single, but I must say I did not check them before the patch (cause were building the site right now).

Apart from the duplication of the actions, the problems with the placeholders are very annoying. Sending a mail to someone with email adres %author never reaches anyone of course. is see that is covered by patches mentioned in this thread http://drupal.org/node/253711 but is seems related?

What do your actions do ?
Well I don't have any experience using combinations of actions and workflow, but looking at the code it seems actions themselves should have mechanics to control "dummy" runs where nothing is changed.
Placeholders problem is separate from this issue. This patch deals only with one problem: duplicate comments in history log.
Meanwhile I'll look into 6.x version further if I have time...

IIRC workflow executes actions in workflow_workflow() function. So I think actions problems are different than this problem, but they are ofcourse connected to #472966: Avoid execute_transition twice (when using Rules, Actions). Workflow runs these "post" and "pre" hooks (which trigger actions in your case) on every run, without control.

I am abserving this issue still with the latest stable release (6.x-1.4) and the latest dev (6.x-1.x-dev) release

The state chances are double logged when
1. A comment is present
2. An effective state change happens (going from stateA to stateA with a comment does not produce a double).

I know there is a more general issue, http://drupal.org/node/472966, but thought I post it here since its about V5 but still present in V6

I modified the module a bit since. Excuse me if this is ugly but thats how I got it fixed.

I check if there is already a record in the log table with the same stamp before inserting.

function _workflow_write_history($node, $sid, $comment) {
global $user;

//tom hack
//ori:
//db_query("INSERT INTO {workflow_node_history} (nid, old_sid, sid, uid, comment, stamp) VALUES (%d, %d, %d, %d, '%s', %d)", $node->nid, $node->_workflow, $sid, $user->uid, $comment, $node->workflow_stamp);
//modi:
$stampQuery = db_query('select * from workflow_node_history where stamp = %d', $node->workflow_stamp);
$log = db_fetch_object($stampQuery);
if (empty($log) == TRUE){
db_query("INSERT INTO {workflow_node_history} (nid, old_sid, sid, uid, comment, stamp) VALUES (%d, %d, %d, %d, '%s', %d)", $node->nid, $node->_workflow, $sid, $user->uid, $comment, $node->workflow_stamp);
}
}

Actually the stamp shouldn't be the same, though it will be in most cases.
After some trials obtained the following values in 'stamp':
1271756533
1271756534

For now saving myself with unset($node->workflow_comment); in function workflow_execute_transition before the string
$result = module_invoke_all('workflow', 'transition post', $old_sid, $sid, $node);
in both cases (in the first case it is included by default).

Of course it's not the solution if comment is needed for some reason.

Since this request is over a year old, I'm going to assume a solution was found or you've moved on. If not, please get updated to the latest versions of all modules and make a patch for it against Drupal 7 and I'll be glad to take a look at it.

I think i am still seeing this behavior on drupal 7. This patch seems to have resolved the issue for me. Basically, i just check the last history entry timestamp against the current request timestamp if the old_sid and new sid are the same. I'm able to add comments even with no transitions and i'm no longer getting dups.

Hey, we have a patch! yay! Setting this to open again, and when I get a chance to go through the various patches currently in the issue queue, this one will get looked at as well. In the meantime, if others can review it as well under 7.x that would be useful.

I'm seeing this in 6.x-1.5 when using Actions to Publish (and save) a node after a workflow state is assigned.

Any chance this patch will get applied (and backported) any time soon? It's above my skill level, so I'm not much help apart from reporting it.

Use logic of patch in comment #14 to create patch for version 7.x-1.x-dev.

@flyingkiki: Do not use tabs in Drupal code, and comments should have a space before the first letter, and should be in sentence case. Thanks for the patch.

Committed to 7.x-1.x-dev.