tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-10-22 14:05:37 +02:00
Code Issues Projects Releases Wiki Activity

FIX Sort without specifying a table name (#10675)

Browse Source 
Using a table name in sort() is not allowed in CMS 5. We could use
orderBy() here but member is the table it will sort on by default anyway
so there's no need.

Also added unit tests, which should have caught this ages ago.
This commit is contained in:
Guy Sartorelli 2023-02-01 13:52:13 +13:00 committed by GitHub
parent 2274b3e765
commit 826028082b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 128 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
src/ORM/DataList.php
View File

@ -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']);

10
src/Security/Member.php
View File

@ -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();
}

120
tests/php/Security/MemberTest.php
View File

@ -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' => [],
],
];
}
}

2
tests/php/Security/MemberTest.yml
View File

@ -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: