From 07d3567244da70b0f5793d1fc323fe5ea4733ee4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 11 May 2020 19:32:42 +0200 Subject: [PATCH] Added progress bar to visits page when loading a lot of visits --- src/index.scss | 4 ++++ src/visits/VisitsStats.js | 17 ++++++++++++----- src/visits/reducers/common.js | 14 +++++++++----- src/visits/reducers/shortUrlVisits.js | 5 +++++ src/visits/reducers/tagVisits.js | 5 +++++ src/visits/types/index.js | 1 + test/visits/VisitsStats.test.js | 11 ++++++++--- test/visits/reducers/shortUrlVisits.test.js | 9 ++++++++- test/visits/reducers/tagVisits.test.js | 7 +++++++ 9 files changed, 59 insertions(+), 14 deletions(-) diff --git a/src/index.scss b/src/index.scss index 77df9f09..be46288b 100644 --- a/src/index.scss +++ b/src/index.scss @@ -61,3 +61,7 @@ body, background-color: darken($mainColor, 12%); } } + +.progress-bar { + background-color: $mainColor; +} diff --git a/src/visits/VisitsStats.js b/src/visits/VisitsStats.js index 6747926d..6b584b75 100644 --- a/src/visits/VisitsStats.js +++ b/src/visits/VisitsStats.js @@ -1,6 +1,6 @@ import { isEmpty, propEq, values } from 'ramda'; import React, { useState, useEffect, useMemo } from 'react'; -import { Button, Card, Collapse } from 'reactstrap'; +import { Button, Card, Collapse, Progress } from 'reactstrap'; import PropTypes from 'prop-types'; import classNames from 'classnames'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; @@ -59,7 +59,7 @@ const VisitsStats = ({ processStatsFromVisits, normalizeVisits }, OpenMapModalBt } }; - const { visits, loading, loadingLarge, error } = visitsInfo; + const { visits, loading, loadingLarge, error, progress } = visitsInfo; const showTableControls = !loading && visits.length > 0; const normalizedVisits = useMemo(() => normalizeVisits(visits), [ visits ]); const { os, browsers, referrers, countries, cities, citiesForMap } = useMemo( @@ -82,10 +82,17 @@ const VisitsStats = ({ processStatsFromVisits, normalizeVisits }, OpenMapModalBt }, [ startDate, endDate ]); const renderVisitsContent = () => { - if (loading) { - const message = loadingLarge ? 'This is going to take a while... :S' : 'Loading...'; + if (loadingLarge) { + return ( + + This is going to take a while... :S + + + ); + } - return {message}; + if (loading) { + return ; } if (error) { diff --git a/src/visits/reducers/common.js b/src/visits/reducers/common.js index 022b4283..3b150dcc 100644 --- a/src/visits/reducers/common.js +++ b/src/visits/reducers/common.js @@ -1,7 +1,11 @@ import { flatten, prop, range, splitEvery } from 'ramda'; const ITEMS_PER_PAGE = 5000; +const PARALLEL_REQUESTS_COUNT = 4; +const PARALLEL_STARTING_PAGE = 2; + const isLastPage = ({ currentPage, pagesCount }) => currentPage >= pagesCount; +const calcProgress = (total, current) => current * 100 / total; export const getVisitsWithLoader = async (visitsLoader, extraFinishActionData, actionMap, dispatch, getState) => { dispatch({ type: actionMap.start }); @@ -15,12 +19,10 @@ export const getVisitsWithLoader = async (visitsLoader, extraFinishActionData, a } // If there are more pages, make requests in blocks of 4 - const parallelRequestsCount = 4; - const parallelStartingPage = 2; - const pagesRange = range(parallelStartingPage, pagination.pagesCount + 1); - const pagesBlocks = splitEvery(parallelRequestsCount, pagesRange); + const pagesRange = range(PARALLEL_STARTING_PAGE, pagination.pagesCount + 1); + const pagesBlocks = splitEvery(PARALLEL_REQUESTS_COUNT, pagesRange); - if (pagination.pagesCount - 1 > parallelRequestsCount) { + if (pagination.pagesCount - 1 > PARALLEL_REQUESTS_COUNT) { dispatch({ type: actionMap.large }); } @@ -36,6 +38,8 @@ export const getVisitsWithLoader = async (visitsLoader, extraFinishActionData, a const data = await loadVisitsInParallel(pagesBlocks[index]); + dispatch({ type: actionMap.progress, progress: calcProgress(pagesBlocks.length, index + PARALLEL_STARTING_PAGE) }); + if (index < pagesBlocks.length - 1) { return data.concat(await loadPagesBlocks(pagesBlocks, index + 1)); } diff --git a/src/visits/reducers/shortUrlVisits.js b/src/visits/reducers/shortUrlVisits.js index 7b785c79..cbbdd081 100644 --- a/src/visits/reducers/shortUrlVisits.js +++ b/src/visits/reducers/shortUrlVisits.js @@ -11,6 +11,7 @@ export const GET_SHORT_URL_VISITS_ERROR = 'shlink/shortUrlVisits/GET_SHORT_URL_V 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'; +export const GET_SHORT_URL_VISITS_PROGRESS_CHANGED = 'shlink/shortUrlVisits/GET_SHORT_URL_VISITS_PROGRESS_CHANGED'; /* eslint-enable padding-line-between-statements */ export const shortUrlVisitsType = PropTypes.shape({ // TODO Should extend from VisitInfoType @@ -20,6 +21,7 @@ export const shortUrlVisitsType = PropTypes.shape({ // TODO Should extend from V loading: PropTypes.bool, loadingLarge: PropTypes.bool, error: PropTypes.bool, + progress: PropTypes.number, }); const initialState = { @@ -30,6 +32,7 @@ const initialState = { loadingLarge: false, error: false, cancelLoad: false, + progress: 0, }; export default handleActions({ @@ -43,6 +46,7 @@ export default handleActions({ }), [GET_SHORT_URL_VISITS_LARGE]: (state) => ({ ...state, loadingLarge: true }), [GET_SHORT_URL_VISITS_CANCEL]: (state) => ({ ...state, cancelLoad: true }), + [GET_SHORT_URL_VISITS_PROGRESS_CHANGED]: (state, { progress }) => ({ ...state, progress }), [CREATE_VISIT]: (state, { shortUrl, visit }) => { // eslint-disable-line object-shorthand const { shortCode, domain, visits } = state; @@ -63,6 +67,7 @@ export const getShortUrlVisits = (buildShlinkApiClient) => (shortCode, query = { large: GET_SHORT_URL_VISITS_LARGE, finish: GET_SHORT_URL_VISITS, error: GET_SHORT_URL_VISITS_ERROR, + progress: GET_SHORT_URL_VISITS_PROGRESS_CHANGED, }; return getVisitsWithLoader(visitsLoader, extraFinishActionData, actionMap, dispatch, getState); diff --git a/src/visits/reducers/tagVisits.js b/src/visits/reducers/tagVisits.js index 453b6db5..d149322b 100644 --- a/src/visits/reducers/tagVisits.js +++ b/src/visits/reducers/tagVisits.js @@ -10,6 +10,7 @@ export const GET_TAG_VISITS_ERROR = 'shlink/tagVisits/GET_TAG_VISITS_ERROR'; export const GET_TAG_VISITS = 'shlink/tagVisits/GET_TAG_VISITS'; export const GET_TAG_VISITS_LARGE = 'shlink/tagVisits/GET_TAG_VISITS_LARGE'; export const GET_TAG_VISITS_CANCEL = 'shlink/tagVisits/GET_TAG_VISITS_CANCEL'; +export const GET_TAG_VISITS_PROGRESS_CHANGED = 'shlink/tagVisits/GET_TAG_VISITS_PROGRESS_CHANGED'; /* eslint-enable padding-line-between-statements */ export const TagVisitsType = PropTypes.shape({ // TODO Should extend from VisitInfoType @@ -18,6 +19,7 @@ export const TagVisitsType = PropTypes.shape({ // TODO Should extend from VisitI loading: PropTypes.bool, loadingLarge: PropTypes.bool, error: PropTypes.bool, + progress: PropTypes.number, }); const initialState = { @@ -27,6 +29,7 @@ const initialState = { loadingLarge: false, error: false, cancelLoad: false, + progress: 0, }; export default handleActions({ @@ -35,6 +38,7 @@ export default handleActions({ [GET_TAG_VISITS]: (state, { visits, tag }) => ({ ...initialState, visits, tag }), [GET_TAG_VISITS_LARGE]: (state) => ({ ...state, loadingLarge: true }), [GET_TAG_VISITS_CANCEL]: (state) => ({ ...state, cancelLoad: true }), + [GET_TAG_VISITS_PROGRESS_CHANGED]: (state, { progress }) => ({ ...state, progress }), [CREATE_VISIT]: (state, { shortUrl, visit }) => { // eslint-disable-line object-shorthand const { tag, visits } = state; @@ -55,6 +59,7 @@ export const getTagVisits = (buildShlinkApiClient) => (tag, query = {}) => (disp large: GET_TAG_VISITS_LARGE, finish: GET_TAG_VISITS, error: GET_TAG_VISITS_ERROR, + progress: GET_TAG_VISITS_PROGRESS_CHANGED, }; return getVisitsWithLoader(visitsLoader, extraFinishActionData, actionMap, dispatch, getState); diff --git a/src/visits/types/index.js b/src/visits/types/index.js index b1464124..bd6d1383 100644 --- a/src/visits/types/index.js +++ b/src/visits/types/index.js @@ -21,4 +21,5 @@ export const VisitsInfoType = PropTypes.shape({ loading: PropTypes.bool, loadingLarge: PropTypes.bool, error: PropTypes.bool, + progress: PropTypes.number, }); diff --git a/test/visits/VisitsStats.test.js b/test/visits/VisitsStats.test.js index 225317cd..212127ad 100644 --- a/test/visits/VisitsStats.test.js +++ b/test/visits/VisitsStats.test.js @@ -1,7 +1,7 @@ import React from 'react'; import { shallow } from 'enzyme'; import { identity } from 'ramda'; -import { Card } from 'reactstrap'; +import { Card, Progress } from 'reactstrap'; import createVisitStats from '../../src/visits/VisitsStats'; import Message from '../../src/utils/Message'; import GraphCard from '../../src/visits/GraphCard'; @@ -35,17 +35,22 @@ describe('', () => { it('renders a preloader when visits are loading', () => { const wrapper = createComponent({ loading: true, visits: [] }); const loadingMessage = wrapper.find(Message); + const progress = wrapper.find(Progress); expect(loadingMessage).toHaveLength(1); expect(loadingMessage.html()).toContain('Loading...'); + expect(progress).toHaveLength(0); }); - it('renders a warning when loading large amounts of visits', () => { - const wrapper = createComponent({ loading: true, loadingLarge: true, visits: [] }); + it('renders a warning and progress bar when loading large amounts of visits', () => { + const wrapper = createComponent({ loading: true, loadingLarge: true, visits: [], progress: 25 }); const loadingMessage = wrapper.find(Message); + const progress = wrapper.find(Progress); expect(loadingMessage).toHaveLength(1); expect(loadingMessage.html()).toContain('This is going to take a while... :S'); + expect(progress).toHaveLength(1); + expect(progress.prop('value')).toEqual(25); }); it('renders an error message when visits could not be loaded', () => { diff --git a/test/visits/reducers/shortUrlVisits.test.js b/test/visits/reducers/shortUrlVisits.test.js index 5bb27dfe..0c295a12 100644 --- a/test/visits/reducers/shortUrlVisits.test.js +++ b/test/visits/reducers/shortUrlVisits.test.js @@ -6,6 +6,7 @@ import reducer, { GET_SHORT_URL_VISITS, GET_SHORT_URL_VISITS_LARGE, GET_SHORT_URL_VISITS_CANCEL, + GET_SHORT_URL_VISITS_PROGRESS_CHANGED, } from '../../../src/visits/reducers/shortUrlVisits'; import { CREATE_VISIT } from '../../../src/visits/reducers/visitCreation'; @@ -66,6 +67,12 @@ describe('shortUrlVisitsReducer', () => { expect(visits).toEqual(expectedVisits); }); + + it('returns new progress on GET_SHORT_URL_VISITS_PROGRESS_CHANGED', () => { + const state = reducer({}, { type: GET_SHORT_URL_VISITS_PROGRESS_CHANGED, progress: 85 }); + + expect(state).toEqual({ progress: 85 }); + }); }); describe('getShortUrlVisits', () => { @@ -127,7 +134,7 @@ describe('shortUrlVisitsReducer', () => { await getShortUrlVisits(() => ShlinkApiClient)('abc123')(dispatchMock, getState); expect(ShlinkApiClient.getShortUrlVisits).toHaveBeenCalledTimes(expectedRequests); - expect(dispatchMock).toHaveBeenNthCalledWith(2, expect.objectContaining({ + expect(dispatchMock).toHaveBeenNthCalledWith(3, expect.objectContaining({ visits: [{}, {}, {}, {}, {}, {}], })); }); diff --git a/test/visits/reducers/tagVisits.test.js b/test/visits/reducers/tagVisits.test.js index a9b45b00..b39444eb 100644 --- a/test/visits/reducers/tagVisits.test.js +++ b/test/visits/reducers/tagVisits.test.js @@ -6,6 +6,7 @@ import reducer, { GET_TAG_VISITS, GET_TAG_VISITS_LARGE, GET_TAG_VISITS_CANCEL, + GET_TAG_VISITS_PROGRESS_CHANGED, } from '../../../src/visits/reducers/tagVisits'; import { CREATE_VISIT } from '../../../src/visits/reducers/visitCreation'; @@ -66,6 +67,12 @@ describe('tagVisitsReducer', () => { expect(visits).toEqual(expectedVisits); }); + + it('returns new progress on GET_TAG_VISITS_PROGRESS_CHANGED', () => { + const state = reducer({}, { type: GET_TAG_VISITS_PROGRESS_CHANGED, progress: 85 }); + + expect(state).toEqual({ progress: 85 }); + }); }); describe('getTagVisits', () => {