Focus PR back to original scope

Getting validation errors to display nicely on blocks is out of scope -
we're just making it so if there _are_ validation errors, block content
is not lost.

client/src/legacy/ElementEditor/entwine.js

@@ -9,8 +9,6 @@ import { destroy } from 'redux-form';
 
 /**
  * Reset the Apollo and Redux stores holding data relating to elemental inline edit forms
- *
- * @param {Array} invalidFieldNames Field names that failed server-side validation
  */
 const resetStores = () => {
   // After page level saves we need to reload all the blocks from the server. We can remove
@@ -42,6 +40,8 @@ const resetStores = () => {
  */
 jQuery.entwine('ss', ($) => {
   $('.js-injector-boot .element-editor__container').entwine({
+    CurrentlySubmitting: false,
+
     onmatch() {
       const context = {};
       const ElementEditorComponent = loadComponent('ElementEditor', context);
@@ -63,32 +63,32 @@ jQuery.entwine('ss', ($) => {
 
     onunmatch() {
       // Reset the store if the user navigates to a different part of the CMS
-      // Ensure there is not a form submission in progress, as this handler will
-      // also be triggered from a form submission
-      if (this.attr('data-submitting-element-editor') !== '1') {
+      // or after submission if there are no validation errors
+      if (!$('.cms-edit-form').data('hasValidationErrors')) {
         resetStores();
       }
       ReactDOM.unmountComponentAtNode(this[0]);
     },
 
-    'from .cms-container': {
-      onsubmitform() {
-        this.attr('data-submitting-element-editor', '1');
-      }
-    },
-
     'from .cms-edit-form': {
       onaftersubmitform(event, data) {
-        this.attr('data-submitting-element-editor', null);
         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 rest if there are any validation errors from either the ElementalAreaField or a
-        // regular page field 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, whether valid or invalid
-        if (validationResult.isValid) {
+        if (validationResultPjax) {
+          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);
+          }
+        } else {
+          // No validation result returned
+          // assume everything is valid and reset stores so data is refetched.
           resetStores();
         }
       }

src/Extensions/ElementalAreasExtension.php

@@ -5,14 +5,12 @@
 use DNADesign\Elemental\Forms\ElementalAreaField;
 use DNADesign\Elemental\Models\BaseElement;
 use DNADesign\Elemental\Models\ElementalArea;
-use DNADesign\Elemental\Validators\ElementalAreasValidator;
 use SilverStripe\CMS\Model\RedirectorPage;
 use SilverStripe\CMS\Model\SiteTree;
 use SilverStripe\CMS\Model\VirtualPage;
 use SilverStripe\Core\ClassInfo;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Extensible;
-use SilverStripe\Forms\CompositeValidator;
 use SilverStripe\Forms\FieldList;
 use SilverStripe\Forms\LiteralField;
 use SilverStripe\ORM\DataExtension;
@@ -246,14 +244,6 @@ public function onBeforeWrite()
         }
     }
 
-    /**
-     * @param CompositeValidator $compositeValidator
-     */
-    public function updateCMSCompositeValidator(CompositeValidator $compositeValidator): void
-    {
-        $compositeValidator->addValidator(ElementalAreasValidator::create());
-    }
-
     /**
      * @return boolean
      */

src/Forms/EditFormFactory.php

@@ -2,12 +2,10 @@
 
 namespace DNADesign\Elemental\Forms;
 
-use DNADesign\Elemental\Models\BaseElement;
 use SilverStripe\Control\RequestHandler;
 use SilverStripe\Core\Config\Configurable;
 use SilverStripe\Forms\DefaultFormFactory;
 use SilverStripe\Forms\FieldList;
-use SilverStripe\Forms\HiddenField;
 use SilverStripe\Forms\HTMLEditor\HTMLEditorField;
 
 class EditFormFactory extends DefaultFormFactory
@@ -37,8 +35,8 @@ public function getForm(RequestHandler $controller = null, $name = self::DEFAULT
         // Namespace all fields - do this after getting getFormFields so they still get populated
         $formFields = $form->Fields();
         $this->namespaceFields($formFields, $context);
-        $this->addClassNameField($formFields, $context['Record']);
         $form->setFields($formFields);
+
         return $form;
     }
 
@@ -72,17 +70,4 @@ protected function namespaceFields(FieldList $fields, array $context)
             $field->setName($namespacedName);
         }
     }
-
-    /**
-     * @param FieldList $formFields
-     * @param BaseElement $record
-     */
-    private function addClassNameField(FieldList $formFields, BaseElement $record)
-    {
-        $fieldName = sprintf(self::FIELD_NAMESPACE_TEMPLATE, $record->ID, 'ClassName');
-        $formFields->addFieldsToTab(
-            'Root.Main',
-            new HiddenField($fieldName, 'ClassName', get_class($record))
-        );
-    }
 }

templates/DNADesign/Elemental/Forms/ElementalAreaField_holder.ss

@@ -1,7 +1,3 @@
-<div>
-    <% if $Message %><p class="alert $AlertType" role="alert" id="message-$ID">$Message</p><% end_if %>
-    <div $AttributesHTML data-schema="$SchemaData.JSON">
-        <%-- Field is rendered by React components --%>
-    </div>
+<div $AttributesHTML data-schema="$SchemaData.JSON">
+    <%-- Field is rendered by React components --%>
 </div>
-

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/Context/FixtureContext.php

@@ -84,7 +84,7 @@ public function thePageHasABrokenElement(string $type, string $pageTitle, string
      */
     public function contentBlocksAreNotInLineEditable()
     {
-        $contentBlockClass = TestElementContent::class;
+        $contentBlockClass = ElementContent::class;
         $config = <<<YAML
 ---
 Name: testonly-content-blocks-not-inline-editable

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

@@ -19,13 +19,6 @@ Feature: Edit elements in the CMS
       And I should see "Alice's Block"
       And I should see "Bob's Block"
 
-  @sboyd
-  Scenario: I retain invalid values client-side
-    When I click on block 1
-    Then I should see "Alice's Block"
-    # Then I change Title
-    # and i save the page
-
   Scenario: I can edit a non in-line editable block
     Given content blocks are not in-line editable
       And I go to "/admin/pages"
@@ -77,7 +70,8 @@ Feature: Edit elements in the CMS
       And I wait 1 second
       Then I should see a "Saved 'Charlie's Block' successfully" notice
     When I click on the caret button for block 2
-      Then I should see "New sample content"
+      Then the "Content" field for block 2 should contain "New sample content"
+      And the "Title" field for block 2 should contain "Charlie's Block"
 
   @unsavedChanges
   Scenario: I can edit inline-editable blocks and save the page as a whole
@@ -97,5 +91,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"