From 901df2b90d996f454fc9e22040398e5df2658ea5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 22 Dec 2022 12:25:25 +0100 Subject: [PATCH 1/9] Added new setting to exclude bots from visits, wherever possible --- src/settings/VisitsSettings.tsx | 21 ++++++++++++++++- src/settings/reducers/settings.ts | 1 + test/settings/VisitsSettings.test.tsx | 33 +++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/settings/VisitsSettings.tsx b/src/settings/VisitsSettings.tsx index 6d6a7bc4..a799bd2a 100644 --- a/src/settings/VisitsSettings.tsx +++ b/src/settings/VisitsSettings.tsx @@ -1,20 +1,39 @@ import { FC } from 'react'; +import { FormGroup } from 'reactstrap'; import { SimpleCard } from '../utils/SimpleCard'; import { DateIntervalSelector } from '../utils/dates/DateIntervalSelector'; import { LabeledFormGroup } from '../utils/forms/LabeledFormGroup'; import { Settings, VisitsSettings as VisitsSettingsConfig } from './reducers/settings'; +import { ToggleSwitch } from '../utils/ToggleSwitch'; +import { FormText } from '../utils/forms/FormText'; +import { DateInterval } from '../utils/helpers/dateIntervals'; interface VisitsProps { settings: Settings; setVisitsSettings: (settings: VisitsSettingsConfig) => void; } +const currentDefaultInterval = (settings: Settings): DateInterval => settings.visits?.defaultInterval ?? 'last30Days'; + export const VisitsSettings: FC = ({ settings, setVisitsSettings }) => ( + + setVisitsSettings( + { defaultInterval: currentDefaultInterval(settings), excludeBots }, + )} + > + Exclude bots wherever possible (this option‘s effect might depend on Shlink server‘s version). + + The visits coming from potential bots will be {settings.visits?.excludeBots ? 'excluded' : 'included'}. + + + setVisitsSettings({ defaultInterval })} /> diff --git a/src/settings/reducers/settings.ts b/src/settings/reducers/settings.ts index e64f3338..c8df4c48 100644 --- a/src/settings/reducers/settings.ts +++ b/src/settings/reducers/settings.ts @@ -34,6 +34,7 @@ export interface UiSettings { export interface VisitsSettings { defaultInterval: DateInterval; + excludeBots?: boolean; } export interface TagsSettings { diff --git a/test/settings/VisitsSettings.test.tsx b/test/settings/VisitsSettings.test.tsx index 8d198272..051bdcf4 100644 --- a/test/settings/VisitsSettings.test.tsx +++ b/test/settings/VisitsSettings.test.tsx @@ -17,6 +17,7 @@ describe('', () => { expect(screen.getByRole('heading')).toHaveTextContent('Visits'); expect(screen.getByText('Default interval to load on visits sections:')).toBeInTheDocument(); + expect(screen.getByText(/^Exclude bots wherever possible/)).toBeInTheDocument(); }); it.each([ @@ -59,4 +60,36 @@ describe('', () => { expect(setVisitsSettings).toHaveBeenNthCalledWith(2, { defaultInterval: 'last180Days' }); expect(setVisitsSettings).toHaveBeenNthCalledWith(3, { defaultInterval: 'yesterday' }); }); + + it.each([ + [ + Mock.all(), + /The visits coming from potential bots will be included.$/, + /The visits coming from potential bots will be excluded.$/, + ], + [ + Mock.of({ visits: { excludeBots: false } }), + /The visits coming from potential bots will be included.$/, + /The visits coming from potential bots will be excluded.$/, + ], + [ + Mock.of({ visits: { excludeBots: true } }), + /The visits coming from potential bots will be excluded.$/, + /The visits coming from potential bots will be included.$/, + ], + ])('displays expected helper text for exclude bots control', (settings, expectedText, notExpectedText) => { + setUp(settings); + + const visitsComponent = screen.getByText(/^Exclude bots wherever possible/); + + expect(visitsComponent).toHaveTextContent(expectedText); + expect(visitsComponent).not.toHaveTextContent(notExpectedText); + }); + + it('invokes setVisitsSettings when bot exclusion is toggled', async () => { + const { user } = setUp(); + + await user.click(screen.getByText(/^Exclude bots wherever possible/)); + expect(setVisitsSettings).toHaveBeenCalledWith(expect.objectContaining({ excludeBots: true })); + }); }); From b9285fd600e5c7584bf7742eda5a8ad6630954b1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 22 Dec 2022 18:07:44 +0100 Subject: [PATCH 2/9] Enrues proper ordering is sent to shlink when ordering by visits --- src/api/types/index.ts | 9 ++++-- src/short-urls/ShortUrlsList.tsx | 16 +++++++--- src/utils/helpers/features.ts | 1 + test/short-urls/ShortUrlsList.test.tsx | 41 ++++++++++++++++++++++---- 4 files changed, 55 insertions(+), 12 deletions(-) diff --git a/src/api/types/index.ts b/src/api/types/index.ts index fc3fe6de..4acda7c0 100644 --- a/src/api/types/index.ts +++ b/src/api/types/index.ts @@ -1,6 +1,7 @@ import { Visit } from '../../visits/types'; import { OptionalString } from '../../utils/utils'; -import { ShortUrl, ShortUrlMeta, ShortUrlsOrder } from '../../short-urls/data'; +import { ShortUrl, ShortUrlMeta } from '../../short-urls/data'; +import { Order } from '../../utils/helpers/ordering'; export interface ShlinkShortUrlsResponse { data: ShortUrl[]; @@ -88,6 +89,10 @@ export interface ShlinkDomainsResponse { export type TagsFilteringMode = 'all' | 'any'; +type ShlinkShortUrlsOrderableFields = 'dateCreated' | 'shortCode' | 'longUrl' | 'title' | 'visits' | 'nonBotVisits'; + +export type ShlinkShortUrlsOrder = Order; + export interface ShlinkShortUrlsListParams { page?: string; itemsPerPage?: number; @@ -95,7 +100,7 @@ export interface ShlinkShortUrlsListParams { searchTerm?: string; startDate?: string; endDate?: string; - orderBy?: ShortUrlsOrder; + orderBy?: ShlinkShortUrlsOrder; tagsMode?: TagsFilteringMode; } diff --git a/src/short-urls/ShortUrlsList.tsx b/src/short-urls/ShortUrlsList.tsx index c41a4f7d..60bfa759 100644 --- a/src/short-urls/ShortUrlsList.tsx +++ b/src/short-urls/ShortUrlsList.tsx @@ -7,14 +7,15 @@ import { getServerId, SelectedServer } from '../servers/data'; import { boundToMercureHub } from '../mercure/helpers/boundToMercureHub'; import { Topics } from '../mercure/helpers/Topics'; import { TableOrderIcon } from '../utils/table/TableOrderIcon'; -import { ShlinkShortUrlsListParams } from '../api/types'; +import { ShlinkShortUrlsListParams, ShlinkShortUrlsOrder } from '../api/types'; import { DEFAULT_SHORT_URLS_ORDERING, Settings } from '../settings/reducers/settings'; import { ShortUrlsList as ShortUrlsListState } from './reducers/shortUrlsList'; import { ShortUrlsTableType } from './ShortUrlsTable'; import { Paginator } from './Paginator'; import { useShortUrlsQuery } from './helpers/hooks'; -import { ShortUrlsOrderableFields } from './data'; +import { ShortUrlsOrder, ShortUrlsOrderableFields } from './data'; import { ShortUrlsFilteringBarType } from './ShortUrlsFilteringBar'; +import { supportsExcludeBotsOnShortUrls } from '../utils/helpers/features'; interface ShortUrlsListProps { selectedServer: SelectedServer; @@ -48,6 +49,13 @@ export const ShortUrlsList = ( (newTag: string) => [...new Set([...tags, newTag])], (updatedTags) => toFirstPage({ tags: updatedTags }), ); + const parseOrderByForShlink = ({ field, dir }: ShortUrlsOrder): ShlinkShortUrlsOrder => { + if (supportsExcludeBotsOnShortUrls(selectedServer) && settings.visits?.excludeBots && field === 'visits') { + return { field: 'nonBotVisits', dir }; + } + + return { field, dir }; + }; useEffect(() => { listShortUrls({ @@ -56,10 +64,10 @@ export const ShortUrlsList = ( tags, startDate, endDate, - orderBy: actualOrderBy, + orderBy: parseOrderByForShlink(actualOrderBy), tagsMode, }); - }, [page, search, tags, startDate, endDate, actualOrderBy, tagsMode]); + }, [page, search, tags, startDate, endDate, actualOrderBy.field, actualOrderBy.dir, tagsMode]); return ( <> diff --git a/src/utils/helpers/features.ts b/src/utils/helpers/features.ts index e3fbbd12..90f81a51 100644 --- a/src/utils/helpers/features.ts +++ b/src/utils/helpers/features.ts @@ -14,3 +14,4 @@ export const supportsDefaultDomainRedirectsEdition = serverMatchesMinVersion('2. export const supportsNonOrphanVisits = serverMatchesMinVersion('3.0.0'); export const supportsAllTagsFiltering = supportsNonOrphanVisits; export const supportsDomainVisits = serverMatchesMinVersion('3.1.0'); +export const supportsExcludeBotsOnShortUrls = serverMatchesMinVersion('3.4.0'); diff --git a/test/short-urls/ShortUrlsList.test.tsx b/test/short-urls/ShortUrlsList.test.tsx index 031dc98e..12a81188 100644 --- a/test/short-urls/ShortUrlsList.test.tsx +++ b/test/short-urls/ShortUrlsList.test.tsx @@ -9,6 +9,7 @@ import { ReachableServer } from '../../src/servers/data'; import { Settings } from '../../src/settings/reducers/settings'; import { ShortUrlsTableType } from '../../src/short-urls/ShortUrlsTable'; import { renderWithEvents } from '../__helpers__/setUpTest'; +import { SemVer } from '../../src/utils/helpers/version'; jest.mock('react-router-dom', () => ({ ...jest.requireActual('react-router-dom'), @@ -35,14 +36,14 @@ describe('', () => { }, }); const ShortUrlsList = createShortUrlsList(ShortUrlsTable, ShortUrlsFilteringBar); - const setUp = (defaultOrdering: ShortUrlsOrder = {}) => renderWithEvents( + const setUp = (settings: Partial = {}, version: SemVer = '3.0.0') => renderWithEvents( ({ mercureInfo: { loading: true } })} listShortUrls={listShortUrlsMock} shortUrlsList={shortUrlsList} - selectedServer={Mock.of({ id: '1' })} - settings={Mock.of({ shortUrlsList: { defaultOrdering } })} + selectedServer={Mock.of({ id: '1', version })} + settings={Mock.of(settings)} /> , ); @@ -82,11 +83,39 @@ describe('', () => { it.each([ [Mock.of({ field: 'visits', dir: 'ASC' }), 'visits', 'ASC'], [Mock.of({ field: 'title', dir: 'DESC' }), 'title', 'DESC'], - [Mock.of(), undefined, undefined], - ])('has expected initial ordering based on settings', (initialOrderBy, field, dir) => { - setUp(initialOrderBy); + [Mock.all(), undefined, undefined], + ])('has expected initial ordering based on settings', (defaultOrdering, field, dir) => { + setUp({ shortUrlsList: { defaultOrdering } }); expect(listShortUrlsMock).toHaveBeenCalledWith(expect.objectContaining({ orderBy: { field, dir }, })); }); + + it.each([ + [Mock.of({ + shortUrlsList: { + defaultOrdering: { field: 'visits', dir: 'ASC' }, + }, + }), '3.3.0' as SemVer, { field: 'visits', dir: 'ASC' }], + [Mock.of({ + shortUrlsList: { + defaultOrdering: { field: 'visits', dir: 'ASC' }, + }, + visits: { excludeBots: true }, + }), '3.3.0' as SemVer, { field: 'visits', dir: 'ASC' }], + [Mock.of({ + shortUrlsList: { + defaultOrdering: { field: 'visits', dir: 'ASC' }, + }, + }), '3.4.0' as SemVer, { field: 'visits', dir: 'ASC' }], + [Mock.of({ + shortUrlsList: { + defaultOrdering: { field: 'visits', dir: 'ASC' }, + }, + visits: { excludeBots: true }, + }), '3.4.0' as SemVer, { field: 'nonBotVisits', dir: 'ASC' }], + ])('parses order by based on server version and config', (settings, serverVersion, expectedOrderBy) => { + setUp(settings, serverVersion); + expect(listShortUrlsMock).toHaveBeenCalledWith(expect.objectContaining({ orderBy: expectedOrderBy })); + }); }); From 2859ba6cd25c18d7c391ec661a4576804f260138 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 22 Dec 2022 18:30:02 +0100 Subject: [PATCH 3/9] Simplified reducer logic to update short URLs when new visits come --- src/short-urls/reducers/shortUrlsList.ts | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/short-urls/reducers/shortUrlsList.ts b/src/short-urls/reducers/shortUrlsList.ts index 66ba341d..5b8d52b1 100644 --- a/src/short-urls/reducers/shortUrlsList.ts +++ b/src/short-urls/reducers/shortUrlsList.ts @@ -1,5 +1,5 @@ import { createSlice } from '@reduxjs/toolkit'; -import { assoc, assocPath, last, pipe, reject } from 'ramda'; +import { assocPath, last, pipe, reject } from 'ramda'; import { shortUrlMatches } from '../helpers'; import { createNewVisits } from '../../visits/reducers/visitCreation'; import { createAsyncThunk } from '../../utils/helpers/redux'; @@ -101,18 +101,12 @@ export const shortUrlsListReducerCreator = ( (state, { payload }) => assocPath( ['shortUrls', 'data'], state.shortUrls?.data?.map( - (currentShortUrl) => { - // Find the last of the new visit for this short URL, and pick the amount of visits from it - const lastVisit = last( - payload.createdVisits.filter( - ({ shortUrl }) => shortUrl && shortUrlMatches(currentShortUrl, shortUrl.shortCode, shortUrl.domain), - ), - ); - - return lastVisit?.shortUrl - ? assoc('visitsCount', lastVisit.shortUrl.visitsCount, currentShortUrl) - : currentShortUrl; - }, + // Find the last of the new visit for this short URL, and pick its short URL. It will have an up-to-date amount of visits. + (currentShortUrl) => last( + payload.createdVisits.filter( + ({ shortUrl }) => shortUrl && shortUrlMatches(currentShortUrl, shortUrl.shortCode, shortUrl.domain), + ), + )?.shortUrl ?? currentShortUrl, ), state, ), From 5942cd6fcf107b2769b2d3cff976c354be7d4c2b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 22 Dec 2022 18:39:09 +0100 Subject: [PATCH 4/9] Ensured proper amount of visits is displayed on short URLs based on config --- src/short-urls/helpers/ShortUrlsRow.tsx | 20 +++++++++++----- src/short-urls/services/provideServices.ts | 3 +++ test/short-urls/helpers/ShortUrlsRow.test.tsx | 23 ++++++++++++++----- 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/src/short-urls/helpers/ShortUrlsRow.tsx b/src/short-urls/helpers/ShortUrlsRow.tsx index e962da53..549266c4 100644 --- a/src/short-urls/helpers/ShortUrlsRow.tsx +++ b/src/short-urls/helpers/ShortUrlsRow.tsx @@ -1,4 +1,4 @@ -import { useEffect, useRef } from 'react'; +import { FC, useEffect, useRef } from 'react'; import { ExternalLink } from 'react-external-link'; import { ColorGenerator } from '../../utils/services/ColorGenerator'; import { TimeoutToggle } from '../../utils/helpers/hooks'; @@ -6,6 +6,7 @@ import { SelectedServer } from '../../servers/data'; import { CopyToClipboardIcon } from '../../utils/CopyToClipboardIcon'; import { ShortUrl } from '../data'; import { Time } from '../../utils/dates/Time'; +import { Settings } from '../../settings/reducers/settings'; import { ShortUrlVisitsCount } from './ShortUrlVisitsCount'; import { ShortUrlsRowMenuType } from './ShortUrlsRowMenu'; import { Tags } from './Tags'; @@ -18,19 +19,26 @@ interface ShortUrlsRowProps { shortUrl: ShortUrl; } +interface ShortUrlsRowConnectProps extends ShortUrlsRowProps { + settings: Settings; +} + +export type ShortUrlsRowType = FC; + export const ShortUrlsRow = ( ShortUrlsRowMenu: ShortUrlsRowMenuType, colorGenerator: ColorGenerator, useTimeoutToggle: TimeoutToggle, -) => ({ shortUrl, selectedServer, onTagClick }: ShortUrlsRowProps) => { +) => ({ shortUrl, selectedServer, onTagClick, settings }: ShortUrlsRowConnectProps) => { const [copiedToClipboard, setCopiedToClipboard] = useTimeoutToggle(); const [active, setActive] = useTimeoutToggle(false, 500); const isFirstRun = useRef(true); + const { visits } = settings; useEffect(() => { !isFirstRun.current && setActive(); isFirstRun.current = false; - }, [shortUrl.visitsCount]); + }, [shortUrl.visitsSummary?.total, shortUrl.visitsSummary?.nonBots, shortUrl.visitsCount]); return ( @@ -64,7 +72,9 @@ export const ShortUrlsRow = ( ); }; - -export type ShortUrlsRowType = ReturnType; diff --git a/src/short-urls/services/provideServices.ts b/src/short-urls/services/provideServices.ts index eeb10ce9..185ab81e 100644 --- a/src/short-urls/services/provideServices.ts +++ b/src/short-urls/services/provideServices.ts @@ -28,7 +28,10 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator) => { )); bottle.serviceFactory('ShortUrlsTable', ShortUrlsTable, 'ShortUrlsRow'); + bottle.serviceFactory('ShortUrlsRow', ShortUrlsRow, 'ShortUrlsRowMenu', 'ColorGenerator', 'useTimeoutToggle'); + bottle.decorator('ShortUrlsRow', connect(['settings'])); + bottle.serviceFactory('ShortUrlsRowMenu', ShortUrlsRowMenu, 'DeleteShortUrlModal', 'QrCodeModal'); bottle.serviceFactory('CreateShortUrlResult', CreateShortUrlResult, 'useTimeoutToggle'); bottle.serviceFactory('ShortUrlForm', ShortUrlForm, 'TagsSelector', 'DomainSelector'); diff --git a/test/short-urls/helpers/ShortUrlsRow.test.tsx b/test/short-urls/helpers/ShortUrlsRow.test.tsx index 35e2a858..9884b0e1 100644 --- a/test/short-urls/helpers/ShortUrlsRow.test.tsx +++ b/test/short-urls/helpers/ShortUrlsRow.test.tsx @@ -5,12 +5,20 @@ import { addDays, formatISO, subDays } from 'date-fns'; import { ShortUrlsRow as createShortUrlsRow } from '../../../src/short-urls/helpers/ShortUrlsRow'; import { TimeoutToggle } from '../../../src/utils/helpers/hooks'; import { ShortUrl, ShortUrlMeta } from '../../../src/short-urls/data'; +import { Settings } from '../../../src/settings/reducers/settings'; import { ReachableServer } from '../../../src/servers/data'; import { parseDate, now } from '../../../src/utils/helpers/date'; import { renderWithEvents } from '../../__helpers__/setUpTest'; import { OptionalString } from '../../../src/utils/utils'; import { colorGeneratorMock } from '../../utils/services/__mocks__/ColorGenerator.mock'; +interface SetUpOptions { + title?: OptionalString; + tags?: string[]; + meta?: ShortUrlMeta; + settings?: Partial; +} + describe('', () => { const timeoutToggle = jest.fn(() => true); const useTimeoutToggle = jest.fn(() => [false, timeoutToggle]) as TimeoutToggle; @@ -35,15 +43,14 @@ describe('', () => { }, }; const ShortUrlsRow = createShortUrlsRow(() => ShortUrlsRowMenu, colorGeneratorMock, useTimeoutToggle); - const setUp = ( - { title, tags = [], meta = {} }: { title?: OptionalString; tags?: string[]; meta?: ShortUrlMeta } = {}, - ) => renderWithEvents( + const setUp = ({ title, tags = [], meta = {}, settings = {} }: SetUpOptions = {}) => renderWithEvents( null} + settings={Mock.of(settings)} />
, @@ -97,9 +104,13 @@ describe('', () => { expectedContents.forEach((content) => expect(cell).toHaveTextContent(content)); }); - it('renders visits count in fifth row', () => { - setUp(); - expect(screen.getAllByRole('cell')[4]).toHaveTextContent(`${shortUrl.visitsCount}`); + it.each([ + [{}, shortUrl.visitsSummary?.total], + [Mock.of({ visits: { excludeBots: false } }), shortUrl.visitsSummary?.total], + [Mock.of({ visits: { excludeBots: true } }), shortUrl.visitsSummary?.nonBots], + ])('renders visits count in fifth row', (settings, expectedAmount) => { + setUp({ settings }); + expect(screen.getAllByRole('cell')[4]).toHaveTextContent(`${expectedAmount}`); }); it('updates state when copied to clipboard', async () => { From 80cea91339ae7e8c9d4ab9e5647ea040110dfeb0 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 23 Dec 2022 10:07:23 +0100 Subject: [PATCH 5/9] Ensured bots exclusion is selected by default in visits filter dropdown, if it was selected in settings --- src/visits/VisitsStats.tsx | 10 ++++++++-- src/visits/helpers/hooks.ts | 8 ++++---- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/visits/VisitsStats.tsx b/src/visits/VisitsStats.tsx index 639d8858..d823fa9d 100644 --- a/src/visits/VisitsStats.tsx +++ b/src/visits/VisitsStats.tsx @@ -93,6 +93,12 @@ export const VisitsStats: FC = ({ [normalizedVisits], ); const mapLocations = values(citiesForMap); + const resolvedFilter = useMemo(() => ( + !isFirstLoad.current ? visitsFilter : { + ...visitsFilter, + excludeBots: visitsFilter.excludeBots ?? settings.visits?.excludeBots, + } + ), [visitsFilter]); const setSelectedVisits = (selectedVisits: NormalizedVisit[]) => { selectedBar = undefined; @@ -115,7 +121,7 @@ export const VisitsStats: FC = ({ useEffect(() => cancelGetVisits, []); useEffect(() => { const resolvedDateRange = !isFirstLoad.current ? dateRange : (dateRange ?? toDateRange(initialInterval.current)); - getVisits({ dateRange: resolvedDateRange, filter: visitsFilter }, isFirstLoad.current); + getVisits({ dateRange: resolvedDateRange, filter: resolvedFilter }, isFirstLoad.current); isFirstLoad.current = false; }, [dateRange, visitsFilter]); useEffect(() => { @@ -301,7 +307,7 @@ export const VisitsStats: FC = ({ className="ms-0 ms-md-2 mt-3 mt-md-0" isOrphanVisits={isOrphanVisits} botsSupported={botsSupported} - selected={visitsFilter} + selected={resolvedFilter} onChange={(newVisitsFilter) => updateFiltering({ visitsFilter: newVisitsFilter })} /> diff --git a/src/visits/helpers/hooks.ts b/src/visits/helpers/hooks.ts index 2c78b65f..54ccff47 100644 --- a/src/visits/helpers/hooks.ts +++ b/src/visits/helpers/hooks.ts @@ -1,7 +1,7 @@ import { DeepPartial } from '@reduxjs/toolkit'; import { useLocation, useNavigate } from 'react-router-dom'; import { useMemo } from 'react'; -import { isEmpty, mergeDeepRight, pipe } from 'ramda'; +import { isEmpty, isNil, mergeDeepRight, pipe } from 'ramda'; import { DateRange, datesToDateRange } from '../../utils/helpers/dateIntervals'; import { OrphanVisitType, VisitsFilter } from '../types'; import { parseQuery, stringifyQuery } from '../../utils/helpers/query'; @@ -11,7 +11,7 @@ interface VisitsQuery { startDate?: string; endDate?: string; orphanVisitsType?: OrphanVisitType; - excludeBots?: 'true'; + excludeBots?: 'true' | 'false'; domain?: string; } @@ -38,7 +38,7 @@ export const useVisitsQuery = (): [VisitsFiltering, UpdateFiltering] => { domain, filtering: { dateRange: startDate != null || endDate != null ? datesToDateRange(startDate, endDate) : undefined, - visitsFilter: { orphanVisitsType, excludeBots: excludeBots === 'true' }, + visitsFilter: { orphanVisitsType, excludeBots: !isNil(excludeBots) ? excludeBots === 'true' : undefined }, }, }), ), @@ -49,7 +49,7 @@ export const useVisitsQuery = (): [VisitsFiltering, UpdateFiltering] => { const query: VisitsQuery = { startDate: (dateRange?.startDate && formatIsoDate(dateRange.startDate)) || '', endDate: (dateRange?.endDate && formatIsoDate(dateRange.endDate)) || '', - excludeBots: visitsFilter.excludeBots ? 'true' : undefined, + excludeBots: visitsFilter.excludeBots ? 'true' : 'false', orphanVisitsType: visitsFilter.orphanVisitsType, domain: theDomain, }; From 1d6f4bf5db9951638e82376df1259436fb4ef1f9 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 23 Dec 2022 20:00:59 +0100 Subject: [PATCH 6/9] =?UTF-8?q?Take=20into=20consideration=20excl=C3=B1ude?= =?UTF-8?q?Bots=20from=20query=20on=20short=20URLs=20row?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/short-urls/data/index.ts | 4 ++ .../helpers/ShortUrlsFilterDropdown.tsx | 38 ++++++++++++++ src/short-urls/helpers/ShortUrlsRow.tsx | 5 +- src/short-urls/helpers/hooks.ts | 15 ++++-- src/utils/utils.ts | 4 ++ src/visits/helpers/hooks.ts | 3 +- test/short-urls/helpers/ShortUrlsRow.test.tsx | 52 +++++++++++++------ 7 files changed, 99 insertions(+), 22 deletions(-) create mode 100644 src/short-urls/helpers/ShortUrlsFilterDropdown.tsx diff --git a/src/short-urls/data/index.ts b/src/short-urls/data/index.ts index 8c22bdec..343f385a 100644 --- a/src/short-urls/data/index.ts +++ b/src/short-urls/data/index.ts @@ -80,3 +80,7 @@ export interface ExportableShortUrl { tags: string; visits: number; } + +export interface ShortUrlsFilter { + excludeBots?: boolean; +} diff --git a/src/short-urls/helpers/ShortUrlsFilterDropdown.tsx b/src/short-urls/helpers/ShortUrlsFilterDropdown.tsx new file mode 100644 index 00000000..47b25814 --- /dev/null +++ b/src/short-urls/helpers/ShortUrlsFilterDropdown.tsx @@ -0,0 +1,38 @@ +import { DropdownItem } from 'reactstrap'; +import { DropdownBtn } from '../../utils/DropdownBtn'; +import { hasValue } from '../../utils/utils'; +import { ShortUrlsFilter } from '../data'; + +interface ShortUrlsFilterDropdownProps { + onChange: (filters: ShortUrlsFilter) => void; + selected?: ShortUrlsFilter; + className?: string; + botsSupported: boolean; +} + +export const ShortUrlsFilterDropdown = ( + { onChange, selected = {}, className, botsSupported }: ShortUrlsFilterDropdownProps, +) => { + if (!botsSupported) { + return null; + } + + const { excludeBots = false } = selected; + const onBotsClick = () => onChange({ ...selected, excludeBots: !selected?.excludeBots }); + + return ( + + {botsSupported && ( + <> + Bots: + Exclude bots visits + + )} + + + onChange({ excludeBots: false })}> + Clear filters + + + ); +}; diff --git a/src/short-urls/helpers/ShortUrlsRow.tsx b/src/short-urls/helpers/ShortUrlsRow.tsx index 549266c4..1104ba83 100644 --- a/src/short-urls/helpers/ShortUrlsRow.tsx +++ b/src/short-urls/helpers/ShortUrlsRow.tsx @@ -11,6 +11,7 @@ import { ShortUrlVisitsCount } from './ShortUrlVisitsCount'; import { ShortUrlsRowMenuType } from './ShortUrlsRowMenu'; import { Tags } from './Tags'; import { ShortUrlStatus } from './ShortUrlStatus'; +import { useShortUrlsQuery } from './hooks'; import './ShortUrlsRow.scss'; interface ShortUrlsRowProps { @@ -33,7 +34,9 @@ export const ShortUrlsRow = ( const [copiedToClipboard, setCopiedToClipboard] = useTimeoutToggle(); const [active, setActive] = useTimeoutToggle(false, 500); const isFirstRun = useRef(true); + const [{ excludeBots }] = useShortUrlsQuery(); const { visits } = settings; + const doExcludeBots = excludeBots ?? visits?.excludeBots; useEffect(() => { !isFirstRun.current && setActive(); @@ -73,7 +76,7 @@ export const ShortUrlsRow = ( ) => void; @@ -33,20 +36,26 @@ export const useShortUrlsQuery = (): [ShortUrlsFiltering, ToFirstPage] => { const filtering = useMemo( pipe( () => parseQuery(search), - ({ orderBy, tags, ...rest }: ShortUrlsQuery): ShortUrlsFiltering => { + ({ orderBy, tags, excludeBots, ...rest }: ShortUrlsQuery): ShortUrlsFiltering => { const parsedOrderBy = orderBy ? stringToOrder(orderBy) : undefined; const parsedTags = tags?.split(',') ?? []; - return { ...rest, orderBy: parsedOrderBy, tags: parsedTags }; + return { + ...rest, + orderBy: parsedOrderBy, + tags: parsedTags, + excludeBots: excludeBots !== undefined ? excludeBots === 'true' : undefined, + }; }, ), [search], ); const toFirstPageWithExtra = (extra: Partial) => { - const { orderBy, tags, ...mergedFiltering } = { ...filtering, ...extra }; + const { orderBy, tags, excludeBots, ...mergedFiltering } = { ...filtering, ...extra }; const query: ShortUrlsQuery = { ...mergedFiltering, orderBy: orderBy && orderToString(orderBy), tags: tags.length > 0 ? tags.join(',') : undefined, + excludeBots: excludeBots === undefined ? undefined : parseBooleanToString(excludeBots), }; const stringifiedQuery = stringifyQuery(query); const queryString = isEmpty(stringifiedQuery) ? '' : `?${stringifiedQuery}`; diff --git a/src/utils/utils.ts b/src/utils/utils.ts index 8eda7256..504d450e 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -26,3 +26,7 @@ export const nonEmptyValueOrNull = (value: T): T | null => (isEmpty(value) ? export const capitalize = (value: T): string => `${value.charAt(0).toUpperCase()}${value.slice(1)}`; export const equals = (value: any) => (otherValue: any) => value === otherValue; + +export type BooleanString = 'true' | 'false'; + +export const parseBooleanToString = (value: boolean): BooleanString => (value ? 'true' : 'false'); diff --git a/src/visits/helpers/hooks.ts b/src/visits/helpers/hooks.ts index 54ccff47..fd647023 100644 --- a/src/visits/helpers/hooks.ts +++ b/src/visits/helpers/hooks.ts @@ -6,12 +6,13 @@ import { DateRange, datesToDateRange } from '../../utils/helpers/dateIntervals'; import { OrphanVisitType, VisitsFilter } from '../types'; import { parseQuery, stringifyQuery } from '../../utils/helpers/query'; import { formatIsoDate } from '../../utils/helpers/date'; +import { BooleanString } from '../../utils/utils'; interface VisitsQuery { startDate?: string; endDate?: string; orphanVisitsType?: OrphanVisitType; - excludeBots?: 'true' | 'false'; + excludeBots?: BooleanString; domain?: string; } diff --git a/test/short-urls/helpers/ShortUrlsRow.test.tsx b/test/short-urls/helpers/ShortUrlsRow.test.tsx index 9884b0e1..83b9b4f8 100644 --- a/test/short-urls/helpers/ShortUrlsRow.test.tsx +++ b/test/short-urls/helpers/ShortUrlsRow.test.tsx @@ -2,6 +2,7 @@ import { screen } from '@testing-library/react'; import { last } from 'ramda'; import { Mock } from 'ts-mockery'; import { addDays, formatISO, subDays } from 'date-fns'; +import { MemoryRouter, useLocation } from 'react-router-dom'; import { ShortUrlsRow as createShortUrlsRow } from '../../../src/short-urls/helpers/ShortUrlsRow'; import { TimeoutToggle } from '../../../src/utils/helpers/hooks'; import { ShortUrl, ShortUrlMeta } from '../../../src/short-urls/data'; @@ -19,6 +20,11 @@ interface SetUpOptions { settings?: Partial; } +jest.mock('react-router-dom', () => ({ + ...jest.requireActual('react-router-dom'), + useLocation: jest.fn().mockReturnValue({}), +})); + describe('', () => { const timeoutToggle = jest.fn(() => true); const useTimeoutToggle = jest.fn(() => [false, timeoutToggle]) as TimeoutToggle; @@ -43,18 +49,24 @@ describe('', () => { }, }; const ShortUrlsRow = createShortUrlsRow(() => ShortUrlsRowMenu, colorGeneratorMock, useTimeoutToggle); - const setUp = ({ title, tags = [], meta = {}, settings = {} }: SetUpOptions = {}) => renderWithEvents( - - - null} - settings={Mock.of(settings)} - /> - -
, - ); + + const setUp = ({ title, tags = [], meta = {}, settings = {} }: SetUpOptions = {}, search = '') => { + (useLocation as any).mockReturnValue({ search }); + return renderWithEvents( + + + + null} + settings={Mock.of(settings)} + /> + +
+
, + ); + }; it.each([ [null, 7], @@ -105,11 +117,17 @@ describe('', () => { }); it.each([ - [{}, shortUrl.visitsSummary?.total], - [Mock.of({ visits: { excludeBots: false } }), shortUrl.visitsSummary?.total], - [Mock.of({ visits: { excludeBots: true } }), shortUrl.visitsSummary?.nonBots], - ])('renders visits count in fifth row', (settings, expectedAmount) => { - setUp({ settings }); + [{}, '', shortUrl.visitsSummary?.total], + [Mock.of({ visits: { excludeBots: false } }), '', shortUrl.visitsSummary?.total], + [Mock.of({ visits: { excludeBots: true } }), '', shortUrl.visitsSummary?.nonBots], + [Mock.of({ visits: { excludeBots: false } }), 'excludeBots=true', shortUrl.visitsSummary?.nonBots], + [Mock.of({ visits: { excludeBots: true } }), 'excludeBots=true', shortUrl.visitsSummary?.nonBots], + [{}, 'excludeBots=true', shortUrl.visitsSummary?.nonBots], + [Mock.of({ visits: { excludeBots: true } }), 'excludeBots=false', shortUrl.visitsSummary?.total], + [Mock.of({ visits: { excludeBots: false } }), 'excludeBots=false', shortUrl.visitsSummary?.total], + [{}, 'excludeBots=false', shortUrl.visitsSummary?.total], + ])('renders visits count in fifth row', (settings, search, expectedAmount) => { + setUp({ settings }, search); expect(screen.getAllByRole('cell')[4]).toHaveTextContent(`${expectedAmount}`); }); From b00f6fadf806dc309a1bd7a47b5962c5063c73c8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 23 Dec 2022 20:03:27 +0100 Subject: [PATCH 7/9] Added test for parseBooleanToString --- test/utils/utils.test.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/utils/utils.test.ts b/test/utils/utils.test.ts index a092a1e5..d08a5fee 100644 --- a/test/utils/utils.test.ts +++ b/test/utils/utils.test.ts @@ -1,4 +1,4 @@ -import { capitalize, nonEmptyValueOrNull, rangeOf } from '../../src/utils/utils'; +import { capitalize, nonEmptyValueOrNull, parseBooleanToString, rangeOf } from '../../src/utils/utils'; describe('utils', () => { describe('rangeOf', () => { @@ -49,4 +49,13 @@ describe('utils', () => { expect(capitalize(value)).toEqual(expectedResult); }); }); + + describe('parseBooleanToString', () => { + it.each([ + [true, 'true'], + [false, 'false'], + ])('parses value as expected', (value, expectedResult) => { + expect(parseBooleanToString(value)).toEqual(expectedResult); + }); + }); }); From e790360de9624777da1ebf0d79b12a3e2b032130 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 23 Dec 2022 20:15:52 +0100 Subject: [PATCH 8/9] Added filtering dropdown to short URLs filtering bar --- src/short-urls/ShortUrlsFilteringBar.tsx | 30 ++++++++++++++----- src/short-urls/ShortUrlsList.tsx | 6 ++-- src/short-urls/helpers/ExportShortUrlsBtn.tsx | 2 +- .../short-urls/ShortUrlsFilteringBar.test.tsx | 20 +++++++++++++ 4 files changed, 47 insertions(+), 11 deletions(-) diff --git a/src/short-urls/ShortUrlsFilteringBar.tsx b/src/short-urls/ShortUrlsFilteringBar.tsx index b49280f7..fa7cb901 100644 --- a/src/short-urls/ShortUrlsFilteringBar.tsx +++ b/src/short-urls/ShortUrlsFilteringBar.tsx @@ -8,7 +8,7 @@ import { SearchField } from '../utils/SearchField'; import { DateRangeSelector } from '../utils/dates/DateRangeSelector'; import { formatIsoDate } from '../utils/helpers/date'; import { DateRange, datesToDateRange } from '../utils/helpers/dateIntervals'; -import { supportsAllTagsFiltering } from '../utils/helpers/features'; +import { supportsAllTagsFiltering, supportsBotVisits } from '../utils/helpers/features'; import { SelectedServer } from '../servers/data'; import { OrderDir } from '../utils/helpers/ordering'; import { OrderingDropdown } from '../utils/OrderingDropdown'; @@ -16,11 +16,14 @@ import { useShortUrlsQuery } from './helpers/hooks'; import { SHORT_URLS_ORDERABLE_FIELDS, ShortUrlsOrder, ShortUrlsOrderableFields } from './data'; import { ExportShortUrlsBtnProps } from './helpers/ExportShortUrlsBtn'; import { TagsSelectorProps } from '../tags/helpers/TagsSelector'; +import { ShortUrlsFilterDropdown } from './helpers/ShortUrlsFilterDropdown'; +import { Settings } from '../settings/reducers/settings'; import './ShortUrlsFilteringBar.scss'; interface ShortUrlsFilteringProps { selectedServer: SelectedServer; order: ShortUrlsOrder; + settings: Settings; handleOrderBy: (orderField?: ShortUrlsOrderableFields, orderDir?: OrderDir) => void; className?: string; shortUrlsAmount?: number; @@ -29,8 +32,8 @@ interface ShortUrlsFilteringProps { export const ShortUrlsFilteringBar = ( ExportShortUrlsBtn: FC, TagsSelector: FC, -): FC => ({ selectedServer, className, shortUrlsAmount, order, handleOrderBy }) => { - const [{ search, tags, startDate, endDate, tagsMode = 'any' }, toFirstPage] = useShortUrlsQuery(); +): FC => ({ selectedServer, className, shortUrlsAmount, order, handleOrderBy, settings }) => { + const [{ search, tags, startDate, endDate, excludeBots, tagsMode = 'any' }, toFirstPage] = useShortUrlsQuery(); const setDates = pipe( ({ startDate: theStartDate, endDate: theEndDate }: DateRange) => ({ startDate: formatIsoDate(theStartDate) ?? undefined, @@ -44,6 +47,7 @@ export const ShortUrlsFilteringBar = ( ); const changeTagSelection = (selectedTags: string[]) => toFirstPage({ tags: selectedTags }); const canChangeTagsMode = supportsAllTagsFiltering(selectedServer); + const botsSupported = supportsBotVisits(selectedServer); const toggleTagsMode = pipe( () => (tagsMode === 'any' ? 'all' : 'any'), (mode) => toFirstPage({ tagsMode: mode }), @@ -69,11 +73,21 @@ export const ShortUrlsFilteringBar = (
- +
+
+ +
+ +
diff --git a/src/short-urls/ShortUrlsList.tsx b/src/short-urls/ShortUrlsList.tsx index 60bfa759..a124368c 100644 --- a/src/short-urls/ShortUrlsList.tsx +++ b/src/short-urls/ShortUrlsList.tsx @@ -31,12 +31,13 @@ export const ShortUrlsList = ( const serverId = getServerId(selectedServer); const { page } = useParams(); const location = useLocation(); - const [{ tags, search, startDate, endDate, orderBy, tagsMode }, toFirstPage] = useShortUrlsQuery(); + const [{ tags, search, startDate, endDate, orderBy, tagsMode, excludeBots }, toFirstPage] = useShortUrlsQuery(); const [actualOrderBy, setActualOrderBy] = useState( // This separated state handling is needed to be able to fall back to settings value, but only once when loaded orderBy ?? settings.shortUrlsList?.defaultOrdering ?? DEFAULT_SHORT_URLS_ORDERING, ); const { pagination } = shortUrlsList?.shortUrls ?? {}; + const doExcludeBots = excludeBots ?? settings.visits?.excludeBots; const handleOrderBy = (field?: ShortUrlsOrderableFields, dir?: OrderDir) => { toFirstPage({ orderBy: { field, dir } }); setActualOrderBy({ field, dir }); @@ -50,7 +51,7 @@ export const ShortUrlsList = ( (updatedTags) => toFirstPage({ tags: updatedTags }), ); const parseOrderByForShlink = ({ field, dir }: ShortUrlsOrder): ShlinkShortUrlsOrder => { - if (supportsExcludeBotsOnShortUrls(selectedServer) && settings.visits?.excludeBots && field === 'visits') { + if (supportsExcludeBotsOnShortUrls(selectedServer) && doExcludeBots && field === 'visits') { return { field: 'nonBotVisits', dir }; } @@ -76,6 +77,7 @@ export const ShortUrlsList = ( shortUrlsAmount={shortUrlsList.shortUrls?.pagination.totalItems} order={actualOrderBy} handleOrderBy={handleOrderBy} + settings={settings} className="mb-3" /> diff --git a/src/short-urls/helpers/ExportShortUrlsBtn.tsx b/src/short-urls/helpers/ExportShortUrlsBtn.tsx index 5a5f1233..35403d1d 100644 --- a/src/short-urls/helpers/ExportShortUrlsBtn.tsx +++ b/src/short-urls/helpers/ExportShortUrlsBtn.tsx @@ -52,7 +52,7 @@ export const ExportShortUrlsBtn = ( longUrl: shortUrl.longUrl, title: shortUrl.title ?? '', tags: shortUrl.tags.join(','), - visits: shortUrl.visitsCount, + visits: shortUrl?.visitsSummary?.total ?? shortUrl.visitsCount, }))); stopLoading(); }; diff --git a/test/short-urls/ShortUrlsFilteringBar.test.tsx b/test/short-urls/ShortUrlsFilteringBar.test.tsx index d8f30942..ad366d98 100644 --- a/test/short-urls/ShortUrlsFilteringBar.test.tsx +++ b/test/short-urls/ShortUrlsFilteringBar.test.tsx @@ -4,6 +4,7 @@ import { endOfDay, formatISO, startOfDay } from 'date-fns'; import { MemoryRouter, useLocation, useNavigate } from 'react-router-dom'; import { ShortUrlsFilteringBar as filteringBarCreator } from '../../src/short-urls/ShortUrlsFilteringBar'; import { ReachableServer, SelectedServer } from '../../src/servers/data'; +import { Settings } from '../../src/settings/reducers/settings'; import { DateRange } from '../../src/utils/helpers/dateIntervals'; import { formatDate } from '../../src/utils/helpers/date'; import { renderWithEvents } from '../__helpers__/setUpTest'; @@ -30,6 +31,7 @@ describe('', () => { selectedServer={selectedServer ?? Mock.all()} order={{}} handleOrderBy={handleOrderBy} + settings={Mock.of({ visits: {} })} /> , ); @@ -114,6 +116,24 @@ describe('', () => { expect(navigate).toHaveBeenCalledWith(expect.stringContaining(expectedRedirectTagsMode)); }); + it.each([ + ['', 'excludeBots=true'], + ['excludeBots=false', 'excludeBots=true'], + ['excludeBots=true', 'excludeBots=false'], + ])('allows to toggle excluding bots through filtering dropdown', async (search, expectedQuery) => { + const { user } = setUp( + search, + Mock.of({ version: '3.4.0' }), + ); + const toggleBots = async (name = 'Exclude bots visits') => { + await user.click(screen.getByRole('button', { name: 'Filters' })); + await user.click(await screen.findByRole('menuitem', { name })); + }; + + await toggleBots(); + expect(navigate).toHaveBeenCalledWith(expect.stringContaining(expectedQuery)); + }); + it('handles order through dropdown', async () => { const { user } = setUp(); const clickMenuItem = async (name: string | RegExp) => { From ddb2c1e6410b1acf51e06ea199af6cae0f11f988 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 23 Dec 2022 20:24:55 +0100 Subject: [PATCH 9/9] Updated changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 825f3a2e..92ef8071 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased] ### Added * [#750](https://github.com/shlinkio/shlink-web-client/issues/750) Added new icon indicators telling if a short URL can be normally visited, it received the max amount of visits, is still not enabled, etc. +* [#764](https://github.com/shlinkio/shlink-web-client/issues/764) Added support to exclude visits from visits on short URLs list when consuming Shlink 3.4.0. + + This feature also comes with a new setting to disable visits from bots by default, both on short URLs lists and visits sections. ### Changed * *Nothing*