Skip to content

Commit

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

ENH Refactor Element and graphql hoc
GuySartorelli authored Apr 30, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
2 parents 13071d0 + 344bc54 commit 5d9393a
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';

@@ -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();
@@ -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;
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);

@@ -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.
69 changes: 0 additions & 69 deletions client/src/components/ElementActions/tests/PublishAction-test.js
Original file line number Diff line number Diff line change
@@ -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
@@ -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
@@ -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() {
@@ -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(
@@ -108,6 +97,5 @@ export default compose(
SummaryComponent, InlineEditFormComponent,
}),
() => 'ElementEditor.ElementList.Element'
),
connect(mapStateToProps)
)
)(Content);
90 changes: 53 additions & 37 deletions client/src/components/ElementEditor/Element.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* global window */

import React, { createContext, useState, useEffect } from 'react';
import React, { useState, useEffect, createContext } from 'react';
import { useMutation } from '@apollo/client';
import PropTypes from 'prop-types';
import { elementType } from 'types/elementType';
import { elementTypeType } from 'types/elementTypeType';
@@ -12,6 +13,8 @@ import { connect } from 'react-redux';
import { submit } from 'redux-form';
import { loadElementFormStateName } from 'state/editor/loadElementFormStateName';
import { loadElementSchemaValue } from 'state/editor/loadElementSchemaValue';
import { publishBlockMutation } from 'state/editor/publishBlockMutation';
import { query as readBlocksForAreaQuery } from 'state/editor/readBlocksForAreaQuery';
import * as TabsActions from 'state/tabs/TabsActions';
import { DragSource, DropTarget } from 'react-dnd';
import { getEmptyImage } from 'react-dnd-html5-backend';
@@ -37,6 +40,7 @@ const Element = (props) => {
const [ensureFormRendered, setEnsureFormRendered] = useState(false);
const [formHasRendered, setFormHasRendered] = useState(false);
const [doDispatchAddFormChanged, setDoDispatchAddFormChanged] = useState(false);
const [publishBlock] = useMutation(publishBlockMutation);

useEffect(() => {
if (props.connectDragPreview) {
@@ -67,11 +71,14 @@ const Element = (props) => {
setDoPublishElement(true);
}
}
}, [justClickedPublishButton, formHasRendered]);

useEffect(() => {
if (doDispatchAddFormChanged) {
setDoDispatchAddFormChanged(false);
props.dispatchAddFormChanged();
}
}, [justClickedPublishButton, formHasRendered]);
}, [doDispatchAddFormChanged]);

const getNoTitle = () => i18n.inject(
i18n._t('ElementHeader.NOTITLE', 'Untitled {type} block'),
@@ -104,6 +111,50 @@ const Element = (props) => {
}
};

// This will trigger a graphql request that will cause this
// element to re-render including any updated title and versioned badge
const refetchElementalArea = () => window.ss.apolloClient.queryManager.refetchQueries({
include: [{
query: readBlocksForAreaQuery,
variables: { id: props.areaId }
}]
});

const handleAfterPublish = (wasError) => {
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);
});
};

// Save action
useEffect(() => {
if (formHasRendered && doSaveElement) {
props.submitForm();
setDoSaveElement(false);
}
}, [formHasRendered, doSaveElement]);

// Publish action
useEffect(() => {
if (formHasRendered && doPublishElement) {
publishBlock({ variables: { blockId: props.element.id } })
.then(() => handleAfterPublish(false))
.catch(() => handleAfterPublish(true));
}
}, [formHasRendered, doPublishElement]);

/**
* Returns the applicable versioned state class names for the element
*
@@ -188,12 +239,6 @@ const Element = (props) => {
}
};

const refetchElementalArea = () => {
// This will trigger a graphql readOneElementalArea request that will cause this
// element to re-render including any updated title and versioned badge
window.ss.apolloClient.queryManager.reFetchObservableQueries();
};

/**
* Update the active tab on tab actions menu button click event. Is passed down to InlineEditForm.
*
@@ -255,28 +300,6 @@ const Element = (props) => {
setEnsureFormRendered(true);
};

const handleAfterSave = () => {
setDoSaveElement(false);
};

const handleAfterPublish = (wasError) => {
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
setTimeout(() => {
props.dispatchRemoveFormChanged();
}, 500);
refetchElementalArea();
};

const handleFormInit = (activeTab) => {
updateFormTab(activeTab);
setFormHasRendered(true);
@@ -328,7 +351,6 @@ const Element = (props) => {
isDragging,
isOver,
onDragEnd,
submitForm,
formDirty,
} = props;

@@ -350,14 +372,8 @@ const Element = (props) => {
// eslint-disable-next-line react/jsx-no-constructed-context-values
const providerValue = {
formDirty,
formHasRendered,
onPublishButtonClick: handlePublishButtonClick,
doPublishElement,
onSaveButtonClick: handleSaveButtonClick,
doSaveElement,
onAfterSave: handleAfterSave,
onAfterPublish: handleAfterPublish,
submitForm,
};

const content = connectDropTarget(<div
29 changes: 18 additions & 11 deletions client/src/components/ElementEditor/tests/Element-test.js
Original file line number Diff line number Diff line change
@@ -2,6 +2,7 @@
/* global jest, test, describe, it, expect */

