Og.rules.inc get group managers doesn't get a full list of managers.

I can confirm this happens. Trying patch above.

Patch above works almost fine but still has an issue. It doesn't list the first admin. I've tried it with an OG group with 3 admins. Only 2nd and 3rd got listed. Might be because the 1st one is also an website administrator ?

Ok I've managed to get the full list by changing the patch above like

  // Get the group admins for the group.
  $members = db_select('og_users_roles', 'our')
    ->fields('our', array('uid'))
    ->condition('our.gid', $entity_id)
    ->condition('our.group_type', $entity_type)
    ->condition('our.rid', $admin_rid)
    ->execute()
    ->fetchCol();
/** Add the group author **/
  $group_node = entity_load_single('node', $entity_id);
  $members[] = $group_node->uid;

  $admins = array_merge($admins, $members);

Might not be the best solution but works for me

I have been running into the same problem, and happened to see this old patch that definitely solves a problem with the rules integration where it doesn't get all the group managers. I was curious why this patch has not received as much attention that it has, and after looking at it, it seems that the patch is a little more aggressive than it needs to be since there are sites out there that already have implemented some of these rules.

I just rewrote this patch that, instead of rewriting the behavior of an existing rule, simply adds a new rule that gets the group managers by group. I then modify the existing rule to utilize the new method, which should also solve that rule as well so that it gets all the group managers. Let me know if this is more accessible to get committed in this module.

Not sure why my first git patch was formatted that way, but here is the reformatted version.

Hi,
Thanks for the patch.
While it is true that there are sites that uses this action and fixing it will mean the behavior will change, I think it is a bug, and as such it needs to be fixed. By adding another action with a similar name that works "the right way" but leaving one that does not, we will just promise ourselves future problems.

I think that if we try to better define "group managers", it actually means "members who has OG_ADMINISTRATOR_ROLE role", which from the code seems to be the direction you took.
Code wise, I am trying to think if there is a better way to do it. I rather use API calls instead of direct db_select() calls inside rules code, to make it a bit "future proof", but I didn't get into it yet.

Furthermore, providing tests for this would be great.

I am totally on board with the notion to break reverse compatibility for the sake of clean progression, but I do not think that these should be done within the same version numbers. I perceive that we could do what you said within a 3.x version of the module, but that still does not help the fact that the 2.x version is still 'broken'. I also should note that the current implementation isn't setup to retrieve group managers provided a group entity, but rather retrieve group owners provided group content. Even if you fix the group owners to group managers 'bug', you still have a problem where existing rules are configured to expect group content vs. the group entity which are two things entirely.

I think for this reason, it calls for an additional rule to get the group managers provided the group entity, as well as FIX the current group owners provided group content to be group managers provided group content.

That is what my patch accomplishes.

Hi

The patch works all right, great job, exactly what I needed, thanks.
But using OG_ADMINISTRATOR_ROLE to fetch group managers is limited to one single manager role and won't work if site admin renames the role to something other than 'administrator member' (which I did).
For more complex purposes we could make use of group manager default roles (admin/config/group/settings).
So in og_rules_get_managers_from_group() function I replaces following lines:

$roles = og_roles($entity_type, $group->getBundle(), $entity_id);
$admin_rid = array_search(OG_ADMINISTRATOR_ROLE, $roles);

with the line

$admin_rid = variable_get('og_group_manager_default_rids_' . $entity_type . '_' . $group->getBundle(), FALSE);

Now I have an array of all manager role ids in $admin_rid variable. And I don't even have to change select query since condition() accepts arrays as well as single values.
One might even consider replacing FALSE here with OG_ADMINISTRATOR_ROLE and have a valid fallback to default role when user didn't care to setup og_group_manager_default_rids* variables.

Patch in #9 is working for me. According to #10 and #11, I guess we should fix the bug in this version.

@shushu According to your comment in #10, there no such API function which returns managers given a GID. Do you think we should create one? An API function returning users given group id and role id makes sense.

I just ran into the same issue. Would be great to see it fixed!

I've adapted the #9 patch a little to replace the existing rule 'og_get_managers' so it takes 'group_content' as the parameter instead of the group itself.

I needed this as I had a Rule that triggered on deleting content with the condition 'Entity is group content', and the node's group wasn't readily available.

This patched fixed the bug where only 1 group admin is retrieved.

Hope it helps.