Problem/Motivation

Webform Group module calls $group->getMembers(['member']);, where the role 'member' is an INSIDER_ID scoped role synced to the global role of AUTHENTICATED_ID. I assume this is similar to the previous default all members default role in Group v1. Webform group supports tokens where it loads all members of a group and emails them. However, calls to ::getMembers() returns empty list, because there doesn't seem to be any accounting for this type of user loading by role ID in group v3 for synchronized roles.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Comments

heddn created an issue. See original summary.

heddn’s picture

Issue summary: View changes
heddn’s picture

The work-around for now, is to check
if ($role->getScope() === PermissionScopeInterface::INSIDER_ID && $role->getGlobalRole()?->id() === RoleInterface::AUTHENTICATED_ID), then not add a condition for the member role to the query.

rondog469’s picture

I think I am running into this issue on 2.x. I have a role for an insider synced with authenticated. They have the permission "View individual group members" checked but when I go to the page to view members (as an insider synced with authenticated), it only lists the current user. I mentioned this in the Group slack channel a couple weeks ago.

kristiaanvandeneynde’s picture

This is the relevant bit from GroupMembershipLoader::loadByGroup()


  /**
   * {@inheritdoc}
   */
  public function loadByGroup(GroupInterface $group, $roles = NULL) {
    $query = $this->groupRelationshipStorage()
      ->getQuery()
      ->accessCheck(FALSE)
      ->condition('gid', $group->id())
      ->condition('plugin_id', 'group_membership');

    if (isset($roles)) {
      $query->condition('group_roles', (array) $roles, 'IN');
    }

    $ids = $query->execute();
    // ... more code here
  }

As you can see it simply filters by the passed in roles. Passing in ['member'] shouldn't work, though, as all roles are prefixed with their group type ID when created through the UI. It could work if Webform created said group role via code, but that would be ill advised as a machine name of "member" is very ambiguous.

So I'm not sure exactly what is going on here. Is there indeed a group role with the machine name "member"? If not, it makes perfect sense that there is no membership returned.

heddn’s picture

I'm speaking about the group ID prefixed role. Which in v1 I think was called {group_type}-member. To get existing tests to pass, I've had to add the following to calling code:

$role = GroupRole::load($role_id);
if ($role->getScope() === PermissionScopeInterface::INSIDER_ID && $role->getGlobalRole()?->id() === RoleInterface::AUTHENTICATED_ID) {
  $role_id = NULL;
}
$members = $group->getMembers($role_id);

This gives me all the members. But ideally, that logic would be added to ::getMembers, or even part of ::loadByGroup, as it is more part of the API and shouldn't rely on calling code to check the group type and null out the role ID if it is a synced role for "all authenticated users".

kristiaanvandeneynde’s picture

Ah yes, I see what you're getting at now. We could indeed check that when roles are passed in, they are either of the scope individual or insider. We should not check for authenticated, though, as it was (and is) perfectly valid to filter on an individually assignable group role.

Why don't you/they just omit the role filter, though? That should also give you all members.

heddn’s picture

In the UI, you get to pick any of the roles to email on form submission (or none at all). There was explicit test coverage (rightfully so) for member, a custom role and outsider roles. The role ID being passed in it just that random role ID selected by the webform editor. It is perfectly valid for the editor to select admins only. Or to email all members. In the use case we use this module, we don't use this feature at all. But I wanted to keep feature parity for those folks who likely to do use that feature. In group v3, the creation of "member" role is more of an option then it was in Group v1. From what I can tell, group v1, it was created automatically. While in v3, you had the option not to create it. And later create it manually with any random name you decide you want to use.

kristiaanvandeneynde’s picture

Okay I see.

Well, if you add an option "All members" to your form and when that is chosen you simply do not pass a filter, you'll be able to e-mail all members. The only exception is that if you want the option to mail all "regular" members, but not the admins, this won't work (as it will return all members, even admins).

Alternatively, the reason we removed the member role in the first place was because some people didn't use it. You could argue that if people want to make a distinction between "regular" members and "special" members, they should add the member role back themselves (either manually or automatically upon group type creation) and then your form will still work like it did before.

kristiaanvandeneynde’s picture

Category: Bug report » Support request
Status: Active » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.