Skip to content

Commit

Permalink
Merge pull request silverstripe#1176 from creative-commoners/pulls/5/…
Browse files Browse the repository at this point in the history
…refactor-graphql3

ENH Refactor Element and graphql hoc
  • Loading branch information
GuySartorelli authored Apr 30, 2024
2 parents 13071d0 + 344bc54 commit 5d9393a
Show file tree
Hide file tree
Showing 11 changed files with 82 additions and 292 deletions.
8 changes: 4 additions & 4 deletions client/dist/js/bundle.js

Large diffs are not rendered by default.

27 changes: 3 additions & 24 deletions client/src/components/ElementActions/PublishAction.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
/* 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';

Expand All @@ -11,21 +9,11 @@ import { ElementContext } from 'components/ElementEditor/Element';
*/
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();
Expand All @@ -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 (
Expand All @@ -69,7 +51,4 @@ const PublishAction = (MenuComponent) => (props) => {

export { PublishAction as Component };

export default compose(
publishBlockMutation,
PublishAction
);
export default PublishAction;
17 changes: 1 addition & 16 deletions client/src/components/ElementActions/SaveAction.js
Original file line number Diff line number Diff line change
@@ -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';

const SaveAction = (MenuComponent) => (props) => {
const {
doSaveElement,
onSaveButtonClick,
onAfterSave,
submitForm,
formHasRendered,
formDirty,
} = useContext(ElementContext);

Expand All @@ -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.
Expand Down
69 changes: 0 additions & 69 deletions client/src/components/ElementActions/tests/PublishAction-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,72 +100,3 @@ test('Clicking button calls onPublishButtonClick', () => {
fireEvent.click(container.querySelector('button.element-editor__actions-publish'));
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 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({
doPublishElement: true,
formHasRendered: false,
})}
>
<ActionComponent {...makeProps({ actions: { handlePublishBlock } })}/>
</ElementContext.Provider>
);
expect(handlePublishBlock).not.toHaveBeenCalled();
});

test('Do not trigger graphql mutation if doPublishElement is false and formHasRendered is true', () => {
const handlePublishBlock = jest.fn(() => Promise.resolve());
render(
<ElementContext.Provider value={makeProviderValue({
doPublishElement: false,
formHasRendered: true,
})}
>
<ActionComponent {...makeProps({ actions: { handlePublishBlock } })}/>
</ElementContext.Provider>
);
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);
});
69 changes: 0 additions & 69 deletions client/src/components/ElementActions/tests/SaveAction-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,72 +98,3 @@ test('Clicking button calls onSaveButtonClick', () => {
fireEvent.click(container.querySelector('button.element-editor__actions-save'));
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 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('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);
});
16 changes: 2 additions & 14 deletions client/src/components/ElementEditor/Content.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,6 @@ import React, { PureComponent } from 'react';
import PropTypes from 'prop-types';
import { inject } from 'lib/Injector';
import { compose } from 'redux';
import { connect } from 'react-redux';
import { loadElementFormStateName } from 'state/editor/loadElementFormStateName';
import { isDirty } from 'redux-form';
import getFormState from 'lib/getFormState';

class Content extends PureComponent {
render() {
Expand Down Expand Up @@ -87,18 +83,11 @@ Content.propTypes = {
onFormInit: PropTypes.func,
ensureFormRendered: PropTypes.bool,
formHasRendered: PropTypes.bool,
formDirty: PropTypes.object,
};

Content.defaultProps = {};

function mapStateToProps(state, ownProps) {
const formName = loadElementFormStateName(ownProps.id);

return {
formDirty: isDirty(`element.${formName}`, getFormState)(state),
};
}

export { Content as Component };

export default compose(
Expand All @@ -108,6 +97,5 @@ export default compose(
SummaryComponent, InlineEditFormComponent,
}),
() => 'ElementEditor.ElementList.Element'
),
connect(mapStateToProps)
)
)(Content);
Loading

0 comments on commit 5d9393a

Please sign in to comment.