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!

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cyberswat’s picture

I'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.

patcon’s picture

Unfortunately 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?

patcon’s picture

Assigned: Unassigned » patcon
uniquename’s picture

Is there any progress here?

wizonesolutions’s picture

Version: 6.x-1.2 » 6.x-1.x-dev
Assigned: patcon » wizonesolutions

Contacted 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.

wizonesolutions’s picture

Status: Active » Needs review
FileSize
22.02 KB

Man, 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.

wizonesolutions’s picture

I really have to stop posting patches so late. Here's one that actually adds autoassignrole.features.inc :)

wizonesolutions’s picture

There'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, and autoassignrole_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.

Tarch’s picture

Hi,
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:

function autoassignrole_menu() {
  static $settings_default;
  if (!isset($settings_default)) {
    $settings_default = module_invoke_all('autoassignrole_settings_defaults');
  }
....
  // Account for any default path configurations not overridden in the DB
  foreach ($settings_default['roles'] as $rid => $path_settings) {
    $db_path_settings = _autoassignrole_get_path_settings($rid, TRUE);
    if (empty($db_path_settings)) {
      $items = array_merge($items, _autoassignrole_path_menu_generate((object) _autoassignrole_get_path_settings($rid)));
    }   
  }
  return $items;
}

in this line:

  foreach ($settings_default['roles'] as $rid => $path_settings) {

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.

wizonesolutions’s picture

Ah, 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.

kenorb’s picture

Issue summary: View changes
Status: Needs review » Closed (outdated)

Drupal 6 is no longer officially supported. If you think this issue is still relevant for 8.x, feel free to re-open.