From 046f6c8bd461aa28f8111fda52b9c5ef17c5156a Mon Sep 17 00:00:00 2001 From: Joshua T Flowers Date: Fri, 12 Jun 2020 09:38:33 +0300 Subject: [PATCH] Create action for installing and activate plugins (https://github.com/woocommerce/woocommerce-admin/pull/4473) * Add installAndActivatePlugins method to plugin store * Use new install/actiate method in Plugins component * Refactor benefits page to use await * Refactor business details page to use new install method * Replace Plugins component in Jetpack CTA * Format and throw errors in plugin data store * Add generic response handling function * Add default error messages to plugin API --- .../stats-overview/install-jetpack-cta.js | 47 ++--- .../client/lib/notices/index.js | 18 ++ .../client/lib/notices/test/index.js | 69 +++++++ .../profile-wizard/steps/benefits/index.js | 176 +++++++++--------- .../profile-wizard/steps/business-details.js | 160 ++++++++-------- .../packages/components/src/plugins/index.js | 71 ++----- .../components/src/plugins/test/index.js | 58 ++---- .../packages/data/src/plugins/actions.js | 75 ++++---- plugins/woocommerce-admin/src/API/Plugins.php | 8 +- 9 files changed, 358 insertions(+), 324 deletions(-) create mode 100644 plugins/woocommerce-admin/client/lib/notices/index.js create mode 100644 plugins/woocommerce-admin/client/lib/notices/test/index.js diff --git a/plugins/woocommerce-admin/client/homepage/stats-overview/install-jetpack-cta.js b/plugins/woocommerce-admin/client/homepage/stats-overview/install-jetpack-cta.js index ce4e3901de0..0607c3bfbb5 100644 --- a/plugins/woocommerce-admin/client/homepage/stats-overview/install-jetpack-cta.js +++ b/plugins/woocommerce-admin/client/homepage/stats-overview/install-jetpack-cta.js @@ -6,36 +6,45 @@ import { compose } from '@wordpress/compose'; import { Button } from '@wordpress/components'; import { useState } from 'react'; import PropTypes from 'prop-types'; +import { withDispatch, withSelect } from '@wordpress/data'; /** * WooCommerce dependencies */ -import { H, Plugins } from '@woocommerce/components'; +import { H } from '@woocommerce/components'; import { getAdminLink } from '@woocommerce/wc-admin-settings'; import { PLUGINS_STORE_NAME, useUserPreferences } from '@woocommerce/data'; /** * Internal dependencies */ -import withSelect from 'wc-api/with-select'; import Connect from 'dashboard/components/connect'; import { recordEvent } from 'lib/tracks'; +import { createNoticesFromResponse } from 'lib/notices'; function InstallJetpackCta( { isJetpackInstalled, isJetpackActivated, isJetpackConnected, + installAndActivatePlugins, + isInstalling, } ) { const { updateUserPreferences, ...userPrefs } = useUserPreferences(); - const [ isInstalling, setIsInstalling ] = useState( false ); const [ isConnecting, setIsConnecting ] = useState( false ); const [ isDismissed, setIsDismissed ] = useState( ( userPrefs.homepage_stats || {} ).installJetpackDismissed ); - function install() { - setIsInstalling( true ); + async function install() { recordEvent( 'statsoverview_install_jetpack' ); + + installAndActivatePlugins( [ 'jetpack' ] ) + .then( () => { + setIsConnecting( ! isJetpackConnected ); + } ) + .catch( ( error ) => { + createNoticesFromResponse( error ); + } ); } function dismiss() { @@ -53,22 +62,6 @@ function InstallJetpackCta( { recordEvent( 'statsoverview_dismiss_install_jetpack' ); } - function getPluginInstaller() { - return ( - { - setIsInstalling( false ); - setIsConnecting( ! isJetpackConnected ); - } } - onError={ () => { - setIsInstalling( false ); - } } - pluginSlugs={ [ 'jetpack' ] } - /> - ); - } - function getConnector() { return ( - { isInstalling && getPluginInstaller() } { isConnecting && getConnector() } ); @@ -141,14 +133,25 @@ export default compose( withSelect( ( select ) => { const { isJetpackConnected, + isPluginsRequesting, getInstalledPlugins, getActivePlugins, } = select( PLUGINS_STORE_NAME ); return { + isInstalling: + isPluginsRequesting( 'installPlugins' ) || + isPluginsRequesting( 'activatePlugins' ), isJetpackInstalled: getInstalledPlugins().includes( 'jetpack' ), isJetpackActivated: getActivePlugins().includes( 'jetpack' ), isJetpackConnected: isJetpackConnected(), }; + } ), + withDispatch( ( dispatch ) => { + const { installAndActivatePlugins } = dispatch( PLUGINS_STORE_NAME ); + + return { + installAndActivatePlugins, + }; } ) )( InstallJetpackCta ); diff --git a/plugins/woocommerce-admin/client/lib/notices/index.js b/plugins/woocommerce-admin/client/lib/notices/index.js new file mode 100644 index 00000000000..a088099ba05 --- /dev/null +++ b/plugins/woocommerce-admin/client/lib/notices/index.js @@ -0,0 +1,18 @@ +/** + * External dependencies + */ +import { dispatch } from '@wordpress/data'; + +export function createNoticesFromResponse( response ) { + const { createNotice } = dispatch( 'core/notices' ); + + if ( response.error_data && response.errors && Object.keys( response.errors ).length ) { + // Loop over multi-error responses. + Object.keys( response.errors ).forEach( errorKey => { + createNotice( 'error', response.errors[ errorKey ].join( ' ' ) ); + } ); + } else if ( response.message ) { + // Handle generic messages. + createNotice( response.code ? 'error' : 'success', response.message ); + } +} \ No newline at end of file diff --git a/plugins/woocommerce-admin/client/lib/notices/test/index.js b/plugins/woocommerce-admin/client/lib/notices/test/index.js new file mode 100644 index 00000000000..66805dc6da4 --- /dev/null +++ b/plugins/woocommerce-admin/client/lib/notices/test/index.js @@ -0,0 +1,69 @@ +/** + * External dependencies + */ +import { dispatch } from '@wordpress/data'; + +/** + * Internal dependencies + */ +import { createNoticesFromResponse } from '../../notices'; + +jest.mock( '@wordpress/data', () => ( { + dispatch: jest.fn().mockReturnValue( { + createNotice: jest.fn(), + } ), +} ) ); + +describe( 'createNoticesFromResponse', () => { + beforeEach( () => { + jest.clearAllMocks(); + } ); + + const { createNotice } = dispatch( 'core/notices' ); + + test( 'should create notice based on message when no errors exist', () => { + const response = { message: 'Generic response message' }; + + createNoticesFromResponse( response ); + expect( createNotice ).toHaveBeenCalledWith( + 'success', + response.message, + ); + } ); + + test( 'should create an error notice when an error code and message exists', () => { + const response = { code: 'invalid_code', message: 'Error message' }; + + createNoticesFromResponse( response ); + expect( createNotice ).toHaveBeenCalledWith( + 'error', + response.message, + ); + } ); + + test( 'should create error messages for each item', () => { + const response = { errors: { + item1: [ + 'Item1 - Error 1.', + 'Item1 - Error 2.', + ], + item2: [ + 'Item2 - Error 1.', + ] + }, error_data: [] }; + + createNoticesFromResponse( response ); + expect( createNotice ).toHaveBeenCalledTimes(2); + const call1 = createNotice.mock.calls[0]; + const call2 = createNotice.mock.calls[1]; + expect( call1 ).toEqual( [ 'error', response.errors.item1.join( ' ' ) ] ); + expect( call2 ).toEqual( [ 'error', response.errors.item2[0] ] ); + } ); + + test( 'should not call createNotice when no message or errors exist', () => { + const response = { data: {} }; + + createNoticesFromResponse( response ); + expect( createNotice ).not.toHaveBeenCalled(); + } ); +} ); diff --git a/plugins/woocommerce-admin/client/profile-wizard/steps/benefits/index.js b/plugins/woocommerce-admin/client/profile-wizard/steps/benefits/index.js index 3b906f3027e..0bfcf498842 100644 --- a/plugins/woocommerce-admin/client/profile-wizard/steps/benefits/index.js +++ b/plugins/woocommerce-admin/client/profile-wizard/steps/benefits/index.js @@ -11,7 +11,7 @@ import { filter } from 'lodash'; /** * WooCommerce dependencies */ -import { Card, H, Plugins } from '@woocommerce/components'; +import { Card, H } from '@woocommerce/components'; import { getAdminLink } from '@woocommerce/wc-admin-settings'; import { pluginNames, @@ -24,6 +24,7 @@ import { * Internal dependencies */ import Connect from 'dashboard/components/connect'; +import { createNoticesFromResponse } from 'lib/notices'; import Logo from './logo'; import ManagementIcon from './images/management'; import SalesTaxIcon from './images/sales_tax'; @@ -34,10 +35,6 @@ import { recordEvent } from 'lib/tracks'; class Benefits extends Component { constructor( props ) { super( props ); - this.state = { - isConnecting: false, - isInstalling: false, - }; this.isJetpackActive = props.activePlugins.includes( 'jetpack' ); this.isWcsActive = props.activePlugins.includes( @@ -59,26 +56,6 @@ class Benefits extends Component { this.skipPluginInstall = this.skipPluginInstall.bind( this ); } - componentDidUpdate( prevProps, prevState ) { - const { goToNextStep } = this.props; - - // No longer pending or updating profile items, go to next step. - if ( - ! this.isPending() && - ( prevProps.isRequesting || - prevState.isConnecting || - prevState.isInstalling ) - ) { - goToNextStep(); - } - } - - isPending() { - const { isConnecting, isInstalling } = this.state; - const { isRequesting } = this.props; - return isConnecting || isInstalling || isRequesting; - } - async skipPluginInstall() { const { createNotice, @@ -108,21 +85,46 @@ class Benefits extends Component { goToNextStep(); } - startPluginInstall() { - const { updateProfileItems, updateOptions } = this.props; - - this.setState( { isInstalling: true } ); - - updateOptions( { - woocommerce_setup_jetpack_opted_in: true, - } ); - + async startPluginInstall() { + const { + createNotice, + goToNextStep, + installAndActivatePlugins, + isJetpackConnected, + updateProfileItems, + updateOptions, + } = this.props; const plugins = this.isJetpackActive ? 'installed-wcs' : 'installed'; + recordEvent( 'storeprofiler_install_plugins', { install: true, plugins, } ); - updateProfileItems( { plugins } ); + + await Promise.all( [ + installAndActivatePlugins( this.pluginsToInstall ), + updateProfileItems( { plugins } ), + updateOptions( { + woocommerce_setup_jetpack_opted_in: true, + } ), + ] ).catch( ( pluginError, profileError ) => { + if ( pluginError ) { + createNoticesFromResponse( pluginError ); + } + if ( profileError ) { + createNotice( + 'error', + __( + 'There was a problem updating your preferences.', + 'woocommerce-admin' + ) + ); + } + } ); + + if ( isJetpackConnected ) { + goToNextStep(); + } } renderBenefit( benefit ) { @@ -200,12 +202,22 @@ class Benefits extends Component { } render() { - const { isConnecting, isInstalling } = this.state; - const { isJetpackConnected, isRequesting } = this.props; + const { + activePlugins, + goToNextStep, + isJetpackConnected, + isInstallingActivating, + isRequesting, + } = this.props; const pluginNamesString = this.pluginsToInstall .map( ( pluginSlug ) => pluginNames[ pluginSlug ] ) .join( ' ' + __( 'and', 'woocommerce-admin' ) + ' ' ); + const pluginsRemaining = this.pluginsToInstall.filter( + ( plugin ) => ! activePlugins.includes( plugin ) + ); + const isInstallAction = + isInstallingActivating || ! pluginsRemaining.length; return ( @@ -222,10 +234,8 @@ class Benefits extends Component {
- { isInstalling && ( - - this.setState( { - isInstalling: false, - isConnecting: ! isJetpackConnected, - } ) - } - onError={ () => - this.setState( { - isInstalling: false, - } ) - } - pluginSlugs={ this.pluginsToInstall } - /> - ) } - { /* Make sure we're finished requesting since this will auto redirect us. */ } - { isConnecting && ! isJetpackConnected && ! isRequesting && ( - { - recordEvent( - 'storeprofiler_jetpack_connect_redirect' - ); - } } - onError={ () => - this.setState( { isConnecting: false } ) - } - redirectUrl={ getAdminLink( - 'admin.php?page=wc-admin&reset_profiler=0' - ) } - /> - ) } + { ! isJetpackConnected && + ! isRequesting && + ! pluginsRemaining.length && ( + { + recordEvent( + 'storeprofiler_jetpack_connect_redirect' + ); + } } + onError={ () => goToNextStep() } + redirectUrl={ getAdminLink( + 'admin.php?page=wc-admin&reset_profiler=0' + ) } + /> + ) }

