Merge pull request #930 from creative-commoners/pulls/4/preview-non-e…

…ditable

FIX Retain value that failed validation on client side

client/src/legacy/ElementEditor/entwine.js

@@ -60,16 +60,29 @@ jQuery.entwine('ss', ($) => {
     },
 
     onunmatch() {
-      resetStores();
+      // Reset the store if the user navigates to a different part of the CMS
+      // or after submission if there are no validation errors
+      if (!$('.cms-edit-form').data('hasValidationErrors')) {
+        resetStores();
+      }
       ReactDOM.unmountComponentAtNode(this[0]);
     },
 
-    /**
-     * Invalidate cache after the form is submitted to force apollo to re-fetch.
-     */
     'from .cms-edit-form': {
-      onaftersubmitform() {
-        resetStores();
+      onaftersubmitform(event, data) {
+        const validationResultPjax = JSON.parse(data.xhr.responseText).ValidationResult;
+        const validationResult = JSON.parse(validationResultPjax.replace(/<\/?script[^>]*?>/g, ''));
+
+        // Reset redux store if form is succesfully submitted so apollo to refetches element data
+        // Do not reset if there are any validation errors because we want redux to hydrate the
+        // form, rather than then refetching which will return a value from the database.
+        // Instead the user should still see any modfied value they just entered.
+        if (validationResult.isValid) {
+          $('.cms-edit-form').data('hasValidationErrors', false);
+          resetStores();
+        } else {
+          $('.cms-edit-form').data('hasValidationErrors', true);
+        }
       }
     },
   });

composer.json

@@ -21,7 +21,7 @@
         "php": "^7.4 || ^8.0",
         "silverstripe/framework": "^4.12@dev",
         "silverstripe/cms": "^4.7@dev",
-        "silverstripe/admin": "^1.7@dev",
+        "silverstripe/admin": "^1.13.2@dev",
         "silverstripe/versioned": "^1.7@dev",
         "silverstripe/versioned-admin": "^1.7@dev",
         "silverstripe/graphql": "^3.5 || ^4",

tests/Behat/Context/FeatureContext.php

@@ -240,19 +240,27 @@ public function stepIFillInForForBlock($value, $name, $blockNumber)
     public function theFieldForBlockShouldContain($field, $blockNumber, $negate, $content)
     {
         $block = $this->getSpecificBlock($blockNumber);
-        $field = $this->findFieldInBlock($block, $field);
-        $isTinyMCE = $field->getAttribute('data-editor') === 'tinyMCE';
+        $fieldElem = $this->findFieldInBlock($block, $field);
+        $isTinyMCE = $fieldElem->getAttribute('data-editor') === 'tinyMCE';
 
         if ($isTinyMCE) {
             $this->cmsContext->theHtmlFieldShouldContain(
-                $field->getAttribute('name'),
+                $fieldElem->getAttribute('name'),
                 $negate,
                 $content
             );
-        } elseif ($negate) {
-            $this->assertFieldNotContains($field, $content);
+            return;
+        }
+
+        $actual = (string) $fieldElem->getValue();
+        $regex = '/^' . preg_quote($content, '/') . '$/ui';
+
+        if ($negate) {
+            $message = sprintf('The field "%s" value is "%s", but "%s" expected.', $field, $actual, $content);
+            Assert::isTrue((bool) preg_match($regex, $actual), $message);
         } else {
-            $this->assertFieldContains($field, $content);
+            $message = sprintf('The field "%s" value is "%s", but it should not be.', $field, $actual);
+            Assert::isFalse((bool) preg_match($regex, $actual), $message);
         }
     }
 

tests/Behat/features/edit-block-element.feature

@@ -69,8 +69,12 @@ Feature: Edit elements in the CMS
     When I press the "Save" button in the actions menu for block 2
       And I wait 1 second
       Then I should see a "Saved 'Charlie's Block' successfully" notice
+      And the "Content" field for block 2 should contain "New sample content"
+      And the "Title" field for block 2 should contain "Charlie's Block"
+    # The block is still expanded. Here we collapse it to see the summary.
     When I click on the caret button for block 2
       Then I should see "New sample content"
+      And I should see "Charlie's Block"
 
   @unsavedChanges
   Scenario: I can edit inline-editable blocks and save the page as a whole
@@ -90,5 +94,6 @@ Feature: Edit elements in the CMS
     When I wait 1 second
       And I click on block 2
       Then the "Content" field for block 2 should contain "<p>Alternate HTML within element 2</p>"
+      And the "Title" field for block 2 should contain "Alice's Much Improved Block"
 
 

