From 3982d7777563ef44f82b955b1f734a9d9ff504b0 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 Mar 2019 19:40:43 +0100 Subject: [PATCH 1/5] Ensured visits loading is cancelled when the visits page is unmounted --- src/visits/ShortUrlVisits.js | 5 ++++ src/visits/reducers/shortUrlVisits.js | 31 +++++++++++++++++---- src/visits/services/provideServices.js | 5 ++-- test/visits/ShortUrlVisits.test.js | 1 + test/visits/reducers/shortUrlVisits.test.js | 12 +++++++- 5 files changed, 45 insertions(+), 9 deletions(-) diff --git a/src/visits/ShortUrlVisits.js b/src/visits/ShortUrlVisits.js index 566074df..6c2f9bf8 100644 --- a/src/visits/ShortUrlVisits.js +++ b/src/visits/ShortUrlVisits.js @@ -23,6 +23,7 @@ const ShortUrlVisits = ({ processStatsFromVisits }) => class ShortUrlVisits exte shortUrlVisits: shortUrlVisitsType, getShortUrlDetail: PropTypes.func, shortUrlDetail: shortUrlDetailType, + cancelGetShortUrlVisits: PropTypes.func, }; state = { startDate: undefined, endDate: undefined }; @@ -49,6 +50,10 @@ const ShortUrlVisits = ({ processStatsFromVisits }) => class ShortUrlVisits exte getShortUrlDetail(shortCode); } + componentWillUnmount() { + this.props.cancelGetShortUrlVisits(); + } + render() { const { shortUrlVisits, shortUrlDetail } = this.props; diff --git a/src/visits/reducers/shortUrlVisits.js b/src/visits/reducers/shortUrlVisits.js index 3d97d4df..bb562b49 100644 --- a/src/visits/reducers/shortUrlVisits.js +++ b/src/visits/reducers/shortUrlVisits.js @@ -1,11 +1,12 @@ import PropTypes from 'prop-types'; -import { flatten, range, splitEvery } from 'ramda'; +import { flatten, prop, range, splitEvery } from 'ramda'; /* eslint-disable padding-line-between-statements, newline-after-var */ export const GET_SHORT_URL_VISITS_START = 'shlink/shortUrlVisits/GET_SHORT_URL_VISITS_START'; export const GET_SHORT_URL_VISITS_ERROR = 'shlink/shortUrlVisits/GET_SHORT_URL_VISITS_ERROR'; export const GET_SHORT_URL_VISITS = 'shlink/shortUrlVisits/GET_SHORT_URL_VISITS'; export const GET_SHORT_URL_VISITS_LARGE = 'shlink/shortUrlVisits/GET_SHORT_URL_VISITS_LARGE'; +export const GET_SHORT_URL_VISITS_CANCEL = 'shlink/shortUrlVisits/GET_SHORT_URL_VISITS_CANCEL'; /* eslint-enable padding-line-between-statements, newline-after-var */ export const shortUrlVisitsType = PropTypes.shape({ @@ -19,6 +20,7 @@ const initialState = { loading: false, loadingLarge: false, error: false, + cancelLoad: false, }; export default function reducer(state = initialState, action) { @@ -28,6 +30,7 @@ export default function reducer(state = initialState, action) { ...state, loading: true, loadingLarge: false, + cancelLoad: false, }; case GET_SHORT_URL_VISITS_ERROR: return { @@ -35,6 +38,7 @@ export default function reducer(state = initialState, action) { loading: false, loadingLarge: false, error: true, + cancelLoad: false, }; case GET_SHORT_URL_VISITS: return { @@ -42,12 +46,18 @@ export default function reducer(state = initialState, action) { loading: false, loadingLarge: false, error: false, + cancelLoad: false, }; case GET_SHORT_URL_VISITS_LARGE: return { ...state, loadingLarge: true, }; + case GET_SHORT_URL_VISITS_CANCEL: + return { + ...state, + cancelLoad: true, + }; default: return state; } @@ -83,6 +93,12 @@ export const getShortUrlVisits = (buildShlinkApiClient) => (shortCode, dates) => }; const loadPagesBlocks = async (pagesBlocks, index = 0) => { + const { shortUrlVisits: { cancelLoad } } = getState(); + + if (cancelLoad) { + return []; + } + const data = await loadVisitsInParallel(pagesBlocks[index]); if (index < pagesBlocks.length - 1) { @@ -93,11 +109,12 @@ export const getShortUrlVisits = (buildShlinkApiClient) => (shortCode, dates) => }; const loadVisitsInParallel = (pages) => - Promise.all(pages.map(async (page) => { - const { data } = await getShortUrlVisits(shortCode, { ...dates, page, itemsPerPage }); - - return data; - })).then(flatten); + Promise.all(pages.map( + (page) => + getShortUrlVisits(shortCode, { ...dates, page, itemsPerPage }) + .then(prop('data')) + )) + .then(flatten); try { const visits = await loadVisits(); @@ -107,3 +124,5 @@ export const getShortUrlVisits = (buildShlinkApiClient) => (shortCode, dates) => dispatch({ type: GET_SHORT_URL_VISITS_ERROR }); } }; + +export const cancelGetShortUrlVisits = () => ({ type: GET_SHORT_URL_VISITS_CANCEL }); diff --git a/src/visits/services/provideServices.js b/src/visits/services/provideServices.js index 0b754868..8dcc498a 100644 --- a/src/visits/services/provideServices.js +++ b/src/visits/services/provideServices.js @@ -1,5 +1,5 @@ import ShortUrlVisits from '../ShortUrlVisits'; -import { getShortUrlVisits } from '../reducers/shortUrlVisits'; +import { cancelGetShortUrlVisits, getShortUrlVisits } from '../reducers/shortUrlVisits'; import { getShortUrlDetail } from '../reducers/shortUrlDetail'; import * as visitsParser from './VisitsParser'; @@ -8,7 +8,7 @@ const provideServices = (bottle, connect) => { bottle.serviceFactory('ShortUrlVisits', ShortUrlVisits, 'VisitsParser'); bottle.decorator('ShortUrlVisits', connect( [ 'shortUrlVisits', 'shortUrlDetail' ], - [ 'getShortUrlVisits', 'getShortUrlDetail' ] + [ 'getShortUrlVisits', 'getShortUrlDetail', 'cancelGetShortUrlVisits' ] )); // Services @@ -17,6 +17,7 @@ const provideServices = (bottle, connect) => { // Actions bottle.serviceFactory('getShortUrlVisits', getShortUrlVisits, 'buildShlinkApiClient'); bottle.serviceFactory('getShortUrlDetail', getShortUrlDetail, 'buildShlinkApiClient'); + bottle.serviceFactory('cancelGetShortUrlVisits', () => cancelGetShortUrlVisits); }; export default provideServices; diff --git a/test/visits/ShortUrlVisits.test.js b/test/visits/ShortUrlVisits.test.js index 8f792852..d485a984 100644 --- a/test/visits/ShortUrlVisits.test.js +++ b/test/visits/ShortUrlVisits.test.js @@ -29,6 +29,7 @@ describe('', () => { match={match} shortUrlVisits={shortUrlVisits} shortUrlDetail={{}} + cancelGetShortUrlVisits={identity} /> ); diff --git a/test/visits/reducers/shortUrlVisits.test.js b/test/visits/reducers/shortUrlVisits.test.js index 93e8fbe4..fcb09953 100644 --- a/test/visits/reducers/shortUrlVisits.test.js +++ b/test/visits/reducers/shortUrlVisits.test.js @@ -5,6 +5,7 @@ import reducer, { GET_SHORT_URL_VISITS_ERROR, GET_SHORT_URL_VISITS, GET_SHORT_URL_VISITS_LARGE, + GET_SHORT_URL_VISITS_CANCEL, } from '../../../src/visits/reducers/shortUrlVisits'; describe('shortUrlVisitsReducer', () => { @@ -23,6 +24,13 @@ describe('shortUrlVisitsReducer', () => { expect(loadingLarge).toEqual(true); }); + it('returns cancelLoad on GET_SHORT_URL_VISITS_CANCEL', () => { + const state = reducer({ cancelLoad: false }, { type: GET_SHORT_URL_VISITS_CANCEL }); + const { cancelLoad } = state; + + expect(cancelLoad).toEqual(true); + }); + it('stops loading and returns error on GET_SHORT_URL_VISITS_ERROR', () => { const state = reducer({ loading: true, error: false }, { type: GET_SHORT_URL_VISITS_ERROR }); const { loading, error } = state; @@ -58,7 +66,9 @@ describe('shortUrlVisitsReducer', () => { getShortUrlVisits: typeof returned === 'function' ? sinon.fake(returned) : sinon.fake.returns(returned), }); const dispatchMock = sinon.spy(); - const getState = () => ({}); + const getState = () => ({ + shortUrlVisits: { cancelVisits: false }, + }); beforeEach(() => dispatchMock.resetHistory()); From de563f9ebf45c9fc0b9d68a4c502118fedd010f7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 Mar 2019 19:42:07 +0100 Subject: [PATCH 2/5] Updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3aa3bfe..284f23e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), #### Fixed * [#120](https://github.com/shlinkio/shlink-web-client/issues/120) Fixed crash when visits page is loaded and there are no visits with known cities. +* [#113](https://github.com/shlinkio/shlink-web-client/issues/113) Ensured visits loading is cancelled when the visits page is unmounted. Requests on flight will still finish. ## 2.0.2 - 2019-03-04 From 4e6ef6ac53e8bd83f793035c431a2b8492d84782 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 Mar 2019 19:43:27 +0100 Subject: [PATCH 3/5] Removed empty line --- src/visits/reducers/shortUrlVisits.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/visits/reducers/shortUrlVisits.js b/src/visits/reducers/shortUrlVisits.js index bb562b49..5aedef99 100644 --- a/src/visits/reducers/shortUrlVisits.js +++ b/src/visits/reducers/shortUrlVisits.js @@ -113,8 +113,7 @@ export const getShortUrlVisits = (buildShlinkApiClient) => (shortCode, dates) => (page) => getShortUrlVisits(shortCode, { ...dates, page, itemsPerPage }) .then(prop('data')) - )) - .then(flatten); + )).then(flatten); try { const visits = await loadVisits(); From d7312d26f7b2b2719a98451a5b0f61a5aae48cd7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 Mar 2019 19:45:35 +0100 Subject: [PATCH 4/5] Added missing test for new action creator --- test/visits/reducers/shortUrlVisits.test.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/visits/reducers/shortUrlVisits.test.js b/test/visits/reducers/shortUrlVisits.test.js index fcb09953..97978609 100644 --- a/test/visits/reducers/shortUrlVisits.test.js +++ b/test/visits/reducers/shortUrlVisits.test.js @@ -1,6 +1,7 @@ import * as sinon from 'sinon'; import reducer, { getShortUrlVisits, + cancelGetShortUrlVisits, GET_SHORT_URL_VISITS_START, GET_SHORT_URL_VISITS_ERROR, GET_SHORT_URL_VISITS, @@ -136,4 +137,9 @@ describe('shortUrlVisitsReducer', () => { expect(visits).toEqual([{}, {}, {}, {}, {}, {}]); }); }); + + describe('cancelGetShortUrlVisits', () => { + it('just returns the action with proper type', () => + expect(cancelGetShortUrlVisits()).toEqual({ type: GET_SHORT_URL_VISITS_CANCEL })); + }); }); From 9f172c308ce8ac11d5f3a50ce87003b2fe1b5ac6 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 Mar 2019 19:50:47 +0100 Subject: [PATCH 5/5] Ensured travis makes use of a working node version for builds --- .travis.yml | 2 +- Dockerfile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 7ceae3d7..2254cc95 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,7 @@ language: node_js node_js: - - "stable" + - "10.15.3" cache: yarn: true diff --git a/Dockerfile b/Dockerfile index 05c1d33a..7c351d26 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM node:10.15.2 as node +FROM node:10.15.3-alpine as node COPY . /shlink-web-client RUN cd /shlink-web-client && yarn install && yarn build