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

@@ -1,13 +1,17 @@
 <?php
+
 /**
  * Provides introspection information about the class tree.
- * It's a cached wrapper around the built-in class functions.  SilverStripe uses class introspection heavily
+ *
- * and without the caching it creates an unfortunate performance hit.
+ * It's a cached wrapper around the built-in class functions.  SilverStripe uses 
+ * class introspection heavily and without the caching it creates an unfortunate 
+ * performance hit.
  *
  * @package framework
  * @subpackage core
  */
 class ClassInfo {
+
 	/**
 	 * Wrapper for classes getter.
 	 */
@@ -245,5 +249,42 @@ class ClassInfo {
 		return self::$method_from_cache[$class][$method] == $compclass;
 	}
 	
+
+	/**
+	 * Returns the table name in the class hierarchy which contains a given 
+	 * field column for a {@link DataObject}. If the field does not exist, this
+	 * will return null.
+	 *
+	 * @param string $candidateClass
+	 * @param string $fieldName
+	 *
+	 * @return string
+	 */
+	public static function table_for_object_field($candidateClass, $fieldName) {
+		if(!$candidateClass || !$fieldName) {
+			return null;
+		}
+
+		$exists = class_exists($candidateClass);
+
+		while($candidateClass && $candidateClass != 'DataObject' && $exists) {
+			if(DataObject::has_own_table($candidateClass)) {
+				$inst = singleton($candidateClass);
+				
+				if($inst->hasOwnTableDatabaseField($fieldName)) {
+					break;
+				}
+			}
+
+			$candidateClass = get_parent_class($candidateClass);
+			$exists = class_exists($candidateClass);
+		}
+
+		if(!$candidateClass || !$exists) {
+			return null;
+		}
+
+		return $candidateClass;
+	}
 }
 

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'
+	 * Returns the manipulated (sorted) DataList. Field names will simply add an 
-	 * clause, relation names will add appropriate joins to the DataQuery first.
+	 * '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
 					);
 

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
+		$candidateClass = ClassInfo::table_for_object_field(
-		// Todo: move to somewhere more appropriate, such as DataMapper, the
+			$this->model, 
-		// magical class-to-be?
+			$this->name
-		$candidateClass = $this->model;
+		);
-
-		while($candidateClass != 'DataObject') {
-			if(DataObject::has_own_table($candidateClass) 
-					&& singleton($candidateClass)->hasOwnTableDatabaseField($this->name)) {
-				break;
-			}
-
-			$candidateClass = get_parent_class($candidateClass);
-		}
 
 		if($candidateClass == 'DataObject') {
 			// fallback to the provided name in the event of a joined column

tests/core/ClassInfoTest.php

@@ -1,4 +1,5 @@
 <?php
+
 /**
  * @package framework
  * @subpackage tests
@@ -78,38 +79,136 @@ class ClassInfoTest extends SapphireTest {
 	public function testDataClassesFor() {
 		$expect = array(
 			'ClassInfoTest_BaseDataClass' => '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'
+	);
 }