From 83221c1066eac326847e3dfcb9fb26d0513169e5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 20 Dec 2020 19:28:09 +0100 Subject: [PATCH 1/3] Added routes to subsections in visits --- src/common/MenuLayout.tsx | 4 +- src/short-urls/ShortUrlsList.tsx | 6 +-- src/utils/helpers/query.ts | 5 ++ src/visits/ShortUrlVisits.tsx | 15 ++++-- src/visits/TagVisits.tsx | 5 +- src/visits/VisitsStats.tsx | 90 ++++++++++++++++++-------------- src/visits/VisitsTable.tsx | 12 ++--- test/visits/VisitsStats.test.tsx | 16 ++---- 8 files changed, 80 insertions(+), 73 deletions(-) create mode 100644 src/utils/helpers/query.ts diff --git a/src/common/MenuLayout.tsx b/src/common/MenuLayout.tsx index e4a32144..11326487 100644 --- a/src/common/MenuLayout.tsx +++ b/src/common/MenuLayout.tsx @@ -65,8 +65,8 @@ const MenuLayout = ( - - {addTagsVisitsRoute && } + + {addTagsVisitsRoute && } List short URLs} diff --git a/src/short-urls/ShortUrlsList.tsx b/src/short-urls/ShortUrlsList.tsx index 485204dd..86397f73 100644 --- a/src/short-urls/ShortUrlsList.tsx +++ b/src/short-urls/ShortUrlsList.tsx @@ -2,12 +2,12 @@ import { faCaretDown as caretDownIcon, faCaretUp as caretUpIcon } from '@fortawe import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import { head, keys, values } from 'ramda'; import { FC, useEffect, useState } from 'react'; -import qs from 'qs'; import { RouteComponentProps } from 'react-router'; import SortingDropdown from '../utils/SortingDropdown'; import { determineOrderDir, OrderDir } from '../utils/utils'; import { SelectedServer } from '../servers/data'; import { boundToMercureHub } from '../mercure/helpers/boundToMercureHub'; +import { parseQuery } from '../utils/helpers/query'; import { ShortUrlsList as ShortUrlsListState } from './reducers/shortUrlsList'; import { OrderableFields, ShortUrlsListParams, SORTABLE_FIELDS } from './reducers/shortUrlsListParams'; import { ShortUrlsTableProps } from './ShortUrlsTable'; @@ -65,8 +65,8 @@ const ShortUrlsList = (ShortUrlsTable: FC) => boundToMercur }; useEffect(() => { - const query = qs.parse(location.search, { ignoreQueryPrefix: true }); - const tags = query.tag ? [ query.tag as string ] : shortUrlsListParams.tags; + const { tag } = parseQuery<{ tag?: string }>(location.search); + const tags = tag ? [ tag ] : shortUrlsListParams.tags; refreshList({ page: match.params.page, tags, itemsPerPage: undefined }); diff --git a/src/utils/helpers/query.ts b/src/utils/helpers/query.ts new file mode 100644 index 00000000..8c59f6eb --- /dev/null +++ b/src/utils/helpers/query.ts @@ -0,0 +1,5 @@ +import qs from 'qs'; + +export const parseQuery = (search: string) => qs.parse(search, { ignoreQueryPrefix: true }) as unknown as T; + +export const stringifyQuery = (query: any): string => qs.stringify(query, { arrayFormat: 'brackets' }); diff --git a/src/visits/ShortUrlVisits.tsx b/src/visits/ShortUrlVisits.tsx index 37024b66..4501347a 100644 --- a/src/visits/ShortUrlVisits.tsx +++ b/src/visits/ShortUrlVisits.tsx @@ -1,8 +1,8 @@ import { useEffect } from 'react'; -import qs from 'qs'; import { RouteComponentProps } from 'react-router'; import { boundToMercureHub } from '../mercure/helpers/boundToMercureHub'; import { ShlinkVisitsParams } from '../utils/services/types'; +import { parseQuery } from '../utils/helpers/query'; import { ShortUrlVisits as ShortUrlVisitsState } from './reducers/shortUrlVisits'; import ShortUrlVisitsHeader from './ShortUrlVisitsHeader'; import { ShortUrlDetail } from './reducers/shortUrlDetail'; @@ -18,7 +18,7 @@ export interface ShortUrlVisitsProps extends RouteComponentProps<{ shortCode: st const ShortUrlVisits = boundToMercureHub(({ history: { goBack }, - match, + match: { params, url }, location: { search }, shortUrlVisits, shortUrlDetail, @@ -26,9 +26,8 @@ const ShortUrlVisits = boundToMercureHub(({ getShortUrlDetail, cancelGetShortUrlVisits, }: ShortUrlVisitsProps) => { - const { params } = match; const { shortCode } = params; - const { domain } = qs.parse(search, { ignoreQueryPrefix: true }) as { domain?: string }; + const { domain } = parseQuery<{ domain?: string }>(search); const loadVisits = (params: Partial) => getShortUrlVisits(shortCode, { ...params, domain }); @@ -37,7 +36,13 @@ const ShortUrlVisits = boundToMercureHub(({ }, []); return ( - + ); diff --git a/src/visits/TagVisits.tsx b/src/visits/TagVisits.tsx index 5f2c0244..b0701737 100644 --- a/src/visits/TagVisits.tsx +++ b/src/visits/TagVisits.tsx @@ -14,17 +14,16 @@ export interface TagVisitsProps extends RouteComponentProps<{ tag: string }> { const TagVisits = (colorGenerator: ColorGenerator) => boundToMercureHub(({ history: { goBack }, - match, + match: { params, url }, getTagVisits, tagVisits, cancelGetTagVisits, }: TagVisitsProps) => { - const { params } = match; const { tag } = params; const loadVisits = (params: ShlinkVisitsParams) => getTagVisits(tag, params); return ( - + ); diff --git a/src/visits/VisitsStats.tsx b/src/visits/VisitsStats.tsx index 1e9dce11..8816e111 100644 --- a/src/visits/VisitsStats.tsx +++ b/src/visits/VisitsStats.tsx @@ -4,6 +4,8 @@ import { Button, Card, Nav, NavLink, Progress } from 'reactstrap'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import { faCalendarAlt, faMapMarkedAlt, faList, faChartPie } from '@fortawesome/free-solid-svg-icons'; import { IconDefinition } from '@fortawesome/fontawesome-common-types'; +import { Route, Switch, NavLink as RouterNavLink, Redirect } from 'react-router-dom'; +import { Location } from 'history'; import { DateRangeSelector } from '../utils/dates/DateRangeSelector'; import Message from '../utils/Message'; import { formatIsoDate } from '../utils/helpers/date'; @@ -22,16 +24,18 @@ export interface VisitsStatsProps { getVisits: (params: Partial) => void; visitsInfo: VisitsInfo; cancelGetVisits: () => void; + baseUrl: string; + domain?: string; } type HighlightableProps = 'referer' | 'country' | 'city'; type Section = 'byTime' | 'byContext' | 'byLocation' | 'list'; -const sections: Record = { +const sections: Record = { byTime: { title: 'By time', icon: faCalendarAlt }, - byContext: { title: 'By context', icon: faChartPie }, - byLocation: { title: 'By location', icon: faMapMarkedAlt }, - list: { title: 'List', icon: faList }, + byContext: { title: 'By context', subPath: 'by-context', icon: faChartPie }, + byLocation: { title: 'By location', subPath: 'by-location', icon: faMapMarkedAlt }, + list: { title: 'List', subPath: 'list', icon: faList }, }; const highlightedVisitsToStats = ( @@ -49,13 +53,16 @@ const highlightedVisitsToStats = ( let selectedBar: string | undefined; const initialInterval: DateInterval = 'last30Days'; -const VisitsStats: FC = ({ children, visitsInfo, getVisits, cancelGetVisits }) => { +const VisitsStats: FC = ({ children, visitsInfo, getVisits, cancelGetVisits, baseUrl, domain }) => { const [ dateRange, setDateRange ] = useState(intervalToDateRange(initialInterval)); const [ highlightedVisits, setHighlightedVisits ] = useState([]); const [ highlightedLabel, setHighlightedLabel ] = useState(); - const [ activeSection, setActiveSection ] = useState
('byTime'); - const onSectionChange = (section: Section) => () => setActiveSection(section); + const buildSectionUrl = (subPath?: string) => { + const query = domain ? `?domain=${domain}` : ''; + + return !subPath ? `${baseUrl}${query}` : `${baseUrl}/${subPath}${query}`; + }; const { visits, loading, loadingLarge, error, progress } = visitsInfo; const normalizedVisits = useMemo(() => normalizeVisits(visits), [ visits ]); const { os, browsers, referrers, countries, cities, citiesForMap } = useMemo( @@ -120,12 +127,15 @@ const VisitsStats: FC = ({ children, visitsInfo, getVisits, ca
- {activeSection === 'byTime' && ( -
- -
- )} - {activeSection === 'byContext' && ( - <> + + +
+ +
+ + +
@@ -168,10 +179,9 @@ const VisitsStats: FC = ({ children, visitsInfo, getVisits, ca onClick={highlightVisitsForProp('referer')} />
- - )} - {activeSection === 'byLocation' && ( - <> + + +
= ({ children, visitsInfo, getVisits, ca onClick={highlightVisitsForProp('city')} />
- - )} - {activeSection === 'list' && ( -
- -
- )} + + + +
+ +
+ + + + ); diff --git a/src/visits/VisitsTable.tsx b/src/visits/VisitsTable.tsx index eab3d774..54e16602 100644 --- a/src/visits/VisitsTable.tsx +++ b/src/visits/VisitsTable.tsx @@ -19,7 +19,6 @@ interface VisitsTableProps { visits: NormalizedVisit[]; selectedVisits?: NormalizedVisit[]; setSelectedVisits: (visits: NormalizedVisit[]) => void; - isSticky?: boolean; matchMedia?: (query: string) => MediaQueryList; } @@ -56,12 +55,9 @@ const VisitsTable = ({ visits, selectedVisits = [], setSelectedVisits, - isSticky = false, matchMedia = window.matchMedia, }: VisitsTableProps) => { - const headerCellsClass = classNames('visits-table__header-cell', { - 'visits-table__sticky': isSticky, - }); + const headerCellsClass = 'visits-table__header-cell visits-table__sticky'; const matchMobile = () => matchMedia('(max-width: 767px)').matches; const [ isMobileDevice, setIsMobileDevice ] = useState(matchMobile()); @@ -105,9 +101,7 @@ const VisitsTable = ({ setSelectedVisits( selectedVisits.length < resultSet.total ? resultSet.visitsGroups.flat() : [], )} @@ -183,7 +177,7 @@ const VisitsTable = ({ {resultSet.total > PAGE_SIZE && ( - +
', () => { getVisits={getVisitsMock} visitsInfo={Mock.of(visitsInfo)} cancelGetVisits={() => {}} + baseUrl={''} />, ); @@ -66,24 +67,15 @@ describe('', () => { expect(message.html()).toContain('There are no visits matching current filter :('); }); - it.each([ - [ 0, 1, 0 ], - [ 1, 3, 0 ], - [ 2, 2, 0 ], - [ 3, 0, 1 ], - ])('renders expected amount of graphics based on active section', (navIndex, expectedGraphics, expectedTables) => { + it('renders expected amount of graphics', () => { const wrapper = createComponent({ loading: false, error: false, visits }); - const nav = wrapper.find(NavLink).at(navIndex); - - nav.simulate('click'); - const graphs = wrapper.find(GraphCard); const sortableBarGraphs = wrapper.find(SortableBarGraph); const lineChart = wrapper.find(LineChartCard); const table = wrapper.find(VisitsTable); - expect(graphs.length + sortableBarGraphs.length + lineChart.length).toEqual(expectedGraphics); - expect(table).toHaveLength(expectedTables); + expect(graphs.length + sortableBarGraphs.length + lineChart.length).toEqual(6); + expect(table).toHaveLength(1); }); it('holds the map button content generator on cities graph extraHeaderContent', () => { From 4642e07fd321536453206039cbbf2110a0304682 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 20 Dec 2020 19:42:37 +0100 Subject: [PATCH 2/3] Reduced duplication when defining routes in visits section --- src/visits/ShortUrlVisits.tsx | 1 - src/visits/VisitsStats.tsx | 25 ++++++++++++------------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/visits/ShortUrlVisits.tsx b/src/visits/ShortUrlVisits.tsx index 4501347a..d7ceb2ef 100644 --- a/src/visits/ShortUrlVisits.tsx +++ b/src/visits/ShortUrlVisits.tsx @@ -28,7 +28,6 @@ const ShortUrlVisits = boundToMercureHub(({ }: ShortUrlVisitsProps) => { const { shortCode } = params; const { domain } = parseQuery<{ domain?: string }>(search); - const loadVisits = (params: Partial) => getShortUrlVisits(shortCode, { ...params, domain }); useEffect(() => { diff --git a/src/visits/VisitsStats.tsx b/src/visits/VisitsStats.tsx index 8816e111..f1b9936a 100644 --- a/src/visits/VisitsStats.tsx +++ b/src/visits/VisitsStats.tsx @@ -31,11 +31,11 @@ export interface VisitsStatsProps { type HighlightableProps = 'referer' | 'country' | 'city'; type Section = 'byTime' | 'byContext' | 'byLocation' | 'list'; -const sections: Record = { - byTime: { title: 'By time', icon: faCalendarAlt }, - byContext: { title: 'By context', subPath: 'by-context', icon: faChartPie }, - byLocation: { title: 'By location', subPath: 'by-location', icon: faMapMarkedAlt }, - list: { title: 'List', subPath: 'list', icon: faList }, +const sections: Record = { + byTime: { title: 'By time', subPath: '', icon: faCalendarAlt }, + byContext: { title: 'By context', subPath: '/by-context', icon: faChartPie }, + byLocation: { title: 'By location', subPath: '/by-location', icon: faMapMarkedAlt }, + list: { title: 'List', subPath: '/list', icon: faList }, }; const highlightedVisitsToStats = ( @@ -61,7 +61,7 @@ const VisitsStats: FC = ({ children, visitsInfo, getVisits, ca const buildSectionUrl = (subPath?: string) => { const query = domain ? `?domain=${domain}` : ''; - return !subPath ? `${baseUrl}${query}` : `${baseUrl}/${subPath}${query}`; + return !subPath ? `${baseUrl}${query}` : `${baseUrl}${subPath}${query}`; }; const { visits, loading, loadingLarge, error, progress } = visitsInfo; const normalizedVisits = useMemo(() => normalizeVisits(visits), [ visits ]); @@ -133,9 +133,8 @@ const VisitsStats: FC = ({ children, visitsInfo, getVisits, ca tag={RouterNavLink} className="visits-stats__nav-link" to={buildSectionUrl(subPath)} - isActive={(_: null, { pathname }: Location) => - (!subPath && pathname.endsWith('/visits')) || (subPath && pathname.endsWith(subPath)) - } + isActive={(_: null, { pathname }: Location) => pathname.endsWith(`/visits${subPath}`)} + replace > {title} @@ -158,7 +157,7 @@ const VisitsStats: FC = ({ children, visitsInfo, getVisits, ca
- +
@@ -181,7 +180,7 @@ const VisitsStats: FC = ({ children, visitsInfo, getVisits, ca
- +
= ({ children, visitsInfo, getVisits, ca
- +
= ({ children, visitsInfo, getVisits, ca className="btn-md-block" onClick={() => setSelectedVisits([])} > - Reset selection {highlightedVisits.length > 0 && <>({highlightedVisits.length})} + Clear selection {highlightedVisits.length > 0 && <>({highlightedVisits.length})}
)} From f5d03ed3a2071c6a135157ff5dbee925f941f46f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 20 Dec 2020 19:51:43 +0100 Subject: [PATCH 3/3] Created query helper test --- test/utils/helpers/query.test.ts | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 test/utils/helpers/query.test.ts diff --git a/test/utils/helpers/query.test.ts b/test/utils/helpers/query.test.ts new file mode 100644 index 00000000..90969bce --- /dev/null +++ b/test/utils/helpers/query.test.ts @@ -0,0 +1,25 @@ +import { parseQuery, stringifyQuery } from '../../../src/utils/helpers/query'; + +describe('query', () => { + describe('parseQuery', () => { + it.each([ + [ '', {}], + [ 'foo=bar', { foo: 'bar' }], + [ '?foo=bar', { foo: 'bar' }], + [ '?foo=bar&baz=123', { foo: 'bar', baz: '123' }], + ])('parses query string as expected', (queryString, expectedResult) => { + expect(parseQuery(queryString)).toEqual(expectedResult); + }); + }); + + describe('stringifyQuery', () => { + it.each([ + [{}, '' ], + [{ foo: 'bar' }, 'foo=bar' ], + [{ foo: 'bar', baz: '123' }, 'foo=bar&baz=123' ], + [{ bar: 'foo', list: [ 'one', 'two' ] }, encodeURI('bar=foo&list[]=one&list[]=two') ], + ])('stringifies query as expected', (queryObj, expectedResult) => { + expect(stringifyQuery(queryObj)).toEqual(expectedResult); + }); + }); +});