Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CMS users lose all blocks when attempting to save page with validation errors #764

Closed
1 task
scott1702 opened this issue Dec 16, 2019 · 6 comments
Closed
1 task

Comments

@scott1702
Copy link
Contributor

scott1702 commented Dec 16, 2019

Overview

This may be more of a framework issue than an elemental issue, but haven't investigated enough to know.

When users attempt to save/publish a page, if the page has any validation errors then any content blocks which have been created get entirely removed as the page fails to save. This can be especially frustrating if there are required fields in other tabs which the user hasn't seen yet.

Steps to reproduce:

  • Create a page with required fields and an elemental area
  • Fill out the page with numerous content blocks and leave the required field empty
  • Save and/or publish the page

Actual result:

  • The content blocks are removed as the page has failed to save due to validation error

Expected result:

  • The content blocks are retained in an unsaved state, in the same manner as other fields

Acceptance Criteria

  • When a validation error against a Page-level field is returned from attempting to save / publish the page, blocks and their unsaved contents are retained in the editor.
  • When a validation error against a Block-level field is returned from attempting to save / publish the page, blocks and their unsaved contents are retained in the editor.

Out of scope

Notes

  • We have some old PR that might fix this issue. If they don't fix the issue and are not salvageable, bring the card back into refinement.

PRs

@ScopeyNZ
Copy link
Contributor

@brynwhyman I was torn between impact/high and impact/critical here - feel free to re-adjust.

This doesn't surprise me, but it's obviously a bad bug. We had to work around the fact that SilverStripe replaces the whole form when saving with our new React editor. Redux should remember all of the form data, but it might be getting cleared on the response from the server, regardless of whether it was successful or not. I wonder if this can be relatively easily fixed by selectively clearing redux here:

https://github.com/dnadesign/silverstripe-elemental/blob/4/client/src/legacy/ElementEditor/entwine.js#L72

@GuySartorelli
Copy link
Member

In my recent experience with version 4.4.0, the blocks themselves remain but the content within the block is cleared.

@chrispenny
Copy link
Contributor

chrispenny commented Nov 19, 2020

It's worth mentioning that you will also experience the same thing if you apply validation requirements to an Element through the validate() extension point.

EG, I use the following addFieldError() method:

$validationResult->addFieldError('MyField', 'Required');

If I click "Save" in the inline editor, I get the following popup (but nothing in the inline editor):
Screen Shot 2020-11-20 at 7 41 56 AM

If I click "Save" on the page, my edits in the Element are lost, and this message is displayed in my Page edit form:
Screen Shot 2020-11-20 at 7 43 52 AM

This is likely related to this open ticket as well:
#329

As it seems that validation requirements that are specified in an Element's getCMSValidator() or (the new) getCMSCompositeValidator() are also not respected.

I think at the moment we have no way to reasonably provide validation requirements if we're using inline editing? Someone please, please correct me if I'm wrong.

Loosely related:
#840

@GuySartorelli
Copy link
Member

PRs merged. In the process of merging up between majors, then I'll tag.

@GuySartorelli
Copy link
Member

Tagged for CMS 4 (update silverstripe/admin to 1.13.2 and dnadesign/silverstripe-elemental to 4.13.1)
CMS 5.0 ran into an error after merge-up so that will need to be handled separately.

In any case, this card can now be closed, as the work directly related to it is complete.

@GuySartorelli
Copy link
Member

This has now also been tagged for CMS 5 - update silvertripe/admin to 2.0.1 and dnadesign/silverstripe-elemental to 5.0.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants