diff --git a/src/visits/GraphCard.js b/src/visits/GraphCard.js index 5814739e..695be8fb 100644 --- a/src/visits/GraphCard.js +++ b/src/visits/GraphCard.js @@ -12,6 +12,7 @@ const propTypes = { stats: PropTypes.object, max: PropTypes.number, highlightedStats: PropTypes.object, + onClick: PropTypes.func, }; const generateGraphData = (title, isBarChart, labels, data, highlightedData) => ({ @@ -19,6 +20,7 @@ const generateGraphData = (title, isBarChart, labels, data, highlightedData) => datasets: [ { title, + label: '', data, backgroundColor: isBarChart ? 'rgba(70, 150, 229, 0.4)' : [ '#97BBCD', @@ -45,17 +47,20 @@ const generateGraphData = (title, isBarChart, labels, data, highlightedData) => const dropLabelIfHidden = (label) => label.startsWith('hidden') ? '' : label; -const renderGraph = (title, isBarChart, stats, max, highlightedStats) => { +const renderGraph = (title, isBarChart, stats, max, highlightedStats, onClick) => { + const hasHighlightedStats = highlightedStats && Object.keys(highlightedStats).length > 0; const Component = isBarChart ? HorizontalBar : Doughnut; const labels = keys(stats).map(dropLabelIfHidden); - const data = values(!highlightedStats ? stats : keys(highlightedStats).reduce((acc, highlightedKey) => { + const data = values(!hasHighlightedStats ? stats : keys(highlightedStats).reduce((acc, highlightedKey) => { if (acc[highlightedKey]) { acc[highlightedKey] -= highlightedStats[highlightedKey]; } return acc; }, { ...stats })); - const highlightedData = highlightedStats && values({ ...zipObj(labels, labels.map(() => 0)), ...highlightedStats }); + const highlightedData = hasHighlightedStats && values( + { ...zipObj(labels, labels.map(() => 0)), ...highlightedStats } + ); const options = { legend: isBarChart ? { display: false } : { position: 'right' }, @@ -79,13 +84,30 @@ const renderGraph = (title, isBarChart, stats, max, highlightedStats) => { const height = isBarChart && labels.length > 20 ? labels.length * 8 : null; // Provide a key based on the height, so that every time the dataset changes, a new graph is rendered - return ; + return ( + { + if (!onClick || !chart) { + return; + } + + const { _index, _chart: { data } } = chart; + const { labels } = data; + + onClick(labels[_index]); + }} + /> + ); }; -const GraphCard = ({ title, footer, isBarChart, stats, max, highlightedStats }) => ( +const GraphCard = ({ title, footer, isBarChart, stats, max, highlightedStats, onClick }) => ( {typeof title === 'function' ? title() : title} - {renderGraph(title, isBarChart, stats, max, highlightedStats)} + {renderGraph(title, isBarChart, stats, max, highlightedStats, onClick)} {footer && {footer}} ); diff --git a/src/visits/ShortUrlVisits.js b/src/visits/ShortUrlVisits.js index fd6026e2..5c233bb9 100644 --- a/src/visits/ShortUrlVisits.js +++ b/src/visits/ShortUrlVisits.js @@ -1,5 +1,5 @@ -import { isEmpty, values } from 'ramda'; -import React, { useState, useEffect } from 'react'; +import { isEmpty, propEq, values } from 'ramda'; +import React, { useState, useEffect, useMemo } from 'react'; import { Button, Card, Collapse } from 'reactstrap'; import PropTypes from 'prop-types'; import qs from 'qs'; @@ -41,10 +41,8 @@ const highlightedVisitsToStats = (highlightedVisits, prop) => highlightedVisits. return acc; }, {}); const format = formatDate(); -let memoizationId; -let timeWhenMounted; -const ShortUrlVisits = ({ processStatsFromVisits }, OpenMapModalBtn) => { +const ShortUrlVisits = ({ processStatsFromVisits, normalizeVisits }, OpenMapModalBtn) => { const ShortUrlVisitsComp = ({ match, location, @@ -62,23 +60,28 @@ const ShortUrlVisits = ({ processStatsFromVisits }, OpenMapModalBtn) => { const [ highlightedVisits, setHighlightedVisits ] = useState([]); const [ isMobileDevice, setIsMobileDevice ] = useState(false); const determineIsMobileDevice = () => setIsMobileDevice(matchMedia('(max-width: 991px)').matches); + const highlightVisitsForProp = (prop) => (value) => setHighlightedVisits( + highlightedVisits.length === 0 ? normalizedVisits.filter(propEq(prop, value)) : [] + ); const { params } = match; const { shortCode } = params; const { search } = location; const { domain } = qs.parse(search, { ignoreQueryPrefix: true }); - const loadVisits = () => { - const start = format(startDate); - const end = format(endDate); + const { visits, loading, loadingLarge, error } = shortUrlVisits; + const showTableControls = !loading && visits.length > 0; + const normalizedVisits = useMemo(() => normalizeVisits(visits), [ visits ]); + const { os, browsers, referrers, countries, cities, citiesForMap } = useMemo( + () => processStatsFromVisits(visits), + [ visits ] + ); + const mapLocations = values(citiesForMap); - // While the "page" is loaded, use the timestamp + filtering dates as memoization IDs for stats calculations - memoizationId = `${timeWhenMounted}_${shortCode}_${start}_${end}`; - getShortUrlVisits(shortCode, { startDate: start, endDate: end, domain }); - }; + const loadVisits = () => + getShortUrlVisits(shortCode, { startDate: format(startDate), endDate: format(endDate), domain }); useEffect(() => { - timeWhenMounted = new Date().getTime(); getShortUrlDetail(shortCode, domain); determineIsMobileDevice(); window.addEventListener('resize', determineIsMobileDevice); @@ -92,9 +95,6 @@ const ShortUrlVisits = ({ processStatsFromVisits }, OpenMapModalBtn) => { loadVisits(); }, [ startDate, endDate ]); - const { visits, loading, loadingLarge, error } = shortUrlVisits; - const showTableControls = !loading && visits.length > 0; - const renderVisitsContent = () => { if (loading) { const message = loadingLarge ? 'This is going to take a while... :S' : 'Loading...'; @@ -114,11 +114,6 @@ const ShortUrlVisits = ({ processStatsFromVisits }, OpenMapModalBtn) => { return There are no visits matching current filter :(; } - const { os, browsers, referrers, countries, cities, citiesForMap } = processStatsFromVisits( - { id: memoizationId, visits } - ); - const mapLocations = values(citiesForMap); - return (
@@ -137,6 +132,7 @@ const ShortUrlVisits = ({ processStatsFromVisits }, OpenMapModalBtn) => { name: 'Referrer name', amount: 'Visits amount', }} + onClick={highlightVisitsForProp('referer')} />
@@ -148,6 +144,7 @@ const ShortUrlVisits = ({ processStatsFromVisits }, OpenMapModalBtn) => { name: 'Country name', amount: 'Visits amount', }} + onClick={highlightVisitsForProp('country')} />
@@ -163,6 +160,7 @@ const ShortUrlVisits = ({ processStatsFromVisits }, OpenMapModalBtn) => { name: 'City name', amount: 'Visits amount', }} + onClick={highlightVisitsForProp('city')} />
@@ -185,11 +183,7 @@ const ShortUrlVisits = ({ processStatsFromVisits }, OpenMapModalBtn) => {
{showTableControls && ( - @@ -201,12 +195,16 @@ const ShortUrlVisits = ({ processStatsFromVisits }, OpenMapModalBtn) => { {showTableControls && ( - + )} diff --git a/src/visits/SortableBarGraph.js b/src/visits/SortableBarGraph.js index 54a1bd9b..aecca4e7 100644 --- a/src/visits/SortableBarGraph.js +++ b/src/visits/SortableBarGraph.js @@ -20,6 +20,7 @@ export default class SortableBarGraph extends React.Component { sortingItems: PropTypes.object.isRequired, extraHeaderContent: PropTypes.func, withPagination: PropTypes.bool, + onClick: PropTypes.func, }; state = { @@ -74,7 +75,7 @@ export default class SortableBarGraph extends React.Component { } render() { - const { stats, sortingItems, title, extraHeaderContent, highlightedStats, withPagination = true } = this.props; + const { stats, sortingItems, title, extraHeaderContent, withPagination = true, ...rest } = this.props; const { currentPageStats, pagination, max } = this.determineStats(stats, sortingItems); const activeCities = keys(currentPageStats); const computeTitle = () => ( @@ -115,7 +116,7 @@ export default class SortableBarGraph extends React.Component { stats={currentPageStats} footer={pagination} max={max} - highlightedStats={highlightedStats} + {...rest} /> ); } diff --git a/src/visits/VisitsTable.js b/src/visits/VisitsTable.js index bd4dcaf1..6c5e082f 100644 --- a/src/visits/VisitsTable.js +++ b/src/visits/VisitsTable.js @@ -2,7 +2,7 @@ import React, { useEffect, useMemo, useState } from 'react'; import PropTypes from 'prop-types'; import Moment from 'react-moment'; import classNames from 'classnames'; -import { map, min, splitEvery } from 'ramda'; +import { min, splitEvery } from 'ramda'; import { faCaretDown as caretDownIcon, faCaretUp as caretUpIcon, @@ -11,15 +11,18 @@ import { import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import SimplePaginator from '../common/SimplePaginator'; import SearchField from '../utils/SearchField'; -import { browserFromUserAgent, extractDomain, osFromUserAgent } from '../utils/helpers/visits'; import { determineOrderDir } from '../utils/utils'; import { prettify } from '../utils/helpers/numbers'; -import { visitType } from './reducers/shortUrlVisits'; import './VisitsTable.scss'; +const NormalizedVisitType = PropTypes.shape({ + +}); + const propTypes = { - visits: PropTypes.arrayOf(visitType).isRequired, - onVisitsSelected: PropTypes.func, + visits: PropTypes.arrayOf(NormalizedVisitType).isRequired, + selectedVisits: PropTypes.arrayOf(NormalizedVisitType), + setSelectedVisits: PropTypes.func.isRequired, isSticky: PropTypes.bool, matchMedia: PropTypes.func, }; @@ -35,34 +38,30 @@ const sortVisits = ({ field, dir }, visits) => visits.sort((a, b) => { return a[field] > b[field] ? greaterThan : smallerThan; }); const calculateVisits = (allVisits, searchTerm, order) => { - const filteredVisits = searchTerm ? searchVisits(searchTerm, allVisits) : allVisits; + const filteredVisits = searchTerm ? searchVisits(searchTerm, allVisits) : [ ...allVisits ]; const sortedVisits = order.dir ? sortVisits(order, filteredVisits) : filteredVisits; const total = sortedVisits.length; const visitsGroups = splitEvery(PAGE_SIZE, sortedVisits); return { visitsGroups, total }; }; -const normalizeVisits = map(({ userAgent, date, referer, visitLocation }) => ({ - date, - browser: browserFromUserAgent(userAgent), - os: osFromUserAgent(userAgent), - referer: extractDomain(referer), - country: (visitLocation && visitLocation.countryName) || 'Unknown', - city: (visitLocation && visitLocation.cityName) || 'Unknown', -})); -const VisitsTable = ({ visits, onVisitsSelected, isSticky = false, matchMedia = window.matchMedia }) => { - const allVisits = normalizeVisits(visits); +const VisitsTable = ({ + visits, + selectedVisits = [], + setSelectedVisits, + isSticky = false, + matchMedia = window.matchMedia, +}) => { const headerCellsClass = classNames('visits-table__header-cell', { 'visits-table__sticky': isSticky, }); const matchMobile = () => matchMedia('(max-width: 767px)').matches; - const [ selectedVisits, setSelectedVisits ] = useState([]); const [ isMobileDevice, setIsMobileDevice ] = useState(matchMobile()); const [ searchTerm, setSearchTerm ] = useState(undefined); const [ order, setOrder ] = useState({ field: undefined, dir: undefined }); - const resultSet = useMemo(() => calculateVisits(allVisits, searchTerm, order), [ searchTerm, order ]); + const resultSet = useMemo(() => calculateVisits(visits, searchTerm, order), [ searchTerm, order ]); const [ page, setPage ] = useState(1); const end = page * PAGE_SIZE; @@ -76,9 +75,6 @@ const VisitsTable = ({ visits, onVisitsSelected, isSticky = false, matchMedia = /> ); - useEffect(() => { - onVisitsSelected && onVisitsSelected(selectedVisits); - }, [ selectedVisits ]); useEffect(() => { const listener = () => setIsMobileDevice(matchMobile()); diff --git a/src/visits/services/VisitsParser.js b/src/visits/services/VisitsParser.js index 32cdf29d..25db20d8 100644 --- a/src/visits/services/VisitsParser.js +++ b/src/visits/services/VisitsParser.js @@ -1,4 +1,4 @@ -import { isEmpty, isNil, memoizeWith, prop } from 'ramda'; +import { isEmpty, isNil, map } from 'ramda'; import { browserFromUserAgent, extractDomain, osFromUserAgent } from '../../utils/helpers/visits'; const visitLocationHasProperty = (visitLocation, propertyName) => @@ -51,7 +51,7 @@ const updateCitiesForMapForVisit = (citiesForMapStats, { visitLocation }) => { citiesForMapStats[cityName] = currentCity; }; -export const processStatsFromVisits = memoizeWith(prop('id'), ({ visits }) => +export const processStatsFromVisits = (visits) => visits.reduce( (stats, visit) => { // We mutate the original object because it has a big side effect when large data sets are processed @@ -65,4 +65,13 @@ export const processStatsFromVisits = memoizeWith(prop('id'), ({ visits }) => return stats; }, { os: {}, browsers: {}, referrers: {}, countries: {}, cities: {}, citiesForMap: {} } - )); + ); + +export const normalizeVisits = map(({ userAgent, date, referer, visitLocation }) => ({ + date, + browser: browserFromUserAgent(userAgent), + os: osFromUserAgent(userAgent), + referer: extractDomain(referer), + country: (visitLocation && visitLocation.countryName) || 'Unknown', + city: (visitLocation && visitLocation.cityName) || 'Unknown', +})); diff --git a/test/visits/ShortUrlVisits.test.js b/test/visits/ShortUrlVisits.test.js index 1adae436..ad8743a1 100644 --- a/test/visits/ShortUrlVisits.test.js +++ b/test/visits/ShortUrlVisits.test.js @@ -20,7 +20,7 @@ describe('', () => { const location = { search: '' }; const createComponent = (shortUrlVisits) => { - const ShortUrlVisits = createShortUrlVisits({ processStatsFromVisits }, () => ''); + const ShortUrlVisits = createShortUrlVisits({ processStatsFromVisits, normalizeVisits: identity }, () => ''); wrapper = shallow( ', () => { const matchMedia = () => ({ matches: false }); + const setSelectedVisits = jest.fn(); let wrapper; - const createWrapper = (visits) => { - wrapper = shallow(); + const createWrapper = (visits, selectedVisits = []) => { + wrapper = shallow( + + ); return wrapper; }; - afterEach(() => wrapper && wrapper.unmount()); + afterEach(() => { + jest.resetAllMocks(); + wrapper && wrapper.unmount(); + }); it('renders columns as expected', () => { const wrapper = createWrapper([]); @@ -44,7 +55,7 @@ describe('', () => { [ 60, 3 ], [ 115, 6 ], ])('renders the expected amount of pages', (visitsCount, expectedAmountOfPages) => { - const wrapper = createWrapper(rangeOf(visitsCount, () => ({ userAgent: '', date: '', referer: '' }))); + const wrapper = createWrapper(rangeOf(visitsCount, () => ({ browser: '', date: '', referer: '' }))); const tr = wrapper.find('tbody').find('tr'); const paginator = wrapper.find(SimplePaginator); @@ -55,7 +66,7 @@ describe('', () => { it.each( rangeOf(20, (value) => [ value ]) )('does not render footer when there is only one page to render', (visitsCount) => { - const wrapper = createWrapper(rangeOf(visitsCount, () => ({ userAgent: '', date: '', referer: '' }))); + const wrapper = createWrapper(rangeOf(visitsCount, () => ({ browser: '', date: '', referer: '' }))); const tr = wrapper.find('tbody').find('tr'); const paginator = wrapper.find(SimplePaginator); @@ -64,39 +75,34 @@ describe('', () => { }); it('selected rows are highlighted', () => { - const wrapper = createWrapper(rangeOf(10, () => ({ userAgent: '', date: '', referer: '' }))); + const visits = rangeOf(10, () => ({ browser: '', date: '', referer: '' })); + const wrapper = createWrapper( + visits, + [ visits[1], visits[2] ], + ); - expect(wrapper.find('.text-primary')).toHaveLength(0); - expect(wrapper.find('.table-primary')).toHaveLength(0); - wrapper.find('tr').at(5).simulate('click'); - expect(wrapper.find('.text-primary')).toHaveLength(2); - expect(wrapper.find('.table-primary')).toHaveLength(1); - wrapper.find('tr').at(3).simulate('click'); expect(wrapper.find('.text-primary')).toHaveLength(3); expect(wrapper.find('.table-primary')).toHaveLength(2); + + // Select one extra + wrapper.find('tr').at(5).simulate('click'); + expect(setSelectedVisits).toHaveBeenCalledWith([ visits[1], visits[2], visits[4] ]); + + // Deselect one wrapper.find('tr').at(3).simulate('click'); - expect(wrapper.find('.text-primary')).toHaveLength(2); - expect(wrapper.find('.table-primary')).toHaveLength(1); + expect(setSelectedVisits).toHaveBeenCalledWith([ visits[1] ]); // Select all wrapper.find('thead').find('th').at(0).simulate('click'); - expect(wrapper.find('.text-primary')).toHaveLength(11); - expect(wrapper.find('.table-primary')).toHaveLength(10); - - // Select none - wrapper.find('thead').find('th').at(0).simulate('click'); - expect(wrapper.find('.text-primary')).toHaveLength(0); - expect(wrapper.find('.table-primary')).toHaveLength(0); + expect(setSelectedVisits).toHaveBeenCalledWith(visits); }); it('orders visits when column is clicked', () => { const wrapper = createWrapper(rangeOf(9, (index) => ({ - userAgent: '', + browser: '', date: `${9 - index}`, referer: `${index}`, - visitLocation: { - countryName: `Country_${index}`, - }, + country: `Country_${index}`, }))); expect(wrapper.find('tbody').find('tr').at(0).find('td').at(2).text()).toContain('Country_1'); @@ -112,8 +118,8 @@ describe('', () => { it('filters list when writing in search box', () => { const wrapper = createWrapper([ - ...rangeOf(7, () => ({ userAgent: 'aaa', date: 'aaa', referer: 'aaa' })), - ...rangeOf(2, () => ({ userAgent: 'bbb', date: 'bbb', referer: 'bbb' })), + ...rangeOf(7, () => ({ browser: 'aaa', date: 'aaa', referer: 'aaa' })), + ...rangeOf(2, () => ({ browser: 'bbb', date: 'bbb', referer: 'bbb' })), ]); const searchField = wrapper.find(SearchField); diff --git a/test/visits/services/VisitsParser.test.js b/test/visits/services/VisitsParser.test.js index 01041e53..a10eef13 100644 --- a/test/visits/services/VisitsParser.test.js +++ b/test/visits/services/VisitsParser.test.js @@ -1,4 +1,4 @@ -import { processStatsFromVisits } from '../../../src/visits/services/VisitsParser'; +import { processStatsFromVisits, normalizeVisits } from '../../../src/visits/services/VisitsParser'; describe('VisitsParser', () => { const visits = [ @@ -47,7 +47,7 @@ describe('VisitsParser', () => { let stats; beforeAll(() => { - stats = processStatsFromVisits({ id: 'id', visits }); + stats = processStatsFromVisits(visits); }); it('properly parses OS stats', () => { @@ -121,4 +121,51 @@ describe('VisitsParser', () => { }); }); }); + + describe('normalizeVisits', () => { + it('properly parses the list of visits', () => { + expect(normalizeVisits(visits)).toEqual([ + { + browser: 'Firefox', + os: 'Windows', + referer: 'google.com', + country: 'Spain', + city: 'Zaragoza', + date: undefined, + }, + { + browser: 'Firefox', + os: 'MacOS', + referer: 'google.com', + country: 'United States', + city: 'New York', + date: undefined, + }, + { + browser: 'Chrome', + os: 'Linux', + referer: 'Direct', + country: 'Spain', + city: 'Unknown', + date: undefined, + }, + { + browser: 'Chrome', + os: 'Linux', + referer: 'm.facebook.com', + country: 'Spain', + city: 'Zaragoza', + date: undefined, + }, + { + browser: 'Opera', + os: 'Linux', + referer: 'Direct', + country: 'Unknown', + city: 'Unknown', + date: undefined, + }, + ]); + }); + }); });