Merge pull request #11655 from creative-commoners/pulls/5.3/search-fallback

FIX Use search context when no database field for label

Author: GuySartorelli

src/Forms/SearchableDropdownTrait.php

@@ -14,7 +14,6 @@
 use SilverStripe\ORM\DataObjectInterface;
 use SilverStripe\ORM\Relation;
 use SilverStripe\ORM\SS_List;
-use SilverStripe\ORM\FieldType\DBHTMLText;
 use SilverStripe\View\ArrayData;
 use SilverStripe\ORM\Search\SearchContext;
 use SilverStripe\Security\SecurityToken;
@@ -225,6 +224,9 @@ public function setUseDynamicPlaceholder(bool $useDynamicPlaceholder): static
 
     /**
      * Get whether to use a search context instead searching on labelField
+     *
+     * If the label field doesn't map to a database field and instead uses a getter e.g. 'MyField' calls
+     * a 'getMyField()' method, then search context will be used regardless of what is returned here
      */
     public function getUseSearchContext(): bool
     {
@@ -233,6 +235,9 @@ public function getUseSearchContext(): bool
 
     /**
      * Set whether to use a search context instead searching on labelField
+     *
+     * If the label field doesn't map to a database field and instead uses a getter e.g. 'MyField' calls
+     * a 'getMyField()' method, then search context will be used regardless of what is set here
      */
     public function setUseSearchContext(bool $useSearchContext): static
     {
@@ -285,8 +290,10 @@ public function setSource($source): static
 
     /**
      * Get the field to use for the label of the option
+     * This field is also used for searching if that field exists in the database and search context is not being used
      *
-     * The default value of 'Title' will map to DataObject::getTitle() if a Title DB field does not exist
+     * If the label field does not map to a database field then a getter will be used if it exists
+     * e.g. the default value of 'Title' will map to DataObject::getTitle() if a Title DB field does not exist
      */
     public function getLabelField(): string
     {
@@ -295,6 +302,7 @@ public function getLabelField(): string
 
     /**
      * Set the field to use for the label of the option
+     * This field is also used for searching if that field exists in the database and search context is not being used
      */
     public function setLabelField(string $labelField): static
     {
@@ -497,9 +505,14 @@ private function getOptionsForSearchRequest(string $term): array
         $labelField = $this->getLabelField();
         /** @var DataObject $obj */
         $obj = $dataClass::create();
-        $key = $this->getUseSearchContext() ? $obj->getGeneralSearchFieldName() : $this->getLabelField();
-        $searchParams = [$key => $term];
         $hasLabelField = (bool) $obj->getSchema()->fieldSpec($dataClass, $labelField);
+        $key = $labelField;
+        // Label field still works for making labels if there's a getter method
+        // on the object e.g. 'Title' ... getTitle(), however we can't use it for database queries
+        if (!$hasLabelField || $this->getUseSearchContext()) {
+            $key = $obj->getGeneralSearchFieldName();
+        }
+        $searchParams = [$key => $term];
         $sort = $hasLabelField ? $labelField : null;
         $limit = $this->getLazyLoadLimit();
         $newList = $this->getSearchContext()->getQuery($searchParams, $sort, $limit);

tests/php/Forms/SearchableDropdownTraitTest.php

@@ -15,6 +15,7 @@
 use SilverStripe\Forms\HiddenField;
 use stdClass;
 use SilverStripe\Forms\Form;
+use SilverStripe\Security\Member;
 
 class SearchableDropdownTraitTest extends SapphireTest
 {
@@ -261,4 +262,55 @@ public function testLazyLoadedDoesntCallGetSource(string $fieldClass, string $me
         $mockField->setForm(new Form());
         $mockField->$methodToCall();
     }
+
+    public static function provideSearchDataObjectWithMethodForLabelField(): array
+    {
+        return [
+            'single' => [
+                'term' => 'alex',
+                'expected' => '[{"value":0,"label":"Aziel, Alex"}]',
+            ],
+            'multi' => [
+                'term' => 'z',
+                'expected' => '[{"value":0,"label":"Aziel, Alex"},{"value":0,"label":"Cazton, Cindy"}]',
+            ],
+            'email' => [
+                'term' => '@example.com',
+                'expected' => implode('', [
+                    '[{"value":0,"label":"Aziel, Alex"},{"value":0,"label":"Brown, Bob"}',
+                    ',{"value":0,"label":"Cazton, Cindy"}]',
+                ]),
+            ],
+        ];
+    }
+
+    /**
+     * Member has a method getTitle() that returns "Surname, FirstName"
+     * The default label field on the field is "Title", which doesn't exist on the Member database table
+     * It should fall back to using search context in this scenario
+     *
+     * @dataProvider provideSearchDataObjectWithMethodForLabelField
+     */
+    public function testSearchDataObjectWithMethodForLabelField(string $term, string $expected): void
+    {
+        $data = [
+            ['FirstName' => 'Alex', 'Surname' => 'Aziel', 'Email' => 'aaron.aziel@example.com'],
+            ['FirstName' => 'Bob', 'Surname' => 'Brown', 'Email' => 'bob.brown@example.com'],
+            ['FirstName' => 'Cindy', 'Surname' => 'Cazton', 'Email' => 'cindy.cazton@example.com'],
+        ];
+        foreach ($data as $row) {
+            $member = new Member($row);
+            $member->write();
+        }
+        // 4 members because of the 3 members above and the default admin member
+        $this->assertSame(4, Member::get()->count());
+        $field = new SearchableDropdownField('MyField', 'MyField', Member::get());
+        $field->setLabelField('Title');
+        $request = new HTTPRequest('GET', 'someurl', ['term' => $term]);
+        $request->addHeader('X-SecurityID', SecurityToken::getSecurityID());
+        $body = $field->search($request)->getBody();
+        // Replace the member ID value with 0 to make the test more stable
+        $body = preg_replace('/"value":[0-9]+/', '"value":0', $body);
+        $this->assertSame($expected, $body);
+    }
 }