The latest version of NAUR gives me strange behaviour:
- when saving the field settings of a 'user reference' field, the Drupal variable_set() is not invoked, so the variable is not changed.
- when showing the field settings again, the saved values are shown again, so it seems they are saved somewhere, but not via variable_get()
I have not checked yet if permissions are broken because of this, but IIRC the variable_set/get() is the way to store the data, as does NANR.
I have compared both modules (which are now very nicely aligned, Thank you!), and the following lines seem suspect to me:
function nodeaccess_XXXXreference_field_update_instance($instance, $prior_instance) {
. . .
if ($old_settings != $new_settings) {
// the following line should exist in NAUR
nodeaccess_nodereference_field_settings($instance['bundle'], $instance['field_name'], $new_settings);
// the following line is correctly missing NAUR
nodeaccess_nodereference_check_cross_access();
. . .
one minor difference:
function nodeaccess_nodereference_field_settings($bundle_name, $field_name = NULL, $variable = NULL) {
$data = variable_get('nodeaccess_nodereference', array());
vs.
function nodeaccess_userreference_field_settings($bundle_name, $field_name = NULL, $variable = NULL) {
$data = variable_get('nodeaccess_userreference', NULL);
This findings still do not explain how the field settings screen recall the previous settings.
Comments
Comment #1
johnvHmm,
In below function, $data holds the field data, and $current holds the variable data.
Then, $current is not used (anymore?) to fill the field settings, but only to set the collapsed state.
- Is the field data then stored redundantly? Isn't this error prone?
- Was the whole task of minifying the variable superfluous, and can we better rely on function field_info_instance($entity_type, $field_name, $bundle_name) ?
Comment #2
danielb CreditAttribution: danielb commentedThere was a line missing which saved the settings, woops, I've committed a fix.
Yes the settings are redundant, it is easier to iterate, and I assume speeds up hook_node_access(). I'd reconsider, but I'd prefer to just get it working now and leave it until a rewrite or something.
Comment #3
johnvThanks.
Now that it is working now, its fine. Redundancy is not a bad thing per se.
Perhaps some optimalisation for the D8-version ;-)