Merge pull request #516 from Gizra/pass-role-ids

Do not require to pass full OgRole objects when we only need the ID.
Gizra · Jun 10, 2019 · f58b7f1 · f58b7f1
2 parents 79b79db + 5e93e60
commit f58b7f1
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 19 deletions.
diff --git a/src/MembershipManager.php b/src/MembershipManager.php
@@ -127,11 +127,7 @@ public function getMembership(EntityInterface $group, AccountInterface $user, ar
   /**
    * {@inheritdoc}
    */
-  public function getUserGroupIdsByRoles(AccountInterface $user, array $roles, array $states = [OgMembershipInterface::STATE_ACTIVE], bool $require_all_roles = TRUE): array {
-    $role_ids = array_map(function (OgRoleInterface $role): string {
-      return $role->id();
-    }, $roles);
-
+  public function getUserGroupIdsByRoleIds(AccountInterface $user, array $role_ids, array $states = [OgMembershipInterface::STATE_ACTIVE], bool $require_all_roles = TRUE): array {
     /** @var \Drupal\og\OgMembershipInterface[] $memberships */
     $memberships = $this->getMemberships($user, $states);
     $memberships = array_filter($memberships, function (OgMembershipInterface $membership) use ($role_ids, $require_all_roles): bool {
@@ -149,8 +145,8 @@ public function getUserGroupIdsByRoles(AccountInterface $user, array $roles, arr
   /**
    * {@inheritdoc}
    */
-  public function getUserGroupsByRoles(AccountInterface $user, array $roles, array $states = [OgMembershipInterface::STATE_ACTIVE], bool $require_all_roles = TRUE): array {
-    $group_ids = $this->getUserGroupIdsByRoles($user, $roles, $states, $require_all_roles);
+  public function getUserGroupsByRoleIds(AccountInterface $user, array $role_ids, array $states = [OgMembershipInterface::STATE_ACTIVE], bool $require_all_roles = TRUE): array {
+    $group_ids = $this->getUserGroupIdsByRoleIds($user, $role_ids, $states, $require_all_roles);
     return $this->loadGroups($group_ids);
   }
 

diff --git a/src/MembershipManagerInterface.php b/src/MembershipManagerInterface.php
@@ -60,8 +60,8 @@ public function getUserGroups(AccountInterface $user, array $states = [OgMembers
    *
    * @param \Drupal\Core\Session\AccountInterface $user
    *   The user to get the groups for.
-   * @param \Drupal\og\OgRoleInterface[] $roles
-   *   A list of og role objects to filter by.
+   * @param string[] $role_ids
+   *   A list of OG role IDs to filter by.
    * @param string[] $states
    *   (optional) An array of states to filter the memberships by.
    * @param bool $require_all_roles
@@ -73,15 +73,15 @@ public function getUserGroups(AccountInterface $user, array $states = [OgMembers
    *   An associative array, keyed by group entity type, each item an array of
    *   group entities.
    */
-  public function getUserGroupsByRoles(AccountInterface $user, array $roles, array $states = [OgMembershipInterface::STATE_ACTIVE], bool $require_all_roles = TRUE): array;
+  public function getUserGroupsByRoleIds(AccountInterface $user, array $role_ids, array $states = [OgMembershipInterface::STATE_ACTIVE], bool $require_all_roles = TRUE): array;
 
   /**
    * Returns an array of groups ids filtered by the og roles of the user.
    *
    * @param \Drupal\Core\Session\AccountInterface $user
    *   The user to get the groups for.
-   * @param \Drupal\og\OgRoleInterface[] $roles
-   *   A list of og role objects to filter by.
+   * @param string[] $role_ids
+   *   A list of OG role IDs to filter by.
    * @param string[] $states
    *   (optional) An array of states to filter the memberships by.
    * @param bool $require_all_roles
@@ -93,7 +93,7 @@ public function getUserGroupsByRoles(AccountInterface $user, array $roles, array
    *   An associative array, keyed by group entity type, each item an array of
    *   group IDs.
    */
-  public function getUserGroupIdsByRoles(AccountInterface $user, array $roles, array $states = [OgMembershipInterface::STATE_ACTIVE], bool $require_all_roles = TRUE): array;
+  public function getUserGroupIdsByRoleIds(AccountInterface $user, array $role_ids, array $states = [OgMembershipInterface::STATE_ACTIVE], bool $require_all_roles = TRUE): array;
 
   /**
    * Returns the group memberships a user is associated with.

diff --git a/tests/src/Kernel/Entity/GetUserGroupsTest.php b/tests/src/Kernel/Entity/GetUserGroupsTest.php
@@ -241,7 +241,7 @@ public function testIsMemberStates() {
   /**
    * Tests retrieval of groups filtered by roles.
    *
-   * @covers ::getUserGroupIdsByRoles
+   * @covers ::getUserGroupIdsByRoleIds
    */
   public function testGetGroupsByRoles() {
     // Create a test role.
@@ -266,26 +266,26 @@ public function testGetGroupsByRoles() {
     // By default only active memberships are retrieved, so if we ask the
     // groups where the user is a normal member of the result should not include
     // group 2 where our test user is blocked.
-    $groups = $this->membershipManager->getUserGroupIdsByRoles($this->user3, [$member_role]);
+    $groups = $this->membershipManager->getUserGroupIdsByRoleIds($this->user3, [$member_role->id()]);
     $this->assertCount(1, $groups['entity_test']);
     $actual = reset($groups['entity_test']);
     $this->assertEquals($this->group1->id(), $actual);
 
     // When asking for the groups where our user has the test role, the result
     // should not include the blocked membership, so it should be empty.
-    $groups = $this->membershipManager->getUserGroupsByRoles($this->user3, [$extra_role_1]);
+    $groups = $this->membershipManager->getUserGroupsByRoleIds($this->user3, [$extra_role_1->id()]);
     $this->assertCount(0, $groups);
 
     // Include all states.
-    $groups = $this->membershipManager->getUserGroupIdsByRoles($this->user3, [$member_role], OgMembershipInterface::ALL_STATES, FALSE);
+    $groups = $this->membershipManager->getUserGroupIdsByRoleIds($this->user3, [$member_role->id()], OgMembershipInterface::ALL_STATES, FALSE);
     $this->assertCount(2, $groups['entity_test']);
 
     // Request any of multiple roles.
-    $groups = $this->membershipManager->getUserGroupsByRoles($this->user3, [$member_role, $extra_role_1], OgMembershipInterface::ALL_STATES, FALSE);
+    $groups = $this->membershipManager->getUserGroupsByRoleIds($this->user3, [$member_role->id(), $extra_role_1->id()], OgMembershipInterface::ALL_STATES, FALSE);
     $this->assertCount(2, $groups['entity_test']);
 
     // Request all of multiple roles.
-    $groups = $this->membershipManager->getUserGroupsByRoles($this->user3, [$member_role, $extra_role_1], OgMembershipInterface::ALL_STATES, TRUE);
+    $groups = $this->membershipManager->getUserGroupsByRoleIds($this->user3, [$member_role->id(), $extra_role_1->id()], OgMembershipInterface::ALL_STATES, TRUE);
     $this->assertCount(1, $groups['entity_test']);
     $actual = reset($groups['entity_test']);
     $this->assertEquals($this->group2->id(), $actual->id());