From 7041c5945c247c1e104507bfe5751d24543aa38c Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 12 May 2016 12:46:30 +1200 Subject: [PATCH 1/4] API Enable requirements to persist between flushes --- .../01_Templates/03_Requirements.md | 3 +++ view/Requirements.php | 16 +++++++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) 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 fdb6085bd..78ca13e64 100644 --- a/docs/en/02_Developer_Guides/01_Templates/03_Requirements.md +++ b/docs/en/02_Developer_Guides/01_Templates/03_Requirements.md @@ -130,6 +130,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`. + ## Clearing assets :::php diff --git a/view/Requirements.php b/view/Requirements.php index 76cad5ffd..0c5a316b3 100644 --- a/view/Requirements.php +++ b/view/Requirements.php @@ -8,11 +8,25 @@ */ class Requirements implements Flushable { + /** + * Flag whether combined files should be deleted on flush. + * + * By default all combined files are deleted on flush. If combined files are stored in source control, + * and thus updated manually, you might want to turn this on to disable this behaviour. + * + * @config + * @var bool + */ + private static $disable_flush_combined = false; + /** * Triggered early in the request when a flush is requested */ public static function flush() { - self::delete_all_combined_files(); + $disabled = Config::inst()->get(__CLASS__, 'disable_flush_combined'); + if(!$disabled) { + self::delete_all_combined_files(); + } } /** From dd554d883f677a39f38093bb089c98adef1593ef Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Fri, 30 Jan 2015 13:47:02 +0000 Subject: [PATCH 2/4] Proving bug with Deep nested many_many relations When adding a filter to a many_many with a shared inheritance, the FROM table is removed and added as a LEFT JOIN which causes a syntax error. This means `$dataList->filter('ManyManyRel.ID', array(1,2))` doesn't work. --- model/DataQuery.php | 6 +++-- tests/model/DataQueryTest.php | 43 ++++++++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/model/DataQuery.php b/model/DataQuery.php index 625848e3f..8fc3b8cc2 100644 --- a/model/DataQuery.php +++ b/model/DataQuery.php @@ -657,8 +657,10 @@ class DataQuery { $componentBaseClass = ClassInfo::baseDataClass($componentClass); $this->query->addInnerJoin($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\""); + } if(ClassInfo::hasTable($componentClass)) { $this->query->addLeftJoin($componentClass, "\"$relationTable\".\"$componentField\" = \"$componentClass\".\"ID\""); diff --git a/tests/model/DataQueryTest.php b/tests/model/DataQueryTest.php index 4820f8d80..570b5634c 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()); + + //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() { @@ -305,7 +334,11 @@ class DataQueryTest_E extends DataQueryTest_C implements TestOnly { private static $db = array( 'SortOrder' => 'Int' ); - + + private static $many_many = array( + 'ManyTestGs' => 'DataQueryTest_G', + ); + private static $default_sort = '"DataQueryTest_E"."SortOrder" ASC'; } @@ -317,3 +350,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', + ); + +} From 5b275376d38b36218b30e1a88d3b3df29f38f8c2 Mon Sep 17 00:00:00 2001 From: Loz Calver Date: Tue, 10 May 2016 15:56:15 +0100 Subject: [PATCH 3/4] Many many bug --- tests/model/DataListTest.php | 2 + tests/model/DataObjectLazyLoadingTest.php | 6 +- tests/model/DataObjectTest.php | 4 +- tests/model/DataObjectTest.yml | 96 +++++++++++++---------- tests/model/HasManyListTest.php | 2 + tests/model/ManyManyListTest.php | 43 ++++++++++ tests/model/MapTest.php | 4 +- tests/model/PaginatedListTest.php | 4 +- 8 files changed, 113 insertions(+), 48 deletions(-) diff --git a/tests/model/DataListTest.php b/tests/model/DataListTest.php index a30bc2209..f3f7da49f 100755 --- a/tests/model/DataListTest.php +++ b/tests/model/DataListTest.php @@ -20,6 +20,8 @@ class DataListTest extends SapphireTest { 'DataObjectTest_Player', 'DataObjectTest_TeamComment', 'DataObjectTest\NamespacedClass', + 'ManyManyListTest_Product', + 'ManyManyListTest_Category', ); public function testFilterDataObjectByCreatedDate() { diff --git a/tests/model/DataObjectLazyLoadingTest.php b/tests/model/DataObjectLazyLoadingTest.php index 83bc8263b..63342c27b 100644 --- a/tests/model/DataObjectLazyLoadingTest.php +++ b/tests/model/DataObjectLazyLoadingTest.php @@ -23,7 +23,9 @@ class DataObjectLazyLoadingTest extends SapphireTest { 'DataObjectTest_Player', 'DataObjectTest_TeamComment', 'VersionedTest_DataObject', - 'VersionedTest_Subclass' + 'VersionedTest_Subclass', + 'ManyManyListTest_Product', + 'ManyManyListTest_Category', ); public function testQueriedColumnsID() { @@ -412,4 +414,4 @@ class VersionedLazySub_DataObject extends VersionedLazy_DataObject { private static $extensions = array( "Versioned('Stage', 'Live')" ); -} \ No newline at end of file +} diff --git a/tests/model/DataObjectTest.php b/tests/model/DataObjectTest.php index e967e5fad..87ca02874 100644 --- a/tests/model/DataObjectTest.php +++ b/tests/model/DataObjectTest.php @@ -17,7 +17,9 @@ class DataObjectTest extends SapphireTest { 'DataObjectTest_ValidatedObject', 'DataObjectTest_Player', 'DataObjectTest_TeamComment', - 'DataObjectTest_ExtendedTeamComment' + 'DataObjectTest_ExtendedTeamComment', + 'ManyManyListTest_Product', + 'ManyManyListTest_Category', ); public function testDb() { diff --git a/tests/model/DataObjectTest.yml b/tests/model/DataObjectTest.yml index 309e46511..6acc43602 100644 --- a/tests/model/DataObjectTest.yml +++ b/tests/model/DataObjectTest.yml @@ -1,47 +1,57 @@ DataObjectTest_Team: - team1: - Title: Team 1 - team2: - Title: Team 2 - team3: - Title: Team 3 + team1: + Title: Team 1 + team2: + Title: Team 2 + team3: + Title: Team 3 DataObjectTest_Player: - captain1: - FirstName: Captain - ShirtNumber: 007 - FavouriteTeam: =>DataObjectTest_Team.team1 - Teams: =>DataObjectTest_Team.team1 - IsRetired: 1 - captain2: - FirstName: Captain 2 - Teams: =>DataObjectTest_Team.team2 - player1: - FirstName: Player 1 - player2: - FirstName: Player 2 - Teams: =>DataObjectTest_Team.team1,=>DataObjectTest_Team.team2 + captain1: + FirstName: Captain + ShirtNumber: 007 + FavouriteTeam: =>DataObjectTest_Team.team1 + Teams: =>DataObjectTest_Team.team1 + IsRetired: 1 + captain2: + FirstName: Captain 2 + Teams: =>DataObjectTest_Team.team2 + player1: + FirstName: Player 1 + player2: + FirstName: Player 2 + Teams: =>DataObjectTest_Team.team1,=>DataObjectTest_Team.team2 DataObjectTest_SubTeam: - subteam1: - Title: Subteam 1 - SubclassDatabaseField: Subclassed 1 - ExtendedDatabaseField: Extended 1 - ParentTeam: =>DataObjectTest_Team.team1 - subteam2_with_player_relation: - Title: Subteam 2 - SubclassDatabaseField: Subclassed 2 - ExtendedHasOneRelationship: =>DataObjectTest_Player.player1 - subteam3_with_empty_fields: - Title: Subteam 3 + subteam1: + Title: Subteam 1 + SubclassDatabaseField: Subclassed 1 + ExtendedDatabaseField: Extended 1 + ParentTeam: =>DataObjectTest_Team.team1 + subteam2_with_player_relation: + Title: Subteam 2 + SubclassDatabaseField: Subclassed 2 + ExtendedHasOneRelationship: =>DataObjectTest_Player.player1 + subteam3_with_empty_fields: + Title: Subteam 3 DataObjectTest_TeamComment: - 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 + 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 +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 diff --git a/tests/model/HasManyListTest.php b/tests/model/HasManyListTest.php index ccce52611..705ffa43b 100644 --- a/tests/model/HasManyListTest.php +++ b/tests/model/HasManyListTest.php @@ -9,6 +9,8 @@ class HasManyListTest extends SapphireTest { 'DataObjectTest_Team', 'DataObjectTest_SubTeam', 'DataObjectTest_Player', + 'ManyManyListTest_Product', + 'ManyManyListTest_Category', ); public function testRelationshipEmptyOnNewRecords() { diff --git a/tests/model/ManyManyListTest.php b/tests/model/ManyManyListTest.php index 6a676d9b8..0e48c6615 100644 --- a/tests/model/ManyManyListTest.php +++ b/tests/model/ManyManyListTest.php @@ -15,6 +15,8 @@ class ManyManyListTest extends SapphireTest { 'DataObjectTest_Company', 'DataObjectTest_TeamComment', 'ManyManyListTest_ExtraFields', + 'ManyManyListTest_Product', + 'ManyManyListTest_Category', ); @@ -267,6 +269,17 @@ class ManyManyListTest extends SapphireTest { $this->assertEquals($expected, $list->sql()); } + 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()); + } + } @@ -291,3 +304,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 e58bcfa58..119c39210 100755 --- a/tests/model/MapTest.php +++ b/tests/model/MapTest.php @@ -18,7 +18,9 @@ class SS_MapTest extends SapphireTest { 'DataObjectTest_FieldlessSubTable', 'DataObjectTest_ValidatedObject', 'DataObjectTest_Player', - 'DataObjectTest_TeamComment' + 'DataObjectTest_TeamComment', + 'ManyManyListTest_Product', + 'ManyManyListTest_Category', ); diff --git a/tests/model/PaginatedListTest.php b/tests/model/PaginatedListTest.php index 334230f8d..1b357f640 100644 --- a/tests/model/PaginatedListTest.php +++ b/tests/model/PaginatedListTest.php @@ -12,7 +12,9 @@ class PaginatedListTest extends SapphireTest { protected $extraDataObjects = array( 'DataObjectTest_Team', 'DataObjectTest_SubTeam', - 'DataObjectTest_Player' + 'DataObjectTest_Player', + 'ManyManyListTest_Product', + 'ManyManyListTest_Category', ); public function testPageStart() { From 3738d888e0fbce48e0d88735edd3455a116937b5 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Tue, 10 May 2016 17:02:37 +0100 Subject: [PATCH 4/4] FIX Empty FROM clause --- model/DataQuery.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model/DataQuery.php b/model/DataQuery.php index 8fc3b8cc2..79ed50ffb 100644 --- a/model/DataQuery.php +++ b/model/DataQuery.php @@ -661,7 +661,7 @@ class DataQuery { $this->query->addLeftJoin($componentBaseClass, "\"$relationTable\".\"$componentField\" = \"$componentBaseClass\".\"ID\""); } - if(ClassInfo::hasTable($componentClass)) { + if(ClassInfo::hasTable($componentClass) && !$this->query->isJoinedTo($componentClass)) { $this->query->addLeftJoin($componentClass, "\"$relationTable\".\"$componentField\" = \"$componentClass\".\"ID\""); }