Disable sorting for an associated_property callable (#6675)

Sorting by a callable would imply using that callable on every row, then sorting by the resulting column. Both operations would perform badly on large number of rows.
sonata-project · Dec 9, 2020 · 03a3799 · 03a3799
1 parent 55311f3
commit 03a3799
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 1 deletion.
diff --git a/src/Datagrid/ListMapper.php b/src/Datagrid/ListMapper.php
@@ -86,7 +86,7 @@ public function add($name, $type = null, array $fieldDescriptionOptions = [])
         // Default sort on "associated_property"
         if (isset($fieldDescriptionOptions['associated_property'])) {
             if (!isset($fieldDescriptionOptions['sortable'])) {
-                $fieldDescriptionOptions['sortable'] = true;
+                $fieldDescriptionOptions['sortable'] = !\is_callable($fieldDescriptionOptions['associated_property']);
             }
             if (!isset($fieldDescriptionOptions['sort_parent_association_mappings'])) {
                 $fieldDescriptionOptions['sort_parent_association_mappings'] = [[

diff --git a/tests/Datagrid/ListMapperTest.php b/tests/Datagrid/ListMapperTest.php
@@ -337,6 +337,32 @@ public function testAutoSortOnAssociatedProperty(): void
         $this->assertSame('fooSortFieldMapping', $fieldManualSort->getOption('sort_field_mapping'));
     }
 
+    public function testCallableAssociationPropertyCannotBeSortable(): void
+    {
+        $this->listMapper->add(
+            'fooNameNotSortable',
+            null,
+            [
+                'associated_property' => static function ($value) {
+                    return (string) $value;
+                },
+            ]
+        );
+        $this->listMapper->add(
+            'fooNameSortable',
+            null,
+            [
+                'associated_property' => 'fooProperty',
+            ]
+        );
+
+        $fieldSortable = $this->listMapper->get('fooNameSortable');
+        $fieldNotSortable = $this->listMapper->get('fooNameNotSortable');
+
+        $this->assertTrue($fieldSortable->getOption('sortable'));
+        $this->assertFalse($fieldNotSortable->getOption('sortable'));
+    }
+
     public function testKeys(): void
     {
         $fieldDescription1 = $this->getFieldDescriptionMock('fooName1', 'fooLabel1');