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.
When I create a userreference field with a long machine-name, the variable name ends up being very long. Since the variable names can only be 48 characters long, the defaults don't get loaded and I get an SQL duplicate entry error.
Attached is a patch that cuts the variable names down to 48 characters. I can already see a potential issue with this since now it's possible for two different fields to have the same variable. Maybe it's more an issue with the Drupal variable system that variable names are limited to 48 characters. That's outside the scope of this patch, however.
Comments
Comment #1
Shawn DeArmond CreditAttribution: Shawn DeArmond commentedI had to make one more change.
Comment #2
danielb CreditAttribution: danielb commentedIf it just gonna cause more problems I wouldn't bother using the truncating method
I reckon a better way might be to create a single variable for this module just called "nodeaccess_userreference" which is an array where the keys are the field names.
Comment #3
Shawn DeArmond CreditAttribution: Shawn DeArmond commentedGood idea. Here's a patch where it's all stored in a single variable. The default values are also stored in that variable. The variable has the potential to grow quite large, but it works.
Comment #4
Shawn DeArmond CreditAttribution: Shawn DeArmond commentedNeeds review.
Just thought of something else. In order for this to be implemented, the module will need an update function or else the users will lose all their settings.
Comment #5
danielb CreditAttribution: danielb commentedHmm good point.
I quickly wrote this function (haven't tried it) we could use something like this instead of variable_get and variable_set, it will check for the old data
To get the settings:
To update the settings:
Comment #6
Shawn DeArmond CreditAttribution: Shawn DeArmond commentedI like the idea of using a separate function to wrap the variable_get and variable_set. It would be a little more intuitive, though, if it was two separate functions. something like:
nodeaccess_userreference_set($type_name, $field_name, $settings = NULL)
and
nodeaccess_userreference_get($type_name, $field_name, $default = NULL)
Also, I'm not sure that incorporating "backwards-compatible" code into the function itself is the best direction. In order to support an older version of the module, wouldn't it be best to just wholesale convert the settings using an update function?
Comment #7
danielb CreditAttribution: danielb commentedI think it's fine
I mean I thought about those when I was doing it, and whilst this isn't perfect, it works and won't be a problem. In terms of being intuitive - it's not an api function for people to use, it does the job of the update without requiring a second function - that's cool. The secondary check shouldn't be too much of a performance hit because variable_get is pretty quick and that part of the code should only ever run once on a particular field anyway, because from then on the default or stored value will be in the new settings - eventually that secondary code can be removed. Doing a mass update would be a little harder too :/
Comment #8
Shawn DeArmond CreditAttribution: Shawn DeArmond commentedFair point.
Okay, I rolled a patch based on your code, though I did have to change a couple things. I tested it as if I was moving from 2.0 to 2.x-dev and it migrated perfectly.
In fact, an odd (but happy) side-effect of this patch is that it'll upgrade itself the first time node_access_rebuild() is called, since it goes through all the types and fields.
Comment #9
danielb CreditAttribution: danielb commentedthanks!