Skip to content

Commit

Permalink
SPIKE Inline validation
Browse files Browse the repository at this point in the history
  • Loading branch information
emteknetnz committed Oct 18, 2023
1 parent 768854b commit ea856d1
Show file tree
Hide file tree
Showing 8 changed files with 4,486 additions and 53 deletions.
3,829 changes: 3,819 additions & 10 deletions client/dist/js/bundle.js

Large diffs are not rendered by default.

482 changes: 481 additions & 1 deletion client/dist/styles/bundle.css

Large diffs are not rendered by default.

13 changes: 11 additions & 2 deletions client/src/components/ElementActions/SaveAction.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';
import React, { useContext } from 'react';
import { compose } from 'redux';
import { connect } from 'react-redux';
import AbstractAction from 'components/ElementActions/AbstractAction';
Expand All @@ -7,12 +7,16 @@ import i18n from 'i18n';
import { loadElementSchemaValue } from 'state/editor/loadElementSchemaValue';
import { loadElementFormStateName } from 'state/editor/loadElementFormStateName';
import { initialize } from 'redux-form';
import { ElementContext } from 'components/ElementEditor/Element';

/**
* Using a REST backend, serialize the current form data and post it to the backend endpoint to save
* the inline edit form's data for the current block.
*/
const SaveAction = (MenuComponent) => (props) => {
const failureHandlers = useContext(ElementContext);
console.log(failureHandlers);

if (!props.expandable || props.type.broken) {
// Some elemental blocks can not be edited inline (e.g. User form blocks)
// We don't want to add a "Save" action for those blocks.
Expand Down Expand Up @@ -70,7 +74,7 @@ const SaveAction = (MenuComponent) => (props) => {
type: 'success'
});
})
.catch(() => {
.catch(e => {
$.noticeAdd({
text: i18n.inject(
i18n._t(
Expand All @@ -82,6 +86,11 @@ const SaveAction = (MenuComponent) => (props) => {
stay: false,
type: 'error'
});
e.response.json()
.then(responseJson => {
// console.log(responseJson.errors); // add this as an arg to a callback
failureHandlers.onFailedSave(responseJson.errors);
});
});
};

Expand Down
87 changes: 54 additions & 33 deletions client/src/components/ElementEditor/Element.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* global window */

import React, { Component } from 'react';
import React, { Component, createContext } from 'react';
import PropTypes from 'prop-types';
import { elementType } from 'types/elementType';
import { elementTypeType } from 'types/elementTypeType';
Expand Down Expand Up @@ -215,6 +215,21 @@ class Element extends Component {
}
}

getFailureHandlers() {
// using method to create object rather then defining object directly in render()
// to prevent linting warning about "consider useMemo() instead"
return {
onFailedSave: (errors) => {
console.log('onFailedSave');
console.log('errors is');
console.log(errors);
// prevent unused variables linting warning
const noop = () => {};
noop(errors);
}
};
}

render() {
const {
element,
Expand Down Expand Up @@ -248,6 +263,8 @@ class Element extends Component {
this.getVersionedStateClassName()
);

const failureHandlers = this.getFailureHandlers();

const content = connectDropTarget(<div
className={elementClassNames}
onClick={this.handleExpand}
Expand All @@ -257,40 +274,41 @@ class Element extends Component {
title={this.getLinkTitle(type)}
key={element.id}
>
<HeaderComponent
element={element}
type={type}
areaId={areaId}
expandable={type.inlineEditable}
link={link}
previewExpanded={previewExpanded && !childRenderingError}
handleEditTabsClick={this.handleTabClick}
activeTab={activeTab}
disableTooltip={isDragging}
onDragEnd={onDragEnd}
/>

{
!childRenderingError &&
<ContentComponent
id={element.id}
fileUrl={element.blockSchema.fileURL}
fileTitle={element.blockSchema.fileTitle}
content={this.getSummary(element, type)}
previewExpanded={previewExpanded && !isDragging}
<ElementContext.Provider value={failureHandlers}>
<HeaderComponent
element={element}
type={type}
areaId={areaId}
expandable={type.inlineEditable}
link={link}
previewExpanded={previewExpanded && !childRenderingError}
handleEditTabsClick={this.handleTabClick}
activeTab={activeTab}
onFormInit={() => this.updateFormTab(activeTab)}
handleLoadingError={this.handleLoadingError}
broken={type.broken}
disableTooltip={isDragging}
onDragEnd={onDragEnd}
failureHandlers={failureHandlers}
/>
}

{
childRenderingError &&
<div className="alert alert-danger mt-2">
{i18n._t('ElementalElement.CHILD_RENDERING_ERROR', 'Something went wrong with this block. Please try saving and refreshing the CMS.')}
</div>
}
{
!childRenderingError &&
<ContentComponent
id={element.id}
fileUrl={element.blockSchema.fileURL}
fileTitle={element.blockSchema.fileTitle}
content={this.getSummary(element, type)}
previewExpanded={previewExpanded && !isDragging}
activeTab={activeTab}
onFormInit={() => this.updateFormTab(activeTab)}
handleLoadingError={this.handleLoadingError}
broken={type.broken}
/>
}
{
childRenderingError &&
<div className="alert alert-danger mt-2">
{i18n._t('ElementalElement.CHILD_RENDERING_ERROR', 'Something went wrong with this block. Please try saving and refreshing the CMS.')}
</div>
}
</ElementContext.Provider>
</div>);

if (!previewExpanded) {
Expand Down Expand Up @@ -342,6 +360,9 @@ function mapDispatchToProps(dispatch, ownProps) {
};
}

const ElementContext = createContext({});
export { ElementContext as ElementContext };

Element.propTypes = {
element: elementType,
type: elementTypeType.isRequired,
Expand Down
2 changes: 1 addition & 1 deletion client/src/components/ElementEditor/ElementActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ ElementActions.propTypes = {
name: PropTypes.string,
})),
handleEditTabsClick: PropTypes.func.isRequired,
expandable: PropTypes.bool
expandable: PropTypes.bool,
};

ElementActions.defaultProps = {
Expand Down
96 changes: 90 additions & 6 deletions src/Controllers/ElementalAreaController.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
use SilverStripe\Core\Convert;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Forms\Form;
use SilverStripe\ORM\ValidationException;
use SilverStripe\Security\SecurityToken;
use SilverStripe\ORM\ValidationResult;

/**
* Controller for "ElementalArea" - handles loading and saving of in-line edit forms in an elemental area in admin
Expand Down Expand Up @@ -115,8 +117,9 @@ public function getElementForm($elementID)
*/
public function apiSaveForm(HTTPRequest $request)
{
$id = $this->urlParams['ID'] ?? 0;
// Validate required input data
if (!isset($this->urlParams['ID'])) {
if ($id === 0) {
$this->jsonError(400);
return null;
}
Expand All @@ -139,7 +142,7 @@ public function apiSaveForm(HTTPRequest $request)
}

/** @var BaseElement $element */
$element = BaseElement::get()->byID($this->urlParams['ID']);
$element = BaseElement::get()->byID($id);
// Ensure the element can be edited by the current user
if (!$element || !$element->canEdit()) {
$this->jsonError(403);
Expand All @@ -149,6 +152,45 @@ public function apiSaveForm(HTTPRequest $request)
// Remove the pseudo namespaces that were added by the form factory
$data = $this->removeNamespacesFromFields($data, $element->ID);

// create a temporary Form to use for validation - will contain existing dataobject values
$form = $this->getElementForm($id);
// remove element namespaces from fields so that something like RequiredFields('Title') works
// element namespaces are added in DNADesign\Elemental\Forms\EditFormFactory
foreach ($form->Fields()->flattenFields() as $field) {
$rx = '#^PageElements_[0-9]+_#';
$namespacedName = $field->getName();
if (!preg_match($rx, $namespacedName)) {
continue;
}
$regularName = preg_replace($rx, '', $namespacedName);
// If there's an existing field with the same name, remove it
// this is probably a workaround for EditFormFactory creating too many fields?
// e.g. for element #2 there's a "Title" field and a "PageElements_2_Title" field
// same with "SecurityID" and "PageElements_2_SecurityID"
// possibly this would be better to just remove fields if they match the rx, not sure,
// this approach seems more conservative
if ($form->Fields()->flattenFields()->fieldByName($regularName)) {
$form->Fields()->removeByName($regularName);
}
// update the name of the field
$field->setName($regularName);
}
// merge submitted data into the form
$form->loadDataFrom($data);

$errorMessages = [];

// Validate the Form
/** @var ValidationResult|null $validationResult */
$validator = $form->getValidator();
if ($validator) {
$validationResult = $validator->validate();
if ($validationResult && !$validationResult->isValid()) {
// add error messages from Form validation
$errorMessages = array_merge($errorMessages, $validationResult->getMessages());
}
}

try {
$updated = false;

Expand All @@ -159,11 +201,18 @@ public function apiSaveForm(HTTPRequest $request)
// Track changes so we can return to the client
$updated = true;
}
} catch (Exception $ex) {
Injector::inst()->get(LoggerInterface::class)->debug($ex->getMessage());
} catch (ValidationException $e) {
// add error messages from DataObject validation
$errorMessages = array_merge($errorMessages, $e->getResult()->getMessages());
}

$this->jsonError(500);
return null;
if (count($errorMessages) > 0) {
// re-prefix fields before sending json error
foreach ($errorMessages as $key => &$message) {
$fieldName = $message['fieldName'];
$message['fieldName'] = "PageElements_{$id}_{$fieldName}";
}
$this->jsonError(400, $errorMessages);
}

$body = json_encode([
Expand All @@ -173,6 +222,41 @@ public function apiSaveForm(HTTPRequest $request)
return HTTPResponse::create($body)->addHeader('Content-Type', 'application/json');
}

/**
* Override LeftAndMain::jsonError() to allow multiple error messages
*
* This is fairly ridicious, it's really for demo purposes
* We could use this though we'd be better off updating LeftAndMain::jsonError() to support multiple errors
*/
public function jsonError($errorCode, $errorMessage = null)
{
try {
parent::jsonError($errorCode, $errorMessage);
} catch (HTTPResponse_Exception $e) {
// JeftAndMain::jsonError() will always throw this exception
if (!is_array($errorMessage)) {
// single error, no need to update
throw $e;
}
// multiple errors
$response = $e->getResponse();
$json = json_decode($response->getBody(), true);
$errors = [];
foreach ($errorMessage as $message) {
$errors[] = [
'type' => 'error',
'code' => $errorCode,
'value' => $message
];
}
$json['errors'] = $errors;
$body = json_encode($json);
$response->setBody($body);
$e->setResponse($response);
throw $e;
}
}

/**
* Provides action control for form fields that are request handlers when they're used in an in-line edit form.
*
Expand Down
28 changes: 28 additions & 0 deletions src/Forms/EditFormFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
use SilverStripe\Forms\DefaultFormFactory;
use SilverStripe\Forms\FieldList;
use SilverStripe\Forms\HTMLEditor\HTMLEditorField;
use SilverStripe\ORM\DataObject;
use SilverStripe\Forms\RequiredFields;

class EditFormFactory extends DefaultFormFactory
{
Expand Down Expand Up @@ -55,6 +57,32 @@ protected function getFormFields(RequestHandler $controller = null, $name, $cont
return $fields;
}

protected function getFormValidator(RequestHandler $controller = null, $name, $context = [])
{
$compositeValidator = parent::getFormValidator($controller, $name, $context);
if (!$compositeValidator) {
return null;
}
$id = $context['Record']->ID;
foreach ($compositeValidator->getValidators() as $validator) {
if (is_a($validator, RequiredFields::class)) {
$requiredFields = $validator->getRequired();
foreach ($requiredFields as $requiredField) {
// Add more required fields with appendend field prefixes
// this is done so that front end validation works, at least for RequiredFields
// you'll end up with two sets of required fields:
// - Title -- used for backend validation when inline saving an element
// - PageElements_<ElementID>_Title -- used for frontend js validation onchange()
// note that if a required field is "missing" from submitted data, this is not a
// problem so it's OK to add extra fields here just for frontend validation
$prefixedRequiredField = "PageElements_{$id}_$requiredField";
$validator->addRequiredField($prefixedRequiredField);
}
}
}
return $compositeValidator;
}

/**
* Given a {@link FieldList}, give all fields a unique name so they can be used in the same context as
* other elemental edit forms and the page (or other DataObject) that owns them.
Expand Down
2 changes: 2 additions & 0 deletions src/Models/BaseElement.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
use SilverStripe\View\Requirements;
use SilverStripe\ORM\CMSPreviewable;
use SilverStripe\Core\Config\Config;
use SilverStripe\Forms\CompositeValidator;
use SilverStripe\Forms\RequiredFields;
use SilverStripe\ORM\DataObjectSchema;

/**
Expand Down

0 comments on commit ea856d1

Please sign in to comment.