Hi,

I recently developed a module called Variable Email. In the process of setting up this module with Commerce Email, I used the following code in the hook_enable of a module:

mailsystem_set(array('variable_email_commerce_email_order' => 'HTMLMailSystem'));
mailsystem_set(array('variable_email_commerce_email_admin_order' => 'HTMLMailSystem'));

The expected result for me was that Mailsystem should have recognized that I wanted the commerce_email_order and the commerce_email_admin_order email keys from the variable_email module to use HTMLMailSystem.

The result however was that MailSystem recognized the second setting as "email_commerce_email_admin_order" as a key for the variable module.

What about using a new dimension in the array that is being used by mailsystem, instead of using a simple underscore as the differentiation ? We could have something like this:

$setting['module_name'] = array(
  'default' => '', // Nothing to use the site-wide MailSystem class
  'email_key' => 'HTMLMailSystem',
);

I believe it would provide a better differentiation between module and key and avoid issues such as the one I described above...

Comments

pillarsdotnet’s picture

pillarsdotnet commented
Status: Active » Closed (won't fix)

The variable_email module is fundamentally incompatible with the variable module and should be renamed.

guillaumev’s picture

guillaumev commented
Status: Closed (won't fix) » Active

Can you be more explicit ? Why is the variable_email module fundamentally incompatible with the variable module ?

pillarsdotnet’s picture

pillarsdotnet commented
Status: Closed (works as designed) » Closed (won't fix)

@#2

Why is the variable_email module fundamentally incompatible with the variable module ?

Because of the precise namespace problem you have reported.

  1. The structure of the mail_system variable is determined by Drupal core, not by this module.

  2. Drupal core will create exactly the same message id when module foo sends a message with key bar_baz as when module foo_bar sends a message with key baz.

  3. For this and many other reasons. naming a module foo_bar when there is (or may someday be) a module named foo is a bad idea. To repeat: including an underscore in your module name is a BAD IDEA.

  4. Even if I wanted to, there is no way that I could change this situation without hacking core.

  5. Therefore, you are asking for a Drupal core feature request not a Contrib module bug fix.

  6. In Drupal 8.x (or possibly 9.x) it is very likely that PHP 5.3 namespaces will solve this problem. Therefore, it is very likely that any fix will necessarily be different between 7.x and 8.x.

  7. Drupal 7.x has been declared "stable" and therefore will not be changed except to fix critical bugs. This is not a critical bug. It is a known design decision. The workaround is to be more careful about naming your module.

With all that said, the relevant code affecting your problem is as found in lines 24-33 and 55-50 of mailsystem.admin.inc:

  $descriptions = array();
  foreach (system_rebuild_module_data() as $item) {
    if ($item->status) {
      $descriptions[$item->name] = (
        empty($item->info['package'])
        ? '' : $item->info['package']
      ) . ' » ' . t('!module module', array('!module' => $item->info['name']));
    }
  }
  asort($descriptions);

...

    // Separate $id into $module and $key.                                          
    $module = $id;
    while ($module && empty($descriptions[$module])) {
      // Remove a key from the end                                                  
      $module = implode('_', explode('_', $module, -1));
    }

So the only reason it should have preferred variable to variable_email is if the Variable Email module was disabled or not installed.

pillarsdotnet’s picture

pillarsdotnet commented
Status: Active » Closed (works as designed)

Corrected status.

guillaumev’s picture

guillaumev commented

Thanks, I understand your points, but there are many modules which use that type of naming scheme (for example if you look at commerce, they have commerce_order, commerce_product etc...) however yes, they don't use it for emails...

I will see what I can do about that, but thanks for the explanations...

pillarsdotnet’s picture

pillarsdotnet commented

there are many modules which use that type of naming scheme

Most of them are closely-integrated sub-modules of the module whose name precedes the underscore. And even in those cases, they can run into problems if they're not careful.

Status: Closed (won't fix) » Closed (works as designed)

Correcting issue status, see #2125263: Issue status not taken from last comment during D7 migration.