From d0bc9c6d233a0a64aab7569ce0e42b70bc9a36ac Mon Sep 17 00:00:00 2001 From: Hamish Friedlander Date: Tue, 21 Aug 2012 15:52:12 +1200 Subject: [PATCH 01/11] FIX Hierarchy#liveChildren couldnt handle lots of pages Hierarchy#liveChildren was generating a list of all IDs of all pages on staging. When a site had lots of pages, this basically killed the tree. Fix by adding new versioned mode, stage_unique, which uses a subselect to only return items from a stage that are in no other stage. --- model/Hierarchy.php | 17 ++--------------- model/Versioned.php | 24 ++++++++++++++++++++++-- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/model/Hierarchy.php b/model/Hierarchy.php index d6100fa29..7aefd0868 100644 --- a/model/Hierarchy.php +++ b/model/Hierarchy.php @@ -575,22 +575,9 @@ class Hierarchy extends DataExtension { if(!$showAll) $children = $children->where('"ShowInMenus" = 1'); // Query the live site - $children->dataQuery()->setQueryParam('Versioned.mode', 'stage'); + $children->dataQuery()->setQueryParam('Versioned.mode', $onlyDeletedFromStage ? 'stage_unique' : 'stage'); $children->dataQuery()->setQueryParam('Versioned.stage', 'Live'); - - if($onlyDeletedFromStage) { - // Note that this makes a second query, and could be optimised to be a join - $stageChildren = DataObject::get($baseClass) - ->where("\"{$baseClass}\".\"ID\" != $id"); - $stageChildren->dataQuery()->setQueryParam('Versioned.mode', 'stage'); - $stageChildren->dataQuery()->setQueryParam('Versioned.stage', ''); - - $ids = $stageChildren->column("ID"); - if($ids) { - $children = $children->where("\"$baseClass\".\"ID\" NOT IN (" . implode(',',$ids) . ")"); - } - } - + return $children; } diff --git a/model/Versioned.php b/model/Versioned.php index 67dedb5bd..aed1bac4c 100644 --- a/model/Versioned.php +++ b/model/Versioned.php @@ -181,8 +181,28 @@ class Versioned extends DataExtension { } } break; - - + + // Reading a specific stage, but only return items that aren't in any other stage + case 'stage_unique': + $stage = $dataQuery->getQueryParam('Versioned.stage'); + + // Recurse to do the default stage behavior (must be first, we rely on stage renaming happening before below) + $dataQuery->setQueryParam('Versioned.mode', 'stage'); + $this->augmentSQL($query, $dataQuery); + + // Now exclude any ID from any other stage. Note that we double rename to avoid the regular stage rename + // renaming all subquery references to be Versioned.stage + foreach($this->stages as $excluding) { + if ($excluding == $stage) continue; + + $tempName = 'ExclusionarySource_'.$excluding; + $excludingTable = $baseTable . ($excluding && $excluding != $this->defaultStage ? "_$excluding" : ''); + + $query->addWhere('"'.$baseTable.'"."ID" NOT IN (SELECT ID FROM "'.$tempName.'")'); + $query->renameTable($tempName, $excludingTable); + } + break; + // Return all version instances case 'all_versions': case 'latest_versions': From 66dfa38d0a3f4dcd1f06e80e4e0e35a48d93bfea Mon Sep 17 00:00:00 2001 From: unclecheese Date: Tue, 21 Aug 2012 12:48:01 -0300 Subject: [PATCH 02/11] ENHANCEMENT: GreaterThanFilter should be consistent with LessThanFilter Numeric or float values weren't supported. --- search/filters/GreaterThanFilter.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/search/filters/GreaterThanFilter.php b/search/filters/GreaterThanFilter.php index 5d9b753cc..8756d908c 100644 --- a/search/filters/GreaterThanFilter.php +++ b/search/filters/GreaterThanFilter.php @@ -14,11 +14,12 @@ class GreaterThanFilter extends SearchFilter { */ public function apply(DataQuery $query) { $this->model = $query->applyRelation($this->relation); - return $query->where(sprintf( - "%s > '%s'", - $this->getDbName(), - Convert::raw2sql($this->getDbFormattedValue()) - )); + $value = $this->getDbFormattedValue(); + + if(is_numeric($value)) $filter = sprintf("%s > %s", $this->getDbName(), Convert::raw2sql($value)); + else $filter = sprintf("%s > '%s'", $this->getDbName(), Convert::raw2sql($value)); + + return $query->where($filter); } public function isEmpty() { From 9a8313dce05d27279675680f1504783d5d61432d Mon Sep 17 00:00:00 2001 From: James Cocker Date: Tue, 21 Aug 2012 23:31:34 +0100 Subject: [PATCH 03/11] BUGFIX: GridField delete icon now correctly deletes, rather than always just unlinking (Fixes 7801) Fixes the handleAction function of GridFieldDeleteAction which wasn't differentiating between a 'deleterecord' action and an 'unlinkrelation' action. Fixes http://open.silverstripe.org/ticket/7801 --- forms/gridfield/GridFieldDeleteAction.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/forms/gridfield/GridFieldDeleteAction.php b/forms/gridfield/GridFieldDeleteAction.php index 8229a8a8a..a1a489aa9 100644 --- a/forms/gridfield/GridFieldDeleteAction.php +++ b/forms/gridfield/GridFieldDeleteAction.php @@ -133,7 +133,11 @@ class GridFieldDeleteAction implements GridField_ColumnProvider, GridField_Actio if($actionName == 'deleterecord' && !$item->canDelete()) { throw new ValidationException(_t('GridFieldAction_Delete.DeletePermissionsFailure',"No delete permissions"),0); } - $gridField->getList()->remove($item); + if($actionName == 'deleterecord') { + $item->delete(); + } else { + $gridField->getList()->remove($item); + } } } } From 69182c21ce15e8ac330c087c515975a09f03ec68 Mon Sep 17 00:00:00 2001 From: Naomi Guyer Date: Wed, 22 Aug 2012 13:42:53 +1200 Subject: [PATCH 04/11] BUG: Installer implies empty template used in tutorial Have reworded the template options so that installers know that the simple template is used for the tutorial --- dev/install/config-form.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dev/install/config-form.html b/dev/install/config-form.html index d4cfc2fc1..9cfecc056 100644 --- a/dev/install/config-form.html +++ b/dev/install/config-form.html @@ -1,4 +1,4 @@ - + @@ -244,8 +244,8 @@

