From 920978df9902b772e5703e6247f3c972a5e83015 Mon Sep 17 00:00:00 2001 From: Will Rossiter Date: Mon, 18 Aug 2014 15:39:42 +1200 Subject: [PATCH] API: Add ClassInfo::table_for_object_field Returns the table name for a field in a class hierarchy. This issue raised itself with GridFieldSortableHeader not supporting sorting on fields from parent class fields. --- core/ClassInfo.php | 45 +++++++- forms/gridfield/GridFieldSortableHeader.php | 20 +++- search/filters/SearchFilter.php | 17 +-- tests/core/ClassInfoTest.php | 115 ++++++++++++++++++-- 4 files changed, 169 insertions(+), 28 deletions(-) diff --git a/core/ClassInfo.php b/core/ClassInfo.php index 5a25a4a5d..fed9fe7b3 100644 --- a/core/ClassInfo.php +++ b/core/ClassInfo.php @@ -1,13 +1,17 @@ hasOwnTableDatabaseField($fieldName)) { + break; + } + } + + $candidateClass = get_parent_class($candidateClass); + $exists = class_exists($candidateClass); + } + + if(!$candidateClass || !$exists) { + return null; + } + + return $candidateClass; + } } diff --git a/forms/gridfield/GridFieldSortableHeader.php b/forms/gridfield/GridFieldSortableHeader.php index 1a4a88967..cfd4983c4 100644 --- a/forms/gridfield/GridFieldSortableHeader.php +++ b/forms/gridfield/GridFieldSortableHeader.php @@ -192,8 +192,9 @@ class GridFieldSortableHeader implements GridField_HTMLProvider, GridField_DataM } /** - * Returns the manipulated (sorted) DataList. Field names will simply add an 'ORDER BY' - * clause, relation names will add appropriate joins to the DataQuery first. + * Returns the manipulated (sorted) DataList. Field names will simply add an + * 'ORDER BY' clause, relation names will add appropriate joins to the + * {@link DataQuery} first. * * @param GridField * @param SS_List @@ -223,10 +224,19 @@ class GridFieldSortableHeader implements GridField_HTMLProvider, GridField_DataM // Traverse to the relational list $tmpItem = $tmpItem->$methodName(); - // Add a left join to the query + $joinClass = ClassInfo::table_for_object_field( + $lastAlias, + $methodName . "ID" + ); + + // if the field isn't in the object tree then it is likely + // been aliased. In that event, assume what the user has + // provided is the correct value + if(!$joinClass) $joinClass = $lastAlias; + $dataList = $dataList->leftJoin( $tmpItem->class, - '"' . $methodName . '"."ID" = "' . $lastAlias . '"."' . $methodName . 'ID"', + '"' . $methodName . '"."ID" = "' . $joinClass . '"."' . $methodName . 'ID"', $methodName ); @@ -244,7 +254,7 @@ class GridFieldSortableHeader implements GridField_HTMLProvider, GridField_DataM // as ->sort() won't do it by itself. Blame PostgreSQL for making this necessary $pieces = explode('.', $column); $column = '"' . implode('"."', $pieces) . '"'; - + return $dataList->sort($column, $state->SortDirection); } } diff --git a/search/filters/SearchFilter.php b/search/filters/SearchFilter.php index 5df64815f..6af60ea73 100644 --- a/search/filters/SearchFilter.php +++ b/search/filters/SearchFilter.php @@ -166,19 +166,10 @@ abstract class SearchFilter extends Object { return $this->name; } - // This code finds the table where the field named $this->name lives - // Todo: move to somewhere more appropriate, such as DataMapper, the - // magical class-to-be? - $candidateClass = $this->model; - - while($candidateClass != 'DataObject') { - if(DataObject::has_own_table($candidateClass) - && singleton($candidateClass)->hasOwnTableDatabaseField($this->name)) { - break; - } - - $candidateClass = get_parent_class($candidateClass); - } + $candidateClass = ClassInfo::table_for_object_field( + $this->model, + $this->name + ); if($candidateClass == 'DataObject') { // fallback to the provided name in the event of a joined column diff --git a/tests/core/ClassInfoTest.php b/tests/core/ClassInfoTest.php index f37ceec04..844b7c84d 100644 --- a/tests/core/ClassInfoTest.php +++ b/tests/core/ClassInfoTest.php @@ -1,4 +1,5 @@ 'ClassInfoTest_BaseDataClass', - 'ClassInfoTest_HasFields' => 'ClassInfoTest_HasFields' + 'ClassInfoTest_HasFields' => 'ClassInfoTest_HasFields', + 'ClassInfoTest_WithRelation' => 'ClassInfoTest_WithRelation' ); $classes = array( 'ClassInfoTest_BaseDataClass', 'ClassInfoTest_NoFields', - 'ClassInfoTest_HasFields' + 'ClassInfoTest_HasFields', ); - foreach ($classes as $class) { - $this->assertEquals($expect, ClassInfo::dataClassesFor($class)); - } + + $this->assertEquals($expect, ClassInfo::dataClassesFor($classes[0])); + $this->assertEquals($expect, ClassInfo::dataClassesFor($classes[1])); + + $expect = array( + 'ClassInfoTest_BaseDataClass' => 'ClassInfoTest_BaseDataClass', + 'ClassInfoTest_HasFields' => 'ClassInfoTest_HasFields', + ); + + $this->assertEquals($expect, ClassInfo::dataClassesFor($classes[2])); } + public function testTableForObjectField() { + $this->assertEquals('ClassInfoTest_WithRelation', + ClassInfo::table_for_object_field('ClassInfoTest_WithRelation', 'RelationID') + ); + + $this->assertEquals('ClassInfoTest_BaseDataClass', + ClassInfo::table_for_object_field('ClassInfoTest_BaseDataClass', 'Title') + ); + + $this->assertEquals('ClassInfoTest_BaseDataClass', + ClassInfo::table_for_object_field('ClassInfoTest_HasFields', 'Title') + ); + + $this->assertEquals('ClassInfoTest_BaseDataClass', + ClassInfo::table_for_object_field('ClassInfoTest_NoFields', 'Title') + ); + + $this->assertEquals('ClassInfoTest_HasFields', + ClassInfo::table_for_object_field('ClassInfoTest_HasFields', 'Description') + ); + + // existing behaviour fallback to DataObject? Should be null. + $this->assertEquals('DataObject', + ClassInfo::table_for_object_field('ClassInfoTest_BaseClass', 'Nonexist') + ); + + $this->assertNull( + ClassInfo::table_for_object_field('SomeFakeClassHere', 'Title') + ); + + $this->assertNull( + ClassInfo::table_for_object_field('Object', 'Title') + ); + + $this->assertNull( + ClassInfo::table_for_object_field(null, null) + ); + } } +/** + * @package framework + * @subpackage tests + */ + class ClassInfoTest_BaseClass extends DataObject { } +/** + * @package framework + * @subpackage tests + */ + class ClassInfoTest_ChildClass extends ClassInfoTest_BaseClass { } +/** + * @package framework + * @subpackage tests + */ + class ClassInfoTest_GrandChildClass extends ClassInfoTest_ChildClass { } +/** + * @package framework + * @subpackage tests + */ + class ClassInfoTest_BaseDataClass extends DataObject { - private static $db = array('Title' => 'Varchar'); + + private static $db = array( + 'Title' => 'Varchar' + ); } -class ClassInfoTest_NoFields extends ClassInfoTest_BaseDataClass {} + +/** + * @package framework + * @subpackage tests + */ + +class ClassInfoTest_NoFields extends ClassInfoTest_BaseDataClass { + +} + +/** + * @package framework + * @subpackage tests + */ + class ClassInfoTest_HasFields extends ClassInfoTest_NoFields { - private static $db = array('Description' => 'Varchar'); + + private static $db = array( + 'Description' => 'Varchar' + ); +} + +/** + * @package framework + * @subpackage tests + */ + +class ClassInfoTest_WithRelation extends ClassInfoTest_NoFields { + + private static $has_one = array( + 'Relation' => 'ClassInfoTest_HasFields' + ); }