From 0c4b2f81576433988a3173a62c7fea8d49dc1385 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Wed, 12 Jun 2013 12:32:42 +0200 Subject: [PATCH 1/8] API SiteTree->validURLSegment() prioritizes extension votes Tri-state, use NULL to ignore the extension result --- code/model/SiteTree.php | 18 +++++++++--------- tests/model/SiteTreeTest.php | 23 ++++++++++++++++++++++- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/code/model/SiteTree.php b/code/model/SiteTree.php index 440cb1f4..05b8155a 100644 --- a/code/model/SiteTree.php +++ b/code/model/SiteTree.php @@ -1588,20 +1588,20 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid } } + $votes = array_filter( + (array)$this->extend('augmentValidURLSegment'), + function($v) {return !is_null($v);} + ); + if($votes) { + return min($votes); + } + $existingPage = DataObject::get_one( 'SiteTree', "\"URLSegment\" = '$this->URLSegment' $IDFilter $parentFilter" ); - if ($existingPage) { - return false; - } - - $votes = $this->extend('augmentValidURLSegment'); - if($votes) { - return min($votes); - } - return true; + return !($existingPage); } /** diff --git a/tests/model/SiteTreeTest.php b/tests/model/SiteTreeTest.php index 863e290b..f26c94b6 100644 --- a/tests/model/SiteTreeTest.php +++ b/tests/model/SiteTreeTest.php @@ -4,8 +4,9 @@ * @subpackage tests */ class SiteTreeTest extends SapphireTest { - protected static $fixture_file = 'SiteTreeTest.yml'; + protected static $fixture_file = 'SiteTreeTest.yml'; + protected $illegalExtensions = array( 'SiteTree' => array('SiteTreeSubsites') ); @@ -700,6 +701,18 @@ class SiteTreeTest extends SapphireTest { $this->assertTrue($sitetree->validURLSegment(), 'Valid URLSegment values are allowed'); } + public function testURLSegmentPrioritizesExtensionVotes() { + $sitetree = new SiteTree(); + $sitetree->URLSegment = 'unique-segment'; + $this->assertTrue($sitetree->validURLSegment()); + + SiteTree::add_extension('SiteTreeTest_Extension'); + $sitetree = new SiteTree(); + $sitetree->URLSegment = 'unique-segment'; + $this->assertFalse($sitetree->validURLSegment()); + SiteTree::remove_extension('SiteTreeTest_Extension'); + } + public function testURLSegmentMultiByte() { $origAllow = Config::inst()->get('URLSegmentFilter', 'default_allow_multibyte'); Config::inst()->update('URLSegmentFilter', 'default_allow_multibyte', true); @@ -960,3 +973,11 @@ class SiteTreeTest_StageStatusInherit extends SiteTree implements TestOnly { return $flags; } } + +class SiteTreeTest_Extension extends DataExtension implements TestOnly { + + public function augmentValidURLSegment() { + return false; + } + +} \ No newline at end of file From 2deb525d478ec05527ff6cb6ffaec7ba876a2e25 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Fri, 10 May 2013 15:00:57 +0200 Subject: [PATCH 2/8] Using new CMSForm class to allow for validation errors in CMS This class allows deferring handling of responses to the parent controller's response negotiator implementation. --- code/controllers/AssetAdmin.php | 5 +++-- code/controllers/CMSFileAddController.php | 5 +++-- code/controllers/CMSMain.php | 16 +++++++++++----- code/controllers/CMSPageAddController.php | 5 ++++- code/controllers/CMSPageHistoryController.php | 6 +++--- code/controllers/CMSSettingsController.php | 5 ++++- code/controllers/ReportAdmin.php | 5 ++++- 7 files changed, 32 insertions(+), 15 deletions(-) diff --git a/code/controllers/AssetAdmin.php b/code/controllers/AssetAdmin.php index c5453026..d3d4e4b2 100644 --- a/code/controllers/AssetAdmin.php +++ b/code/controllers/AssetAdmin.php @@ -395,7 +395,7 @@ JS public function AddForm() { $folder = singleton('Folder'); - $form = new Form( + $form = CMSForm::create( $this, 'AddForm', new FieldList( @@ -407,7 +407,8 @@ JS ->addExtraClass('ss-ui-action-constructive')->setAttribute('data-icon', 'accept') ->setTitle(_t('AssetAdmin.ActionAdd', 'Add folder')) ) - ); + )->setHTMLID('Form_AddForm'); + $form->setResponseNegotiator($this->getResponseNegotiator()); $form->setTemplate($this->getTemplatesWithSuffix('_EditForm')); // TODO Can't merge $FormAttributes in template at the moment $form->addExtraClass('add-form cms-add-form cms-edit-form cms-panel-padded center ' . $this->BaseCSSClasses()); diff --git a/code/controllers/CMSFileAddController.php b/code/controllers/CMSFileAddController.php index 6c41820a..5b01213d 100644 --- a/code/controllers/CMSFileAddController.php +++ b/code/controllers/CMSFileAddController.php @@ -76,7 +76,7 @@ class CMSFileAddController extends LeftAndMain { asort($exts); $uploadField->Extensions = implode(', ', $exts); - $form = new Form( + $form = CMSForm::create( $this, 'getEditForm', new FieldList( @@ -84,7 +84,8 @@ class CMSFileAddController extends LeftAndMain { new HiddenField('ID') ), new FieldList() - ); + )->setHTMLID('Form_getEditForm'); + $form->setResponseNegotiator($this->getResponseNegotiator()); $form->addExtraClass('center cms-edit-form ' . $this->BaseCSSClasses()); // Don't use AssetAdmin_EditForm, as it assumes a different panel structure $form->setTemplate($this->getTemplatesWithSuffix('_EditForm')); diff --git a/code/controllers/CMSMain.php b/code/controllers/CMSMain.php index 87d7358a..c7ae441c 100644 --- a/code/controllers/CMSMain.php +++ b/code/controllers/CMSMain.php @@ -663,7 +663,10 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr $validator = new RequiredFields(); } - $form = new Form($this, "EditForm", $fields, $actions, $validator); + $form = CMSForm::create( + $this, "EditForm", $fields, $actions, $validator + )->setHTMLID('Form_EditForm'); + $form->setResponseNegotiator($this->getResponseNegotiator()); $form->loadDataFrom($record); $form->disableDefaultAction(); $form->addExtraClass('cms-edit-form'); @@ -686,9 +689,11 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr $this->extend('updateEditForm', $form); return $form; } else if($id) { - return new Form($this, "EditForm", new FieldList( + $form = CMSForm::create( $this, "EditForm", new FieldList( new LabelField('PageDoesntExistLabel',_t('CMSMain.PAGENOTEXISTS',"This page doesn't exist"))), new FieldList() - ); + )->setHTMLID('Form_EditForm'); + $form->setResponseNegotiator($this->getResponseNegotiator()); + return $form; } } @@ -788,13 +793,14 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr } )); - $listview = new Form( + $listview = CMSForm::create( $this, 'ListViewForm', new FieldList($gridField), new FieldList() - ); + )->setHTMLID('Form_ListViewForm'); $listview->setAttribute('data-pjax-fragment', 'ListViewForm'); + $listview->setResponseNegotiator($this->getResponseNegotiator()); $this->extend('updateListView', $listview); diff --git a/code/controllers/CMSPageAddController.php b/code/controllers/CMSPageAddController.php index 3824b661..06f5ae48 100644 --- a/code/controllers/CMSPageAddController.php +++ b/code/controllers/CMSPageAddController.php @@ -106,7 +106,10 @@ class CMSPageAddController extends CMSPageEditController { $this->extend('updatePageOptions', $fields); - $form = new Form($this, "AddForm", $fields, $actions); + $form = CMSForm::create( + $this, "AddForm", $fields, $actions + )->setHTMLID('Form_AddForm'); + $form->setResponseNegotiator($this->getResponseNegotiator()); $form->addExtraClass('cms-add-form stacked cms-content center cms-edit-form ' . $this->BaseCSSClasses()); $form->setTemplate($this->getTemplatesWithSuffix('_EditForm')); diff --git a/code/controllers/CMSPageHistoryController.php b/code/controllers/CMSPageHistoryController.php index 9444f141..71de9a8e 100644 --- a/code/controllers/CMSPageHistoryController.php +++ b/code/controllers/CMSPageHistoryController.php @@ -251,13 +251,13 @@ class CMSPageHistoryController extends CMSMain { // Use