Theme selection Step 4 of 5

You can change the theme or download another from the SilverStripe website after installation.

    -
  • checked="checked">
    • -
  • checked="checked">
    • +
  • checked="checked">
    • +
  • checked="checked">

Confirm Install Step 5 of 5

From ae9c2e78a11260cecaf1d2719be5d20497312d6a Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Wed, 22 Aug 2012 15:33:28 +0200 Subject: [PATCH 05/11] BUG Restore tree children after updateNode() (fixes #7761) --- admin/javascript/LeftAndMain.Tree.js | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/admin/javascript/LeftAndMain.Tree.js b/admin/javascript/LeftAndMain.Tree.js index 1e2ea02ac..bfa8b5b3d 100644 --- a/admin/javascript/LeftAndMain.Tree.js +++ b/admin/javascript/LeftAndMain.Tree.js @@ -285,7 +285,8 @@ } // Replace inner content - node.addClass(origClasses).html(newNode.html()); + var origChildren = node.children('ul').detach(); + node.addClass(origClasses).html(newNode.html()).append(origChildren); if (nextNode && nextNode.length) { this.jstree('move_node', node, nextNode, 'before'); @@ -354,23 +355,25 @@ return; } + var correctStateFn = function(node) { + self.jstree('deselect_all'); + self.jstree('select_node', node); + // Similar to jstree's correct_state, but doesn't remove children + var hasChildren = (node.children('ul').length > 0); + node.toggleClass('jstree-leaf', !hasChildren); + if(!hasChildren) node.removeClass('jstree-closed jstree-open'); + }; + // Check if node exists, create if necessary if(node.length) { self.updateNode(node, nodeData.html, nodeData); setTimeout(function() { - self.jstree('deselect_all'); - self.jstree('select_node', node); - // Manually correct state, which checks for children and - // removes toggle arrow (should really be done by jstree internally) - self.jstree('correct_state', node); + correctStateFn(node) ; }, 500); } else { includesNewNode = true; self.createNode(nodeData.html, nodeData, function(newNode) { - self.jstree('deselect_all'); - self.jstree('select_node', newNode); - // Manually remove toggle node, see above - self.jstree('correct_state', newNode); + correctStateFn(newNode); }); } }); From 9ebac90864a941a34972f221ec1bd84c846ad6cb Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Wed, 22 Aug 2012 16:29:37 +0200 Subject: [PATCH 06/11] Removed 'relation filters' from datamodel docs Not implemented yet, so we shouldn't document it either to avoid confusing devs. See #7595 --- docs/en/topics/datamodel.md | 38 ------------------------------------- 1 file changed, 38 deletions(-) diff --git a/docs/en/topics/datamodel.md b/docs/en/topics/datamodel.md index bccc2de7a..ac415d84b 100755 --- a/docs/en/topics/datamodel.md +++ b/docs/en/topics/datamodel.md @@ -246,44 +246,6 @@ use case could be when you want to find all the members that does not exist in a // ... Finding all members that does not belong to $group. $otherMembers = Member::get()->subtract($group->Members()); - -### Relation filters - -So far we have only filtered a data list by fields on the object that you're requesting. For simple cases, this might -be okay, but often, a data model is made up of a number of related objects. For example, in SilverStripe each member -can be placed in a number of groups, and each group has a number of permissions. - -For this, the SilverStripe ORM supports **Relation Filters**. Any ORM request can be filtered by fields on a related -object by specifying the filter key as `.`. You can chain relations together -as many times as is necessary. - -For example, this will return all members assigned to a group that has a permission record with the code "ADMIN". In -other words, it will return all administrators. - - :::php - $members = Member::get()->filter(array( - 'Groups.Permissions.Code:ExactMatch' => 'ADMIN', - )); - -Note that we are just joining these tables to filter the records. Even if a member is in more than 1 administrator -group, unique members will still be returned by this query. - -The other features of filters can be applied to relation filters as well. This will return all members in groups whose -names start with 'A' or 'B'. - - :::php - $members = Member::get()->filter(array( - 'Groups.Title:StartsWith' => array('A', 'B'), - )); - -You can even follow a relation back to the original model class! This will return all members are in at least 1 group -that also has a member called Sam. - - :::php - $members = Member::get()->filter(array( - 'Groups.Members.FirstName' => 'Sam' - )); - ### Raw SQL options for advanced users Occasionally, the system described above won't let you do exactly what you need to do. In these situtations, we have From 3e0782267c80407e7ce7ee025ce8ea209cd9d597 Mon Sep 17 00:00:00 2001 From: Fred Condo Date: Wed, 22 Aug 2012 16:13:29 -0700 Subject: [PATCH 07/11] Allow scheme-relative URLs in requirements The Requirements class currently treats only absolute URLs as URLs, and tries to interpret anything else as a filesystem path. This prevents using scheme-relative URLs for requirements. Example: <% require javascript(//ajax.googleapis.com/ajax/libs/jquery/1.7.2/jquery.min.js) %> This forces the unfortunate choice of not using a CDN for common scripts, always using an https absolute URL, or accepting that some browsers will throw a security warning when viewing the site in https. This change allows scheme-relative URLs & updates RequirementsTest. --- tests/forms/RequirementsTest.php | 10 ++++++++++ view/Requirements.php | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/forms/RequirementsTest.php b/tests/forms/RequirementsTest.php index ac588a0ba..0958fd8bc 100644 --- a/tests/forms/RequirementsTest.php +++ b/tests/forms/RequirementsTest.php @@ -18,8 +18,10 @@ class RequirementsTest extends SapphireTest { $backend->javascript('http://www.mydomain.com/test.js'); $backend->javascript('https://www.mysecuredomain.com/test.js'); + $backend->javascript('//scheme-relative.example.com/test.js'); $backend->css('http://www.mydomain.com/test.css'); $backend->css('https://www.mysecuredomain.com/test.css'); + $backend->css('//scheme-relative.example.com/test.css'); $html = $backend->includeInHTML(false, self::$html_template); @@ -31,6 +33,10 @@ class RequirementsTest extends SapphireTest { (strpos($html, 'https://www.mysecuredomain.com/test.js') !== false), 'Load external secure javascript URL' ); + $this->assertTrue( + (strpos($html, '//scheme-relative.example.com/test.js') !== false), + 'Load external scheme-relative javascript URL' + ); $this->assertTrue( (strpos($html, 'http://www.mydomain.com/test.css') !== false), 'Load external CSS URL' @@ -39,6 +45,10 @@ class RequirementsTest extends SapphireTest { (strpos($html, 'https://www.mysecuredomain.com/test.css') !== false), 'Load external secure CSS URL' ); + $this->assertTrue( + (strpos($html, '//scheme-relative.example.com/test.css') !== false), + 'Load scheme-relative CSS URL' + ); } protected function setupCombinedRequirements($backend) { diff --git a/view/Requirements.php b/view/Requirements.php index e7e8ab2fa..57bfce3ed 100644 --- a/view/Requirements.php +++ b/view/Requirements.php @@ -784,7 +784,7 @@ class Requirements_Backend { * @return string|boolean */ protected function path_for_file($fileOrUrl) { - if(preg_match('/^http[s]?/', $fileOrUrl)) { + if(preg_match('{^//|http[s]?}', $fileOrUrl)) { return $fileOrUrl; } elseif(Director::fileExists($fileOrUrl)) { $prefix = Director::baseURL(); From f6334dd01774d36e1e3453ccafb7db69b8a2f30d Mon Sep 17 00:00:00 2001 From: Sam Minnee Date: Tue, 21 Aug 2012 16:23:00 +1200 Subject: [PATCH 08/11] Added default sort to test data for better cross-db performance. --- tests/model/DataObjectLazyLoadingTest.php | 17 +++++++++++------ tests/model/DataObjectTest.php | 2 ++ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/tests/model/DataObjectLazyLoadingTest.php b/tests/model/DataObjectLazyLoadingTest.php index 8ade64c9c..4b555f9f9 100644 --- a/tests/model/DataObjectLazyLoadingTest.php +++ b/tests/model/DataObjectLazyLoadingTest.php @@ -28,8 +28,10 @@ class DataObjectLazyLoadingTest extends SapphireTest { $expected = 'SELECT DISTINCT "DataObjectTest_Team"."ClassName", "DataObjectTest_Team"."Created", ' . '"DataObjectTest_Team"."LastEdited", "DataObjectTest_Team"."ID", CASE WHEN '. '"DataObjectTest_Team"."ClassName" IS NOT NULL THEN "DataObjectTest_Team"."ClassName" ELSE ' . - $db->prepStringForDB('DataObjectTest_Team').' END AS "RecordClassName" FROM "DataObjectTest_Team" WHERE ' . - '("DataObjectTest_Team"."ClassName" IN ('.$db->prepStringForDB('DataObjectTest_SubTeam').'))'; + $db->prepStringForDB('DataObjectTest_Team').' END AS "RecordClassName", "DataObjectTest_Team"."Title" '. + 'FROM "DataObjectTest_Team" ' . + 'WHERE ("DataObjectTest_Team"."ClassName" IN ('.$db->prepStringForDB('DataObjectTest_SubTeam').'))' . + ' ORDER BY "DataObjectTest_Team"."Title" ASC'; $this->assertEquals($expected, $playerList->sql()); } @@ -43,7 +45,8 @@ class DataObjectLazyLoadingTest extends SapphireTest { '"DataObjectTest_Team"."ClassName" IS NOT NULL THEN "DataObjectTest_Team"."ClassName" ELSE ' . $db->prepStringForDB('DataObjectTest_Team').' END AS "RecordClassName" FROM "DataObjectTest_Team" LEFT JOIN ' . '"DataObjectTest_SubTeam" ON "DataObjectTest_SubTeam"."ID" = "DataObjectTest_Team"."ID" WHERE ' . - '("DataObjectTest_Team"."ClassName" IN ('.$db->prepStringForDB('DataObjectTest_SubTeam').'))'; + '("DataObjectTest_Team"."ClassName" IN ('.$db->prepStringForDB('DataObjectTest_SubTeam').')) ' . + 'ORDER BY "DataObjectTest_Team"."Title" ASC'; $this->assertEquals($expected, $playerList->sql()); } @@ -55,7 +58,8 @@ class DataObjectLazyLoadingTest extends SapphireTest { '"DataObjectTest_Team"."LastEdited", "DataObjectTest_Team"."Title", "DataObjectTest_Team"."ID", ' . 'CASE WHEN "DataObjectTest_Team"."ClassName" IS NOT NULL THEN "DataObjectTest_Team"."ClassName" ELSE ' . $db->prepStringForDB('DataObjectTest_Team').' END AS "RecordClassName" FROM "DataObjectTest_Team" WHERE ' . - '("DataObjectTest_Team"."ClassName" IN ('.$db->prepStringForDB('DataObjectTest_SubTeam').'))'; + '("DataObjectTest_Team"."ClassName" IN ('.$db->prepStringForDB('DataObjectTest_SubTeam').')) ' . + 'ORDER BY "DataObjectTest_Team"."Title" ASC'; $this->assertEquals($expected, $playerList->sql()); } @@ -66,9 +70,10 @@ class DataObjectLazyLoadingTest extends SapphireTest { $expected = 'SELECT DISTINCT "DataObjectTest_Team"."ClassName", "DataObjectTest_Team"."Created", ' . '"DataObjectTest_Team"."LastEdited", "DataObjectTest_SubTeam"."SubclassDatabaseField", ' . '"DataObjectTest_Team"."ID", CASE WHEN "DataObjectTest_Team"."ClassName" IS NOT NULL THEN ' . - '"DataObjectTest_Team"."ClassName" ELSE '.$db->prepStringForDB('DataObjectTest_Team').' END AS "RecordClassName" FROM ' . + '"DataObjectTest_Team"."ClassName" ELSE '.$db->prepStringForDB('DataObjectTest_Team').' END AS "RecordClassName", "DataObjectTest_Team"."Title" FROM ' . '"DataObjectTest_Team" LEFT JOIN "DataObjectTest_SubTeam" ON "DataObjectTest_SubTeam"."ID" = ' . - '"DataObjectTest_Team"."ID" WHERE ("DataObjectTest_Team"."ClassName" IN ('.$db->prepStringForDB('DataObjectTest_SubTeam').'))'; + '"DataObjectTest_Team"."ID" WHERE ("DataObjectTest_Team"."ClassName" IN ('.$db->prepStringForDB('DataObjectTest_SubTeam').')) ' . + 'ORDER BY "DataObjectTest_Team"."Title" ASC'; $this->assertEquals($expected, $playerList->sql()); } diff --git a/tests/model/DataObjectTest.php b/tests/model/DataObjectTest.php index 0d88f0ace..344c9d3ee 100644 --- a/tests/model/DataObjectTest.php +++ b/tests/model/DataObjectTest.php @@ -1111,6 +1111,8 @@ class DataObjectTest_Team extends DataObject implements TestOnly { ) ); + static $default_sort = "Title"; + function MyTitle() { return 'Team ' . $this->Title; } From dd302a68a7b86e58caa1d31f06396f18e596ec7e Mon Sep 17 00:00:00 2001 From: Sam Minnee Date: Tue, 21 Aug 2012 16:23:33 +1200 Subject: [PATCH 09/11] BUGFIX: Ensure that all_versions are sorted explicitly for better cross-db behaviour. --- model/Versioned.php | 1 + 1 file changed, 1 insertion(+) diff --git a/model/Versioned.php b/model/Versioned.php index e3dbb845a..12ce90446 100644 --- a/model/Versioned.php +++ b/model/Versioned.php @@ -228,6 +228,7 @@ class Versioned extends DataExtension { $query->selectField(sprintf('"%s_versions"."%s"', $baseTable, $name), $name); } $query->selectField(sprintf('"%s_versions"."%s"', $baseTable, 'RecordID'), "ID"); + $query->addOrderBy(sprintf('"%s_versions"."%s"', $baseTable, 'Version')); // latest_version has one more step // Return latest version instances, regardless of whether they are on a particular stage From 296ee1fa0fc27e77d4d5e451c72321ed766d210e Mon Sep 17 00:00:00 2001 From: Sam Minnee Date: Tue, 21 Aug 2012 16:24:05 +1200 Subject: [PATCH 10/11] BUGFIX: Add double quotes to index columns for more reliable DB-schema management. --- model/Versioned.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model/Versioned.php b/model/Versioned.php index 12ce90446..027ea7e26 100644 --- a/model/Versioned.php +++ b/model/Versioned.php @@ -384,7 +384,7 @@ class Versioned extends DataExtension { $versionIndexes = array_merge( array( - 'RecordID_Version' => array('type' => 'unique', 'value' => 'RecordID,Version'), + 'RecordID_Version' => array('type' => 'unique', 'value' => '"RecordID","Version"'), 'RecordID' => true, 'Version' => true, ), From ed0341e82fb3cd678341f4d8c41db0441cf0f7fa Mon Sep 17 00:00:00 2001 From: Sam Minnee Date: Thu, 23 Aug 2012 12:35:20 +1200 Subject: [PATCH 11/11] BUGFIX: Ensure that subtracting a sorted DataList works. --- model/DataQuery.php | 1 + 1 file changed, 1 insertion(+) diff --git a/model/DataQuery.php b/model/DataQuery.php index 2445df599..625dbe502 100644 --- a/model/DataQuery.php +++ b/model/DataQuery.php @@ -656,6 +656,7 @@ function max($field) { $fieldExpression = $this->expressionForField($field, $subSelect); $subSelect->setSelect(array()); $subSelect->selectField($fieldExpression, $field); + $subSelect->setOrderBy(null); $this->where($this->expressionForField($field, $this).' NOT IN ('.$subSelect->sql().')'); return $this;