From ad876cab2e217eb073827afe90d710cfb1a0c074 Mon Sep 17 00:00:00 2001 From: Jordan West Date: Mon, 8 Feb 2016 18:01:10 +1000 Subject: [PATCH] Export: Automatically fetch advanced settings when switching to exporter Squashed: Export: Show placeholders while loading options Export: Make existing tests pass Export: Add unit tests for fetchingAdvancedSettings reducer Export: Remove redundant serialize/deserialize --- .../my-sites/exporter/advanced-settings.jsx | 20 ++++++- client/my-sites/exporter/exporter.jsx | 14 ++++- client/my-sites/exporter/index.jsx | 13 +++-- client/my-sites/exporter/option-fieldset.jsx | 27 ++++++--- client/my-sites/exporter/style.scss | 11 ++++ .../state/site-settings/exporter/actions.js | 12 +++- .../state/site-settings/exporter/reducers.js | 28 ++++++++++ .../state/site-settings/exporter/selectors.js | 1 + .../site-settings/exporter/test/actions.js | 11 +++- .../site-settings/exporter/test/reducer.js | 56 ++++++++++++++++++- 10 files changed, 171 insertions(+), 22 deletions(-) diff --git a/client/my-sites/exporter/advanced-settings.jsx b/client/my-sites/exporter/advanced-settings.jsx index 58a9acb5dd4d2..bb05a8db59760 100644 --- a/client/my-sites/exporter/advanced-settings.jsx +++ b/client/my-sites/exporter/advanced-settings.jsx @@ -2,6 +2,7 @@ * External dependencies */ import React, { PropTypes } from 'react'; +import { connect } from 'react-redux'; /** * Internal dependencies @@ -9,6 +10,8 @@ import React, { PropTypes } from 'react'; import OptionFieldset from 'my-sites/exporter/option-fieldset'; import SpinnerButton from './spinner-button'; +import { advancedSettings } from 'state/site-settings/exporter/selectors'; + /** * Displays additional options for customising an export * @@ -16,7 +19,7 @@ import SpinnerButton from './spinner-button'; * exported. Posts and Pages can also be filtered by Authors, Statuses, * and Date. */ -export default React.createClass( { +const AdvancedSettings = React.createClass( { displayName: 'AdvancedSettings', propTypes: { @@ -56,7 +59,8 @@ export default React.createClass( { legend: legends[ key ], isEnabled: this.props.postType === key, menus: menus[ key ], - onSelect: () => this.props.onSelectPostType( key ) + onSelect: () => this.props.onSelectPostType( key ), + shouldShowPlaceholders: this.props.shouldShowPlaceholders } ); return ( @@ -81,7 +85,7 @@ export default React.createClass( { { + const siteId = state.ui.selectedSiteId; + return { + siteId: siteId, + shouldShowPlaceholders: ! advancedSettings( state, siteId ) + }; +}; + +export default connect( mapStateToProps )( AdvancedSettings ); diff --git a/client/my-sites/exporter/exporter.jsx b/client/my-sites/exporter/exporter.jsx index 9914d5985fd43..45e567927f65c 100644 --- a/client/my-sites/exporter/exporter.jsx +++ b/client/my-sites/exporter/exporter.jsx @@ -16,9 +16,21 @@ export default React.createClass( { propTypes: { startExport: PropTypes.func.isRequired, setPostType: PropTypes.func.isRequired, + advancedSettingsFetch: PropTypes.func.isRequired, shouldShowProgress: PropTypes.bool.isRequired, - postType: PropTypes.string + postType: PropTypes.string, + siteId: PropTypes.number + }, + + componentWillMount() { + this.props.advancedSettingsFetch( this.props.siteId ); + }, + + componentWillReceiveProps( newProps ) { + if ( newProps.siteId !== this.props.siteId ) { + this.props.advancedSettingsFetch( newProps.siteId ); + } }, render: function() { diff --git a/client/my-sites/exporter/index.jsx b/client/my-sites/exporter/index.jsx index 072d568179beb..d3bd4987d46c8 100644 --- a/client/my-sites/exporter/index.jsx +++ b/client/my-sites/exporter/index.jsx @@ -12,11 +12,15 @@ import { shouldShowProgress, getSelectedPostType, } from 'state/site-settings/exporter/selectors'; -import { setPostType, startExport } from 'state/site-settings/exporter/actions'; +import { + setPostType, + startExport, + advancedSettingsFetch +} from 'state/site-settings/exporter/actions'; -function mapStateToProps( state, ownProps ) { +function mapStateToProps( state ) { return { - site: ownProps.site, + siteId: state.ui.selectedSiteId, postType: getSelectedPostType( state ), shouldShowProgress: shouldShowProgress( state ) }; @@ -25,7 +29,8 @@ function mapStateToProps( state, ownProps ) { function mapDispatchToProps( dispatch ) { return { setPostType: compose( dispatch, setPostType ), - startExport: () => startExport()( dispatch ) + startExport: () => startExport()( dispatch ), + advancedSettingsFetch: compose( dispatch, advancedSettingsFetch ) }; } diff --git a/client/my-sites/exporter/option-fieldset.jsx b/client/my-sites/exporter/option-fieldset.jsx index e4fad31f8ff4a..66edf05165769 100644 --- a/client/my-sites/exporter/option-fieldset.jsx +++ b/client/my-sites/exporter/option-fieldset.jsx @@ -27,7 +27,8 @@ module.exports = React.createClass( { onSelect: PropTypes.func, legend: PropTypes.string.isRequired, - isEnabled: PropTypes.bool.isRequired + isEnabled: PropTypes.bool.isRequired, + shouldShowPlaceholders: PropTypes.bool.isRequired, }, getDefaultProps() { @@ -37,6 +38,20 @@ module.exports = React.createClass( { }, render() { + const placeholderMap = ( menu, menuIndex ) => ( +
+ { this.translate( 'Loading options…' ) } +
+ ) + + const selectMap = ( menu, menuIndex ) => ( + + ) + return (
@@ -54,13 +69,9 @@ module.exports = React.createClass( { }
- { this.props.menus.map( ( menu, menuIndex ) => ( - - ) ) } + { this.props.menus.map( + this.props.shouldShowPlaceholders ? placeholderMap : selectMap + ) }
diff --git a/client/my-sites/exporter/style.scss b/client/my-sites/exporter/style.scss index 4f35b4179cc23..9b81bffc3e5a5 100644 --- a/client/my-sites/exporter/style.scss +++ b/client/my-sites/exporter/style.scss @@ -26,6 +26,17 @@ float: right; } +.exporter__placeholder-text { + padding-left: 24px; + padding-right: 24px; + margin-bottom: 24px; + height: 32px; + + color: transparent; + background-color: lighten( $gray, 30% ); + animation: loading-fade 1.6s ease-in-out infinite; +} + .exporter__advanced-settings { padding-bottom: 50px; } diff --git a/client/state/site-settings/exporter/actions.js b/client/state/site-settings/exporter/actions.js index a86f5bcd46e7a..a72275360f1d8 100644 --- a/client/state/site-settings/exporter/actions.js +++ b/client/state/site-settings/exporter/actions.js @@ -35,7 +35,15 @@ export function setPostType( postType ) { * @return {thunk} An action thunk for fetching the advanced settings */ export function advancedSettingsFetch( siteId ) { - return ( dispatch ) => { + return ( dispatch, getState ) => { + if ( siteId === null || typeof siteId === 'undefined' ) { + return; + } + + if ( getState().siteSettings.exporter.fetchingAdvancedSettings[ siteId ] === true ) { + return; + } + dispatch( { type: EXPORT_ADVANCED_SETTINGS_FETCH, siteId @@ -50,7 +58,7 @@ export function advancedSettingsFetch( siteId ) { return wpcom.undocumented() .getExportSettings( siteId ) .then( updateExportSettings ) - .catch( fetchFail ) + .catch( fetchFail ); } } diff --git a/client/state/site-settings/exporter/reducers.js b/client/state/site-settings/exporter/reducers.js index 1fd9a46832ee6..2785f813f4bdc 100644 --- a/client/state/site-settings/exporter/reducers.js +++ b/client/state/site-settings/exporter/reducers.js @@ -7,6 +7,8 @@ import { combineReducers } from 'redux'; * Internal dependencies */ import { + EXPORT_ADVANCED_SETTINGS_FAIL, + EXPORT_ADVANCED_SETTINGS_FETCH, EXPORT_ADVANCED_SETTINGS_RECEIVE, SET_EXPORT_POST_TYPE, REQUEST_START_EXPORT, @@ -48,6 +50,31 @@ export function exportingState( state = States.READY, action ) { return state; } +/** + * Tracks whether the advanced settings for a site are currently being fetched + * @param {Object} state Current global state tree + * @param {Object} action Action payload + * @return {Object} Updated state + */ +export function fetchingAdvancedSettings( state = {}, action ) { + switch ( action.type ) { + case EXPORT_ADVANCED_SETTINGS_FETCH: + return Object.assign( {}, state, { + [ action.siteId ]: true + } ); + case EXPORT_ADVANCED_SETTINGS_FAIL: + case EXPORT_ADVANCED_SETTINGS_RECEIVE: + return Object.assign( {}, state, { + [ action.siteId ]: false + } ); + case SERIALIZE: + return {}; + case DESERIALIZE: + return {}; + } + return state; +} + /** * Tracks available advanced settings for sites. * @param {Object} state Current global state tree @@ -72,5 +99,6 @@ export function advancedSettings( state = {}, action ) { export default combineReducers( { selectedPostType, exportingState, + fetchingAdvancedSettings, advancedSettings } ); diff --git a/client/state/site-settings/exporter/selectors.js b/client/state/site-settings/exporter/selectors.js index 7b9d0a47a1288..5a5e85d215703 100644 --- a/client/state/site-settings/exporter/selectors.js +++ b/client/state/site-settings/exporter/selectors.js @@ -17,3 +17,4 @@ export const shouldShowProgress = ( state ) => { export const getSelectedPostType = ( state ) => state.siteSettings.exporter.selectedPostType; export const getExportingState = ( state ) => state.siteSettings.exporter.exportingState; +export const advancedSettings = ( state, siteId ) => state.siteSettings.exporter.advancedSettings[ siteId ]; diff --git a/client/state/site-settings/exporter/test/actions.js b/client/state/site-settings/exporter/test/actions.js index 8f150c28c34df..5cbbbe06a310a 100644 --- a/client/state/site-settings/exporter/test/actions.js +++ b/client/state/site-settings/exporter/test/actions.js @@ -28,6 +28,11 @@ Chai.use( sinonChai ); describe( '#advancedSettingsFetch()', () => { const spy = sinon.spy(); + const getState = () => ( { + siteSettings: { exporter: { + fetchingAdvancedSettings: {} + } } + } ); before( () => { nock( 'https://public-api.wordpress.com:443' ) @@ -45,7 +50,7 @@ describe( '#advancedSettingsFetch()', () => { } ); it( 'should dispatch fetch action when thunk triggered', () => { - advancedSettingsFetch( 100658273 )( spy ); + advancedSettingsFetch( 100658273 )( spy, getState ); expect( spy ).to.have.been.calledWith( { type: EXPORT_ADVANCED_SETTINGS_FETCH, @@ -54,7 +59,7 @@ describe( '#advancedSettingsFetch()', () => { } ); it( 'should dispatch receive action when request completes', ( done ) => { - advancedSettingsFetch( 100658273 )( spy ).then( () => { + advancedSettingsFetch( 100658273 )( spy, getState ).then( () => { expect( spy ).to.have.been.calledWithMatch( { type: EXPORT_ADVANCED_SETTINGS_RECEIVE, siteId: 100658273, @@ -66,7 +71,7 @@ describe( '#advancedSettingsFetch()', () => { } ); it( 'should dispatch fail action when request fails', ( done ) => { - advancedSettingsFetch( 0 )( spy ).then( () => { + advancedSettingsFetch( 0 )( spy, getState ).then( () => { expect( spy ).to.have.been.calledWithMatch( { type: EXPORT_ADVANCED_SETTINGS_FETCH_FAIL, siteId: 0 diff --git a/client/state/site-settings/exporter/test/reducer.js b/client/state/site-settings/exporter/test/reducer.js index deefc7c5ee0a2..2168425e7759b 100644 --- a/client/state/site-settings/exporter/test/reducer.js +++ b/client/state/site-settings/exporter/test/reducer.js @@ -7,6 +7,8 @@ import { expect } from 'chai'; * Internal dependencies */ import { + EXPORT_ADVANCED_SETTINGS_FAIL, + EXPORT_ADVANCED_SETTINGS_FETCH, EXPORT_ADVANCED_SETTINGS_RECEIVE, DESERIALIZE, SERIALIZE @@ -14,7 +16,8 @@ import { import { selectedPostType, exportingState, - advancedSettings + advancedSettings, + fetchingAdvancedSettings } from '../reducers'; import { SAMPLE_ADVANCED_SETTINGS, @@ -47,6 +50,57 @@ describe( 'reducer', () => { } ); } ); + describe( '#fetchingAdvancedSettings()', () => { + it( 'should not persist state', () => { + const state = fetchingAdvancedSettings( { 100658273: true }, { type: SERIALIZE } ); + expect( state ).to.eql( {} ); + } ); + + it( 'should not load persisted state', () => { + const state = fetchingAdvancedSettings( { 100658273: true }, { type: DESERIALIZE } ); + expect( state ).to.eql( {} ); + } ); + + it( 'should index fetching status by site ID', () => { + const state = fetchingAdvancedSettings( null, { + type: EXPORT_ADVANCED_SETTINGS_FETCH, + siteId: 100658273 + } ); + expect( state ).to.eql( { 100658273: true } ); + } ); + + it( 'should reset fetching status after receive', () => { + const state = fetchingAdvancedSettings( null, { + type: EXPORT_ADVANCED_SETTINGS_RECEIVE, + siteId: 100658273, + advancedSettings: {} + } ); + expect( state ).to.eql( { 100658273: false } ); + } ); + + it( 'should reset fetching status after fail', () => { + const state = fetchingAdvancedSettings( null, { + type: EXPORT_ADVANCED_SETTINGS_FAIL, + siteId: 100658273, + advancedSettings: {} + } ); + expect( state ).to.eql( { 100658273: false } ); + } ); + + it( 'should not replace fetching status with other site', () => { + const state = fetchingAdvancedSettings( { + 100658273: true + }, { + type: EXPORT_ADVANCED_SETTINGS_FETCH, + siteId: 12345 + } ); + expect( state ).to.eql( { + 100658273: true, + 12345: true + } ); + } ); + } ); + describe( '#advancedSettings()', () => { it( 'should persist state', () => { const settings = { 100658273: SAMPLE_ADVANCED_SETTINGS };