Merge pull request #749 from creative-commoners/pulls/5.0/option-fix-option-fix

FIX Ensure duplicated multiple option field is written (has an ID) before duplicating options

code/Extension/UserFormFieldEditorExtension.php

@@ -225,6 +225,7 @@ class UserFormFieldEditorExtension extends DataExtension
         // List of EditableFieldGroups, where the key of the array is the ID of the old end group
         $fieldGroups = [];
         foreach ($oldPage->Fields() as $field) {
+            /** @var EditableFormField $newField */
             $newField = $field->duplicate(false);
             $newField->ParentID = $this->owner->ID;
             $newField->ParentClass = $this->owner->ClassName;

code/Model/EditableFormField/EditableMultipleOptionField.php

@@ -123,9 +123,10 @@ class EditableMultipleOptionField extends EditableFormField
             $manyMany = null;
         }
 
-        $clonedNode = parent::duplicate($doWrite, $manyMany);
+        $clonedNode = parent::duplicate(true, $manyMany);
 
         foreach ($this->Options() as $field) {
+            /** @var EditableOption $newField */
             $newField = $field->duplicate(false);
             $newField->ParentID = $clonedNode->ID;
             $newField->Version = 0;

tests/Model/EditableFormField/EditableDropdownTest.php

@@ -44,4 +44,14 @@ class EditableDropdownTest extends SapphireTest
         $field->Name = 'EditableFormField_123456';
         $this->assertEmpty($field->getFormField()->Title());
     }
+
+    public function testDuplicate()
+    {
+        /** @var EditableDropdown $dropdown */
+        $dropdown = $this->objFromFixture(EditableDropdown::class, 'basic-dropdown');
+        $this->assertCount(2, $dropdown->Options());
+
+        $duplicatedDropdown = $dropdown->duplicate();
+        $this->assertSame($dropdown->Options()->count(), $duplicatedDropdown->Options()->count());
+    }
 }

tests/Model/UserDefinedFormTest.php

@@ -375,6 +375,32 @@ class UserDefinedFormTest extends FunctionalTest
         $this->assertNotEquals($form2GroupEnd->ID, $form3GroupStart->EndID);
     }
 
+    public function testDuplicateFormDuplicatesRecursively()
+    {
+        $this->logInWithPermission('ADMIN');
+        /** @var UserDefinedForm $form */
+        $form = $this->objFromFixture(UserDefinedForm::class, 'form-with-multioptions');
+
+        $this->assertGreaterThanOrEqual(1, $form->Fields()->count(), 'Fixtured page has a field');
+        $this->assertCount(
+            2,
+            $form->Fields()->Last()->Options(),
+            'Fixtured multiple option field has two options'
+        );
+
+        $newForm = $form->duplicate();
+        $this->assertEquals(
+            $form->Fields()->count(),
+            $newForm->Fields()->count(),
+            'Duplicated page has same number of fields'
+        );
+        $this->assertEquals(
+            $form->Fields()->Last()->Options()->count(),
+            $newForm->Fields()->Last()->Options()->count(),
+            'Duplicated dropdown field from duplicated form has duplicated options'
+        );
+    }
+
     public function testFormOptions()
     {
         $this->logInWithPermission('ADMIN');

tests/UserFormsTest.yml

@@ -344,3 +344,8 @@ SilverStripe\UserForms\Model\UserDefinedForm:
     Fields:
       - =>SilverStripe\UserForms\Model\EditableFormField\EditableEmailField.another-email-field
       - =>SilverStripe\UserForms\Model\EditableFormField\EditableTextField.another-required
+
+  form-with-multioptions:
+    Title: Form with MultipleOption fields
+    Fields:
+      - =>SilverStripe\UserForms\Model\EditableFormField\EditableDropdown.basic-dropdown