Merge pull request #546 from acelaya-forks/feature/order-by-to-query

Feature/order by to query

CHANGELOG.md

@@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 * [#535](https://github.com/shlinkio/shlink-web-client/pull/535) Allowed editing default domain redirects when consuming Shlink 2.10 or newer.
 * [#531](https://github.com/shlinkio/shlink-web-client/pull/531) Added custom slug field to the basic creation form in the Overview page.
 * [#537](https://github.com/shlinkio/shlink-web-client/pull/537) Allowed to customize the ordering for every list in the app that supports it, being currently tags and short URLs.
+* [#542](https://github.com/shlinkio/shlink-web-client/pull/542) Added ordering for short URLs to the query, so that it is consistent with the rest of the filtering params.
 
 ### Changed
 * [#534](https://github.com/shlinkio/shlink-web-client/pull/534) Updated axios.

src/api/services/ShlinkApiClient.ts

@@ -19,17 +19,14 @@ import {
   ShlinkShortUrlsListNormalizedParams,
 } from '../types';
 import { stringifyQuery } from '../../utils/helpers/query';
+import { orderToString } from '../../utils/helpers/ordering';
 
 const buildShlinkBaseUrl = (url: string, apiVersion: number) => url ? `${url}/rest/v${apiVersion}` : '';
 const rejectNilProps = reject(isNil);
 const normalizeOrderByInParams = (params: ShlinkShortUrlsListParams): ShlinkShortUrlsListNormalizedParams => {
   const { orderBy = {}, ...rest } = params;
-  const { field, dir } = orderBy;
 
-  return !dir ? rest : {
-    ...rest,
-    orderBy: `${field}-${dir}`,
-  };
+  return { ...rest, orderBy: orderToString(orderBy) };
 };
 
 export default class ShlinkApiClient {

src/short-urls/SearchBar.scss

@@ -1,3 +0,0 @@
-.search-bar__tags-icon {
-  vertical-align: bottom;
-}

src/short-urls/ShortUrlsFilteringBar.scss

@@ -0,0 +1,3 @@
+.short-urls-filtering-bar__tags-icon {
+  vertical-align: bottom;
+}

src/short-urls/ShortUrlsFilteringBar.tsx

@@ -10,13 +10,13 @@ import { formatIsoDate } from '../utils/helpers/date';
 import ColorGenerator from '../utils/services/ColorGenerator';
 import { DateRange } from '../utils/dates/types';
 import { ShortUrlListRouteParams, useShortUrlsQuery } from './helpers/hooks';
-import './SearchBar.scss';
+import './ShortUrlsFilteringBar.scss';
 
-export type SearchBarProps = RouteChildrenProps<ShortUrlListRouteParams>;
+export type ShortUrlsFilteringProps = RouteChildrenProps<ShortUrlListRouteParams>;
 
 const dateOrNull = (date?: string) => date ? parseISO(date) : null;
 
-const SearchBar = (colorGenerator: ColorGenerator) => (props: SearchBarProps) => {
+const ShortUrlsFilteringBar = (colorGenerator: ColorGenerator) => (props: ShortUrlsFilteringProps) => {
   const [{ search, tags, startDate, endDate }, toFirstPage ] = useShortUrlsQuery(props);
   const selectedTags = tags?.split(',') ?? [];
   const setDates = pipe(
@@ -37,7 +37,7 @@ const SearchBar = (colorGenerator: ColorGenerator) => (props: SearchBarProps) =>
   );
 
   return (
-    <div className="search-bar-container">
+    <div className="short-urls-filtering-bar-container">
       <SearchField initialValue={search} onChange={setSearch} />
 
       <div className="mt-3">
@@ -56,8 +56,8 @@ const SearchBar = (colorGenerator: ColorGenerator) => (props: SearchBarProps) =>
       </div>
 
       {selectedTags.length > 0 && (
-        <h4 className="search-bar__selected-tag mt-3">
-          <FontAwesomeIcon icon={tagsIcon} className="search-bar__tags-icon" />
+        <h4 className="short-urls-filtering-bar__selected-tag mt-3">
+          <FontAwesomeIcon icon={tagsIcon} className="short-urls-filtering-bar__tags-icon" />
           &nbsp;
           {selectedTags.map((tag) =>
             <Tag colorGenerator={colorGenerator} key={tag} text={tag} clearable onClose={() => removeTag(tag)} />)}
@@ -67,4 +67,4 @@ const SearchBar = (colorGenerator: ColorGenerator) => (props: SearchBarProps) =>
   );
 };
 
-export default SearchBar;
+export default ShortUrlsFilteringBar;

src/short-urls/ShortUrlsList.tsx

@@ -14,7 +14,7 @@ import { ShortUrlsList as ShortUrlsListState } from './reducers/shortUrlsList';
 import { ShortUrlsTableProps } from './ShortUrlsTable';
 import Paginator from './Paginator';
 import { ShortUrlListRouteParams, useShortUrlsQuery } from './helpers/hooks';
-import { ShortUrlsOrderableFields, ShortUrlsOrder, SHORT_URLS_ORDERABLE_FIELDS } from './data';
+import { ShortUrlsOrderableFields, SHORT_URLS_ORDERABLE_FIELDS } from './data';
 
 interface ShortUrlsListProps extends RouteComponentProps<ShortUrlListRouteParams> {
   selectedServer: SelectedServer;
@@ -23,7 +23,7 @@ interface ShortUrlsListProps extends RouteComponentProps<ShortUrlListRouteParams
   settings: Settings;
 }
 
-const ShortUrlsList = (ShortUrlsTable: FC<ShortUrlsTableProps>, SearchBar: FC) => boundToMercureHub(({
+const ShortUrlsList = (ShortUrlsTable: FC<ShortUrlsTableProps>, ShortUrlsFilteringBar: FC) => boundToMercureHub(({
   listShortUrls,
   match,
   location,
@@ -33,16 +33,21 @@ const ShortUrlsList = (ShortUrlsTable: FC<ShortUrlsTableProps>, SearchBar: FC) =
   settings,
 }: ShortUrlsListProps) => {
   const serverId = getServerId(selectedServer);
-  const initialOrderBy = settings.shortUrlsList?.defaultOrdering ?? DEFAULT_SHORT_URLS_ORDERING;
-  const [ order, setOrder ] = useState<ShortUrlsOrder>(initialOrderBy);
-  const [{ tags, search, startDate, endDate }, toFirstPage ] = useShortUrlsQuery({ history, match, location });
+  const [{ tags, search, startDate, endDate, orderBy }, toFirstPage ] = useShortUrlsQuery({ history, match, location });
+  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 selectedTags = useMemo(() => tags?.split(',') ?? [], [ tags ]);
   const { pagination } = shortUrlsList?.shortUrls ?? {};
-
-  const handleOrderBy = (field?: ShortUrlsOrderableFields, dir?: OrderDir) => setOrder({ field, dir });
+  const handleOrderBy = (field?: ShortUrlsOrderableFields, dir?: OrderDir) => {
+    toFirstPage({ orderBy: { field, dir } });
+    setActualOrderBy({ field, dir });
+  };
   const orderByColumn = (field: ShortUrlsOrderableFields) => () =>
-    handleOrderBy(field, determineOrderDir(field, order.field, order.dir));
-  const renderOrderIcon = (field: ShortUrlsOrderableFields) => <TableOrderIcon currentOrder={order} field={field} />;
+    handleOrderBy(field, determineOrderDir(field, actualOrderBy.field, actualOrderBy.dir));
+  const renderOrderIcon = (field: ShortUrlsOrderableFields) =>
+    <TableOrderIcon currentOrder={actualOrderBy} field={field} />;
   const addTag = pipe(
     (newTag: string) => [ ...new Set([ ...selectedTags, newTag ]) ].join(','),
     (tags) => toFirstPage({ tags }),
@@ -53,18 +58,17 @@ const ShortUrlsList = (ShortUrlsTable: FC<ShortUrlsTableProps>, SearchBar: FC) =
       page: match.params.page,
       searchTerm: search,
       tags: selectedTags,
-      itemsPerPage: undefined,
       startDate,
       endDate,
-      orderBy: order,
+      orderBy: actualOrderBy,
     });
-  }, [ match.params.page, search, selectedTags, startDate, endDate, order ]);
+  }, [ match.params.page, search, selectedTags, startDate, endDate, actualOrderBy ]);
 
   return (
     <>
-      <div className="mb-3"><SearchBar /></div>
+      <div className="mb-3"><ShortUrlsFilteringBar /></div>
       <div className="d-block d-lg-none mb-3">
-        <OrderingDropdown items={SHORT_URLS_ORDERABLE_FIELDS} order={order} onChange={handleOrderBy} />
+        <OrderingDropdown items={SHORT_URLS_ORDERABLE_FIELDS} order={actualOrderBy} onChange={handleOrderBy} />
       </div>
       <Card body className="pb-1">
         <ShortUrlsTable

src/short-urls/helpers/hooks.ts

@@ -1,27 +1,50 @@
 import { RouteChildrenProps } from 'react-router-dom';
 import { useMemo } from 'react';
-import { isEmpty } from 'ramda';
+import { isEmpty, pipe } from 'ramda';
 import { parseQuery, stringifyQuery } from '../../utils/helpers/query';
+import { ShortUrlsOrder, ShortUrlsOrderableFields } from '../data';
+import { orderToString, stringToOrder } from '../../utils/helpers/ordering';
 
 type ServerIdRouteProps = RouteChildrenProps<{ serverId: string }>;
-type ToFirstPage = (extra: Partial<ShortUrlsQuery>) => void;
+type ToFirstPage = (extra: Partial<ShortUrlsFiltering>) => void;
 
 export interface ShortUrlListRouteParams {
   page: string;
   serverId: string;
 }
 
-interface ShortUrlsQuery {
+interface ShortUrlsQueryCommon {
   tags?: string;
   search?: string;
   startDate?: string;
   endDate?: string;
 }
 
-export const useShortUrlsQuery = ({ history, location, match }: ServerIdRouteProps): [ShortUrlsQuery, ToFirstPage] => {
-  const query = useMemo(() => parseQuery<ShortUrlsQuery>(location.search), [ location ]);
-  const toFirstPageWithExtra = (extra: Partial<ShortUrlsQuery>) => {
-    const evolvedQuery = stringifyQuery({ ...query, ...extra });
+interface ShortUrlsQuery extends ShortUrlsQueryCommon {
+  orderBy?: string;
+}
+
+interface ShortUrlsFiltering extends ShortUrlsQueryCommon {
+  orderBy?: ShortUrlsOrder;
+}
+
+export const useShortUrlsQuery = (
+  { history, location, match }: ServerIdRouteProps,
+): [ShortUrlsFiltering, ToFirstPage] => {
+  const query = useMemo(
+    pipe(
+      () => parseQuery<ShortUrlsQuery>(location.search),
+      ({ orderBy, ...rest }: ShortUrlsQuery): ShortUrlsFiltering => !orderBy ? rest : {
+        ...rest,
+        orderBy: stringToOrder<ShortUrlsOrderableFields>(orderBy),
+      },
+    ),
+    [ location.search ],
+  );
+  const toFirstPageWithExtra = (extra: Partial<ShortUrlsFiltering>) => {
+    const { orderBy, ...mergedQuery } = { ...query, ...extra };
+    const normalizedQuery: ShortUrlsQuery = { ...mergedQuery, orderBy: orderBy && orderToString(orderBy) };
+    const evolvedQuery = stringifyQuery(normalizedQuery);
     const queryString = isEmpty(evolvedQuery) ? '' : `?${evolvedQuery}`;
 
     history.push(`/server/${match?.params.serverId}/list-short-urls/1${queryString}`);

src/short-urls/services/provideServices.ts

@@ -1,5 +1,5 @@
 import Bottle, { Decorator } from 'bottlejs';
-import SearchBar from '../SearchBar';
+import ShortUrlsFilteringBar from '../ShortUrlsFilteringBar';
 import ShortUrlsList from '../ShortUrlsList';
 import ShortUrlsRow from '../helpers/ShortUrlsRow';
 import ShortUrlsRowMenu from '../helpers/ShortUrlsRowMenu';
@@ -19,7 +19,7 @@ import { getShortUrlDetail } from '../reducers/shortUrlDetail';
 
 const provideServices = (bottle: Bottle, connect: ConnectDecorator, withRouter: Decorator) => {
   // Components
-  bottle.serviceFactory('ShortUrlsList', ShortUrlsList, 'ShortUrlsTable', 'SearchBar');
+  bottle.serviceFactory('ShortUrlsList', ShortUrlsList, 'ShortUrlsTable', 'ShortUrlsFilteringBar');
   bottle.decorator('ShortUrlsList', connect(
     [ 'selectedServer', 'mercureInfo', 'shortUrlsList', 'settings' ],
     [ 'listShortUrls', 'createNewVisits', 'loadMercureInfo' ],
@@ -50,8 +50,8 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator, withRouter:
   bottle.decorator('QrCodeModal', connect([ 'selectedServer' ]));
 
   // Services
-  bottle.serviceFactory('SearchBar', SearchBar, 'ColorGenerator');
-  bottle.decorator('SearchBar', withRouter);
+  bottle.serviceFactory('ShortUrlsFilteringBar', ShortUrlsFilteringBar, 'ColorGenerator');
+  bottle.decorator('ShortUrlsFilteringBar', withRouter);
 
   // Actions
   bottle.serviceFactory('listShortUrls', listShortUrls, 'buildShlinkApiClient');

src/utils/helpers/ordering.ts

@@ -30,3 +30,12 @@ export const sortList = <List>(list: List[], { field, dir }: Order<Partial<keyof
 
     return a[field] > b[field] ? greaterThan : smallerThan;
   });
+
+export const orderToString = <T>(order: Order<T>): string | undefined =>
+  order.dir ? `${order.field}-${order.dir}` : undefined;
+
+export const stringToOrder = <T>(order: string): Order<T> => {
+  const [ field, dir ] = order.split('-') as [ T | undefined, OrderDir | undefined ];
+
+  return { field, dir };
+};

test/servers/ManageServers.test.tsx

@@ -33,20 +33,20 @@ describe('<ManageServers />', () => {
       bar: createServerMock('bar'),
       baz: createServerMock('baz'),
     });
-    const searchBar = wrapper.find(SearchField);
+    const searchField = wrapper.find(SearchField);
 
     expect(wrapper.find(ManageServersRow)).toHaveLength(3);
     expect(wrapper.find('tbody').find('tr')).toHaveLength(0);
 
-    searchBar.simulate('change', 'foo');
+    searchField.simulate('change', 'foo');
     expect(wrapper.find(ManageServersRow)).toHaveLength(1);
     expect(wrapper.find('tbody').find('tr')).toHaveLength(0);
 
-    searchBar.simulate('change', 'ba');
+    searchField.simulate('change', 'ba');
     expect(wrapper.find(ManageServersRow)).toHaveLength(2);
     expect(wrapper.find('tbody').find('tr')).toHaveLength(0);
 
-    searchBar.simulate('change', 'invalid');
+    searchField.simulate('change', 'invalid');
     expect(wrapper.find(ManageServersRow)).toHaveLength(0);
     expect(wrapper.find('tbody').find('tr')).toHaveLength(1);
   });

test/short-urls/SearchBar.test.tsx

@@ -3,21 +3,21 @@ import { Mock } from 'ts-mockery';
 import { History, Location } from 'history';
 import { match } from 'react-router';
 import { formatISO } from 'date-fns';
-import searchBarCreator, { SearchBarProps } from '../../src/short-urls/SearchBar';
+import filteringBarCreator, { ShortUrlsFilteringProps } from '../../src/short-urls/ShortUrlsFilteringBar';
 import SearchField from '../../src/utils/SearchField';
 import Tag from '../../src/tags/helpers/Tag';
 import { DateRangeSelector } from '../../src/utils/dates/DateRangeSelector';
 import ColorGenerator from '../../src/utils/services/ColorGenerator';
 import { ShortUrlListRouteParams } from '../../src/short-urls/helpers/hooks';
 
-describe('<SearchBar />', () => {
+describe('<ShortUrlsFilteringBar />', () => {
   let wrapper: ShallowWrapper;
-  const SearchBar = searchBarCreator(Mock.all<ColorGenerator>());
+  const ShortUrlsFilteringBar = filteringBarCreator(Mock.all<ColorGenerator>());
   const push = jest.fn();
   const now = new Date();
-  const createWrapper = (props: Partial<SearchBarProps> = {}) => {
+  const createWrapper = (props: Partial<ShortUrlsFilteringProps> = {}) => {
     wrapper = shallow(
-      <SearchBar
+      <ShortUrlsFilteringBar
         history={Mock.of<History>({ push })}
         location={Mock.of<Location>({ search: '' })}
         match={Mock.of<match<ShortUrlListRouteParams>>({ params: { serverId: '1' } })}

test/short-urls/ShortUrlsList.test.tsx

@@ -16,7 +16,7 @@ import { Settings } from '../../src/settings/reducers/settings';
 describe('<ShortUrlsList />', () => {
   let wrapper: ShallowWrapper;
   const ShortUrlsTable = () => null;
-  const SearchBar = () => null;
+  const ShortUrlsFilteringBar = () => null;
   const listShortUrlsMock = jest.fn();
   const push = jest.fn();
   const shortUrlsList = Mock.of<ShortUrlsListModel>({
@@ -31,7 +31,7 @@ describe('<ShortUrlsList />', () => {
       ],
     },
   });
-  const ShortUrlsList = shortUrlsListCreator(ShortUrlsTable, SearchBar);
+  const ShortUrlsList = shortUrlsListCreator(ShortUrlsTable, ShortUrlsFilteringBar);
   const createWrapper = (defaultOrdering: ShortUrlsOrder = {}) => shallow(
     <ShortUrlsList
       {...Mock.of<MercureBoundProps>({ mercureInfo: { loading: true } })}
@@ -56,7 +56,7 @@ describe('<ShortUrlsList />', () => {
     expect(wrapper.find(ShortUrlsTable)).toHaveLength(1);
     expect(wrapper.find(OrderingDropdown)).toHaveLength(1);
     expect(wrapper.find(Paginator)).toHaveLength(1);
-    expect(wrapper.find(SearchBar)).toHaveLength(1);
+    expect(wrapper.find(ShortUrlsFilteringBar)).toHaveLength(1);
   });
 
   it('passes current query to paginator', () => {

test/utils/helpers/date.test.ts

@@ -34,7 +34,7 @@ describe('date', () => {
   });
 
   describe('isBetween', () => {
-    test.each([
+    it.each([
       [ now, undefined, undefined, true ],
       [ now, subDays(now, 1), undefined, true ],
       [ now, now, undefined, true ],
@@ -52,7 +52,7 @@ describe('date', () => {
   });
 
   describe('isBeforeOrEqual', () => {
-    test.each([
+    it.each([
       [ now, now, true ],
       [ now, addDays(now, 1), true ],
       [ now, subDays(now, 1), false ],

test/utils/helpers/ordering.test.ts

@@ -1,4 +1,4 @@
-import { determineOrderDir } from '../../../src/utils/helpers/ordering';
+import { determineOrderDir, OrderDir, orderToString, stringToOrder } from '../../../src/utils/helpers/ordering';
 
 describe('ordering', () => {
   describe('determineOrderDir', () => {
@@ -22,4 +22,24 @@ describe('ordering', () => {
       expect(determineOrderDir('bar', 'bar', 'DESC')).toBeUndefined();
     });
   });
+
+  describe('orderToString', () => {
+    it.each([
+      [{}, undefined ],
+      [{ field: 'foo' }, undefined ],
+      [{ field: 'foo', dir: 'ASC' as OrderDir }, 'foo-ASC' ],
+      [{ field: 'bar', dir: 'DESC' as OrderDir }, 'bar-DESC' ],
+    ])('casts the order to string', (order, expectedResult) => {
+      expect(orderToString(order)).toEqual(expectedResult);
+    });
+  });
+
+  describe('stringToOrder', () => {
+    it.each([
+      [ 'foo-ASC', { field: 'foo', dir: 'ASC' }],
+      [ 'bar-DESC', { field: 'bar', dir: 'DESC' }],
+    ])('casts a string to an order objects', (order, expectedResult) => {
+      expect(stringToOrder(order)).toEqual(expectedResult);
+    });
+  });
 });