FIX 11577 Performance issues with large list of groups

silverstripe · Jan 27, 2025 · 120baae · 120baae
1 parent 227e178
commit 120baae
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 29 deletions.
diff --git a/src/Security/Group.php b/src/Security/Group.php
@@ -23,13 +23,16 @@
 use SilverStripe\Forms\ListboxField;
 use SilverStripe\Forms\LiteralField;
 use SilverStripe\Forms\RequiredFields;
+use SilverStripe\Forms\SearchableDropdownField;
 use SilverStripe\Forms\Tab;
 use SilverStripe\Forms\TabSet;
 use SilverStripe\Forms\TextareaField;
 use SilverStripe\Forms\TextField;
+use SilverStripe\Forms\TreeDropdownField;
 use SilverStripe\ORM\ArrayList;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\DataQuery;
+use SilverStripe\ORM\FieldType\DBForeignKey;
 use SilverStripe\ORM\HasManyList;
 use SilverStripe\ORM\Hierarchy\Hierarchy;
 use SilverStripe\ORM\ManyManyList;
@@ -83,6 +86,11 @@ class Group extends DataObject
         Hierarchy::class,
     ];
 
+    private static $searchable_fields = [
+        'Title',
+        'Description',
+    ];
+
     private static $table_name = "Group";
 
     private static $indexes = [
@@ -122,18 +130,28 @@ private function getDecodedBreadcrumbs()
      */
     public function getCMSFields()
     {
+        $threshold = DBForeignKey::config()->get('dropdown_field_threshold');
+        $overThreshold = $groups->count() > $threshold;
+
         $fields = new FieldList(
             new TabSet(
                 "Root",
                 new Tab(
                     'Members',
                     _t(__CLASS__ . '.MEMBERS', 'Members'),
                     new TextField("Title", $this->fieldLabel('Title')),
-                    $parentidfield = DropdownField::create(
+                    $parentidfield = SearchableDropdownField::create(
                         'ParentID',
                         $this->fieldLabel('Parent'),
-                        $this->getDecodedBreadcrumbs()
-                    )->setEmptyString(' '),
+                        Group::get(),
+                        null,
+                        'BreadcrumbTitle'
+                    )
+                        ->setIsSearchable(true)
+                        ->setUseSearchContext(true)
+                        ->setPlaceholder(' ')
+                        ->setIsLazyLoaded($overThreshold)
+                        ->setLazyLoadLimit($threshold),
                     new TextareaField('Description', $this->fieldLabel('Description'))
                 ),
                 $permissionsTab = new Tab(
@@ -460,6 +478,14 @@ public function stageChildren()
             ->sort('"Sort"');
     }
 
+    /**
+     * @return string
+     */
+    public function getBreadcrumbTitle(): string
+    {
+        return $this->getBreadcrumbs(' > ');
+    }
+
     /**
      * @return string
      */
@@ -503,12 +529,12 @@ public function validate()
             }
         }
 
-        $currentGroups = Group::get()
-            ->filter('ID:not', $this->ID)
-            ->map('Code', 'Title')
-            ->toArray();
+        $hasGroupWithSameTitle = Group::get()
+            ->exclude('ID', $this->ID)
+            ->filter('Title', $this->Title)
+            ->exists();
 
-        if (in_array($this->Title, $currentGroups)) {
+        if ($hasGroupWithSameTitle) {
             $result->addError(
                 _t(
                     'SilverStripe\\Security\\Group.ValidationIdentifierAlreadyExists',
@@ -711,16 +737,19 @@ public function requireDefaultRecords()
      */
     private function dedupeCode(): void
     {
-        $currentGroups = Group::get()
-            ->exclude('ID', $this->ID)
-            ->map('Code', 'Title')
-            ->toArray();
         $code = $this->Code;
         $count = 2;
-        while (isset($currentGroups[$code])) {
+
+        while ($this->checkIfCodeExists($code)) {
             $code = $this->Code . '-' . $count;
             $count++;
         }
+
         $this->setField('Code', $code);
     }
+
+    private function checkIfCodeExists(string $code): bool
+    {
+        return Group::get()->filter('Code', $code)->exclude('ID', $this->ID)->exists();
+    }
 }
diff --git a/src/Security/Member.php b/src/Security/Member.php
@@ -21,14 +21,17 @@
 use SilverStripe\Forms\FieldList;
 use SilverStripe\Forms\HTMLEditor\HTMLEditorConfig;
 use SilverStripe\Forms\ListboxField;
+use SilverStripe\Forms\SearchableMultiDropdownField;
 use SilverStripe\Forms\Tab;
 use SilverStripe\Forms\TabSet;
+use SilverStripe\Forms\TreeMultiselectField;
 use SilverStripe\i18n\i18n;
 use SilverStripe\ORM\ArrayList;
 use SilverStripe\ORM\DataList;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\DB;
 use SilverStripe\ORM\FieldType\DBDatetime;
+use SilverStripe\ORM\FieldType\DBForeignKey;
 use SilverStripe\ORM\HasManyList;
 use SilverStripe\ORM\ManyManyList;
 use SilverStripe\ORM\Map;
@@ -1365,29 +1368,31 @@ public function getCMSFields()
             $fields->removeByName('RememberLoginHashes');
 
             if (Permission::check('EDIT_PERMISSIONS')) {
-                // Filter allowed groups
                 $groups = Group::get();
-                $disallowedGroupIDs = $this->disallowedGroups();
-                if ($disallowedGroupIDs) {
+
+                if ($disallowedGroupIDs = $this->disallowedGroups()) {
                     $groups = $groups->exclude('ID', $disallowedGroupIDs);
                 }
-                $groupsMap = [];
-                foreach ($groups as $group) {
-                    // Listboxfield values are escaped, use ASCII char instead of &raquo;
-                    $groupsMap[$group->ID] = $group->getBreadcrumbs(' > ');
-                }
-                asort($groupsMap);
+
+                $threshold = DBForeignKey::config()->get('dropdown_field_threshold');
+                $overThreshold = $groups->count() > $threshold;
+
                 $fields->addFieldToTab(
                     'Root.Main',
-                    ListboxField::create('DirectGroups', Group::singleton()->i18n_plural_name())
-                        ->setSource($groupsMap)
-                        ->setAttribute(
-                            'data-placeholder',
-                            _t(__CLASS__ . '.ADDGROUP', 'Add group', 'Placeholder text for a dropdown')
-                        )
+                    SearchableMultiDropdownField::create(
+                        'DirectGroups',
+                        Group::singleton()->i18n_plural_name(),
+                        $groups,
+                        null,
+                        'BreadcrumbTitle'
+                    )
+                        ->setIsSearchable(true)
+                        ->setUseSearchContext(true)
+                        ->setIsLazyLoaded($overThreshold)
+                        ->setLazyLoadLimit($threshold)
+                        ->setPlaceholder(_t(__CLASS__ . '.ADDGROUP', 'Add group', 'Placeholder text for a dropdown'))
                 );
 
-
                 // Add permission field (readonly to avoid complicated group assignment logic).
                 // This should only be available for existing records, as new records start
                 // with no permissions until they have a group assignment anyway.