From 5f3382a483da078e5cca444709707aef9e801b4a Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Mon, 13 Mar 2017 15:12:23 -0700 Subject: [PATCH 1/4] fix(util): fix queryIsValid check also add test case for this function --- __tests__/util/state.js | 37 ++++++++++++++++++++++++++++++++++++- lib/util/state.js | 2 +- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/__tests__/util/state.js b/__tests__/util/state.js index 2e9387cca..954c150bd 100644 --- a/__tests__/util/state.js +++ b/__tests__/util/state.js @@ -1,10 +1,45 @@ /* globals describe, expect, it */ -import {getDefaultQuery} from '../../lib/util/state' +import {getDefaultQuery, queryIsValid} from '../../lib/util/state' describe('util > state', () => { it('getDefaultQuery should parse window hash if available', () => { window.location.hash = '#plan?arriveBy=false&date=2017-02-03&fromPlace=12,34&toPlace=34,12&time=12:34' expect(getDefaultQuery()).toMatchSnapshot() }) + + describe('queryIsValid', () => { + const fakeFromLocation = { + lat: 12, + lon: 34 + } + const fakeToLocation = { + lat: 34, + lon: 12 + } + const testCases = [{ + expected: false, + input: { + currentQuery: { + from: fakeFromLocation + } + }, + title: 'should not be valid with only from location' + }, { + expected: true, + input: { + currentQuery: { + from: fakeFromLocation, + to: fakeToLocation + } + }, + title: 'should be valid with from and to locations' + }] + + testCases.forEach((testCase) => { + it(testCase.title, () => { + expect(queryIsValid(testCase.input))[testCase.expected ? 'toBeTruthy' : 'toBeFalsy']() + }) + }) + }) }) diff --git a/lib/util/state.js b/lib/util/state.js index 469846435..88fb08e4f 100644 --- a/lib/util/state.js +++ b/lib/util/state.js @@ -56,7 +56,7 @@ export { hasValidLocation } function queryIsValid (otpState) { return hasValidLocation(otpState, 'from') && - hasValidLocation(otpState, 'from') + hasValidLocation(otpState, 'to') // TODO: add mode validation // TODO: add date/time validation } From 91fd5ee9bd405e8e0f99cd7e79303799513a1a12 Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Mon, 13 Mar 2017 16:59:10 -0700 Subject: [PATCH 2/4] feat(api): add another way of customizing otp query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This option allows setting the custom builder function in the config when creating the reducer’s initial state. When using all of the reducers together they make calls to the planTrip action, but there wasn’t really a way to pass in a custom function that way. For now it seems like the only way to do this could be to set something in the config of the state. This PR includes the work done in #12. --- __tests__/actions/__snapshots__/api.js.snap | 44 +++++++- __tests__/actions/api.js | 108 +++++++++++++------- lib/actions/api.js | 6 +- 3 files changed, 116 insertions(+), 42 deletions(-) diff --git a/__tests__/actions/__snapshots__/api.js.snap b/__tests__/actions/__snapshots__/api.js.snap index 6da1719c7..ec5bb843b 100644 --- a/__tests__/actions/__snapshots__/api.js.snap +++ b/__tests__/actions/__snapshots__/api.js.snap @@ -1,8 +1,48 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`actions > api should make a query to OTP 1`] = `"/api/plan?arriveBy=false&fromPlace=12%2C34&showIntermediateStops=true&toPlace=34%2C12"`; +exports[`actions > api > planTrip should make a query to OTP with customOtpQueryBuilder in state config 1`] = `"/api/plan?from=here&to=there"`; -exports[`actions > api should make a query to OTP 2`] = ` +exports[`actions > api > planTrip should make a query to OTP with customOtpQueryBuilder in state config 2`] = ` +Array [ + Array [ + Object { + "type": "PLAN_REQUEST", + }, + ], + Array [ + Object { + "payload": Object { + "fake": "response", + }, + "type": "PLAN_RESPONSE", + }, + ], +] +`; + +exports[`actions > api > planTrip should make a query to OTP with customOtpQueryBuilder sent to action 1`] = `"/api/plan?from=here&to=there"`; + +exports[`actions > api > planTrip should make a query to OTP with customOtpQueryBuilder sent to action 2`] = ` +Array [ + Array [ + Object { + "type": "PLAN_REQUEST", + }, + ], + Array [ + Object { + "payload": Object { + "fake": "response", + }, + "type": "PLAN_RESPONSE", + }, + ], +] +`; + +exports[`actions > api > planTrip should make a query to OTP with default settings 1`] = `"/api/plan?arriveBy=false&fromPlace=12%2C34&showIntermediateStops=true&toPlace=34%2C12"`; + +exports[`actions > api > planTrip should make a query to OTP with default settings 2`] = ` Array [ Array [ Object { diff --git a/__tests__/actions/api.js b/__tests__/actions/api.js index 3316e006b..4c592efc5 100644 --- a/__tests__/actions/api.js +++ b/__tests__/actions/api.js @@ -10,48 +10,82 @@ function timeoutPromise (ms) { }) } -const planTripAction = planTrip() - describe('actions > api', () => { - it('should make a query to OTP', async () => { - nock('http://mock-host.com') - .get(/api\/plan/) - .reply(200, { - fake: 'response' - }) - .on('request', (req, interceptor) => { - expect(req.path).toMatchSnapshot() - }) + describe('> planTrip', () => { + const defaultState = { + otp: { + config: { + api: { + host: 'http://mock-host.com', + path: '/api', + port: 80 + } + }, + currentQuery: { + from: { lat: 12, lon: 34 }, + to: { lat: 34, lon: 12 } + }, + searches: [] + } + } - const mockDispatch = jest.fn() - planTripAction(mockDispatch, () => { - return { - otp: { - config: { - api: { - host: 'http://mock-host.com', - path: '/api', - port: 80 - } - }, - currentQuery: { - from: { - lat: 12, - lon: 34 - }, - to: { - lat: 34, - lon: 12 - } + const customOtpQueryBuilder = () => { + return `http://mock-host.com/api/plan?from=here&to=there` + } + + const stateWithCustomBuilderFunction = { + otp: { + config: { + api: { + host: 'http://mock-host.com', + path: '/api', + port: 80 }, - searches: [] - } + customOtpQueryBuilder + }, + currentQuery: { + from: { lat: 12, lon: 34 }, + to: { lat: 34, lon: 12 } + }, + searches: [] } - }) + } - // wait for request to complete - await timeoutPromise(100) + const testCases = [{ + state: defaultState, + title: 'default settings' + }, { + customOtpQueryBuilder, + state: defaultState, + title: 'customOtpQueryBuilder sent to action' + }, { + state: stateWithCustomBuilderFunction, + title: 'customOtpQueryBuilder in state config' + }] - expect(mockDispatch.mock.calls).toMatchSnapshot() + testCases.forEach((testCase) => { + it(`should make a query to OTP with ${testCase.title}`, async () => { + const planTripAction = planTrip(testCase.customOtpQueryBuilder) + + nock('http://mock-host.com') + .get(/api\/plan/) + .reply(200, { + fake: 'response' + }) + .on('request', (req, interceptor) => { + expect(req.path).toMatchSnapshot() + }) + + const mockDispatch = jest.fn() + planTripAction(mockDispatch, () => { + return testCase.state + }) + + // wait for request to complete + await timeoutPromise(100) + + expect(mockDispatch.mock.calls).toMatchSnapshot() + }) + }) }) }) diff --git a/lib/actions/api.js b/lib/actions/api.js index f8b57c183..5ab306828 100644 --- a/lib/actions/api.js +++ b/lib/actions/api.js @@ -8,7 +8,7 @@ if (typeof (fetch) === 'undefined') { require('isomorphic-fetch') } -import { hasValidLocation } from '../util/state' +import { queryIsValid } from '../util/state' export const receivedPlanResponse = createAction('PLAN_RESPONSE') export const requestPlanResponse = createAction('PLAN_REQUEST') @@ -22,9 +22,9 @@ export function planTrip (customOtpQueryBuilder) { console.log('query hasn\'t changed') return } - if (!hasValidLocation(otpState, 'from') || !hasValidLocation(otpState, 'to')) return // TODO: replace with isQueryValid? + if (!queryIsValid(otpState)) return dispatch(requestPlanResponse()) - const queryBuilderFn = customOtpQueryBuilder || constructPlanQuery + const queryBuilderFn = customOtpQueryBuilder || otpState.config.customOtpQueryBuilder || constructPlanQuery const url = queryBuilderFn(otpState.config.api, otpState.currentQuery) // setURLSearch(url) fetch(url) From 967a995608cf342ec06fd3424a86ee6050b06bd6 Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Wed, 22 Mar 2017 17:42:27 -0700 Subject: [PATCH 3/4] feat(actions): add ability to switch locations --- example.js | 5 ++++- lib/actions/map.js | 22 ++++++++++++++++--- lib/components/form/switchButton.js | 33 +++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 4 deletions(-) create mode 100644 lib/components/form/switchButton.js diff --git a/example.js b/example.js index f5ffa17f9..bc38a9e8d 100644 --- a/example.js +++ b/example.js @@ -7,7 +7,7 @@ import thunk from 'redux-thunk' import createLogger from 'redux-logger' // import Bootstrap Grid components for layout -import { Navbar, Grid, Row, Col } from 'react-bootstrap' +import { Navbar, Grid, Row, Col, Button } from 'react-bootstrap' // import OTP-RR components import { @@ -25,6 +25,8 @@ import { ErrorMessage } from './lib' +import SwitchButton from './lib/components/form/switchButton' + // load the OTP configuration import otpConfig from './config.yml' @@ -63,6 +65,7 @@ class OtpRRExample extends Component { + diff --git a/lib/actions/map.js b/lib/actions/map.js index 53fcf3b6e..5e4f98ed9 100644 --- a/lib/actions/map.js +++ b/lib/actions/map.js @@ -13,8 +13,16 @@ import { formChanged } from './form' * } */ -export const settingLocation = createAction('SET_LOCATION') export const clearingLocation = createAction('CLEAR_LOCATION') +export const settingLocation = createAction('SET_LOCATION') +export const switchingLocations = createAction('SWITCH_LOCATIONS') + +export function clearLocation (payload) { + return function (dispatch, getState) { + dispatch(clearingLocation(payload)) + dispatch(formChanged()) + } +} export function setLocation (payload) { return function (dispatch, getState) { @@ -23,9 +31,17 @@ export function setLocation (payload) { } } -export function clearLocation (payload) { +export function switchLocations () { return function (dispatch, getState) { - dispatch(clearingLocation(payload)) + const {from, to} = getState().otp.currentQuery + dispatch(settingLocation({ + type: 'from', + location: to + })) + dispatch(settingLocation({ + type: 'to', + location: from + })) dispatch(formChanged()) } } diff --git a/lib/components/form/switchButton.js b/lib/components/form/switchButton.js new file mode 100644 index 000000000..aa8f917ff --- /dev/null +++ b/lib/components/form/switchButton.js @@ -0,0 +1,33 @@ +import React, { Component, PropTypes } from 'react' +import { Button } from 'react-bootstrap' +import { connect } from 'react-redux' + +import { switchLocations } from '../../actions/map' + +class SwitchButton extends Component { + static propTypes = { + onClick: PropTypes.func + } + _onClick = () => { + this.props.switchLocations() + } + render () { + return ( + + ) + } +} + +const mapStateToProps = (state, ownProps) => { + return {} +} + +const mapDispatchToProps = (dispatch, ownProps) => { + return { + switchLocations: () => { dispatch(switchLocations()) } + } +} + +export default connect(mapStateToProps, mapDispatchToProps)(SwitchButton) From 136ed607af2a619282f9be23898eb4385fc084f1 Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Thu, 23 Mar 2017 08:21:32 -0700 Subject: [PATCH 4/4] refactor(example): rename and alphabetize some things * Add Switch Button to lib * Alphabetize all the things in lib/index.js * rename file for switch button --- example.js | 7 +++-- .../{switchButton.js => switch-button.js} | 0 lib/index.js | 26 ++++++++++--------- 3 files changed, 17 insertions(+), 16 deletions(-) rename lib/components/form/{switchButton.js => switch-button.js} (100%) diff --git a/example.js b/example.js index bc38a9e8d..4f811ed0e 100644 --- a/example.js +++ b/example.js @@ -7,7 +7,7 @@ import thunk from 'redux-thunk' import createLogger from 'redux-logger' // import Bootstrap Grid components for layout -import { Navbar, Grid, Row, Col, Button } from 'react-bootstrap' +import { Navbar, Grid, Row, Col } from 'react-bootstrap' // import OTP-RR components import { @@ -22,11 +22,10 @@ import { // OsmBaseLayer, PlanTripButton, createOtpReducer, - ErrorMessage + ErrorMessage, + SwitchButton } from './lib' -import SwitchButton from './lib/components/form/switchButton' - // load the OTP configuration import otpConfig from './config.yml' diff --git a/lib/components/form/switchButton.js b/lib/components/form/switch-button.js similarity index 100% rename from lib/components/form/switchButton.js rename to lib/components/form/switch-button.js diff --git a/lib/index.js b/lib/index.js index 62e454234..7a5ee3ced 100644 --- a/lib/index.js +++ b/lib/index.js @@ -1,31 +1,33 @@ -import LocationField from './components/form/location-field' -import PlanTripButton from './components/form/plan-trip-button' -import ModeSelector from './components/form/mode-selector' import DateTimeSelector from './components/form/date-time-selector' import ErrorMessage from './components/form/error-message' +import LocationField from './components/form/location-field' +import ModeSelector from './components/form/mode-selector' +import PlanTripButton from './components/form/plan-trip-button' +import SwitchButton from './components/form/switch-button' -import NarrativeItineraries from './components/narrative/narrative-itineraries' -import NarrativeItinerary from './components/narrative/narrative-itinerary' - -import BaseMap from './components/map/base-map' import BaseLayers from './components/map/base-layers' -import OsmBaseLayer from './components/map/osm-base-layer' +import BaseMap from './components/map/base-map' import EndpointsOverlay from './components/map/endpoints-overlay' import ItineraryOverlay from './components/map/itinerary-overlay' +import OsmBaseLayer from './components/map/osm-base-layer' + +import NarrativeItineraries from './components/narrative/narrative-itineraries' +import NarrativeItinerary from './components/narrative/narrative-itinerary' import createOtpReducer from './reducers/create-otp-reducer' export { // form components - LocationField, - PlanTripButton, - ModeSelector, DateTimeSelector, ErrorMessage, + LocationField, + ModeSelector, + PlanTripButton, + SwitchButton, // map components - BaseMap, BaseLayers, + BaseMap, EndpointsOverlay, ItineraryOverlay, OsmBaseLayer,