Skip to content

Commit

Permalink
FIX Retain value that failed validation on client side
Browse files Browse the repository at this point in the history
  • Loading branch information
emteknetnz authored and GuySartorelli committed May 10, 2023
1 parent 1d6a386 commit fc19dc9
Show file tree
Hide file tree
Showing 10 changed files with 227 additions and 33 deletions.
2 changes: 1 addition & 1 deletion client/dist/js/bundle.js

Large diffs are not rendered by default.

33 changes: 27 additions & 6 deletions client/src/legacy/ElementEditor/entwine.js
Expand Up @@ -9,6 +9,8 @@ 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
Expand Down Expand Up @@ -60,16 +62,35 @@ jQuery.entwine('ss', ($) => {
},

onunmatch() {
resetStores();
// 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') {
resetStores();
}
ReactDOM.unmountComponentAtNode(this[0]);
},

/**
* Invalidate cache after the form is submitted to force apollo to re-fetch.
*/
'from .cms-container': {
onsubmitform() {
this.attr('data-submitting-element-editor', '1');
}
},

'from .cms-edit-form': {
onaftersubmitform() {
resetStores();
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) {
resetStores();
}
}
},
});
Expand Down
10 changes: 10 additions & 0 deletions src/Extensions/ElementalAreasExtension.php
Expand Up @@ -5,12 +5,14 @@
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;
Expand Down Expand Up @@ -244,6 +246,14 @@ public function onBeforeWrite()
}
}

/**
* @param CompositeValidator $compositeValidator
*/
public function updateCMSCompositeValidator(CompositeValidator $compositeValidator): void
{
$compositeValidator->addValidator(ElementalAreasValidator::create());
}

/**
* @return boolean
*/
Expand Down
17 changes: 16 additions & 1 deletion src/Forms/EditFormFactory.php
Expand Up @@ -2,10 +2,12 @@

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
Expand Down Expand Up @@ -35,8 +37,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;
}

Expand Down Expand Up @@ -70,4 +72,17 @@ 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))
);
}
}
132 changes: 132 additions & 0 deletions src/Validators/ElementalAreasValidator.php
@@ -0,0 +1,132 @@
<?php

namespace DNADesign\Elemental\Validators;

use DNADesign\Elemental\Controllers\ElementalAreaController;
use DNADesign\Elemental\Forms\EditFormFactory;
use DNADesign\Elemental\Models\BaseElement;
use DNADesign\Elemental\Models\ElementalArea;
use SilverStripe\Core\Config\Config;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Forms\Validator;
use SilverStripe\ORM\DataObject;
use SilverStripe\ORM\ValidationResult;

