Skip to content

Commit

Permalink
Export: Automatically fetch advanced settings when switching to exporter
Browse files Browse the repository at this point in the history
Squashed:
Export: Show placeholders while loading options
Export: Make existing tests pass
Export: Add unit tests for fetchingAdvancedSettings reducer
Export: Remove redundant serialize/deserialize
  • Loading branch information
jordwest committed Feb 9, 2016
1 parent fed9774 commit ad876ca
Show file tree
Hide file tree
Showing 10 changed files with 171 additions and 22 deletions.
20 changes: 17 additions & 3 deletions client/my-sites/exporter/advanced-settings.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,24 @@
* External dependencies
*/
import React, { PropTypes } from 'react';
import { connect } from 'react-redux';

/**
* Internal dependencies
*/
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
*
* Allows the user to select whether Pages, Posts and Feedback are
* exported. Posts and Pages can also be filtered by Authors, Statuses,
* and Date.
*/
export default React.createClass( {
const AdvancedSettings = React.createClass( {
displayName: 'AdvancedSettings',

propTypes: {
Expand Down Expand Up @@ -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 (
Expand All @@ -81,7 +85,7 @@ export default React.createClass( {
</div>
<SpinnerButton
className="exporter__export-button"
disabled={ !this.props.postType }
disabled={ ! this.props.postType }
loading={ this.props.shouldShowProgress }
isPrimary={ true }
onClick={ this.props.onClickExport }
Expand All @@ -91,3 +95,13 @@ export default React.createClass( {
);
}
} );

const mapStateToProps = ( state ) => {
const siteId = state.ui.selectedSiteId;
return {
siteId: siteId,
shouldShowPlaceholders: ! advancedSettings( state, siteId )
};
};

export default connect( mapStateToProps )( AdvancedSettings );
14 changes: 13 additions & 1 deletion client/my-sites/exporter/exporter.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
13 changes: 9 additions & 4 deletions client/my-sites/exporter/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 )
};
Expand All @@ -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 )
};
}

Expand Down
27 changes: 19 additions & 8 deletions client/my-sites/exporter/option-fieldset.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -37,6 +38,20 @@ module.exports = React.createClass( {
},

render() {
const placeholderMap = ( menu, menuIndex ) => (
<div key={ menuIndex } className="exporter__placeholder-text">
{ this.translate( 'Loading options…' ) }
</div>
)

const selectMap = ( menu, menuIndex ) => (
<Select key={ menuIndex } disabled={ ! this.props.isEnabled }>
{ menu.options.map( ( option, optionIndex ) => (
<option value={ optionIndex } key={ optionIndex }>{ option }</option>
) ) }
</Select>
)

return (
<div className="exporter__option-fieldset">

Expand All @@ -54,13 +69,9 @@ module.exports = React.createClass( {
}

<div className="exporter__option-fieldset-fields">
{ this.props.menus.map( ( menu, menuIndex ) => (
<Select key={ menuIndex } disabled={ !this.props.isEnabled }>
{ menu.options.map( ( option, optionIndex ) => (
<option value={ optionIndex } key={ optionIndex }>{ option }</option>
) ) }
</Select>
) ) }
{ this.props.menus.map(
this.props.shouldShowPlaceholders ? placeholderMap : selectMap
) }
</div>

</div>
Expand Down
11 changes: 11 additions & 0 deletions client/my-sites/exporter/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
12 changes: 10 additions & 2 deletions client/state/site-settings/exporter/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -50,7 +58,7 @@ export function advancedSettingsFetch( siteId ) {
return wpcom.undocumented()
.getExportSettings( siteId )
.then( updateExportSettings )
.catch( fetchFail )
.catch( fetchFail );
}
}

Expand Down
28 changes: 28 additions & 0 deletions client/state/site-settings/exporter/reducers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -72,5 +99,6 @@ export function advancedSettings( state = {}, action ) {
export default combineReducers( {
selectedPostType,
exportingState,
fetchingAdvancedSettings,
advancedSettings
} );
1 change: 1 addition & 0 deletions client/state/site-settings/exporter/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ];
11 changes: 8 additions & 3 deletions client/state/site-settings/exporter/test/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' )
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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
Expand Down
56 changes: 55 additions & 1 deletion client/state/site-settings/exporter/test/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,17 @@ import { expect } from 'chai';
* Internal dependencies
*/
import {
EXPORT_ADVANCED_SETTINGS_FAIL,
EXPORT_ADVANCED_SETTINGS_FETCH,
EXPORT_ADVANCED_SETTINGS_RECEIVE,
DESERIALIZE,
SERIALIZE
} from 'state/action-types';
import {
selectedPostType,
exportingState,
advancedSettings
advancedSettings,
fetchingAdvancedSettings
} from '../reducers';
import {
SAMPLE_ADVANCED_SETTINGS,
Expand Down Expand Up @@ -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 };
Expand Down

0 comments on commit ad876ca

Please sign in to comment.