From b9285fd600e5c7584bf7742eda5a8ad6630954b1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 22 Dec 2022 18:07:44 +0100 Subject: [PATCH] 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 })); + }); });