diff --git a/src/ORM/DataList.php b/src/ORM/DataList.php index 158039445..e0dd048ab 100644 --- a/src/ORM/DataList.php +++ b/src/ORM/DataList.php @@ -323,6 +323,7 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li * * @param string|array $args * @example $list = $list->sort('Name'); // default ASC sorting + * @example $list = $list->sort('"Name"'); // field names can have double quotes around them * @example $list = $list->sort('Name ASC, Age DESC'); * @example $list = $list->sort('Name', 'ASC'); * @example $list = $list->sort(['Name' => 'ASC', 'Age' => 'DESC']); diff --git a/src/Security/Member.php b/src/Security/Member.php index 1e3262943..1e294f1fd 100644 --- a/src/Security/Member.php +++ b/src/Security/Member.php @@ -1188,15 +1188,15 @@ class Member extends DataObject * * 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 @@ class Member extends DataObject 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 @@ class Member extends DataObject ]); } - 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 18aa85174..40748ddc7 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\i18n\i18n; 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,121 @@ class MemberTest extends FunctionalTest $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 provideMapInCMSGroups + */ + public function testMapInCMSGroups(array $groupFixtures, array $groupCodes, array $expectedUsers) + { + 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]); + } + + // Convert user fixtures to IDs + foreach ($expectedUsers as $index => $userFixtureName) { + // This is not an actual fixture - it was created by $this->logInWithPermission('ADMIN') + if ($userFixtureName === 'ADMIN User') { + $expectedUsers[$index] = Member::get()->filter(['Email' => 'ADMIN@example.org'])->first()->ID; + } else { + $expectedUsers[$index] = $this->objFromFixture(Member::class, $userFixtureName)->ID; + } + } + + $result = Member::mapInCMSGroups($groups); + $this->assertInstanceOf(Map::class, $result); + + $this->assertSame($expectedUsers, $result->keys()); + } + + public function provideMapInCMSGroups() + { + // Note: "ADMIN User" is not from the fixtures, that user is created by $this->logInWithPermission('ADMIN') + return [ + 'defaults' => [ + 'groupFixtures' => [], + 'groupCodes' => [], + 'expectedUsers' => [ + 'admin', + 'other-admin', + 'ADMIN User', + ], + ], + 'single group in a list' => [ + 'groupFixtures' => [], + 'groupCodes' => [ + 'staffgroup' + ], + 'expectedUsers' => [ + 'staffmember', + ], + ], + 'single group in IDs array' => [ + 'groups' => [ + 'staffgroup', + ], + 'groupCodes' => [], + 'expectedUsers' => [ + 'staffmember', + ], + ], + 'multiple groups in a list' => [ + 'groupFixtures' => [], + 'groupCodes' => [ + 'staffgroup', + 'securityadminsgroup', + ], + 'expectedUsers' => [ + 'staffmember', + 'test', + ], + ], + 'multiple groups in IDs array' => [ + 'groupFixtures' => [ + 'staffgroup', + 'securityadminsgroup', + ], + 'groupCodes' => [], + 'expectedUsers' => [ + 'staffmember', + 'test', + ], + ], + 'group with no members' => [ + 'groupFixtures' => [], + 'groupCodes' => [ + 'memberless', + ], + 'expectedUsers' => [], + ], + ]; + } } diff --git a/tests/php/Security/MemberTest.yml b/tests/php/Security/MemberTest.yml index 0f9ed3c91..f67254ee0 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: