From 48bf46215e72ccfdcf4c72bbb8dca6143456bfc1 Mon Sep 17 00:00:00 2001 From: Dylan Wagstaff Date: Mon, 5 Mar 2018 14:31:33 +1300 Subject: [PATCH] FIX stop form items double duplicating (#728) When calling `duplicate` on a form, a form field, or suchlike, the results would be that all related items to the thing being duplicated (such as fields for a form, or options to an editabledropdown) would be duplicated _twice_; ie. where a form had two fields, it's new duplicate would have four (each one occurring twice). We have stopped this in a backwards compatible way - that is the bug was introduced with core 4.1, and this change leaves the userforms module compatible with 4.0. --- .travis.yml | 8 ++++---- code/Model/EditableFormField.php | 2 ++ .../EditableFormField/EditableMultipleOptionField.php | 5 +++++ code/UserForm.php | 2 ++ tests/Model/EditableFormFieldTest.php | 6 +++++- tests/Model/UserDefinedFormTest.php | 2 +- 6 files changed, 19 insertions(+), 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index 82067e2..e693610 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,11 +7,11 @@ env: matrix: include: - php: 5.6 - env: DB=MYSQL PHPCS_TEST=1 PHPUNIT_TEST=1 + env: DB=MYSQL RECIPE_VERSION=1.0.x-dev PHPCS_TEST=1 PHPUNIT_TEST=1 - php: 7.0 - env: DB=MYSQL PHPUNIT_TEST=1 + env: DB=MYSQL RECIPE_VERSION=1.1.x-dev PHPUNIT_TEST=1 - php: 7.1 - env: DB=MYSQL PHPUNIT_COVERAGE_TEST=1 + env: DB=MYSQL RECIPE_VERSION=1.x-dev PHPUNIT_COVERAGE_TEST=1 before_script: # Init PHP @@ -21,7 +21,7 @@ before_script: # Install composer dependencies - composer validate - - composer require --no-update silverstripe/recipe-cms:1.0.x-dev silverstripe-themes/simple:~3.2 + - composer require --no-update silverstripe/recipe-cms:$RECIPE_VERSION - composer install --prefer-dist --no-interaction --no-progress --no-suggest --optimize-autoloader --verbose --profile script: diff --git a/code/Model/EditableFormField.php b/code/Model/EditableFormField.php index 7b9d6c5..5b37110 100755 --- a/code/Model/EditableFormField.php +++ b/code/Model/EditableFormField.php @@ -177,6 +177,8 @@ class EditableFormField extends DataObject 'DisplayRules', ]; + private static $cascade_duplicates = false; + /** * @var bool */ diff --git a/code/Model/EditableFormField/EditableMultipleOptionField.php b/code/Model/EditableFormField/EditableMultipleOptionField.php index 6b8aeb9..48e3ad9 100644 --- a/code/Model/EditableFormField/EditableMultipleOptionField.php +++ b/code/Model/EditableFormField/EditableMultipleOptionField.php @@ -118,6 +118,11 @@ class EditableMultipleOptionField extends EditableFormField */ public function duplicate($doWrite = true, $manyMany = 'many_many') { + // Versioned 1.0 has a bug where [] will result in _all_ relations being duplicated + if ($manyMany === 'many_many' && !$this->manyMany()) { + $manyMany = null; + } + $clonedNode = parent::duplicate($doWrite, $manyMany); foreach ($this->Options() as $field) { diff --git a/code/UserForm.php b/code/UserForm.php index ca5ab38..f0e868e 100644 --- a/code/UserForm.php +++ b/code/UserForm.php @@ -124,6 +124,8 @@ trait UserForm 'EmailRecipients', ]; + private static $cascade_duplicates = false; + /** * @var array * @config diff --git a/tests/Model/EditableFormFieldTest.php b/tests/Model/EditableFormFieldTest.php index 3fd9562..145d559 100644 --- a/tests/Model/EditableFormFieldTest.php +++ b/tests/Model/EditableFormFieldTest.php @@ -137,7 +137,11 @@ class EditableFormFieldTest extends FunctionalTest $clone = $dropdown->duplicate(); - $this->assertEquals($dropdown->Options()->Count(), $clone->Options()->Count()); + $this->assertEquals( + $dropdown->Options()->Count(), + $clone->Options()->Count(), + "The duplicate should have contain same number of options" + ); foreach ($clone->Options() as $option) { $original = $dropdown->Options()->find('Title', $option->Title); diff --git a/tests/Model/UserDefinedFormTest.php b/tests/Model/UserDefinedFormTest.php index 7ba426a..625ad42 100644 --- a/tests/Model/UserDefinedFormTest.php +++ b/tests/Model/UserDefinedFormTest.php @@ -171,7 +171,7 @@ class UserDefinedFormTest extends FunctionalTest $result = $recipient->getEmailTemplateDropdownValues(); // Installation path can be as a project when testing in Travis, so check partial match - $this->assertContains('email/SubmittedFormEmail', key($result)); + $this->assertContains('email' . DIRECTORY_SEPARATOR . 'SubmittedFormEmail', key($result)); $this->assertSame('SubmittedFormEmail', current($result)); }