@@ -308,31 +298,35 @@ export default compose( isOnboardingRequesting, } = select( ONBOARDING_STORE_NAME ); - const { getActivePlugins, isJetpackConnected } = select( - PLUGINS_STORE_NAME - ); - - const isProfileItemsError = Boolean( - getOnboardingError( 'updateProfileItems' ) - ); - const activePlugins = getActivePlugins(); - const profileItems = getProfileItems(); + const { + getActivePlugins, + isJetpackConnected, + isPluginsRequesting, + } = select( PLUGINS_STORE_NAME ); return { - activePlugins, - isProfileItemsError, - profileItems, + activePlugins: getActivePlugins(), + isProfileItemsError: Boolean( + getOnboardingError( 'updateProfileItems' ) + ), + profileItems: getProfileItems(), isJetpackConnected: isJetpackConnected(), isRequesting: isOnboardingRequesting( 'updateProfileItems' ), + isInstallingActivating: + isPluginsRequesting( 'installPlugins' ) || + isPluginsRequesting( 'activatePlugins' ) || + isPluginsRequesting( 'getJetpackConnectUrl' ), }; } ), withDispatch( ( dispatch ) => { + const { installAndActivatePlugins } = dispatch( PLUGINS_STORE_NAME ); const { updateProfileItems } = dispatch( ONBOARDING_STORE_NAME ); const { updateOptions } = dispatch( OPTIONS_STORE_NAME ); const { createNotice } = dispatch( 'core/notices' ); return { createNotice, + installAndActivatePlugins, updateProfileItems, updateOptions, }; diff --git a/plugins/woocommerce-admin/client/profile-wizard/steps/business-details.js b/plugins/woocommerce-admin/client/profile-wizard/steps/business-details.js index c1a7d3aae6f..631ee4f10b0 100644 --- a/plugins/woocommerce-admin/client/profile-wizard/steps/business-details.js +++ b/plugins/woocommerce-admin/client/profile-wizard/steps/business-details.js @@ -13,7 +13,12 @@ import { keys, get, pickBy } from 'lodash'; */ import { formatValue } from '@woocommerce/number'; import { getSetting } from '@woocommerce/wc-admin-settings'; -import { ONBOARDING_STORE_NAME, pluginNames, SETTINGS_STORE_NAME } from '@woocommerce/data'; +import { + ONBOARDING_STORE_NAME, + PLUGINS_STORE_NAME, + pluginNames, + SETTINGS_STORE_NAME, +} from '@woocommerce/data'; /** * Internal dependencies @@ -24,12 +29,11 @@ import { SelectControl, Form, TextControl, - Plugins, } from '@woocommerce/components'; -import withWCApiSelect from 'wc-api/with-select'; import { recordEvent } from 'lib/tracks'; import { getCurrencyRegion } from 'dashboard/utils'; import { CurrencyContext } from 'lib/currency-context'; +import { createNoticesFromResponse } from 'lib/notices'; const wcAdminAssetUrl = getSetting( 'wcAdminAssetUrl', '' ); @@ -60,12 +64,6 @@ class BusinessDetails extends Component { : true, }; - this.state = { - installExtensions: false, - isInstallingExtensions: false, - extensionInstallError: false, - }; - this.extensions = [ 'facebook-for-woocommerce', 'mailchimp-for-woocommerce', @@ -82,7 +80,7 @@ class BusinessDetails extends Component { const { createNotice, goToNextStep, - isError, + installAndActivatePlugins, updateProfileItems, } = this.props; const { @@ -125,27 +123,38 @@ class BusinessDetails extends Component { } } ); - await updateProfileItems( updates ); + const promises = [ + updateProfileItems( updates ).catch( () => { + createNotice( + 'error', + __( + 'There was a problem updating your business details.', + 'woocommerce-admin' + ) + ); + throw new Error(); + } ), + ]; - if ( ! isError ) { - if ( businessExtensions.length === 0 ) { - goToNextStep(); - return; - } - - this.setState( { - installExtensions: true, - isInstallingExtensions: true, - } ); - } else { - createNotice( - 'error', - __( - 'There was a problem updating your business details.', - 'woocommerce-admin' - ) + if ( businessExtensions.length ) { + promises.push( + installAndActivatePlugins( businessExtensions ) + .then( ( response ) => { + createNoticesFromResponse( response ); + } ) + .catch( ( error ) => { + this.setState( { + hasInstallActivateError: true, + } ); + createNoticesFromResponse( error ); + throw new Error(); + } ) ); } + + Promise.all( promises ).then( () => { + goToNextStep(); + } ); } validate( values ) { @@ -274,7 +283,7 @@ class BusinessDetails extends Component { } renderBusinessExtensionHelpText( values ) { - const { isInstallingExtensions } = this.state; + const { isInstallingActivating } = this.props; const extensions = this.getBusinessExtensions( values ); if ( extensions.length === 0 ) { @@ -287,7 +296,7 @@ class BusinessDetails extends Component { } ) .join( ', ' ); - if ( isInstallingExtensions ) { + if ( isInstallingActivating ) { return (

{ sprintf( @@ -319,9 +328,6 @@ class BusinessDetails extends Component { } renderBusinessExtensions( values, getInputProps ) { - const { installExtensions, extensionInstallError } = this.state; - const { goToNextStep } = this.props; - const extensionsToInstall = this.getBusinessExtensions( values ); const extensionBenefits = [ { slug: 'facebook-for-woocommerce', @@ -382,33 +388,16 @@ class BusinessDetails extends Component { ) ) } - - { installExtensions && ( -

- { - goToNextStep(); - } } - onSkip={ () => { - goToNextStep(); - } } - onError={ () => { - this.setState( { - extensionInstallError: true, - isInstallingExtensions: false, - } ); - } } - autoInstall={ ! extensionInstallError } - pluginSlugs={ extensionsToInstall } - /> -
- ) } ); } render() { - const { isInstallingExtensions, extensionInstallError } = this.state; + const { + goToNextStep, + isInstallingActivating, + hasInstallActivateError, + } = this.props; const { formatCurrency } = this.context; const productCountOptions = [ { @@ -662,20 +651,35 @@ class BusinessDetails extends Component { getInputProps ) } - { ! extensionInstallError && ( +
- ) } + { hasInstallActivateError && ( + + ) } +
@@ -692,37 +696,43 @@ class BusinessDetails extends Component { BusinessDetails.contextType = CurrencyContext; export default compose( - withWCApiSelect( ( select ) => { - const { getProfileItems, getOnboardingError } = select( ONBOARDING_STORE_NAME ); - - return { - isError: Boolean( getOnboardingError( 'updateProfileItems' ) ), - profileItems: getProfileItems(), - }; - } ), withSelect( ( select ) => { const { getSettings, getSettingsError, isGetSettingsRequesting, } = select( SETTINGS_STORE_NAME ); - + const { getProfileItems, getOnboardingError } = select( + ONBOARDING_STORE_NAME + ); + const { getPluginsError, isPluginsRequesting } = select( + PLUGINS_STORE_NAME + ); const { general: settings = {} } = getSettings( 'general' ); - const isSettingsError = Boolean( getSettingsError( 'general' ) ); - const isSettingsRequesting = isGetSettingsRequesting( 'general' ); return { - isSettingsError, - isSettingsRequesting, + hasInstallActivateError: + getPluginsError( 'installPlugins' ) || + getPluginsError( 'activatePlugins' ), + isError: Boolean( getOnboardingError( 'updateProfileItems' ) ), + profileItems: getProfileItems(), + isSettingsError: Boolean( getSettingsError( 'general' ) ), + isSettingsRequesting: isGetSettingsRequesting( 'general' ), settings, + isInstallingActivating: + isPluginsRequesting( 'installPlugins' ) || + isPluginsRequesting( 'activatePlugins' ) || + isPluginsRequesting( 'getJetpackConnectUrl' ), }; } ), withDispatch( ( dispatch ) => { const { updateProfileItems } = dispatch( ONBOARDING_STORE_NAME ); + const { installAndActivatePlugins } = dispatch( PLUGINS_STORE_NAME ); const { createNotice } = dispatch( 'core/notices' ); return { createNotice, + installAndActivatePlugins, updateProfileItems, }; } ) diff --git a/plugins/woocommerce-admin/packages/components/src/plugins/index.js b/plugins/woocommerce-admin/packages/components/src/plugins/index.js index bc8455e8fbd..6c232a3a4df 100644 --- a/plugins/woocommerce-admin/packages/components/src/plugins/index.js +++ b/plugins/woocommerce-admin/packages/components/src/plugins/index.js @@ -11,7 +11,8 @@ import { withSelect, withDispatch } from '@wordpress/data'; /** * WooCommerce dependencies */ -import { pluginNames, PLUGINS_STORE_NAME } from '@woocommerce/data'; +import { createNoticesFromResponse } from 'lib/notices'; +import { PLUGINS_STORE_NAME } from '@woocommerce/data'; export class Plugins extends Component { constructor() { @@ -41,9 +42,8 @@ export class Plugins extends Component { } const { + installAndActivatePlugins, isRequesting, - installPlugins, - activatePlugins, pluginSlugs, } = this.props; @@ -52,60 +52,26 @@ export class Plugins extends Component { return false; } - const installs = await installPlugins( pluginSlugs ); - - if ( installs.errors && Object.keys( installs.errors.errors ).length ) { - this.handleErrors( installs.errors ); - return; - } - - const activations = await activatePlugins( pluginSlugs ); - - if ( activations.success && activations.data.activated ) { - this.handleSuccess( activations.data.activated ); - return; - } - - if ( activations.errors ) { - this.handleErrors( activations.errors ); - } + installAndActivatePlugins( pluginSlugs ) + .then( ( response ) => { + createNoticesFromResponse( response ); + this.handleSuccess( response.data.activated ); + } ) + .catch( ( error ) => { + createNoticesFromResponse( error ); + this.handleErrors( error.errors ); + } ); } handleErrors( errors ) { - const { onError, createNotice } = this.props; - const { errors: pluginErrors } = errors; - - if ( pluginErrors ) { - Object.keys( pluginErrors ).forEach( ( plugin ) => { - createNotice( - 'error', - // Replace the slug with a plugin name if a constant exists. - pluginNames[ plugin ] - ? pluginErrors[ plugin ][ 0 ].replace( - `\`${ plugin }\``, - pluginNames[ plugin ] - ) - : pluginErrors[ plugin ][ 0 ] - ); - } ); - } else if ( errors.message ) { - createNotice( 'error', errors.message ); - } + const { onError } = this.props; this.setState( { hasErrors: true } ); onError( errors ); } handleSuccess( activePlugins ) { - const { createNotice, onComplete } = this.props; - - createNotice( - 'success', - __( - 'Plugins were successfully installed and activated.', - 'woocommerce-admin' - ) - ); + const { onComplete } = this.props; onComplete( activePlugins ); } @@ -221,15 +187,10 @@ export default compose( }; } ), withDispatch( ( dispatch ) => { - const { createNotice } = dispatch( 'core/notices' ); - const { activatePlugins, installPlugins } = dispatch( - PLUGINS_STORE_NAME - ); + const { installAndActivatePlugins } = dispatch( PLUGINS_STORE_NAME ); return { - activatePlugins, - createNotice, - installPlugins, + installAndActivatePlugins, }; } ) )( Plugins ); diff --git a/plugins/woocommerce-admin/packages/components/src/plugins/test/index.js b/plugins/woocommerce-admin/packages/components/src/plugins/test/index.js index 823cf2828f5..4ba51bd9fa8 100644 --- a/plugins/woocommerce-admin/packages/components/src/plugins/test/index.js +++ b/plugins/woocommerce-admin/packages/components/src/plugins/test/index.js @@ -46,16 +46,12 @@ describe( 'Rendering', () => { describe( 'Installing and activating', () => { let pluginsWrapper; - const installPlugins = jest.fn().mockReturnValue( { - success: true, - } ); - const activatePlugins = jest.fn().mockReturnValue( { + const installAndActivatePlugins = jest.fn().mockResolvedValue( { success: true, data: { activated: [ 'jetpack' ], }, } ); - const createNotice = jest.fn(); const onComplete = jest.fn(); beforeEach( () => { @@ -63,40 +59,23 @@ describe( 'Installing and activating', () => { ); } ); - it( 'should call installPlugins', async () => { + it( 'should call installAndActivatePlugins', async () => { const installButton = pluginsWrapper.find( Button ).at( 0 ); installButton.simulate( 'click' ); - expect( installPlugins ).toHaveBeenCalledWith( [ 'jetpack' ] ); + expect( installAndActivatePlugins ).toHaveBeenCalledWith( [ 'jetpack' ] ); } ); - it( 'should call activatePlugin', async () => { - const installButton = pluginsWrapper.find( Button ).at( 0 ); - installButton.simulate( 'click' ); - - expect( activatePlugins ).toHaveBeenCalledWith( [ 'jetpack' ] ); - } ); - it( 'should create a success notice', async () => { - const installButton = pluginsWrapper.find( Button ).at( 0 ); - installButton.simulate( 'click' ); - - expect( createNotice ).toHaveBeenCalledWith( - 'success', - 'Plugins were successfully installed and activated.' - ); - } ); it( 'should call the onComplete callback', async () => { const installButton = pluginsWrapper.find( Button ).at( 0 ); installButton.simulate( 'click' ); - expect( onComplete ).toHaveBeenCalledWith( [ 'jetpack' ] ); + await expect( onComplete ).toHaveBeenCalledWith( [ 'jetpack' ] ); } ); } ); @@ -107,43 +86,28 @@ describe( 'Installing and activating errors', () => { 'failed-plugin': [ 'error message' ], }, }; - const installPlugins = jest.fn().mockReturnValue( { + const installAndActivatePlugins = jest.fn().mockRejectedValue( { errors, } ); - const activatePlugins = jest.fn().mockReturnValue( { - success: false, - } ); - const createNotice = jest.fn(); + const onComplete = jest.fn(); const onError = jest.fn(); beforeEach( () => { pluginsWrapper = shallow( {} } - installPlugins={ installPlugins } - activatePlugins={ activatePlugins } - createNotice={ createNotice } + onComplete={ onComplete } + installAndActivatePlugins={ installAndActivatePlugins } onError={ onError } /> ); } ); - it( 'should not call activatePlugin on install error', async () => { + it( 'should not call onComplete on install error', async () => { const installButton = pluginsWrapper.find( Button ).at( 0 ); installButton.simulate( 'click' ); - expect( activatePlugins ).not.toHaveBeenCalled(); - } ); - - it( 'should create an error notice', async () => { - const installButton = pluginsWrapper.find( Button ).at( 0 ); - installButton.simulate( 'click' ); - - expect( createNotice ).toHaveBeenCalledWith( - 'error', - errors.errors[ 'failed-plugin' ][ 0 ] - ); + expect( onComplete ).not.toHaveBeenCalled(); } ); it( 'should call the onError callback', async () => { diff --git a/plugins/woocommerce-admin/packages/data/src/plugins/actions.js b/plugins/woocommerce-admin/packages/data/src/plugins/actions.js index c48f7090180..2be3007e365 100644 --- a/plugins/woocommerce-admin/packages/data/src/plugins/actions.js +++ b/plugins/woocommerce-admin/packages/data/src/plugins/actions.js @@ -1,13 +1,12 @@ /** * External Dependencies */ - -import { apiFetch } from '@wordpress/data-controls'; -import { __ } from '@wordpress/i18n'; +import { apiFetch, dispatch } from '@wordpress/data-controls'; /** * Internal Dependencies */ +import { pluginNames, STORE_NAME } from './constants'; import TYPES from './action-types'; import { WC_ADMIN_NAMESPACE } from '../constants'; @@ -68,16 +67,12 @@ export function* installPlugins( plugins ) { data: { plugins: plugins.join( ',' ) }, } ); - if ( ! results ) { - throw new Error(); - } - - if ( results.success && results.data.installed ) { + if ( results.data.installed.length ) { yield updateInstalledPlugins( results.data.installed ); } if ( Object.keys( results.errors.errors ).length ) { - yield setError( 'installPlugins', results.errors ); + throw results.errors; } yield setIsRequesting( 'installPlugins', false ); @@ -85,16 +80,7 @@ export function* installPlugins( plugins ) { return results; } catch ( error ) { yield setError( 'installPlugins', error ); - yield setIsRequesting( 'installPlugins', false ); - - return { - errors: { - message: __( - 'Something went wrong while trying to install your plugins.', - 'woocommerce-admin' - ), - }, - }; + throw formatErrors( error ); } } @@ -108,24 +94,47 @@ export function* activatePlugins( plugins ) { data: { plugins: plugins.join( ',' ) }, } ); - if ( results.success && results.data.activated ) { + if ( results.data.activated.length ) { yield updateActivePlugins( results.data.activated ); - yield setIsRequesting( 'activatePlugins', false ); - return results; } - throw new Error(); - } catch ( error ) { - yield setError( 'activatePlugins', error ); + if ( Object.keys( results.errors.errors ).length ) { + throw results.errors; + } + yield setIsRequesting( 'activatePlugins', false ); - return { - errors: { - message: __( - 'Something went wrong while trying to activate your plugins.', - 'woocommerce-admin' - ), - } - }; + return results; + } catch ( error ) { + yield setError( 'activatePlugins', error ); + throw formatErrors( error ); } } + +export function* installAndActivatePlugins( plugins ) { + try { + yield dispatch( STORE_NAME, 'installPlugins', plugins ); + const activations = yield dispatch( STORE_NAME, 'activatePlugins', plugins ); + return activations; + } catch ( error ) { + throw error; + } +} + +export function formatErrors( response ) { + if ( response.errors ) { + // Replace the slug with a plugin name if a constant exists. + Object.keys( response.errors ).forEach( plugin => { + response.errors[ plugin ] = response.errors[ plugin ].map( pluginError => { + return pluginNames[ plugin ] + ? pluginError.replace( + `\`${ plugin }\``, + pluginNames[ plugin ] + ) + : pluginError; + } ); + } ); + } + + return response; +} diff --git a/plugins/woocommerce-admin/src/API/Plugins.php b/plugins/woocommerce-admin/src/API/Plugins.php index 00f7396051a..a3559254586 100644 --- a/plugins/woocommerce-admin/src/API/Plugins.php +++ b/plugins/woocommerce-admin/src/API/Plugins.php @@ -234,7 +234,7 @@ class Plugins extends \WC_REST_Data_Controller { $errors->add( $plugin, /* translators: %s: plugin slug (example: woocommerce-services) */ - sprintf( __( 'The requested plugin `%s`. is not in the list of allowed plugins.', 'woocommerce-admin' ), $slug ) + sprintf( __( 'The requested plugin `%s` is not in the list of allowed plugins.', 'woocommerce-admin' ), $slug ) ); continue; } @@ -315,6 +315,9 @@ class Plugins extends \WC_REST_Data_Controller { ), 'errors' => $errors, 'success' => count( $errors->errors ) === 0, + 'message' => count( $errors->errors ) === 0 + ? __( 'Plugins were successfully installed.', 'woocommerce-admin' ) + : __( 'There was a problem installing some of the requested plugins.', 'woocommerce-admin' ), ); } @@ -422,6 +425,9 @@ class Plugins extends \WC_REST_Data_Controller { ), 'errors' => $errors, 'success' => count( $errors->errors ) === 0, + 'message' => count( $errors->errors ) === 0 + ? __( 'Plugins were successfully activated.', 'woocommerce-admin' ) + : __( 'There was a problem activating some of the requested plugins.', 'woocommerce-admin' ), ) ); }