tests/Behat/features/validation-failure.feature

@@ -0,0 +1,52 @@
+@javascript
+Feature: Don't lose content when page or block is invalid
+  As a CMS user
+  I want to retain my unsaved content when a validation error occurs
+  So that I can fix the content and save it without recreating content
+
+  Background:
+    Given I add an extension "DNADesign\Elemental\Extensions\ElementalPageExtension" to the "Page" class
+      And a "page" "Blocks Page" with a "Alice's Block" content element with "original content" content
+      And the "group" "EDITOR" has permissions "Access to 'Pages' section"
+      And I am logged in as a member of "EDITOR" group
+
+  # The "unsaved changes" dialog causes errors unless this is tagged with "@unsavedChanges"
+  @unsavedChanges
+  Scenario: If a page is invalid, changes aren't lost
+    Given I add an extension "DNADesign\Elemental\Tests\Src\ValidationFailedExtension" to the "Page" class
+      And I go to "/admin/pages"
+      And I left click on "Blocks Page" in the tree
+    Then I should see a list of blocks
+      And I should see "Alice's Block"
+      And I should not see the ".element-editor-header__version-state--unsaved" element
+    When I click on the caret button for block 1
+      And I fill in "<p>New sample content</p>" for "Content" for block 1
+      And I fill in "Charlie's Block" for "Title" for block 1
+      And I press the "Save" button
+    Then I should see a "Validation error" error toast
+      And I should see "Page is invalid"
+      And I should see the ".element-editor-header__version-state--unsaved" element
+    When I click on the caret button for block 1
+    Then the "Content" field for block 1 should contain "New sample content"
+      And the "Title" field for block 1 should contain "Charlie's Block"
+      And I should see the ".element-editor-header__version-state--unsaved" element
+
+  @unsavedChanges
+  Scenario: If a block is invalid, changes aren't lost
+    Given I add an extension "DNADesign\Elemental\Tests\Src\ValidationFailedExtension" to the "DNADesign\Elemental\Models\BaseElement" class
+      And I go to "/admin/pages"
+      And I left click on "Blocks Page" in the tree
+    Then I should see a list of blocks
+      And I should see "Alice's Block"
+      And I should not see the ".element-editor-header__version-state--unsaved" element
+    When I click on the caret button for block 1
+      And I fill in "<p>New sample content</p>" for "Content" for block 1
+      And I fill in "Charlie's Block" for "Title" for block 1
+      And I press the "Save" button
+    Then I should see a "Validation error" error toast
+      And I should see "ElementContent is invalid"
+      And I should see the ".element-editor-header__version-state--unsaved" element
+    When I click on the caret button for block 1
+    Then the "Content" field for block 1 should contain "New sample content"
+      And the "Title" field for block 1 should contain "Charlie's Block"
+      And I should see the ".element-editor-header__version-state--unsaved" element

tests/Src/ValidationFailedExtension.php

@@ -0,0 +1,18 @@
+<?php
+
+namespace DNADesign\Elemental\Tests\Src;
+
+use SilverStripe\Core\ClassInfo;
+use SilverStripe\Core\Extension;
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\ValidationResult;
+
+class ValidationFailedExtension extends Extension implements TestOnly
+{
+    public const INVALID_TITLE_MESSAGE = '%s is invalid';
+
+    public function validate(ValidationResult $result)
+    {
+        $result->addFieldError('Title', sprintf(static::INVALID_TITLE_MESSAGE, ClassInfo::shortName($this->owner)));
+    }
+}

yarn.lock

@@ -2601,7 +2601,7 @@ debug@^3.1.0, debug@^3.2.7:
   dependencies:
     ms "^2.1.1"
 
-debuglog@*, debuglog@^1.0.1:
+debuglog@^1.0.1:
   version "1.0.1"
   resolved "https://registry.yarnpkg.com/debuglog/-/debuglog-1.0.1.tgz#aa24ffb9ac3df9a2351837cfb2d279360cd78492"
   integrity sha1-qiT/uaw9+aI1GDfPstJ5NgzXhJI=
@@ -4701,7 +4701,7 @@ imports-loader@^0.6.5:
     loader-utils "0.2.x"
     source-map "0.1.x"
 
-imurmurhash@*, imurmurhash@^0.1.4:
+imurmurhash@^0.1.4:
   version "0.1.4"
   resolved "https://registry.yarnpkg.com/imurmurhash/-/imurmurhash-0.1.4.tgz#9218b9b2b928a238b13dc4fb6b6d576f231453ea"
   integrity sha1-khi5srkoojixPcT7a21XbyMUU+o=