import React from 'react';
import { ApolloClient, ApolloProvider, InMemoryCache } from '@apollo/client';
import { render } from '@testing-library/react';
import { Component as Element } from '../Element';

@@ -40,74 +41,80 @@ function makeProps(obj = {}) {
}

test('Element should render the HeaderComponent and the ContentComponent', () => {
const { container } = render(<Element {...makeProps()}/>);
const client = new ApolloClient({ cache: new InMemoryCache() });
const { container } = render(<ApolloProvider client={client}><Element {...makeProps()}/></ApolloProvider>);
expect(container.querySelectorAll('.element-editor__element .test-header')).toHaveLength(1);
expect(container.querySelectorAll('.element-editor__element .test-content')).toHaveLength(1);
});

test('Element should not render at all if no ID is given', () => {
const client = new ApolloClient({ cache: new InMemoryCache() });
const { container } = render(
<Element {...makeProps({
<ApolloProvider client={client}><Element {...makeProps({
element: {
...makeProps().element,
id: ''
}
})}
/>
/></ApolloProvider>
);
expect(container.querySelectorAll('.element-editor__element')).toHaveLength(0);
});

test('Element should render even if the element is broken', () => {
const client = new ApolloClient({ cache: new InMemoryCache() });
const { container } = render(
<Element {...makeProps({
<ApolloProvider client={client}><Element {...makeProps({
type: {
broken: true
}
})}
/>
/></ApolloProvider>
);
expect(container.querySelectorAll('.element-editor__element .test-header')).toHaveLength(1);
expect(container.querySelectorAll('.element-editor__element .test-content')).toHaveLength(1);
});

test('Element getVersionedStateClassName() should identify draft elements', () => {
const client = new ApolloClient({ cache: new InMemoryCache() });
const { container } = render(
<Element {...makeProps({
<ApolloProvider client={client}><Element {...makeProps({
element: {
...makeProps().element,
isPublished: false
}
})}
/>
/></ApolloProvider>
);
expect(container.querySelector('.element-editor__element').classList.contains('element-editor__element--draft')).toBe(true);
});

test('Element getVersionedStateClassName() should identify modified elements', () => {
const client = new ApolloClient({ cache: new InMemoryCache() });
const { container } = render(
<Element {...makeProps({
<ApolloProvider client={client}><Element {...makeProps({
element: {
...makeProps().element,
isPublished: true,
isLiveVersion: false
}
})}
/>
/></ApolloProvider>
);
expect(container.querySelectorAll('.element-editor__element--modified')).toHaveLength(1);
});

test('Element getVersionedStateClassName() should identify published elements', () => {
const client = new ApolloClient({ cache: new InMemoryCache() });
const { container } = render(
<Element {...makeProps({
<ApolloProvider client={client}><Element {...makeProps({
element: {
...makeProps().element,
isPublished: true,
isLiveVersion: true
}
})}
/>
/></ApolloProvider>
);
expect(container.querySelectorAll('.element-editor__element--published')).toHaveLength(1);
});
33 changes: 1 addition & 32 deletions client/src/state/editor/publishBlockMutation.js
Original file line number Diff line number Diff line change
@@ -1,40 +1,9 @@
import { graphql } from '@apollo/client/react/hoc';
import gql from 'graphql-tag';
import { config as readBlocksConfig, query as readBlocksQuery } from './readBlocksForAreaQuery';

// GraphQL query for saving a specific block
const mutation = gql`
export const publishBlockMutation = gql`
mutation PublishBlock($blockId:ID!) {
publishBlock(id: $blockId) {
id
}
}
`;

const config = {
props: ({ mutate, ownProps: { actions } }) => {
const handlePublishBlock = (blockId) => mutate({
variables: {
blockId
},
});

return {
actions: {
...actions,
handlePublishBlock,
},
};
},
options: ({ areaId }) => ({
// Refetch versions after mutation is completed
refetchQueries: [{
query: readBlocksQuery,
variables: readBlocksConfig.options({ areaId }).variables
}]
}),
};

export { mutation, config };

export default graphql(mutation, config);
15 changes: 0 additions & 15 deletions tests/Behat/features/individual-block-validation.feature
Original file line number Diff line number Diff line change
@@ -71,18 +71,3 @@ Feature: Blocks are validated when inline saving individual blocks
And I wait for 1 second
# Need to save the whole page to stop the alert
And I press the "Save" button

Scenario: I can save a closed block after saving a page with a validation error
When I fill in "x" for "My Field" for block 1
And I press the "Save" button
# Page validation error
Then I should see "MyField cannot be x" in the ".alert.error" element
When I press the "View actions" button
And I click on the ".element-editor__actions-save" element
# Same validation error after page load with inline save
And I should see "MyField cannot be x" in the ".form__validation-message" element
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
1 change: 0 additions & 1 deletion tests/Behat/features/unpublish-block-element.feature
Original file line number Diff line number Diff line change
@@ -20,7 +20,6 @@ Feature: Unpublish elements in the CMS
Then I should see "Block A"
And I should see "Block B"

@xsboyd
@modal
Scenario: I can unpublish a block
Given I see a list of blocks

0 comments on commit 5d9393a

Please sign in to comment.