Skip to content

Commit 120baae

Browse files
committed
FIX 11577 Performance issues with large list of groups
1 parent 227e178 commit 120baae

File tree

2 files changed

+63
-29
lines changed

2 files changed

+63
-29
lines changed

src/Security/Group.php

Lines changed: 42 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,16 @@
2323
use SilverStripe\Forms\ListboxField;
2424
use SilverStripe\Forms\LiteralField;
2525
use SilverStripe\Forms\RequiredFields;
26+
use SilverStripe\Forms\SearchableDropdownField;
2627
use SilverStripe\Forms\Tab;
2728
use SilverStripe\Forms\TabSet;
2829
use SilverStripe\Forms\TextareaField;
2930
use SilverStripe\Forms\TextField;
31+
use SilverStripe\Forms\TreeDropdownField;
3032
use SilverStripe\ORM\ArrayList;
3133
use SilverStripe\ORM\DataObject;
3234
use SilverStripe\ORM\DataQuery;
35+
use SilverStripe\ORM\FieldType\DBForeignKey;
3336
use SilverStripe\ORM\HasManyList;
3437
use SilverStripe\ORM\Hierarchy\Hierarchy;
3538
use SilverStripe\ORM\ManyManyList;
@@ -83,6 +86,11 @@ class Group extends DataObject
8386
Hierarchy::class,
8487
];
8588

89+
private static $searchable_fields = [
90+
'Title',
91+
'Description',
92+
];
93+
8694
private static $table_name = "Group";
8795

