user_access() within a group's home page (e.g. for hook_link()) doesn't work

This is exactly what the OG module does: calls user_access to determine which "create" links are displayed on the group menu for a user. Are you saying that og_user_roles is not displaying the correct group menu links, or just the link from your module?

By "create" links do you mean the 'Create <content type>' links within the Group details block? That's not what I'm talking about. When you have a teaser list of nodes, each node can have a list of links associated with it. For example, I've hacked modr8 to display 'Approve' or 'Delete' links alongside each node that's pending moderation (I also had to write a menu callback handler for this to work). Links can be added with hook_link().

Have you tried weighting OG User Roles lighter than this module?

How do you weight modules?

Same thing happens when sitewide caching disabled?

It will do, yes: if you look at the code for user_access(), it doesn't check to see if caching is enabled.

Have you looked at the og_user_test table to see what permissions are being returned by og_user_roles at this url?

I've var_dump()'d the $user object in user_access() when the appropriate call goes through. $user->roles has been correctly set to array('authenticated user', 'contributor', 'editor') at this point. However, this isn't looked at, as the caching that user_access() does (inside its local and static $perm variable) holds the permissions that were fetched the very first time that user_access() was called in this request, before og_user_roles had a change to add the extra items to $user->roles. These cached permissions are the ones that are used.

Do you have any suggestions for loading roles earlier than init()? Right now, it's done when (if) the $user object is loaded and in hook_init().

I would have thought that doing it when the $user object is loaded would be early enough, as this should be earlier than any user_access() calls. What do you mean be "(if)" here? Under what circumstances would $user not get loaded?

I would have thought that doing it when the $user object is loaded would be early enough, as this should be earlier than any user_access() calls. What do you mean be "(if)" here? Under what circumstances would $user not get loaded?

After digging a bit more, I found the problem was that the code executed in og_user_roles_user('load') wasn't grabbing the group ID successfully. Adding the following code in og_user_roles_getgid(), before the 'return $gid' line, fixed the issue for me:

  $node = 
    db_fetch_object(
      db_query('SELECT nid, type FROM {node} WHERE nid = ' . $nid));
  if (og_is_group_type($node->type)) {
    $gid = $node->nid;
  } elseif (is_array($groups = og_get_node_groups($node))) {
    foreach ($groups as $id => $group) {
      if ($group) $gid = $id;
    }
  }

I'm wondering whether this code will be enough to grab the gid every time, or whether we still need the two other queries in that function.

I've attached a patch which contains my fix.

If the node belongs to more than one group, and the group in question isn't the one returned, then we have the same problem.

This is a valid concern. I guess, instead of looking for the group ID, we need to find all group IDs which apply to this page.

Also: Suppose that this node belongs to two groups, and the user belongs to both groups, but has different permissions in each group? How do we determine which gid to bring back?

I think the right thing to do is to give the user all roles from all groups which apply to this node. My reasoning is that if you're looking at a node in a given group, you expect to have the roles for that group. This applies to all groups, so you would expect to have the roles for all groups. To do this og_user_roles_getgid() and og_user_roles_all_roles() need to be refactored to deal with more than one gid, and to fetch the roles for all of these gids.

Would you see if this code works for you?:

It works fine. However, I'm not sure it's the right approach, I think my above comments would be a better solution.

Thanks for your attention on this issue.

OK. It will indeed be a large undertaking. Perhaps you could apply that patch, close this issue, then begin work on multiple gid support. In the mean time consider the issue fixed for me, as in my case a group will never be a member of more than one group.

Err, sorry, I meant a node will never be a member of more than one group!