Review feedback, Linting failures & tag for behat-extensions update

silverstripe · Oct 16, 2018 · 9f632d7 · 9f632d7
1 parent a927b8e
commit 9f632d7
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 8 deletions.
diff --git a/src/Forms/ElementalAreaField.php b/src/Forms/ElementalAreaField.php
@@ -3,12 +3,12 @@
 namespace DNADesign\Elemental\Forms;
 
 use BlocksPage;
+use DNADesign\Elemental\Controllers\ElementalAreaController;
 use DNADesign\Elemental\Models\BaseElement;
 use DNADesign\Elemental\Models\ElementalArea;
 use DNADesign\Elemental\Services\ElementTabProvider;
 use SilverStripe\Control\Controller;
 use SilverStripe\Core\Config\Config;
-use SilverStripe\Core\Convert;
 use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Forms\CompositeField;
 use SilverStripe\Forms\FieldGroup;
@@ -215,7 +215,7 @@ public function performReadonlyTransformation()
     public function setSubmittedValue($value, $data = null)
     {
         // Content comes through as a JSON encoded list through a hidden field.
-        $this->setValue(Convert::json2array($value));
+        return $this->setValue(json_decode($value, true));
     }
 
     public function saveInto(DataObjectInterface $dataObject)
@@ -224,10 +224,11 @@ public function saveInto(DataObjectInterface $dataObject)
         parent::saveInto($dataObject);
 
         $elementData = $this->Value();
+        $idPrefixLength = strlen(sprintf(ElementalAreaController::FORM_NAME_TEMPLATE, ''));
 
         foreach ($elementData as $form => $data) {
             // Extract the ID
-            $elementId = (int) substr($form, 12);
+            $elementId = (int) substr($form, $idPrefixLength);
 
             /** @var BaseElement $element */
             $element = $this->getArea()->Elements()->byID($elementId);

diff --git a/tests/Behat/Context/FeatureContext.php b/tests/Behat/Context/FeatureContext.php
@@ -375,10 +375,12 @@ protected function findFieldInBlock($block, $name)
 
         assertNotNull($label, sprintf('Could not find a label for a field with the content "%s"', $name));
         assertCount(
-            1, $label, sprintf(
-            'Found more than one label containing the phrase "%s".',
-            $name
-        )
+            1,
+            $label,
+            sprintf(
+                'Found more than one label containing the phrase "%s".',
+                $name
+            )
         );
 
         $label = array_shift($label);

diff --git a/tests/Behat/Context/FixtureContext.php b/tests/Behat/Context/FixtureContext.php
@@ -31,7 +31,9 @@ public function theHasAContentElementWithContent($type, $pageTitle, $elementTitl
         }
 
         $elementalArea = $page->ElementalArea();
-        $elementalArea->Elements()->add($this->getFixtureFactory()->createObject(ElementContent::class, $elementTitle));
+        $elementalArea->Elements()->add(
+            $this->getFixtureFactory()->createObject(ElementContent::class, $elementTitle)
+        );
 
         // Create element
         $element = $this->getFixtureFactory()->get(ElementContent::class, $elementTitle);

diff --git a/tests/Behat/features/edit-block-element.feature b/tests/Behat/features/edit-block-element.feature
@@ -37,6 +37,8 @@ Feature: Edit elements in the CMS
     Then I should see "Eve's Block"
       But I should not see "Alice's Block"
 
+  # The "unsaved changes" dialog causes errors unless this is tagged with "@unsavedChanges"
+  @unsavedChanges
   Scenario: I can preview a block and hide the form again
     Given I see a list of blocks
     Then I should see block 1