Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Attached is patch which should make the rules conditions more robust.
Don't check for additional empty values, when there are multiple values.
Fix checking for empty values.
Don't respect the value of private properties (starting with '_').
Please help testing if that works fine.
Comment | File | Size | Author |
---|---|---|---|
#20 | cck_condition.patch | 896 bytes | fago |
#18 | content.rules_.inc_.patch | 710 bytes | mani.atico |
#9 | 446390-9-cck-condition.patch | 1.37 KB | amitaibu |
#3 | snap1.png | 8.06 KB | amitaibu |
#4 | 446390-4-cck-condition.patch | 1.73 KB | amitaibu |
Comments
Comment #1
fagoComment #2
fagoAs this works fine for me, I've just committed it.
Should also fix #442476: "field has changed" condition evaluates to true even if the field has not changed (cck text field).
Comment #3
amitaibuThe fix isn't working on my case, I get
>> warning: array_diff_assoc() [function.array-diff-assoc]: Argument #2 is not an array in /var/www/XXX/sites/all/modules/cck/includes/content.rules.inc on line 338.
In the attached image you can see the cause of the problem is that we accept the node value to be an array, but it is not.
Comment #4
amitaibuAttached patch deals with comparing an empty node field and an empty CCK field.
Comment #5
fagohm, with which field are you experiencing this? In my tests it worked fine with an empty textfield.
Anyway, what about adding just an empty array? Doesn't that suffice?
I'd prefer better readable code if possible.
Comment #6
amitaibuI'm using a filefield and textfield. I'll check the
$node_value += array($delta => array());
, adn re-roll.Comment #7
amitaibu$node_value += array($delta => array());
isn't good enough because an empty file field is field_foo[0] = NULL, so the above statement will be skipped ($node_value isn't empty in delta 0).
Comment #8
fagoah, I see.
so then I think it should just be:
Does that work for you?
Comment #9
amitaibuIt works with code from #8 - re-rolled.
Comment #10
amitaibuOk, I have re-checked and the solution in #9 is *not* correct. Patch in #4 is still the correct one.
Comment #11
amitaibu@Fago,
Can you please revisit this issue.
Comment #12
drupup CreditAttribution: drupup commentedI think my situation relates to this issue.
I've applied the #4 patch to the most recent version of content.rules.inc (cleanup patch already committed). All modules are latest and greatest and Drupal is 6.12.
I'm trying to set up a rule to notify the admin when users add a non-flv. video to their profile (we need to convert them ourselves). The field is a filefield for the non-.flv file. I only want to be contacted when the field has been changed and when there's something there (I don't care if they remove an upload).
Conditions:
Existing content has been updated
Updated content's field has been changed (non-.flv filefield)
Updated content's field has a value (non-.flv filefield not NULL)
I thought the problem was with the value being evaluated, but after a lot of testing I've found it's the "field has been changed" condition that's causing the error.
If I ONLY have the "field has been changed" condition, I get:
warning: array_diff_assoc() [function.array-diff-assoc]: Argument #1 is not an array in sites/all/modules/cck/includes/content.rules.inc on line 338.
If I have the two conditions together, I get:
warning: array_diff_assoc() [function.array-diff-assoc]: Argument #1 is not an array in sites/all/modules/cck/includes/content.rules.inc on line 347.
Curiously, this only happens when I upload a file. When I remove an existing upload, the rule works correctly.
The "field has value" condition is evaluating correctly with the "return array( 0 => array('value' => NULL));" entered (and Negated). I thought this was what I was testing all this time! Careful about what you assume...
I also note that when I delete the "field has value" condition from the rule, I get this warning:
warning: Invalid argument supplied for foreach() in sites/all/modules/filefield/filefield_widget.inc on line 512.
I've run this through several hours of testing, and these have all happened consistently.
Apologies if this is a separate issue, but I think a lot of people will use this logic in setting up their conditions. This will be an extremely valuable addition to Rules for admins when all the kinks are worked out. Thanks for the work! Hope this helps.
Comment #13
amitaibuI was able to fix my issue without patching. @drupup, I assume it will work for you as-well, otherwise please re-open the issue.
I have an PHP execute action, that clears fields (this is the working action):
Comment #14
drupup CreditAttribution: drupup commentedTried adding the code as an action. No change. Still get:
warning: array_diff_assoc() [function.array-diff-assoc]: Argument #1 is not an array in /home2/recfunds/public_html/sites/all/modules/cck/includes/content.rules.inc on line 347.
If I upload a video.
If I remove a video, it works fine. This is true whether I set the "has value" condition to check for a NULL value, or to check for a NULL value/negated.
The issue appears to be when the "field has been changed" condition evaluates and the field has a value.
Also, if I remove or edit the "has value" condition, I continue to get this:
warning: Invalid argument supplied for foreach() in sites/all/modules/filefield/filefield_widget.inc on line 512.
And to make things even more interesting, I sometimes get this when I add the action with the PHP code:
Parse error: syntax error, unexpected $end, expecting '(' in /home2/recfunds/public_html/sites/all/modules/rules/rules/modules/php.rules.inc(107) : eval()'d code on line 1
I believe I entered and edited the code correctly, without typos.
Interesting.
Comment #15
amitaibuDon't include the
<?php ?>
tags in the action.Comment #16
drupup CreditAttribution: drupup commented...which I knew. You test too much, you start getting stupid.
However, reinserting action without the tags, I still get the error message. Action looks like this:
// Clear fields.
$fields = array(
// Your fields below.
'field_video_nonflv',
);
foreach ($fields as $field) {
$node->$field = array(0 => array()); // Before I used to array(0 => NULL), which caused the error.
}
return array("node" => $node);
That's the name of the field in question.
Comment #17
mani.atico CreditAttribution: mani.atico commentedI'm having this same problem with cck 2.5. I want to evaluate if an imagefield does have an image, but I get a
warning: array_diff_assoc() [function.array-diff-assoc]: Argument #2 is not an array in /xxxx/drupal/sites/all/modules/cck/includes/content.rules.inc on line 338.
meesage.
An interesting thing is that the condition works: it does execute the action when it should - the problem is this error message.
I tried addind the field cleaning php action on #13, but it didn't affect the results.
Is there a workaround for this?
Thank you
Comment #18
mani.atico CreditAttribution: mani.atico commentedI'm not sure if another issue should be opened, but the problem has been stated here so I think is coherent to present this patch at this post.
As mentioned on the previous post I was having trouble using the "field has value" condition and a filefield field. The problem was that empty filefield fields were loaded as
Array ( [0] => NULL )
, and the method comparing values expected that the first key ([0]) held an array as a value. This caused the following error message to appear:warning: array_diff_assoc() [function.array-diff-assoc]: Argument #2 is not an array in /xxxx/drupal/sites/all/modules/cck/includes/content.rules.inc on line 338.
I was able to avoid this by applying the attached patch. It adds few lines to the _content_rules_field_has_value function to check if the values being compared are arrays.
Although this patch worked for me, I don't if it might intefere with other cck fields, and neither do I know if this might be a filefield module issue :: I'm no expert on cck's code and standards, but there is a compatibility issue on what cck expects to be an empty field and how filefield handles one. Anyone who knows more about this please give us a light.
Comment #19
mani.atico CreditAttribution: mani.atico commentedRe-opening to get some attention. Excuse me if this could be another issue.
Comment #20
fagoThat patch looks good, I took it and fixed the coding style. Does it work right for others too?
Comment #21
thekayra CreditAttribution: thekayra commentedI am using Embedded Media Field as well as filefield->imagefield and trying to check if the fields have changed pretty much the same way @drupup at #12 is doing.
I applied the patch at #20 on dev. It looks like imagefield check goes well but unfortunately I am having problem problem with Embedded Media Field this time.
So in terms of testing #20 for filefield, I can confirm that it works for me. But as new/side issue it does not work with Embedded Media Field (tired of writing this should have copied and pasted)
Comment #22
suydam CreditAttribution: suydam commentedPatch #20 worked great for me. Thanks @fago
Comment #23
Bilmar CreditAttribution: Bilmar commentedsubscribing
Comment #24
josepvalls CreditAttribution: josepvalls commentedHi,
I ran into the same problem here. I applied the patch, now, how should I set up my rule so it sets a value whenever the user leaves it blank or deletes an existing file?
I tried leaving the file field in rules blank, I also tried the following php code:
return NULL;
return array();
return array(NULL);
return array(0 => NULL);
return array(array());
I tried using the events after updating existing content and content is going to be saved.
Comment #25
fagothanks for testing, the patch for #20 seems to be an improvement and makes the code more robust nevertheless, so I committed it. Thanks!
@#24: Try using the the devel module and its provided "dev load" tab for the node to see how the data structure looks like.
Comment #26
Daniel A. Beilinson CreditAttribution: Daniel A. Beilinson commentedHello!
I think event "content is going to be saved" with cck is totally broken.
I have latest dev versions of rules and cck.
And I have four rules for my content type. They said if I change my date field 1 then, please, populate another field with value yes and if I NOT change it then populate no value. And another pair of rules for another date field. So this rules don't work for me. They populate yes value even if I create new revision without changes anywhere.
But this rules worked for me with event "after updating existing content".
What do you think?
Comment #27
Daniel A. Beilinson CreditAttribution: Daniel A. Beilinson commentedI just switched off 3 of this rules. So I have one rule which says if i didn't change date field populate value no. And it isn't work.
Comment #28
Daniel A. Beilinson CreditAttribution: Daniel A. Beilinson commentedSo, as I see, this's cck date problems.
Comment #29
Daniel A. Beilinson CreditAttribution: Daniel A. Beilinson commentedJust open separate issue for this bug.
#668122: date changing when content is going to be saved
Comment #31
iosif.peterfi CreditAttribution: iosif.peterfi commentedI tried this on a filefiled image with multiple values.
the count comparision breaks the validation. I have count($node_value) evaluates to 1 while count($value) evaluates to 5.
I'd like to add that the fields are empty and probably i had no multiple values when i created the node. Now i can add up to 5 values.
Here's the code:
function _content_rules_field_has_value($node_value, $value) {
+ foreach ($node_value as $delta => $sub_value)
+ if(empty($node_value[$delta])) unset($node_value[$delta]);
+
+ foreach ($value as $delta => $sub_value)
+ if(empty($value[$delta])) unset($value[$delta]);
+
if (count($value) != count($node_value)) {
return FALSE;
}
This removes the empty values from the arrays and works fine.
Comment #32
thekenshow CreditAttribution: thekenshow commentedI applied the patch from #20 but still can't successfully test for an empty filefield. I have a custom node with multiple filefields, but only one file per field (I believe this is different from #31, who has multiple files per filefield.)
Here's my configuration:
Notes:
Testing other field types works fine. Suggestions would be great, thanks.
Comment #33
thekenshow CreditAttribution: thekenshow commentedI found a workaround to my empty FileField test here: http://groups.drupal.org/node/25672. I'm not sure if my issue is the sort of problem that patch #20 was intended to address, but if so it doesn't appear to work yet.
Ideally, the Form Has Value test would return a meaningful result for FileField without having to resort to PHP, but at least now I can work around it.
Comment #34
robby.smith CreditAttribution: robby.smith commented+1 subscribing
Comment #35
fagoPlease reopen that issue, it's way too long and confusing already. Next there is no patch for review.
If there are still problems with that please open a new issue.
Comment #36
tim.plunkettThe error also applies to 6.x-3.x-dev, as does the fix from #20. Can you commit it?
Comment #37
fagothanks, indeed the fix of #20 was missing for 3.x. Committed it.