Incorrect use of content_fields [#521358]

http://drupalcontrib.org/api/function/content_fields
from that, begining of content_fields function

function content_fields($field_name = NULL, $content_type_name = NULL) {
  $info = _content_type_info();
  if (isset($info['fields'])) {
    if (empty($field_name)) {
      return $info['fields'];
    }

in this module nodeaccess_userreference_node_access_records "

$info = content_fields(NULL, $node->type);

If no field name is supplied content fields returns generic information about all fields on the site.

That becomes an issue here

$variable = 'nodeaccess_userreference_'. $field['field_name'] .'_'. $field['type_name'];

If multiple content types have that field, type_name is not necessary that node type so incorrect information is returned from the variable_get. (ie type name != $node->type sometimes)..
A way to correct it

    $variable = 'nodeaccess_userreference_'. $field['field_name'] .'_'.$node->type;

Attached is a patch that does that.

However, that's a quick fix and may be better to use
http://drupalcontrib.org/api/function/content_types/6
On the content type, which I believe has a ['fields'] that contains fields specific to the content type. Not sure if it's correct type_name there, but it may speed up the foreach field.

... I decided to try it out after writing that, it appears to be working without even changing type_name.

Changed the content fields call to

  $info = content_types($node->type);
  $info = $info['fields'];

Comment	File	Size	Author
	nodeaccess_userreference_type_name_2.patch	982 bytes	hefox
	nodeaccess_userreference_type_name.patch	796 bytes	hefox

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

danielb CreditAttribution: danielb commented 17 July 2009 at 03:15

Thanks I'll look into this when I can, I've had trouble with this in other modules too.

Comment #2

danielb CreditAttribution: danielb commented 19 July 2009 at 23:54

So do you recommend applying both patches, or just the 2nd one?

(will change them a bit)


-        $variable = 'nodeaccess_userreference_'. $field['field_name'] .'_'. $field['type_name'];
+        $variable = 'nodeaccess_userreference_'. $field['field_name'] .'_'. $node->type;

-  $info = content_fields(NULL, $node->type);
+  $content_types = content_types($node->type);
+  $info = &$content_types['fields'];

Will also need to check my other nodeaccess and *reference modules for similar calls to content_fields() and possibly the variable storage by type as well.

Comment #3

hefox CreditAttribution: hefox commented 20 July 2009 at 04:41

Both work from what I can tell, together or independently, but the 2nd one is likely the better one since to my understanding both content_fields and content_types is accessing the same pre-made data, just in different ways and content_types is returning a smaller but accurate amount of field data. I'd go with number 1 also just in case it somehow doesn't return the right type in type_name, but I suspect it does when using content_types since it did for me (where it had the wrong type for content_fields). So yeah, either, or, both, XD.