diff --git a/docs/en/02_Developer_Guides/01_Templates/03_Requirements.md b/docs/en/02_Developer_Guides/01_Templates/03_Requirements.md index 4d82a42dc..9849e07f8 100644 --- a/docs/en/02_Developer_Guides/01_Templates/03_Requirements.md +++ b/docs/en/02_Developer_Guides/01_Templates/03_Requirements.md @@ -206,6 +206,9 @@ the third paramter of the `combine_files` function: Requirements::combine_files('print.css', $printStylesheets, 'print'); +By default, all requirements files are flushed (deleted) when ?flush querystring parameter is set. +This can be disabled by setting the `Requirements.disable_flush_combined` config to `true`. +
When combining CSS files, take care of relative urls, as these will not be re-written to match the destination location of the resulting combined CSS. diff --git a/model/DataQuery.php b/model/DataQuery.php index f3b14294c..55f25e073 100644 --- a/model/DataQuery.php +++ b/model/DataQuery.php @@ -805,10 +805,12 @@ class DataQuery { $relationTable, "\"$relationTable\".\"$parentField\" = \"$parentBaseClass\".\"ID\"" ); - $this->query->addLeftJoin( - $componentBaseClass, - "\"$relationTable\".\"$componentField\" = \"$componentBaseClass\".\"ID\"" - ); + if (!$this->query->isJoinedTo($componentBaseClass)) { + $this->query->addLeftJoin( + $componentBaseClass, + "\"$relationTable\".\"$componentField\" = \"$componentBaseClass\".\"ID\"" + ); + } /** * add join clause to the component's ancestry classes so that the search filter could search on @@ -817,7 +819,7 @@ class DataQuery { $ancestry = ClassInfo::ancestry($componentClass, true); $ancestry = array_reverse($ancestry); foreach($ancestry as $ancestor){ - if($ancestor != $componentBaseClass){ + if($ancestor != $componentBaseClass && !$this->query->isJoinedTo($ancestor)){ $this->query->addInnerJoin($ancestor, "\"$componentBaseClass\".\"ID\" = \"$ancestor\".\"ID\""); } } diff --git a/tests/model/DataListTest.php b/tests/model/DataListTest.php index a0d84f4ae..90f9c74f5 100755 --- a/tests/model/DataListTest.php +++ b/tests/model/DataListTest.php @@ -32,6 +32,8 @@ class DataListTest extends SapphireTest { 'DataObjectTest_Play', 'DataObjectTest_Ploy', 'DataObjectTest_Bogey', + 'ManyManyListTest_Product', + 'ManyManyListTest_Category', ); public function testFilterDataObjectByCreatedDate() { diff --git a/tests/model/DataObjectLazyLoadingTest.php b/tests/model/DataObjectLazyLoadingTest.php index 8bf2068d1..94642fd9d 100644 --- a/tests/model/DataObjectLazyLoadingTest.php +++ b/tests/model/DataObjectLazyLoadingTest.php @@ -35,6 +35,9 @@ class DataObjectLazyLoadingTest extends SapphireTest { 'DataObjectTest_Play', 'DataObjectTest_Ploy', 'DataObjectTest_Bogey', + // From ManyManyListTest + 'ManyManyListTest_Product', + 'ManyManyListTest_Category', // From VersionedTest 'VersionedTest_DataObject', 'VersionedTest_Subclass', diff --git a/tests/model/DataObjectTest.php b/tests/model/DataObjectTest.php index 01cceec44..03720fc49 100644 --- a/tests/model/DataObjectTest.php +++ b/tests/model/DataObjectTest.php @@ -32,6 +32,8 @@ class DataObjectTest extends SapphireTest { 'DataObjectTest_Play', 'DataObjectTest_Ploy', 'DataObjectTest_Bogey', + 'ManyManyListTest_Product', + 'ManyManyListTest_Category', ); public function testDb() { diff --git a/tests/model/DataObjectTest.yml b/tests/model/DataObjectTest.yml index 70292f8c7..87144e226 100644 --- a/tests/model/DataObjectTest.yml +++ b/tests/model/DataObjectTest.yml @@ -63,6 +63,28 @@ DataObjectTest_TeamComment: Name: Phil Comment: Phil is a unique guy, and comments on team2 Team: =>DataObjectTest_Team.team2 +ManyManyListTest_Product: + producta: + Title: 'Product A' + productb: + Title: 'Product B' + RelatedProducts: =>ManyManyListTest_Product.producta +ManyManyListTest_Category: + categorya: + Title: 'Category A' + Products: =>ManyManyListTest_Product.producta,=>ManyManyListTest_Product.productb + comment1: + Name: Joe + Comment: This is a team comment by Joe + Team: =>DataObjectTest_Team.team1 + comment2: + Name: Bob + Comment: This is a team comment by Bob + Team: =>DataObjectTest_Team.team1 + comment3: + Name: Phil + Comment: Phil is a unique guy, and comments on team2 + Team: =>DataObjectTest_Team.team2 DataObjectTest_Fan: fan1: Name: Damian diff --git a/tests/model/DataQueryTest.php b/tests/model/DataQueryTest.php index 685f375e2..958f9c8f9 100644 --- a/tests/model/DataQueryTest.php +++ b/tests/model/DataQueryTest.php @@ -11,6 +11,7 @@ class DataQueryTest extends SapphireTest { 'DataQueryTest_D', 'DataQueryTest_E', 'DataQueryTest_F', + 'DataQueryTest_G', ); @@ -63,11 +64,39 @@ class DataQueryTest extends SapphireTest { } public function testApplyReplationDeepInheretence() { + //test has_one relation $newDQ = new DataQuery('DataQueryTest_E'); //apply a relation to a relation from an ancestor class $newDQ->applyRelation('TestA'); $this->assertTrue($newDQ->query()->isJoinedTo('DataQueryTest_C')); $this->assertContains('"DataQueryTest_A"."ID" = "DataQueryTest_C"."TestAID"', $newDQ->sql($params)); + + //test many_many relation + + //test many_many with separate inheritance + $newDQ = new DataQuery('DataQueryTest_C'); + $baseDBTable = ClassInfo::baseDataClass('DataQueryTest_C'); + $newDQ->applyRelation('ManyTestAs'); + //check we are "joined" to the DataObject's table (there is no distinction between FROM or JOIN clauses) + $this->assertTrue($newDQ->query()->isJoinedTo($baseDBTable)); + //check we are explicitly selecting "FROM" the DO's table + $this->assertContains("FROM \"$baseDBTable\"", $newDQ->sql()); + + //test many_many with shared inheritance + $newDQ = new DataQuery('DataQueryTest_E'); + $baseDBTable = ClassInfo::baseDataClass('DataQueryTest_E'); + //check we are "joined" to the DataObject's table (there is no distinction between FROM or JOIN clauses) + $this->assertTrue($newDQ->query()->isJoinedTo($baseDBTable)); + //check we are explicitly selecting "FROM" the DO's table + $this->assertContains("FROM \"$baseDBTable\"", $newDQ->sql(), 'The FROM clause is missing from the query'); + $newDQ->applyRelation('ManyTestGs'); + //confirm we are still joined to the base table + $this->assertTrue($newDQ->query()->isJoinedTo($baseDBTable)); + //double check it is the "FROM" clause + $this->assertContains("FROM \"$baseDBTable\"", $newDQ->sql(), 'The FROM clause has been removed from the query'); + //another (potentially less crude check) for checking "FROM" clause + $fromTables = $newDQ->query()->getFrom(); + $this->assertEquals('"' . $baseDBTable . '"', $fromTables[$baseDBTable]); } public function testRelationReturn() { @@ -310,6 +339,10 @@ class DataQueryTest_E extends DataQueryTest_C implements TestOnly { 'SortOrder' => 'Int' ); + private static $many_many = array( + 'ManyTestGs' => 'DataQueryTest_G', + ); + private static $default_sort = '"DataQueryTest_E"."SortOrder" ASC'; } @@ -321,3 +354,11 @@ class DataQueryTest_F extends DataObject implements TestOnly { 'MyString' => 'Text' ); } + +class DataQueryTest_G extends DataQueryTest_C implements TestOnly { + + private static $belongs_many_many = array( + 'ManyTestEs' => 'DataQueryTest_E', + ); + +} diff --git a/tests/model/HasManyListTest.php b/tests/model/HasManyListTest.php index 35bc8cd63..06f3a49df 100644 --- a/tests/model/HasManyListTest.php +++ b/tests/model/HasManyListTest.php @@ -28,6 +28,8 @@ class HasManyListTest extends SapphireTest { 'DataObjectTest_Play', 'DataObjectTest_Ploy', 'DataObjectTest_Bogey', + 'ManyManyListTest_Product', + 'ManyManyListTest_Category', ); public function testRelationshipEmptyOnNewRecords() { diff --git a/tests/model/ManyManyListTest.php b/tests/model/ManyManyListTest.php index eaa60c3f2..d71641dca 100644 --- a/tests/model/ManyManyListTest.php +++ b/tests/model/ManyManyListTest.php @@ -34,7 +34,9 @@ class ManyManyListTest extends SapphireTest { 'DataObjectTest_Ploy', 'DataObjectTest_Bogey', // From ManyManyListTest - 'ManyManyListTest_ExtraFields' + 'ManyManyListTest_ExtraFields', + 'ManyManyListTest_Product', + 'ManyManyListTest_Category', ); @@ -287,6 +289,17 @@ class ManyManyListTest extends SapphireTest { $this->assertSQLEquals($expected, $list->sql($parameters)); } + public function testFilteringOnPreviouslyJoinedTable() { + + /** @var ManyManyListTest_Category $category */ + $category = $this->objFromFixture('ManyManyListTest_Category', 'categorya'); + + /** @var ManyManyList $productsRelatedToProductB */ + $productsRelatedToProductB = $category->Products()->filter('RelatedProducts.Title', 'Product B'); + + $this->assertEquals(1, $productsRelatedToProductB->count()); + } + } @@ -311,3 +324,33 @@ class ManyManyListTest_ExtraFields extends DataObject implements TestOnly { ) ); } + +class ManyManyListTest_Product extends DataObject implements TestOnly { + + private static $db = array( + 'Title' => 'Varchar' + ); + + private static $many_many = array( + 'RelatedProducts' => 'ManyManyListTest_Product' + ); + + private static $belongs_many_many = array( + 'RelatedTo' => 'ManyManyListTest_Product', + 'Categories' => 'ManyManyListTest_Category' + ); + +} + +class ManyManyListTest_Category extends DataObject implements TestOnly { + + private static $db = array( + 'Title' => 'Varchar' + ); + + private static $many_many = array( + 'Products' => 'ManyManyListTest_Product' + ); + +} + diff --git a/tests/model/MapTest.php b/tests/model/MapTest.php index 7590450f0..2f229c0fe 100755 --- a/tests/model/MapTest.php +++ b/tests/model/MapTest.php @@ -5,7 +5,7 @@ * @subpackage tests */ class SS_MapTest extends SapphireTest { - + // Borrow the model from DataObjectTest protected static $fixture_file = 'DataObjectTest.yml'; @@ -32,8 +32,10 @@ class SS_MapTest extends SapphireTest { 'DataObjectTest_Play', 'DataObjectTest_Ploy', 'DataObjectTest_Bogey', + 'ManyManyListTest_Product', + 'ManyManyListTest_Category', ); - + public function testValues() { $list = DataObjectTest_TeamComment::get()->sort('Name'); @@ -89,13 +91,13 @@ class SS_MapTest extends SapphireTest { . "Bob: This is a team comment by Bob\n" . "Phil: Phil is a unique guy, and comments on team2\n", $text); } - + public function testDefaultConfigIsIDAndTitle() { $list = DataObjectTest_Team::get(); $map = new SS_Map($list); $this->assertEquals('Team 1', $map[$this->idFromFixture('DataObjectTest_Team', 'team1')]); } - + public function testSetKeyFieldAndValueField() { $list = DataObjectTest_TeamComment::get(); $map = new SS_Map($list); @@ -103,7 +105,7 @@ class SS_MapTest extends SapphireTest { $map->setValueField('Comment'); $this->assertEquals('This is a team comment by Joe', $map['Joe']); } - + public function testToArray() { $list = DataObjectTest_TeamComment::get(); $map = new SS_Map($list, 'Name', 'Comment'); @@ -181,10 +183,10 @@ class SS_MapTest extends SapphireTest { "Phil" => "Phil is a unique guy, and comments on team2"), $map->toArray()); $map->unshift(0, '(Select)'); - + $this->assertEquals('(All)', $map[-1]); $this->assertEquals('(Select)', $map[0]); - + $this->assertEquals(array( 0 => "(Select)", -1 => "(All)", @@ -230,7 +232,7 @@ class SS_MapTest extends SapphireTest { 1 => "(All)" ), $map->toArray()); } - + public function testCount() { $list = DataObjectTest_TeamComment::get(); $map = new SS_Map($list, 'Name', 'Comment'); @@ -275,7 +277,7 @@ class SS_MapTest extends SapphireTest { $list = DataObjectTest_TeamComment::get()->sort('ID'); $map = new SS_Map($list, 'Name', 'Comment'); $map->push(1, 'Pushed'); - + $text = ""; foreach($map as $k => $v) { @@ -298,7 +300,7 @@ class SS_MapTest extends SapphireTest { foreach($map as $k => $v) { $text .= "$k: $v\n"; } - + $this->assertEquals("1: unshifted\n", $text); } @@ -311,7 +313,7 @@ class SS_MapTest extends SapphireTest { foreach($map as $k => $v) { $text .= "$k: $v\n"; } - + $this->assertEquals("1: pushed\n", $text); } } diff --git a/tests/model/PaginatedListTest.php b/tests/model/PaginatedListTest.php index eb9ac4d41..34abb090d 100644 --- a/tests/model/PaginatedListTest.php +++ b/tests/model/PaginatedListTest.php @@ -32,6 +32,8 @@ class PaginatedListTest extends SapphireTest { 'DataObjectTest_Play', 'DataObjectTest_Ploy', 'DataObjectTest_Bogey', + 'ManyManyListTest_Product', + 'ManyManyListTest_Category', ); public function testPageStart() { diff --git a/view/Requirements.php b/view/Requirements.php index 245436b41..5f27dd2ce 100644 --- a/view/Requirements.php +++ b/view/Requirements.php @@ -27,8 +27,8 @@ class Requirements implements Flushable { public static function flush() { $disabled = Config::inst()->get(static::class, 'disable_flush_combined'); if(!$disabled) { - self::delete_all_combined_files(); - } + self::delete_all_combined_files(); + } } /**