From 5f3382a483da078e5cca444709707aef9e801b4a Mon Sep 17 00:00:00 2001 From: Evan Siroky Date: Mon, 13 Mar 2017 15:12:23 -0700 Subject: [PATCH 1/2] 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/2] 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)