From 2c32dc8d875b315dc5bc41609305ca11dfd45c10 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Mon, 15 May 2023 13:53:24 +1200 Subject: [PATCH] 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/dist/js/bundle.js | 2 +- client/src/legacy/ElementEditor/entwine.js | 24 ++-- composer.json | 2 +- src/Extensions/ElementalAreasExtension.php | 10 -- src/Forms/EditFormFactory.php | 17 +-- src/Validators/ElementalAreasValidator.php | 132 ------------------ .../Forms/ElementalAreaField_holder.ss | 8 +- tests/Behat/Context/FeatureContext.php | 20 ++- tests/Behat/Context/FixtureContext.php | 2 +- .../Behat/features/edit-block-element.feature | 12 +- .../Behat/features/validation-failure.feature | 52 +++++++ tests/Src/TestContentElement.php | 22 --- tests/Src/ValidationFailedExtension.php | 18 +++ 13 files changed, 103 insertions(+), 218 deletions(-) delete mode 100644 src/Validators/ElementalAreasValidator.php create mode 100644 tests/Behat/features/validation-failure.feature delete mode 100644 tests/Src/TestContentElement.php create mode 100644 tests/Src/ValidationFailedExtension.php diff --git a/client/dist/js/bundle.js b/client/dist/js/bundle.js index eff0cda3..ebc4e630 100644 --- a/client/dist/js/bundle.js +++ b/client/dist/js/bundle.js @@ -1 +1 @@ -!function(e){function t(r){if(n[r])return n[r].exports;var o=n[r]={i:r,l:!1,exports:{}};return e[r].call(o.exports,o,o.exports,t),o.l=!0,o.exports}var n={};t.m=e,t.c=n,t.i=function(e){return e},t.d=function(e,n,r){t.o(e,n)||Object.defineProperty(e,n,{configurable:!1,enumerable:!0,get:r})},t.n=function(e){var n=e&&e.__esModule?function(){return e.default}:function(){return e};return t.d(n,"a",n),n},t.o=function(e,t){return Object.prototype.hasOwnProperty.call(e,t)},t.p="",t(t.s="./client/src/bundles/bundle.js")}({"./client/src/boot/index.js":function(e,t,n){"use strict";function r(e){return e&&e.__esModule?e:{default:e}}var o=n("./client/src/boot/registerComponents.js"),a=r(o),i=n("./client/src/boot/registerTransforms.js"),l=r(i);window.document.addEventListener("DOMContentLoaded",function(){(0,a.default)(),(0,l.default)()})},"./client/src/boot/registerComponents.js":function(e,t,n){"use strict";function r(e){return e&&e.__esModule?e:{default:e}}Object.defineProperty(t,"__esModule",{value:!0});var o=n(3),a=r(o),i=n("./client/src/components/ElementEditor/Element.js"),l=r(i),u=n("./client/src/components/ElementEditor/ElementActions.js"),c=r(u),s=n("./client/src/components/ElementEditor/ElementEditor.js"),d=r(s),f=n("./client/src/components/ElementEditor/ElementList.js"),p=r(f),m=n("./client/src/components/ElementEditor/Toolbar.js"),b=r(m),y=n("./client/src/components/ElementEditor/AddNewButton.js"),v=r(y),h=n("./client/src/components/ElementEditor/Header.js"),g=r(h),E=n("./client/src/components/ElementEditor/Content.js"),_=r(E),O=n("./client/src/components/ElementEditor/Summary.js"),j=r(O),T=n("./client/src/components/ElementEditor/InlineEditForm.js"),k=r(T),w=n("./client/src/components/ElementEditor/AddElementPopover.js"),I=r(w),S=n("./client/src/components/ElementEditor/HoverBar.js"),A=r(S),P=n("./client/src/components/ElementEditor/DragPositionIndicator.js"),D=r(P),C=n("./client/src/components/TextCheckboxGroupField/TextCheckboxGroupField.js"),N=r(C);t.default=function(){a.default.component.registerMany({ElementEditor:d.default,ElementToolbar:b.default,ElementAddNewButton:v.default,ElementList:p.default,Element:l.default,ElementActions:c.default,ElementHeader:g.default,ElementContent:_.default,ElementSummary:j.default,ElementInlineEditForm:k.default,AddElementPopover:I.default,HoverBar:A.default,DragPositionIndicator:D.default,TextCheckboxGroupField:N.default})}},"./client/src/boot/registerTransforms.js":function(e,t,n){"use strict";function r(e){return e&&e.__esModule?e:{default:e}}Object.defineProperty(t,"__esModule",{value:!0});var o=n(3),a=r(o),i=n("./client/src/state/history/readOneBlockQuery.js"),l=r(i),u=n("./client/src/components/HistoricElementView/HistoricElementView.js"),c=r(u),s=n("./client/src/state/history/revertToBlockVersionMutation.js"),d=r(s),f=n("./client/src/state/editor/readBlocksForAreaQuery.js"),p=r(f),m=n("./client/src/state/editor/addElementMutation.js"),b=r(m),y=n("./client/src/components/ElementActions/ArchiveAction.js"),v=r(y),h=n("./client/src/components/ElementActions/DuplicateAction.js"),g=r(h),E=n("./client/src/components/ElementActions/PublishAction.js"),_=r(E),O=n("./client/src/components/ElementActions/SaveAction.js"),j=r(O),T=n("./client/src/components/ElementActions/UnpublishAction.js"),k=r(T);t.default=function(){a.default.transform("elemental-fieldgroup",function(e){e.component("FieldGroup.HistoryViewer.VersionDetail",c.default,"HistoricElement")},{after:"field-holders"}),a.default.transform("elements-history",function(e){e.component("HistoryViewer.Form_ItemEditForm",l.default,"ElementHistoryViewer")}),a.default.transform("blocks-history-revert",function(e){e.component("HistoryViewerToolbar.VersionedAdmin.HistoryViewer.Element.HistoryViewerVersionDetail",d.default,"BlockRevertMutation")}),a.default.transform("cms-element-editor",function(e){e.component("ElementList",p.default,"PageElements")}),a.default.transform("cms-element-adder",function(e){e.component("AddElementPopover",b.default,"ElementAddButton")}),a.default.transform("element-actions",function(e){e.component("ElementActions",j.default,"ElementActionsWithSave"),e.component("ElementActions",_.default,"ElementActionsWithPublish"),e.component("ElementActions",k.default,"ElementActionsWithUnpublish"),e.component("ElementActions",g.default,"ElementActionsWithDuplicate"),e.component("ElementActions",v.default,"ElementActionsWithArchive")})}},"./client/src/bundles/bundle.js":function(e,t,n){"use strict";n("./client/src/legacy/ElementEditor/entwine.js"),n("./client/src/boot/index.js")},"./client/src/components/ElementActions/AbstractAction.js":function(e,t,n){"use strict";function r(e){return e&&e.__esModule?e:{default:e}}Object.defineProperty(t,"__esModule",{value:!0});var o=Object.assign||function(e){for(var t=1;t0&&void 0!==arguments[0]?arguments[0]:null,t=arguments.length>1&&void 0!==arguments[1]?arguments[1]:null,n=!!e&&e.id;this.setState({dragTargetElementId:n,dragSpot:!1===t?"bottom":"top"})}},{key:"handleDragEnd",value:function(e,t){var n=this.props;(0,n.actions.handleSortBlock)(e,t,n.areaId).then(function(){var e=window.jQuery(".cms-preview");e.entwine("ss.preview")._loadUrl(e.find("iframe").attr("src"))}),this.setState({dragTargetElementId:null,dragSpot:null})}},{key:"render",value:function(){var e=this.props,t=e.fieldName,n=e.formState,r=e.ToolbarComponent,o=e.ListComponent,a=e.areaId,i=e.elementTypes,l=e.isDraggingOver,u=e.connectDropTarget,c=e.allowedElements,s=this.state,d=s.dragTargetElementId,p=s.dragSpot,m=c.map(function(e){return i.find(function(t){return t.class===e})});return u(f.default.createElement("div",{className:"element-editor"},f.default.createElement(r,{elementTypes:m,areaId:a,onDragOver:this.handleDragOver}),f.default.createElement(o,{allowedElementTypes:m,elementTypes:i,areaId:a,onDragOver:this.handleDragOver,onDragStart:this.handleDragStart,onDragEnd:this.handleDragEnd,dragSpot:p,isDraggingOver:l,dragTargetElementId:d}),f.default.createElement(T.default,{elementTypes:i}),f.default.createElement("input",{name:t,type:"hidden",value:JSON.stringify(n)||"",className:"no-change-track"})))}}]),t}(d.PureComponent);I.propTypes={fieldName:m.default.string,elementTypes:m.default.arrayOf(v.elementTypeType).isRequired,allowedElements:m.default.arrayOf(m.default.string).isRequired,areaId:m.default.number.isRequired,actions:m.default.shape({handleSortBlock:m.default.func})},t.Component=I,t.default=(0,y.compose)(w.default,(0,E.DropTarget)("element",{},function(e,t){return{connectDropTarget:e.dropTarget(),isDraggingOver:t.isOver()}}),(0,h.connect)(u),(0,b.inject)(["ElementToolbar","ElementList"],function(e,t){return{ToolbarComponent:e,ListComponent:t}},function(){return"ElementEditor"}),O.default)(I)},"./client/src/components/ElementEditor/ElementList.js":function(e,t,n){"use strict";function r(e){return e&&e.__esModule?e:{default:e}}function o(e,t){if(!(e instanceof t))throw new TypeError("Cannot call a class as a function")}function a(e,t){if(!e)throw new ReferenceError("this hasn't been initialised - super() hasn't been called");return!t||"object"!=typeof t&&"function"!=typeof t?e:t}function i(e,t){if("function"!=typeof t&&null!==t)throw new TypeError("Super expression must either be null or a function, not "+typeof t);e.prototype=Object.create(t&&t.prototype,{constructor:{value:e,enumerable:!1,writable:!0,configurable:!0}}),t&&(Object.setPrototypeOf?Object.setPrototypeOf(e,t):e.__proto__=t)}Object.defineProperty(t,"__esModule",{value:!0}),t.Component=void 0;var l=Object.assign||function(e){for(var t=1;t]*?>/g,"")).isValid&&m()}}}),e(".js-injector-boot .element-editor__container .element-form-dirty-state").entwine({onmatch:function(){e(".cms-edit-form").trigger("change")},onunmatch:function(){e(".cms-edit-form").trigger("change")}}),e(".cms-edit-form").entwine({getChangeTrackerOptions:function(){var t=void 0===this.entwineData("ChangeTrackerOptions"),n=this._super();return t&&(n=e.extend({},n),n.ignoreFieldSelector+=", .elementalarea :input:not(.element-form-dirty-state)",this.setChangeTrackerOptions(n)),n}})})},"./client/src/lib/dragHelpers.js":function(e,t,n){"use strict";Object.defineProperty(t,"__esModule",{value:!0}),t.elementDragSource=t.getDragIndicatorIndex=t.isOverTop=void 0;var r=n(15);t.isOverTop=function(e,t){var n=e.getClientOffset(),o=(0,r.findDOMNode)(t).getBoundingClientRect();return n.y1&&void 0!==arguments[1]?arguments[1]:null,n=e.blockSchema.typeName,r=Array.isArray(t)?t:a().elementTypes,o=r.find(function(e){return e.class===n||e.name===n});return e.obsoleteClassName&&(o=Object.assign({obsoleteClassName:e.obsoleteClassName},o),Object.preventExtensions(o)),o}},"./client/src/state/editor/loadElementFormStateName.js":function(e,t,n){"use strict";Object.defineProperty(t,"__esModule",{value:!0}),t.loadElementFormStateName=void 0;var r=n(12),o=function(e){return e&&e.__esModule?e:{default:e}}(r);t.loadElementFormStateName=function(){var e=arguments.length>0&&void 0!==arguments[0]?arguments[0]:null,t=o.default.getSection("DNADesign\\Elemental\\Controllers\\ElementalAreaController"),n=t.form.elementForm.formNameTemplate;return e?n.replace("{id}",e):n}},"./client/src/state/editor/loadElementSchemaValue.js":function(e,t,n){"use strict";Object.defineProperty(t,"__esModule",{value:!0}),t.loadElementSchemaValue=void 0;var r=n(12),o=function(e){return e&&e.__esModule?e:{default:e}}(r);t.loadElementSchemaValue=function(e){var t=arguments.length>1&&void 0!==arguments[1]?arguments[1]:null,n=o.default.getSection("DNADesign\\Elemental\\Controllers\\ElementalAreaController"),r=n.form.elementForm[e]||"";return t?r+"/"+t:r}},"./client/src/state/editor/publishBlockMutation.js":function(e,t,n){"use strict";Object.defineProperty(t,"__esModule",{value:!0}),t.config=t.mutation=void 0;var r=Object.assign||function(e){for(var t=1;t0&&void 0!==arguments[0]?arguments[0]:null,t=arguments.length>1&&void 0!==arguments[1]?arguments[1]:null,n=!!e&&e.id;this.setState({dragTargetElementId:n,dragSpot:!1===t?"bottom":"top"})}},{key:"handleDragEnd",value:function(e,t){var n=this.props;(0,n.actions.handleSortBlock)(e,t,n.areaId).then(function(){var e=window.jQuery(".cms-preview");e.entwine("ss.preview")._loadUrl(e.find("iframe").attr("src"))}),this.setState({dragTargetElementId:null,dragSpot:null})}},{key:"render",value:function(){var e=this.props,t=e.fieldName,n=e.formState,r=e.ToolbarComponent,o=e.ListComponent,a=e.areaId,i=e.elementTypes,l=e.isDraggingOver,u=e.connectDropTarget,c=e.allowedElements,s=this.state,d=s.dragTargetElementId,p=s.dragSpot,m=c.map(function(e){return i.find(function(t){return t.class===e})});return u(f.default.createElement("div",{className:"element-editor"},f.default.createElement(r,{elementTypes:m,areaId:a,onDragOver:this.handleDragOver}),f.default.createElement(o,{allowedElementTypes:m,elementTypes:i,areaId:a,onDragOver:this.handleDragOver,onDragStart:this.handleDragStart,onDragEnd:this.handleDragEnd,dragSpot:p,isDraggingOver:l,dragTargetElementId:d}),f.default.createElement(T.default,{elementTypes:i}),f.default.createElement("input",{name:t,type:"hidden",value:JSON.stringify(n)||"",className:"no-change-track"})))}}]),t}(d.PureComponent);I.propTypes={fieldName:m.default.string,elementTypes:m.default.arrayOf(v.elementTypeType).isRequired,allowedElements:m.default.arrayOf(m.default.string).isRequired,areaId:m.default.number.isRequired,actions:m.default.shape({handleSortBlock:m.default.func})},t.Component=I,t.default=(0,y.compose)(w.default,(0,E.DropTarget)("element",{},function(e,t){return{connectDropTarget:e.dropTarget(),isDraggingOver:t.isOver()}}),(0,h.connect)(u),(0,b.inject)(["ElementToolbar","ElementList"],function(e,t){return{ToolbarComponent:e,ListComponent:t}},function(){return"ElementEditor"}),O.default)(I)},"./client/src/components/ElementEditor/ElementList.js":function(e,t,n){"use strict";function r(e){return e&&e.__esModule?e:{default:e}}function o(e,t){if(!(e instanceof t))throw new TypeError("Cannot call a class as a function")}function a(e,t){if(!e)throw new ReferenceError("this hasn't been initialised - super() hasn't been called");return!t||"object"!=typeof t&&"function"!=typeof t?e:t}function i(e,t){if("function"!=typeof t&&null!==t)throw new TypeError("Super expression must either be null or a function, not "+typeof t);e.prototype=Object.create(t&&t.prototype,{constructor:{value:e,enumerable:!1,writable:!0,configurable:!0}}),t&&(Object.setPrototypeOf?Object.setPrototypeOf(e,t):e.__proto__=t)}Object.defineProperty(t,"__esModule",{value:!0}),t.Component=void 0;var l=Object.assign||function(e){for(var t=1;t]*?>/g,"")).isValid?(e(".cms-edit-form").data("hasValidationErrors",!1),m()):e(".cms-edit-form").data("hasValidationErrors",!0)}}}),e(".js-injector-boot .element-editor__container .element-form-dirty-state").entwine({onmatch:function(){e(".cms-edit-form").trigger("change")},onunmatch:function(){e(".cms-edit-form").trigger("change")}}),e(".cms-edit-form").entwine({getChangeTrackerOptions:function(){var t=void 0===this.entwineData("ChangeTrackerOptions"),n=this._super();return t&&(n=e.extend({},n),n.ignoreFieldSelector+=", .elementalarea :input:not(.element-form-dirty-state)",this.setChangeTrackerOptions(n)),n}})})},"./client/src/lib/dragHelpers.js":function(e,t,n){"use strict";Object.defineProperty(t,"__esModule",{value:!0}),t.elementDragSource=t.getDragIndicatorIndex=t.isOverTop=void 0;var r=n(15);t.isOverTop=function(e,t){var n=e.getClientOffset(),o=(0,r.findDOMNode)(t).getBoundingClientRect();return n.y1&&void 0!==arguments[1]?arguments[1]:null,n=e.blockSchema.typeName,r=Array.isArray(t)?t:a().elementTypes,o=r.find(function(e){return e.class===n||e.name===n});return e.obsoleteClassName&&(o=Object.assign({obsoleteClassName:e.obsoleteClassName},o),Object.preventExtensions(o)),o}},"./client/src/state/editor/loadElementFormStateName.js":function(e,t,n){"use strict";Object.defineProperty(t,"__esModule",{value:!0}),t.loadElementFormStateName=void 0;var r=n(12),o=function(e){return e&&e.__esModule?e:{default:e}}(r);t.loadElementFormStateName=function(){var e=arguments.length>0&&void 0!==arguments[0]?arguments[0]:null,t=o.default.getSection("DNADesign\\Elemental\\Controllers\\ElementalAreaController"),n=t.form.elementForm.formNameTemplate;return e?n.replace("{id}",e):n}},"./client/src/state/editor/loadElementSchemaValue.js":function(e,t,n){"use strict";Object.defineProperty(t,"__esModule",{value:!0}),t.loadElementSchemaValue=void 0;var r=n(12),o=function(e){return e&&e.__esModule?e:{default:e}}(r);t.loadElementSchemaValue=function(e){var t=arguments.length>1&&void 0!==arguments[1]?arguments[1]:null,n=o.default.getSection("DNADesign\\Elemental\\Controllers\\ElementalAreaController"),r=n.form.elementForm[e]||"";return t?r+"/"+t:r}},"./client/src/state/editor/publishBlockMutation.js":function(e,t,n){"use strict";Object.defineProperty(t,"__esModule",{value:!0}),t.config=t.mutation=void 0;var r=Object.assign||function(e){for(var t=1;t { // After page level saves we need to reload all the blocks from the server. We can remove @@ -63,33 +61,27 @@ 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 + // 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); } } }, diff --git a/composer.json b/composer.json index 622f6b95..01eb2e75 100644 --- a/composer.json +++ b/composer.json @@ -21,7 +21,7 @@ "php": "^7.4 || ^8.0", "silverstripe/framework": "^4.12@dev", "silverstripe/cms": "^4.7@dev", - "silverstripe/admin": "^1.7@dev", + "silverstripe/admin": "^1.13.2@dev", "silverstripe/versioned": "^1.7@dev", "silverstripe/versioned-admin": "^1.7@dev", "silverstripe/graphql": "^3.5 || ^4", diff --git a/src/Extensions/ElementalAreasExtension.php b/src/Extensions/ElementalAreasExtension.php index 2e511e4a..19878e89 100644 --- a/src/Extensions/ElementalAreasExtension.php +++ b/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 */ diff --git a/src/Forms/EditFormFactory.php b/src/Forms/EditFormFactory.php index 63ee4ecb..a3c46eee 100644 --- a/src/Forms/EditFormFactory.php +++ b/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)) - ); - } } diff --git a/src/Validators/ElementalAreasValidator.php b/src/Validators/ElementalAreasValidator.php deleted file mode 100644 index bfd57004..00000000 --- a/src/Validators/ElementalAreasValidator.php +++ /dev/null @@ -1,132 +0,0 @@ -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('
', $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 ''; - } -} diff --git a/templates/DNADesign/Elemental/Forms/ElementalAreaField_holder.ss b/templates/DNADesign/Elemental/Forms/ElementalAreaField_holder.ss index a0ab4e53..4b54e5c2 100644 --- a/templates/DNADesign/Elemental/Forms/ElementalAreaField_holder.ss +++ b/templates/DNADesign/Elemental/Forms/ElementalAreaField_holder.ss @@ -1,7 +1,3 @@ -
- <% if $Message %><% end_if %> -
- <%-- Field is rendered by React components --%> -
+
+ <%-- Field is rendered by React components --%>
- diff --git a/tests/Behat/Context/FeatureContext.php b/tests/Behat/Context/FeatureContext.php index 65373d8d..74df32c9 100644 --- a/tests/Behat/Context/FeatureContext.php +++ b/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); } } diff --git a/tests/Behat/Context/FixtureContext.php b/tests/Behat/Context/FixtureContext.php index 6c850452..4a3389b9 100644 --- a/tests/Behat/Context/FixtureContext.php +++ b/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 = <<Alternate HTML within element 2

" + And the "Title" field for block 2 should contain "Alice's Much Improved Block" diff --git a/tests/Behat/features/validation-failure.feature b/tests/Behat/features/validation-failure.feature new file mode 100644 index 00000000..f2776628 --- /dev/null +++ b/tests/Behat/features/validation-failure.feature @@ -0,0 +1,52 @@ +@javascript +Feature: Don't lose content when page or block is invalid + As a CMS user + I want to retain my unsaved content when a validation error occurs + So that I can fix the content and save it without recreating content + + Background: + Given I add an extension "DNADesign\Elemental\Extensions\ElementalPageExtension" to the "Page" class + And a "page" "Blocks Page" with a "Alice's Block" content element with "original content" content + And the "group" "EDITOR" has permissions "Access to 'Pages' section" + And I am logged in as a member of "EDITOR" group + + # The "unsaved changes" dialog causes errors unless this is tagged with "@unsavedChanges" + @unsavedChanges + Scenario: If a page is invalid, changes aren't lost + Given I add an extension "DNADesign\Elemental\Tests\Src\ValidationFailedExtension" to the "Page" class + And I go to "/admin/pages" + And I left click on "Blocks Page" in the tree + Then I should see a list of blocks + And I should see "Alice's Block" + And I should not see the ".element-editor-header__version-state--unsaved" element + When I click on the caret button for block 1 + And I fill in "

New sample content

" for "Content" for block 1 + And I fill in "Charlie's Block" for "Title" for block 1 + And I press the "Save" button + Then I should see a "Validation error" error toast + And I should see "Page is invalid" + And I should see the ".element-editor-header__version-state--unsaved" element + When I click on the caret button for block 1 + Then the "Content" field for block 1 should contain "New sample content" + And the "Title" field for block 1 should contain "Charlie's Block" + And I should see the ".element-editor-header__version-state--unsaved" element + + @unsavedChanges + Scenario: If a block is invalid, changes aren't lost + Given I add an extension "DNADesign\Elemental\Tests\Src\ValidationFailedExtension" to the "DNADesign\Elemental\Models\BaseElement" class + And I go to "/admin/pages" + And I left click on "Blocks Page" in the tree + Then I should see a list of blocks + And I should see "Alice's Block" + And I should not see the ".element-editor-header__version-state--unsaved" element + When I click on the caret button for block 1 + And I fill in "

New sample content

" for "Content" for block 1 + And I fill in "Charlie's Block" for "Title" for block 1 + And I press the "Save" button + Then I should see a "Validation error" error toast + And I should see "ElementContent is invalid" + And I should see the ".element-editor-header__version-state--unsaved" element + When I click on the caret button for block 1 + Then the "Content" field for block 1 should contain "New sample content" + And the "Title" field for block 1 should contain "Charlie's Block" + And I should see the ".element-editor-header__version-state--unsaved" element diff --git a/tests/Src/TestContentElement.php b/tests/Src/TestContentElement.php deleted file mode 100644 index 879c6347..00000000 --- a/tests/Src/TestContentElement.php +++ /dev/null @@ -1,22 +0,0 @@ -Content === static::INVALID_TITLE) { - $validationResult->addFieldError('Content', static::INVALID_TITLE_MESSAGE); - } - return $validationResult; - } -} diff --git a/tests/Src/ValidationFailedExtension.php b/tests/Src/ValidationFailedExtension.php new file mode 100644 index 00000000..e1e99167 --- /dev/null +++ b/tests/Src/ValidationFailedExtension.php @@ -0,0 +1,18 @@ +addFieldError('Title', sprintf(static::INVALID_TITLE_MESSAGE, ClassInfo::shortName($this->owner))); + } +}