API Enable DataList::sort to support composite field names (similar to filter)

This commit is contained in:
Damian Mooyman 2015-09-17 10:19:03 +12:00
parent 37957b7ee8
commit 2b1e5ee071
8 changed files with 268 additions and 115 deletions

View File

@ -208,53 +208,6 @@ class GridFieldSortableHeader implements GridField_HTMLProvider, GridField_DataM
return $dataList;
}
$column = $state->SortColumn;
// if we have a relation column with dot notation
if(strpos($column, '.') !== false) {
$lastAlias = $dataList->dataClass();
$tmpItem = singleton($lastAlias);
$parts = explode('.', $state->SortColumn);
for($idx = 0; $idx < sizeof($parts); $idx++) {
$methodName = $parts[$idx];
// If we're not on the last item, we're looking at a relation
if($idx !== sizeof($parts) - 1) {
// Traverse to the relational list
$tmpItem = $tmpItem->$methodName();
$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" = "' . $joinClass . '"."' . $methodName . 'ID"',
$methodName
);
// Store the last 'alias' name as it'll be used for the next
// join, or the 'sort' column
$lastAlias = $methodName;
} else {
// Change relation.relation.fieldname to alias.fieldname
$column = $lastAlias . '.' . $methodName;
}
}
}
// We need to manually create our ORDER BY "Foo"."Bar" string for relations,
// 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('asc'));
return $dataList->sort($state->SortColumn, $state->SortDirection('asc'));
}
}

View File

