Merge pull request #122 from acelaya/feature/cancel-visits-load

Feature/cancel visits load

.travis.yml

@@ -1,7 +1,7 @@
 language: node_js
 
 node_js:
-  - "stable"
+  - "10.15.3"
 
 cache:
   yarn: true

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

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
 

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;
 

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,11 @@ 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 +123,5 @@ export const getShortUrlVisits = (buildShlinkApiClient) => (shortCode, dates) =>
     dispatch({ type: GET_SHORT_URL_VISITS_ERROR });
   }
 };
+
+export const cancelGetShortUrlVisits = () => ({ type: GET_SHORT_URL_VISITS_CANCEL });

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;

test/visits/ShortUrlVisits.test.js

@@ -29,6 +29,7 @@ describe('<ShortUrlVisits />', () => {
         match={match}
         shortUrlVisits={shortUrlVisits}
         shortUrlDetail={{}}
+        cancelGetShortUrlVisits={identity}
       />
     );
 

test/visits/reducers/shortUrlVisits.test.js

@@ -1,10 +1,12 @@
 import * as sinon from 'sinon';
 import reducer, {
   getShortUrlVisits,
+  cancelGetShortUrlVisits,
   GET_SHORT_URL_VISITS_START,
   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 +25,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 +67,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());
 
@@ -126,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 }));
+  });
 });