Using the Me Aliases module with uc_addresses causes addresses to not allow users to edit their address.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Ron Williams’s picture

I was able to workaround this issue by replacing me aliases with Menu Token. This may not properly work for all users as external links with the /me (or configured path) alias will not work and Menu Token does not provide the same feature.

In my case, all I used me aliases for was to send users to various sections of their user profile from a menu, so replacement only required recreating the menu items. Note: it may be faster to install menu token and change the existing menu paths rather than recreating the menu items.

MegaChriz’s picture

Status: Active » Closed (won't fix)

Hi,

I looked at it and it appears to be the error is caused by the Me-module, Drupal core, or even PHP (tested with PHP 5.3). The error happens because the menu system does a call_user_func_array() which will pass all variables by value. Because the me-module wants a reference instead of a value, PHP 5.3 complains about passing variables by value and so refuses to call me_uid_optional_load().
This was the error I got with error reporting on:
warning: Parameter 2 to me_uid_optional_load() expected to be a reference, value given in /Websites/drupal/drupalsites/drupal6/includes/menu.inc on line 408.

Possible fixes:
- Remove the ampersand before $map at line 419 in me.module.
- menu.inc line 408: place an ampersand before $args, so line 408 will be:
$return = call_user_func_array($function, &$args);

The me-module must have a special reason for those ampersands, I don't know.
Anyway, the root of this problem does not lie by Ubercart Addresses, so I marking this issue as "won't fix".

cheplv’s picture

Please attached patch of uc_addresses for correct support of "me" aliases and probably for other modules.

MegaChriz’s picture

Thanks for the patch. A few questions and notes:
- The patch doesn't fully follow the Drupal coding standards: for example, it adds extra whitespace. Please run your changes through a code review using the coder module.
- Can't this problem be solved without using calls to the me module? I see a lot of module_exists('me') in your patch.
- Where are the parameters $map and $map_index used for? Please describe them in the function comments using @param.
- Have you tested if the code still works when the me-module is disabled?

cheplv’s picture

Status: Closed (won't fix) » Patch (to be ported)
FileSize
4.35 KB

1) ok - coder filtered.
2) yes - this problem can be solved without this me rewriting, but this is nice feature for this addresses module
3) Described in @params, more info in "hook_menu"
4) Yes - works w/o "me" aliases module

MegaChriz’s picture

Status: Patch (to be ported) » Needs work

If there is a patch to review, then you could better set the status to "needs review". The status "patch (to be ported)" is used when a patch needs to be ported from one major version to another. See also http://drupal.org/node/156119 for descriptions of what each issue status means.

1. You have fixed some coding standards issue, but not all yet. There is still some extra whitespace in your patch. If you execute a code review with the "minor (most)" warning level, you'll get the most coding standards issues.

This may not be picked up by the coding review, but you could better also use the longer syntax for a control structure.

Instead of:

if (module_exists('me')) $uid = _me_get_me_alias();

type:

if (module_exists('me')) {
  $uid = _me_get_me_alias();
}

This increases the readability of the code.

2. I appreciate your efforts and time in fixing this issue. But I still feel the patch tries to fix a problem that seem to be caused by the me module. I'd rather see it fixed in that module. If the incompatibility can be solved by a simple fix in the uc_addresses module, without any calls to the me module and without adding too much complexity to the module code, I will consider to add a fix to the uc_addresses module. To be clear: I do not mean the me module should add a module_exists('uc_addresses') there, an ideal fix would be a fix where the modules don't (need to) know each other, like is the case with a module_exists() call.

3. OK, although I had to read it a few times to get an idea on what it meant.

4. Thanks for testing.

I'm setting the status to "needs work" because of the coding standard issues, but also for my concerns noted about the module_exists() call.

Thanks for working on this so far.

MegaChriz’s picture

Issue summary: View changes

fixed bad html