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.
Finding that autoassignroles is one of the few modules that strongarm won't let me at least wrangle some settings out of. Any thoughts on making this module features compliant by intergrating ctools exportables?
Thanks!
Comments
Comment #1
cyberswat CreditAttribution: cyberswat commentedI'm personally not a big of ctools and have no interest in writing code to integrate with it. I will happily review any patch that provides that integration though and won't block it from being accepted if it is solid code that is supported by tests.
Comment #2
patcon CreditAttribution: patcon commentedUnfortunately I'm not versed in the simpletest framework, but I'll try to find time to integrate ctools and it'd be great if someone could help me out with the tests. Any takers?
Comment #3
patcon CreditAttribution: patcon commentedComment #4
uniquename CreditAttribution: uniquename commentedIs there any progress here?
Comment #5
wizonesolutionsContacted patcon, and he wasn't able to do this. So I'm going to try implementing this with standard Features. Doesn't make sense to require Chaos Tools for this particular module, although if doing it with Features turns out to be a real pain I might cave in and add a soft dependency on Chaos Tools.
Comment #6
wizonesolutionsMan, next time I'm just using CTools :)
Here's a patch. It adds Features integration and support of default values for the various AAR configuration variables in both tables.
It has flaws, but it works. You're on your own for making sure the role IDs are the same across your sites and that if you use a custom node for the registration page that you fix that setting (unless the node IDs are the same as well). This could be solved by using Features's ability to manage user roles by name and by offering optional integration with UUID features. And I'd definitely use both those features but ran out of time to implement them myself.
It looks to be working pretty well, but I think it needs new tests (for the defaults functions) and testing.
Comment #7
wizonesolutionsI really have to stop posting patches so late. Here's one that actually adds autoassignrole.features.inc :)
Comment #8
wizonesolutionsThere's a minor typecasting issue here in
_autoassignrole_get_path_settings
when retrieving values from the DB, rid winds up being a string. When retrieving them from code, it winds up being an integer. This is because when retrieving from code, the $rid is discovered based on the array key index, andautoassignrole_settings_features_export_render
typecasts this to an int prior to_autoassignrole_get_path_settings
. This is because array key indices are type-sensitive.Anyway, I decided that the one true type for
$autoassignrole_settings['roles']['rid']
will be string, and here is a patch establishing that.Comment #9
Tarch CreditAttribution: Tarch commentedHi,
attached you find the patch rerolled against 6.x-1.2 version of the module in case someone needs this.
I get a couple of errors because the code assumes the existence of at least one autoassignrole_settings_defaults. Shouldn't exist at least one dummy function in the module that returns an empty array. Or maybe all the code should initialize the $settings_default cache to an empty array?
Following you find an example of where the problem happens:
in this line:
I get the error "warning: Invalid argument supplied for foreach() in modules/autoassignrole/autoassignrole.module on line 48." because $settings_default is not initialized when there are no implementations of the autoassignrole_settings_defaults hook.
Thanks!
Bye,
Tarch.
Comment #10
wizonesolutionsAh, thanks for catching that. I'll roll another patch if I get some time.
Patches should always be against the development version, at least if it's intended that they get committed.
Comment #11
kenorb CreditAttribution: kenorb commentedDrupal 6 is no longer officially supported. If you think this issue is still relevant for 8.x, feel free to re-open.