Merge pull request #558 from open-sausages/pulls/4.2/fix-publishing

BUG Use non-destructive publishing for editable options
silverstripe · Mar 16, 2017 · 6c97611 · 6c97611
2 parents bb000ca + c349ae9
commit 6c97611
Show file tree

Hide file tree

Showing 3 changed files with 112 additions and 27 deletions.
diff --git 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
@@ -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
@@ -0,0 +1,49 @@
+<?php
+
+
+class UserFormsVersionedTest extends SapphireTest
+{
+    protected static $fixture_file = 'UserDefinedFormTest.yml';
+
+    public function setUp()
+    {
+        parent::setUp();
+        Versioned::reading_stage('Stage');
+    }
+
+    public function testPublishing()
+    {
+        /** @var UserDefinedForm $form */
+        $form = $this->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);
+    }
+}