Add/Remove group role for a user programmatically

@Ginovski I've added checks:
1) If a user has the role
2) if a role exists for the current group type

I will add tests later

This looks great. Could you add a test perhaps? You have some safety checks in there and it would be nice for a test to confirm they work as expected. It can be as simple as a unit test with a few prophecy mocks.

Also:

'group_type' => $this->groupContent->getGroup()->getGroupType()->id(),

This can save a few ms by not having to load the group type:

$this->groupContent->getGroup()->bundle(),

Thanks for working on this!

Just a somewhat related comment:

I guess most people want to run this code on a User entity? When i needed to make a view to be able to set/remove a role from members i created a Group content (membership) view; and then wrote a new VBO action to add as simply:

  $entity->group_roles->target_id = 'company-coordinator';
  $entity->save();

The entity in this case is "Group content" but not membership; even though i am filtering my group content on "group membership" content type. So even though i can see group_roles in my $entity values (and apparently set them); i never load the membership. There is also no need to check if already set. It is my only (real) role at the moment; so not sure how this works if i create a 2nd role. Maybe it doesn't and i could use these new methods in what would now require a somewhat convoluted approach:

  $member = User::load($entity->entity_id->target_id);
  $group = $entity->getGroup();
  $membership = $group->getMember($member);
  $membership->addRole('company-coordinator');
  $membership->save();

Do I have this all correct? It does seem odd that i can set role by either modifying the group content entity or the membership entity.

Without your new remove method; not sure how to go about this as i am doing it now (against group content entities); possibly just set target_id = ''.

Addressed #7:

- Used $this->groupContent->getGroup()->bundle() to save a few ms by not having to load the group type
- Added some tests, couldn't think of a way to use a unit test, since I couldn't see any way to get this to work without using getRoles(), so I used a KernelTest.
- Updated some comments.

Coding Standard messages and 9.2 failure (looks like expectExceptionMessageRegExp, which was already deprecated, has been removed from PHPUnit 9.5.8) are unrelated to this patch AFAICT.

I agree, Unit tests are suitable in this case. I have tried to do it also, it didn't work for me.
I checked the tests everything looks ok. I have added some minor changes.
Just to improve code a bit.

I believe we can mark it as RTBTC

It would be awesome to also have a hasRole() method checking for a certain role. What do you think?

Regarding the hasRole method I also needed something like that.
I've opened a child issue and will post the complete patch here.

Sorry, I'm not able to make an interdiff right now. I don't have patchutils installed and have an issue with my package manager.

Patch 11 + hasRole method in attachment.

Well, maybe a patch not made in the rush would be better. Finally, got patchutils installed also, that helps.

Sorry for previous rushy patch.

Change back to "Needs work" because of the failing tests.

I think the roles added into the array in a reverse order.

With the current solution every ::addRole() and ::removeRole() triggers an individual ::groupContent->save().

I think the following methods would be useful.

class GroupMembership implements CacheableDependencyInterface {

  /**
   * Add or remove certain roles.
   *
   * @param iterable<string, bool> $role_ids
   *   Key: Role ID.
   *   Value: Indicates that if the role has to be added(TRUE) or removed(FALSE).
   *
   * @return $this
   */
  public function updateRoles(iterable $role_ids) {
    // @todo
    return $this;
  }

  /**
   * Add all the roles listed in $role_ids, remove those which aren't.
   * 
   * @param iterable<string> $role_ids
   *
   * @return $this
   */
  public function setRoles(iterable $role_ids) {
    // @todo
    return $this;
  }

Will try to add this to next minor release.

The patches are not yet relevant to Group 3.x.

I found that I had to do the following to programmatically add a group role for a user with an existing membership:

      // Get the user's membership in the group.
      $membership = $group->getMember($account);

      // If the user is already a member of the group.
      if ($membership) {
        /** @var \Drupal\group\GroupMembership $membership */

        $group_roles = $membership->getRoles(FALSE);

        $has_member_role = FALSE;

        foreach ($group_roles as $group_role) {
          if ($group_role->id() == $member_role) {
            $has_member_role = TRUE;
            break;
          }
        }

        // If the user doesn't already have the member role in this group.
        if (!$has_member_role) {
          // Add the member role for this user.
          $group_relationship = $membership->getGroupRelationship();
          $group_relationship->group_roles[] = $member_role;
          $group_relationship->save();
        }
      }

Re-rolling the patch to 2.2.x-dev, using the proper groupRelationship instead of groupContent and omitting the changes to ChainGroupPermissionCalculatorTest.php.

This needs test coverage for the new methods and I'm not sure if we can simply get away with calling the shared bundle class from the old wrapper like that. For new methods it would make sense, as there is nothing to keep BC for.

Hiding patches in favor of MR

Okay, this looks ready to me now. Will add a follow-up for moving the role checking logic in a preSave() hook.

Change record here: https://www.drupal.org/node/3425776
Follow-up here: #3425781: Unify preSave and postSave logic for memberships.