From e1674f84b11590d24fc8f80dde532ee0e27b708a Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Thu, 2 May 2024 11:58:00 +1200 Subject: [PATCH] FIX Improve non-inline validation --- src/Forms/ElementalAreaField.php | 81 ++++++++++++++++--- src/Forms/TextCheckboxGroupField.php | 52 ++++++++++++ src/Models/BaseElement.php | 2 + ...eature => inline-block-validation.feature} | 52 ++++++++++++ .../non-inline-block-validation.feature | 56 +++++++++++++ 5 files changed, 230 insertions(+), 13 deletions(-) rename tests/Behat/features/{individual-block-validation.feature => inline-block-validation.feature} (64%) create mode 100644 tests/Behat/features/non-inline-block-validation.feature diff --git a/src/Forms/ElementalAreaField.php b/src/Forms/ElementalAreaField.php index eb95ddb2..c7ab941d 100644 --- a/src/Forms/ElementalAreaField.php +++ b/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,85 @@ 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->validationResult(); + 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 for will use - see GridFieldDetailForm::handleItem() + $requestHandler = $requestHandler = $this->getForm()->getController(); + $gridFieldDetailForm = new GridFieldDetailForm(); + // The validator need 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; } } diff --git a/src/Forms/TextCheckboxGroupField.php b/src/Forms/TextCheckboxGroupField.php index 27c9c392..403c8d08 100644 --- a/src/Forms/TextCheckboxGroupField.php +++ b/src/Forms/TextCheckboxGroupField.php @@ -32,6 +32,58 @@ public function __construct($title = null) $this->setTitle($title); } + /** + * @return string + */ + public function getMessage() + { + $message = parent::getMessage(); + if ($message) { + return $message; + } + foreach ($this->getChildren() as $field) { + $message = $field->getMessage(); + if ($message) { + return $message; + } + } + return ''; + } + + /** + * @return string + */ + public function getMessageType() + { + $message = parent::getMessage(); + if ($message) { + return parent::getMessageType(); + } + foreach ($this->getChildren() as $field) { + if ($field->getMessage()) { + return $field->getMessageType(); + } + } + return ''; + } + + /** + * @return string + */ + 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 * diff --git a/src/Models/BaseElement.php b/src/Models/BaseElement.php index e1078558..54b8c312 100644 --- a/src/Models/BaseElement.php +++ b/src/Models/BaseElement.php @@ -654,6 +654,7 @@ public function getRenderTemplates($suffix = '') */ public function updateFromFormData($data) { + /** @var FieldList $cmsFields */ $cmsFields = $this->getCMSFields(); foreach ($data as $field => $datum) { @@ -664,6 +665,7 @@ public function updateFromFormData($data) } $field->setSubmittedValue($datum); + // $field->validate(); $field->saveInto($this); } } diff --git a/tests/Behat/features/individual-block-validation.feature b/tests/Behat/features/inline-block-validation.feature similarity index 64% rename from tests/Behat/features/individual-block-validation.feature rename to tests/Behat/features/inline-block-validation.feature index 7fe9be48..fb568771 100644 --- a/tests/Behat/features/individual-block-validation.feature +++ b/tests/Behat/features/inline-block-validation.feature @@ -14,7 +14,11 @@ 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 + # This section deals with validation when inline saving individual blocks + # # 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 # sometimes lead to the value of field being suffixed rather than replaced @@ -86,3 +90,51 @@ Feature: Blocks are validated when inline saving individual blocks And I wait for 1 second # Need to save the whole page to stop the alert And I press the "Save" button + + + # This section deals with validation when page saving inline blocks + # + # Note that unlike inline saving individual blocks, we don't need to use seperate scenarios + # which means that everything runs faster + + @retry + 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 diff --git a/tests/Behat/features/non-inline-block-validation.feature b/tests/Behat/features/non-inline-block-validation.feature new file mode 100644 index 00000000..4f45c3fb --- /dev/null +++ b/tests/Behat/features/non-inline-block-validation.feature @@ -0,0 +1,56 @@ +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 I add an extension "SilverStripe\FrameworkTest\Elemental\Extension\NonInlineEditableExtension" to the "DNADesign\Elemental\Models\ElementContent" 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 + + @retry + 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