FIX Make change tracking work again for inline editable blocks

client/src/components/ElementEditor/Element.js

@@ -10,7 +10,7 @@ import { inject } from 'lib/Injector';
 import i18n from 'i18n';
 import classNames from 'classnames';
 import { connect } from 'react-redux';
-import { submit } from 'redux-form';
+import { submit, isDirty } from 'redux-form';
 import { loadElementFormStateName } from 'state/editor/loadElementFormStateName';
 import { loadElementSchemaValue } from 'state/editor/loadElementSchemaValue';
 import { publishBlockMutation } from 'state/editor/publishBlockMutation';
@@ -20,7 +20,7 @@ import { DragSource, DropTarget } from 'react-dnd';
 import { getEmptyImage } from 'react-dnd-html5-backend';
 import { elementDragSource, isOverTop } from 'lib/dragHelpers';
 import * as toastsActions from 'state/toasts/ToastsActions';
-import { addFormChanged, removeFormChanged } from 'state/unsavedForms/UnsavedFormsActions';
+import getFormState from 'lib/getFormState';
 
 export const ElementContext = createContext(null);
 
@@ -39,7 +39,6 @@ const Element = (props) => {
   const [doPublishElementAfterSave, setDoPublishElementAfterSave] = useState(false);
   const [ensureFormRendered, setEnsureFormRendered] = useState(false);
   const [formHasRendered, setFormHasRendered] = useState(false);
-  const [doDispatchAddFormChanged, setDoDispatchAddFormChanged] = useState(false);
   const [hasUnsavedChanges, setHasUnsavedChanges] = useState(false);
   const [publishBlock] = useMutation(publishBlockMutation);
 
@@ -50,7 +49,7 @@ const Element = (props) => {
     // which is confusing as the block still has unsaved changes, hence why we create
     // this state variable to track this instead
     // props.formDirty is either undefined (when pristine) or an object (when dirty)
-    const formDirty = typeof props.formDirty !== 'undefined';
+    const formDirty = props.formDirty || typeof props.formDirty === 'undefined';
     if (formDirty && !hasUnsavedChanges) {
       setHasUnsavedChanges(true);
     }
@@ -92,13 +91,6 @@ const Element = (props) => {
     }
   }, [justClickedPublishButton, formHasRendered]);
 
