From fe81e023e8a8fd5abe7260a987c863b89fd9f4eb Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Nov 2021 22:34:29 +0100 Subject: [PATCH 01/11] Moved table sorting icon to its own component wrapping the logic --- src/short-urls/ShortUrlsList.tsx | 6 ++---- src/tags/TagsTable.tsx | 13 ++++++------- src/utils/table/TableOrderIcon.tsx | 19 +++++++++++++++++++ src/visits/VisitsTable.tsx | 16 ++++------------ test/short-urls/ShortUrlsList.test.tsx | 8 ++++---- 5 files changed, 35 insertions(+), 27 deletions(-) create mode 100644 src/utils/table/TableOrderIcon.tsx diff --git a/src/short-urls/ShortUrlsList.tsx b/src/short-urls/ShortUrlsList.tsx index 8f3ed9e2..7c8fcda4 100644 --- a/src/short-urls/ShortUrlsList.tsx +++ b/src/short-urls/ShortUrlsList.tsx @@ -1,5 +1,3 @@ -import { faCaretDown as caretDownIcon, faCaretUp as caretUpIcon } from '@fortawesome/free-solid-svg-icons'; -import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import { head, keys, values } from 'ramda'; import { FC, useEffect, useState } from 'react'; import { RouteComponentProps } from 'react-router'; @@ -10,6 +8,7 @@ import { getServerId, SelectedServer } from '../servers/data'; import { boundToMercureHub } from '../mercure/helpers/boundToMercureHub'; import { parseQuery } from '../utils/helpers/query'; import { Topics } from '../mercure/helpers/Topics'; +import { TableOrderIcon } from '../utils/table/TableOrderIcon'; import { ShortUrlsList as ShortUrlsListState } from './reducers/shortUrlsList'; import { OrderableFields, ShortUrlsListParams, SORTABLE_FIELDS } from './reducers/shortUrlsListParams'; import { ShortUrlsTableProps } from './ShortUrlsTable'; @@ -52,8 +51,7 @@ const ShortUrlsList = (ShortUrlsTable: FC) => boundToMercur }; const orderByColumn = (field: OrderableFields) => () => handleOrderBy(field, determineOrderDir(field, order.field, order.dir)); - const renderOrderIcon = (field: OrderableFields) => order.dir && order.field === field && - ; + const renderOrderIcon = (field: OrderableFields) => ; useEffect(() => { const { tag } = parseQuery<{ tag?: string }>(location.search); diff --git a/src/tags/TagsTable.tsx b/src/tags/TagsTable.tsx index e313ebd4..cf24542c 100644 --- a/src/tags/TagsTable.tsx +++ b/src/tags/TagsTable.tsx @@ -1,12 +1,11 @@ import { FC, useEffect, useRef } from 'react'; import { splitEvery } from 'ramda'; -import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; -import { faCaretDown as caretDownIcon, faCaretUp as caretUpIcon } from '@fortawesome/free-solid-svg-icons'; import { RouteChildrenProps } from 'react-router'; import { SimpleCard } from '../utils/SimpleCard'; import SimplePaginator from '../common/SimplePaginator'; import { useQueryState } from '../utils/helpers/hooks'; import { parseQuery } from '../utils/helpers/query'; +import { TableOrderIcon } from '../utils/table/TableOrderIcon'; import { OrderableFields, TagsListChildrenProps, TagsOrder } from './data/TagsListChildrenProps'; import { TagsTableRowProps } from './TagsTableRow'; import './TagsTable.scss'; @@ -27,8 +26,6 @@ export const TagsTable = (TagsTableRow: FC) => ( const pages = splitEvery(TAGS_PER_PAGE, sortedTags); const showPaginator = pages.length > 1; const currentPage = pages[page - 1] ?? []; - const renderOrderIcon = (field: OrderableFields) => currentOrder.dir && currentOrder.field === field && - ; useEffect(() => { !isFirstLoad.current && setPage(1); @@ -43,12 +40,14 @@ export const TagsTable = (TagsTableRow: FC) => ( - + diff --git a/src/utils/table/TableOrderIcon.tsx b/src/utils/table/TableOrderIcon.tsx new file mode 100644 index 00000000..00434aad --- /dev/null +++ b/src/utils/table/TableOrderIcon.tsx @@ -0,0 +1,19 @@ +import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; +import { faCaretDown as caretDownIcon, faCaretUp as caretUpIcon } from '@fortawesome/free-solid-svg-icons'; +import { Order } from '../helpers/ordering'; + +interface TableOrderIconProps { + currentOrder: Order; + field: T; + className?: string; +} + +export function TableOrderIcon( + { currentOrder, field, className = 'ml-1' }: TableOrderIconProps, +) { + if (!currentOrder.dir || currentOrder.field !== field) { + return null; + } + + return ; +} diff --git a/src/visits/VisitsTable.tsx b/src/visits/VisitsTable.tsx index 202f1357..842eac34 100644 --- a/src/visits/VisitsTable.tsx +++ b/src/visits/VisitsTable.tsx @@ -1,12 +1,7 @@ import { useEffect, useMemo, useState, useRef } from 'react'; import classNames from 'classnames'; import { min, splitEvery } from 'ramda'; -import { - faCaretDown as caretDownIcon, - faCaretUp as caretUpIcon, - faCheck as checkIcon, - faRobot as botIcon, -} from '@fortawesome/free-solid-svg-icons'; +import { faCheck as checkIcon, faRobot as botIcon } from '@fortawesome/free-solid-svg-icons'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import { UncontrolledTooltip } from 'reactstrap'; import SimplePaginator from '../common/SimplePaginator'; @@ -16,6 +11,7 @@ import { prettify } from '../utils/helpers/numbers'; import { supportsBotVisits } from '../utils/helpers/features'; import { SelectedServer } from '../servers/data'; import { Time } from '../utils/Time'; +import { TableOrderIcon } from '../utils/table/TableOrderIcon'; import { NormalizedOrphanVisit, NormalizedVisit } from './types'; import './VisitsTable.scss'; @@ -72,12 +68,8 @@ const VisitsTable = ({ const orderByColumn = (field: OrderableFields) => () => setOrder({ field, dir: determineOrderDir(field, order.field, order.dir) }); - const renderOrderIcon = (field: OrderableFields) => order.dir && order.field === field && ( - - ); + const renderOrderIcon = (field: OrderableFields) => + ; useEffect(() => { const listener = () => setIsMobileDevice(matchMobile()); diff --git a/test/short-urls/ShortUrlsList.test.tsx b/test/short-urls/ShortUrlsList.test.tsx index e000a09f..3524e3b7 100644 --- a/test/short-urls/ShortUrlsList.test.tsx +++ b/test/short-urls/ShortUrlsList.test.tsx @@ -77,15 +77,15 @@ describe('', () => { it('invokes order icon rendering', () => { const renderIcon = (field: OrderableFields) => - (wrapper.find(ShortUrlsTable).prop('renderOrderIcon') as (field: OrderableFields) => ReactElement | null)(field); + (wrapper.find(ShortUrlsTable).prop('renderOrderIcon') as (field: OrderableFields) => ReactElement)(field); - expect(renderIcon('visits')).toEqual(undefined); + expect(renderIcon('visits').props.currentOrder).toEqual({}); wrapper.find(SortingDropdown).simulate('change', 'visits'); - expect(renderIcon('visits')).toEqual(undefined); + expect(renderIcon('visits').props.currentOrder).toEqual({ field: 'visits' }); wrapper.find(SortingDropdown).simulate('change', 'visits', 'ASC'); - expect(renderIcon('visits')).not.toEqual(undefined); + expect(renderIcon('visits').props.currentOrder).toEqual({ field: 'visits', dir: 'ASC' }); }); it('handles order by through table', () => { From 303900756dd7ed5c3a071f65a00cae379a7f4ec9 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Nov 2021 22:46:40 +0100 Subject: [PATCH 02/11] Added TableOrderIcon test --- test/utils/table/TableOrderIcon.test.tsx | 47 ++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 test/utils/table/TableOrderIcon.test.tsx diff --git a/test/utils/table/TableOrderIcon.test.tsx b/test/utils/table/TableOrderIcon.test.tsx new file mode 100644 index 00000000..2d0c4815 --- /dev/null +++ b/test/utils/table/TableOrderIcon.test.tsx @@ -0,0 +1,47 @@ +import { shallow, ShallowWrapper } from 'enzyme'; +import { faCaretDown as caretDownIcon, faCaretUp as caretUpIcon } from '@fortawesome/free-solid-svg-icons'; +import { TableOrderIcon } from '../../../src/utils/table/TableOrderIcon'; +import { OrderDir } from '../../../src/utils/helpers/ordering'; + +describe('', () => { + let wrapper: ShallowWrapper; + const createWrapper = (field: string, currentDir?: OrderDir, className?: string) => { + wrapper = shallow( + , + ); + + return wrapper; + }; + + afterEach(() => wrapper?.unmount()); + + it.each([ + [ 'foo', undefined ], + [ 'bar', 'DESC' as OrderDir ], + [ 'bar', 'ASC' as OrderDir ], + ])('renders empty when not all conditions are met', (field, dir) => { + const wrapper = createWrapper(field, dir); + + expect(wrapper.html()).toEqual(null); + }); + + it.each([ + [ 'DESC' as OrderDir, caretDownIcon ], + [ 'ASC' as OrderDir, caretUpIcon ], + ])('renders an icon when all conditions are met', (dir, expectedIcon) => { + const wrapper = createWrapper('foo', dir); + + expect(wrapper.html()).not.toEqual(null); + expect(wrapper.prop('icon')).toEqual(expectedIcon); + }); + + it.each([ + [ undefined, 'ml-1' ], + [ 'foo', 'foo' ], + [ 'bar', 'bar' ], + ])('renders expected classname', (className, expectedClassName) => { + const wrapper = createWrapper('foo', 'ASC', className); + + expect(wrapper.prop('className')).toEqual(expectedClassName); + }); +}); From 109baef82881dfc8766a7cbefbcbdbe0aa69a11f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 7 Nov 2021 11:03:31 +0100 Subject: [PATCH 03/11] Minor changes on tags filtering for short URLs --- src/common/MenuLayout.tsx | 4 ++-- src/common/services/provideServices.ts | 2 +- src/container/index.ts | 2 +- src/servers/Overview.tsx | 2 +- src/short-urls/ShortUrls.tsx | 23 --------------------- src/short-urls/services/provideServices.ts | 13 +++++------- src/tags/TagCard.tsx | 2 +- src/tags/TagsTableRow.tsx | 2 +- src/utils/SearchField.tsx | 5 +++-- test/short-urls/ShortUrls.test.tsx | 24 ---------------------- test/tags/TagCard.test.tsx | 6 +++--- test/tags/TagsTableRow.test.tsx | 2 +- 12 files changed, 19 insertions(+), 68 deletions(-) delete mode 100644 src/short-urls/ShortUrls.tsx delete mode 100644 test/short-urls/ShortUrls.test.tsx diff --git a/src/common/MenuLayout.tsx b/src/common/MenuLayout.tsx index 6672583e..b8205b2f 100644 --- a/src/common/MenuLayout.tsx +++ b/src/common/MenuLayout.tsx @@ -13,7 +13,7 @@ import './MenuLayout.scss'; const MenuLayout = ( TagsList: FC, - ShortUrls: FC, + ShortUrlsList: FC, AsideMenu: FC, CreateShortUrl: FC, ShortUrlVisits: FC, @@ -49,7 +49,7 @@ const MenuLayout = ( - + diff --git a/src/common/services/provideServices.ts b/src/common/services/provideServices.ts index ab4ab1cd..b714c8d5 100644 --- a/src/common/services/provideServices.ts +++ b/src/common/services/provideServices.ts @@ -35,7 +35,7 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator, withRouter: 'MenuLayout', MenuLayout, 'TagsList', - 'ShortUrls', + 'ShortUrlsList', 'AsideMenu', 'CreateShortUrl', 'ShortUrlVisits', diff --git a/src/container/index.ts b/src/container/index.ts index c7b50040..ce39f10e 100644 --- a/src/container/index.ts +++ b/src/container/index.ts @@ -36,7 +36,7 @@ const connect: ConnectDecorator = (propsFromState: string[] | null, actionServic provideAppServices(bottle, connect); provideCommonServices(bottle, connect, withRouter); provideApiServices(bottle); -provideShortUrlsServices(bottle, connect); +provideShortUrlsServices(bottle, connect, withRouter); provideServersServices(bottle, connect, withRouter); provideTagsServices(bottle, connect); provideVisitsServices(bottle, connect); diff --git a/src/servers/Overview.tsx b/src/servers/Overview.tsx index 806e218a..f85ed23f 100644 --- a/src/servers/Overview.tsx +++ b/src/servers/Overview.tsx @@ -107,7 +107,7 @@ export const Overview = ( shortUrlsList={shortUrlsList} selectedServer={selectedServer} className="mb-0" - onTagClick={(tag) => history.push(`/server/${serverId}/list-short-urls/1?tag=${tag}`)} + onTagClick={(tag) => history.push(`/server/${serverId}/list-short-urls/1?tags=${encodeURIComponent(tag)}`)} /> diff --git a/src/short-urls/ShortUrls.tsx b/src/short-urls/ShortUrls.tsx deleted file mode 100644 index 13814837..00000000 --- a/src/short-urls/ShortUrls.tsx +++ /dev/null @@ -1,23 +0,0 @@ -import { FC, useEffect, useState } from 'react'; -import { ShortUrlsListProps } from './ShortUrlsList'; - -const ShortUrls = (SearchBar: FC, ShortUrlsList: FC) => (props: ShortUrlsListProps) => { - const { match } = props; - const { page = '1', serverId = '' } = match?.params ?? {}; - const [ urlsListKey, setUrlsListKey ] = useState(`${serverId}_${page}`); - - // Using a key on a component makes react to create a new instance every time the key changes - // Without it, pagination on the URL will not make the component to be refreshed - useEffect(() => { - setUrlsListKey(`${serverId}_${page}`); - }, [ serverId, page ]); - - return ( - <> -
- - - ); -}; - -export default ShortUrls; diff --git a/src/short-urls/services/provideServices.ts b/src/short-urls/services/provideServices.ts index 48541144..197c6941 100644 --- a/src/short-urls/services/provideServices.ts +++ b/src/short-urls/services/provideServices.ts @@ -1,5 +1,4 @@ -import Bottle from 'bottlejs'; -import ShortUrls from '../ShortUrls'; +import Bottle, { Decorator } from 'bottlejs'; import SearchBar from '../SearchBar'; import ShortUrlsList from '../ShortUrlsList'; import ShortUrlsRow from '../helpers/ShortUrlsRow'; @@ -19,14 +18,11 @@ import { ShortUrlForm } from '../ShortUrlForm'; import { EditShortUrl } from '../EditShortUrl'; import { getShortUrlDetail } from '../reducers/shortUrlDetail'; -const provideServices = (bottle: Bottle, connect: ConnectDecorator) => { +const provideServices = (bottle: Bottle, connect: ConnectDecorator, withRouter: Decorator) => { // Components - bottle.serviceFactory('ShortUrls', ShortUrls, 'SearchBar', 'ShortUrlsList'); - bottle.decorator('ShortUrls', connect([ 'shortUrlsList' ])); - - bottle.serviceFactory('ShortUrlsList', ShortUrlsList, 'ShortUrlsTable'); + bottle.serviceFactory('ShortUrlsList', ShortUrlsList, 'ShortUrlsTable', 'SearchBar'); bottle.decorator('ShortUrlsList', connect( - [ 'selectedServer', 'shortUrlsListParams', 'mercureInfo' ], + [ 'selectedServer', 'shortUrlsListParams', 'mercureInfo', 'shortUrlsList' ], [ 'listShortUrls', 'resetShortUrlParams', 'createNewVisits', 'loadMercureInfo' ], )); @@ -57,6 +53,7 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator) => { // Services bottle.serviceFactory('SearchBar', SearchBar, 'ColorGenerator'); bottle.decorator('SearchBar', connect([ 'shortUrlsListParams' ], [ 'listShortUrls' ])); + bottle.decorator('SearchBar', withRouter); // Actions bottle.serviceFactory('listShortUrls', listShortUrls, 'buildShlinkApiClient'); diff --git a/src/tags/TagCard.tsx b/src/tags/TagCard.tsx index 482a998c..3d233a65 100644 --- a/src/tags/TagCard.tsx +++ b/src/tags/TagCard.tsx @@ -61,7 +61,7 @@ const TagCard = ( Short URLs diff --git a/src/tags/TagsTableRow.tsx b/src/tags/TagsTableRow.tsx index c030e4f8..b10ce318 100644 --- a/src/tags/TagsTableRow.tsx +++ b/src/tags/TagsTableRow.tsx @@ -32,7 +32,7 @@ export const TagsTableRow = ( {tag.tag} diff --git a/src/utils/SearchField.tsx b/src/utils/SearchField.tsx index c373ffc4..e76fa1ee 100644 --- a/src/utils/SearchField.tsx +++ b/src/utils/SearchField.tsx @@ -12,10 +12,11 @@ interface SearchFieldProps { className?: string; large?: boolean; noBorder?: boolean; + initialValue?: string; } -const SearchField = ({ onChange, className, large = true, noBorder = false }: SearchFieldProps) => { - const [ searchTerm, setSearchTerm ] = useState(''); +const SearchField = ({ onChange, className, large = true, noBorder = false, initialValue = '' }: SearchFieldProps) => { + const [ searchTerm, setSearchTerm ] = useState(initialValue); const resetTimer = () => { timer && clearTimeout(timer); diff --git a/test/short-urls/ShortUrls.test.tsx b/test/short-urls/ShortUrls.test.tsx deleted file mode 100644 index f4caed88..00000000 --- a/test/short-urls/ShortUrls.test.tsx +++ /dev/null @@ -1,24 +0,0 @@ -import { shallow, ShallowWrapper } from 'enzyme'; -import { Mock } from 'ts-mockery'; -import shortUrlsCreator from '../../src/short-urls/ShortUrls'; -import { ShortUrlsListProps } from '../../src/short-urls/ShortUrlsList'; - -describe('', () => { - let wrapper: ShallowWrapper; - const SearchBar = () => null; - const ShortUrlsList = () => null; - - beforeEach(() => { - const ShortUrls = shortUrlsCreator(SearchBar, ShortUrlsList); - - wrapper = shallow( - ()} />, - ); - }); - afterEach(() => wrapper.unmount()); - - it('wraps a SearchBar and ShortUrlsList', () => { - expect(wrapper.find(SearchBar)).toHaveLength(1); - expect(wrapper.find(ShortUrlsList)).toHaveLength(1); - }); -}); diff --git a/test/tags/TagCard.test.tsx b/test/tags/TagCard.test.tsx index a3c96b75..87a2c021 100644 --- a/test/tags/TagCard.test.tsx +++ b/test/tags/TagCard.test.tsx @@ -30,8 +30,8 @@ describe('', () => { afterEach(jest.resetAllMocks); it.each([ - [ 'ssr', '/server/1/list-short-urls/1?tag=ssr' ], - [ 'ssr-&-foo', '/server/1/list-short-urls/1?tag=ssr-%26-foo' ], + [ 'ssr', '/server/1/list-short-urls/1?tags=ssr' ], + [ 'ssr-&-foo', '/server/1/list-short-urls/1?tags=ssr-%26-foo' ], ])('shows a TagBullet and a link to the list filtering by the tag', (tag, expectedLink) => { const wrapper = createWrapper(tag); const links = wrapper.find(Link); @@ -61,7 +61,7 @@ describe('', () => { const links = wrapper.find(Link); expect(links).toHaveLength(2); - expect(links.at(0).prop('to')).toEqual('/server/1/list-short-urls/1?tag=ssr'); + expect(links.at(0).prop('to')).toEqual('/server/1/list-short-urls/1?tags=ssr'); expect(links.at(0).text()).toContain('48'); expect(links.at(1).prop('to')).toEqual('/server/1/tag/ssr/visits'); expect(links.at(1).text()).toContain('23,257'); diff --git a/test/tags/TagsTableRow.test.tsx b/test/tags/TagsTableRow.test.tsx index d264bd32..40306224 100644 --- a/test/tags/TagsTableRow.test.tsx +++ b/test/tags/TagsTableRow.test.tsx @@ -35,7 +35,7 @@ describe('', () => { const visitsLink = links.last(); expect(shortUrlsLink.prop('children')).toEqual(expectedShortUrls); - expect(shortUrlsLink.prop('to')).toEqual(`/server/abc123/list-short-urls/1?tag=${encodeURIComponent('foo&bar')}`); + expect(shortUrlsLink.prop('to')).toEqual(`/server/abc123/list-short-urls/1?tags=${encodeURIComponent('foo&bar')}`); expect(visitsLink.prop('children')).toEqual(expectedVisits); expect(visitsLink.prop('to')).toEqual('/server/abc123/tag/foo&bar/visits'); }); From a2421ee2d3cdc01f85517d066d932d974153416a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 7 Nov 2021 11:22:29 +0100 Subject: [PATCH 04/11] Created helper function to evolve a query string based on an object --- src/utils/helpers/query.ts | 3 +++ test/utils/helpers/query.test.ts | 13 ++++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/utils/helpers/query.ts b/src/utils/helpers/query.ts index 8c59f6eb..7d378f04 100644 --- a/src/utils/helpers/query.ts +++ b/src/utils/helpers/query.ts @@ -3,3 +3,6 @@ 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' }); + +export const evolveStringifiedQuery = (currentQuery: string, extra: any): string => + stringifyQuery({ ...parseQuery(currentQuery), ...extra }); diff --git a/test/utils/helpers/query.test.ts b/test/utils/helpers/query.test.ts index 90969bce..585af91a 100644 --- a/test/utils/helpers/query.test.ts +++ b/test/utils/helpers/query.test.ts @@ -1,4 +1,4 @@ -import { parseQuery, stringifyQuery } from '../../../src/utils/helpers/query'; +import { evolveStringifiedQuery, parseQuery, stringifyQuery } from '../../../src/utils/helpers/query'; describe('query', () => { describe('parseQuery', () => { @@ -22,4 +22,15 @@ describe('query', () => { expect(stringifyQuery(queryObj)).toEqual(expectedResult); }); }); + + describe('evolveStringifiedQuery', () => { + it.each([ + [ '?foo=bar', { baz: 123 }, 'foo=bar&baz=123' ], + [ 'foo=bar', { baz: 123 }, 'foo=bar&baz=123' ], + [ 'foo=bar&baz=hello', { baz: 'world' }, 'foo=bar&baz=world' ], + [ '?', { foo: 'some', bar: 'thing' }, 'foo=some&bar=thing' ], + ])('adds and overwrites params on provided query string', (query, extra, expected) => { + expect(evolveStringifiedQuery(query, extra)).toEqual(expected); + }); + }); }); From 3bc5b4c154fe03226af91434751c5550d1a8ce53 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 8 Nov 2021 22:13:37 +0100 Subject: [PATCH 05/11] Extended ShortUrlsPaginator so that it allows appending current query string --- src/short-urls/Paginator.tsx | 30 ++++++++--------- test/short-urls/Paginator.test.tsx | 52 ++++++++++++++++++++++-------- 2 files changed, 53 insertions(+), 29 deletions(-) diff --git a/src/short-urls/Paginator.tsx b/src/short-urls/Paginator.tsx index 82aa54c1..8103623f 100644 --- a/src/short-urls/Paginator.tsx +++ b/src/short-urls/Paginator.tsx @@ -1,15 +1,24 @@ import { Link } from 'react-router-dom'; import { Pagination, PaginationItem, PaginationLink } from 'reactstrap'; -import { pageIsEllipsis, keyForPage, progressivePagination, prettifyPageNumber } from '../utils/helpers/pagination'; +import { + pageIsEllipsis, + keyForPage, + progressivePagination, + prettifyPageNumber, + NumberOrEllipsis, +} from '../utils/helpers/pagination'; import { ShlinkPaginator } from '../api/types'; interface PaginatorProps { paginator?: ShlinkPaginator; serverId: string; + currentQueryString?: string; } -const Paginator = ({ paginator, serverId }: PaginatorProps) => { +const Paginator = ({ paginator, serverId, currentQueryString = '' }: PaginatorProps) => { const { currentPage = 0, pagesCount = 0 } = paginator ?? {}; + const urlForPage = (pageNumber: NumberOrEllipsis) => + `/server/${serverId}/list-short-urls/${pageNumber}${currentQueryString}`; if (pagesCount <= 1) { return null; @@ -22,10 +31,7 @@ const Paginator = ({ paginator, serverId }: PaginatorProps) => { disabled={pageIsEllipsis(pageNumber)} active={currentPage === pageNumber} > - + {prettifyPageNumber(pageNumber)} @@ -34,19 +40,11 @@ const Paginator = ({ paginator, serverId }: PaginatorProps) => { return ( - + {renderPages()} = pagesCount}> - + ); diff --git a/test/short-urls/Paginator.test.tsx b/test/short-urls/Paginator.test.tsx index d85a9ebe..3a844379 100644 --- a/test/short-urls/Paginator.test.tsx +++ b/test/short-urls/Paginator.test.tsx @@ -1,28 +1,54 @@ import { shallow, ShallowWrapper } from 'enzyme'; -import { PaginationItem } from 'reactstrap'; +import { PaginationItem, PaginationLink } from 'reactstrap'; +import { Mock } from 'ts-mockery'; import Paginator from '../../src/short-urls/Paginator'; +import { ShlinkPaginator } from '../../src/api/types'; +import { ELLIPSIS } from '../../src/utils/helpers/pagination'; describe('', () => { let wrapper: ShallowWrapper; + const buildPaginator = (pagesCount?: number) => Mock.of({ pagesCount, currentPage: 1 }); afterEach(() => wrapper?.unmount()); - it('renders nothing if the number of pages is below 2', () => { - wrapper = shallow(); + it.each([ + [ undefined ], + [ buildPaginator() ], + [ buildPaginator(0) ], + [ buildPaginator(1) ], + ])('renders nothing if the number of pages is below 2', (paginator) => { + wrapper = shallow(); expect(wrapper.text()).toEqual(''); }); - it('renders previous, next and the list of pages', () => { - const paginator = { - currentPage: 1, - pagesCount: 5, - totalItems: 10, - }; - const extraPagesPrevNext = 2; - const expectedItems = paginator.pagesCount + extraPagesPrevNext; - + it.each([ + [ buildPaginator(2), 4, 0 ], + [ buildPaginator(3), 5, 0 ], + [ buildPaginator(4), 6, 0 ], + [ buildPaginator(5), 7, 1 ], + [ buildPaginator(6), 7, 1 ], + [ buildPaginator(23), 7, 1 ], + ])('renders previous, next and the list of pages, with ellipses when expected', ( + paginator, + expectedPages, + expectedEllipsis, + ) => { wrapper = shallow(); + const items = wrapper.find(PaginationItem); + const ellipsis = items.filterWhere((item) => item.find(PaginationLink).prop('children') === ELLIPSIS); - expect(wrapper.find(PaginationItem)).toHaveLength(expectedItems); + expect(items).toHaveLength(expectedPages); + expect(ellipsis).toHaveLength(expectedEllipsis); + }); + + it('appends query string to all pages', () => { + const paginator = buildPaginator(3); + const currentQueryString = '?foo=bar'; + + wrapper = shallow(); + const links = wrapper.find(PaginationLink); + + expect(links).toHaveLength(5); + links.forEach((link) => expect(link.prop('to')).toContain(currentQueryString)); }); }); From 5f33059de1664911b3de09c300a383c6988b1767 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 8 Nov 2021 23:23:45 +0100 Subject: [PATCH 06/11] Improved SearchBar test --- src/short-urls/SearchBar.tsx | 40 +++++++++--------- src/short-urls/helpers/hooks.ts | 29 +++++++++++++ src/utils/helpers/query.ts | 3 -- test/short-urls/SearchBar.test.tsx | 68 ++++++++++++++++-------------- test/utils/helpers/query.test.ts | 13 +----- 5 files changed, 88 insertions(+), 65 deletions(-) create mode 100644 src/short-urls/helpers/hooks.ts diff --git a/src/short-urls/SearchBar.tsx b/src/short-urls/SearchBar.tsx index 97225485..0590a183 100644 --- a/src/short-urls/SearchBar.tsx +++ b/src/short-urls/SearchBar.tsx @@ -2,6 +2,7 @@ import { faTags as tagsIcon } from '@fortawesome/free-solid-svg-icons'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import { isEmpty, pipe } from 'ramda'; import { parseISO } from 'date-fns'; +import { RouteChildrenProps } from 'react-router-dom'; import SearchField from '../utils/SearchField'; import Tag from '../tags/helpers/Tag'; import { DateRangeSelector } from '../utils/dates/DateRangeSelector'; @@ -9,17 +10,21 @@ import { formatIsoDate } from '../utils/helpers/date'; import ColorGenerator from '../utils/services/ColorGenerator'; import { DateRange } from '../utils/dates/types'; import { ShortUrlsListParams } from './reducers/shortUrlsListParams'; +import { ShortUrlListRouteParams, useShortUrlsQuery } from './helpers/hooks'; import './SearchBar.scss'; -interface SearchBarProps { +export interface SearchBarProps extends RouteChildrenProps { listShortUrls: (params: ShortUrlsListParams) => void; shortUrlsListParams: ShortUrlsListParams; } const dateOrNull = (date?: string) => date ? parseISO(date) : null; -const SearchBar = (colorGenerator: ColorGenerator) => ({ listShortUrls, shortUrlsListParams }: SearchBarProps) => { - const selectedTags = shortUrlsListParams.tags ?? []; +const SearchBar = (colorGenerator: ColorGenerator) => ( + { listShortUrls, shortUrlsListParams, ...rest }: SearchBarProps, +) => { + const [{ search, tags }, toFirstPage ] = useShortUrlsQuery(rest); + const selectedTags = tags?.split(',').map(decodeURIComponent) ?? []; const setDates = pipe( ({ startDate, endDate }: DateRange) => ({ startDate: formatIsoDate(startDate) ?? undefined, @@ -27,10 +32,19 @@ const SearchBar = (colorGenerator: ColorGenerator) => ({ listShortUrls, shortUrl }), (dates) => listShortUrls({ ...shortUrlsListParams, ...dates }), ); + const setSearch = pipe( + (searchTerm: string) => isEmpty(searchTerm) ? undefined : searchTerm, + (search) => toFirstPage({ search }), + ); + const removeTag = pipe( + (tag: string) => selectedTags.filter((selectedTag) => selectedTag !== tag), + (tagsList) => tagsList.length === 0 ? undefined : tagsList.join(','), + (tags) => toFirstPage({ tags }), + ); return (
- listShortUrls({ ...shortUrlsListParams, searchTerm })} /> +
@@ -47,24 +61,12 @@ const SearchBar = (colorGenerator: ColorGenerator) => ({ listShortUrls, shortUrl
- {!isEmpty(selectedTags) && ( + {selectedTags.length > 0 && (

  - {selectedTags.map((tag) => ( - listShortUrls( - { - ...shortUrlsListParams, - tags: selectedTags.filter((selectedTag) => selectedTag !== tag), - }, - )} - /> - ))} + {selectedTags.map((tag) => + removeTag(tag)} />)}

)}
diff --git a/src/short-urls/helpers/hooks.ts b/src/short-urls/helpers/hooks.ts new file mode 100644 index 00000000..e7ff9f78 --- /dev/null +++ b/src/short-urls/helpers/hooks.ts @@ -0,0 +1,29 @@ +import { RouteChildrenProps } from 'react-router-dom'; +import { useMemo } from 'react'; +import { isEmpty } from 'ramda'; +import { parseQuery, stringifyQuery } from '../../utils/helpers/query'; + +type ServerIdRouteProps = RouteChildrenProps<{ serverId: string }>; +type ToFirstPage = (extra: Partial) => void; + +export interface ShortUrlListRouteParams { + page: string; + serverId: string; +} + +interface ShortUrlsQuery { + tags?: string; + search?: string; +} + +export const useShortUrlsQuery = ({ history, location, match }: ServerIdRouteProps): [ShortUrlsQuery, ToFirstPage] => { + const query = useMemo(() => parseQuery(location.search), [ location ]); + const toFirstPageWithExtra = (extra: Partial) => { + const evolvedQuery = stringifyQuery({ ...query, ...extra }); + const queryString = isEmpty(evolvedQuery) ? '' : `?${evolvedQuery}`; + + history.push(`/server/${match?.params.serverId}/list-short-urls/1${queryString}`); + }; + + return [ query, toFirstPageWithExtra ]; +}; diff --git a/src/utils/helpers/query.ts b/src/utils/helpers/query.ts index 7d378f04..8c59f6eb 100644 --- a/src/utils/helpers/query.ts +++ b/src/utils/helpers/query.ts @@ -3,6 +3,3 @@ 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' }); - -export const evolveStringifiedQuery = (currentQuery: string, extra: any): string => - stringifyQuery({ ...parseQuery(currentQuery), ...extra }); diff --git a/test/short-urls/SearchBar.test.tsx b/test/short-urls/SearchBar.test.tsx index f16ba98d..cc609369 100644 --- a/test/short-urls/SearchBar.test.tsx +++ b/test/short-urls/SearchBar.test.tsx @@ -1,69 +1,75 @@ import { shallow, ShallowWrapper } from 'enzyme'; import { Mock } from 'ts-mockery'; -import searchBarCreator from '../../src/short-urls/SearchBar'; +import { History, Location } from 'history'; +import { match } from 'react-router'; +import searchBarCreator, { SearchBarProps } from '../../src/short-urls/SearchBar'; 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('', () => { let wrapper: ShallowWrapper; const listShortUrlsMock = jest.fn(); const SearchBar = searchBarCreator(Mock.all()); + const push = jest.fn(); + const createWrapper = (props: Partial = {}) => { + wrapper = shallow( + ({ push })} + location={Mock.of({ search: '' })} + match={Mock.of>({ params: { serverId: '1' } })} + {...props} + />, + ); + + return wrapper; + }; afterEach(jest.clearAllMocks); afterEach(() => wrapper?.unmount()); - it('renders a SearchField', () => { - wrapper = shallow(); + it('renders some children components SearchField', () => { + const wrapper = createWrapper(); expect(wrapper.find(SearchField)).toHaveLength(1); - }); - - it('renders a DateRangeSelector', () => { - wrapper = shallow(); - expect(wrapper.find(DateRangeSelector)).toHaveLength(1); }); - it('renders no tags when the list of tags is empty', () => { - wrapper = shallow(); + it.each([ + [ 'tags=foo,bar,baz', 3 ], + [ 'tags=foo,baz', 2 ], + [ '', 0 ], + [ 'foo=bar', 0 ], + ])('renders the proper amount of tags', (search, expectedTagComps) => { + const wrapper = createWrapper({ location: Mock.of({ search }) }); - expect(wrapper.find(Tag)).toHaveLength(0); - }); - - it('renders the proper amount of tags', () => { - const tags = [ 'foo', 'bar', 'baz' ]; - - wrapper = shallow(); - - expect(wrapper.find(Tag)).toHaveLength(tags.length); + expect(wrapper.find(Tag)).toHaveLength(expectedTagComps); }); it('updates short URLs list when search field changes', () => { - wrapper = shallow(); + const wrapper = createWrapper(); const searchField = wrapper.find(SearchField); - expect(listShortUrlsMock).not.toHaveBeenCalled(); - searchField.simulate('change'); - expect(listShortUrlsMock).toHaveBeenCalledTimes(1); + expect(push).not.toHaveBeenCalled(); + searchField.simulate('change', 'search-term'); + expect(push).toHaveBeenCalledWith('/server/1/list-short-urls/1?search=search-term'); }); it('updates short URLs list when a tag is removed', () => { - wrapper = shallow( - , - ); + const wrapper = createWrapper({ location: Mock.of({ search: 'tags=foo,bar' }) }); const tag = wrapper.find(Tag).first(); - expect(listShortUrlsMock).not.toHaveBeenCalled(); + expect(push).not.toHaveBeenCalled(); tag.simulate('close'); - expect(listShortUrlsMock).toHaveBeenCalledTimes(1); + expect(push).toHaveBeenCalledWith('/server/1/list-short-urls/1?tags=bar'); }); it('updates short URLs list when date range changes', () => { - wrapper = shallow( - , - ); + const wrapper = createWrapper(); const dateRange = wrapper.find(DateRangeSelector); expect(listShortUrlsMock).not.toHaveBeenCalled(); diff --git a/test/utils/helpers/query.test.ts b/test/utils/helpers/query.test.ts index 585af91a..90969bce 100644 --- a/test/utils/helpers/query.test.ts +++ b/test/utils/helpers/query.test.ts @@ -1,4 +1,4 @@ -import { evolveStringifiedQuery, parseQuery, stringifyQuery } from '../../../src/utils/helpers/query'; +import { parseQuery, stringifyQuery } from '../../../src/utils/helpers/query'; describe('query', () => { describe('parseQuery', () => { @@ -22,15 +22,4 @@ describe('query', () => { expect(stringifyQuery(queryObj)).toEqual(expectedResult); }); }); - - describe('evolveStringifiedQuery', () => { - it.each([ - [ '?foo=bar', { baz: 123 }, 'foo=bar&baz=123' ], - [ 'foo=bar', { baz: 123 }, 'foo=bar&baz=123' ], - [ 'foo=bar&baz=hello', { baz: 'world' }, 'foo=bar&baz=world' ], - [ '?', { foo: 'some', bar: 'thing' }, 'foo=some&bar=thing' ], - ])('adds and overwrites params on provided query string', (query, extra, expected) => { - expect(evolveStringifiedQuery(query, extra)).toEqual(expected); - }); - }); }); From ed038b97997b79c913ca3daee3185f8783eafc38 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 8 Nov 2021 23:41:17 +0100 Subject: [PATCH 07/11] Fixed ShortUrlsList test --- src/short-urls/ShortUrlsList.tsx | 43 +++++++++++----------- src/short-urls/ShortUrlsTable.tsx | 4 ++- test/short-urls/ShortUrlsList.test.tsx | 50 +++++++++++++------------- 3 files changed, 50 insertions(+), 47 deletions(-) diff --git a/src/short-urls/ShortUrlsList.tsx b/src/short-urls/ShortUrlsList.tsx index 7c8fcda4..1a303cbd 100644 --- a/src/short-urls/ShortUrlsList.tsx +++ b/src/short-urls/ShortUrlsList.tsx @@ -1,25 +1,20 @@ -import { head, keys, values } from 'ramda'; -import { FC, useEffect, useState } from 'react'; +import { head, keys, pipe, values } from 'ramda'; +import { FC, useEffect, useMemo, useState } from 'react'; import { RouteComponentProps } from 'react-router'; import { Card } from 'reactstrap'; import SortingDropdown from '../utils/SortingDropdown'; import { determineOrderDir, Order, OrderDir } from '../utils/helpers/ordering'; import { getServerId, SelectedServer } from '../servers/data'; import { boundToMercureHub } from '../mercure/helpers/boundToMercureHub'; -import { parseQuery } from '../utils/helpers/query'; import { Topics } from '../mercure/helpers/Topics'; import { TableOrderIcon } from '../utils/table/TableOrderIcon'; import { ShortUrlsList as ShortUrlsListState } from './reducers/shortUrlsList'; import { OrderableFields, ShortUrlsListParams, SORTABLE_FIELDS } from './reducers/shortUrlsListParams'; import { ShortUrlsTableProps } from './ShortUrlsTable'; import Paginator from './Paginator'; +import { ShortUrlListRouteParams, useShortUrlsQuery } from './helpers/hooks'; -interface RouteParams { - page: string; - serverId: string; -} - -export interface ShortUrlsListProps extends RouteComponentProps { +interface ShortUrlsListProps extends RouteComponentProps { selectedServer: SelectedServer; shortUrlsList: ShortUrlsListState; listShortUrls: (params: ShortUrlsListParams) => void; @@ -29,21 +24,26 @@ export interface ShortUrlsListProps extends RouteComponentProps { type ShortUrlsOrder = Order; -const ShortUrlsList = (ShortUrlsTable: FC) => boundToMercureHub(({ +const ShortUrlsList = (ShortUrlsTable: FC, SearchBar: FC) => boundToMercureHub(({ listShortUrls, resetShortUrlParams, shortUrlsListParams, match, location, + history, shortUrlsList, selectedServer, }: ShortUrlsListProps) => { + const serverId = getServerId(selectedServer); const { orderBy } = shortUrlsListParams; const [ order, setOrder ] = useState({ field: orderBy && (head(keys(orderBy)) as OrderableFields), dir: orderBy && head(values(orderBy)), }); + const [{ tags, search }, toFirstPage ] = useShortUrlsQuery({ history, match, location }); + const decodedTags = useMemo(() => tags?.split(',').map(decodeURIComponent) ?? [], [ tags ]); const { pagination } = shortUrlsList?.shortUrls ?? {}; + const refreshList = (extraParams: ShortUrlsListParams) => listShortUrls({ ...shortUrlsListParams, ...extraParams }); const handleOrderBy = (field?: OrderableFields, dir?: OrderDir) => { setOrder({ field, dir }); @@ -52,30 +52,31 @@ const ShortUrlsList = (ShortUrlsTable: FC) => boundToMercur const orderByColumn = (field: OrderableFields) => () => handleOrderBy(field, determineOrderDir(field, order.field, order.dir)); const renderOrderIcon = (field: OrderableFields) => ; + const addTag = pipe( + (newTag: string) => [ ...new Set([ ...decodedTags, newTag ]) ].join(','), + (tags) => toFirstPage({ tags }), + ); + useEffect(() => resetShortUrlParams, []); useEffect(() => { - const { tag } = parseQuery<{ tag?: string }>(location.search); - const tags = tag ? [ decodeURIComponent(tag) ] : shortUrlsListParams.tags; - - refreshList({ page: match.params.page, tags, itemsPerPage: undefined }); - - return resetShortUrlParams; - }, []); + refreshList({ page: match.params.page, searchTerm: search, tags: decodedTags, itemsPerPage: undefined }); + }, [ match.params.page, search, decodedTags ]); return ( <> +
refreshList({ tags: [ ...shortUrlsListParams.tags ?? [], tag ] })} + orderByColumn={orderByColumn} + renderOrderIcon={renderOrderIcon} + onTagClick={addTag} /> - + ); diff --git a/src/short-urls/ShortUrlsTable.tsx b/src/short-urls/ShortUrlsTable.tsx index 5a51db85..ae73ad9e 100644 --- a/src/short-urls/ShortUrlsTable.tsx +++ b/src/short-urls/ShortUrlsTable.tsx @@ -35,7 +35,9 @@ export const ShortUrlsTable = (ShortUrlsRow: FC) => ({ if (error) { return ( - + ); } diff --git a/test/short-urls/ShortUrlsList.test.tsx b/test/short-urls/ShortUrlsList.test.tsx index 3524e3b7..ebb09941 100644 --- a/test/short-urls/ShortUrlsList.test.tsx +++ b/test/short-urls/ShortUrlsList.test.tsx @@ -1,19 +1,24 @@ import { shallow, ShallowWrapper } from 'enzyme'; import { ReactElement } from 'react'; import { Mock } from 'ts-mockery'; -import shortUrlsListCreator, { ShortUrlsListProps } from '../../src/short-urls/ShortUrlsList'; +import { History, Location } from 'history'; +import { match } from 'react-router'; +import shortUrlsListCreator from '../../src/short-urls/ShortUrlsList'; import { ShortUrl } from '../../src/short-urls/data'; import { MercureBoundProps } from '../../src/mercure/helpers/boundToMercureHub'; import { ShortUrlsList as ShortUrlsListModel } from '../../src/short-urls/reducers/shortUrlsList'; import SortingDropdown from '../../src/utils/SortingDropdown'; import { OrderableFields, OrderBy } from '../../src/short-urls/reducers/shortUrlsListParams'; import Paginator from '../../src/short-urls/Paginator'; +import { ReachableServer } from '../../src/servers/data'; +import { ShortUrlListRouteParams } from '../../src/short-urls/helpers/hooks'; describe('', () => { let wrapper: ShallowWrapper; const ShortUrlsTable = () => null; + const SearchBar = () => null; const listShortUrlsMock = jest.fn(); - const resetShortUrlParamsMock = jest.fn(); + const push = jest.fn(); const shortUrlsList = Mock.of({ shortUrls: { data: [ @@ -26,22 +31,18 @@ describe('', () => { ], }, }); - const ShortUrlsList = shortUrlsListCreator(ShortUrlsTable); + const ShortUrlsList = shortUrlsListCreator(ShortUrlsTable, SearchBar); const createWrapper = (orderBy: OrderBy = {}) => shallow( ()} {...Mock.of({ mercureInfo: { loading: true } })} listShortUrls={listShortUrlsMock} - resetShortUrlParams={resetShortUrlParamsMock} - shortUrlsListParams={{ - page: '1', - tags: [ 'test tag' ], - searchTerm: 'example.com', - orderBy, - }} - match={{ params: {} } as any} - location={{} as any} + resetShortUrlParams={jest.fn()} + shortUrlsListParams={{ page: '1', orderBy }} + match={Mock.of>({ params: {} })} + location={Mock.of({ search: '?tags=test%20tag&search=example.com' })} shortUrlsList={shortUrlsList} + history={Mock.of({ push })} + selectedServer={Mock.of({ id: '1' })} />, ).dive(); // Dive is needed as this component is wrapped in a HOC @@ -56,6 +57,11 @@ describe('', () => { expect(wrapper.find(ShortUrlsTable)).toHaveLength(1); expect(wrapper.find(SortingDropdown)).toHaveLength(1); expect(wrapper.find(Paginator)).toHaveLength(1); + expect(wrapper.find(SearchBar)).toHaveLength(1); + }); + + it('passes current query to paginator', () => { + expect(wrapper.find(Paginator).prop('currentQueryString')).toEqual('?tags=test%20tag&search=example.com'); }); it('gets list refreshed every time a tag is clicked', () => { @@ -63,16 +69,10 @@ describe('', () => { wrapper.find(ShortUrlsTable).simulate('tagClick', 'bar'); wrapper.find(ShortUrlsTable).simulate('tagClick', 'baz'); - expect(listShortUrlsMock).toHaveBeenCalledTimes(3); - expect(listShortUrlsMock).toHaveBeenNthCalledWith(1, expect.objectContaining({ - tags: [ 'test tag', 'foo' ], - })); - expect(listShortUrlsMock).toHaveBeenNthCalledWith(2, expect.objectContaining({ - tags: [ 'test tag', 'bar' ], - })); - expect(listShortUrlsMock).toHaveBeenNthCalledWith(3, expect.objectContaining({ - tags: [ 'test tag', 'baz' ], - })); + expect(push).toHaveBeenCalledTimes(3); + expect(push).toHaveBeenNthCalledWith(1, expect.stringContaining(`tags=${encodeURIComponent('test tag,foo')}`)); + expect(push).toHaveBeenNthCalledWith(2, expect.stringContaining(`tags=${encodeURIComponent('test tag,bar')}`)); + expect(push).toHaveBeenNthCalledWith(3, expect.stringContaining(`tags=${encodeURIComponent('test tag,baz')}`)); }); it('invokes order icon rendering', () => { @@ -88,7 +88,7 @@ describe('', () => { expect(renderIcon('visits').props.currentOrder).toEqual({ field: 'visits', dir: 'ASC' }); }); - it('handles order by through table', () => { + it('handles order through table', () => { const orderByColumn: (field: OrderableFields) => Function = wrapper.find(ShortUrlsTable).prop('orderByColumn'); orderByColumn('visits')(); @@ -107,7 +107,7 @@ describe('', () => { })); }); - it('handles order by through dropdown', () => { + it('handles order through dropdown', () => { expect(wrapper.find(SortingDropdown).prop('order')).toEqual({}); wrapper.find(SortingDropdown).simulate('change', 'visits', 'ASC'); From 21b8e05e354619d226b6746681e03f6dfd0e6b0a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 10 Nov 2021 22:25:56 +0100 Subject: [PATCH 08/11] Moved dates handling in short URLs list to query --- src/short-urls/SearchBar.tsx | 15 ++++++-------- src/short-urls/ShortUrlsList.tsx | 12 ++++++----- src/short-urls/helpers/hooks.ts | 2 ++ src/short-urls/services/provideServices.ts | 2 +- src/utils/dates/DateRangeSelector.tsx | 18 ++++++----------- test/short-urls/SearchBar.test.tsx | 23 ++++++++++++++-------- 6 files changed, 37 insertions(+), 35 deletions(-) diff --git a/src/short-urls/SearchBar.tsx b/src/short-urls/SearchBar.tsx index 0590a183..3397c55c 100644 --- a/src/short-urls/SearchBar.tsx +++ b/src/short-urls/SearchBar.tsx @@ -14,23 +14,20 @@ import { ShortUrlListRouteParams, useShortUrlsQuery } from './helpers/hooks'; import './SearchBar.scss'; export interface SearchBarProps extends RouteChildrenProps { - listShortUrls: (params: ShortUrlsListParams) => void; shortUrlsListParams: ShortUrlsListParams; } const dateOrNull = (date?: string) => date ? parseISO(date) : null; -const SearchBar = (colorGenerator: ColorGenerator) => ( - { listShortUrls, shortUrlsListParams, ...rest }: SearchBarProps, -) => { - const [{ search, tags }, toFirstPage ] = useShortUrlsQuery(rest); - const selectedTags = tags?.split(',').map(decodeURIComponent) ?? []; +const SearchBar = (colorGenerator: ColorGenerator) => ({ shortUrlsListParams, ...rest }: SearchBarProps) => { + const [{ search, tags, startDate, endDate }, toFirstPage ] = useShortUrlsQuery(rest); + const selectedTags = tags?.split(',') ?? []; const setDates = pipe( ({ startDate, endDate }: DateRange) => ({ startDate: formatIsoDate(startDate) ?? undefined, endDate: formatIsoDate(endDate) ?? undefined, }), - (dates) => listShortUrls({ ...shortUrlsListParams, ...dates }), + toFirstPage, ); const setSearch = pipe( (searchTerm: string) => isEmpty(searchTerm) ? undefined : searchTerm, @@ -52,8 +49,8 @@ const SearchBar = (colorGenerator: ColorGenerator) => ( diff --git a/src/short-urls/ShortUrlsList.tsx b/src/short-urls/ShortUrlsList.tsx index 1a303cbd..1fc22f1e 100644 --- a/src/short-urls/ShortUrlsList.tsx +++ b/src/short-urls/ShortUrlsList.tsx @@ -40,8 +40,8 @@ const ShortUrlsList = (ShortUrlsTable: FC, SearchBar: FC) = field: orderBy && (head(keys(orderBy)) as OrderableFields), dir: orderBy && head(values(orderBy)), }); - const [{ tags, search }, toFirstPage ] = useShortUrlsQuery({ history, match, location }); - const decodedTags = useMemo(() => tags?.split(',').map(decodeURIComponent) ?? [], [ tags ]); + const [{ tags, search, startDate, endDate }, toFirstPage ] = useShortUrlsQuery({ history, match, location }); + const selectedTags = useMemo(() => tags?.split(',') ?? [], [ tags ]); const { pagination } = shortUrlsList?.shortUrls ?? {}; const refreshList = (extraParams: ShortUrlsListParams) => listShortUrls({ ...shortUrlsListParams, ...extraParams }); @@ -53,14 +53,16 @@ const ShortUrlsList = (ShortUrlsTable: FC, SearchBar: FC) = handleOrderBy(field, determineOrderDir(field, order.field, order.dir)); const renderOrderIcon = (field: OrderableFields) => ; const addTag = pipe( - (newTag: string) => [ ...new Set([ ...decodedTags, newTag ]) ].join(','), + (newTag: string) => [ ...new Set([ ...selectedTags, newTag ]) ].join(','), (tags) => toFirstPage({ tags }), ); useEffect(() => resetShortUrlParams, []); useEffect(() => { - refreshList({ page: match.params.page, searchTerm: search, tags: decodedTags, itemsPerPage: undefined }); - }, [ match.params.page, search, decodedTags ]); + refreshList( + { page: match.params.page, searchTerm: search, tags: selectedTags, itemsPerPage: undefined, startDate, endDate }, + ); + }, [ match.params.page, search, selectedTags, startDate, endDate ]); return ( <> diff --git a/src/short-urls/helpers/hooks.ts b/src/short-urls/helpers/hooks.ts index e7ff9f78..0cbbca1f 100644 --- a/src/short-urls/helpers/hooks.ts +++ b/src/short-urls/helpers/hooks.ts @@ -14,6 +14,8 @@ export interface ShortUrlListRouteParams { interface ShortUrlsQuery { tags?: string; search?: string; + startDate?: string; + endDate?: string; } export const useShortUrlsQuery = ({ history, location, match }: ServerIdRouteProps): [ShortUrlsQuery, ToFirstPage] => { diff --git a/src/short-urls/services/provideServices.ts b/src/short-urls/services/provideServices.ts index 197c6941..6f1b70dc 100644 --- a/src/short-urls/services/provideServices.ts +++ b/src/short-urls/services/provideServices.ts @@ -52,7 +52,7 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator, withRouter: // Services bottle.serviceFactory('SearchBar', SearchBar, 'ColorGenerator'); - bottle.decorator('SearchBar', connect([ 'shortUrlsListParams' ], [ 'listShortUrls' ])); + bottle.decorator('SearchBar', connect([ 'shortUrlsListParams' ])); bottle.decorator('SearchBar', withRouter); // Actions diff --git a/src/utils/dates/DateRangeSelector.tsx b/src/utils/dates/DateRangeSelector.tsx index 1b1ec95c..b0a3c104 100644 --- a/src/utils/dates/DateRangeSelector.tsx +++ b/src/utils/dates/DateRangeSelector.tsx @@ -22,18 +22,16 @@ export interface DateRangeSelectorProps { export const DateRangeSelector = ( { onDatesChange, initialDateRange, defaultText, disabled }: DateRangeSelectorProps, ) => { - const [ activeInterval, setActiveInterval ] = useState( - rangeIsInterval(initialDateRange) ? initialDateRange : undefined, - ); - const [ activeDateRange, setActiveDateRange ] = useState( - !rangeIsInterval(initialDateRange) ? initialDateRange : undefined, - ); + const initialIntervalIsRange = rangeIsInterval(initialDateRange); + const [ activeInterval, setActiveInterval ] = useState(initialIntervalIsRange ? initialDateRange : undefined); + const [ activeDateRange, setActiveDateRange ] = useState(initialIntervalIsRange ? undefined : initialDateRange); + const updateDateRange = (dateRange: DateRange) => { setActiveInterval(dateRangeIsEmpty(dateRange) ? 'all' : undefined); setActiveDateRange(dateRange); onDatesChange(dateRange); }; - const updateInterval = (dateInterval: DateInterval) => () => { + const updateInterval = (dateInterval: DateInterval) => { setActiveInterval(dateInterval); setActiveDateRange(undefined); onDatesChange(intervalToDateRange(dateInterval)); @@ -41,11 +39,7 @@ export const DateRangeSelector = ( return ( - updateInterval(interval)()} - /> + Custom: diff --git a/test/short-urls/SearchBar.test.tsx b/test/short-urls/SearchBar.test.tsx index cc609369..f698f665 100644 --- a/test/short-urls/SearchBar.test.tsx +++ b/test/short-urls/SearchBar.test.tsx @@ -2,6 +2,7 @@ import { shallow, ShallowWrapper } from 'enzyme'; 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 SearchField from '../../src/utils/SearchField'; import Tag from '../../src/tags/helpers/Tag'; @@ -11,13 +12,12 @@ import { ShortUrlListRouteParams } from '../../src/short-urls/helpers/hooks'; describe('', () => { let wrapper: ShallowWrapper; - const listShortUrlsMock = jest.fn(); const SearchBar = searchBarCreator(Mock.all()); const push = jest.fn(); + const now = new Date(); const createWrapper = (props: Partial = {}) => { wrapper = shallow( ({ push })} location={Mock.of({ search: '' })} @@ -50,7 +50,7 @@ describe('', () => { expect(wrapper.find(Tag)).toHaveLength(expectedTagComps); }); - it('updates short URLs list when search field changes', () => { + it('redirects to first page when search field changes', () => { const wrapper = createWrapper(); const searchField = wrapper.find(SearchField); @@ -59,7 +59,7 @@ describe('', () => { expect(push).toHaveBeenCalledWith('/server/1/list-short-urls/1?search=search-term'); }); - it('updates short URLs list when a tag is removed', () => { + it('redirects to first page when a tag is removed', () => { const wrapper = createWrapper({ location: Mock.of({ search: 'tags=foo,bar' }) }); const tag = wrapper.find(Tag).first(); @@ -68,12 +68,19 @@ describe('', () => { expect(push).toHaveBeenCalledWith('/server/1/list-short-urls/1?tags=bar'); }); - it('updates short URLs list when date range changes', () => { + it.each([ + [{ startDate: now }, `startDate=${encodeURIComponent(formatISO(now))}` ], + [{ endDate: now }, `endDate=${encodeURIComponent(formatISO(now))}` ], + [ + { startDate: now, endDate: now }, + `startDate=${encodeURIComponent(formatISO(now))}&endDate=${encodeURIComponent(formatISO(now))}`, + ], + ])('redirects to first page when date range changes', (dates, expectedQuery) => { const wrapper = createWrapper(); const dateRange = wrapper.find(DateRangeSelector); - expect(listShortUrlsMock).not.toHaveBeenCalled(); - dateRange.simulate('datesChange', {}); - expect(listShortUrlsMock).toHaveBeenCalledTimes(1); + expect(push).not.toHaveBeenCalled(); + dateRange.simulate('datesChange', dates); + expect(push).toHaveBeenCalledWith(`/server/1/list-short-urls/1?${expectedQuery}`); }); }); From 2e77cd196970a3a20d35b190983821c7e2236cc4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 11 Nov 2021 21:28:17 +0100 Subject: [PATCH 09/11] Removed handling of most short URLs list params from a reducer --- src/api/services/ShlinkApiClient.ts | 4 ++-- src/api/types/index.ts | 11 +++++++++++ src/common/services/provideServices.ts | 2 +- src/servers/Overview.tsx | 4 ++-- src/short-urls/SearchBar.tsx | 9 +++------ src/short-urls/ShortUrlsList.tsx | 5 ++++- src/short-urls/reducers/shortUrlsList.ts | 4 ++-- src/short-urls/reducers/shortUrlsListParams.ts | 4 ---- src/short-urls/services/provideServices.ts | 1 - 9 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/api/services/ShlinkApiClient.ts b/src/api/services/ShlinkApiClient.ts index f724b43a..6c783531 100644 --- a/src/api/services/ShlinkApiClient.ts +++ b/src/api/services/ShlinkApiClient.ts @@ -1,6 +1,5 @@ import { isEmpty, isNil, reject } from 'ramda'; import { AxiosInstance, AxiosResponse, Method } from 'axios'; -import { ShortUrlsListParams } from '../../short-urls/reducers/shortUrlsListParams'; import { ShortUrl, ShortUrlData } from '../../short-urls/data'; import { OptionalString } from '../../utils/utils'; import { @@ -17,6 +16,7 @@ import { ShlinkVisitsOverview, ShlinkEditDomainRedirects, ShlinkDomainRedirects, + ShlinkShortUrlsListParams, } from '../types'; import { stringifyQuery } from '../../utils/helpers/query'; @@ -34,7 +34,7 @@ export default class ShlinkApiClient { this.apiVersion = 2; } - public readonly listShortUrls = async (params: ShortUrlsListParams = {}): Promise => + public readonly listShortUrls = async (params: ShlinkShortUrlsListParams = {}): Promise => this.performRequest<{ shortUrls: ShlinkShortUrlsResponse }>('/short-urls', 'GET', params) .then(({ data }) => data.shortUrls); diff --git a/src/api/types/index.ts b/src/api/types/index.ts index b16111b9..478194fc 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 } from '../../short-urls/data'; +import { OrderBy } from '../../short-urls/reducers/shortUrlsListParams'; export interface ShlinkShortUrlsResponse { data: ShortUrl[]; @@ -85,6 +86,16 @@ export interface ShlinkDomainsResponse { data: ShlinkDomain[]; } +export interface ShlinkShortUrlsListParams { + page?: string; + itemsPerPage?: number; + tags?: string[]; + searchTerm?: string; + startDate?: string; + endDate?: string; + orderBy?: OrderBy; +} + export interface ProblemDetailsError { type: string; detail: string; diff --git a/src/common/services/provideServices.ts b/src/common/services/provideServices.ts index b714c8d5..9d51ccdb 100644 --- a/src/common/services/provideServices.ts +++ b/src/common/services/provideServices.ts @@ -46,7 +46,7 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator, withRouter: 'EditShortUrl', 'ManageDomains', ); - bottle.decorator('MenuLayout', connect([ 'selectedServer', 'shortUrlsListParams' ], [ 'selectServer' ])); + bottle.decorator('MenuLayout', connect([ 'selectedServer' ], [ 'selectServer' ])); bottle.decorator('MenuLayout', withRouter); bottle.serviceFactory('AsideMenu', AsideMenu, 'DeleteServerButton'); diff --git a/src/servers/Overview.tsx b/src/servers/Overview.tsx index f85ed23f..9b99cc6f 100644 --- a/src/servers/Overview.tsx +++ b/src/servers/Overview.tsx @@ -1,7 +1,6 @@ import { FC, useEffect } from 'react'; import { Card, CardBody, CardHeader, CardText, CardTitle, Row } from 'reactstrap'; import { Link, useHistory } from 'react-router-dom'; -import { ShortUrlsListParams } from '../short-urls/reducers/shortUrlsListParams'; import { ShortUrlsList as ShortUrlsListState } from '../short-urls/reducers/shortUrlsList'; import { prettify } from '../utils/helpers/numbers'; import { TagsList } from '../tags/reducers/tagsList'; @@ -11,12 +10,13 @@ import { CreateShortUrlProps } from '../short-urls/CreateShortUrl'; import { VisitsOverview } from '../visits/reducers/visitsOverview'; import { Versions } from '../utils/helpers/version'; import { Topics } from '../mercure/helpers/Topics'; +import { ShlinkShortUrlsListParams } from '../api/types'; import { getServerId, SelectedServer } from './data'; import './Overview.scss'; interface OverviewConnectProps { shortUrlsList: ShortUrlsListState; - listShortUrls: (params: ShortUrlsListParams) => void; + listShortUrls: (params: ShlinkShortUrlsListParams) => void; listTags: Function; tagsList: TagsList; selectedServer: SelectedServer; diff --git a/src/short-urls/SearchBar.tsx b/src/short-urls/SearchBar.tsx index 3397c55c..be650422 100644 --- a/src/short-urls/SearchBar.tsx +++ b/src/short-urls/SearchBar.tsx @@ -9,18 +9,15 @@ import { DateRangeSelector } from '../utils/dates/DateRangeSelector'; import { formatIsoDate } from '../utils/helpers/date'; import ColorGenerator from '../utils/services/ColorGenerator'; import { DateRange } from '../utils/dates/types'; -import { ShortUrlsListParams } from './reducers/shortUrlsListParams'; import { ShortUrlListRouteParams, useShortUrlsQuery } from './helpers/hooks'; import './SearchBar.scss'; -export interface SearchBarProps extends RouteChildrenProps { - shortUrlsListParams: ShortUrlsListParams; -} +export type SearchBarProps = RouteChildrenProps; const dateOrNull = (date?: string) => date ? parseISO(date) : null; -const SearchBar = (colorGenerator: ColorGenerator) => ({ shortUrlsListParams, ...rest }: SearchBarProps) => { - const [{ search, tags, startDate, endDate }, toFirstPage ] = useShortUrlsQuery(rest); +const SearchBar = (colorGenerator: ColorGenerator) => (props: SearchBarProps) => { + const [{ search, tags, startDate, endDate }, toFirstPage ] = useShortUrlsQuery(props); const selectedTags = tags?.split(',') ?? []; const setDates = pipe( ({ startDate, endDate }: DateRange) => ({ diff --git a/src/short-urls/ShortUrlsList.tsx b/src/short-urls/ShortUrlsList.tsx index 1fc22f1e..b5fd5250 100644 --- a/src/short-urls/ShortUrlsList.tsx +++ b/src/short-urls/ShortUrlsList.tsx @@ -8,6 +8,7 @@ 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 { ShortUrlsList as ShortUrlsListState } from './reducers/shortUrlsList'; import { OrderableFields, ShortUrlsListParams, SORTABLE_FIELDS } from './reducers/shortUrlsListParams'; import { ShortUrlsTableProps } from './ShortUrlsTable'; @@ -44,7 +45,9 @@ const ShortUrlsList = (ShortUrlsTable: FC, SearchBar: FC) = const selectedTags = useMemo(() => tags?.split(',') ?? [], [ tags ]); const { pagination } = shortUrlsList?.shortUrls ?? {}; - const refreshList = (extraParams: ShortUrlsListParams) => listShortUrls({ ...shortUrlsListParams, ...extraParams }); + const refreshList = (extraParams: ShlinkShortUrlsListParams) => listShortUrls( + { ...shortUrlsListParams, ...extraParams }, + ); const handleOrderBy = (field?: OrderableFields, dir?: OrderDir) => { setOrder({ field, dir }); refreshList({ orderBy: field ? { [field]: dir } : undefined }); diff --git a/src/short-urls/reducers/shortUrlsList.ts b/src/short-urls/reducers/shortUrlsList.ts index 9fa4b2fa..72290bea 100644 --- a/src/short-urls/reducers/shortUrlsList.ts +++ b/src/short-urls/reducers/shortUrlsList.ts @@ -5,7 +5,7 @@ import { CREATE_VISITS, CreateVisitsAction } from '../../visits/reducers/visitCr import { buildReducer } from '../../utils/helpers/redux'; import { GetState } from '../../container/types'; import { ShlinkApiClientBuilder } from '../../api/services/ShlinkApiClientBuilder'; -import { ShlinkShortUrlsResponse } from '../../api/types'; +import { ShlinkShortUrlsListParams, ShlinkShortUrlsResponse } from '../../api/types'; import { DeleteShortUrlAction, SHORT_URL_DELETED } from './shortUrlDeletion'; import { ShortUrlsListParams } from './shortUrlsListParams'; import { CREATE_SHORT_URL, CreateShortUrlAction } from './shortUrlCreation'; @@ -101,7 +101,7 @@ export default buildReducer({ }, initialState); export const listShortUrls = (buildShlinkApiClient: ShlinkApiClientBuilder) => ( - params: ShortUrlsListParams = {}, + params: ShlinkShortUrlsListParams = {}, ) => async (dispatch: Dispatch, getState: GetState) => { dispatch({ type: LIST_SHORT_URLS_START }); const { listShortUrls } = buildShlinkApiClient(getState); diff --git a/src/short-urls/reducers/shortUrlsListParams.ts b/src/short-urls/reducers/shortUrlsListParams.ts index 1f16e562..c05d0ccc 100644 --- a/src/short-urls/reducers/shortUrlsListParams.ts +++ b/src/short-urls/reducers/shortUrlsListParams.ts @@ -19,10 +19,6 @@ export type OrderBy = Partial>; export interface ShortUrlsListParams { page?: string; itemsPerPage?: number; - tags?: string[]; - searchTerm?: string; - startDate?: string; - endDate?: string; orderBy?: OrderBy; } diff --git a/src/short-urls/services/provideServices.ts b/src/short-urls/services/provideServices.ts index 6f1b70dc..bd7c7daf 100644 --- a/src/short-urls/services/provideServices.ts +++ b/src/short-urls/services/provideServices.ts @@ -52,7 +52,6 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator, withRouter: // Services bottle.serviceFactory('SearchBar', SearchBar, 'ColorGenerator'); - bottle.decorator('SearchBar', connect([ 'shortUrlsListParams' ])); bottle.decorator('SearchBar', withRouter); // Actions From 0642443aa91c4bc7731c3b17ecdccdb5f317f342 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 11 Nov 2021 21:32:28 +0100 Subject: [PATCH 10/11] Updated changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 33b49f27..d291f4d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), * [#490](https://github.com/shlinkio/shlink-web-client/issues/490) Now a server can be marked as auto-connect, skipping home screen when that happens. * [#492](https://github.com/shlinkio/shlink-web-client/issues/492) Improved tags table, by supporting sorting by column and making the header sticky. * [#515](https://github.com/shlinkio/shlink-web-client/issues/515) Allowed to sort tags even when using the cards display mode. +* [#518](https://github.com/shlinkio/shlink-web-client/issues/518) Improved short URLs list filtering by moving selected tags, search text and dates to the query string, allowing to navigate back and forth or even bookmark filters. ### Changed * Moved ci workflow to external repo and reused @@ -24,6 +25,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Fixed * [#252](https://github.com/shlinkio/shlink-web-client/issues/252) Fixed visits coming from mercure being added in real time, even when selected date interval does not match tha visit's date. +* [#48](https://github.com/shlinkio/shlink-web-client/issues/48) Fixed error when selected page gets out of range after filtering short URLs list by text, tags or dates. Now the page is reset to 1 in any of those cases. ## [3.3.2] - 2021-10-17 From 7bc3819ebec44c0bddbab6b111e19e9cdcf6507b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 11 Nov 2021 21:38:37 +0100 Subject: [PATCH 11/11] Fixed TS error in SearchBar test --- test/short-urls/SearchBar.test.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/test/short-urls/SearchBar.test.tsx b/test/short-urls/SearchBar.test.tsx index f698f665..c4697abe 100644 --- a/test/short-urls/SearchBar.test.tsx +++ b/test/short-urls/SearchBar.test.tsx @@ -18,7 +18,6 @@ describe('', () => { const createWrapper = (props: Partial = {}) => { wrapper = shallow( ({ push })} location={Mock.of({ search: '' })} match={Mock.of>({ params: { serverId: '1' } })}
Tag {renderOrderIcon('tag')} + Tag + - Short URLs {renderOrderIcon('shortUrls')} + Short URLs - Visits {renderOrderIcon('visits')} + Visits
- + {prettify(tag.shortUrls)}
Something went wrong while loading short URLs :( + Something went wrong while loading short URLs :( +