In some cases, an action under "Content is going to be saved" will trigger rules to save the content after all actions are done, but before the content is actually "saved" from the event which raised the trigger.
This causes recusion to occure though the trigger; In my tests it always ran the same trigger twice before catching on.
The fix will probably be to ignore requests to save content if that content is in a "Content going to be saved" event run.
Also somewhat related, the save was being triggered by "Set CCK field". This is CCK specific, but it raises the question: Should actions trigger a node save when relevant, or should that be left to the rules user to make a "Save content" call? User-friendlyness points to the former, but that would directly cause the case of this bug unless the above fix is implemented (which of course will not be an issue if it is).
Comments
Comment #1
RoboPhred commentedWhy this is important: My specific setup doesn't care much. However, if you had a one-time action (rather than field changes as I do), it would run repeatedly:
Sending e-mails
Creating *
Duplicate messages (drupal_set_message / error).
Comment #2
fago>The fix will probably be to ignore requests to save content if that content is in a "Content going to be saved" event run.
It should already work that way. I'm going to give it a test.
Comment #3
RoboPhred commentedI tested it on my own with a new content type clearing a cck value as my broken rule does. It worked as expected... However, my actual production content types still have this issue.
Here is the relevant trace output from my bug report content type.
In this trace, I changed its status from "fixed" to "open". This should:
1. Clear the "target" cck field, becasue we were changed from "fixed".
2. Enable comments and post a comment, because we were changed to "open".
To help you make sense of this, I have two other rule sets being called from here. Both act like functions, in that they have a single rule with the same name and no conditions. They both have a single action of custom php code.
They are "Comments: Set enabled" which sets $node->comment without saving it, and "Comments: Create RPDNet comment" which creates a comment using the comment_edit function. Both of them are passed the content type as an argument, and the "create RPDNet comment" also has strings for subject and comment text.
Perhapse passing the content as an argument to a rule set has something to do with it?
I re-tested this with the current -dev (April 16) with no change.
Comment #4
RoboPhred commentedMore testing:
If I disable the rule "Executing the rule Status changed from Open but not to Resolved on rule set Bug report submitted" (which is misnamed, should be "...from Fixed or Resolved but not to either"), the issue does not occure. The only action this rule contains is setting the cck field to nothing (the field is a non-required noderef field). It also goes away if I leave the rule enabled, but remove its only action.
Giving a noderef field to the Test content type, and adding the same clearing action to it does not reproduce the problem...
Comment #5
RoboPhred commentedI tried throwing debug messages in the code, all I came up with is that somewhere between
node.rules.inc
and rules.variables.inc
$this->info['saved'] is being explicitly set to false.
A quick look in cck's content.rules.inc shows no actual changes to it.
Without knowing what actually happens, it might be possible that the node argument definition in content.rules.inc...
...is somehow overriding the settings of the existing node argument that is passed in to it and clearing the 'saved' value. However, this should be occurring every time then, and does not explain why it only happens with a "populate field" on the bug report set of rules and not on a separate content type...
Comment #6
fagoOh, I think I know what's the issue here. It's probably a duplicate of #373268: Using rule set invokes node_save() when not needed.
The rule set doesn't know about the variable info, it just knows that there is a node, and when it does changes, it saves them itself. That need to be improved, yes.
Comment #7
fagoAs you see in the other issue, it's fixed now. Please test and report whether it works now. But let's keep the discussion in the other issue.
Comment #8
mitchell commentedUpdated component.