From 49e548c83e1c6adeccb52ef6870f7c3460e45c48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Thomas?= Date: Mon, 1 Feb 2016 16:13:44 +0100 Subject: [PATCH 01/23] Plans: Fix redirection failing when clicking success notice in Plans page This fixes an 'Uncaught TypeError: this.props.getSelectedSite is not a function' error thrown when dismissing the notice displayed upon a successful free trial cancelation. --- client/my-sites/plans/main.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/my-sites/plans/main.jsx b/client/my-sites/plans/main.jsx index d916fe08f8a8ce..da2069e60ab7bb 100644 --- a/client/my-sites/plans/main.jsx +++ b/client/my-sites/plans/main.jsx @@ -81,7 +81,7 @@ var Plans = React.createClass( { }, redirectToDefault() { - page.redirect( paths.plans( this.props.getSelectedSite().slug ) ); + page.redirect( paths.plans( this.props.sites.getSelectedSite().slug ) ); }, renderNotice() { From 59b2a91d622e5579cffdb179eb56972d307e66ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Thomas?= Date: Mon, 1 Feb 2016 16:17:51 +0100 Subject: [PATCH 02/23] Plans: Remove duplicate action types constants There can be only one. --- client/state/sites/plans/action-types.js | 3 --- client/state/sites/plans/actions.js | 2 +- client/state/sites/plans/reducer.js | 4 ++-- 3 files changed, 3 insertions(+), 6 deletions(-) delete mode 100644 client/state/sites/plans/action-types.js diff --git a/client/state/sites/plans/action-types.js b/client/state/sites/plans/action-types.js deleted file mode 100644 index e863d0d88b511b..00000000000000 --- a/client/state/sites/plans/action-types.js +++ /dev/null @@ -1,3 +0,0 @@ -export const FETCH_SITE_PLANS = 'FETCH_SITE_PLANS'; -export const FETCH_SITE_PLANS_COMPLETED = 'FETCH_SITE_PLANS_COMPLETED'; -export const REMOVE_SITE_PLANS = 'REMOVE_SITE_PLANS'; diff --git a/client/state/sites/plans/actions.js b/client/state/sites/plans/actions.js index 74fe1eb4bb21d3..fc65a14954624a 100644 --- a/client/state/sites/plans/actions.js +++ b/client/state/sites/plans/actions.js @@ -16,7 +16,7 @@ import { FETCH_SITE_PLANS, FETCH_SITE_PLANS_COMPLETED, REMOVE_SITE_PLANS -} from './action-types'; +} from 'state/action-types'; /** * Clears plans for the given site. diff --git a/client/state/sites/plans/reducer.js b/client/state/sites/plans/reducer.js index 354e2eb727c224..3145e7618609ed 100644 --- a/client/state/sites/plans/reducer.js +++ b/client/state/sites/plans/reducer.js @@ -5,7 +5,7 @@ import { FETCH_SITE_PLANS, FETCH_SITE_PLANS_COMPLETED, REMOVE_SITE_PLANS -} from './action-types'; +} from 'state/action-types'; import { SERIALIZE, DESERIALIZE } from 'state/action-types'; import omit from 'lodash/object/omit'; @@ -43,4 +43,4 @@ export function plans( state = {}, action ) { } return state; -} +}; From 47105530c1a77b1d57c78469f949ab896496c393 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Thomas?= Date: Mon, 1 Feb 2016 16:24:40 +0100 Subject: [PATCH 03/23] Plans: Rename action type constants for fetching site plans to follow naming convention --- client/state/action-types.js | 4 ++-- client/state/sites/plans/actions.js | 8 ++++---- client/state/sites/plans/reducer.js | 8 ++++---- client/state/sites/plans/test/actions.js | 4 ++-- client/state/sites/plans/test/reducer.js | 8 ++++---- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/client/state/action-types.js b/client/state/action-types.js index 46ccab5f33ee64..b232e3ac1c81eb 100644 --- a/client/state/action-types.js +++ b/client/state/action-types.js @@ -22,8 +22,8 @@ export const EXPORT_ADVANCED_SETTINGS_FETCH = 'EXPORT_ADVANCED_SETTINGS_FETCH'; export const EXPORT_ADVANCED_SETTINGS_FETCH_FAIL = 'EXPORT_ADVANCED_SETTINGS_FETCH_FAIL'; export const EXPORT_ADVANCED_SETTINGS_RECEIVE = 'EXPORT_ADVANCED_SETTINGS_RECEIVE'; export const FAIL_EXPORT = 'FAIL_EXPORT'; -export const FETCH_SITE_PLANS = 'FETCH_SITE_PLANS'; -export const FETCH_SITE_PLANS_COMPLETED = 'FETCH_SITE_PLANS_COMPLETED'; +export const SITE_PLANS_FETCH = 'SITE_PLANS_FETCH'; +export const SITE_PLANS_FETCH_COMPLETED = 'SITE_PLANS_FETCH_COMPLETED'; export const FETCH_WPORG_PLUGIN_DATA = 'FETCH_WPORG_PLUGIN_DATA'; export const NEW_NOTICE = 'NEW_NOTICE'; export const POST_REQUEST = 'POST_REQUEST'; diff --git a/client/state/sites/plans/actions.js b/client/state/sites/plans/actions.js index fc65a14954624a..6a1e25ad85597c 100644 --- a/client/state/sites/plans/actions.js +++ b/client/state/sites/plans/actions.js @@ -13,8 +13,8 @@ const debug = debugFactory( 'calypso:site-plans:actions' ); import { createSitePlanObject } from './assembler'; import wpcom from 'lib/wp'; import { - FETCH_SITE_PLANS, - FETCH_SITE_PLANS_COMPLETED, + SITE_PLANS_FETCH, + SITE_PLANS_FETCH_COMPLETED, REMOVE_SITE_PLANS } from 'state/action-types'; @@ -42,7 +42,7 @@ export function clearSitePlans( siteId ) { export function fetchSitePlans( siteId ) { return ( dispatch ) => { dispatch( { - type: FETCH_SITE_PLANS, + type: SITE_PLANS_FETCH, siteId } ); @@ -72,7 +72,7 @@ export function fetchSitePlansCompleted( siteId, plans ) { plans = reject( plans, '_headers' ); return { - type: FETCH_SITE_PLANS_COMPLETED, + type: SITE_PLANS_FETCH_COMPLETED, siteId, plans: map( plans, createSitePlanObject ) }; diff --git a/client/state/sites/plans/reducer.js b/client/state/sites/plans/reducer.js index 3145e7618609ed..3cf1a6cdc89704 100644 --- a/client/state/sites/plans/reducer.js +++ b/client/state/sites/plans/reducer.js @@ -2,8 +2,8 @@ * Internal dependencies */ import { - FETCH_SITE_PLANS, - FETCH_SITE_PLANS_COMPLETED, + SITE_PLANS_FETCH, + SITE_PLANS_FETCH_COMPLETED, REMOVE_SITE_PLANS } from 'state/action-types'; import { SERIALIZE, DESERIALIZE } from 'state/action-types'; @@ -18,13 +18,13 @@ export const initialSiteState = { export function plans( state = {}, action ) { switch ( action.type ) { - case FETCH_SITE_PLANS: + case SITE_PLANS_FETCH: return Object.assign( {}, state, { [ action.siteId ]: Object.assign( {}, initialSiteState, state[ action.siteId ], { isFetching: true } ) } ); - case FETCH_SITE_PLANS_COMPLETED: + case SITE_PLANS_FETCH_COMPLETED: return Object.assign( {}, state, { [ action.siteId ]: Object.assign( {}, state[ action.siteId ], { error: null, diff --git a/client/state/sites/plans/test/actions.js b/client/state/sites/plans/test/actions.js index b24b4aab8270df..190ab3b9ab0891 100644 --- a/client/state/sites/plans/test/actions.js +++ b/client/state/sites/plans/test/actions.js @@ -6,7 +6,7 @@ import { expect } from 'chai'; /** * Internal dependencies */ -import { FETCH_SITE_PLANS_COMPLETED } from 'state/action-types'; +import { SITE_PLANS_FETCH_COMPLETED } from 'state/action-types'; import { fetchSitePlansCompleted } from '../actions'; describe( 'actions', () => { @@ -16,7 +16,7 @@ describe( 'actions', () => { action = fetchSitePlansCompleted( siteId ); expect( action ).to.eql( { - type: FETCH_SITE_PLANS_COMPLETED, + type: SITE_PLANS_FETCH_COMPLETED, siteId, plans: [] } ); diff --git a/client/state/sites/plans/test/reducer.js b/client/state/sites/plans/test/reducer.js index 5e75f2c5149c67..5aa2e83ba6ba4b 100644 --- a/client/state/sites/plans/test/reducer.js +++ b/client/state/sites/plans/test/reducer.js @@ -7,7 +7,7 @@ import { expect } from 'chai'; * Internal dependencies */ import { - FETCH_SITE_PLANS, + SITE_PLANS_FETCH, REMOVE_SITE_PLANS, SERIALIZE, DESERIALIZE @@ -25,7 +25,7 @@ describe( 'reducer', () => { it( 'should index plans by site ID', () => { const siteId = 11111111, state = plans( undefined, { - type: FETCH_SITE_PLANS, + type: SITE_PLANS_FETCH, siteId: siteId } ); @@ -39,7 +39,7 @@ describe( 'reducer', () => { 11111111: initialSiteState } ), state = plans( original, { - type: FETCH_SITE_PLANS, + type: SITE_PLANS_FETCH, siteId: 55555555 } ); @@ -54,7 +54,7 @@ describe( 'reducer', () => { 11111111: initialSiteState } ), state = plans( original, { - type: FETCH_SITE_PLANS, + type: SITE_PLANS_FETCH, siteId: 11111111 } ); From 68ca6d961d65cc6729b65aa395235e384d3ab10d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Thomas?= Date: Mon, 1 Feb 2016 16:42:19 +0100 Subject: [PATCH 04/23] Plans: Rename action type constants for removing site plans to follow naming convention --- client/state/action-types.js | 2 +- client/state/sites/plans/actions.js | 6 +++--- client/state/sites/plans/reducer.js | 4 ++-- client/state/sites/plans/test/reducer.js | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/client/state/action-types.js b/client/state/action-types.js index b232e3ac1c81eb..0b5fc7d2af51f9 100644 --- a/client/state/action-types.js +++ b/client/state/action-types.js @@ -39,7 +39,7 @@ export const PUBLICIZE_CONNECTIONS_REQUEST_FAILURE = 'PUBLICIZE_CONNECTIONS_REQU export const READER_SIDEBAR_LISTS_TOGGLE = 'READER_SIDEBAR_LISTS_TOGGLE'; export const READER_SIDEBAR_TAGS_TOGGLE = 'READER_SIDEBAR_TAGS_TOGGLE'; export const REMOVE_NOTICE = 'REMOVE_NOTICE'; -export const REMOVE_SITE_PLANS = 'REMOVE_SITE_PLANS'; +export const SITE_PLANS_REMOVE = 'SITE_PLANS_REMOVE'; export const REPLY_START_EXPORT = 'REPLY_START_EXPORT'; export const REQUEST_START_EXPORT = 'REQUEST_START_EXPORT'; export const SELECTED_SITE_SET = 'SELECTED_SITE_SET'; diff --git a/client/state/sites/plans/actions.js b/client/state/sites/plans/actions.js index 6a1e25ad85597c..758f1028b6203f 100644 --- a/client/state/sites/plans/actions.js +++ b/client/state/sites/plans/actions.js @@ -15,7 +15,7 @@ import wpcom from 'lib/wp'; import { SITE_PLANS_FETCH, SITE_PLANS_FETCH_COMPLETED, - REMOVE_SITE_PLANS + SITE_PLANS_REMOVE } from 'state/action-types'; /** @@ -27,7 +27,7 @@ import { export function clearSitePlans( siteId ) { return ( dispatch ) => { dispatch( { - type: REMOVE_SITE_PLANS, + type: SITE_PLANS_REMOVE, siteId } ); } @@ -87,7 +87,7 @@ export function fetchSitePlansCompleted( siteId, plans ) { export function refreshSitePlans( siteId ) { return ( dispatch ) => { dispatch( { - type: REMOVE_SITE_PLANS, + type: SITE_PLANS_REMOVE, siteId } ); diff --git a/client/state/sites/plans/reducer.js b/client/state/sites/plans/reducer.js index 3cf1a6cdc89704..23fa13968d7ce3 100644 --- a/client/state/sites/plans/reducer.js +++ b/client/state/sites/plans/reducer.js @@ -4,7 +4,7 @@ import { SITE_PLANS_FETCH, SITE_PLANS_FETCH_COMPLETED, - REMOVE_SITE_PLANS + SITE_PLANS_REMOVE } from 'state/action-types'; import { SERIALIZE, DESERIALIZE } from 'state/action-types'; import omit from 'lodash/object/omit'; @@ -33,7 +33,7 @@ export function plans( state = {}, action ) { data: action.plans } ) } ); - case REMOVE_SITE_PLANS: + case SITE_PLANS_REMOVE: return omit( state, action.siteId ); case SERIALIZE: //TODO: we have full instances of moment.js on sites.plans[siteID].data diff --git a/client/state/sites/plans/test/reducer.js b/client/state/sites/plans/test/reducer.js index 5aa2e83ba6ba4b..b510ca87180d02 100644 --- a/client/state/sites/plans/test/reducer.js +++ b/client/state/sites/plans/test/reducer.js @@ -8,7 +8,7 @@ import { expect } from 'chai'; */ import { SITE_PLANS_FETCH, - REMOVE_SITE_PLANS, + SITE_PLANS_REMOVE, SERIALIZE, DESERIALIZE } from 'state/action-types'; @@ -69,7 +69,7 @@ describe( 'reducer', () => { 22222222: initialSiteState } ), state = plans( original, { - type: REMOVE_SITE_PLANS, + type: SITE_PLANS_REMOVE, siteId: 11111111 } ); From 155272f566c3c6228a80060aa0ad91dd38e8ee11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Thomas?= Date: Mon, 1 Feb 2016 16:43:45 +0100 Subject: [PATCH 05/23] Framework: Order action type constants alphabetically --- client/state/action-types.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/client/state/action-types.js b/client/state/action-types.js index 0b5fc7d2af51f9..b194a3ec0566f5 100644 --- a/client/state/action-types.js +++ b/client/state/action-types.js @@ -22,8 +22,6 @@ export const EXPORT_ADVANCED_SETTINGS_FETCH = 'EXPORT_ADVANCED_SETTINGS_FETCH'; export const EXPORT_ADVANCED_SETTINGS_FETCH_FAIL = 'EXPORT_ADVANCED_SETTINGS_FETCH_FAIL'; export const EXPORT_ADVANCED_SETTINGS_RECEIVE = 'EXPORT_ADVANCED_SETTINGS_RECEIVE'; export const FAIL_EXPORT = 'FAIL_EXPORT'; -export const SITE_PLANS_FETCH = 'SITE_PLANS_FETCH'; -export const SITE_PLANS_FETCH_COMPLETED = 'SITE_PLANS_FETCH_COMPLETED'; export const FETCH_WPORG_PLUGIN_DATA = 'FETCH_WPORG_PLUGIN_DATA'; export const NEW_NOTICE = 'NEW_NOTICE'; export const POST_REQUEST = 'POST_REQUEST'; @@ -39,7 +37,6 @@ export const PUBLICIZE_CONNECTIONS_REQUEST_FAILURE = 'PUBLICIZE_CONNECTIONS_REQU export const READER_SIDEBAR_LISTS_TOGGLE = 'READER_SIDEBAR_LISTS_TOGGLE'; export const READER_SIDEBAR_TAGS_TOGGLE = 'READER_SIDEBAR_TAGS_TOGGLE'; export const REMOVE_NOTICE = 'REMOVE_NOTICE'; -export const SITE_PLANS_REMOVE = 'SITE_PLANS_REMOVE'; export const REPLY_START_EXPORT = 'REPLY_START_EXPORT'; export const REQUEST_START_EXPORT = 'REQUEST_START_EXPORT'; export const SELECTED_SITE_SET = 'SELECTED_SITE_SET'; @@ -47,6 +44,9 @@ export const SERIALIZE = 'SERIALIZE'; export const SET_EXPORT_POST_TYPE = 'SET_EXPORT_POST_TYPE'; export const SET_ROUTE = 'SET_ROUTE'; export const SET_SECTION = 'SET_SECTION'; +export const SITE_PLANS_FETCH = 'SITE_PLANS_FETCH'; +export const SITE_PLANS_FETCH_COMPLETED = 'SITE_PLANS_FETCH_COMPLETED'; +export const SITE_PLANS_REMOVE = 'SITE_PLANS_REMOVE'; export const SITE_RECEIVE = 'SITE_RECEIVE'; export const SUPPORT_USER_TOKEN_FETCH = 'SUPPORT_USER_TOKEN_FETCH'; export const SUPPORT_USER_TOKEN_SET = 'SUPPORT_USER_TOKEN_SET'; From 023bb89c005eecfc5c2ecb0f3c1b0824c73fa400 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Thomas?= Date: Mon, 1 Feb 2016 16:46:45 +0100 Subject: [PATCH 06/23] Plans: Move import to the right place in Site Plans reducer --- client/state/sites/plans/reducer.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/client/state/sites/plans/reducer.js b/client/state/sites/plans/reducer.js index 23fa13968d7ce3..7a9a6ca0973abd 100644 --- a/client/state/sites/plans/reducer.js +++ b/client/state/sites/plans/reducer.js @@ -1,3 +1,8 @@ +/** + * External dependencies + */ +import omit from 'lodash/object/omit'; + /** * Internal dependencies */ @@ -7,7 +12,6 @@ import { SITE_PLANS_REMOVE } from 'state/action-types'; import { SERIALIZE, DESERIALIZE } from 'state/action-types'; -import omit from 'lodash/object/omit'; export const initialSiteState = { error: null, @@ -24,6 +28,7 @@ export function plans( state = {}, action ) { isFetching: true } ) } ); + case SITE_PLANS_FETCH_COMPLETED: return Object.assign( {}, state, { [ action.siteId ]: Object.assign( {}, state[ action.siteId ], { @@ -33,11 +38,14 @@ export function plans( state = {}, action ) { data: action.plans } ) } ); + case SITE_PLANS_REMOVE: return omit( state, action.siteId ); + case SERIALIZE: //TODO: we have full instances of moment.js on sites.plans[siteID].data return {}; + case DESERIALIZE: return {}; } From 9f9922fded04b0a5910593dd8649a7a9e1f1a87f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Thomas?= Date: Mon, 1 Feb 2016 16:51:27 +0100 Subject: [PATCH 07/23] Plans: Order imports alphabetically in Site Plans actions --- client/state/sites/plans/actions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/state/sites/plans/actions.js b/client/state/sites/plans/actions.js index 758f1028b6203f..42caaa639e691c 100644 --- a/client/state/sites/plans/actions.js +++ b/client/state/sites/plans/actions.js @@ -11,12 +11,12 @@ const debug = debugFactory( 'calypso:site-plans:actions' ); * Internal dependencies */ import { createSitePlanObject } from './assembler'; -import wpcom from 'lib/wp'; import { SITE_PLANS_FETCH, SITE_PLANS_FETCH_COMPLETED, SITE_PLANS_REMOVE } from 'state/action-types'; +import wpcom from 'lib/wp'; /** * Clears plans for the given site. From 45f6e31e5c40a8eb2a5c4697850388419043a40b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Thomas?= Date: Mon, 1 Feb 2016 16:58:24 +0100 Subject: [PATCH 08/23] Plans: Update action for clearing site plans to return an action object There is no need to return an action thunk here. --- client/state/sites/plans/actions.js | 20 +++++++------------- client/state/sites/plans/test/reducer.js | 2 +- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/client/state/sites/plans/actions.js b/client/state/sites/plans/actions.js index 42caaa639e691c..796e1e5b33b385 100644 --- a/client/state/sites/plans/actions.js +++ b/client/state/sites/plans/actions.js @@ -19,18 +19,16 @@ import { import wpcom from 'lib/wp'; /** - * Clears plans for the given site. + * Returns an action object to be used in signalling that plans for the given site has been cleared. * * @param {Number} siteId identifier of the site - * @returns {Function} the corresponding action thunk + * @returns {Object} the corresponding action object */ export function clearSitePlans( siteId ) { - return ( dispatch ) => { - dispatch( { - type: SITE_PLANS_REMOVE, - siteId - } ); - } + return { + type: SITE_PLANS_REMOVE, + siteId + }; } /** @@ -86,11 +84,7 @@ export function fetchSitePlansCompleted( siteId, plans ) { */ export function refreshSitePlans( siteId ) { return ( dispatch ) => { - dispatch( { - type: SITE_PLANS_REMOVE, - siteId - } ); - + dispatch( clearSitePlans( siteId ) ); dispatch( fetchSitePlans( siteId ) ); } } diff --git a/client/state/sites/plans/test/reducer.js b/client/state/sites/plans/test/reducer.js index b510ca87180d02..68131ae432e945 100644 --- a/client/state/sites/plans/test/reducer.js +++ b/client/state/sites/plans/test/reducer.js @@ -63,7 +63,7 @@ describe( 'reducer', () => { } ); } ); - it( 'should remove plans for a given site ID', () => { + it( 'should clear plans for a given site ID', () => { const original = Object.freeze( { 11111111: initialSiteState, 22222222: initialSiteState From 46cd8ee28a5699b457df152de5bd5b3346867b64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Thomas?= Date: Mon, 1 Feb 2016 19:37:18 +0100 Subject: [PATCH 09/23] Plans: Standardize descriptions of unit tests of Site Plans reducer --- client/state/sites/plans/test/reducer.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/client/state/sites/plans/test/reducer.js b/client/state/sites/plans/test/reducer.js index 68131ae432e945..0cb77b813b5eef 100644 --- a/client/state/sites/plans/test/reducer.js +++ b/client/state/sites/plans/test/reducer.js @@ -16,13 +16,13 @@ import { initialSiteState, plans } from '../reducer'; describe( 'reducer', () => { describe( '#plans()', () => { - it( 'should default to an empty object', () => { + it( 'should return an empty state when original state is undefined and action is empty', () => { const state = plans( undefined, {} ); expect( state ).to.eql( {} ); } ); - it( 'should index plans by site ID', () => { + it( 'should return the initial state with fetching enabled when fetching is triggered', () => { const siteId = 11111111, state = plans( undefined, { type: SITE_PLANS_FETCH, @@ -49,7 +49,7 @@ describe( 'reducer', () => { } ); } ); - it( 'should override previous plans of same site ID', () => { + it( 'should override previous plans of the same site', () => { const original = Object.freeze( { 11111111: initialSiteState } ), @@ -63,7 +63,7 @@ describe( 'reducer', () => { } ); } ); - it( 'should clear plans for a given site ID', () => { + it( 'should remove plans for a given site when removal is triggered', () => { const original = Object.freeze( { 11111111: initialSiteState, 22222222: initialSiteState From c10fc881b35aa9c232970d4dcf6db4690f8a9fdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Thomas?= Date: Mon, 1 Feb 2016 19:48:28 +0100 Subject: [PATCH 10/23] Plans: Update unit tests of Site Plans reducer to be easier to reason about This moves from using initial site state constants to using plain objects to describe assertions. The goal is to make these unit tests easier to understand (since the expected results are obvious and don't require any interpretation of Object.assign()) and more flexible (by specifying different original states). --- client/state/sites/plans/test/reducer.js | 74 +++++++++++++++++++----- 1 file changed, 59 insertions(+), 15 deletions(-) diff --git a/client/state/sites/plans/test/reducer.js b/client/state/sites/plans/test/reducer.js index 0cb77b813b5eef..2b801fb4ce4730 100644 --- a/client/state/sites/plans/test/reducer.js +++ b/client/state/sites/plans/test/reducer.js @@ -12,7 +12,7 @@ import { SERIALIZE, DESERIALIZE } from 'state/action-types'; -import { initialSiteState, plans } from '../reducer'; +import { plans } from '../reducer'; describe( 'reducer', () => { describe( '#plans()', () => { @@ -23,20 +23,29 @@ describe( 'reducer', () => { } ); it( 'should return the initial state with fetching enabled when fetching is triggered', () => { - const siteId = 11111111, - state = plans( undefined, { - type: SITE_PLANS_FETCH, - siteId: siteId - } ); + const state = plans( undefined, { + type: SITE_PLANS_FETCH, + siteId: 11111111 + } ); expect( state ).to.eql( { - [ siteId ]: Object.assign( {}, initialSiteState, { isFetching: true } ) + 11111111: { + data: null, + error: null, + hasLoadedFromServer: false, + isFetching: true + } } ); } ); it( 'should accumulate plans for different sites', () => { const original = Object.freeze( { - 11111111: initialSiteState + 11111111: { + data: [], + error: null, + hasLoadedFromServer: true, + isFetching: false + } } ), state = plans( original, { type: SITE_PLANS_FETCH, @@ -44,14 +53,29 @@ describe( 'reducer', () => { } ); expect( state ).to.eql( { - 11111111: initialSiteState, - 55555555: Object.assign( {}, initialSiteState, { isFetching: true } ) + 11111111: { + data: [], + error: null, + hasLoadedFromServer: true, + isFetching: false + }, + 55555555: { + data: null, + error: null, + hasLoadedFromServer: false, + isFetching: true + } } ); } ); it( 'should override previous plans of the same site', () => { const original = Object.freeze( { - 11111111: initialSiteState + 11111111: { + data: null, + error: 'Unable to fetch site plans', + hasLoadedFromServer: false, + isFetching: false + } } ), state = plans( original, { type: SITE_PLANS_FETCH, @@ -59,14 +83,29 @@ describe( 'reducer', () => { } ); expect( state ).to.eql( { - 11111111: Object.assign( {}, initialSiteState, { isFetching: true } ) + 11111111: { + data: null, + error: 'Unable to fetch site plans', + hasLoadedFromServer: false, + isFetching: true + } } ); } ); it( 'should remove plans for a given site when removal is triggered', () => { const original = Object.freeze( { - 11111111: initialSiteState, - 22222222: initialSiteState + 11111111: { + data: null, + error: 'Unable to fetch site plans', + hasLoadedFromServer: false, + isFetching: false + }, + 22222222: { + data: [], + error: null, + hasLoadedFromServer: true, + isFetching: false + } } ), state = plans( original, { type: SITE_PLANS_REMOVE, @@ -74,7 +113,12 @@ describe( 'reducer', () => { } ); expect( state ).to.eql( { - 22222222: initialSiteState + 22222222: { + data: [], + error: null, + hasLoadedFromServer: true, + isFetching: false + } } ); } ); From 012549db5ed185db0247fc595047bc3be74b9e97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Thomas?= Date: Mon, 1 Feb 2016 19:51:15 +0100 Subject: [PATCH 11/23] Plans: Add more unit tests for Site Plans reducer --- client/state/sites/plans/test/reducer.js | 77 ++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/client/state/sites/plans/test/reducer.js b/client/state/sites/plans/test/reducer.js index 2b801fb4ce4730..21bfc0e0ef0220 100644 --- a/client/state/sites/plans/test/reducer.js +++ b/client/state/sites/plans/test/reducer.js @@ -8,6 +8,7 @@ import { expect } from 'chai'; */ import { SITE_PLANS_FETCH, + SITE_PLANS_FETCH_COMPLETED, SITE_PLANS_REMOVE, SERIALIZE, DESERIALIZE @@ -22,6 +23,39 @@ describe( 'reducer', () => { expect( state ).to.eql( {} ); } ); + it( 'should return an empty state when original state and action are empty', () => { + const original = Object.freeze( {} ), + state = plans( original, {} ); + + expect( state ).to.eql( original ); + } ); + + it( 'should return an empty state when original state is undefined and action is unknown', () => { + const state = plans( undefined, { + type: 'SAY_HELLO', + siteId: 11111111 + } ); + + expect( state ).to.eql( {} ); + } ); + + it( 'should return the original state when action is unknown', () => { + const original = Object.freeze( { + 11111111: { + data: [], + error: null, + hasLoadedFromServer: true, + isFetching: false + } + } ), + state = plans( original, { + type: 'MAKE_COFFEE', + siteId: 11111111 + } ); + + expect( state ).to.eql( original ); + } ); + it( 'should return the initial state with fetching enabled when fetching is triggered', () => { const state = plans( undefined, { type: SITE_PLANS_FETCH, @@ -38,6 +72,23 @@ describe( 'reducer', () => { } ); } ); + it( 'should return a list of plans as well as loaded from server enabled and fetching disabled when fetching completed', () => { + const state = plans( undefined, { + type: SITE_PLANS_FETCH_COMPLETED, + siteId: 11111111, + plans: [] + } ); + + expect( state ).to.eql( { + 11111111: { + data: [], + error: null, + hasLoadedFromServer: true, + isFetching: false + } + } ); + } ); + it( 'should accumulate plans for different sites', () => { const original = Object.freeze( { 11111111: { @@ -92,6 +143,32 @@ describe( 'reducer', () => { } ); } ); + it( 'should return an empty state when original state is undefined and removal is triggered', () => { + const state = plans( undefined, { + type: SITE_PLANS_REMOVE, + siteId: 11111111 + } ); + + expect( state ).to.eql( {} ); + } ); + + it( 'should return the original state when removal is triggered for an unknown site', () => { + const original = Object.freeze( { + 11111111: { + data: null, + error: 'Unable to fetch site plans', + hasLoadedFromServer: false, + isFetching: false + } + } ), + state = plans( original, { + type: SITE_PLANS_REMOVE, + siteId: 22222222 + } ); + + expect( state ).to.eql( original ); + } ); + it( 'should remove plans for a given site when removal is triggered', () => { const original = Object.freeze( { 11111111: { From 277466322309ce7ec49d54efa37d021a1671a8e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Thomas?= Date: Mon, 1 Feb 2016 19:55:14 +0100 Subject: [PATCH 12/23] Plans: Fix error not reset when fetching site plans --- client/state/sites/plans/reducer.js | 5 +++-- client/state/sites/plans/test/reducer.js | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/client/state/sites/plans/reducer.js b/client/state/sites/plans/reducer.js index 7a9a6ca0973abd..c5f7091f3d9464 100644 --- a/client/state/sites/plans/reducer.js +++ b/client/state/sites/plans/reducer.js @@ -14,10 +14,10 @@ import { import { SERIALIZE, DESERIALIZE } from 'state/action-types'; export const initialSiteState = { + data: null, error: null, hasLoadedFromServer: false, - isFetching: false, - data: null + isFetching: false }; export function plans( state = {}, action ) { @@ -25,6 +25,7 @@ export function plans( state = {}, action ) { case SITE_PLANS_FETCH: return Object.assign( {}, state, { [ action.siteId ]: Object.assign( {}, initialSiteState, state[ action.siteId ], { + error: null, isFetching: true } ) } ); diff --git a/client/state/sites/plans/test/reducer.js b/client/state/sites/plans/test/reducer.js index 21bfc0e0ef0220..0f61e253b99e15 100644 --- a/client/state/sites/plans/test/reducer.js +++ b/client/state/sites/plans/test/reducer.js @@ -136,7 +136,7 @@ describe( 'reducer', () => { expect( state ).to.eql( { 11111111: { data: null, - error: 'Unable to fetch site plans', + error: null, hasLoadedFromServer: false, isFetching: true } From 8ddd47f2539acbf348f864a8f2f39c7245571c7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Thomas?= Date: Mon, 1 Feb 2016 20:05:54 +0100 Subject: [PATCH 13/23] Plans: Add error handling when fetching site plans This also fixes the case where isFetching would not be reset in case of failure. --- client/state/action-types.js | 1 + client/state/sites/plans/actions.js | 9 ++++++++ client/state/sites/plans/reducer.js | 9 ++++++++ client/state/sites/plans/test/reducer.js | 26 ++++++++++++++++++++++++ 4 files changed, 45 insertions(+) diff --git a/client/state/action-types.js b/client/state/action-types.js index b194a3ec0566f5..3d7bd61903a67d 100644 --- a/client/state/action-types.js +++ b/client/state/action-types.js @@ -46,6 +46,7 @@ export const SET_ROUTE = 'SET_ROUTE'; export const SET_SECTION = 'SET_SECTION'; export const SITE_PLANS_FETCH = 'SITE_PLANS_FETCH'; export const SITE_PLANS_FETCH_COMPLETED = 'SITE_PLANS_FETCH_COMPLETED'; +export const SITE_PLANS_FETCH_FAILED = 'SITE_PLANS_FETCH_FAILED'; export const SITE_PLANS_REMOVE = 'SITE_PLANS_REMOVE'; export const SITE_RECEIVE = 'SITE_RECEIVE'; export const SUPPORT_USER_TOKEN_FETCH = 'SUPPORT_USER_TOKEN_FETCH'; diff --git a/client/state/sites/plans/actions.js b/client/state/sites/plans/actions.js index 796e1e5b33b385..a1b02d60a03a4c 100644 --- a/client/state/sites/plans/actions.js +++ b/client/state/sites/plans/actions.js @@ -14,6 +14,7 @@ import { createSitePlanObject } from './assembler'; import { SITE_PLANS_FETCH, SITE_PLANS_FETCH_COMPLETED, + SITE_PLANS_FETCH_FAILED, SITE_PLANS_REMOVE } from 'state/action-types'; import wpcom from 'lib/wp'; @@ -48,6 +49,14 @@ export function fetchSitePlans( siteId ) { wpcom.undocumented().getSitePlans( siteId, ( error, data ) => { if ( error ) { debug( 'Fetching site plans failed: ', error ); + + const errorMessage = error.message || this.translate( 'There was a problem fetching site plans. Please try again later or contact support.' ); + + dispatch( { + type: SITE_PLANS_FETCH_FAILED, + siteId, + error: errorMessage + } ); } else { dispatch( fetchSitePlansCompleted( siteId, data ) ); } diff --git a/client/state/sites/plans/reducer.js b/client/state/sites/plans/reducer.js index c5f7091f3d9464..bf543971594ee3 100644 --- a/client/state/sites/plans/reducer.js +++ b/client/state/sites/plans/reducer.js @@ -9,6 +9,7 @@ import omit from 'lodash/object/omit'; import { SITE_PLANS_FETCH, SITE_PLANS_FETCH_COMPLETED, + SITE_PLANS_FETCH_FAILED, SITE_PLANS_REMOVE } from 'state/action-types'; import { SERIALIZE, DESERIALIZE } from 'state/action-types'; @@ -40,6 +41,14 @@ export function plans( state = {}, action ) { } ) } ); + case SITE_PLANS_FETCH_FAILED: + return Object.assign( {}, state, { + [ action.siteId ]: Object.assign( {}, initialSiteState, state[ action.siteId ], { + error: action.error, + isFetching: false + } ) + } ); + case SITE_PLANS_REMOVE: return omit( state, action.siteId ); diff --git a/client/state/sites/plans/test/reducer.js b/client/state/sites/plans/test/reducer.js index 0f61e253b99e15..4e5fbdd5058f45 100644 --- a/client/state/sites/plans/test/reducer.js +++ b/client/state/sites/plans/test/reducer.js @@ -9,6 +9,7 @@ import { expect } from 'chai'; import { SITE_PLANS_FETCH, SITE_PLANS_FETCH_COMPLETED, + SITE_PLANS_FETCH_FAILED, SITE_PLANS_REMOVE, SERIALIZE, DESERIALIZE @@ -72,6 +73,31 @@ describe( 'reducer', () => { } ); } ); + it( 'should return the original state with an error and fetching disabled when fetching failed', () => { + const original = Object.freeze( { + 11111111: { + data: [], + error: null, + hasLoadedFromServer: true, + isFetching: true + } + } ), + state = plans( original, { + type: SITE_PLANS_FETCH_FAILED, + siteId: 11111111, + error: 'Unable to fetch site plans', + } ); + + expect( state ).to.eql( { + 11111111: { + data: [], + error: 'Unable to fetch site plans', + hasLoadedFromServer: true, + isFetching: false + } + } ); + } ); + it( 'should return a list of plans as well as loaded from server enabled and fetching disabled when fetching completed', () => { const state = plans( undefined, { type: SITE_PLANS_FETCH_COMPLETED, From 9a914ca9ca849e227ec1e20d5e74b4c124397f66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Thomas?= Date: Mon, 1 Feb 2016 20:15:13 +0100 Subject: [PATCH 14/23] Plans: Update action for fetching site plans upon completion This makes obvious that this action expects a list of plans returned by the API - which will then be converted to a different data structure by the assembler. --- client/state/sites/plans/README.md | 4 ++-- client/state/sites/plans/actions.js | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/client/state/sites/plans/README.md b/client/state/sites/plans/README.md index bb377fde453594..331f2021da36a6 100644 --- a/client/state/sites/plans/README.md +++ b/client/state/sites/plans/README.md @@ -11,9 +11,9 @@ Used in combination with the Redux store instance `dispatch` function, actions c Fetches plans for the site with the given site ID. -### `fetchSitePlansCompleted( siteId: Number, plans: Object )` +### `fetchSitePlansCompleted( siteId: Number, data: Object )` -Adds the plans to the set of plans for the given site ID. +Adds the plans fetched from the API to the set of plans for the given site ID. ```js import { fetchSitePlans, fetchSitePlansCompleted } from 'state/sites/plans/actions'; diff --git a/client/state/sites/plans/actions.js b/client/state/sites/plans/actions.js index a1b02d60a03a4c..e4cc0b6087cf48 100644 --- a/client/state/sites/plans/actions.js +++ b/client/state/sites/plans/actions.js @@ -72,16 +72,16 @@ export function fetchSitePlans( siteId ) { * the plans for a given site have been received. * * @param {Number} siteId identifier of the site - * @param {Object} plans list of plans received from the API + * @param {Object} data list of plans received from the API * @returns {Object} the corresponding action object */ -export function fetchSitePlansCompleted( siteId, plans ) { - plans = reject( plans, '_headers' ); +export function fetchSitePlansCompleted( siteId, data ) { + data = reject( data, '_headers' ); return { type: SITE_PLANS_FETCH_COMPLETED, siteId, - plans: map( plans, createSitePlanObject ) + plans: map( data, createSitePlanObject ) }; } From 0d764435412bd39f2f37d429b2498e4776b5a702 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Thomas?= Date: Mon, 1 Feb 2016 20:38:54 +0100 Subject: [PATCH 15/23] Plans: Add new actions for canceling a plan free trial This also adds the corresponding unit tests. --- client/state/action-types.js | 3 + client/state/sites/plans/actions.js | 47 ++++++++- client/state/sites/plans/reducer.js | 35 ++++++- client/state/sites/plans/test/reducer.js | 118 ++++++++++++++++++++--- 4 files changed, 184 insertions(+), 19 deletions(-) diff --git a/client/state/action-types.js b/client/state/action-types.js index 3d7bd61903a67d..b8d860d0983934 100644 --- a/client/state/action-types.js +++ b/client/state/action-types.js @@ -48,6 +48,9 @@ export const SITE_PLANS_FETCH = 'SITE_PLANS_FETCH'; export const SITE_PLANS_FETCH_COMPLETED = 'SITE_PLANS_FETCH_COMPLETED'; export const SITE_PLANS_FETCH_FAILED = 'SITE_PLANS_FETCH_FAILED'; export const SITE_PLANS_REMOVE = 'SITE_PLANS_REMOVE'; +export const SITE_PLANS_TRIAL_CANCEL = 'SITE_PLANS_TRIAL_CANCEL'; +export const SITE_PLANS_TRIAL_CANCEL_COMPLETED = 'SITE_PLANS_TRIAL_CANCEL_COMPLETED'; +export const SITE_PLANS_TRIAL_CANCEL_FAILED = 'SITE_PLANS_TRIAL_CANCEL_FAILED'; export const SITE_RECEIVE = 'SITE_RECEIVE'; export const SUPPORT_USER_TOKEN_FETCH = 'SUPPORT_USER_TOKEN_FETCH'; export const SUPPORT_USER_TOKEN_SET = 'SUPPORT_USER_TOKEN_SET'; diff --git a/client/state/sites/plans/actions.js b/client/state/sites/plans/actions.js index e4cc0b6087cf48..c9f4a5ff4909e1 100644 --- a/client/state/sites/plans/actions.js +++ b/client/state/sites/plans/actions.js @@ -15,10 +15,55 @@ import { SITE_PLANS_FETCH, SITE_PLANS_FETCH_COMPLETED, SITE_PLANS_FETCH_FAILED, - SITE_PLANS_REMOVE + SITE_PLANS_REMOVE, + SITE_PLANS_TRIAL_CANCEL, + SITE_PLANS_TRIAL_CANCEL_COMPLETED, + SITE_PLANS_TRIAL_CANCEL_FAILED } from 'state/action-types'; import wpcom from 'lib/wp'; +/** + * Cancels the specified plan trial for the given site. + * + * @param {Number} siteId identifier of the site + * @param {Number} planId identifier of the plan + * @returns {Function} a promise that will resolve once updating is completed + */ +export function cancelSitePlanTrial( siteId, planId ) { + return ( dispatch ) => { + dispatch( { + type: SITE_PLANS_TRIAL_CANCEL, + siteId + } ); + + return new Promise( ( resolve, reject ) => { + wpcom.undocumented().cancelPlanTrial( planId, ( error, data ) => { + if ( data && data.success ) { + dispatch( { + type: SITE_PLANS_TRIAL_CANCEL_COMPLETED, + siteId, + plans: map( data.plans, createSitePlanObject ) + } ); + + resolve(); + } else { + debug( 'Canceling site plan trial failed: ', error ); + + const errorMessage = error.message || this.translate( 'There was a problem canceling the plan trial. Please try again later or contact support.' ); + + dispatch( { + type: SITE_PLANS_TRIAL_CANCEL_FAILED, + siteId, + error: errorMessage + } ); + + reject( errorMessage ); + } + } ); + } ); + } +} + /** * Returns an action object to be used in signalling that plans for the given site has been cleared. * diff --git a/client/state/sites/plans/reducer.js b/client/state/sites/plans/reducer.js index bf543971594ee3..5a34928946bee0 100644 --- a/client/state/sites/plans/reducer.js +++ b/client/state/sites/plans/reducer.js @@ -10,7 +10,10 @@ import { SITE_PLANS_FETCH, SITE_PLANS_FETCH_COMPLETED, SITE_PLANS_FETCH_FAILED, - SITE_PLANS_REMOVE + SITE_PLANS_REMOVE, + SITE_PLANS_TRIAL_CANCEL, + SITE_PLANS_TRIAL_CANCEL_COMPLETED, + SITE_PLANS_TRIAL_CANCEL_FAILED } from 'state/action-types'; import { SERIALIZE, DESERIALIZE } from 'state/action-types'; @@ -18,7 +21,8 @@ export const initialSiteState = { data: null, error: null, hasLoadedFromServer: false, - isFetching: false + isFetching: false, + isUpdating: false }; export function plans( state = {}, action ) { @@ -33,7 +37,7 @@ export function plans( state = {}, action ) { case SITE_PLANS_FETCH_COMPLETED: return Object.assign( {}, state, { - [ action.siteId ]: Object.assign( {}, state[ action.siteId ], { + [ action.siteId ]: Object.assign( {}, initialSiteState, state[ action.siteId ], { error: null, hasLoadedFromServer: true, isFetching: false, @@ -52,6 +56,31 @@ export function plans( state = {}, action ) { case SITE_PLANS_REMOVE: return omit( state, action.siteId ); + case SITE_PLANS_TRIAL_CANCEL: + return Object.assign( {}, state, { + [ action.siteId ]: Object.assign( {}, initialSiteState, state[ action.siteId ], { + isUpdating: true + } ) + } ); + + case SITE_PLANS_TRIAL_CANCEL_COMPLETED: + return Object.assign( {}, state, { + [ action.siteId ]: Object.assign( {}, initialSiteState, state[ action.siteId ], { + error: null, + hasLoadedFromServer: true, + isUpdating: false, + data: action.plans + } ) + } ); + + case SITE_PLANS_TRIAL_CANCEL_FAILED: + return Object.assign( {}, state, { + [ action.siteId ]: Object.assign( {}, initialSiteState, state[ action.siteId ], { + error: action.error, + isUpdating: false + } ) + } ); + case SERIALIZE: //TODO: we have full instances of moment.js on sites.plans[siteID].data return {}; diff --git a/client/state/sites/plans/test/reducer.js b/client/state/sites/plans/test/reducer.js index 4e5fbdd5058f45..be4425a29cedbc 100644 --- a/client/state/sites/plans/test/reducer.js +++ b/client/state/sites/plans/test/reducer.js @@ -10,6 +10,9 @@ import { SITE_PLANS_FETCH, SITE_PLANS_FETCH_COMPLETED, SITE_PLANS_FETCH_FAILED, + SITE_PLANS_TRIAL_CANCEL, + SITE_PLANS_TRIAL_CANCEL_FAILED, + SITE_PLANS_TRIAL_CANCEL_COMPLETED, SITE_PLANS_REMOVE, SERIALIZE, DESERIALIZE @@ -46,7 +49,8 @@ describe( 'reducer', () => { data: [], error: null, hasLoadedFromServer: true, - isFetching: false + isFetching: false, + isUpdating: false } } ), state = plans( original, { @@ -68,7 +72,8 @@ describe( 'reducer', () => { data: null, error: null, hasLoadedFromServer: false, - isFetching: true + isFetching: true, + isUpdating: false } } ); } ); @@ -79,13 +84,14 @@ describe( 'reducer', () => { data: [], error: null, hasLoadedFromServer: true, - isFetching: true + isFetching: true, + isUpdating: false } } ), state = plans( original, { type: SITE_PLANS_FETCH_FAILED, siteId: 11111111, - error: 'Unable to fetch site plans', + error: 'Unable to fetch site plans' } ); expect( state ).to.eql( { @@ -93,7 +99,8 @@ describe( 'reducer', () => { data: [], error: 'Unable to fetch site plans', hasLoadedFromServer: true, - isFetching: false + isFetching: false, + isUpdating: false } } ); } ); @@ -110,7 +117,8 @@ describe( 'reducer', () => { data: [], error: null, hasLoadedFromServer: true, - isFetching: false + isFetching: false, + isUpdating: false } } ); } ); @@ -121,7 +129,8 @@ describe( 'reducer', () => { data: [], error: null, hasLoadedFromServer: true, - isFetching: false + isFetching: false, + isUpdating: false } } ), state = plans( original, { @@ -134,13 +143,15 @@ describe( 'reducer', () => { data: [], error: null, hasLoadedFromServer: true, - isFetching: false + isFetching: false, + isUpdating: false }, 55555555: { data: null, error: null, hasLoadedFromServer: false, - isFetching: true + isFetching: true, + isUpdating: false } } ); } ); @@ -151,7 +162,8 @@ describe( 'reducer', () => { data: null, error: 'Unable to fetch site plans', hasLoadedFromServer: false, - isFetching: false + isFetching: false, + isUpdating: false } } ), state = plans( original, { @@ -164,7 +176,79 @@ describe( 'reducer', () => { data: null, error: null, hasLoadedFromServer: false, - isFetching: true + isFetching: true, + isUpdating: false + } + } ); + } ); + + it( 'should return the original state with updating enabled when trial cancelation is triggered', () => { + const original = Object.freeze( { + 11111111: { + data: [], + error: null, + hasLoadedFromServer: false, + isFetching: false, + isUpdating: false + } + } ), + state = plans( original, { + type: SITE_PLANS_TRIAL_CANCEL, + siteId: 11111111 + } ); + + expect( state ).to.eql( { + 11111111: { + data: [], + error: null, + hasLoadedFromServer: false, + isFetching: false, + isUpdating: true + } + } ); + } ); + + it( 'should return the original state with an error and updating disabled when trial cancelation failed', () => { + const original = Object.freeze( { + 11111111: { + data: [], + error: null, + hasLoadedFromServer: true, + isFetching: false, + isUpdating: false + } + } ), + state = plans( original, { + type: SITE_PLANS_TRIAL_CANCEL_FAILED, + siteId: 11111111, + error: 'Unable to cancel plan trial' + } ); + + expect( state ).to.eql( { + 11111111: { + data: [], + error: 'Unable to cancel plan trial', + hasLoadedFromServer: true, + isFetching: false, + isUpdating: false + } + } ); + } ); + + it( 'should return a list of plans as well as loaded from server enabled and updating disabled when trial cancelation completed', () => { + const state = plans( undefined, { + type: SITE_PLANS_TRIAL_CANCEL_COMPLETED, + siteId: 11111111, + plans: [] + } ); + + expect( state ).to.eql( { + 11111111: { + data: [], + error: null, + hasLoadedFromServer: true, + isFetching: false, + isUpdating: false } } ); } ); @@ -184,7 +268,8 @@ describe( 'reducer', () => { data: null, error: 'Unable to fetch site plans', hasLoadedFromServer: false, - isFetching: false + isFetching: false, + isUpdating: false } } ), state = plans( original, { @@ -201,13 +286,15 @@ describe( 'reducer', () => { data: null, error: 'Unable to fetch site plans', hasLoadedFromServer: false, - isFetching: false + isFetching: false, + isUpdating: false }, 22222222: { data: [], error: null, hasLoadedFromServer: true, - isFetching: false + isFetching: false, + isUpdating: false } } ), state = plans( original, { @@ -220,7 +307,8 @@ describe( 'reducer', () => { data: [], error: null, hasLoadedFromServer: true, - isFetching: false + isFetching: false, + isUpdating: false } } ); } ); From fc552a94d860309ff95ec70e9c02984cf295426b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Thomas?= Date: Mon, 1 Feb 2016 20:52:54 +0100 Subject: [PATCH 16/23] Plans: Extract duplicate code into function that update state in Site Plans reducer Less is more. --- client/state/sites/plans/reducer.js | 68 +++++++++++++++-------------- 1 file changed, 35 insertions(+), 33 deletions(-) diff --git a/client/state/sites/plans/reducer.js b/client/state/sites/plans/reducer.js index 5a34928946bee0..d9c2a85da3c716 100644 --- a/client/state/sites/plans/reducer.js +++ b/client/state/sites/plans/reducer.js @@ -25,60 +25,62 @@ export const initialSiteState = { isUpdating: false }; +/** + * Returns a new state with the given attributes updated for the specified site. + * + * @param {Object} state current state + * @param {Number} siteId identifier of the site + * @param {Object} attributes list of attributes and their values + * @returns {Object} the new state + */ +function updateSiteState( state, siteId, attributes ) { + return Object.assign( {}, state, { + [ siteId ]: Object.assign( {}, initialSiteState, state[ siteId ], attributes ) + } ); +} + export function plans( state = {}, action ) { switch ( action.type ) { case SITE_PLANS_FETCH: - return Object.assign( {}, state, { - [ action.siteId ]: Object.assign( {}, initialSiteState, state[ action.siteId ], { - error: null, - isFetching: true - } ) + return updateSiteState( state, action.siteId, { + error: null, + isFetching: true } ); case SITE_PLANS_FETCH_COMPLETED: - return Object.assign( {}, state, { - [ action.siteId ]: Object.assign( {}, initialSiteState, state[ action.siteId ], { - error: null, - hasLoadedFromServer: true, - isFetching: false, - data: action.plans - } ) + return updateSiteState( state, action.siteId, { + error: null, + hasLoadedFromServer: true, + isFetching: false, + data: action.plans } ); case SITE_PLANS_FETCH_FAILED: - return Object.assign( {}, state, { - [ action.siteId ]: Object.assign( {}, initialSiteState, state[ action.siteId ], { - error: action.error, - isFetching: false - } ) + return updateSiteState( state, action.siteId, { + error: action.error, + isFetching: false } ); case SITE_PLANS_REMOVE: return omit( state, action.siteId ); case SITE_PLANS_TRIAL_CANCEL: - return Object.assign( {}, state, { - [ action.siteId ]: Object.assign( {}, initialSiteState, state[ action.siteId ], { - isUpdating: true - } ) + return updateSiteState( state, action.siteId, { + isUpdating: true } ); case SITE_PLANS_TRIAL_CANCEL_COMPLETED: - return Object.assign( {}, state, { - [ action.siteId ]: Object.assign( {}, initialSiteState, state[ action.siteId ], { - error: null, - hasLoadedFromServer: true, - isUpdating: false, - data: action.plans - } ) + return updateSiteState( state, action.siteId, { + error: null, + hasLoadedFromServer: true, + isUpdating: false, + data: action.plans } ); case SITE_PLANS_TRIAL_CANCEL_FAILED: - return Object.assign( {}, state, { - [ action.siteId ]: Object.assign( {}, initialSiteState, state[ action.siteId ], { - error: action.error, - isUpdating: false - } ) + return updateSiteState( state, action.siteId, { + error: action.error, + isUpdating: false } ); case SERIALIZE: From a9848cc683b8c71930570c51d3b52961d859847b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Thomas?= Date: Mon, 1 Feb 2016 20:56:41 +0100 Subject: [PATCH 17/23] Plans: Reduxify the Plan Remove component --- client/my-sites/plans/main.jsx | 3 +- client/my-sites/plans/plan-overview/index.jsx | 6 +- .../plans/plan-overview/plan-remove/index.jsx | 57 +++++++++++-------- 3 files changed, 36 insertions(+), 30 deletions(-) diff --git a/client/my-sites/plans/main.jsx b/client/my-sites/plans/main.jsx index da2069e60ab7bb..b1c35548340c7f 100644 --- a/client/my-sites/plans/main.jsx +++ b/client/my-sites/plans/main.jsx @@ -148,8 +148,7 @@ var Plans = React.createClass( { cart={ this.props.cart } destinationType={ this.props.context.params.destinationType } plan={ currentPlan } - selectedSite={ selectedSite } - store={ this.props.context.store } /> + selectedSite={ selectedSite } /> ); } diff --git a/client/my-sites/plans/plan-overview/index.jsx b/client/my-sites/plans/plan-overview/index.jsx index c24a284e9e7e0f..6fe12a6cd7f779 100644 --- a/client/my-sites/plans/plan-overview/index.jsx +++ b/client/my-sites/plans/plan-overview/index.jsx @@ -25,8 +25,7 @@ const PlanOverview = React.createClass( { selectedSite: React.PropTypes.oneOfType( [ React.PropTypes.object, React.PropTypes.bool - ] ).isRequired, - store: React.PropTypes.object.isRequired + ] ).isRequired }, redirectToDefault() { @@ -70,8 +69,7 @@ const PlanOverview = React.createClass( { + selectedSite={ this.props.selectedSite } /> ); diff --git a/client/my-sites/plans/plan-overview/plan-remove/index.jsx b/client/my-sites/plans/plan-overview/plan-remove/index.jsx index 4418ace733b0dd..4c038906b26292 100644 --- a/client/my-sites/plans/plan-overview/plan-remove/index.jsx +++ b/client/my-sites/plans/plan-overview/plan-remove/index.jsx @@ -8,54 +8,48 @@ import React from 'react'; /** * Internal dependencies */ +import { cancelSitePlanTrial } from 'state/sites/plans/actions'; import CompactCard from 'components/card/compact'; +import { connect } from 'react-redux'; import Dialog from 'components/dialog'; +import { getPlansBySite } from 'state/sites/plans/selectors'; import { isInGracePeriod } from 'lib/plans'; import notices from 'notices'; import paths from '../../paths'; -import { fetchSitePlansCompleted } from 'state/sites/plans/actions'; -import wpcom from 'lib/wp'; const PlanRemove = React.createClass( { propTypes: { + cancelSitePlanTrial: React.PropTypes.func.isRequired, + isUpdating: React.PropTypes.bool.isRequired, plan: React.PropTypes.object.isRequired, selectedSite: React.PropTypes.oneOfType( [ React.PropTypes.object, React.PropTypes.bool - ] ).isRequired, - store: React.PropTypes.object.isRequired + ] ).isRequired }, getInitialState() { return { - isCanceling: false, showDialog: false }; }, removePlan( closeDialog ) { - this.setState( { isCanceling: true } ); + this.props.cancelSitePlanTrial( this.props.selectedSite.ID, this.props.plan.id ).then( () => { + Dispatcher.handleViewAction( { + type: 'FETCH_SITES' + } ); - wpcom.undocumented().cancelPlanTrial( this.props.plan.id, ( error, data ) => { - if ( data && data.success ) { - this.props.store.dispatch( fetchSitePlansCompleted( this.props.selectedSite.ID, data.plans ) ); + page( paths.plansDestination( this.props.selectedSite.slug, 'free-trial-canceled' ) ); + } ).catch( ( error ) => { + closeDialog(); - Dispatcher.handleViewAction( { - type: 'FETCH_SITES' - } ); - - page( paths.plansDestination( this.props.selectedSite.slug, 'free-trial-canceled' ) ); - } else { - closeDialog(); - - notices.error( error.message || this.translate( 'There was a problem removing the plan. Please try again later or contact support.' ) ); - } + notices.error( error ); } ); }, closeDialog() { - this.setState( { - isCanceling: false, + this.setState( { showDialog: false } ); }, @@ -84,12 +78,12 @@ const PlanRemove = React.createClass( { const buttons = [ { action: 'cancel', - disabled: this.state.isCanceling, + disabled: this.props.isUpdating, label: this.translate( 'Cancel' ) }, { action: 'remove', - disabled: this.state.isCanceling, + disabled: this.props.isUpdating, isPrimary: true, label: this.translate( 'Remove Now' ), onClick: this.removePlan @@ -135,4 +129,19 @@ const PlanRemove = React.createClass( { } } ); -export default PlanRemove; +export default connect( + ( state, props ) => { + const plans = getPlansBySite( state, props.selectedSite ); + + return { + isUpdating: plans.isUpdating + }; + }, + ( dispatch ) => { + return { + cancelSitePlanTrial: ( siteId, planId ) => { + return dispatch( cancelSitePlanTrial( siteId, planId ) ); + } + }; + } +)( PlanRemove ); From 192fabd1f2226fcf3e02b4097d6cd00b4dcd28e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Thomas?= Date: Tue, 2 Feb 2016 16:50:50 +0100 Subject: [PATCH 18/23] Plans: Refine description of unit tests for Site Plans reducer --- client/state/sites/plans/test/reducer.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/state/sites/plans/test/reducer.js b/client/state/sites/plans/test/reducer.js index be4425a29cedbc..78f48d6807948a 100644 --- a/client/state/sites/plans/test/reducer.js +++ b/client/state/sites/plans/test/reducer.js @@ -105,7 +105,7 @@ describe( 'reducer', () => { } ); } ); - it( 'should return a list of plans as well as loaded from server enabled and fetching disabled when fetching completed', () => { + it( 'should return a list of plans with loaded from server enabled and fetching disabled when fetching completed', () => { const state = plans( undefined, { type: SITE_PLANS_FETCH_COMPLETED, siteId: 11111111, @@ -235,7 +235,7 @@ describe( 'reducer', () => { } ); } ); - it( 'should return a list of plans as well as loaded from server enabled and updating disabled when trial cancelation completed', () => { + it( 'should return a list of plans with loaded from server enabled and updating disabled when trial cancelation completed', () => { const state = plans( undefined, { type: SITE_PLANS_TRIAL_CANCEL_COMPLETED, siteId: 11111111, From aa10d11fe170980fb66b55bc05a2c00bf460ee11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Thomas?= Date: Tue, 2 Feb 2016 16:52:22 +0100 Subject: [PATCH 19/23] Plans: Fixed wrong translate calls in Site Plans actions --- client/state/sites/plans/actions.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/client/state/sites/plans/actions.js b/client/state/sites/plans/actions.js index c9f4a5ff4909e1..27a6cd8c81dc6f 100644 --- a/client/state/sites/plans/actions.js +++ b/client/state/sites/plans/actions.js @@ -11,6 +11,7 @@ const debug = debugFactory( 'calypso:site-plans:actions' ); * Internal dependencies */ import { createSitePlanObject } from './assembler'; +import i18n from 'lib/mixins/i18n'; import { SITE_PLANS_FETCH, SITE_PLANS_FETCH_COMPLETED, @@ -49,7 +50,7 @@ export function cancelSitePlanTrial( siteId, planId ) { } else { debug( 'Canceling site plan trial failed: ', error ); - const errorMessage = error.message || this.translate( 'There was a problem canceling the plan trial. Please try again later or contact support.' ); + const errorMessage = error.message || i18n.translate( 'There was a problem canceling the plan trial. Please try again later or contact support.' ); dispatch( { type: SITE_PLANS_TRIAL_CANCEL_FAILED, @@ -95,7 +96,7 @@ export function fetchSitePlans( siteId ) { if ( error ) { debug( 'Fetching site plans failed: ', error ); - const errorMessage = error.message || this.translate( 'There was a problem fetching site plans. Please try again later or contact support.' ); + const errorMessage = error.message || i18n.translate( 'There was a problem fetching site plans. Please try again later or contact support.' ); dispatch( { type: SITE_PLANS_FETCH_FAILED, From ff3702d19d14d4d1701a5864d84090c2bb51eed5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Thomas?= Date: Fri, 5 Feb 2016 15:10:52 +0100 Subject: [PATCH 20/23] Plans: Merge imports in Site Plans reducer --- client/state/sites/plans/reducer.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/client/state/sites/plans/reducer.js b/client/state/sites/plans/reducer.js index d9c2a85da3c716..1c01c86e0de988 100644 --- a/client/state/sites/plans/reducer.js +++ b/client/state/sites/plans/reducer.js @@ -13,9 +13,10 @@ import { SITE_PLANS_REMOVE, SITE_PLANS_TRIAL_CANCEL, SITE_PLANS_TRIAL_CANCEL_COMPLETED, - SITE_PLANS_TRIAL_CANCEL_FAILED + SITE_PLANS_TRIAL_CANCEL_FAILED, + SERIALIZE, + DESERIALIZE } from 'state/action-types'; -import { SERIALIZE, DESERIALIZE } from 'state/action-types'; export const initialSiteState = { data: null, From f3d3e12f651abdbe2823a9303b644012a9207329 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Thomas?= Date: Fri, 5 Feb 2016 15:15:09 +0100 Subject: [PATCH 21/23] Plans: Fixed Site Plans reducer unit tests failing after a merge --- client/state/sites/plans/test/reducer.js | 39 +++++++++++++++++++----- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/client/state/sites/plans/test/reducer.js b/client/state/sites/plans/test/reducer.js index 78f48d6807948a..c78c1f47209775 100644 --- a/client/state/sites/plans/test/reducer.js +++ b/client/state/sites/plans/test/reducer.js @@ -315,19 +315,42 @@ describe( 'reducer', () => { it( 'never persists state because this is not implemented', () => { const original = Object.freeze( { - 11111111: initialSiteState, - 22222222: initialSiteState - } ); - const state = plans( original, { type: SERIALIZE } ); + 11111111: { + data: null, + error: 'Unable to fetch site plans', + hasLoadedFromServer: false, + isFetching: false, + isUpdating: false + } + } ), + state = plans( original, { + type: SERIALIZE + } ); + expect( state ).to.eql( {} ); } ); it( 'never loads persisted state because this is not implemented', () => { const original = Object.freeze( { - 11111111: initialSiteState, - 22222222: initialSiteState - } ); - const state = plans( original, { type: DESERIALIZE } ); + 11111111: { + data: null, + error: null, + hasLoadedFromServer: false, + isFetching: false, + isUpdating: false + }, + 22222222: { + data: [], + error: null, + hasLoadedFromServer: true, + isFetching: false, + isUpdating: false + } + } ), + state = plans( original, { + type: DESERIALIZE + } ); + expect( state ).to.eql( {} ); } ); } ); From 2355f59a900dcc7d85c574cbf37bb50c01b8c5b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Thomas?= Date: Fri, 5 Feb 2016 16:08:50 +0100 Subject: [PATCH 22/23] Plans: Rename updating flag in Site Plans reducer This is the first step to merging this flag with the one for fetching since we don't really need to manage both states after all. --- .../plans/plan-overview/plan-remove/index.jsx | 8 ++-- client/state/sites/plans/reducer.js | 8 ++-- client/state/sites/plans/test/reducer.js | 44 +++++++++---------- 3 files changed, 30 insertions(+), 30 deletions(-) diff --git a/client/my-sites/plans/plan-overview/plan-remove/index.jsx b/client/my-sites/plans/plan-overview/plan-remove/index.jsx index 4c038906b26292..0de305e31c9460 100644 --- a/client/my-sites/plans/plan-overview/plan-remove/index.jsx +++ b/client/my-sites/plans/plan-overview/plan-remove/index.jsx @@ -20,7 +20,7 @@ import paths from '../../paths'; const PlanRemove = React.createClass( { propTypes: { cancelSitePlanTrial: React.PropTypes.func.isRequired, - isUpdating: React.PropTypes.bool.isRequired, + isRequesting: React.PropTypes.bool.isRequired, plan: React.PropTypes.object.isRequired, selectedSite: React.PropTypes.oneOfType( [ React.PropTypes.object, @@ -78,12 +78,12 @@ const PlanRemove = React.createClass( { const buttons = [ { action: 'cancel', - disabled: this.props.isUpdating, + disabled: this.props.isRequesting, label: this.translate( 'Cancel' ) }, { action: 'remove', - disabled: this.props.isUpdating, + disabled: this.props.isRequesting, isPrimary: true, label: this.translate( 'Remove Now' ), onClick: this.removePlan @@ -134,7 +134,7 @@ export default connect( const plans = getPlansBySite( state, props.selectedSite ); return { - isUpdating: plans.isUpdating + isRequesting: plans.isRequesting }; }, ( dispatch ) => { diff --git a/client/state/sites/plans/reducer.js b/client/state/sites/plans/reducer.js index 1c01c86e0de988..11b72704bbb73f 100644 --- a/client/state/sites/plans/reducer.js +++ b/client/state/sites/plans/reducer.js @@ -23,7 +23,7 @@ export const initialSiteState = { error: null, hasLoadedFromServer: false, isFetching: false, - isUpdating: false + isRequesting: false }; /** @@ -67,21 +67,21 @@ export function plans( state = {}, action ) { case SITE_PLANS_TRIAL_CANCEL: return updateSiteState( state, action.siteId, { - isUpdating: true + isRequesting: true } ); case SITE_PLANS_TRIAL_CANCEL_COMPLETED: return updateSiteState( state, action.siteId, { error: null, hasLoadedFromServer: true, - isUpdating: false, + isRequesting: false, data: action.plans } ); case SITE_PLANS_TRIAL_CANCEL_FAILED: return updateSiteState( state, action.siteId, { error: action.error, - isUpdating: false + isRequesting: false } ); case SERIALIZE: diff --git a/client/state/sites/plans/test/reducer.js b/client/state/sites/plans/test/reducer.js index c78c1f47209775..21156a50366df3 100644 --- a/client/state/sites/plans/test/reducer.js +++ b/client/state/sites/plans/test/reducer.js @@ -50,7 +50,7 @@ describe( 'reducer', () => { error: null, hasLoadedFromServer: true, isFetching: false, - isUpdating: false + isRequesting: false } } ), state = plans( original, { @@ -73,7 +73,7 @@ describe( 'reducer', () => { error: null, hasLoadedFromServer: false, isFetching: true, - isUpdating: false + isRequesting: false } } ); } ); @@ -85,7 +85,7 @@ describe( 'reducer', () => { error: null, hasLoadedFromServer: true, isFetching: true, - isUpdating: false + isRequesting: false } } ), state = plans( original, { @@ -100,7 +100,7 @@ describe( 'reducer', () => { error: 'Unable to fetch site plans', hasLoadedFromServer: true, isFetching: false, - isUpdating: false + isRequesting: false } } ); } ); @@ -118,7 +118,7 @@ describe( 'reducer', () => { error: null, hasLoadedFromServer: true, isFetching: false, - isUpdating: false + isRequesting: false } } ); } ); @@ -130,7 +130,7 @@ describe( 'reducer', () => { error: null, hasLoadedFromServer: true, isFetching: false, - isUpdating: false + isRequesting: false } } ), state = plans( original, { @@ -144,14 +144,14 @@ describe( 'reducer', () => { error: null, hasLoadedFromServer: true, isFetching: false, - isUpdating: false + isRequesting: false }, 55555555: { data: null, error: null, hasLoadedFromServer: false, isFetching: true, - isUpdating: false + isRequesting: false } } ); } ); @@ -163,7 +163,7 @@ describe( 'reducer', () => { error: 'Unable to fetch site plans', hasLoadedFromServer: false, isFetching: false, - isUpdating: false + isRequesting: false } } ), state = plans( original, { @@ -177,7 +177,7 @@ describe( 'reducer', () => { error: null, hasLoadedFromServer: false, isFetching: true, - isUpdating: false + isRequesting: false } } ); } ); @@ -189,7 +189,7 @@ describe( 'reducer', () => { error: null, hasLoadedFromServer: false, isFetching: false, - isUpdating: false + isRequesting: false } } ), state = plans( original, { @@ -203,7 +203,7 @@ describe( 'reducer', () => { error: null, hasLoadedFromServer: false, isFetching: false, - isUpdating: true + isRequesting: true } } ); } ); @@ -215,7 +215,7 @@ describe( 'reducer', () => { error: null, hasLoadedFromServer: true, isFetching: false, - isUpdating: false + isRequesting: false } } ), state = plans( original, { @@ -230,7 +230,7 @@ describe( 'reducer', () => { error: 'Unable to cancel plan trial', hasLoadedFromServer: true, isFetching: false, - isUpdating: false + isRequesting: false } } ); } ); @@ -248,7 +248,7 @@ describe( 'reducer', () => { error: null, hasLoadedFromServer: true, isFetching: false, - isUpdating: false + isRequesting: false } } ); } ); @@ -269,7 +269,7 @@ describe( 'reducer', () => { error: 'Unable to fetch site plans', hasLoadedFromServer: false, isFetching: false, - isUpdating: false + isRequesting: false } } ), state = plans( original, { @@ -287,14 +287,14 @@ describe( 'reducer', () => { error: 'Unable to fetch site plans', hasLoadedFromServer: false, isFetching: false, - isUpdating: false + isRequesting: false }, 22222222: { data: [], error: null, hasLoadedFromServer: true, isFetching: false, - isUpdating: false + isRequesting: false } } ), state = plans( original, { @@ -308,7 +308,7 @@ describe( 'reducer', () => { error: null, hasLoadedFromServer: true, isFetching: false, - isUpdating: false + isRequesting: false } } ); } ); @@ -320,7 +320,7 @@ describe( 'reducer', () => { error: 'Unable to fetch site plans', hasLoadedFromServer: false, isFetching: false, - isUpdating: false + isRequesting: false } } ), state = plans( original, { @@ -337,14 +337,14 @@ describe( 'reducer', () => { error: null, hasLoadedFromServer: false, isFetching: false, - isUpdating: false + isRequesting: false }, 22222222: { data: [], error: null, hasLoadedFromServer: true, isFetching: false, - isUpdating: false + isRequesting: false } } ), state = plans( original, { From 16e700eea8cfc50866e1d2f2dcecb9c35e9a46c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Thomas?= Date: Fri, 5 Feb 2016 16:23:01 +0100 Subject: [PATCH 23/23] Plans: Rename fetching flag in Site Plans reducer This is the last step to merging this flag with the one used previously for updating - so we only have to manage a single state. --- client/lib/plans/index.js | 2 +- client/state/sites/plans/reducer.js | 7 ++-- client/state/sites/plans/test/reducer.js | 42 ++++++------------------ 3 files changed, 14 insertions(+), 37 deletions(-) diff --git a/client/lib/plans/index.js b/client/lib/plans/index.js index ef70ea06a0b55c..254bafb9bf0aa4 100644 --- a/client/lib/plans/index.js +++ b/client/lib/plans/index.js @@ -59,7 +59,7 @@ export function isInGracePeriod( plan ) { }; export function shouldFetchSitePlans( sitePlans, selectedSite ) { - return ! sitePlans.hasLoadedFromServer && ! sitePlans.isFetching && selectedSite; + return ! sitePlans.hasLoadedFromServer && ! sitePlans.isRequesting && selectedSite; }; export function filterPlansBySiteAndProps( plans, site, hideFreePlan ) { diff --git a/client/state/sites/plans/reducer.js b/client/state/sites/plans/reducer.js index 11b72704bbb73f..e851191fe0e501 100644 --- a/client/state/sites/plans/reducer.js +++ b/client/state/sites/plans/reducer.js @@ -22,7 +22,6 @@ export const initialSiteState = { data: null, error: null, hasLoadedFromServer: false, - isFetching: false, isRequesting: false }; @@ -45,21 +44,21 @@ export function plans( state = {}, action ) { case SITE_PLANS_FETCH: return updateSiteState( state, action.siteId, { error: null, - isFetching: true + isRequesting: true } ); case SITE_PLANS_FETCH_COMPLETED: return updateSiteState( state, action.siteId, { error: null, hasLoadedFromServer: true, - isFetching: false, + isRequesting: false, data: action.plans } ); case SITE_PLANS_FETCH_FAILED: return updateSiteState( state, action.siteId, { error: action.error, - isFetching: false + isRequesting: false } ); case SITE_PLANS_REMOVE: diff --git a/client/state/sites/plans/test/reducer.js b/client/state/sites/plans/test/reducer.js index 21156a50366df3..f5bf7c1b62deca 100644 --- a/client/state/sites/plans/test/reducer.js +++ b/client/state/sites/plans/test/reducer.js @@ -49,7 +49,6 @@ describe( 'reducer', () => { data: [], error: null, hasLoadedFromServer: true, - isFetching: false, isRequesting: false } } ), @@ -61,7 +60,7 @@ describe( 'reducer', () => { expect( state ).to.eql( original ); } ); - it( 'should return the initial state with fetching enabled when fetching is triggered', () => { + it( 'should return the initial state with requesting enabled when fetching is triggered', () => { const state = plans( undefined, { type: SITE_PLANS_FETCH, siteId: 11111111 @@ -72,20 +71,18 @@ describe( 'reducer', () => { data: null, error: null, hasLoadedFromServer: false, - isFetching: true, - isRequesting: false + isRequesting: true } } ); } ); - it( 'should return the original state with an error and fetching disabled when fetching failed', () => { + it( 'should return the original state with an error and requesting disabled when fetching failed', () => { const original = Object.freeze( { 11111111: { data: [], error: null, hasLoadedFromServer: true, - isFetching: true, - isRequesting: false + isRequesting: true } } ), state = plans( original, { @@ -99,13 +96,12 @@ describe( 'reducer', () => { data: [], error: 'Unable to fetch site plans', hasLoadedFromServer: true, - isFetching: false, isRequesting: false } } ); } ); - it( 'should return a list of plans with loaded from server enabled and fetching disabled when fetching completed', () => { + it( 'should return a list of plans with loaded from server enabled and requesting disabled when fetching completed', () => { const state = plans( undefined, { type: SITE_PLANS_FETCH_COMPLETED, siteId: 11111111, @@ -117,7 +113,6 @@ describe( 'reducer', () => { data: [], error: null, hasLoadedFromServer: true, - isFetching: false, isRequesting: false } } ); @@ -129,7 +124,6 @@ describe( 'reducer', () => { data: [], error: null, hasLoadedFromServer: true, - isFetching: false, isRequesting: false } } ), @@ -143,15 +137,13 @@ describe( 'reducer', () => { data: [], error: null, hasLoadedFromServer: true, - isFetching: false, isRequesting: false }, 55555555: { data: null, error: null, hasLoadedFromServer: false, - isFetching: true, - isRequesting: false + isRequesting: true } } ); } ); @@ -162,7 +154,6 @@ describe( 'reducer', () => { data: null, error: 'Unable to fetch site plans', hasLoadedFromServer: false, - isFetching: false, isRequesting: false } } ), @@ -176,8 +167,7 @@ describe( 'reducer', () => { data: null, error: null, hasLoadedFromServer: false, - isFetching: true, - isRequesting: false + isRequesting: true } } ); } ); @@ -188,7 +178,6 @@ describe( 'reducer', () => { data: [], error: null, hasLoadedFromServer: false, - isFetching: false, isRequesting: false } } ), @@ -202,20 +191,18 @@ describe( 'reducer', () => { data: [], error: null, hasLoadedFromServer: false, - isFetching: false, isRequesting: true } } ); } ); - it( 'should return the original state with an error and updating disabled when trial cancelation failed', () => { + it( 'should return the original state with an error and requesting disabled when trial cancelation failed', () => { const original = Object.freeze( { 11111111: { data: [], error: null, hasLoadedFromServer: true, - isFetching: false, - isRequesting: false + isRequesting: true } } ), state = plans( original, { @@ -229,13 +216,12 @@ describe( 'reducer', () => { data: [], error: 'Unable to cancel plan trial', hasLoadedFromServer: true, - isFetching: false, isRequesting: false } } ); } ); - it( 'should return a list of plans with loaded from server enabled and updating disabled when trial cancelation completed', () => { + it( 'should return a list of plans with loaded from server enabled and requesting disabled when trial cancelation completed', () => { const state = plans( undefined, { type: SITE_PLANS_TRIAL_CANCEL_COMPLETED, siteId: 11111111, @@ -247,7 +233,6 @@ describe( 'reducer', () => { data: [], error: null, hasLoadedFromServer: true, - isFetching: false, isRequesting: false } } ); @@ -268,7 +253,6 @@ describe( 'reducer', () => { data: null, error: 'Unable to fetch site plans', hasLoadedFromServer: false, - isFetching: false, isRequesting: false } } ), @@ -286,14 +270,12 @@ describe( 'reducer', () => { data: null, error: 'Unable to fetch site plans', hasLoadedFromServer: false, - isFetching: false, isRequesting: false }, 22222222: { data: [], error: null, hasLoadedFromServer: true, - isFetching: false, isRequesting: false } } ), @@ -307,7 +289,6 @@ describe( 'reducer', () => { data: [], error: null, hasLoadedFromServer: true, - isFetching: false, isRequesting: false } } ); @@ -319,7 +300,6 @@ describe( 'reducer', () => { data: null, error: 'Unable to fetch site plans', hasLoadedFromServer: false, - isFetching: false, isRequesting: false } } ), @@ -336,14 +316,12 @@ describe( 'reducer', () => { data: null, error: null, hasLoadedFromServer: false, - isFetching: false, isRequesting: false }, 22222222: { data: [], error: null, hasLoadedFromServer: true, - isFetching: false, isRequesting: false } } ),