Save action doesn't reflect dirty state after validation warning in CMS Page

[brynwhyman]

Overview

There is a scenario where the save action is not accurately reflecting that state of the CMS form - showing that it is 'Saved' when the content is still 'dirty'/ not saved to the database.

AC's

• A "CMS page form and GridFieldDetail Form" is automatically in a dirty state when it comes back from the server with a validation warning
• The publish button is automatically in a dirty state when it comes back from the server with a validation warning
• The CMS comes back in a clean state when it comes back without any validation warnings

Reproduction steps

1. Have a page with a required field (test site had it in another tab)
2. Enter content in the WYSIWYG, do not enter content in the required field
3. Attempt to save the form
   Expected result: the page does not save due to validation errors, the save action remains in a 'dirty' state as "Save"
   Actual result (bug): the page does not save due to validation errors, the save action changes to a "Saved" state while the content has not been saved
4. Refresh the page to confirm that the WYSIWYG content did not save

Version

CMS 4.6.1

Designs

Patch designs
CMS Design System - Validation errors future state

PRs

• NEW Set action button state on form validation fail silverstripe-framework#9703

[brynwhyman]

Also noting that a page template that includes an elemental area will be important when developing/ testing! ;P

[michalkleiner]

impact/medium? It seems to have the capability to confuse CMS users in a way they would think the content saved (based on the button change).

[emteknetnz]

@brynwhyman it's possible that the elemental area may make more sense to be tackled in a separate issue as elemental uses react/redux while regular page content does not. This means there kind of two validation systems that aren't necessarily aware of each other. This manifests itself as issues such as silverstripe/silverstripe-elemental#764

Still, hopefully there's a way to easily cover both use-cases using the same code in this issue.

[emteknetnz]

Related #1113

[emteknetnz]

Note: there is no central location where button state is handled, so it's unrealistic to expect that we will "fix it once, fix it for everything", not unless we basically rebuild the CMS

This bug relates specifically to Pages, so I think we should fix it only for pages, and not worry about model admins or elemental for now.

For instance, searching for "btn-primary" in the vendor folder, which is the dirty state button class, filtering by php + js files:

[emteknetnz]

Have updated ACs from "CMS Form" to "CMS page form and GridFieldDetail Form" after discussing internally about limiting the scope on this. This is based on the assumption that Pages and DataObjects have their button state managed in the same place.

[brynwhyman]

A related issue had been raised and closed as a duplicate of this issue: silverstripe/silverstripe-framework#9701

That issue had replication steps using elemental. While this issue had split elemental ACs out, it would be worth testing the elemental use case to check if the fix carried over. Otherwise, I'll create a new elemental-specific issue.

[brynwhyman]

Following on from my comment above, I've raised a related issue for elemental: silverstripe/silverstripe-elemental#840

They may need different implementations but we'll need to keep the scope of fixing this for elemental in mind