Skip to content

Commit

Permalink
FIX: Sorting a DataQuery over a relation.
Browse files Browse the repository at this point in the history
When sorting a DataQuery over a relation, the SQLQuery automatically included the sort column. The issue with the implement is that potentially the joined record has a field with the same name as the source record causing it to be overridden.

In the attached test case, without the patch the title will be set to 'Bar' rather than 'Foo'.

This patch aliases the sort column. Alternativally a patch would be to
  • Loading branch information
Will Rossiter committed Aug 26, 2014
1 parent 70dfc55 commit 7993875
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 6 deletions.
19 changes: 14 additions & 5 deletions model/DataQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ protected function ensureSelectContainsOrderbyColumns($query, $originalSelect =

if($orderby = $query->getOrderBy()) {
$newOrderby = array();
$i = 0;
foreach($orderby as $k => $dir) {
$newOrderby[$k] = $dir;

Expand All @@ -268,7 +269,10 @@ protected function ensureSelectContainsOrderbyColumns($query, $originalSelect =

// Pull through SortColumn references from the originalSelect variables
if(preg_match('/_SortColumn/', $col)) {
if(isset($originalSelect[$col])) $query->selectField($originalSelect[$col], $col);
if(isset($originalSelect[$col])) {
$query->selectField($originalSelect[$col], $col);
}

continue;
}

Expand All @@ -287,6 +291,7 @@ protected function ensureSelectContainsOrderbyColumns($query, $originalSelect =

// remove original sort
unset($newOrderby[$k]);

// add new columns sort
$newOrderby[$qualCol] = $dir;

Expand All @@ -298,13 +303,17 @@ protected function ensureSelectContainsOrderbyColumns($query, $originalSelect =
}
} else {
$qualCol = '"' . implode('"."', $parts) . '"';

// To-do: Remove this if block once SQLQuery::$select has been refactored to store getSelect()
// format internally; then this check can be part of selectField()

if(!in_array($qualCol, $query->getSelect())) {
$query->selectField($qualCol);
unset($newOrderby[$k]);

$newOrderby["\"_SortColumn$i\""] = $dir;
$query->selectField($qualCol, "_SortColumn$i");

$i++;
}
}

}

$query->setOrderBy($newOrderby);
Expand Down
25 changes: 24 additions & 1 deletion tests/model/DataQueryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,27 @@ class DataQueryTest extends SapphireTest {
'DataQueryTest_E',
);


public function testSortByJoinedFieldRetainsSourceInformation() {
$bar = new DataQueryTest_C();
$bar->Title = "Bar";
$bar->write();

$foo = new DataQueryTest_B();
$foo->Title = "Foo";
$foo->TestC = $bar->ID;
$foo->write();

$query = new DataQuery('DataQueryTest_B');
$result = $query->leftJoin(
'DataQueryTest_C',
"\"DataQueryTest_B\".\"TestCID\" = \"DataQueryTest_B\".\"ID\""
)->sort('"DataQueryTest_B"."Title"', 'ASC');

$result = $result->execute()->record();
$this->assertEquals('Foo', $result['Title']);
}

/**
* Test the leftJoin() and innerJoin method of the DataQuery object
*/
Expand Down Expand Up @@ -138,6 +159,7 @@ public function testDefaultSort() {


class DataQueryTest_A extends DataObject implements TestOnly {

private static $db = array(
'Name' => 'Varchar',
);
Expand All @@ -147,7 +169,8 @@ class DataQueryTest_A extends DataObject implements TestOnly {
);
}

class DataQueryTest_B extends DataQueryTest_A {
class DataQueryTest_B extends DataObject implements TestOnly {

private static $db = array(
'Title' => 'Varchar',
);
Expand Down

0 comments on commit 7993875

Please sign in to comment.