From 0523faee2bfe9bb1abeb7e57577de175e9fa68ad Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Tue, 23 Jan 2018 19:05:08 +0100 Subject: [PATCH] withQuery and withMutation support for notifyOnError (default: true) --- .../src/containers/BanUserDialog.js | 18 +++----- .../src/containers/SuspendUserDialog.js | 19 +++----- .../coral-admin/src/containers/UserDetail.js | 7 ++- .../Community/containers/FlaggedAccounts.js | 5 +- .../src/routes/Community/containers/People.js | 7 ++- .../containers/RejectUsernameDialog.js | 4 +- .../routes/Configure/containers/Configure.js | 9 ++-- .../Moderation/containers/Moderation.js | 5 +- .../configure/containers/AssetStatusInfo.js | 16 +++++-- .../src/tabs/configure/containers/Settings.js | 6 +-- .../tabs/stream/containers/ChangeUsername.js | 15 +++++- client/coral-framework/hocs/index.js | 2 - .../coral-framework/hocs/notifyOnDataError.js | 32 ------------- .../hocs/notifyOnMutationError.js | 38 --------------- client/coral-framework/hocs/withMutation.js | 43 ++++++++++++++--- client/coral-framework/hocs/withQuery.js | 46 ++++++++++++++++--- 16 files changed, 133 insertions(+), 139 deletions(-) delete mode 100644 client/coral-framework/hocs/notifyOnDataError.js delete mode 100644 client/coral-framework/hocs/notifyOnMutationError.js diff --git a/client/coral-admin/src/containers/BanUserDialog.js b/client/coral-admin/src/containers/BanUserDialog.js index d2dc642a7..642f7f6ee 100644 --- a/client/coral-admin/src/containers/BanUserDialog.js +++ b/client/coral-admin/src/containers/BanUserDialog.js @@ -10,7 +10,6 @@ import { } from 'coral-framework/graphql/mutations'; import { compose } from 'react-apollo'; import t from 'coral-framework/services/i18n'; -import { getErrorMessages } from 'coral-framework/utils'; import { notify } from 'coral-framework/actions/notification'; class BanUserDialogContainer extends Component { @@ -22,16 +21,11 @@ class BanUserDialogContainer extends Component { banUser, setCommentStatus, hideBanUserDialog, - notify, } = this.props; - try { - await banUser({ id: userId, message: '' }); - hideBanUserDialog(); - if (commentId && commentStatus && commentStatus !== 'REJECTED') { - await setCommentStatus({ commentId, status: 'REJECTED' }); - } - } catch (err) { - notify('error', getErrorMessages(err)); + await banUser({ id: userId, message: '' }); + hideBanUserDialog(); + if (commentId && commentStatus && commentStatus !== 'REJECTED') { + await setCommentStatus({ commentId, status: 'REJECTED' }); } }; @@ -85,7 +79,7 @@ const mapDispatchToProps = dispatch => ({ }); export default compose( + connect(mapStateToProps, mapDispatchToProps), withBanUser, - withSetCommentStatus, - connect(mapStateToProps, mapDispatchToProps) + withSetCommentStatus )(BanUserDialogContainer); diff --git a/client/coral-admin/src/containers/SuspendUserDialog.js b/client/coral-admin/src/containers/SuspendUserDialog.js index e058faad6..f03d48f52 100644 --- a/client/coral-admin/src/containers/SuspendUserDialog.js +++ b/client/coral-admin/src/containers/SuspendUserDialog.js @@ -11,7 +11,6 @@ import { import { compose, gql } from 'react-apollo'; import t, { timeago } from 'coral-framework/services/i18n'; import withQuery from 'coral-framework/hocs/withQuery'; -import { getErrorMessages } from 'coral-framework/utils'; import get from 'lodash/get'; import { notify } from 'coral-framework/actions/notification'; @@ -28,17 +27,13 @@ class SuspendUserDialogContainer extends Component { notify, } = this.props; hideSuspendUserDialog(); - try { - await suspendUser({ id: userId, message, until }); - notify( - 'success', - t('suspenduser.notify_suspend_until', username, timeago(until)) - ); - if (commentId && commentStatus && commentStatus !== 'REJECTED') { - await setCommentStatus({ commentId, status: 'REJECTED' }); - } - } catch (err) { - notify('error', getErrorMessages(err)); + await suspendUser({ id: userId, message, until }); + notify( + 'success', + t('suspenduser.notify_suspend_until', username, timeago(until)) + ); + if (commentId && commentStatus && commentStatus !== 'REJECTED') { + await setCommentStatus({ commentId, status: 'REJECTED' }); } }; diff --git a/client/coral-admin/src/containers/UserDetail.js b/client/coral-admin/src/containers/UserDetail.js index 62eb32dc5..5b5c12bec 100644 --- a/client/coral-admin/src/containers/UserDetail.js +++ b/client/coral-admin/src/containers/UserDetail.js @@ -26,7 +26,7 @@ import UserDetailComment from './UserDetailComment'; import update from 'immutability-helper'; import { showBanUserDialog } from 'actions/banUserDialog'; import { showSuspendUserDialog } from 'actions/suspendUserDialog'; -import { notifyOnMutationError, notifyOnDataError } from 'coral-framework/hocs'; +import { notify } from 'coral-framework/actions/notification'; const commentConnectionFragment = gql` fragment CoralAdmin_UserDetail_CommentConnection on CommentConnection { @@ -272,6 +272,7 @@ const mapDispatchToProps = dispatch => ({ viewUserDetail, hideUserDetail, toggleSelectAllCommentInUserDetail, + notify, }, dispatch ), @@ -282,7 +283,5 @@ export default compose( withUserDetailQuery, withSetCommentStatus, withUnbanUser, - withUnsuspendUser, - notifyOnMutationError(['unbanUser', 'unsuspendUser', 'setCommentStatus']), - notifyOnDataError + withUnsuspendUser )(UserDetailContainer); diff --git a/client/coral-admin/src/routes/Community/containers/FlaggedAccounts.js b/client/coral-admin/src/routes/Community/containers/FlaggedAccounts.js index 054c5dd5f..1d7e089e2 100644 --- a/client/coral-admin/src/routes/Community/containers/FlaggedAccounts.js +++ b/client/coral-admin/src/routes/Community/containers/FlaggedAccounts.js @@ -15,7 +15,6 @@ import { handleFlaggedUsernameChange } from '../graphql'; import { notify } from 'coral-framework/actions/notification'; import { isFlaggedUserDangling } from '../utils'; import t from 'coral-framework/services/i18n'; -import { notifyOnMutationError, notifyOnDataError } from 'coral-framework/hocs'; import FlaggedAccounts from '../components/FlaggedAccounts'; import FlaggedUser from '../containers/FlaggedUser'; @@ -295,7 +294,6 @@ const mapDispatchToProps = dispatch => export default compose( connect(null, mapDispatchToProps), withApproveUsername, - notifyOnMutationError(['approveUsername']), withQuery( gql` query TalkAdmin_Community_FlaggedAccounts { @@ -334,6 +332,5 @@ export default compose( fetchPolicy: 'network-only', }, } - ), - notifyOnDataError + ) )(FlaggedAccountsContainer); diff --git a/client/coral-admin/src/routes/Community/containers/People.js b/client/coral-admin/src/routes/Community/containers/People.js index c5d36517c..d958d7378 100644 --- a/client/coral-admin/src/routes/Community/containers/People.js +++ b/client/coral-admin/src/routes/Community/containers/People.js @@ -16,7 +16,7 @@ import { appendNewNodes } from 'plugin-api/beta/client/utils'; import update from 'immutability-helper'; import { Spinner } from 'coral-ui'; import withQuery from 'coral-framework/hocs/withQuery'; -import { notifyOnMutationError, notifyOnDataError } from 'coral-framework/hocs'; +import { notify } from 'coral-framework/actions/notification'; class PeopleContainer extends React.Component { timer = null; @@ -132,6 +132,7 @@ const mapDispatchToProps = dispatch => viewUserDetail, showSuspendUserDialog, showBanUserDialog, + notify, }, dispatch ); @@ -205,7 +206,6 @@ export default compose( withSetUserRole, withUnsuspendUser, withUnbanUser, - notifyOnMutationError(['setUserRole', 'unsuspendUser', 'unbanUser']), withQuery( gql` query TalkAdmin_Community_People { @@ -241,6 +241,5 @@ export default compose( fetchPolicy: 'network-only', }, } - ), - notifyOnDataError + ) )(PeopleContainer); diff --git a/client/coral-admin/src/routes/Community/containers/RejectUsernameDialog.js b/client/coral-admin/src/routes/Community/containers/RejectUsernameDialog.js index f9ada0945..bf4490284 100644 --- a/client/coral-admin/src/routes/Community/containers/RejectUsernameDialog.js +++ b/client/coral-admin/src/routes/Community/containers/RejectUsernameDialog.js @@ -5,7 +5,6 @@ import { connect } from 'react-redux'; import { bindActionCreators } from 'redux'; import { compose } from 'react-apollo'; import { notify } from 'coral-framework/actions/notification'; -import { notifyOnMutationError } from 'coral-framework/hocs'; const mapStateToProps = state => ({ user: state.community.user, @@ -23,6 +22,5 @@ const mapDispatchToProps = dispatch => export default compose( connect(mapStateToProps, mapDispatchToProps), - withRejectUsername, - notifyOnMutationError(['rejectUsername']) + withRejectUsername )(RejectUsernameDialog); diff --git a/client/coral-admin/src/routes/Configure/containers/Configure.js b/client/coral-admin/src/routes/Configure/containers/Configure.js index e94c89788..254e39d70 100644 --- a/client/coral-admin/src/routes/Configure/containers/Configure.js +++ b/client/coral-admin/src/routes/Configure/containers/Configure.js @@ -12,7 +12,7 @@ import TechSettings from './TechSettings'; import ModerationSettings from './ModerationSettings'; import { clearPending, setActiveSection } from '../../../actions/configure'; import Configure from '../components/Configure'; -import { notifyOnMutationError, notifyOnDataError } from 'coral-framework/hocs'; +import { notify } from 'coral-framework/actions/notification'; class ConfigureContainer extends Component { savePending = async () => { @@ -83,16 +83,15 @@ const mapDispatchToProps = dispatch => { clearPending, setActiveSection, + notify, }, dispatch ); export default compose( - withUpdateSettings, - notifyOnMutationError(['updateSettings']), - withConfigureQuery, - notifyOnDataError, connect(mapStateToProps, mapDispatchToProps), + withUpdateSettings, + withConfigureQuery, withMergedSettings('root.settings', 'pending', 'mergedSettings') )(ConfigureContainer); diff --git a/client/coral-admin/src/routes/Moderation/containers/Moderation.js b/client/coral-admin/src/routes/Moderation/containers/Moderation.js index adb60fc31..e0e3f14fe 100644 --- a/client/coral-admin/src/routes/Moderation/containers/Moderation.js +++ b/client/coral-admin/src/routes/Moderation/containers/Moderation.js @@ -35,7 +35,6 @@ import { Spinner } from 'coral-ui'; import Moderation from '../components/Moderation'; import Comment from './Comment'; import baseQueueConfig from '../queueConfig'; -import { notifyOnMutationError, notifyOnDataError } from 'coral-framework/hocs'; function prepareNotificationText(text) { return truncate(text, { length: 50 }).replace('\n', ' '); @@ -535,7 +534,5 @@ export default compose( withQueueConfig(baseQueueConfig), connect(mapStateToProps, mapDispatchToProps), withSetCommentStatus, - notifyOnMutationError(['setCommentStatus']), - withModQueueQuery, - notifyOnDataError + withModQueueQuery )(ModerationContainer); diff --git a/client/coral-embed-stream/src/tabs/configure/containers/AssetStatusInfo.js b/client/coral-embed-stream/src/tabs/configure/containers/AssetStatusInfo.js index db74beff6..c97d8e068 100644 --- a/client/coral-embed-stream/src/tabs/configure/containers/AssetStatusInfo.js +++ b/client/coral-embed-stream/src/tabs/configure/containers/AssetStatusInfo.js @@ -7,7 +7,9 @@ import { withUpdateAssetStatus, withCloseAsset, } from 'coral-framework/graphql/mutations'; -import { notifyOnMutationError } from 'coral-framework/hocs'; +import { notify } from 'coral-framework/actions/notification'; +import { connect } from 'react-redux'; +import { bindActionCreators } from 'redux'; class AssetStatusInfoContainer extends React.Component { openAsset = () => @@ -43,11 +45,19 @@ const withAssetStatusInfoFragments = withFragments({ `, }); +const mapDispatchToProps = dispatch => + bindActionCreators( + { + notify, + }, + dispatch + ); + const enhance = compose( + connect(null, mapDispatchToProps), withAssetStatusInfoFragments, withUpdateAssetStatus, - withCloseAsset, - notifyOnMutationError(['updateAssetStatus', 'closeAsset']) + withCloseAsset ); export default enhance(AssetStatusInfoContainer); diff --git a/client/coral-embed-stream/src/tabs/configure/containers/Settings.js b/client/coral-embed-stream/src/tabs/configure/containers/Settings.js index 22be4ef9a..9bfeb149e 100644 --- a/client/coral-embed-stream/src/tabs/configure/containers/Settings.js +++ b/client/coral-embed-stream/src/tabs/configure/containers/Settings.js @@ -8,7 +8,7 @@ import { withUpdateAssetSettings } from 'coral-framework/graphql/mutations'; import { connect } from 'react-redux'; import { bindActionCreators } from 'redux'; import { clearPending, updatePending } from '../../../actions/configure'; -import { notifyOnMutationError } from 'coral-framework/hocs'; +import { notify } from 'coral-framework/actions/notification'; const slots = ['streamSettings']; @@ -129,15 +129,15 @@ const mapDispatchToProps = dispatch => { clearPending, updatePending, + notify, }, dispatch ); const enhance = compose( + connect(mapStateToProps, mapDispatchToProps), withSettingsFragments, withUpdateAssetSettings, - notifyOnMutationError(['updateAssetSettings']), - connect(mapStateToProps, mapDispatchToProps), withMergedSettings('asset.settings', 'pending', 'mergedSettings') ); diff --git a/client/coral-embed-stream/src/tabs/stream/containers/ChangeUsername.js b/client/coral-embed-stream/src/tabs/stream/containers/ChangeUsername.js index cc0b84f02..098513aaf 100644 --- a/client/coral-embed-stream/src/tabs/stream/containers/ChangeUsername.js +++ b/client/coral-embed-stream/src/tabs/stream/containers/ChangeUsername.js @@ -1,5 +1,18 @@ import { compose } from 'react-apollo'; import { withChangeUsername } from 'coral-framework/graphql/mutations'; import ChangeUsername from '../components/ChangeUsername'; +import { notify } from 'coral-framework/actions/notification'; +import { connect } from 'react-redux'; +import { bindActionCreators } from 'redux'; -export default compose(withChangeUsername)(ChangeUsername); +const mapDispatchToProps = dispatch => + bindActionCreators( + { + notify, + }, + dispatch + ); + +export default compose(connect(null, mapDispatchToProps), withChangeUsername)( + ChangeUsername +); diff --git a/client/coral-framework/hocs/index.js b/client/coral-framework/hocs/index.js index 7094ad53d..174f25a63 100644 --- a/client/coral-framework/hocs/index.js +++ b/client/coral-framework/hocs/index.js @@ -6,5 +6,3 @@ export { default as withEmit } from './withEmit'; export { default as excludeIf } from './excludeIf'; export { default as connect } from './connect'; export { default as withMergedSettings } from './withMergedSettings'; -export { default as notifyOnMutationError } from './notifyOnMutationError'; -export { default as notifyOnDataError } from './notifyOnDataError'; diff --git a/client/coral-framework/hocs/notifyOnDataError.js b/client/coral-framework/hocs/notifyOnDataError.js deleted file mode 100644 index 77e675ba1..000000000 --- a/client/coral-framework/hocs/notifyOnDataError.js +++ /dev/null @@ -1,32 +0,0 @@ -import { connect } from 'react-redux'; -import { bindActionCreators } from 'redux'; -import { notify } from 'coral-framework/actions/notification'; -import { branch, lifecycle, compose } from 'recompose'; -import { get } from 'lodash'; - -const notifyOnMutationError = compose( - branch( - ({ notify }) => !notify, - connect(null, dispatch => - bindActionCreators( - { - notify, - }, - dispatch - ) - ) - ), - lifecycle({ - componentWillReceiveProps(next) { - if ( - get(next, 'data.error.message') && - get(this.props, 'data.error.message') !== - get(next, 'data.error.message') - ) { - return this.props.notify('error', next.data.error.message); - } - }, - }) -); - -export default notifyOnMutationError; diff --git a/client/coral-framework/hocs/notifyOnMutationError.js b/client/coral-framework/hocs/notifyOnMutationError.js deleted file mode 100644 index 4046d8927..000000000 --- a/client/coral-framework/hocs/notifyOnMutationError.js +++ /dev/null @@ -1,38 +0,0 @@ -import { connect } from 'react-redux'; -import { bindActionCreators } from 'redux'; -import { compose } from 'react-apollo'; -import { notify } from 'coral-framework/actions/notification'; -import { forEachError } from 'coral-framework/utils'; -import { withProps, branch } from 'recompose'; - -const notifyOnMutationError = keys => - compose( - branch( - ({ notify }) => !notify, - connect(null, dispatch => - bindActionCreators( - { - notify, - }, - dispatch - ) - ) - ), - withProps(ownProps => - keys.reduce((props, key) => { - props[key] = async (...args) => { - try { - return await ownProps[key](...args); - } catch (e) { - forEachError(e, ({ msg }) => { - ownProps.notify('error', msg); - }); - throw e; - } - }; - return props; - }, {}) - ) - ); - -export default notifyOnMutationError; diff --git a/client/coral-framework/hocs/withMutation.js b/client/coral-framework/hocs/withMutation.js index b72e56f40..c31e89ce8 100644 --- a/client/coral-framework/hocs/withMutation.js +++ b/client/coral-framework/hocs/withMutation.js @@ -4,7 +4,11 @@ import merge from 'lodash/merge'; import uniq from 'lodash/uniq'; import flatten from 'lodash/flatten'; import isEmpty from 'lodash/isEmpty'; -import { getDefinitionName, getResponseErrors } from '../utils'; +import { + getDefinitionName, + getResponseErrors, + getErrorMessages, +} from '../utils'; import PropTypes from 'prop-types'; import t from 'coral-framework/services/i18n'; import hoistStatics from 'recompose/hoistStatics'; @@ -27,11 +31,7 @@ class ResponseError { } } -/** - * Exports a HOC with the same signature as `graphql`, that will - * apply mutation options registered in the graphRegistry. - */ -export default (document, config = {}) => +const createHOC = (document, config, { notifyOnError = true }) => hoistStatics(WrappedComponent => { config = { ...config, @@ -46,10 +46,25 @@ export default (document, config = {}) => graphql: PropTypes.object, }; + static propTypes = { + notify: PropTypes.func, + }; + get graphqlRegistry() { return this.context.graphql.registry; } + notifyErrors(messages) { + if (this.props.notify) { + this.props.notify('error', messages); + } else { + console.error( + '`notifyOnError` is set to `true` but missing `notify` property' + ); + console.error(messages); + } + } + resolveDocument(documentOrCallback) { return this.context.graphql.resolveDocument( documentOrCallback, @@ -165,6 +180,11 @@ export default (document, config = {}) => variables, error, }); + + // Show errors as notifications. + if (notifyOnError) { + this.notifyErrors(getErrorMessages(error)); + } throw error; }); }; @@ -213,3 +233,14 @@ export default (document, config = {}) => } }; }); + +/** + * Exports a HOC with the same signature as `graphql`, that will + * apply mutation options registered in the graphRegistry. + */ +export default (document, config = {}) => settingsOrComponent => { + if (typeof settingsOrComponent === 'function') { + return createHOC(document, config, {})(settingsOrComponent); + } + return createHOC(document, config, settingsOrComponent); +}; diff --git a/client/coral-framework/hocs/withQuery.js b/client/coral-framework/hocs/withQuery.js index d5609c340..08556b1b3 100644 --- a/client/coral-framework/hocs/withQuery.js +++ b/client/coral-framework/hocs/withQuery.js @@ -9,6 +9,7 @@ import PropTypes from 'prop-types'; import hoistStatics from 'recompose/hoistStatics'; import { getOperationName } from 'apollo-client/queries/getFromAST'; import throttle from 'lodash/throttle'; +import get from 'lodash/get'; const withSkipOnErrors = reducer => (prev, action, ...rest) => { if ( @@ -36,15 +37,12 @@ function networkStatusToString(networkStatus) { return 'ready'; case 8: return 'error'; + default: + throw new Error(`Unknown network status ${networkStatus}`); } - throw new Error(`Unknown network status ${networkStatus}`); } -/** - * Exports a HOC with the same signature as `graphql`, that will - * apply query options registered in the graphRegistry. - */ -export default (document, config = {}) => +const createHOC = (document, config, { notifyOnError = true }) => hoistStatics(WrappedComponent => { return class WithQuery extends React.Component { static contextTypes = { @@ -53,6 +51,10 @@ export default (document, config = {}) => client: PropTypes.object, }; + static propTypes = { + notify: PropTypes.func, + }; + // Lazily resolve fragments from graphRegistry to support circular dependencies. memoized = null; resolvedDocument = null; @@ -166,10 +168,31 @@ export default (document, config = {}) => return () => this.client.networkInterface.unsubscribe(id); }; + notifyErrors(messages) { + if (this.props.notify) { + this.props.notify('error', messages); + } else { + console.error( + '`notifyOnError` is set to `true` but missing `notify` property' + ); + console.error(messages); + } + } + nextData(data) { this.apolloData = data; this.emitWhenNeeded(data); + if ( + get(data, 'error.message') && + get(this, 'data.error.message') !== get(data, 'error.message') + ) { + // Show errors as notifications. + if (notifyOnError) { + this.notifyErrors(data.error.message); + } + } + // If data was previously set, we update it in a immutable way. if (this.data) { if (this.data.loading && !data.loading) { @@ -319,3 +342,14 @@ export default (document, config = {}) => } }; }); + +/** + * Exports a HOC with the same signature as `graphql`, that will + * apply query options registered in the graphRegistry. + */ +export default (document, config = {}) => settingsOrComponent => { + if (typeof settingsOrComponent === 'function') { + return createHOC(document, config, {})(settingsOrComponent); + } + return createHOC(document, config, settingsOrComponent); +};