class ElementalAreasValidator extends Validator
{
/**
* @param array $data
*/
public function php($data)
{
$valid = true;
$areaErrors = [];
$areaFieldNames = $this->getElementalAreaFieldNames($data['ClassName']);
foreach ($areaFieldNames as $areaFieldName) {
$elementsData = $data[$areaFieldName] ?? [];
if (empty($elementsData)) {
continue;
}
foreach (array_values($elementsData) as $elementData) {
$elementID = $this->getElementID($elementData);
if (!$elementID) {
continue;
}
$key = sprintf(EditFormFactory::FIELD_NAMESPACE_TEMPLATE, $elementID, 'ClassName');
$className = $elementData[$key] ?? '';
if (!$className) {
continue;
}
/** @var BaseElement $element */
$element = DataObject::get_by_id($className, $elementID, false);
if (!$element) {
continue;
}
$originalTitle = $element->Title ??
sprintf('(Untitled %s)', ucfirst($element->config()->get('singular_name')));
$formData = ElementalAreaController::removeNamespacesFromFields($elementData, $elementID);
$element->updateFromFormData($formData);
/** @var ValidationResult $validationResult */
$validationResult = $element->validate();
if ($validationResult->isValid()) {
continue;
}
if (!array_key_exists($areaFieldName, $areaErrors)) {
$areaErrors[$areaFieldName] = [
'The elements below have the following errors:' // TODO _t()
];
}
foreach ($validationResult->getMessages() as $message) {
$this->validationError(
"PageElements_{$elementID}_{$message['fieldName']}",
$message['message'],
$message['messageType'],
$message['messageCast']
);
$areaErrors[$areaFieldName][] = sprintf(
'%s - %s',
$originalTitle,
$message['message']
);
}
$valid = false;
}
}
if (!$valid) {
foreach ($areaErrors as $areaFieldName => $errors) {
$this->validationError(
$areaFieldName,
implode('<br>', $errors),
ValidationResult::TYPE_ERROR,
ValidationResult::CAST_HTML
);
}
// TODO: see what happens when you have multiple cms tabs
// Show a generic form message. Ideally this would be done in admin LeftAndMain.EditForm.js
// TODO: this is defined in en.js, needs to be in en.yml too (preferably admin, not elemental)
$msg = _t(
'VALIDATION_ERRORS_ON_PAGE',
'There are validation errors on this page, please fix them before saving or publishing.'
);
// If message above is change to javascript, instead set a blank string here to hide the
// generic form message by overriding any PageElement_3_Title type of message which will
// show as a generic form message since it won't match dataFieldByName($field) in
// Form::loadMessageFrom($data)
$this->validationError('GenericFormMessage', $msg);
}
return $valid;
}

/**
* @param string $parentClassName
* @return array
*/
private function getElementalAreaFieldNames(string $parentClassName): array
{
$fieldNames = [];
$hasOnes = Config::inst()->get($parentClassName, 'has_one');
foreach ($hasOnes as $fieldName => $className) {
if (!(Injector::inst()->get($className) instanceof ElementalArea)) {
continue;
}
$fieldNames[] = $fieldName;
}
return $fieldNames;
}

/**
* @param array $elementData
* @return string
*/
private function getElementID(array $elementData): string
{
foreach (array_keys($elementData) as $key) {
$rx = str_replace(['%d', '%s'], ['([0-9]+)', '(.+)'], EditFormFactory::FIELD_NAMESPACE_TEMPLATE);
if (!preg_match("#^{$rx}$#", $key, $match)) {
continue;
}
return $match[1];
}
return '';
}
}
@@ -1,3 +1,7 @@
<div $AttributesHTML data-schema="$SchemaData.JSON">
<%-- Field is rendered by React components --%>
<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>

2 changes: 1 addition & 1 deletion tests/Behat/Context/FixtureContext.php
Expand Up @@ -84,7 +84,7 @@ public function thePageHasABrokenElement(string $type, string $pageTitle, string
*/
public function contentBlocksAreNotInLineEditable()
{
$contentBlockClass = ElementContent::class;
$contentBlockClass = TestElementContent::class;
$config = <<<YAML
---
Name: testonly-content-blocks-not-inline-editable
Expand Down
7 changes: 7 additions & 0 deletions tests/Behat/features/edit-block-element.feature
Expand Up @@ -19,6 +19,13 @@ 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"
Expand Down
22 changes: 22 additions & 0 deletions tests/Src/TestContentElement.php
@@ -0,0 +1,22 @@
<?php

namespace DNADesign\Elemental\Tests\Src;

use DNADesign\Elemental\Models\ElementContent;
use SilverStripe\Dev\TestOnly;

class TestContentElement extends ElementContent implements TestOnly
{
public const INVALID_TITLE = 'INVALID_TITLE';

public const INVALID_TITLE_MESSAGE = 'INVALID_TITLE_MESSAGE';

public function validate()
{
$validationResult = parent::validate();
if ($this->Content === static::INVALID_TITLE) {
$validationResult->addFieldError('Content', static::INVALID_TITLE_MESSAGE);
}
return $validationResult;
}
}
27 changes: 5 additions & 22 deletions yarn.lock
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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=
Expand Down Expand Up @@ -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=
Expand Down

0 comments on commit fc19dc9

Please sign in to comment.