Enrues proper ordering is sent to shlink when ordering by visits

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<ShlinkShortUrlsOrderableFields>;
+
 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;
 }
 

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 (
     <>

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');

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('<ShortUrlsList />', () => {
     },
   });
   const ShortUrlsList = createShortUrlsList(ShortUrlsTable, ShortUrlsFilteringBar);
-  const setUp = (defaultOrdering: ShortUrlsOrder = {}) => renderWithEvents(
+  const setUp = (settings: Partial<Settings> = {}, version: SemVer = '3.0.0') => renderWithEvents(
     <MemoryRouter>
       <ShortUrlsList
         {...Mock.of<MercureBoundProps>({ mercureInfo: { loading: true } })}
         listShortUrls={listShortUrlsMock}
         shortUrlsList={shortUrlsList}
-        selectedServer={Mock.of<ReachableServer>({ id: '1' })}
+        selectedServer={Mock.of<ReachableServer>({ id: '1', version })}
-        settings={Mock.of<Settings>({ shortUrlsList: { defaultOrdering } })}
+        settings={Mock.of<Settings>(settings)}
       />
     </MemoryRouter>,
   );
@@ -82,11 +83,39 @@ describe('<ShortUrlsList />', () => {
   it.each([
     [Mock.of<ShortUrlsOrder>({ field: 'visits', dir: 'ASC' }), 'visits', 'ASC'],
     [Mock.of<ShortUrlsOrder>({ field: 'title', dir: 'DESC' }), 'title', 'DESC'],
-    [Mock.of<ShortUrlsOrder>(), undefined, undefined],
+    [Mock.all<ShortUrlsOrder>(), undefined, undefined],
-  ])('has expected initial ordering based on settings', (initialOrderBy, field, dir) => {
+  ])('has expected initial ordering based on settings', (defaultOrdering, field, dir) => {
-    setUp(initialOrderBy);
+    setUp({ shortUrlsList: { defaultOrdering } });
     expect(listShortUrlsMock).toHaveBeenCalledWith(expect.objectContaining({
       orderBy: { field, dir },
     }));
   });
+
+  it.each([
+    [Mock.of<Settings>({
+      shortUrlsList: {
+        defaultOrdering: { field: 'visits', dir: 'ASC' },
+      },
+    }), '3.3.0' as SemVer, { field: 'visits', dir: 'ASC' }],
+    [Mock.of<Settings>({
+      shortUrlsList: {
+        defaultOrdering: { field: 'visits', dir: 'ASC' },
+      },
+      visits: { excludeBots: true },
+    }), '3.3.0' as SemVer, { field: 'visits', dir: 'ASC' }],
+    [Mock.of<Settings>({
+      shortUrlsList: {
+        defaultOrdering: { field: 'visits', dir: 'ASC' },
+      },
+    }), '3.4.0' as SemVer, { field: 'visits', dir: 'ASC' }],
+    [Mock.of<Settings>({
+      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 }));
+  });
 });