Closed (outdated)
Project:
Auto Assign Role
Version:
6.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Reporter:
Created:
3 Nov 2010 at 04:47 UTC
Updated:
27 Jun 2018 at 13:08 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
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 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 commentedComment #4
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_settingswhen 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_rendertypecasts 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 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 commentedDrupal 6 is no longer officially supported. If you think this issue is still relevant for 8.x, feel free to re-open.