@@ -6213,11 +6213,6 @@ lodash._basecopy@^3.0.0:
   resolved "https://registry.yarnpkg.com/lodash._basecopy/-/lodash._basecopy-3.0.1.tgz#8da0e6a876cf344c0ad8a54882111dd3c5c7ca36"
   integrity sha1-jaDmqHbPNEwK2KVIghEd08XHyjY=
 
-lodash._baseindexof@*:
-  version "3.1.0"
-  resolved "https://registry.yarnpkg.com/lodash._baseindexof/-/lodash._baseindexof-3.1.0.tgz#fe52b53a1c6761e42618d654e4a25789ed61822c"
-  integrity sha1-/lK1OhxnYeQmGNZU5KJXie1hgiw=
-
 lodash._baseuniq@~4.6.0:
   version "4.6.0"
   resolved "https://registry.yarnpkg.com/lodash._baseuniq/-/lodash._baseuniq-4.6.0.tgz#0ebb44e456814af7905c6212fa2c9b2d51b841e8"
@@ -6226,16 +6221,11 @@ lodash._baseuniq@~4.6.0:
     lodash._createset "~4.0.0"
     lodash._root "~3.0.0"
 
-lodash._bindcallback@*, lodash._bindcallback@^3.0.0:
+lodash._bindcallback@^3.0.0:
   version "3.0.1"
   resolved "https://registry.yarnpkg.com/lodash._bindcallback/-/lodash._bindcallback-3.0.1.tgz#e531c27644cf8b57a99e17ed95b35c748789392e"
   integrity sha1-5THCdkTPi1epnhftlbNcdIeJOS4=
 
-lodash._cacheindexof@*:
-  version "3.0.2"
-  resolved "https://registry.yarnpkg.com/lodash._cacheindexof/-/lodash._cacheindexof-3.0.2.tgz#3dc69ac82498d2ee5e3ce56091bafd2adc7bde92"
-  integrity sha1-PcaayCSY0u5ePOVgkbr9Ktx73pI=
-
 lodash._createassigner@^3.0.0:
   version "3.1.1"
   resolved "https://registry.yarnpkg.com/lodash._createassigner/-/lodash._createassigner-3.1.1.tgz#838a5bae2fdaca63ac22dee8e19fa4e6d6970b11"
@@ -6245,19 +6235,12 @@ lodash._createassigner@^3.0.0:
     lodash._isiterateecall "^3.0.0"
     lodash.restparam "^3.0.0"
 
-lodash._createcache@*:
-  version "3.1.2"
-  resolved "https://registry.yarnpkg.com/lodash._createcache/-/lodash._createcache-3.1.2.tgz#56d6a064017625e79ebca6b8018e17440bdcf093"
-  integrity sha1-VtagZAF2JeeevKa4AY4XRAvc8JM=
-  dependencies:
-    lodash._getnative "^3.0.0"
-
 lodash._createset@~4.0.0:
   version "4.0.3"
   resolved "https://registry.yarnpkg.com/lodash._createset/-/lodash._createset-4.0.3.tgz#0f4659fbb09d75194fa9e2b88a6644d363c9fe26"
   integrity sha1-D0ZZ+7CddRlPqeK4imZE02PJ/iY=
 
-lodash._getnative@*, lodash._getnative@^3.0.0:
+lodash._getnative@^3.0.0:
   version "3.9.1"
   resolved "https://registry.yarnpkg.com/lodash._getnative/-/lodash._getnative-3.9.1.tgz#570bc7dede46d61cdcde687d65d3eecbaa3aaff5"
   integrity sha1-VwvH3t5G1hzc3mh9ZdPuy6o6r/U=
@@ -6368,7 +6351,7 @@ lodash.memoize@^4.1.2:
   resolved "https://registry.yarnpkg.com/lodash.memoize/-/lodash.memoize-4.1.2.tgz#bcc6c49a42a2840ed997f323eada5ecd182e0bfe"
   integrity sha1-vMbEmkKihA7Zl/Mj6tpezRguC/4=
 
-lodash.restparam@*, lodash.restparam@^3.0.0:
+lodash.restparam@^3.0.0:
   version "3.6.1"
   resolved "https://registry.yarnpkg.com/lodash.restparam/-/lodash.restparam-3.6.1.tgz#936a4e309ef330a7645ed4145986c85ae5b20805"
   integrity sha1-k2pOMJ7zMKdkXtQUWYbIWuWyCAU=