From c349ae980e587bd27dcc0296ddc118715e7378df Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Fri, 10 Mar 2017 17:48:47 +1300 Subject: [PATCH] BUG Use non-destructive pubilshing for editable options Fixes #544 --- .../editableformfields/EditableFormField.php | 48 ++++++++++++------ .../EditableMultipleOptionField.php | 42 +++++++++++----- tests/UserFormsVersionedTest.php | 49 +++++++++++++++++++ 3 files changed, 112 insertions(+), 27 deletions(-) create mode 100644 tests/UserFormsVersionedTest.php diff --git a/code/model/editableformfields/EditableFormField.php b/code/model/editableformfields/EditableFormField.php index c241bc3e6..6c5259204 100755 --- a/code/model/editableformfields/EditableFormField.php +++ b/code/model/editableformfields/EditableFormField.php @@ -16,6 +16,7 @@ * @property string $CustomErrorMessage * @method UserDefinedForm Parent() Parent page * @method DataList DisplayRules() List of EditableCustomRule objects + * @mixin Versioned */ class EditableFormField extends DataObject { @@ -482,32 +483,47 @@ public function canUnpublish($member = null) * Publish this Form Field to the live site * * Wrapper for the {@link Versioned} publish function + * + * @param string $fromStage + * @param string $toStage + * @param bool $createNewVersion */ public function doPublish($fromStage, $toStage, $createNewVersion = false) { $this->publish($fromStage, $toStage, $createNewVersion); + $this->publishRules($fromStage, $toStage, $createNewVersion); + } - $seenIDs = array(); + /** + * Publish all field rules + * + * @param string $fromStage + * @param string $toStage + * @param bool $createNewVersion + */ + protected function publishRules($fromStage, $toStage, $createNewVersion) + { + $seenRuleIDs = array(); - // Don't forget to publish the related custom rules... - foreach ($this->DisplayRules() as $rule) { - $seenIDs[] = $rule->ID; - $rule->doPublish($fromStage, $toStage, $createNewVersion); - $rule->destroy(); - } + // Don't forget to publish the related custom rules... + foreach ($this->DisplayRules() as $rule) { + $seenRuleIDs[] = $rule->ID; + $rule->doPublish($fromStage, $toStage, $createNewVersion); + $rule->destroy(); + } - // remove any orphans from the "fromStage" + // remove any orphans from the "fromStage" $rules = Versioned::get_by_stage('EditableCustomRule', $toStage) ->filter('ParentID', $this->ID); - if (!empty($seenIDs)) { - $rules = $rules->exclude('ID', $seenIDs); + if (!empty($seenRuleIDs)) { + $rules = $rules->exclude('ID', $seenRuleIDs); } - foreach ($rules as $rule) { - $rule->deleteFromStage($toStage); - } - } + foreach ($rules as $rule) { + $rule->deleteFromStage($toStage); + } + } /** * Delete this field from a given stage @@ -528,7 +544,7 @@ public function doDeleteFromStage($stage) } /** - * checks wether record is new, copied from Sitetree + * checks whether record is new, copied from SiteTree */ public function isNew() { @@ -593,7 +609,7 @@ public function setSetting($key, $value) /** * Set the allowed css classes for the extraClass custom setting * - * @param array The permissible CSS classes to add + * @param array $allowed The permissible CSS classes to add */ public function setAllowedCss(array $allowed) { diff --git a/code/model/editableformfields/EditableMultipleOptionField.php b/code/model/editableformfields/EditableMultipleOptionField.php index f5ed98fd6..d29499ebf 100644 --- a/code/model/editableformfields/EditableMultipleOptionField.php +++ b/code/model/editableformfields/EditableMultipleOptionField.php @@ -87,25 +87,45 @@ public function getCMSFields() * When publishing it needs to handle copying across / publishing * each of the individual field options * - * @return void + * @param string $fromStage + * @param string $toStage + * @param bool $createNewVersion */ public function doPublish($fromStage, $toStage, $createNewVersion = false) { - $live = Versioned::get_by_stage("EditableOption", "Live", "\"EditableOption\".\"ParentID\" = $this->ID"); + parent::doPublish($fromStage, $toStage, $createNewVersion); + $this->publishOptions($fromStage, $toStage, $createNewVersion); + } - if ($live) { - foreach ($live as $option) { - $option->deleteFromStage('Live'); - } + + /** + * Publish list options + * + * @param string $fromStage + * @param string $toStage + * @param bool $createNewVersion + */ + protected function publishOptions($fromStage, $toStage, $createNewVersion) + { + $seenIDs = array(); + + // Publish all options + foreach ($this->Options() as $option) { + $seenIDs[] = $option->ID; + $option->publish($fromStage, $toStage, $createNewVersion); } - if ($this->Options()) { - foreach ($this->Options() as $option) { - $option->publish($fromStage, $toStage, $createNewVersion); - } + // remove any orphans from the "fromStage" + $options = Versioned::get_by_stage('EditableOption', $toStage) + ->filter('ParentID', $this->ID); + + if (!empty($seenIDs)) { + $options = $options->exclude('ID', $seenIDs); } - parent::doPublish($fromStage, $toStage, $createNewVersion); + foreach ($options as $rule) { + $rule->deleteFromStage($toStage); + } } /** diff --git a/tests/UserFormsVersionedTest.php b/tests/UserFormsVersionedTest.php new file mode 100644 index 000000000..dfe00957a --- /dev/null +++ b/tests/UserFormsVersionedTest.php @@ -0,0 +1,49 @@ +objFromFixture('UserDefinedForm', 'filtered-form-page'); + + // Get id of options + $optionID = $this->idFromFixture('EditableOption', 'option-3'); + $this->assertEmpty(Versioned::get_one_by_stage('EditableOption', 'Live', array('"ID" = ?' => $optionID))); + + // Publishing writes this to live + $form->doPublish(); + $liveVersion = Versioned::get_versionnumber_by_stage('EditableOption', 'Live', $optionID, false); + $this->assertNotEmpty($liveVersion); + + // Add new option, and repeat publish process + /** @var EditableCheckboxGroupField $list */ + $list = $this->objFromFixture('EditableCheckboxGroupField', 'checkbox-group'); + $newOption = new EditableOption(); + $newOption->Title = 'New option'; + $newOption->Value = 'ok'; + $newOption->write(); + $newOptionID = $newOption->ID; + $list->Options()->add($newOption); + + $form->doPublish(); + + // Un-modified option should not create a new version + $newLiveVersion = Versioned::get_versionnumber_by_stage('EditableOption', 'Live', $optionID, false); + $this->assertNotEmpty($newLiveVersion); + $this->assertEquals($liveVersion, $newLiveVersion); + + // New option is successfully published + $newOptionLiveVersion = Versioned::get_versionnumber_by_stage('EditableOption', 'Live', $newOptionID, false); + $this->assertNotEmpty($newOptionLiveVersion); + } +}