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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fago’s picture

Status: Active » Needs review
fago’s picture

Status: Needs review » Fixed
amitaibu’s picture

Assigned: amitaibu » Unassigned
Status: Needs work » Active
FileSize
8.06 KB

The 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.

amitaibu’s picture

Assigned: Unassigned » amitaibu
Status: Fixed » Needs review
FileSize
1.73 KB

Attached patch deals with comparing an empty node field and an empty CCK field.

fago’s picture

Status: Needs review » Needs work

hm, 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?

$node_value += array($delta => array());

I'd prefer better readable code if possible.

amitaibu’s picture

I'm using a filefield and textfield. I'll check the $node_value += array($delta => array());, adn re-roll.

amitaibu’s picture

$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).

fago’s picture

ah, I see.

so then I think it should just be:

    if (empty($node_value[$delta])) {
      $node_value[$delta] = array();
    }

Does that work for you?

amitaibu’s picture

Assigned: Unassigned » amitaibu
Category: task » bug
Status: Active » Needs review
FileSize
1.37 KB

It works with code from #8 - re-rolled.

amitaibu’s picture

Ok, I have re-checked and the solution in #9 is *not* correct. Patch in #4 is still the correct one.

amitaibu’s picture

@Fago,
Can you please revisit this issue.

drupup’s picture

I 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.

amitaibu’s picture

Status: Needs review » Fixed

I 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):

  // Clear fields.
  $fields = array(
    // Your fields below.
    'field_file1',
    'field_file2',
    'field_text',
  );
  foreach ($fields as $field) {
    $node->$field = array(0 => array()); // Before I used to array(0 => NULL), which caused the error.
  }
  return array("node" => $node);
drupup’s picture

Tried 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.

amitaibu’s picture

Don't include the <?php ?> tags in the action.

drupup’s picture

...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.

mani.atico’s picture

I'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

mani.atico’s picture

FileSize
710 bytes

I'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.

mani.atico’s picture

Status: Fixed » Needs work

Re-opening to get some attention. Excuse me if this could be another issue.

fago’s picture

Status: Needs work » Needs review
FileSize
896 bytes

That patch looks good, I took it and fixed the coding style. Does it work right for others too?

thekayra’s picture

I 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)

suydam’s picture

Patch #20 worked great for me. Thanks @fago

Bilmar’s picture

subscribing

josepvalls’s picture

Hi,

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.

fago’s picture

Status: Needs review » Fixed

thanks 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.

Daniel A. Beilinson’s picture

Status: Fixed » Active

Hello!
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?

Daniel A. Beilinson’s picture

I 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.

Daniel A. Beilinson’s picture

So, as I see, this's cck date problems.

Daniel A. Beilinson’s picture

Status: Active » Fixed

Just open separate issue for this bug.
#668122: date changing when content is going to be saved

Status: Fixed » Closed (fixed)

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

iosif.peterfi’s picture

Assigned: amitaibu » Unassigned
Status: Closed (fixed) » Needs review

I 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.

thekenshow’s picture

I 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:

  • Trigger: Form is being built
  • Condition: Form element has value
  • Form element ID: field_abc[0]
  • Value(s): NULL

Notes:

  • I've tried an element id of 'field_abc' and 'field_abc[filename]'. With display form element ids enabled, I see only 'container element id' listed for the filefield elements, but 'container element id' and 'element id' for the other cck fields. I thought the field name was my only problem until I found this thread.
  • I've tried 0 and FALSE for the value.

Testing other field types works fine. Suggestions would be great, thanks.

thekenshow’s picture

I 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.

robby.smith’s picture

+1 subscribing

fago’s picture

Status: Needs review » Closed (fixed)

Please 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.

tim.plunkett’s picture

Version: 6.x-2.x-dev » 6.x-3.x-dev
Status: Closed (fixed) » Reviewed & tested by the community

The error also applies to 6.x-3.x-dev, as does the fix from #20. Can you commit it?

fago’s picture

Status: Reviewed & tested by the community » Fixed

thanks, indeed the fix of #20 was missing for 3.x. Committed it.

Status: Fixed » Closed (fixed)

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