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

ENH Refactor Element and graphql hoc #1176

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading