From 688510afa831af2d33eb40058a9c983cf1a01d0d Mon Sep 17 00:00:00 2001 From: Dennis Snell Date: Wed, 2 May 2018 19:05:53 +0200 Subject: [PATCH 1/7] State: Use getHttpData() for geolocation data In #22549 we added the `getHttpData()` abstraction for simple network data needs. In this patch we're moving the `geo` state subtree over to this new abstraction and removing the old code it previously required. This approach is build on the data-layer ideas and extends it to store data outside of Redux. --- client/components/data/query-geo/README.md | 18 --- client/components/data/query-geo/index.jsx | 36 ------ client/components/language-picker/index.jsx | 10 +- client/lib/abtest/README.md | 4 +- .../my-sites/checkout/checkout/checkout.jsx | 2 - client/state/data-layer/http-data/getters.js | 38 ++++++ client/state/data-layer/http-data/index.js | 6 +- client/state/geo/actions.js | 57 --------- client/state/geo/reducer.js | 49 ------- client/state/geo/schema.js | 12 -- client/state/geo/selectors.js | 47 ------- client/state/geo/test/actions.js | 116 ----------------- client/state/geo/test/reducer.js | 121 ------------------ client/state/geo/test/selectors.js | 109 ---------------- client/state/index.js | 2 - .../get-current-user-payment-methods.js | 4 +- 16 files changed, 50 insertions(+), 581 deletions(-) delete mode 100644 client/components/data/query-geo/README.md delete mode 100644 client/components/data/query-geo/index.jsx create mode 100644 client/state/data-layer/http-data/getters.js delete mode 100644 client/state/geo/actions.js delete mode 100644 client/state/geo/reducer.js delete mode 100644 client/state/geo/schema.js delete mode 100644 client/state/geo/selectors.js delete mode 100644 client/state/geo/test/actions.js delete mode 100644 client/state/geo/test/reducer.js delete mode 100644 client/state/geo/test/selectors.js diff --git a/client/components/data/query-geo/README.md b/client/components/data/query-geo/README.md deleted file mode 100644 index cb2418932506e7..00000000000000 --- a/client/components/data/query-geo/README.md +++ /dev/null @@ -1,18 +0,0 @@ -Query Sites -=========== - -`` is a React component used in managing network requests for browser IP geolocation. - -## Usage - -Render the component. The component does not accept any children, nor does it render any of its own. - -```jsx -function MyComponent() { - return -} -``` - -## Props - -This component does not accept any props. diff --git a/client/components/data/query-geo/index.jsx b/client/components/data/query-geo/index.jsx deleted file mode 100644 index 5f243262c129d1..00000000000000 --- a/client/components/data/query-geo/index.jsx +++ /dev/null @@ -1,36 +0,0 @@ -/** @format */ - -/** - * External dependencies - */ - -import PropTypes from 'prop-types'; -import { Component } from 'react'; -import { connect } from 'react-redux'; - -/** - * Internal dependencies - */ -import { isRequestingGeo } from 'state/geo/selectors'; -import { requestGeo } from 'state/geo/actions'; - -class QueryGeo extends Component { - componentWillMount() { - if ( ! this.props.requesting ) { - this.props.requestGeo(); - } - } - - render() { - return null; - } -} - -QueryGeo.propTypes = { - requesting: PropTypes.bool, - requestGeo: PropTypes.func, -}; - -export default connect( state => ( { requesting: isRequestingGeo( state ) } ), { requestGeo } )( - QueryGeo -); diff --git a/client/components/language-picker/index.jsx b/client/components/language-picker/index.jsx index 4cd0b1e275463b..cf7f30957f002d 100644 --- a/client/components/language-picker/index.jsx +++ b/client/components/language-picker/index.jsx @@ -14,10 +14,9 @@ import { find, isString, noop } from 'lodash'; /** * Internal dependencies */ -import { getGeoCountryShort } from 'state/geo/selectors'; -import QueryGeo from 'components/data/query-geo'; import LanguagePickerModal from './modal'; import QueryLanguageNames from 'components/data/query-language-names'; +import { requestGeoLocation } from 'state/data-layer/http-data/getters'; import { getLanguageCodeLabels } from './utils'; export class LanguagePicker extends PureComponent { @@ -174,13 +173,12 @@ export class LanguagePicker extends PureComponent { { this.renderModal( language.langSlug ) } - ); } } -export default connect( state => ( { - countryCode: getGeoCountryShort( state ), -} ) )( localize( LanguagePicker ) ); +export default connect( () => ( { countryCode: requestGeoLocation().data } ) )( + localize( LanguagePicker ) +); diff --git a/client/lib/abtest/README.md b/client/lib/abtest/README.md index b00c8ee6b69320..d120d69d7180d4 100644 --- a/client/lib/abtest/README.md +++ b/client/lib/abtest/README.md @@ -68,7 +68,7 @@ Also, the user's variation is saved in local storage. You can see this in Chrome Here's another example with country targeting: ```jsx -const userCountryCode = getGeoCountryShort( state ); +const userCountryCode = requestGeoLocation().data; let buttonWording; if ( abtest( 'freeTrialButtonWordingForIndia', userCountryCode ) === 'startFreeTrial' ) { @@ -161,7 +161,7 @@ You would need to update [config/default.json](https://github.com/Automattic/wp- "knownABTestKeys": [ "freeTrialButtonWording" ] - + "overrideABTests": [ [ "freeTrialButtonWording_201502160", "startFreeTrial" ] ] diff --git a/client/my-sites/checkout/checkout/checkout.jsx b/client/my-sites/checkout/checkout/checkout.jsx index 2b2c01a4c2b09f..665853d0d0f053 100644 --- a/client/my-sites/checkout/checkout/checkout.jsx +++ b/client/my-sites/checkout/checkout/checkout.jsx @@ -32,7 +32,6 @@ import { managePurchase } from 'me/purchases/paths'; import SubscriptionLengthPicker from 'blocks/subscription-length-picker'; import QueryContactDetailsCache from 'components/data/query-contact-details-cache'; import QueryStoredCards from 'components/data/query-stored-cards'; -import QueryGeo from 'components/data/query-geo'; import QuerySitePlans from 'components/data/query-site-plans'; import QueryPlans from 'components/data/query-plans'; import SecurePaymentForm from './secure-payment-form'; @@ -610,7 +609,6 @@ export class Checkout extends React.Component { - { this.content() } diff --git a/client/state/data-layer/http-data/getters.js b/client/state/data-layer/http-data/getters.js new file mode 100644 index 00000000000000..6b299505d237a2 --- /dev/null +++ b/client/state/data-layer/http-data/getters.js @@ -0,0 +1,38 @@ +/** @format */ +/** + * Internal dependencies + */ +import makeJsonSchemaParser from 'lib/make-json-schema-parser'; +import { http as rawHttp } from 'state/http/actions'; +import { requestHttpData } from 'state/data-layer/http-data/common'; + +export const requestGeoLocation = () => + requestHttpData( + 'geo', + rawHttp( { + method: 'GET', + url: 'https://public-api.wordpress.com/geo/', + } ), + { + fromApi: makeJsonSchemaParser( + { + type: 'object', + properties: { + body: { + type: [ 'object', 'null' ], + properties: { + latitude: { type: 'string' }, + longitude: { type: 'string' }, + country_short: { type: 'string' }, + country_long: { type: 'string' }, + region: { type: 'string' }, + city: { type: 'string' }, + }, + }, + }, + }, + // we only use the short code currently + ( { body: { country_short } } ) => [ [ 'geo', country_short ] ] + ), + } + ); diff --git a/client/state/data-layer/http-data/index.js b/client/state/data-layer/http-data/index.js index d9a0de381b8d99..22dade749603d9 100644 --- a/client/state/data-layer/http-data/index.js +++ b/client/state/data-layer/http-data/index.js @@ -105,8 +105,10 @@ const parseResponse = ( data, fromApi ) => { }; const onSuccess = ( action, apiData ) => { - const fromApi = 'function' === typeof action.fromApi && action.fromApi(); - const [ error, data ] = fromApi ? parseResponse( apiData, fromApi ) : [ undefined, [] ]; + const [ error, data ] = + 'function' === typeof action.fromApi + ? parseResponse( apiData, action.fromApi ) + : [ undefined, [] ]; if ( undefined !== error ) { return onError( action, error ); diff --git a/client/state/geo/actions.js b/client/state/geo/actions.js deleted file mode 100644 index ea141ed27478bd..00000000000000 --- a/client/state/geo/actions.js +++ /dev/null @@ -1,57 +0,0 @@ -/** @format */ - -/** - * Internal dependencies - */ - -import request from 'superagent'; -import { - GEO_RECEIVE, - GEO_REQUEST, - GEO_REQUEST_FAILURE, - GEO_REQUEST_SUCCESS, -} from 'state/action-types'; - -/** - * Constants - */ -export const GEO_ENDPOINT = 'https://public-api.wordpress.com/geo/'; - -/** - * Returns an action object used in signalling that the current browser IP - * geolocation has been received. - * - * @param {Object} geo Geolocation data - * @return {Object} Action object - */ -export function receiveGeo( geo ) { - return { - type: GEO_RECEIVE, - geo, - }; -} - -/** - * Returns a function which, when invoked, triggers a network request to fetch - * browser IP geolocation. - * - * @return {Function} Action thunk - */ -export function requestGeo() { - return dispatch => { - dispatch( { type: GEO_REQUEST } ); - - return request - .get( GEO_ENDPOINT ) - .then( ( { body: geo } ) => { - dispatch( { type: GEO_REQUEST_SUCCESS } ); - dispatch( receiveGeo( geo ) ); - } ) - .catch( error => { - dispatch( { - type: GEO_REQUEST_FAILURE, - error, - } ); - } ); - }; -} diff --git a/client/state/geo/reducer.js b/client/state/geo/reducer.js deleted file mode 100644 index 4e7b9e5ac41f90..00000000000000 --- a/client/state/geo/reducer.js +++ /dev/null @@ -1,49 +0,0 @@ -/** @format */ - -/** - * Internal dependencies - */ - -import { - GEO_RECEIVE, - GEO_REQUEST, - GEO_REQUEST_FAILURE, - GEO_REQUEST_SUCCESS, -} from 'state/action-types'; -import { combineReducers, createReducer } from 'state/utils'; -import { geoSchema } from './schema'; - -/** - * Returns the updated requesting state after an action has been dispatched. - * Requesting state tracks whether a geolocation request is in progress. - * - * @param {Object} state Current state - * @param {Object} action Action object - * @return {Object} Updated state - */ -export const requesting = createReducer( false, { - [ GEO_REQUEST ]: () => true, - [ GEO_REQUEST_FAILURE ]: () => false, - [ GEO_REQUEST_SUCCESS ]: () => false, -} ); - -/** - * Returns the updated requesting state after an action has been dispatched. - * Requesting state tracks current browser IP geolocation. - * - * @param {?Object} state Current state - * @param {Object} action Action object - * @return {?Object} Updated state - */ -export const geo = createReducer( - null, - { - [ GEO_RECEIVE ]: ( state, action ) => action.geo, - }, - geoSchema -); - -export default combineReducers( { - requesting, - geo, -} ); diff --git a/client/state/geo/schema.js b/client/state/geo/schema.js deleted file mode 100644 index 62299916d68d67..00000000000000 --- a/client/state/geo/schema.js +++ /dev/null @@ -1,12 +0,0 @@ -/** @format */ -export const geoSchema = { - type: [ 'object', 'null' ], - properties: { - latitude: { type: 'string' }, - longitude: { type: 'string' }, - country_short: { type: 'string' }, - country_long: { type: 'string' }, - region: { type: 'string' }, - city: { type: 'string' }, - }, -}; diff --git a/client/state/geo/selectors.js b/client/state/geo/selectors.js deleted file mode 100644 index 3dd7f151b703c0..00000000000000 --- a/client/state/geo/selectors.js +++ /dev/null @@ -1,47 +0,0 @@ -/** @format */ - -/** - * External dependencies - */ - -import { get } from 'lodash'; - -/** - * Returns whether a browser IP geolocation request is in progress. - * - * @param {Object} state Global state tree - * @return {Boolean} Whether request is in progress - */ -export function isRequestingGeo( state ) { - return state.geo.requesting; -} - -/** - * Returns the current browser IP geolocation data. - * - * @param {Object} state Global state tree - * @return {?Object} Current browser IP geolocation data - */ -export function getGeo( state ) { - return state.geo.geo; -} - -/** - * Returns the current browser IP geolocation full country name. - * - * @param {Object} state Global state tree - * @return {?String} Current browser IP geolocation data - */ -export function getGeoCountry( state ) { - return get( getGeo( state ), 'country_long', null ); -} - -/** - * Returns the current browser IP geolocation abbreviated country name. - * - * @param {Object} state Global state tree - * @return {?String} Current browser IP geolocation short country name - */ -export function getGeoCountryShort( state ) { - return get( getGeo( state ), 'country_short', null ); -} diff --git a/client/state/geo/test/actions.js b/client/state/geo/test/actions.js deleted file mode 100644 index d47d74c63f8890..00000000000000 --- a/client/state/geo/test/actions.js +++ /dev/null @@ -1,116 +0,0 @@ -/** @format */ -/** - * External dependencies - */ -import { expect } from 'chai'; -import { match } from 'sinon'; - -/** - * Internal dependencies - */ -import { receiveGeo, requestGeo } from '../actions'; -import { - GEO_RECEIVE, - GEO_REQUEST, - GEO_REQUEST_SUCCESS, - GEO_REQUEST_FAILURE, -} from 'state/action-types'; -import useNock from 'test/helpers/use-nock'; -import { useSandbox } from 'test/helpers/use-sinon'; - -describe( 'actions', () => { - let spy; - useSandbox( sandbox => ( spy = sandbox.spy() ) ); - - describe( 'receiveGeo()', () => { - test( 'should return an action object', () => { - const action = receiveGeo( { - latitude: '39.36006', - longitude: '-84.30994', - country_short: 'US', - country_long: 'United States', - region: 'Ohio', - city: 'Mason', - } ); - - expect( action ).to.eql( { - type: GEO_RECEIVE, - geo: { - latitude: '39.36006', - longitude: '-84.30994', - country_short: 'US', - country_long: 'United States', - region: 'Ohio', - city: 'Mason', - }, - } ); - } ); - } ); - - describe( 'requestGeo()', () => { - test( 'should dispatch fetch action when thunk triggered', () => { - requestGeo()( spy ); - - expect( spy ).to.have.been.calledWith( { - type: GEO_REQUEST, - } ); - } ); - - describe( 'success', () => { - useNock( nock => { - nock( 'https://public-api.wordpress.com:443' ) - .persist() - .get( '/geo/' ) - .reply( 200, { - latitude: '39.36006', - longitude: '-84.30994', - country_short: 'US', - country_long: 'United States', - region: 'Ohio', - city: 'Mason', - } ); - } ); - - test( 'should dispatch receive action when request completes', () => { - return requestGeo()( spy ).then( () => { - expect( spy ).to.have.been.calledWith( - receiveGeo( { - latitude: '39.36006', - longitude: '-84.30994', - country_short: 'US', - country_long: 'United States', - region: 'Ohio', - city: 'Mason', - } ) - ); - } ); - } ); - - test( 'should dispatch request success action when request completes', () => { - return requestGeo()( spy ).then( () => { - expect( spy ).to.have.been.calledWith( { - type: GEO_REQUEST_SUCCESS, - } ); - } ); - } ); - } ); - - describe( 'failure', () => { - useNock( nock => { - nock( 'https://public-api.wordpress.com:443' ) - .persist() - .get( '/geo/' ) - .reply( 500 ); - } ); - - test( 'should dispatch fail action when request fails', () => { - return requestGeo()( spy ).then( () => { - expect( spy ).to.have.been.calledWith( { - type: GEO_REQUEST_FAILURE, - error: match( { message: 'Internal Server Error' } ), - } ); - } ); - } ); - } ); - } ); -} ); diff --git a/client/state/geo/test/reducer.js b/client/state/geo/test/reducer.js deleted file mode 100644 index 512cdfe47a95a7..00000000000000 --- a/client/state/geo/test/reducer.js +++ /dev/null @@ -1,121 +0,0 @@ -/** @format */ - -/** - * External dependencies - */ -import { expect } from 'chai'; -import deepFreeze from 'deep-freeze'; - -/** - * Internal dependencies - */ -import reducer, { requesting, geo } from '../reducer'; -import { - GEO_RECEIVE, - GEO_REQUEST, - GEO_REQUEST_FAILURE, - GEO_REQUEST_SUCCESS, - DESERIALIZE, -} from 'state/action-types'; -import { useSandbox } from 'test/helpers/use-sinon'; - -describe( 'reducer', () => { - useSandbox( sandbox => sandbox.stub( console, 'warn' ) ); - - test( 'should include expected keys in return value', () => { - expect( reducer( undefined, {} ) ).to.have.keys( [ 'requesting', 'geo' ] ); - } ); - - describe( 'requesting()', () => { - test( 'should default to false', () => { - const state = requesting( undefined, {} ); - - expect( state ).to.be.false; - } ); - - test( 'should set site ID to true value if request in progress', () => { - const state = requesting( undefined, { - type: GEO_REQUEST, - } ); - - expect( state ).to.eql( true ); - } ); - - test( 'should set site ID to false if request succeeds', () => { - const state = requesting( true, { - type: GEO_REQUEST_SUCCESS, - } ); - - expect( state ).to.eql( false ); - } ); - - test( 'should set site ID to false if request fails', () => { - const state = requesting( true, { - type: GEO_REQUEST_FAILURE, - } ); - - expect( state ).to.eql( false ); - } ); - } ); - - describe( 'geo()', () => { - test( 'should default to null', () => { - const state = geo( undefined, {} ); - - expect( state ).to.be.null; - } ); - - test( 'should track received geo data', () => { - const state = geo( undefined, { - type: GEO_RECEIVE, - geo: { - latitude: '39.36006', - longitude: '-84.30994', - country_short: 'US', - country_long: 'United States', - region: 'Ohio', - city: 'Mason', - }, - } ); - - expect( state ).to.eql( { - latitude: '39.36006', - longitude: '-84.30994', - country_short: 'US', - country_long: 'United States', - region: 'Ohio', - city: 'Mason', - } ); - } ); - - test( 'should load valid persisted state', () => { - const original = deepFreeze( { - latitude: '39.36006', - longitude: '-84.30994', - country_short: 'US', - country_long: 'United States', - region: 'Ohio', - city: 'Mason', - } ); - const state = geo( original, { type: DESERIALIZE } ); - - expect( state ).to.eql( { - latitude: '39.36006', - longitude: '-84.30994', - country_short: 'US', - country_long: 'United States', - region: 'Ohio', - city: 'Mason', - } ); - } ); - - test( 'should not load invalid persisted state', () => { - const original = deepFreeze( { - country_short: true, - } ); - const state = geo( original, { type: DESERIALIZE } ); - - expect( state ).to.be.null; - } ); - } ); -} ); diff --git a/client/state/geo/test/selectors.js b/client/state/geo/test/selectors.js deleted file mode 100644 index 7e46f681d53299..00000000000000 --- a/client/state/geo/test/selectors.js +++ /dev/null @@ -1,109 +0,0 @@ -/** @format */ - -/** - * External dependencies - */ -import { expect } from 'chai'; - -/** - * Internal dependencies - */ -import { isRequestingGeo, getGeo, getGeoCountry, getGeoCountryShort } from '../selectors'; - -describe( 'selectors', () => { - describe( 'isRequestingGeo()', () => { - test( 'should return requesting state', () => { - const isRequesting = isRequestingGeo( { - geo: { - requesting: true, - }, - } ); - - expect( isRequesting ).to.be.true; - } ); - } ); - - describe( 'getGeo()', () => { - test( 'should return geo data state', () => { - const geo = getGeo( { - geo: { - geo: { - latitude: '39.36006', - longitude: '-84.30994', - country_short: 'US', - country_long: 'United States', - region: 'Ohio', - city: 'Mason', - }, - }, - } ); - - expect( geo ).to.eql( { - latitude: '39.36006', - longitude: '-84.30994', - country_short: 'US', - country_long: 'United States', - region: 'Ohio', - city: 'Mason', - } ); - } ); - } ); - - describe( 'getGeoCountry()', () => { - test( 'should return null if no geo data state', () => { - const country = getGeoCountry( { - geo: { - geo: null, - }, - } ); - - expect( country ).to.be.null; - } ); - - test( 'should return full country name of geo data state', () => { - const country = getGeoCountry( { - geo: { - geo: { - latitude: '39.36006', - longitude: '-84.30994', - country_short: 'US', - country_long: 'United States', - region: 'Ohio', - city: 'Mason', - }, - }, - } ); - - expect( country ).to.equal( 'United States' ); - } ); - } ); - - describe( 'getGeoCountryShort()', () => { - test( 'should return null if no geo data state', () => { - const country = getGeoCountryShort( { - geo: { - geo: null, - }, - } ); - - expect( country ).to.be.null; - } ); - - test( 'should return abbreviated country name of geo data state', () => { - const country = getGeoCountryShort( { - geo: { - geo: { - latitude: '39.36006', - longitude: '-84.30994', - country_short: 'US', - country_long: 'United States', - region: 'Ohio', - city: 'Mason', - }, - }, - } ); - - expect( country ).to.equal( 'US' ); - } ); - } ); -} ); diff --git a/client/state/index.js b/client/state/index.js index dea22121fbee21..64147c19126115 100644 --- a/client/state/index.js +++ b/client/state/index.js @@ -40,7 +40,6 @@ import currentUser from './current-user/reducer'; import { reducer as dataRequests } from './data-layer/wpcom-http/utils'; import documentHead from './document-head/reducer'; import domains from './domains/reducer'; -import geo from './geo/reducer'; import googleAppsUsers from './google-apps-users/reducer'; import googleMyBusiness from './google-my-business/reducer'; import help from './help/reducer'; @@ -131,7 +130,6 @@ const reducers = { domains, extensions, form, - geo, googleAppsUsers, googleMyBusiness, happinessEngineers, diff --git a/client/state/selectors/get-current-user-payment-methods.js b/client/state/selectors/get-current-user-payment-methods.js index e554ce5880ab53..21293ca47111c6 100644 --- a/client/state/selectors/get-current-user-payment-methods.js +++ b/client/state/selectors/get-current-user-payment-methods.js @@ -10,7 +10,7 @@ import { lowerCase, upperCase } from 'lodash'; * Internal dependencies */ import { getCurrentUserLocale } from 'state/current-user/selectors'; -import { getGeoCountryShort } from 'state/geo/selectors'; +import { requestGeoLocation } from 'state/data-layer/http-data/getters'; /** * Constants @@ -44,7 +44,7 @@ const paymentMethods = { * @return {Array} Preferred payment methods */ export default function getCurrentUserPaymentMethods( state ) { - const countryCode = getGeoCountryShort( state ); + const countryCode = requestGeoLocation().data; const wpcomLang = getCurrentUserLocale( state ); const generatedLocale = lowerCase( wpcomLang ) + '-' + upperCase( countryCode ); From 46d2e84bc73ccf26331b96e4aa3fb4f3ca6db745 Mon Sep 17 00:00:00 2001 From: Dennis Snell Date: Thu, 3 May 2018 18:00:00 +0200 Subject: [PATCH 2/7] Make `fromApi` lazy and skip parser creation --- client/state/data-layer/http-data/getters.js | 26 ++------------------ 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/client/state/data-layer/http-data/getters.js b/client/state/data-layer/http-data/getters.js index 6b299505d237a2..5166ae9b7a5fe0 100644 --- a/client/state/data-layer/http-data/getters.js +++ b/client/state/data-layer/http-data/getters.js @@ -2,9 +2,8 @@ /** * Internal dependencies */ -import makeJsonSchemaParser from 'lib/make-json-schema-parser'; import { http as rawHttp } from 'state/http/actions'; -import { requestHttpData } from 'state/data-layer/http-data/common'; +import { requestHttpData } from 'state/data-layer/http-data'; export const requestGeoLocation = () => requestHttpData( @@ -13,26 +12,5 @@ export const requestGeoLocation = () => method: 'GET', url: 'https://public-api.wordpress.com/geo/', } ), - { - fromApi: makeJsonSchemaParser( - { - type: 'object', - properties: { - body: { - type: [ 'object', 'null' ], - properties: { - latitude: { type: 'string' }, - longitude: { type: 'string' }, - country_short: { type: 'string' }, - country_long: { type: 'string' }, - region: { type: 'string' }, - city: { type: 'string' }, - }, - }, - }, - }, - // we only use the short code currently - ( { body: { country_short } } ) => [ [ 'geo', country_short ] ] - ), - } + { fromApi: ( { body: { country_short } } ) => [ [ 'geo', parseInt( country_short, 10 ) ] ] } ); From 9df67f64f220cdb124d27ffa3105112afb8b8aec Mon Sep 17 00:00:00 2001 From: Dennis Snell Date: Sun, 6 May 2018 18:09:09 +0200 Subject: [PATCH 3/7] Fix merge conflict --- client/state/data-layer/http-data/index.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/client/state/data-layer/http-data/index.js b/client/state/data-layer/http-data/index.js index 22dade749603d9..d9a0de381b8d99 100644 --- a/client/state/data-layer/http-data/index.js +++ b/client/state/data-layer/http-data/index.js @@ -105,10 +105,8 @@ const parseResponse = ( data, fromApi ) => { }; const onSuccess = ( action, apiData ) => { - const [ error, data ] = - 'function' === typeof action.fromApi - ? parseResponse( apiData, action.fromApi ) - : [ undefined, [] ]; + const fromApi = 'function' === typeof action.fromApi && action.fromApi(); + const [ error, data ] = fromApi ? parseResponse( apiData, fromApi ) : [ undefined, [] ]; if ( undefined !== error ) { return onError( action, error ); From d78abea4c724163a88653f4f02e978e0a466940a Mon Sep 17 00:00:00 2001 From: Dennis Snell Date: Sun, 6 May 2018 18:19:50 +0200 Subject: [PATCH 4/7] Update getter for thunk `fromApi` --- client/state/data-layer/http-data/getters.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/client/state/data-layer/http-data/getters.js b/client/state/data-layer/http-data/getters.js index 5166ae9b7a5fe0..09ee22245cac56 100644 --- a/client/state/data-layer/http-data/getters.js +++ b/client/state/data-layer/http-data/getters.js @@ -12,5 +12,9 @@ export const requestGeoLocation = () => method: 'GET', url: 'https://public-api.wordpress.com/geo/', } ), - { fromApi: ( { body: { country_short } } ) => [ [ 'geo', parseInt( country_short, 10 ) ] ] } + { + fromApi: () => ( { body: { country_short } } ) => [ + [ 'geo', parseInt( country_short, 10 ) ], + ], + } ); From 4f3620978136715d776513a228915220009470d3 Mon Sep 17 00:00:00 2001 From: Dennis Snell Date: Sun, 6 May 2018 19:05:49 +0200 Subject: [PATCH 5/7] Update selector test code --- .../test/get-current-user-payment-methods.js | 57 +++++-------------- 1 file changed, 15 insertions(+), 42 deletions(-) diff --git a/client/state/selectors/test/get-current-user-payment-methods.js b/client/state/selectors/test/get-current-user-payment-methods.js index d2f984c7ca3cfa..5d91ec65ad41ba 100644 --- a/client/state/selectors/test/get-current-user-payment-methods.js +++ b/client/state/selectors/test/get-current-user-payment-methods.js @@ -8,16 +8,11 @@ import { expect } from 'chai'; /** * Internal dependencies */ +import { update } from 'state/data-layer/http-data'; import { getCurrentUserPaymentMethods } from 'state/selectors'; describe( 'getCurrentUserPaymentMethods()', () => { const enLangUsCountryState = { - geo: { - geo: { - country_short: 'US', - }, - }, - users: { items: { 73705554: { ID: 73705554, login: 'testonesite2014', localeSlug: 'en' }, @@ -30,12 +25,6 @@ describe( 'getCurrentUserPaymentMethods()', () => { }; const enLangDeCountryState = { - geo: { - geo: { - country_short: 'DE', - }, - }, - users: { items: { 73705554: { ID: 73705554, login: 'testonesite2014', localeSlug: 'en' }, @@ -48,12 +37,6 @@ describe( 'getCurrentUserPaymentMethods()', () => { }; const deLangDeCountryState = { - geo: { - geo: { - country_short: 'DE', - }, - }, - users: { items: { 73705554: { ID: 73705554, login: 'testonesite2014', localeSlug: 'de' }, @@ -66,12 +49,6 @@ describe( 'getCurrentUserPaymentMethods()', () => { }; const deLangAtCountryState = { - geo: { - geo: { - country_short: 'AT', - }, - }, - users: { items: { 73705554: { ID: 73705554, login: 'testonesite2014', localeSlug: 'de' }, @@ -84,12 +61,6 @@ describe( 'getCurrentUserPaymentMethods()', () => { }; const nlCountryState = { - geo: { - geo: { - country_short: 'NL', - }, - }, - users: { items: { 73705554: { ID: 73705554, login: 'testonesite2014', localeSlug: 'nl' }, @@ -102,12 +73,6 @@ describe( 'getCurrentUserPaymentMethods()', () => { }; const PlCountryState = { - geo: { - geo: { - country_short: 'PL', - }, - }, - users: { items: { 73705554: { ID: 73705554, login: 'testonesite2014', localeSlug: 'pl' }, @@ -120,12 +85,6 @@ describe( 'getCurrentUserPaymentMethods()', () => { }; const frLangFRCountryState = { - geo: { - geo: { - country_short: 'FR', - }, - }, - users: { items: { 73705554: { ID: 73705554, login: 'testonesite2014', localeSlug: 'fr' }, @@ -138,6 +97,8 @@ describe( 'getCurrentUserPaymentMethods()', () => { }; test( 'en-US should return credit card primary, PayPal secondary', () => { + update( 'geo', 'success', 'US' ); + expect( getCurrentUserPaymentMethods( enLangUsCountryState ) ).to.eql( [ 'credit-card', 'paypal', @@ -145,6 +106,8 @@ describe( 'getCurrentUserPaymentMethods()', () => { } ); test( 'en-DE should return CC, GiroPay, Paypal', () => { + update( 'geo', 'success', 'DE' ); + expect( getCurrentUserPaymentMethods( enLangDeCountryState ) ).to.eql( [ 'credit-card', 'giropay', @@ -153,6 +116,8 @@ describe( 'getCurrentUserPaymentMethods()', () => { } ); test( 'de-DE should return CC, Giropay, Paypal', () => { + update( 'geo', 'success', 'DE' ); + expect( getCurrentUserPaymentMethods( deLangDeCountryState ) ).to.eql( [ 'credit-card', 'giropay', @@ -161,6 +126,8 @@ describe( 'getCurrentUserPaymentMethods()', () => { } ); test( 'de-AT should return CC, EPS, Paypal', () => { + update( 'geo', 'success', 'AT' ); + expect( getCurrentUserPaymentMethods( deLangAtCountryState ) ).to.eql( [ 'credit-card', 'eps', @@ -169,6 +136,8 @@ describe( 'getCurrentUserPaymentMethods()', () => { } ); test( 'nl-NL should return credit card, iDEAL, PayPal ', () => { + update( 'geo', 'success', 'NL' ); + expect( getCurrentUserPaymentMethods( nlCountryState ) ).to.eql( [ 'credit-card', 'ideal', @@ -177,6 +146,8 @@ describe( 'getCurrentUserPaymentMethods()', () => { } ); test( 'pl-PL should return credit card, p24, PayPal ', () => { + update( 'geo', 'success', 'PL' ); + expect( getCurrentUserPaymentMethods( PlCountryState ) ).to.eql( [ 'credit-card', 'p24', @@ -185,6 +156,8 @@ describe( 'getCurrentUserPaymentMethods()', () => { } ); test( 'fr-FR should return credit card primary, PayPal secondary', () => { + update( 'geo', 'success', 'FR' ); + expect( getCurrentUserPaymentMethods( frLangFRCountryState ) ).to.eql( [ 'credit-card', 'paypal', From 16c766ccfb3c01cc23f4895869d17018b36c8a77 Mon Sep 17 00:00:00 2001 From: Dennis Snell Date: Tue, 8 May 2018 13:56:27 +0200 Subject: [PATCH 6/7] Rename and move files around --- client/components/language-picker/index.jsx | 2 +- .../{data-layer/http-data/getters.js => data-getters/index.js} | 0 client/state/data-layer/{http-data/index.js => http-data.js} | 0 client/state/selectors/get-current-user-payment-methods.js | 2 +- client/state/selectors/get-http-data.js | 2 +- 5 files changed, 3 insertions(+), 3 deletions(-) rename client/state/{data-layer/http-data/getters.js => data-getters/index.js} (100%) rename client/state/data-layer/{http-data/index.js => http-data.js} (100%) diff --git a/client/components/language-picker/index.jsx b/client/components/language-picker/index.jsx index cf7f30957f002d..022c04ebf7dfb2 100644 --- a/client/components/language-picker/index.jsx +++ b/client/components/language-picker/index.jsx @@ -16,7 +16,7 @@ import { find, isString, noop } from 'lodash'; */ import LanguagePickerModal from './modal'; import QueryLanguageNames from 'components/data/query-language-names'; -import { requestGeoLocation } from 'state/data-layer/http-data/getters'; +import { requestGeoLocation } from 'state/data-getters'; import { getLanguageCodeLabels } from './utils'; export class LanguagePicker extends PureComponent { diff --git a/client/state/data-layer/http-data/getters.js b/client/state/data-getters/index.js similarity index 100% rename from client/state/data-layer/http-data/getters.js rename to client/state/data-getters/index.js diff --git a/client/state/data-layer/http-data/index.js b/client/state/data-layer/http-data.js similarity index 100% rename from client/state/data-layer/http-data/index.js rename to client/state/data-layer/http-data.js diff --git a/client/state/selectors/get-current-user-payment-methods.js b/client/state/selectors/get-current-user-payment-methods.js index 21293ca47111c6..8a888a4e8b984d 100644 --- a/client/state/selectors/get-current-user-payment-methods.js +++ b/client/state/selectors/get-current-user-payment-methods.js @@ -10,7 +10,7 @@ import { lowerCase, upperCase } from 'lodash'; * Internal dependencies */ import { getCurrentUserLocale } from 'state/current-user/selectors'; -import { requestGeoLocation } from 'state/data-layer/http-data/getters'; +import { requestGeoLocation } from 'state/data-getters'; /** * Constants diff --git a/client/state/selectors/get-http-data.js b/client/state/selectors/get-http-data.js index e64f0dbdf9e6dd..35ffa2fd225b9c 100644 --- a/client/state/selectors/get-http-data.js +++ b/client/state/selectors/get-http-data.js @@ -1,3 +1,3 @@ /** @format */ -export { getHttpData as default } from 'state/data-layer/http-data/common'; +export { getHttpData as default } from 'state/data-layer/http-data'; From f1f547d8fc0abb19b701fc5a6db29163898f5208 Mon Sep 17 00:00:00 2001 From: Dennis Snell Date: Tue, 8 May 2018 18:24:19 +0200 Subject: [PATCH 7/7] Fix bug in getter --- client/state/data-getters/index.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/client/state/data-getters/index.js b/client/state/data-getters/index.js index 09ee22245cac56..569880f44ef9c7 100644 --- a/client/state/data-getters/index.js +++ b/client/state/data-getters/index.js @@ -12,9 +12,5 @@ export const requestGeoLocation = () => method: 'GET', url: 'https://public-api.wordpress.com/geo/', } ), - { - fromApi: () => ( { body: { country_short } } ) => [ - [ 'geo', parseInt( country_short, 10 ) ], - ], - } + { fromApi: () => ( { body: { country_short } } ) => [ [ 'geo', country_short ] ] } );