@ -297,28 +297,26 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
if ($count == 2) {
list($col, $dir) = func_get_args();
// Validate direction
if(!in_array(strtolower($dir),array('desc','asc'))){
user_error('Second argument to sort must be either ASC or DESC');
}
$sort = array($col => $dir);
}
else {
$sort = func_get_arg(0);
}
return $this->alterDataQuery(function($query, $list) use ($sort, $col, $dir){
return $this->alterDataQuery(function(DataQuery $query, DataList $list) use ($sort){
if ($col) {
// sort('Name','Desc')
if(!in_array(strtolower($dir),array('desc','asc'))){
user_error('Second argument to sort must be either ASC or DESC');
}
$query->sort($col, $dir);
}
else if(is_string($sort) && $sort){
// sort('Name ASC')
if(is_string($sort) && $sort){
if(stristr($sort, ' asc') || stristr($sort, ' desc')) {
$query->sort($sort);
} else {
$query->sort($sort, 'ASC');
$list->applyRelation($sort, $column);
$query->sort($column, 'ASC');
}
}
@ -326,15 +324,11 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
// sort(array('Name'=>'desc'));
$query->sort(null, null); // wipe the sort
foreach($sort as $col => $dir) {
foreach($sort as $column => $direction) {
// Convert column expressions to SQL fragment, while still allowing the passing of raw SQL
// fragments.
try {
$relCol = $list->getRelationName($col);
} catch(InvalidArgumentException $e) {
$relCol = $col;
}
$query->sort($relCol, $dir, false);
$list->applyRelation($column, $relationColumn);
$query->sort($relationColumn, $direction, false);
}
}
});
@ -478,6 +472,44 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
return $output;
}
/**
* Given a field or relation name, apply it safely to this datalist.
*
* Unlike getRelationName, this is immutable and will fallback to the quoted field
* name if not a relation.
*
* @param string $field Name of field or relation to apply
* @param string &$columnName Quoted column name
* @return DataList DataList with this relation applied
*/
public function applyRelation($field, &$columnName = null) {
// If field is invalid, return it without modification
if(!$this->isValidRelationName($field)) {
$columnName = $field;
return $this;
}
// Simple fields without relations are mapped directly
if(strpos($field,'.') === false) {
$columnName = '"'.$field.'"';
return $this;
}
return $this->alterDataQuery(function(DataQuery $query, DataList $list) use ($field, &$columnName) {
$columnName = $list->getRelationName($field);
});
}
/**
* Check if the given field specification could be interpreted as an unquoted relation name
*
* @param string $field
* @return bool
*/
protected function isValidRelationName($field) {
return preg_match('/^[A-Z0-9._]+$/i', $field);
}
/**
* Translates a {@link Object} relation name to a Database name and apply
* the relation join to the query. Throws an InvalidArgumentException if
@ -489,7 +521,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
* @return string
*/
public function getRelationName($field) {
if(!preg_match('/^[A-Z0-9._]+$/i', $field)) {
if(!$this->isValidRelationName($field)) {
throw new InvalidArgumentException("Bad field expression $field");
}
@ -507,7 +539,13 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
$fieldName = array_pop($relations);
$relationModelName = $this->dataQuery->applyRelation($field);
return '"'.$relationModelName.'"."'.$fieldName.'"';
// Find the db field the relation belongs to
$className = ClassInfo::table_for_object_field($relationModelName, $fieldName);
if(empty($className)) {
$className = $relationModelName;
}
return '"'.$className.'"."'.$fieldName.'"';
}
/**

View File

@ -670,7 +670,7 @@ class DataQuery {
$ancestry = array_reverse($ancestry);
foreach($ancestry as $ancestor){
if($ancestor != $component){
$this->query->addInnerJoin($ancestor, "\"$component\".\"ID\" = \"$ancestor\".\"ID\"");
$this->query->addLeftJoin($ancestor, "\"$component\".\"ID\" = \"$ancestor\".\"ID\"");
}
}
}
@ -703,7 +703,7 @@ class DataQuery {
list($parentClass, $componentClass, $parentField, $componentField, $relationTable) = $component;
$parentBaseClass = ClassInfo::baseDataClass($parentClass);
$componentBaseClass = ClassInfo::baseDataClass($componentClass);
$this->query->addInnerJoin($relationTable,
$this->query->addLeftJoin($relationTable,
"\"$relationTable\".\"$parentField\" = \"$parentBaseClass\".\"ID\"");
$this->query->addLeftJoin($componentBaseClass,
"\"$relationTable\".\"$componentField\" = \"$componentBaseClass\".\"ID\"");

View File

@ -10,8 +10,10 @@ class GridFieldSortableHeaderTest extends SapphireTest {
protected $extraDataObjects = array(
'GridFieldSortableHeaderTest_Team',
'GridFieldSortableHeaderTest_TeamGroup',
'GridFieldSortableHeaderTest_Cheerleader',
'GridFieldSortableHeaderTest_CheerleaderHat'
'GridFieldSortableHeaderTest_CheerleaderHat',
'GridFieldSortableHeaderTest_Mom'
);
/**
@ -103,6 +105,94 @@ class GridFieldSortableHeaderTest extends SapphireTest {
);
}
/**
* Test getManipulatedData on subclassed dataobjects
*/
public function testInheritedGetManiplatedData() {
$list = GridFieldSortableHeaderTest_TeamGroup::get();
$config = new GridFieldConfig_RecordEditor();
$gridField = new GridField('testfield', 'testfield', $list, $config);
$state = $gridField->State->GridFieldSortableHeader;
$compontent = $gridField->getConfig()->getComponentByType('GridFieldSortableHeader');
// Test that inherited dataobjects will work correctly
$state->SortColumn = 'Cheerleader.Hat.Colour';
$state->SortDirection = 'asc';
$relationListA = $compontent->getManipulatedData($gridField, $list);
$relationListAsql = Convert::nl2os($relationListA->sql(), ' ');
// Assert that all tables are joined properly
$this->assertContains('FROM "GridFieldSortableHeaderTest_Team"', $relationListAsql);
$this->assertContains(
'LEFT JOIN "GridFieldSortableHeaderTest_TeamGroup" '
. 'ON "GridFieldSortableHeaderTest_TeamGroup"."ID" = "GridFieldSortableHeaderTest_Team"."ID"',
$relationListAsql
);
$this->assertContains(
'LEFT JOIN "GridFieldSortableHeaderTest_Cheerleader" '
. 'ON "GridFieldSortableHeaderTest_Cheerleader"."ID" = "GridFieldSortableHeaderTest_Team"."CheerleaderID"',
$relationListAsql
);
$this->assertContains(
'LEFT JOIN "GridFieldSortableHeaderTest_CheerleaderHat" '
. 'ON "GridFieldSortableHeaderTest_CheerleaderHat"."ID" = "GridFieldSortableHeaderTest_Cheerleader"."HatID"', $relationListAsql);
// Test sorting is correct
$this->assertEquals(
array('Cologne', 'Auckland', 'Wellington', 'Melbourne'),
$relationListA->column('City')
);
$state->SortDirection = 'desc';
$relationListAdesc = $compontent->getManipulatedData($gridField, $list);
$this->assertEquals(
array('Melbourne', 'Wellington', 'Auckland', 'Cologne'),
$relationListAdesc->column('City')
);
// Test subclasses of tables
$state->SortColumn = 'CheerleadersMom.Hat.Colour';
$state->SortDirection = 'asc';
$relationListB = $compontent->getManipulatedData($gridField, $list);
$relationListBsql = $relationListB->sql();
// Assert that subclasses are included in the query
$this->assertContains('FROM "GridFieldSortableHeaderTest_Team"', $relationListBsql);
$this->assertContains(
'LEFT JOIN "GridFieldSortableHeaderTest_TeamGroup" '
. 'ON "GridFieldSortableHeaderTest_TeamGroup"."ID" = "GridFieldSortableHeaderTest_Team"."ID"',
$relationListBsql
);
$this->assertContains(
'LEFT JOIN "GridFieldSortableHeaderTest_Mom" '
. 'ON "GridFieldSortableHeaderTest_Mom"."ID" = "GridFieldSortableHeaderTest_Team"."CheerleadersMomID"',
$relationListBsql
);
// Note that cheerleader is no longer aliased, as it is an implicit join
$this->assertContains(
'LEFT JOIN "GridFieldSortableHeaderTest_Cheerleader" '
. 'ON "GridFieldSortableHeaderTest_Mom"."ID" = "GridFieldSortableHeaderTest_Cheerleader"."ID"',
$relationListBsql
);
$this->assertContains(
'LEFT JOIN "GridFieldSortableHeaderTest_CheerleaderHat" '
. 'ON "GridFieldSortableHeaderTest_CheerleaderHat"."ID" = "GridFieldSortableHeaderTest_Cheerleader"."HatID"',
$relationListBsql
);
// Test sorting is correct
$this->assertEquals(
array('Cologne', 'Auckland', 'Wellington', 'Melbourne'),
$relationListB->column('City')
);
$state->SortDirection = 'desc';
$relationListBdesc = $compontent->getManipulatedData($gridField, $list);
$this->assertEquals(
array('Melbourne', 'Wellington', 'Auckland', 'Cologne'),
$relationListBdesc->column('City')
);
}
}
class GridFieldSortableHeaderTest_Team extends DataObject implements TestOnly {
@ -119,11 +209,18 @@ class GridFieldSortableHeaderTest_Team extends DataObject implements TestOnly {
);
private static $has_one = array(
'Cheerleader' => 'GridFieldSortableHeaderTest_Cheerleader'
'Cheerleader' => 'GridFieldSortableHeaderTest_Cheerleader',
'CheerleadersMom' => 'GridFieldSortableHeaderTest_Mom'
);
}
class GridFieldSortableHeaderTest_TeamGroup extends GridFieldSortableHeaderTest_Team implements TestOnly {
private static $db = array(
'GroupName' => 'Varchar'
);
}
class GridFieldSortableHeaderTest_Cheerleader extends DataObject implements TestOnly {
private static $db = array(
@ -137,6 +234,15 @@ class GridFieldSortableHeaderTest_Cheerleader extends DataObject implements Test
}
/**
* Should have access to same properties as cheerleader
*/
class GridFieldSortableHeaderTest_Mom extends GridFieldSortableHeaderTest_Cheerleader implements TestOnly {
private static $db = array(
'NumberOfCookiesBaked' => 'Int'
);
}
class GridFieldSortableHeaderTest_CheerleaderHat extends DataObject implements TestOnly {
private static $db = array(

View File

@ -1,39 +1,76 @@
GridFieldSortableHeaderTest_CheerleaderHat:
hat1:
Colour: Blue
hat2:
Colour: Red
hat3:
Colour: Green
hat4:
Colour: Pink
hat1:
Colour: Blue
hat2:
Colour: Red
hat3:
Colour: Green
hat4:
Colour: Pink
GridFieldSortableHeaderTest_Cheerleader:
cheerleader1:
Name: Heather
Hat: =>GridFieldSortableHeaderTest_CheerleaderHat.hat2
cheerleader2:
Name: Bob
Hat: =>GridFieldSortableHeaderTest_CheerleaderHat.hat4
cheerleader3:
Name: Jenny
Hat: =>GridFieldSortableHeaderTest_CheerleaderHat.hat1
cheerleader4:
Name: Sam
Hat: =>GridFieldSortableHeaderTest_CheerleaderHat.hat3
cheerleader1:
Name: Heather
Hat: =>GridFieldSortableHeaderTest_CheerleaderHat.hat2
cheerleader2:
Name: Bob
Hat: =>GridFieldSortableHeaderTest_CheerleaderHat.hat4
cheerleader3:
Name: Jenny
Hat: =>GridFieldSortableHeaderTest_CheerleaderHat.hat1
cheerleader4:
Name: Sam
Hat: =>GridFieldSortableHeaderTest_CheerleaderHat.hat3
GridFieldSortableHeaderTest_Mom:
mom1:
Name: Ethel
Hat: =>GridFieldSortableHeaderTest_CheerleaderHat.hat2
mom2:
Name: Eileene
Hat: =>GridFieldSortableHeaderTest_CheerleaderHat.hat4
mom3:
Name: Gertrude
Hat: =>GridFieldSortableHeaderTest_CheerleaderHat.hat1
mom4:
Name: Andrew
Hat: =>GridFieldSortableHeaderTest_CheerleaderHat.hat3
GridFieldSortableHeaderTest_Team:
team1:
Name: Team 1
City: Cologne
Cheerleader: =>GridFieldSortableHeaderTest_Cheerleader.cheerleader3
team2:
Name: Team 2
City: Wellington
Cheerleader: =>GridFieldSortableHeaderTest_Cheerleader.cheerleader2
team3:
Name: Team 3
City: Auckland
Cheerleader: =>GridFieldSortableHeaderTest_Cheerleader.cheerleader4
team4:
Name: Team 4
City: Melbourne
Cheerleader: =>GridFieldSortableHeaderTest_Cheerleader.cheerleader1
team1:
Name: Team 1
City: Cologne
Cheerleader: =>GridFieldSortableHeaderTest_Cheerleader.cheerleader3
team2:
Name: Team 2
City: Wellington
Cheerleader: =>GridFieldSortableHeaderTest_Cheerleader.cheerleader2
team3:
Name: Team 3
City: Auckland
Cheerleader: =>GridFieldSortableHeaderTest_Cheerleader.cheerleader4
team4:
Name: Team 4
City: Melbourne
Cheerleader: =>GridFieldSortableHeaderTest_Cheerleader.cheerleader1
GridFieldSortableHeaderTest_TeamGroup:
group1:
Name: Group 1
City: Cologne
Cheerleader: =>GridFieldSortableHeaderTest_Cheerleader.cheerleader3
CheerleadersMom: =>GridFieldSortableHeaderTest_Mom.mom3
group2:
Name: Group 2
City: Wellington
Cheerleader: =>GridFieldSortableHeaderTest_Cheerleader.cheerleader2
CheerleadersMom: =>GridFieldSortableHeaderTest_Mom.mom2
group3:
Name: Group 3
City: Auckland
Cheerleader: =>GridFieldSortableHeaderTest_Cheerleader.cheerleader4
CheerleadersMom: =>GridFieldSortableHeaderTest_Mom.mom4
group4:
Name: Group 4
City: Melbourne
Cheerleader: =>GridFieldSortableHeaderTest_Cheerleader.cheerleader1
CheerleadersMom: =>GridFieldSortableHeaderTest_Mom.mom1

View File

@ -521,6 +521,15 @@ class DataListTest extends SapphireTest {
$this->assertEquals('Phil', $list->last()->Name, 'Last comment should be from Phil');
}
public function testSortWithCompositeSyntax() {
// Phil commented on team with founder surname "Aaron"
$list = DataObjectTest_TeamComment::get();
$list = $list->sort('Team.Founder.Surname', 'asc');
$this->assertEquals('Phil', $list->first()->Name);
$list = $list->sort('Team.Founder.Surname', 'desc');
$this->assertEquals('Phil', $list->last()->Name);
}
/**
* $list->filter('Name', 'bob'); // only bob in the list
*/

View File

@ -830,11 +830,12 @@ class DataObjectTest extends SapphireTest {
'DatabaseField',
'ExtendedDatabaseField',
'CaptainID',
'FounderID',
'HasOneRelationshipID',
'ExtendedHasOneRelationshipID'
),
array_keys($teamInstance->db()),
'db() contains all fields defined on instance: base, extended and foreign keys'
array_keys($teamInstance->inheritedDatabaseFields()),
'inheritedDatabaseFields() contains all fields defined on instance: base, extended and foreign keys'
);
$this->assertEquals(
@ -847,11 +848,12 @@ class DataObjectTest extends SapphireTest {
'DatabaseField',
'ExtendedDatabaseField',
'CaptainID',
'FounderID',
'HasOneRelationshipID',
'ExtendedHasOneRelationshipID'
),
array_keys(DataObjectTest_Team::database_fields()),
'database_fields() contains only fields defined on instance, including base, extended and foreign keys'
array_keys(DataObject::database_fields('DataObjectTest_Team', false)),
'databaseFields() contains only fields defined on instance, including base, extended and foreign keys'
);
$this->assertEquals(
@ -864,6 +866,7 @@ class DataObjectTest extends SapphireTest {
'DatabaseField',
'ExtendedDatabaseField',
'CaptainID',
'FounderID',
'HasOneRelationshipID',
'ExtendedHasOneRelationshipID',
'SubclassDatabaseField',
@ -1706,7 +1709,9 @@ class DataObjectTest_Player extends Member implements TestOnly {
);
private static $has_many = array(
'Fans' => 'DataObjectTest_Fan.Favourite' // Polymorphic - Player fans
'Fans' => 'DataObjectTest_Fan.Favourite', // Polymorphic - Player fans
'CaptainTeams' => 'DataObjectTest_Team.Captain',
'FoundingTeams' => 'DataObjectTest_Team.Founder'
);
private static $belongs_to = array (
@ -1728,6 +1733,7 @@ class DataObjectTest_Team extends DataObject implements TestOnly {
private static $has_one = array(
"Captain" => 'DataObjectTest_Player',
"Founder" => 'DataObjectTest_Player',
'HasOneRelationship' => 'DataObjectTest_Player',
);

View File

@ -20,13 +20,17 @@ DataObjectTest_Team:
DataObjectTest_Player:
captain1:
FirstName: Captain
Surname: Zookeeper
ShirtNumber: 007
FavouriteTeam: =>DataObjectTest_Team.team1
Teams: =>DataObjectTest_Team.team1
FoundingTeams: =>DataObjectTest_Team.team1
IsRetired: 1
captain2:
FirstName: Captain 2
Surname: Aaron
Teams: =>DataObjectTest_Team.team2
FoundingTeams: =>DataObjectTest_Team.team2
player1:
FirstName: Player 1
player2: