diff --git a/model/DataQuery.php b/model/DataQuery.php index 9ccb5912b..9d6e50e6b 100644 --- a/model/DataQuery.php +++ b/model/DataQuery.php @@ -763,14 +763,14 @@ class DataQuery { } /** - * Select the given fields from the given table. + * Select only the given fields from the given table. * * @param String $table Unquoted table name (will be escaped automatically) * @param Array $fields Database column names (will be escaped automatically) */ public function selectFromTable($table, $fields) { $fieldExpressions = array_map(function($item) use($table) { - return "\"$table\".\"$item\""; + return Convert::symbol2sql("{$table}.{$item}"); }, $fields); $this->query->setSelect($fieldExpressions); @@ -778,6 +778,22 @@ class DataQuery { return $this; } + /** + * Add the given fields from the given table to the select statement. + * + * @param String $table Unquoted table name (will be escaped automatically) + * @param Array $fields Database column names (will be escaped automatically) + */ + public function addSelectFromTable($table, $fields) { + $fieldExpressions = array_map(function($item) use($table) { + return Convert::symbol2sql("{$table}.{$item}"); + }, $fields); + + $this->query->addSelect($fieldExpressions); + + return $this; + } + /** * Query the given field column from the database and return as an array. * diff --git a/model/ManyManyList.php b/model/ManyManyList.php index e5b041195..bf586edc7 100644 --- a/model/ManyManyList.php +++ b/model/ManyManyList.php @@ -104,7 +104,7 @@ class ManyManyList extends RelationList { } } - $this->dataQuery->selectFromTable($this->joinTable, $finalized); + $this->dataQuery->addSelectFromTable($this->joinTable, $finalized); } /** @@ -352,7 +352,7 @@ class ManyManyList extends RelationList { */ public function getExtraData($componentName, $itemID) { $result = array(); - + // Skip if no extrafields or unsaved record if(empty($this->extraFields) || empty($itemID)) { return $result; diff --git a/tests/model/ManyManyListTest.php b/tests/model/ManyManyListTest.php index f912ba753..7990a8b51 100644 --- a/tests/model/ManyManyListTest.php +++ b/tests/model/ManyManyListTest.php @@ -11,6 +11,10 @@ class ManyManyListTest extends SapphireTest { protected $extraDataObjects = array( 'DataObjectTest_Team', 'DataObjectTest_SubTeam', + 'DataObjectTest_Fan', + 'DataObjectTest_Sortable', + 'DataObjectTest_EquipmentCompany', + 'DataObjectTest_SubEquipmentCompany', 'DataObjectTest_Player', 'DataObjectTest_Company', 'DataObjectTest_TeamComment', @@ -41,6 +45,28 @@ class ManyManyListTest extends SapphireTest { $this->assertEquals(100, $check->Worth->getAmount()); } + /** + * This test targets a bug where appending many_many_extraFields to a query would + * result in erroneous queries for sort orders that rely on _SortColumn0 + */ + public function testAddCompositedExtraFieldsWithSortColumn0() { + $obj = new ManyManyListTest_ExtraFields(); + $obj->write(); + + $product = new ManyManyListTest_Product(); + $product->Title = 'Test Product'; + $product->write(); + + // the actual test is that this does not generate an error in the sql. + $obj->Products()->add($product, array( + 'Reference' => 'Foo' + )); + + $result = $obj->Products()->First(); + $this->assertEquals('Foo', $result->Reference, 'Basic scalar fields should exist'); + $this->assertEquals('Test Product', $result->Title); + } + public function testCreateList() { $list = ManyManyList::create('DataObjectTest_Team','DataObjectTest_Team_Players', 'DataObjectTest_TeamID', 'DataObjectTest_PlayerID'); @@ -290,7 +316,8 @@ class ManyManyListTest extends SapphireTest { class ManyManyListTest_ExtraFields extends DataObject implements TestOnly { private static $many_many = array( - 'Clients' => 'ManyManyListTest_ExtraFields' + 'Clients' => 'ManyManyListTest_ExtraFields', + 'Products' => 'ManyManyListTest_Product' ); private static $belongs_many_many = array( @@ -301,6 +328,9 @@ class ManyManyListTest_ExtraFields extends DataObject implements TestOnly { 'Clients' => array( 'Reference' => 'Varchar', 'Worth' => 'Money' + ), + 'Products' => array( + 'Reference' => 'Varchar' ) ); } @@ -320,6 +350,8 @@ class ManyManyListTest_Product extends DataObject implements TestOnly { 'Categories' => 'ManyManyListTest_Category' ); + private static $default_sort = '"Title" IS NOT NULL ASC, "Title" ASC'; + } class ManyManyListTest_Category extends DataObject implements TestOnly {