Merge pull request silverstripe#1178 from creative-commoners/pulls/5/…

…non-inline-validation

FIX Improve non-inline validation

src/Controllers/ElementalAreaController.php

@@ -139,7 +139,7 @@ public function save(array $data, Form $form): HTTPResponse
         $dataWithoutNamespaces = static::removeNamespacesFromFields($data, $element->ID);
 
         // Update and write the data object which will trigger model validation
-        $element->update($dataWithoutNamespaces);
+        $element->updateFromFormData($dataWithoutNamespaces);
         if ($element->isChanged()) {
             try {
                 $element->write();

src/Forms/ElementalAreaField.php

@@ -10,11 +10,14 @@
 use SilverStripe\Forms\CompositeField;
 use SilverStripe\Forms\FieldGroup;
 use SilverStripe\Forms\FieldList;
+use SilverStripe\Forms\Form;
 use SilverStripe\Forms\FormField;
 use SilverStripe\Forms\GridField\GridField;
 use SilverStripe\Forms\TabSet;
 use SilverStripe\ORM\DataObjectInterface;
 use Symbiote\GridFieldExtensions\GridFieldAddNewMultiClass;
+use SilverStripe\Forms\GridField\GridFieldDetailForm;
+use SilverStripe\Forms\GridField\GridFieldDetailForm_ItemRequest;
 
 class ElementalAreaField extends GridField
 {
@@ -214,33 +217,84 @@ public function setSubmittedValue($value, $data = null)
         return $this->setValue(json_decode($value ?? '', true));
     }
 
+    /**
+     * This will perform FormField validation
+     * DataObject validation will happen in saveInto() as part of $element->write()
+     */
+    public function validate($validator)
+    {
+        $result = true;
+        foreach ($this->getElementData() as $arr) {
+            /** @var BaseElement $element */
+            /** @var array $data */
+            $element = $arr['element'];
+            $data = $arr['data'];
+            $elementForm = $this->getElementForm($element);
+            if (!$elementForm) {
+                continue;
+            }
+            $elementForm->loadDataFrom($data);
+            $formValidatorResult = $elementForm->getValidator()->validate();
+            if (!$formValidatorResult->isValid()) {
+                $validator->getResult()->combineAnd($formValidatorResult);
+                $result = false;
+            }
+        }
+        return $this->extendValidationResult($result, $validator);
+    }
+
     public function saveInto(DataObjectInterface $dataObject)
     {
         parent::saveInto($dataObject);
-
-        $elementData = $this->Value();
-        $idPrefixLength = strlen(sprintf(ElementalAreaController::FORM_NAME_TEMPLATE ?? '', ''));
-
-        if (!$elementData) {
-            return;
+        foreach ($this->getElementData() as $arr) {
+            /** @var BaseElement $element */
+            /** @var array $data */
+            $element = $arr['element'];
+            $data = $arr['data'];
+            $element->updateFromFormData($data);
+            $element->write();
         }
+    }
 
-        foreach ($elementData as $form => $data) {
+    private function getElementData(): array
+    {
+        $elementData = [];
+        $value = $this->Value();
+        if (!$value) {
+            return [];
+        }
+        $idPrefixLength = strlen(sprintf(ElementalAreaController::FORM_NAME_TEMPLATE ?? '', ''));
+        foreach ($value as $form => $data) {
             // Extract the ID
             $elementId = (int) substr($form ?? '', $idPrefixLength ?? 0);
-
-            /** @var BaseElement $element */
             $element = $this->getArea()->Elements()->byID($elementId);
-
             if (!$element) {
                 // Ignore invalid elements
                 continue;
             }
-
             $data = ElementalAreaController::removeNamespacesFromFields($data, $element->ID);
-
-            $element->updateFromFormData($data);
-            $element->write();
+            $elementData[] = ['element' => $element, 'data' => $data];
         }
+        return $elementData;
+    }
+
+    private function getElementForm(BaseElement $element): ?Form
+    {
+        // This is essentially the same method used to generate a form to edit an element
+        // that a non-inline edit form will use - see GridFieldDetailForm::handleItem()
+        $requestHandler = $requestHandler = $this->getForm()->getController();
+        $gridFieldDetailForm = new GridFieldDetailForm();
+        // The validator needs to be explicitly copied from the element to the form
+        $validator = $element->getCMSCompositeValidator();
+        $gridFieldDetailForm->setValidator($validator);
+        $gridFieldDetailFormItemRequest = new GridFieldDetailForm_ItemRequest(
+            $this,
+            $gridFieldDetailForm,
+            $element,
+            $requestHandler,
+            ''
+        );
+        $form = $gridFieldDetailFormItemRequest->ItemEditForm();
+        return $form;
     }
 }

src/Forms/TextCheckboxGroupField.php

@@ -32,6 +32,49 @@ public function __construct($title = null)
         $this->setTitle($title);
     }
 
+    public function getMessage()
+    {
+        $message = parent::getMessage();
+        if ($message) {
+            return $message;
+        }
+        foreach ($this->getChildren() as $field) {
+            $message = $field->getMessage();
+            if ($message) {
+                return $message;
+            }
+        }
+        return '';
+    }
+
+    public function getMessageType()
+    {
+        $message = parent::getMessage();
+        if ($message) {
+            return parent::getMessageType();
+        }
+        foreach ($this->getChildren() as $field) {
+            if ($field->getMessage()) {
+                return $field->getMessageType();
+            }
+        }
+        return '';
+    }
+
+    public function getMessageCast()
+    {
+        $message = parent::getMessage();
+        if ($message) {
+            return parent::getMessageCast();
+        }
+        foreach ($this->getChildren() as $field) {
+            if ($field->getMessage()) {
+                return $field->getMessageCast();
+            }
+        }
+        return '';
+    }
+
     /**
      * Don't use the custom template for readonly states
      *

tests/Behat/features/individual-block-validation.feature → tests/Behat/features/inline-block-validation.feature

@@ -1,3 +1,4 @@
+@retry
 Feature: Blocks are validated when inline saving individual blocks
   As a CMS user
   I want to blocks have be validating when individual saving them
@@ -14,6 +15,10 @@ Feature: Blocks are validated when inline saving individual blocks
     And I go to "/admin/pages"
     And I follow "Blocks Page"
     And I click on the caret button for block 1
+    And I click on the "#Form_ElementForm_1_PageElements_1_MyPageID" element
+    And I click on the ".ss-searchable-dropdown-field__option:nth-of-type(2)" element
+    And I press the "View actions" button
+    And I click on the ".element-editor__actions-save" element
 
   # Note that each test is split into a seperate scenario instead a large single scenario which would
   # be faster due to a limitation with behat testing react where changing the value of a field can
@@ -51,6 +56,13 @@ Feature: Blocks are validated when inline saving individual blocks
     # Need to save the whole page to stop the alert
     And I press the "Save" button
 
+  Scenario: Related data validation error
+    When I click on the "#Form_ElementForm_1_PageElements_1_MyPageID" element
+    And I click on the ".ss-searchable-dropdown-field__option:nth-of-type(1)" element
+    And I press the "View actions" button
+    And I click on the ".element-editor__actions-save" element
+    Then I should see "\"My page\" is required" in the ".form__validation-message" element
+
   Scenario: Publishing triggers validation error
     When I fill in "x" for "Title" for block 1
     When I press the "View actions" button

tests/Behat/features/non-inline-block-validation.feature

@@ -0,0 +1,56 @@
+@retry
+Feature: Blocks are validated when non-inline saving blocks
+  As a CMS user
+  I want to blocks have be validating when non-inline saving them
+  So that I can see what content needs to be fixed
+
+  Background:
+    Given I add an extension "DNADesign\Elemental\Extensions\ElementalPageExtension" to the "Page" class
+    And I add an extension "SilverStripe\FrameworkTest\Elemental\Extension\ElementContentExtension" to the "DNADesign\Elemental\Models\ElementContent" class
+    And I add an extension "SilverStripe\FrameworkTest\Elemental\Extension\NumericFieldExtension" to the "SilverStripe\Forms\NumericField" class
+    And content blocks are not in-line editable
+    And I go to "/dev/build?flush"
+    And a "page" "Blocks Page" with a "My title" content element with "My content" content
+    And the "group" "EDITOR" has permissions "Access to 'Pages' section"
+    And I am logged in as a member of "EDITOR" group
+    And I go to "/admin/pages"
+    And I follow "Blocks Page"
+    And I click on the caret button for block 1
+
+  Scenario: Non-inline block validation
+
+    # Related has_one RequiredFields
+    When I press the "Save" button
+    Then I should see "\"My page\" is required" in the "#message-Form_ItemEditForm_MyPageID" element
+    And I click on the "#Form_ItemEditForm_MyPageID" element
+    And I click on the ".ss-searchable-dropdown-field__option:nth-of-type(2)" element
+
+    # RequiredFields on TextCheckboxGroupField (composite) field
+    When I fill in "Title" with ""
+    And I press the "Save" button
+    Then I should see "\"Title\" is required" in the "#message-Form_ItemEditForm_Title" element
+    And I fill in "Title" with "My title"
+
+    # FormField::validate()
+    When I fill in "My Int" with "1"
+    And I press the "Save" button
+    Then I should see "This field cannot be 1" in the "#message-Form_ItemEditForm_MyInt" element
+    And I fill in "My Int" with "2"
+
+    # DataObject::validate() addFieldError()
+    When I fill in "My Field" with "x"
+    And I press the "Save" button
+    Then I should see "MyField cannot be x" in the "#message-Form_ItemEditForm_MyField" element
+    And I fill in "My Field" with "lorem"
+
+    # DataObject::validate() addError()
+    When I fill in "Title" with "z"
+    And I fill in "My Field" with "z"
+    And I press the "Save" button
+    Then I should see "This is a general error message" in the "#Form_ItemEditForm_error" element
+    And I fill in "Title" with "My title"
+    And I fill in "My Field" with "lorem"
+
+    # Success message
+    When I press the "Save" button
+    Then I should see "Saved content block \"My title\"" in the "#Form_ItemEditForm_error" element

tests/Behat/features/page-save-validation.feature

@@ -0,0 +1,60 @@
+@retry
+Feature: Blocks are validated when page saving blocks
+  As a CMS user
+  I want to blocks be validated when page saving blocks
+  So that I can see what content needs to be fixed
+
+  Background:
+    Given I add an extension "DNADesign\Elemental\Extensions\ElementalPageExtension" to the "Page" class
+    And I add an extension "SilverStripe\FrameworkTest\Elemental\Extension\ElementContentExtension" to the "DNADesign\Elemental\Models\ElementContent" class
+    And I add an extension "SilverStripe\FrameworkTest\Elemental\Extension\NumericFieldExtension" to the "SilverStripe\Forms\NumericField" class
+    And I go to "/dev/build?flush"
+    And a "page" "Blocks Page" with a "My title" content element with "My content" content
+    And the "group" "EDITOR" has permissions "Access to 'Pages' section"
+    And I am logged in as a member of "EDITOR" group
+    And I go to "/admin/pages"
+    And I follow "Blocks Page"
+    And I click on the caret button for block 1
+    And I click on the "#Form_ElementForm_1_PageElements_1_MyPageID" element
+    And I click on the ".ss-searchable-dropdown-field__option:nth-of-type(2)" element
+
+  Scenario: Validation when page saving inline blocks
+
+    # Related has_one RequiredFields
+    When I click on the "#Form_ElementForm_1_PageElements_1_MyPageID" element
+    And I click on the ".ss-searchable-dropdown-field__option:nth-of-type(1)" element
+    And I press the "Save" button
+    Then I should see "\"My page\" is required" in the "#Form_EditForm_error" element
+    When I click on the caret button for block 1
+    And I click on the "#Form_ElementForm_1_PageElements_1_MyPageID" element
+    And I click on the ".ss-searchable-dropdown-field__option:nth-of-type(2)" element
+
+    # FormField::validate()
+    And I fill in "1" for "My Int" for block 1
+    And I press the "Save" button
+    Then I should see "This field cannot be 1" in the "#Form_EditForm_error" element
+    And I click on the caret button for block 1
+    And I fill in "2" for "My Int" for block 1
+
+    # DataObject::validate() addFieldError()
+    # Note that this currently has poor UX and will show the validation message on the
+    # page Title field, rather than on the element Title field. This is expected.
+    And I fill in "x" for "Title" for block 1
+    And I press the "Save" button
+    Then I should see "There are validation errors on this page, please fix them before saving or publishing." in the "#Form_EditForm_error" element
+    Then I should see "Title cannot be x" in the "#message-Form_EditForm_Title" element
+    And I click on the caret button for block 1
+    And I fill in "lorem" for "Title" for block 1
+
+    # DataObject::validate() addError()
+    And I fill in "z" for "Title" for block 1
+    And I fill in "z" for "My Field" for block 1
+    And I press the "Save" button
+    Then I should see "This is a general error message" in the "#Form_EditForm_error" element
+    And I click on the caret button for block 1
+    And I fill in "some title" for "Title" for block 1
+    And I fill in "lorem" for "My Field" for block 1
+
+    # Success message
+    When I press the "Save" button
+    Then I should see a "Saved 'Blocks Page' successfully." success toast