-  useEffect(() => {
-    if (doDispatchAddFormChanged) {
-      setDoDispatchAddFormChanged(false);
-      props.dispatchAddFormChanged();
-    }
-  }, [doDispatchAddFormChanged]);
-
   const getNoTitle = () => i18n.inject(
     i18n._t('ElementHeader.NOTITLE', 'Untitled {type} block'),
     { type: props.type.title }
@@ -143,18 +135,7 @@ const Element = (props) => {
     showPublishedElementToast(wasError);
     setDoPublishElement(false);
     setDoPublishElementAfterSave(false);
-    // Ensure that formDirty becomes falsey after publishing
-    // We need to call at a later render rather than straight away or redux-form may override this
-    // and set the form state to dirty under certain conditions
-    // setTimeout is a hackish way to do this, though I'm not sure how else we can do this
-    // The core issue is that redux-form will detect changes when a form is hydrated for the first
-    // time under certain conditions, specifically during a behat test when trying to publish a closed
-    // block when presumably the apollo cache is empty (or something like that). This happens late and
-    // there are no hooks/callbacks available after this happens the input onchange handlers are fired
-    Promise.all(refetchElementalArea())
-      .then(() => {
-        setTimeout(() => props.dispatchRemoveFormChanged(), 250);
-      });
+    refetchElementalArea();
   };
 
   // Save action
@@ -337,10 +318,6 @@ const Element = (props) => {
       if (props.type.inlineEditable) {
         setPreviewExpanded(true);
       }
-      // Ensure that formDirty remains truthy
-      // Note we need to call props.dispatchAddFormChanged() on the next render rather than straight away
-      // or it will get unset by code somewhere else, probably redux-form
-      setDoDispatchAddFormChanged(true);
       // Don't accidentally auto publish the element once validation errors are fixed
       if (doPublishElementAfterSave) {
         setDoPublishElementAfterSave(false);
@@ -435,6 +412,7 @@ const Element = (props) => {
         broken={type.broken}
         onFormSchemaSubmitResponse={handleFormSchemaSubmitResponse}
         onFormInit={() => handleFormInit(activeTab)}
+        formDirty={formDirty}
       />
     </ElementContext.Provider>
   </div>);
@@ -462,7 +440,9 @@ function mapStateToProps(state, ownProps) {
 
   const tabSetName = tabSet && tabSet.id;
   const uniqueFieldId = `element.${elementName}__${tabSetName}`;
-  const formDirty = state.unsavedForms.find((unsaved) => unsaved.name === `element.${elementName}`);
+
+  const formName = loadElementFormStateName(ownProps.element.id);
+  const formDirty = isDirty(`element.${formName}`, getFormState)(state);
 
   // Find name of the active tab in the tab set
   // Only defined once an element form is expanded for the first time
@@ -490,16 +470,6 @@ function mapDispatchToProps(dispatch, ownProps) {
       // Perform a redux-form remote-submit
       dispatch(submit(`element.${elementName}`));
     },
-    dispatchAddFormChanged() {
-      // Ensures the form identifier is in unsavedForms in the global redux state
-      // This is used to derive the formDirty prop in mapStateToProps
-      dispatch(addFormChanged(`element.${elementName}`));
-    },
-    dispatchRemoveFormChanged() {
-      // Removes the form identifier from unsavedForms in the global redux store
-      // Opposite of beheaviour of dispatchAddFormChanged()
-      dispatch(removeFormChanged(`element.${elementName}`));
-    },
     actions: {
       toasts: bindActionCreators(toastsActions, dispatch),
     },

client/src/components/ElementEditor/ElementEditor.js

@@ -68,6 +68,8 @@ class ElementEditor extends PureComponent {
 
   render() {
     const {
+      fieldName,
+      formState,
       ToolbarComponent,
       ListComponent,
       areaId,
@@ -104,6 +106,12 @@ class ElementEditor extends PureComponent {
           sharedObject={sharedObject}
         />
         <ElementDragPreview elementTypes={elementTypes} />
+        <input
+          name={fieldName}
+          type="hidden"
+          value={JSON.stringify(formState) || ''}
+          className="no-change-track"
+        />
       </div>
     );
   }

tests/Behat/features/file-upload.feature

@@ -8,12 +8,54 @@ Feature: Files can be saved in and removed from elemental blocks
     And I add an extension "SilverStripe\FrameworkTest\Elemental\Extension\FileElementalExtension" to the "DNADesign\Elemental\Models\ElementContent" class
     And I go to "/dev/build?flush"
     And a "image" "file1.jpg"
+    And a "image" "file2.jpg"
     And a "page" "Blocks Page" with a "My title" content element with "My content" content
     And the "group" "EDITOR" has permissions "Access to 'Pages' section"
     And I am logged in as a member of "EDITOR" group
     And I go to "/admin/pages"
     And I follow "Blocks Page"
 
+  Scenario: Add a file and save the block, then remove it and add a different one
+    # Add a file to the block
+    Given I click on the caret button for block 1
+    Then I should not see "file1"
+    When I click "Choose existing" in the "#Form_ElementForm_1 .uploadfield" element
+    And I press the "Back" HTML field button
+    And I click on the file named "file1" in the gallery
+    And I press the "Insert" button
+    And I press the "View actions" button
+    And I click on the ".element-editor__actions-save" element
+    Then I should see a "Saved 'My title' successfully" success toast
+    # Check we see the file both in the current page load (react state is correct) and after reloading the form
+    Then I should see "file1"
+    When I go to "/admin/pages"
+    And I follow "Blocks Page"
+    And I click on the caret button for block 1
+    Then I should see "file1"
+    # Then remove the file from the block
+    And I click on the "#Form_ElementForm_1 .uploadfield-item__remove-btn" element
+    Then I should not see "file1"
+    # Try adding the same file back
+    When I click "Choose existing" in the "#Form_ElementForm_1 .uploadfield" element
+    And I press the "Back" HTML field button
+    And I click on the file named "file1" in the gallery
+    And I press the "Insert" button
+    And I press the "View actions" button
+    # same file, so we shouldn't see the button
+    Then I should not see the save button for block 1
+    # Add a different file
+    And I click on the "#Form_ElementForm_1 .uploadfield-item__remove-btn" element
+    When I click "Choose existing" in the "#Form_ElementForm_1 .uploadfield" element
+    # Note we don't have to press "Back" here because react knows what folder we were just in before
+    And I click on the file named "file2" in the gallery
+    And I press the "Insert" button
+    And I press the "View actions" button
+    Then I should see the save button for block 1
+    And I click on the ".element-editor__actions-save" element
+    Then I should see a "Saved 'My title' successfully" success toast
+    And I should see "file2"
+    And I should not see "file1"
+
   Scenario: Add a file and save the block, then remove the file and save the block
     # Add a file to the block
     Given I click on the caret button for block 1
@@ -33,6 +75,7 @@ Feature: Files can be saved in and removed from elemental blocks
     Then I should see "file1"
     # Then remove the file from the block
     And I click on the "#Form_ElementForm_1 .uploadfield-item__remove-btn" element
+    Then I should not see "file1"
     And I press the "View actions" button
     And I click on the ".element-editor__actions-save" element
     Then I should see a "Saved 'My title' successfully" success toast

tests/Behat/features/inline-block-validation.feature

@@ -1,4 +1,4 @@
-@retry @job7
+@retry @job7 @gsat
 Feature: Blocks are validated when inline saving individual blocks
   As a CMS user
   I want to blocks have be validating when individual saving them
@@ -9,6 +9,7 @@ Feature: Blocks are validated when inline saving individual blocks
     And I add an extension "SilverStripe\FrameworkTest\Elemental\Extension\ElementContentExtension" to the "DNADesign\Elemental\Models\ElementContent" class
     And I add an extension "SilverStripe\FrameworkTest\Elemental\Extension\NumericFieldExtension" to the "SilverStripe\Forms\NumericField" class
     And a "image" "file1.jpg"
+    And a "image" "file2.jpg"
     And I go to "/dev/build?flush"
     And a "page" "Blocks Page" with a "My title" content element with "My content" content
     And the "group" "EDITOR" has permissions "Access to 'Pages' section"
@@ -29,8 +30,10 @@ Feature: Blocks are validated when inline saving individual blocks
   # be faster due to a limitation with behat testing react where changing the value of a field can
   # sometimes lead to the value of field being suffixed rather than replaced
 
+  @unsavedChanges
   Scenario: Field validation error
     When I fill in "x" for "Title" for block 1
+    And I take a screenshot after every step
     When I press the "View actions" button
     And I click on the ".element-editor__actions-save" element
     Then I should see "Title cannot be x" in the ".form__validation-message" element
@@ -47,11 +50,9 @@ Feature: Blocks are validated when inline saving individual blocks
     When I press the "View actions" button
     And I click on the ".element-editor__actions-save" element
     Then I should see "This field cannot be 1" in the ".form__validation-message" element
-    # Reset value to something valid to prevent "unsaved changes" alert
+    # Reset value to something valid and resave - validates saving after validation message works
     Then I fill in "2" for "My Int" for block 1
-    # Ensure react field is filled in before submitting
     And I wait for 1 second
-    # Need to save the whole page to stop the alert
     And I press the "Save" button
 
   Scenario: Related data validation error with ID suffix (MyPageID)
@@ -72,7 +73,7 @@ Feature: Blocks are validated when inline saving individual blocks
     And I click on the ".element-editor__actions-save" element
     Then I should see "\"My File\" is required" in the ".form__validation-message" element
     When I click "Choose existing" in the ".uploadfield" element
-    And I click on the file named "file1" in the gallery
+    And I click on the file named "file2" in the gallery
     And I press the "Insert" button
     And I press the "View actions" button
     And I click on the ".element-editor__actions-save" element
@@ -92,9 +93,7 @@ Feature: Blocks are validated when inline saving individual blocks
     And I click on the ".element-editor__actions-save" element
     Then I should see "My Field"
     And I should see "MyField cannot be x" in the ".form__validation-message" element
-    # Reset value to something valid to prevent "unsaved changes" alert
+    # Reset value to something valid and resave - validates saving after validation message works
     Then I fill in "abc" for "My Field" for block 1
-    # Ensure react field is filled in before submitting
     And I wait for 1 second
-    # Need to save the whole page to stop the alert
     And I press the "Save" button