From 5e93e6040fdfa589ca8167f0466dc31e08cfb6e7 Mon Sep 17 00:00:00 2001 From: Pieter Frenssen Date: Mon, 10 Jun 2019 14:10:57 +0300 Subject: [PATCH] Do not require to pass full OgRole objects when we only need the ID. --- src/MembershipManager.php | 10 +++------- src/MembershipManagerInterface.php | 12 ++++++------ tests/src/Kernel/Entity/GetUserGroupsTest.php | 12 ++++++------ 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/src/MembershipManager.php b/src/MembershipManager.php index 3097fd881..f1ab2498b 100644 --- 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 index 96bc42aed..2366ae083 100644 --- 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 index 36cc94ddc..1b13450e6 100644 --- 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());