8896
private static $indexes = [
@@ -122,18 +130,28 @@ private function getDecodedBreadcrumbs()
122130
*/
123131
public function getCMSFields()
124132
{
133+
$threshold = DBForeignKey::config()->get('dropdown_field_threshold');
134+
$overThreshold = $groups->count() > $threshold;
135+
125136
$fields = new FieldList(
126137
new TabSet(
127138
"Root",
128139
new Tab(
129140
'Members',
130141
_t(__CLASS__ . '.MEMBERS', 'Members'),
131142
new TextField("Title", $this->fieldLabel('Title')),
132-
$parentidfield = DropdownField::create(
143+
$parentidfield = SearchableDropdownField::create(
133144
'ParentID',
134145
$this->fieldLabel('Parent'),
135-
$this->getDecodedBreadcrumbs()
136-
)->setEmptyString(' '),
146+
Group::get(),
147+
null,
148+
'BreadcrumbTitle'
149+
)
150+
->setIsSearchable(true)
151+
->setUseSearchContext(true)
152+
->setPlaceholder(' ')
153+
->setIsLazyLoaded($overThreshold)
154+
->setLazyLoadLimit($threshold),
137155
new TextareaField('Description', $this->fieldLabel('Description'))
138156
),
139157
$permissionsTab = new Tab(
@@ -460,6 +478,14 @@ public function stageChildren()
460478
->sort('"Sort"');
461479
}
462480

481+
/**
482+
* @return string
483+
*/
484+
public function getBreadcrumbTitle(): string
485+
{
486+
return $this->getBreadcrumbs(' > ');
487+
}
488+
463489
/**
464490
* @return string
465491
*/
@@ -503,12 +529,12 @@ public function validate()
503529
}
504530
}
505531

506-
$currentGroups = Group::get()
507-
->filter('ID:not', $this->ID)
508-
->map('Code', 'Title')
509-
->toArray();
532+
$hasGroupWithSameTitle = Group::get()
533+
->exclude('ID', $this->ID)
534+
->filter('Title', $this->Title)
535+
->exists();
510536

511-
if (in_array($this->Title, $currentGroups)) {
537+
if ($hasGroupWithSameTitle) {
512538
$result->addError(
513539
_t(
514540
'SilverStripe\\Security\\Group.ValidationIdentifierAlreadyExists',
@@ -711,16 +737,19 @@ public function requireDefaultRecords()
711737
*/
712738
private function dedupeCode(): void
713739
{
714-
$currentGroups = Group::get()
715-
->exclude('ID', $this->ID)
716-
->map('Code', 'Title')
717-
->toArray();
718740
$code = $this->Code;
719741
$count = 2;
720-
while (isset($currentGroups[$code])) {
742+
743+
while ($this->checkIfCodeExists($code)) {
721744
$code = $this->Code . '-' . $count;
722745
$count++;
723746
}
747+
724748
$this->setField('Code', $code);
725749
}
750+
751+
private function checkIfCodeExists(string $code): bool
752+
{
753+
return Group::get()->filter('Code', $code)->exclude('ID', $this->ID)->exists();
754+
}
726755
}

src/Security/Member.php

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,17 @@
2121
use SilverStripe\Forms\FieldList;
2222
use SilverStripe\Forms\HTMLEditor\HTMLEditorConfig;
2323
use SilverStripe\Forms\ListboxField;
24+
use SilverStripe\Forms\SearchableMultiDropdownField;
2425
use SilverStripe\Forms\Tab;
2526
use SilverStripe\Forms\TabSet;
27+
use SilverStripe\Forms\TreeMultiselectField;
2628
use SilverStripe\i18n\i18n;
2729
use SilverStripe\ORM\ArrayList;
2830
use SilverStripe\ORM\DataList;
2931
use SilverStripe\ORM\DataObject;
3032
use SilverStripe\ORM\DB;
3133
use SilverStripe\ORM\FieldType\DBDatetime;
34+
use SilverStripe\ORM\FieldType\DBForeignKey;
3235
use SilverStripe\ORM\HasManyList;
3336
use SilverStripe\ORM\ManyManyList;
3437
use SilverStripe\ORM\Map;
@@ -1365,29 +1368,31 @@ public function getCMSFields()
13651368
$fields->removeByName('RememberLoginHashes');
13661369

13671370
if (Permission::check('EDIT_PERMISSIONS')) {
1368-
// Filter allowed groups
13691371
$groups = Group::get();
1370-
$disallowedGroupIDs = $this->disallowedGroups();
1371-
if ($disallowedGroupIDs) {
1372+
1373+
if ($disallowedGroupIDs = $this->disallowedGroups()) {
13721374
$groups = $groups->exclude('ID', $disallowedGroupIDs);
13731375
}
1374-
$groupsMap = [];
1375-
foreach ($groups as $group) {
1376-
// Listboxfield values are escaped, use ASCII char instead of »
1377-
$groupsMap[$group->ID] = $group->getBreadcrumbs(' > ');
1378-
}
1379-
asort($groupsMap);
1376+
1377+
$threshold = DBForeignKey::config()->get('dropdown_field_threshold');
1378+
$overThreshold = $groups->count() > $threshold;
1379+
13801380
$fields->addFieldToTab(
13811381
'Root.Main',
1382-
ListboxField::create('DirectGroups', Group::singleton()->i18n_plural_name())
1383-
->setSource($groupsMap)
1384-
->setAttribute(
1385-
'data-placeholder',
1386-
_t(__CLASS__ . '.ADDGROUP', 'Add group', 'Placeholder text for a dropdown')
1387-
)
1382+
SearchableMultiDropdownField::create(
1383+
'DirectGroups',
1384+
Group::singleton()->i18n_plural_name(),
1385+
$groups,
1386+
null,
1387+
'BreadcrumbTitle'
1388+
)
1389+
->setIsSearchable(true)
1390+
->setUseSearchContext(true)
1391+
->setIsLazyLoaded($overThreshold)
1392+
->setLazyLoadLimit($threshold)
1393+
->setPlaceholder(_t(__CLASS__ . '.ADDGROUP', 'Add group', 'Placeholder text for a dropdown'))
13881394
);
13891395

1390-
13911396
// Add permission field (readonly to avoid complicated group assignment logic).
13921397
// This should only be available for existing records, as new records start
13931398
// with no permissions until they have a group assignment anyway.

0 commit comments

Comments
 (0)