diff --git a/forms/gridfield/GridFieldSortableHeader.php b/forms/gridfield/GridFieldSortableHeader.php index 3f693dfac..f430f7976 100644 --- a/forms/gridfield/GridFieldSortableHeader.php +++ b/forms/gridfield/GridFieldSortableHeader.php @@ -100,23 +100,27 @@ class GridFieldSortableHeader implements GridField_HTMLProvider, GridField_DataM $allowSort = ($title && $list->canSortBy($columnField)); - if(strpos($columnField, '.') !== false) { + if(!$allowSort && strpos($columnField, '.') !== false) { // we have a relation column with dot notation + // @see DataObject::relField for approximation $parts = explode('.', $columnField); - $tmpItem = singleton($list->dataClass()); for($idx = 0; $idx < sizeof($parts); $idx++) { $methodName = $parts[$idx]; - if($tmpItem instanceof DataObject && $tmpItem->hasField($methodName)) { - // If we've found a field, we can sort on it - $allowSort = true; - } else if ($tmpItem instanceof SS_List) { + if($tmpItem instanceof SS_List) { // It's impossible to sort on a HasManyList/ManyManyList - throw new Exception(__CLASS__ . ' is unable to sort on has_many/many_many relations,' - . ' please only specify has_one relations'); - } else { - // Else, the part is a relation name, so get the object/list from it + break; + } elseif($tmpItem->hasMethod($methodName)) { + // The part is a relation name, so get the object/list from it $tmpItem = $tmpItem->$methodName(); + } elseif($tmpItem instanceof DataObject && $tmpItem->hasField($methodName)) { + // Else, if we've found a field at the end of the chain, we can sort on it. + // If a method is applied further to this field (E.g. 'Cost.Currency') then don't try to sort. + $allowSort = $idx === sizeof($parts) - 1; + break; + } else { + // If neither method nor field, then unable to sort + break; } } } diff --git a/tests/forms/gridfield/GridFieldSortableHeaderTest.php b/tests/forms/gridfield/GridFieldSortableHeaderTest.php index c7c6052ca..79cfca5d8 100644 --- a/tests/forms/gridfield/GridFieldSortableHeaderTest.php +++ b/tests/forms/gridfield/GridFieldSortableHeaderTest.php @@ -13,6 +13,31 @@ class GridFieldSortableHeaderTest extends SapphireTest { 'GridFieldSortableHeaderTest_Cheerleader', 'GridFieldSortableHeaderTest_CheerleaderHat' ); + + /** + * Tests that the appropriate sortable headers are generated + */ + public function testRenderHeaders() { + + // Generate sortable header and extract HTML + $list = new DataList('GridFieldSortableHeaderTest_Team'); + $config = new GridFieldConfig_RecordEditor(); + $form = new Form(Controller::curr(), 'Form', new FieldList(), new FieldList()); + $gridField = new GridField('testfield', 'testfield', $list, $config); + $gridField->setForm($form); + $compontent = $gridField->getConfig()->getComponentByType('GridFieldSortableHeader'); + $htmlFragment = $compontent->getHTMLFragments($gridField); + + // Check that the output shows name and hat as sortable fields, but not city + $this->assertContains('City', $htmlFragment['header']); + $this->assertContains('value="Name" class="action ss-gridfield-sort" id="action_SetOrderName"', $htmlFragment['header']); + $this->assertContains('value="Cheerleader Hat" class="action ss-gridfield-sort" id="action_SetOrderCheerleader-Hat-Colour"', $htmlFragment['header']); + + // Check inverse of above + $this->assertNotContains('value="City" class="action ss-gridfield-sort" id="action_SetOrderCity"', $htmlFragment['header']); + $this->assertNotContains('Name', $htmlFragment['header']); + $this->assertNotContains('Cheerleader Hat', $htmlFragment['header']); + } public function testGetManipulatedData() { $list = new DataList('GridFieldSortableHeaderTest_Team'); @@ -77,6 +102,12 @@ class GridFieldSortableHeaderTest extends SapphireTest { } class GridFieldSortableHeaderTest_Team extends DataObject implements TestOnly { + + private static $summary_fields = array( + 'Name' => 'Name', + 'City.Initial' => 'City', + 'Cheerleader.Hat.Colour' => 'Cheerleader Hat' + ); private static $db = array( 'Name' => 'Varchar',