diff --git a/package.json b/package.json index 2720942b..6b7a4245 100644 --- a/package.json +++ b/package.json @@ -37,7 +37,7 @@ "promise": "^8.0.1", "prop-types": "^15.6.2", "qs": "^6.5.2", - "ramda": "^0.25.0", + "ramda": "^0.26.1", "react": "^16.7.0", "react-autosuggest": "^9.4.0", "react-chartjs-2": "^2.7.4", diff --git a/src/visits/ShortUrlVisits.js b/src/visits/ShortUrlVisits.js index d3644404..cb064ee2 100644 --- a/src/visits/ShortUrlVisits.js +++ b/src/visits/ShortUrlVisits.js @@ -8,20 +8,13 @@ import DateInput from '../utils/DateInput'; import MutedMessage from '../utils/MuttedMessage'; import SortableBarGraph from './SortableBarGraph'; import { shortUrlVisitsType } from './reducers/shortUrlVisits'; -import { VisitsHeader } from './VisitsHeader'; +import VisitsHeader from './VisitsHeader'; import GraphCard from './GraphCard'; import { shortUrlDetailType } from './reducers/shortUrlDetail'; import './ShortUrlVisits.scss'; import OpenMapModalBtn from './helpers/OpenMapModalBtn'; -const ShortUrlVisits = ({ - processOsStats, - processBrowserStats, - processCountriesStats, - processCitiesStats, - processReferrersStats, - processCitiesStatsForMap, -}) => class ShortUrlVisits extends React.Component { +const ShortUrlVisits = ({ processStatsFromVisits }) => class ShortUrlVisits extends React.PureComponent { static propTypes = { match: PropTypes.shape({ params: PropTypes.object, @@ -35,18 +28,25 @@ const ShortUrlVisits = ({ state = { startDate: undefined, endDate: undefined }; loadVisits = () => { const { match: { params }, getShortUrlVisits } = this.props; - - getShortUrlVisits(params.shortCode, mapObjIndexed( + const { shortCode } = params; + const dates = mapObjIndexed( (value) => value && value.format ? value.format('YYYY-MM-DD') : value, this.state - )); + ); + const { startDate, endDate } = dates; + + // While the "page" is loaded, use the timestamp + filtering dates as memoization IDs for stats calcs + this.memoizationId = `${new Date().getTime()}_${shortCode}_${startDate}_${endDate}`; + + getShortUrlVisits(shortCode, dates); }; componentDidMount() { const { match: { params }, getShortUrlDetail } = this.props; + const { shortCode } = params; this.loadVisits(); - getShortUrlDetail(params.shortCode); + getShortUrlDetail(shortCode); } render() { @@ -71,17 +71,21 @@ const ShortUrlVisits = ({ return There are no visits matching current filter :(; } + const { os, browsers, referrers, countries, cities, citiesForMap } = processStatsFromVisits( + { id: this.memoizationId, visits } + ); + return (
- +
- +
( ), ]} diff --git a/src/visits/VisitsHeader.js b/src/visits/VisitsHeader.js index 3103c2a4..34bb0288 100644 --- a/src/visits/VisitsHeader.js +++ b/src/visits/VisitsHeader.js @@ -11,7 +11,7 @@ const propTypes = { shortUrlVisits: shortUrlVisitsType.isRequired, }; -export function VisitsHeader({ shortUrlDetail, shortUrlVisits }) { +export default function VisitsHeader({ shortUrlDetail, shortUrlVisits }) { const { shortUrl, loading } = shortUrlDetail; const { visits } = shortUrlVisits; const shortLink = shortUrl && shortUrl.shortUrl ? shortUrl.shortUrl : ''; diff --git a/src/visits/services/VisitsParser.js b/src/visits/services/VisitsParser.js index 174291d1..76ba477b 100644 --- a/src/visits/services/VisitsParser.js +++ b/src/visits/services/VisitsParser.js @@ -1,4 +1,4 @@ -import { assoc, isNil, isEmpty, reduce } from 'ramda'; +import { isNil, isEmpty, memoizeWith, prop, reduce } from 'ramda'; const osFromUserAgent = (userAgent) => { const lowerUserAgent = userAgent.toLowerCase(); @@ -42,79 +42,70 @@ const extractDomain = (url) => { return domain.split(':')[0]; }; -export const processOsStats = (visits) => - reduce( - (stats, { userAgent }) => { - const os = isNil(userAgent) ? 'Others' : osFromUserAgent(userAgent); - - return assoc(os, (stats[os] || 0) + 1, stats); - }, - {}, - visits, - ); - -export const processBrowserStats = (visits) => - reduce( - (stats, { userAgent }) => { - const browser = isNil(userAgent) ? 'Others' : browserFromUserAgent(userAgent); - - return assoc(browser, (stats[browser] || 0) + 1, stats); - }, - {}, - visits, - ); - -export const processReferrersStats = (visits) => - reduce( - (stats, visit) => { - const notHasDomain = isNil(visit.referer) || isEmpty(visit.referer); - const domain = notHasDomain ? 'Unknown' : extractDomain(visit.referer); - - return assoc(domain, (stats[domain] || 0) + 1, stats); - }, - {}, - visits, - ); - const visitLocationHasProperty = (visitLocation, propertyName) => !isNil(visitLocation) && !isNil(visitLocation[propertyName]) && !isEmpty(visitLocation[propertyName]); -const buildLocationStatsProcessorByProperty = (propertyName) => (visits) => +const updateOsStatsForVisit = (osStats, { userAgent }) => { + const os = isNil(userAgent) ? 'Others' : osFromUserAgent(userAgent); + + osStats[os] = (osStats[os] || 0) + 1; +}; + +const updateBrowsersStatsForVisit = (browsersStats, { userAgent }) => { + const browser = isNil(userAgent) ? 'Others' : browserFromUserAgent(userAgent); + + browsersStats[browser] = (browsersStats[browser] || 0) + 1; +}; + +const updateReferrersStatsForVisit = (referrersStats, { referer }) => { + const notHasDomain = isNil(referer) || isEmpty(referer); + const domain = notHasDomain ? 'Unknown' : extractDomain(referer); + + referrersStats[domain] = (referrersStats[domain] || 0) + 1; +}; + +const updateLocationsStatsForVisit = (propertyName) => (stats, { visitLocation }) => { + const hasLocationProperty = visitLocationHasProperty(visitLocation, propertyName); + const value = hasLocationProperty ? visitLocation[propertyName] : 'Unknown'; + + stats[value] = (stats[value] || 0) + 1; +}; + +const updateCountriesStatsForVisit = updateLocationsStatsForVisit('countryName'); +const updateCitiesStatsForVisit = updateLocationsStatsForVisit('cityName'); + +const updateCitiesForMapForVisit = (citiesForMapStats, { visitLocation }) => { + if (!visitLocationHasProperty(visitLocation, 'cityName')) { + return; + } + + const { cityName, latitude, longitude } = visitLocation; + const currentCity = citiesForMapStats[cityName] || { + cityName, + count: 0, + latLong: [ parseFloat(latitude), parseFloat(longitude) ], + }; + + currentCity.count++; + + citiesForMapStats[cityName] = currentCity; +}; + +export const processStatsFromVisits = memoizeWith(prop('id'), ({ visits }) => reduce( - (stats, { visitLocation }) => { - const hasLocationProperty = visitLocationHasProperty(visitLocation, propertyName); - const value = hasLocationProperty ? visitLocation[propertyName] : 'Unknown'; + (stats, visit) => { + // We mutate the original object because it has a big side effect when large data sets are processed + updateOsStatsForVisit(stats.os, visit); + updateBrowsersStatsForVisit(stats.browsers, visit); + updateReferrersStatsForVisit(stats.referrers, visit); + updateCountriesStatsForVisit(stats.countries, visit); + updateCitiesStatsForVisit(stats.cities, visit); + updateCitiesForMapForVisit(stats.citiesForMap, visit); - return assoc(value, (stats[value] || 0) + 1, stats); + return stats; }, - {}, + { os: {}, browsers: {}, referrers: {}, countries: {}, cities: {}, citiesForMap: {} }, visits, - ); - -export const processCountriesStats = buildLocationStatsProcessorByProperty('countryName'); - -export const processCitiesStats = buildLocationStatsProcessorByProperty('cityName'); - -export const processCitiesStatsForMap = (visits) => - reduce( - (stats, { visitLocation }) => { - if (!visitLocationHasProperty(visitLocation, 'cityName')) { - return stats; - } - - const { cityName, latitude, longitude } = visitLocation; - const currentCity = stats[cityName] || { - cityName, - count: 0, - latLong: [ parseFloat(latitude), parseFloat(longitude) ], - }; - - currentCity.count++; - - return assoc(cityName, currentCity, stats); - }, - {}, - visits, - ); + )); diff --git a/test/visits/ShortUrlVisits.test.js b/test/visits/ShortUrlVisits.test.js index 843bc68a..a24e8645 100644 --- a/test/visits/ShortUrlVisits.test.js +++ b/test/visits/ShortUrlVisits.test.js @@ -11,21 +11,16 @@ import SortableBarGraph from '../../src/visits/SortableBarGraph'; describe('', () => { let wrapper; - const statsProcessor = () => ({}); + const processStatsFromVisits = () => ( + { os: {}, browsers: {}, referrers: {}, countries: {}, cities: {}, citiesForMap: {} } + ); const getShortUrlVisitsMock = sinon.spy(); const match = { params: { shortCode: 'abc123' }, }; const createComponent = (shortUrlVisits) => { - const ShortUrlVisits = createShortUrlVisits({ - processBrowserStats: statsProcessor, - processCountriesStats: statsProcessor, - processOsStats: statsProcessor, - processReferrersStats: statsProcessor, - processCitiesStats: statsProcessor, - processCitiesStatsForMap: statsProcessor, - }); + const ShortUrlVisits = createShortUrlVisits({ processStatsFromVisits }); wrapper = shallow( ', () => { diff --git a/test/visits/services/VisitsParser.test.js b/test/visits/services/VisitsParser.test.js index f5002e7c..c6dfd717 100644 --- a/test/visits/services/VisitsParser.test.js +++ b/test/visits/services/VisitsParser.test.js @@ -1,11 +1,4 @@ -import { - processOsStats, - processBrowserStats, - processReferrersStats, - processCountriesStats, - processCitiesStats, - processCitiesStatsForMap, -} from '../../../src/visits/services/VisitsParser'; +import { processStatsFromVisits } from '../../../src/visits/services/VisitsParser'; describe('VisitsParser', () => { const visits = [ @@ -50,64 +43,71 @@ describe('VisitsParser', () => { }, ]; - describe('processOsStats', () => { + describe('processStatsFromVisits', () => { + let stats; + + beforeAll(() => { + stats = processStatsFromVisits({ id: 'id', visits }); + }); + it('properly parses OS stats', () => { - expect(processOsStats(visits)).toEqual({ + const { os } = stats; + + expect(os).toEqual({ Linux: 3, Windows: 1, MacOS: 1, }); }); - }); - describe('processBrowserStats', () => { it('properly parses browser stats', () => { - expect(processBrowserStats(visits)).toEqual({ + const { browsers } = stats; + + expect(browsers).toEqual({ Firefox: 2, Chrome: 2, Opera: 1, }); }); - }); - describe('processReferrersStats', () => { it('properly parses referrer stats', () => { - expect(processReferrersStats(visits)).toEqual({ + const { referrers } = stats; + + expect(referrers).toEqual({ 'Unknown': 2, 'google.com': 2, 'm.facebook.com': 1, }); }); - }); - describe('processCountriesStats', () => { it('properly parses countries stats', () => { - expect(processCountriesStats(visits)).toEqual({ + const { countries } = stats; + + expect(countries).toEqual({ 'Spain': 3, 'United States': 1, 'Unknown': 1, }); }); - }); - describe('processCitiesStats', () => { it('properly parses cities stats', () => { - expect(processCitiesStats(visits)).toEqual({ + const { cities } = stats; + + expect(cities).toEqual({ 'Zaragoza': 2, 'New York': 1, 'Unknown': 2, }); }); - }); - describe('processCitiesStatsForMap', () => { it('properly parses cities stats with lat and long', () => { + const { citiesForMap } = stats; const zaragozaLat = 123.45; const zaragozaLong = -543.21; const newYorkLat = 1029; const newYorkLong = 6758; - expect(processCitiesStatsForMap(visits)).toEqual({ + expect(citiesForMap).toEqual({ 'Zaragoza': { cityName: 'Zaragoza', count: 2, diff --git a/yarn.lock b/yarn.lock index ef2345f2..cfb4bbea 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7910,9 +7910,10 @@ railroad-diagrams@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/railroad-diagrams/-/railroad-diagrams-1.0.0.tgz#eb7e6267548ddedfb899c1b90e57374559cddb7e" -ramda@^0.25.0: - version "0.25.0" - resolved "https://registry.yarnpkg.com/ramda/-/ramda-0.25.0.tgz#8fdf68231cffa90bc2f9460390a0cb74a29b29a9" +ramda@^0.26.1: + version "0.26.1" + resolved "https://registry.yarnpkg.com/ramda/-/ramda-0.26.1.tgz#8d41351eb8111c55353617fc3bbffad8e4d35d06" + integrity sha512-hLWjpy7EnsDBb0p+Z3B7rPi3GDeRG5ZtiI33kJhTt+ORCd38AbAIjB/9zRIUoeTbE/AVX5ZkU7m6bznsvrf8eQ== randexp@0.4.6: version "0.4.6"