diff --git a/src/Security/Member.php b/src/Security/Member.php index 1e3262943c9..1e294f1fd2d 100644 --- a/src/Security/Member.php +++ b/src/Security/Member.php @@ -1188,15 +1188,15 @@ public static function map_in_groups($groups = null) * * If no groups are passed, all groups with CMS permissions will be used. * - * @param array $groups Groups to consider or NULL to use all groups with + * @param SS_List|array|null $groups Groups to consider or NULL to use all groups with * CMS permissions. * @return Map Returns a map of all members in the groups given that * have CMS permissions. */ - public static function mapInCMSGroups($groups = null) + public static function mapInCMSGroups(SS_List|array|null $groups = null): Map { // non-countable $groups will issue a warning when using count() in PHP 7.2+ - if (!$groups) { + if ($groups === null) { $groups = []; } @@ -1205,7 +1205,7 @@ public static function mapInCMSGroups($groups = null) return ArrayList::create()->map(); } - if (count($groups ?? []) == 0) { + if (count($groups) === 0) { $perms = ['ADMIN', 'CMS_ACCESS_AssetAdmin']; if (class_exists(CMSMain::class)) { @@ -1246,7 +1246,7 @@ public static function mapInCMSGroups($groups = null) ]); } - return $members->sort('"Member"."Surname", "Member"."FirstName"')->map(); + return $members->sort('"Surname", "FirstName"')->map(); } diff --git a/tests/php/Security/MemberTest.php b/tests/php/Security/MemberTest.php index 18aa8517446..47a9eb08062 100644 --- a/tests/php/Security/MemberTest.php +++ b/tests/php/Security/MemberTest.php @@ -2,6 +2,8 @@ namespace SilverStripe\Security\Tests; +use InvalidArgumentException; +use SilverStripe\Admin\LeftAndMain; use SilverStripe\Control\Cookie; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Convert; @@ -12,6 +14,7 @@ use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DB; use SilverStripe\ORM\FieldType\DBDatetime; +use SilverStripe\ORM\Map; use SilverStripe\ORM\ValidationException; use SilverStripe\ORM\ValidationResult; use SilverStripe\Security\Group; @@ -1618,4 +1621,111 @@ public function testChangePasswordToBlankIsValidated() $result = $member->changePassword(''); $this->assertFalse($result->isValid()); } + + public function testMapInCMSGroupsNoLeftAndMain() + { + if (class_exists(LeftAndMain::class)) { + $this->markTestSkipped('LeftAndMain must not exist for this test.'); + } + $result = Member::mapInCMSGroups(); + $this->assertInstanceOf(Map::class, $result); + + $this->assertEmpty($result, 'Without LeftAndMain, no groups are CMS groups.'); + } + + /** + * @dataProvider mapInCMSGroupsProvider + */ + public function testMapInCMSGroups(array $groupFixtures, array $groupCodes, array $expectedValue) + { + if (!empty($groupFixtures) && !empty($groupCodes)) { + throw new InvalidArgumentException('Data provider is misconfigured for this test.'); + } + + if (!class_exists(LeftAndMain::class)) { + $this->markTestSkipped('LeftAndMain must exist for this test.'); + } + + $groups = []; + + // Convert fixture names to IDs + if (!empty($groupFixtures)) { + foreach ($groupFixtures as $index => $groupFixtureName) { + $groups[$index] = $this->objFromFixture(Group::class, $groupFixtureName)->ID; + } + } + + // Convert codes to DataList + if (!empty($groupCodes)) { + $groups = Group::get()->filter(['Code' => $groupCodes]); + } + + $result = Member::mapInCMSGroups($groups); + $this->assertInstanceOf(Map::class, $result); + + $this->assertSame($expectedValue, $result->values()); + } + + public function mapInCMSGroupsProvider() + { + // Note: "ADMIN User" is not from the fixtures, that user is created by $this->logInWithPermission('ADMIN') + return [ + 'defaults' => [ + 'groupFixtures' => [], + 'groupCodes' => [], + 'expectedValue' => [ + 'Admin ', + 'OtherAdmin ', + 'ADMIN User', + ], + ], + 'single group in a list' => [ + 'groupFixtures' => [], + 'groupCodes' => [ + 'staffgroup' + ], + 'expectedValue' => [ + 'Staff User', + ], + ], + 'single group in IDs array' => [ + 'groups' => [ + 'staffgroup', + ], + 'groupCodes' => [], + 'expectedValue' => [ + 'Staff User', + ], + ], + 'multiple groups in a list' => [ + 'groupFixtures' => [], + 'groupCodes' => [ + 'staffgroup', + 'securityadminsgroup', + ], + 'expectedValue' => [ + 'Staff User', + 'Test User', + ], + ], + 'multiple groups in IDs array' => [ + 'groupFixtures' => [ + 'staffgroup', + 'securityadminsgroup', + ], + 'groupCodes' => [], + 'expectedValue' => [ + 'Staff User', + 'Test User', + ], + ], + 'group with no members' => [ + 'groupFixtures' => [], + 'groupCodes' => [ + 'memberless', + ], + 'expectedValue' => [], + ], + ]; + } } diff --git a/tests/php/Security/MemberTest.yml b/tests/php/Security/MemberTest.yml index 0f9ed3c9147..f67254ee038 100644 --- a/tests/php/Security/MemberTest.yml +++ b/tests/php/Security/MemberTest.yml @@ -58,6 +58,8 @@ Email: noexpiry@silverstripe.com Password: 1nitialPassword staffmember: + FirstName: Staff + Surname: User Email: staffmember@test.com Groups: '=>SilverStripe\Security\Group.staffgroup' managementmember: