Issue with hook_domain_conf() invoke [#1708224]

Hi,

There is a bug in hook invoke or I think it is. If it is not I think this should be included as a feature in next release.
The problem is following:

When you call the function $extra = domain_conf_api(TRUE, $settings); you do not pass the current loaded domain ( current arg(4) ). In this function you are invoking the hook_domain_conf() with parameter current domain that is actually a global variable so it can be accessed everywhere and I personally cannot see why we need this. So if we pass the loaded by menu system domain parameter to domain_conf_api() and after that we pass this variable to module_invoke it makes much more sense. I've made a patch for this.

Regards.

Comment	File	Size	Author
	Domain conf patch for hook_domain_conf()	1.13 KB	skek

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

agentrickard

he/him

English

Georgia (US)

CreditAttribution: agentrickard commented 3 August 2012 at 01:52

Status:

Active

» Needs work

Actually, I think passing any domain value at all is a bug. This is designed to be a form registration function, and the domain value should not be relevant. The documentation for hook_domain_conf() does not include any parameters.

Note: please set patches to "needs review" when posting them.

Comment #2

skek CreditAttribution: skek commented 3 August 2012 at 16:06

Hi @agentrickard,

Basically I think it is a good to have the domain array there because if your hook need to get some special settings about the domain you will have the loaded domain array and it is not necessary to load it from arg() function. Because in previous versions of the module the domain arg was three or something and after the update you should go and edit every custom module that use the url arg.

p.s. Sorry not tagging the patch.

Comment #3

agentrickard

he/him

English

Georgia (US)

CreditAttribution: agentrickard commented 6 August 2012 at 18:10

Category:

bug

» feature

Well, then that would be an API change request. I don't think, however, that you should be reacting to the $domain value in this hook. It's just a form builder.

Comment #4

skek CreditAttribution: skek commented 15 July 2013 at 12:32

@agentrickard,

I saw in the last stable version of the module the following code:

  global $_domain;
  $options = array();
  $extra = module_invoke_all('domain_conf', $_domain);
  ....

which doesn't make sense to me. Why we need to pass the global variable as a parameter?
The idea behind the patch I've provide is to have the current domain you are editing from /admin/structure/domain to be available into hook_domain_conf() instead of using arg() to load it again.
For example if your module is fully dependable on domain access module and have a custom form to storing the relation between domain_id and your data you need to load the domain from URL instead of having it into the hook.