FIX 11577 Performance issues with large list of groups

silverstripe · Feb 5, 2025 · 7ffbd21 · 7ffbd21
1 parent 227e178
commit 7ffbd21
Show file tree

Hide file tree

Showing 2 changed files with 87 additions and 32 deletions.
diff --git a/src/Security/Group.php b/src/Security/Group.php
@@ -23,6 +23,7 @@
 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;
@@ -33,6 +34,7 @@
 use SilverStripe\ORM\HasManyList;
 use SilverStripe\ORM\Hierarchy\Hierarchy;
 use SilverStripe\ORM\ManyManyList;
+use SilverStripe\ORM\Search\SearchContext;
 use SilverStripe\ORM\UnsavedRelationList;
 
 /**
@@ -91,6 +93,25 @@ class Group extends DataObject
         'Sort' => true,
     ];
 
+    private static $dropdown_field_threshold = 100;
+
+    public static function get_search_context_for_dropdown(): SearchContext
+    {
+        $group = Group::singleton();
+
+        return SearchContext::create(
+            Group::class,
+            FieldList::create(
+                TextField::create('Title', $group->fieldLabel('Title')),
+                TextField::create('Description', $group->fieldLabel('Description')),
+            ),
+            [
+                'Title' => 'PartialFilter',
+                'Description' => 'PartialFilter',
+            ]
+        );
+    }
+
     public function getAllChildren()
     {
         $doSet = new ArrayList();
@@ -122,18 +143,33 @@ private function getDecodedBreadcrumbs()
      */
     public function getCMSFields()
     {
+        $threshold = Group::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)
+                        ->setSearchContext(Group::get_search_context_for_dropdown())
+                        ->setPlaceholder(_t(
+                            __CLASS__ . '.PARENT_GROUP_PLACEHOLDER',
+                            'Select parent group',
+                            'Placeholder text for a dropdown'
+                        ))
+                        ->setIsLazyLoaded($overThreshold)
+                        ->setLazyLoadLimit($threshold),
                     new TextareaField('Description', $this->fieldLabel('Description'))
                 ),
                 $permissionsTab = new Tab(
@@ -460,6 +496,14 @@ public function stageChildren()
             ->sort('"Sort"');
     }
 
+    /**
+     * @return string
+     */
+    public function getBreadcrumbTitle(): string
+    {
+        return $this->getBreadcrumbs(' > ');
+    }
+
     /**
      * @return string
      */
@@ -503,12 +547,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 +755,21 @@ 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])) {
-            $code = $this->Code . '-' . $count;
-            $count++;
+
+        if ($code) {
+            while ($this->checkIfCodeExists($code)) {
+                $code = $this->Code . '-' . $count;
+                $count++;
+            }
+
+            $this->setField('Code', $code);
         }
-        $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,32 @@ 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 = Group::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)
+                        ->setSearchContext(Group::get_search_context_for_dropdown())
+                        ->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.