From 321362d07a763e8b790034866717c66f500a08a0 Mon Sep 17 00:00:00 2001 From: Harsh Chokshi Date: Tue, 17 Jul 2018 15:58:17 +1200 Subject: [PATCH 1/6] Reduce nesting in item request default parent logic --- src/Forms/CatalogPageGridFieldItemRequest.php | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/Forms/CatalogPageGridFieldItemRequest.php b/src/Forms/CatalogPageGridFieldItemRequest.php index bbe9d30..4328b79 100644 --- a/src/Forms/CatalogPageGridFieldItemRequest.php +++ b/src/Forms/CatalogPageGridFieldItemRequest.php @@ -23,15 +23,22 @@ class CatalogPageGridFieldItemRequest extends VersionedGridFieldItemRequest */ public function ItemEditForm() { - if (!$this->record->ParentID) { - // set a parent id for the record, even if it will change - $parents = $this->record->getCatalogParents(); - if ($parents !== null && $parents->exists()) { - $this->record->ParentID = $parents->first()->ID; - } + $editForm = parent::ItemEditForm(); + + // Already has parent + if ($this->record->ParentID) return $editForm; + + // Set a default parent id for the record, even if it will change + $parents = $this->record->getCatalogParents(); + + if ($parents === null) return $editForm; // Root page + + $first = $parents->first(); + if ($first !== null) { + $this->record->ParentID = $first->ID; } - return parent::ItemEditForm(); + return $editForm; } /** From 7131137b5cc968fa82c1efe8ea8e3b50a34c0b1f Mon Sep 17 00:00:00 2001 From: Harsh Chokshi Date: Tue, 17 Jul 2018 15:58:37 +1200 Subject: [PATCH 2/6] Handle case of root catalog pages (empty parent array) --- src/Extensions/CatalogPageExtension.php | 6 ++++-- src/ModelAdmin/CatalogPageAdmin.php | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Extensions/CatalogPageExtension.php b/src/Extensions/CatalogPageExtension.php index 5166261..37d6c3f 100644 --- a/src/Extensions/CatalogPageExtension.php +++ b/src/Extensions/CatalogPageExtension.php @@ -73,6 +73,7 @@ public function updateCMSFields(FieldList $fields) $pages = $this->getCatalogParents(); if ($pages === null) { + // Root page return; } @@ -113,7 +114,7 @@ public function getCatalogParents() $parentClasses = $this->getParentClasses(); $parents = null; - if ($parentClasses !== null) { + if (!empty($parentClasses)) { $parents = SiteTree::get()->filter('ClassName', $parentClasses); } $this->owner->extend('updateCatalogParents', $parents); @@ -158,7 +159,8 @@ public static function getClassSortFieldName($class) */ public function canCreate($member) { - return $this->getCatalogParents()->count() === 0 + $parentCandidates = $this->getCatalogParents(); + return $parentCandidates !== null && $parentCandidates->count() === 0 ? false : null; } diff --git a/src/ModelAdmin/CatalogPageAdmin.php b/src/ModelAdmin/CatalogPageAdmin.php index b5f579f..3bfeffb 100755 --- a/src/ModelAdmin/CatalogPageAdmin.php +++ b/src/ModelAdmin/CatalogPageAdmin.php @@ -102,7 +102,8 @@ protected function getCatalogEditForm($id = null, $fields = null, $model) new FieldList() )->setHTMLID('Form_EditForm'); - if ($model->getCatalogParents()->count() === 0) { + $parentCandidates = $model->getCatalogParents(); + if ($parentCandidates !== null && $parentCandidates->count() === 0) { $form->setMessage($this->getMissingParentsMessage($model), ValidationResult::TYPE_WARNING); } From 8b36473e0de21097e9683f2a43fbdb1e983eb252 Mon Sep 17 00:00:00 2001 From: Harsh Chokshi Date: Tue, 11 Sep 2018 11:24:42 +1200 Subject: [PATCH 3/6] Fix required parameter before optional paramters --- src/ModelAdmin/CatalogPageAdmin.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ModelAdmin/CatalogPageAdmin.php b/src/ModelAdmin/CatalogPageAdmin.php index 3bfeffb..1c380be 100755 --- a/src/ModelAdmin/CatalogPageAdmin.php +++ b/src/ModelAdmin/CatalogPageAdmin.php @@ -47,7 +47,7 @@ public function getEditForm($id = null, $fields = null) $model = singleton($this->modelClass); if ($model->has_extension(CatalogPageExtension::class)) { - $form = $this->getCatalogEditForm($id, $fields, $model); + $form = $this->getCatalogEditForm($model, $id, $fields); } elseif (method_exists($model, 'getAdminListField')) { $form = Form::create( $this, @@ -64,12 +64,12 @@ public function getEditForm($id = null, $fields = null) } /** - * @param null $id - * @param null $fields * @param \SilverStripe\CMS\Model\SiteTree|\LittleGiant\CatalogManager\Extensions\CatalogPageExtension $model + * @param null|string $id + * @param null|string $fields * @return \SilverStripe\Forms\Form */ - protected function getCatalogEditForm($id = null, $fields = null, $model) + protected function getCatalogEditForm($model, $id = null, $fields = null) { $originalStage = Versioned::get_stage(); Versioned::set_stage(Versioned::DRAFT); From f6d8f7e17511846c7c333dbe814afe545cc5371e Mon Sep 17 00:00:00 2001 From: Harsh Chokshi Date: Tue, 11 Sep 2018 11:25:04 +1200 Subject: [PATCH 4/6] Simplify auto-publish config check --- src/Extensions/AutoPublishSortExtension.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Extensions/AutoPublishSortExtension.php b/src/Extensions/AutoPublishSortExtension.php index 21d63fd..a9768b7 100644 --- a/src/Extensions/AutoPublishSortExtension.php +++ b/src/Extensions/AutoPublishSortExtension.php @@ -3,6 +3,7 @@ namespace LittleGiant\CatalogManager\Extensions; use SilverStripe\CMS\Model\SiteTree; +use SilverStripe\Core\Config\Config; use SilverStripe\Core\Extension; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DB; @@ -23,9 +24,7 @@ class AutoPublishSortExtension extends Extension public function onAfterReorderItems(SS_List &$list, array $values, array $sortedIDs) { $modelClass = $list->dataClass(); - /** @var CatalogPageExtension|SiteTree $model */ - $model = singleton($modelClass); - if (!$model::config()->get('automatic_live_sort')) { + if (!Config::forClass($modelClass)->get('automatic_live_sort')) { return; } From 3218decb1cc6be1d534bec570b12e5bcc7da40a0 Mon Sep 17 00:00:00 2001 From: Harsh Chokshi Date: Tue, 11 Sep 2018 11:25:33 +1200 Subject: [PATCH 5/6] Treat null as no parent candidates --- src/Extensions/CatalogPageExtension.php | 2 +- src/Forms/CatalogPageGridFieldItemRequest.php | 12 +++++------- src/ModelAdmin/CatalogPageAdmin.php | 2 +- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/Extensions/CatalogPageExtension.php b/src/Extensions/CatalogPageExtension.php index 37d6c3f..e8caa4f 100644 --- a/src/Extensions/CatalogPageExtension.php +++ b/src/Extensions/CatalogPageExtension.php @@ -160,7 +160,7 @@ public static function getClassSortFieldName($class) public function canCreate($member) { $parentCandidates = $this->getCatalogParents(); - return $parentCandidates !== null && $parentCandidates->count() === 0 + return $parentCandidates === null || $parentCandidates->count() === 0 ? false : null; } diff --git a/src/Forms/CatalogPageGridFieldItemRequest.php b/src/Forms/CatalogPageGridFieldItemRequest.php index 4328b79..03474fc 100644 --- a/src/Forms/CatalogPageGridFieldItemRequest.php +++ b/src/Forms/CatalogPageGridFieldItemRequest.php @@ -25,16 +25,14 @@ public function ItemEditForm() { $editForm = parent::ItemEditForm(); - // Already has parent - if ($this->record->ParentID) return $editForm; + if ($this->record->ParentID) { + // Already has parent + return $editForm; + } // Set a default parent id for the record, even if it will change $parents = $this->record->getCatalogParents(); - - if ($parents === null) return $editForm; // Root page - - $first = $parents->first(); - if ($first !== null) { + if ($parents !== null && $first = $parents->first()) { $this->record->ParentID = $first->ID; } diff --git a/src/ModelAdmin/CatalogPageAdmin.php b/src/ModelAdmin/CatalogPageAdmin.php index 1c380be..99094fc 100755 --- a/src/ModelAdmin/CatalogPageAdmin.php +++ b/src/ModelAdmin/CatalogPageAdmin.php @@ -103,7 +103,7 @@ protected function getCatalogEditForm($model, $id = null, $fields = null) )->setHTMLID('Form_EditForm'); $parentCandidates = $model->getCatalogParents(); - if ($parentCandidates !== null && $parentCandidates->count() === 0) { + if ($parentCandidates === null || $parentCandidates->count() === 0) { $form->setMessage($this->getMissingParentsMessage($model), ValidationResult::TYPE_WARNING); } From 04da59030c5069b85f3ae319f9c836498e9f5645 Mon Sep 17 00:00:00 2001 From: Harsh Chokshi Date: Tue, 11 Sep 2018 11:37:05 +1200 Subject: [PATCH 6/6] Use count to short-circuit null check --- src/Extensions/CatalogPageExtension.php | 20 ++++++++----------- src/Forms/CatalogPageGridFieldItemRequest.php | 20 ++++++++----------- src/ModelAdmin/CatalogPageAdmin.php | 3 +-- 3 files changed, 17 insertions(+), 26 deletions(-) diff --git a/src/Extensions/CatalogPageExtension.php b/src/Extensions/CatalogPageExtension.php index e8caa4f..2645760 100644 --- a/src/Extensions/CatalogPageExtension.php +++ b/src/Extensions/CatalogPageExtension.php @@ -69,23 +69,20 @@ public function isPublishedNice() */ public function updateCMSFields(FieldList $fields) { - $parentClass = $this->getParentClasses(); - $pages = $this->getCatalogParents(); - - if ($pages === null) { + $parentPages = $this->getCatalogParents(); + if ($parentPages === null) { // Root page return; } - $parentCount = $pages->count(); - + $parentCount = count($parentPages); if ($parentCount === 0) { - throw new Exception('You must create a parent page with one of these classes: ' . implode(', ', $parentClass)); + throw new Exception('You must create a parent page with one of these classes: ' . implode(', ', $this->getParentClasses())); } elseif ($parentCount === 1) { - $field = HiddenField::create('ParentID', 'ParentID', $pages->first()->ID); + $field = HiddenField::create('ParentID', 'ParentID', $parentPages->first()->ID); } else { - $defaultParentID = $this->owner->ParentID ?: $pages->first()->ID; - $field = DropdownField::create('ParentID', _t(__CLASS__ . '.PARENTPAGE', 'Parent Page'), $pages->map(), $defaultParentID); + $defaultParentID = $this->owner->ParentID ?: $parentPages->first()->ID; + $field = DropdownField::create('ParentID', _t(__CLASS__ . '.PARENTPAGE', 'Parent Page'), $parentPages->map(), $defaultParentID); } $fields->addFieldToTab('Root.Main', $field); @@ -159,8 +156,7 @@ public static function getClassSortFieldName($class) */ public function canCreate($member) { - $parentCandidates = $this->getCatalogParents(); - return $parentCandidates === null || $parentCandidates->count() === 0 + return count($this->getCatalogParents()) === 0 ? false : null; } diff --git a/src/Forms/CatalogPageGridFieldItemRequest.php b/src/Forms/CatalogPageGridFieldItemRequest.php index 03474fc..ae537d3 100644 --- a/src/Forms/CatalogPageGridFieldItemRequest.php +++ b/src/Forms/CatalogPageGridFieldItemRequest.php @@ -23,20 +23,16 @@ class CatalogPageGridFieldItemRequest extends VersionedGridFieldItemRequest */ public function ItemEditForm() { - $editForm = parent::ItemEditForm(); - - if ($this->record->ParentID) { - // Already has parent - return $editForm; - } - - // Set a default parent id for the record, even if it will change - $parents = $this->record->getCatalogParents(); - if ($parents !== null && $first = $parents->first()) { - $this->record->ParentID = $first->ID; + if (!empty($this->record) && !$this->record->ParentID) { + // Set a default parent id for the record, even if it will change + $parents = $this->record->getCatalogParents(); + $first = $parents !== null ? $parents->first() : null; + if ($first !== null) { + $this->record->ParentID = $first->ID; + } } - return $editForm; + return parent::ItemEditForm(); } /** diff --git a/src/ModelAdmin/CatalogPageAdmin.php b/src/ModelAdmin/CatalogPageAdmin.php index 99094fc..9b16e75 100755 --- a/src/ModelAdmin/CatalogPageAdmin.php +++ b/src/ModelAdmin/CatalogPageAdmin.php @@ -102,8 +102,7 @@ protected function getCatalogEditForm($model, $id = null, $fields = null) new FieldList() )->setHTMLID('Form_EditForm'); - $parentCandidates = $model->getCatalogParents(); - if ($parentCandidates === null || $parentCandidates->count() === 0) { + if (count($model->getCatalogParents()) === 0) { $form->setMessage($this->getMissingParentsMessage($model), ValidationResult::TYPE_WARNING); }