Greetings,

I'm using the CCK field privacy module on a project for ISL (http://islco.com).

We've found CCK field privacy to complete most of what is needed, but we've come across a couple of bugs for which I'd like to submit a patch:

(1) When trying to restrict access to a field that's in a group, the setting gets ignored. Currently, the code in cck_field_privacy_nodeapi() for $op == 'view' does the following:

          $access_clear = _cck_field_privacy_access_check($fieldpriv, $node, $user, $node_user);
          if ($access_clear == FALSE) {
            $node->$field_name['#access'] = FALSE;
            $node->content[$field_name]['#access'] = FALSE;
          }
          else {
                 ....
          }

However, the code doesn't account for the situation where $field_name is part of a group in the $node->content array. This patch accounts for it by changing the code to the following:

          $access_clear = _cck_field_privacy_access_check($fieldpriv, $node, $user, $node_user);
          if ($access_clear == FALSE) {
            $node->$field_name['#access'] = FALSE;
            $node->content[$field_name]['#access'] = FALSE;
            // if the thing is part of a group, set access to false within the group
            
            if ($type_fields[$field_name]['display_settings']['parent'] && !is_numeric($type_fields[$field_name]['display_settings']['parent'])) {
              $group_name = $type_fields[$field_name]['display_settings']['parent'];
              $node->content[$group_name]['group'][$field_name]['#access'] = FALSE;
              $node->content[$group_name]['group'][$field_name]['field']['#access'] = FALSE;
            }
          }
          else {
             ..........
          }

$type_fields is set prior to the beginning of the loop to be content_fields(NULL, $node_type)

(2) The code does not add a padlock to some fields that are in groups. Currently, the recursive function _cck_field_privacy_add_padlock(&$elements, &$needpadlock) runs through the top-level fields for each needed padlock and for whatever reason, it will encounter elements with the same key as a field name that requires a padlock, but the value of that key won't be set to an actual form element. The real form element is nested deeper in the form array. The if-else that checks for the presence of the element will wrongly set $needpadlock[$field] = FALSE where there is the comment /* field type not properly handled */. I've changed the code so it actually only sets it to false, when it actually adds a padlock. Otherwise, keep recursing through the form array and only set that array value to false if you find the field.

(3) There is currently code in the hook_form_alter() that tries to keep the padlock from being added to the node form when the node form is a part of the registration form. Unfortunately, in our case, we're including a node form (for node-based profiles) within our registration form. Since we're calling the node form from within our call to get the registration form, this hook is called twice. When called during the building of the node form, the hook doesn't recognize it as part of the user registration form. I've changed one of the checks from

if (($form_id != "user_register") && is_array($values[$node_type])) {

to

if (!(arg(0) == 'user' && arg(1) == 'register' || $form_id == "user_register") && is_array($values[$node_type])) {

Comments

chaosprinz’s picture

yeah, subscribing for this problem. I am asking to get sure: Do i have to rename "cckfp.diff.txt" to "cckfp.diff.patch" and than apply it as an patch ?

ankur’s picture

Not necessarily. Since this patch only applies to one file (cck_field_privacy.module), you can patch the file at a UNIX/linux command-line with the following if you're already in the module's directory:

patch cck_field_privacy.module cckfp.diff.txt

OR, you can try

patch < cckfp.diff.txt

It's the contents of the patch file that matter, not really the extension.

-Ankur

obsidiandesign’s picture

Assigned: Unassigned » obsidiandesign
StatusFileSize
new8.83 KB

@ankur - I've rerolled this patch against the latest -dev. From what I could test, it appears everything still works as you explained; could you please test it against the latest -dev to make sure it does before I commit?

Thanks,
Bryan O'Shea