ENH Refactor Element to use mutation hook

silverstripe · Apr 22, 2024 · c0a0206 · c0a0206
1 parent e5cc486
commit c0a0206
Show file tree

Hide file tree

Showing 9 changed files with 192 additions and 246 deletions.
diff --git a/client/dist/js/bundle.js b/client/dist/js/bundle.js
diff --git a/client/src/components/ElementActions/PublishAction.js b/client/src/components/ElementActions/PublishAction.js
@@ -1,31 +1,19 @@
 /* global window */
-import React, { useContext, useEffect } from 'react';
-import { compose } from 'redux';
+import React, { useContext } from 'react';
 import AbstractAction from 'components/ElementActions/AbstractAction';
-import publishBlockMutation from 'state/editor/publishBlockMutation';
 import i18n from 'i18n';
-import { ElementContext } from 'components/ElementEditor/Element';
+import { ElementContext } from 'components/ElementEditor/ElementContext';
 
 /**
  * Adds the elemental menu action to publish a draft/modified block
  */
 const PublishAction = (MenuComponent) => (props) => {
   const {
-    doPublishElement,
     formDirty,
-    formHasRendered,
-    onAfterPublish,
     onPublishButtonClick,
   } = useContext(ElementContext);
 
-  const { element, actions } = props;
-
-  const publishElement = () => {
-    // handlePublishBlock is a graphql mutation defined in publishBlockMutation.js
-    actions.handlePublishBlock(element.id)
-      .then(() => onAfterPublish(false))
-      .catch(() => onAfterPublish(true));
-  };
+  const { element } = props;
 
   const handleClick = (event) => {
     event.stopPropagation();
@@ -46,12 +34,6 @@ const PublishAction = (MenuComponent) => (props) => {
     toggle: props.toggle,
   };
 
-  useEffect(() => {
-    if (formHasRendered && doPublishElement) {
-      publishElement();
-    }
-  }, [formHasRendered, doPublishElement]);
-
   if (props.type.broken) {
     // Don't allow this action for a broken element.
     return (
@@ -69,7 +51,4 @@ const PublishAction = (MenuComponent) => (props) => {
 
 export { PublishAction as Component };
 
-export default compose(
-  publishBlockMutation,
-  PublishAction
-);
+export default PublishAction;
diff --git a/client/src/components/ElementActions/SaveAction.js b/client/src/components/ElementActions/SaveAction.js
@@ -1,15 +1,11 @@
-import React, { useContext, useEffect } from 'react';
+import React, { useContext } from 'react';
 import AbstractAction from 'components/ElementActions/AbstractAction';
 import i18n from 'i18n';
-import { ElementContext } from 'components/ElementEditor/Element';
+import { ElementContext } from 'components/ElementEditor/ElementContext';
 
 const SaveAction = (MenuComponent) => (props) => {
   const {
-    doSaveElement,
     onSaveButtonClick,
-    onAfterSave,
-    submitForm,
-    formHasRendered,
     formDirty,
   } = useContext(ElementContext);
 
@@ -18,24 +14,13 @@ const SaveAction = (MenuComponent) => (props) => {
     onSaveButtonClick();
   };
 
-  const saveElement = () => {
-    submitForm();
-    onAfterSave();
-  };
-
   const newProps = {
     title: i18n._t('ElementSaveAction.SAVE', 'Save'),
     className: 'element-editor__actions-save',
     onClick: handleClick,
     toggle: props.toggle,
   };
 
-  useEffect(() => {
-    if (formHasRendered && doSaveElement) {
-      saveElement();
-    }
-  }, [formHasRendered, doSaveElement]);
-
   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.

diff --git a/client/src/components/ElementActions/tests/PublishAction-test.js b/client/src/components/ElementActions/tests/PublishAction-test.js
@@ -3,7 +3,7 @@
 import React from 'react';
 import { fireEvent, render } from '@testing-library/react';
 import { Component as PublishAction } from '../PublishAction';
-import { ElementContext } from '../../ElementEditor/Element';
+import { ElementContext } from '../../ElementEditor/ElementContext';
 
 window.jQuery = {
   noticeAdd: () => null
@@ -101,22 +101,21 @@ test('Clicking button calls onPublishButtonClick', () => {
   expect(onPublishButtonClick).toHaveBeenCalled();
 });
 
-test('Do trigger graphql mutation if doPublishElement is true and formHasRendered is true', () => {
-  const handlePublishBlock = jest.fn(() => Promise.resolve());
-  render(
-    <ElementContext.Provider value={makeProviderValue({
-      doPublishElement: true,
-      formHasRendered: true,
-    })}
-    >
-      <ActionComponent {...makeProps({ actions: { handlePublishBlock } })}/>
-    </ElementContext.Provider>
-  );
-  expect(handlePublishBlock).toHaveBeenCalledWith(123);
-});
+// test('Do trigger graphql mutation if doPublishElement is true and formHasRendered is true', () => {
+//   const handlePublishBlock = jest.fn(() => Promise.resolve());
+//   render(
+//     <ElementContext.Provider value={makeProviderValue({
+//       doPublishElement: true,
+//       formHasRendered: true,
+//     })}
+//     >
+//       <ActionComponent {...makeProps({ actions: { handlePublishBlock } })}/>
+//     </ElementContext.Provider>
+//   );
+//   expect(handlePublishBlock).toHaveBeenCalledWith(123);
+// });
 
 test('Do not trigger graphql mutation if doPublishElement is true and formHasRendered is false', () => {
-  // handlePublishBlock is a graphql mutation defined in publishBlockMutation.js
   const handlePublishBlock = jest.fn(() => Promise.resolve());
   render(
     <ElementContext.Provider value={makeProviderValue({
@@ -144,28 +143,28 @@ test('Do not trigger graphql mutation if doPublishElement is false and formHasRe
   expect(handlePublishBlock).not.toHaveBeenCalled();
 });
 
-test('onAfterPublish is called after graphql mutation', async () => {
-  let value = 1;
-  const handlePublishBlock = jest.fn(() => {
-    value = 2;
-    return Promise.resolve();
-  });
-  const onAfterPublish = jest.fn(() => {
-    value = 3;
-  });
-  render(
-    <ElementContext.Provider value={makeProviderValue({
-      doPublishElement: true,
-      formHasRendered: true,
-      onAfterPublish,
-    })}
-    >
-      <ActionComponent {...makeProps({ actions: { handlePublishBlock } })}/>
-    </ElementContext.Provider>
-  );
-  // This is required to ensure the resolved promised returned by handlePublishBlock is handled
-  await new Promise(resolve => setTimeout(resolve, 0));
-  expect(handlePublishBlock).toHaveBeenCalled();
-  expect(onAfterPublish).toHaveBeenCalled();
-  expect(value).toBe(3);
-});
+// test('onAfterPublish is called after graphql mutation', async () => {
+//   let value = 1;
+//   const handlePublishBlock = jest.fn(() => {
+//     value = 2;
+//     return Promise.resolve();
+//   });
+//   const onAfterPublish = jest.fn(() => {
+//     value = 3;
+//   });
+//   render(
+//     <ElementContext.Provider value={makeProviderValue({
+//       doPublishElement: true,
+//       formHasRendered: true,
+//       onAfterPublish,
+//     })}
+//     >
+//       <ActionComponent {...makeProps({ actions: { handlePublishBlock } })}/>
+//     </ElementContext.Provider>
+//   );
+//   // This is required to ensure the resolved promised returned by handlePublishBlock is handled
+//   await new Promise(resolve => setTimeout(resolve, 0));
+//   expect(handlePublishBlock).toHaveBeenCalled();
+//   expect(onAfterPublish).toHaveBeenCalled();
+//   expect(value).toBe(3);
+// });
diff --git a/client/src/components/ElementActions/tests/SaveAction-test.js b/client/src/components/ElementActions/tests/SaveAction-test.js
@@ -4,7 +4,7 @@
 import React from 'react';
 import { fireEvent, render } from '@testing-library/react';
 import { Component as SaveAction } from '../SaveAction';
-import { ElementContext } from '../../ElementEditor/Element';
+import { ElementContext } from '../../ElementEditor/ElementContext';
 
 function makeProps(obj = {}) {
   return {
@@ -28,11 +28,7 @@ function makeProps(obj = {}) {
 
 function makeProviderValue(obj = {}) {
   return {
-    formHasRendered: false,
-    onAfterSave: () => {},
-    doSaveElement: false,
     onSaveButtonClick: () => {},
-    submitForm: () => {},
     formDirty: true,
     ...obj,
   };
@@ -99,71 +95,71 @@ test('Clicking button calls onSaveButtonClick', () => {
   expect(onSaveButtonClick).toHaveBeenCalled();
 });
 
-test('submitForm is called if formHasRendered is true and doSaveElement is true', () => {
-  const submitForm = jest.fn();
-  render(
-    <ElementContext.Provider value={makeProviderValue({
-      formHasRendered: true,
-      doSaveElement: true,
-      submitForm,
-    })}
-    >
-      <ActionComponent {...makeProps()} />
-    </ElementContext.Provider>
-  );
-  expect(submitForm).toHaveBeenCalled();
-});
+// test('submitForm is called if formHasRendered is true and doSaveElement is true', () => {
+//   const submitForm = jest.fn();
+//   render(
+//     <ElementContext.Provider value={makeProviderValue({
+//       formHasRendered: true,
+//       doSaveElement: true,
+//       submitForm,
+//     })}
+//     >
+//       <ActionComponent {...makeProps()} />
+//     </ElementContext.Provider>
+//   );
+//   expect(submitForm).toHaveBeenCalled();
+// });
 
-test('submitForm is not called if formHasRendered is false and doSaveElement is true', () => {
-  const submitForm = jest.fn();
-  render(
-    <ElementContext.Provider value={makeProviderValue({
-      formHasRendered: false,
-      doSaveElement: true,
-      submitForm,
-    })}
-    >
-      <ActionComponent {...makeProps()} />
-    </ElementContext.Provider>
-  );
-  expect(submitForm).not.toHaveBeenCalled();
-});
+// test('submitForm is not called if formHasRendered is false and doSaveElement is true', () => {
+//   const submitForm = jest.fn();
+//   render(
+//     <ElementContext.Provider value={makeProviderValue({
+//       formHasRendered: false,
+//       doSaveElement: true,
+//       submitForm,
+//     })}
+//     >
+//       <ActionComponent {...makeProps()} />
+//     </ElementContext.Provider>
+//   );
+//   expect(submitForm).not.toHaveBeenCalled();
+// });
 
-test('submitForm is not called if formHasRendered is true and doSaveElement is false', () => {
-  const submitForm = jest.fn();
-  render(
-    <ElementContext.Provider value={makeProviderValue({
-      formHasRendered: true,
-      doSaveElement: false,
-      submitForm,
-    })}
-    >
-      <ActionComponent {...makeProps()} />
-    </ElementContext.Provider>
-  );
-  expect(submitForm).not.toHaveBeenCalled();
-});
+// test('submitForm is not called if formHasRendered is true and doSaveElement is false', () => {
+//   const submitForm = jest.fn();
+//   render(
+//     <ElementContext.Provider value={makeProviderValue({
+//       formHasRendered: true,
+//       doSaveElement: false,
+//       submitForm,
+//     })}
+//     >
+//       <ActionComponent {...makeProps()} />
+//     </ElementContext.Provider>
+//   );
+//   expect(submitForm).not.toHaveBeenCalled();
+// });
 
-test('onAfterSave is called after submitForm', () => {
-  let value = 1;
-  const submitForm = jest.fn(() => {
-    value = 2;
-  });
-  const onAfterSave = jest.fn(() => {
-    value = 3;
-  });
-  render(
-    <ElementContext.Provider value={makeProviderValue({
-      formHasRendered: true,
-      doSaveElement: true,
-      submitForm,
-      onAfterSave,
-    })}
-    >
-      <ActionComponent {...makeProps()}/>
-    </ElementContext.Provider>
-  );
-  expect(submitForm).toHaveBeenCalled();
-  expect(onAfterSave).toHaveBeenCalled();
-  expect(value).toBe(3);
-});
+// test('onAfterSave is called after submitForm', () => {
+//   let value = 1;
+//   const submitForm = jest.fn(() => {
+//     value = 2;
+//   });
+//   const onAfterSave = jest.fn(() => {
+//     value = 3;
+//   });
+//   render(
+//     <ElementContext.Provider value={makeProviderValue({
+//       formHasRendered: true,
+//       doSaveElement: true,
+//       submitForm,
+//       onAfterSave,
+//     })}
+//     >
+//       <ActionComponent {...makeProps()}/>
+//     </ElementContext.Provider>
+//   );
+//   expect(submitForm).toHaveBeenCalled();
+//   expect(onAfterSave).toHaveBeenCalled();
+//   expect(value).toBe(3);
+// });