From 6876c48423987812c5c13b5120eeff5ff6bb628d Mon Sep 17 00:00:00 2001 From: Oliver Sand Date: Mon, 16 Aug 2021 11:17:32 +0200 Subject: [PATCH] Implement `offset` and `limit` based paging in frontend Signed-off-by: Oliver Sand --- .../app/src/components/search/SearchPage.tsx | 93 +++++++------------ plugins/search/src/apis.test.ts | 6 +- .../components/SearchBar/SearchBar.test.tsx | 2 +- .../SearchContext/SearchContext.test.tsx | 42 ++++++--- .../SearchContext/SearchContext.tsx | 47 +++++----- .../SearchFilter/SearchFilter.test.tsx | 2 +- .../components/SearchPage/SearchPage.test.tsx | 24 +++-- .../SearchResult/SearchResult.test.tsx | 66 ++++++++++++- .../components/SearchResult/SearchResult.tsx | 26 +++++- .../components/SearchType/SearchType.test.tsx | 2 +- 10 files changed, 193 insertions(+), 117 deletions(-) diff --git a/packages/app/src/components/search/SearchPage.tsx b/packages/app/src/components/search/SearchPage.tsx index 9b0ae7eecf..1c8d750e08 100644 --- a/packages/app/src/components/search/SearchPage.tsx +++ b/packages/app/src/components/search/SearchPage.tsx @@ -14,20 +14,18 @@ * limitations under the License. */ -import React, { useState } from 'react'; -import { makeStyles, Theme, Grid, List, Paper } from '@material-ui/core'; -import Pagination from '@material-ui/lab/Pagination'; +import { Content, Header, Lifecycle, Page } from '@backstage/core-components'; import { CatalogResultListItem } from '@backstage/plugin-catalog'; import { + DefaultResultListItem, SearchBar, SearchFilter, SearchResult, SearchType, - DefaultResultListItem, } from '@backstage/plugin-search'; -import { Content, Header, Lifecycle, Page } from '@backstage/core-components'; import { DocsResultListItem } from '@backstage/plugin-techdocs'; -import { SearchResultSet } from '@backstage/search-common'; +import { Grid, List, makeStyles, Paper, Theme } from '@material-ui/core'; +import React from 'react'; const useStyles = makeStyles((theme: Theme) => ({ bar: { @@ -43,59 +41,6 @@ const useStyles = makeStyles((theme: Theme) => ({ }, })); -// TODO: Move this into the search plugin once pagination is natively supported. -// See: https://github.com/backstage/backstage/issues/6062 -const SearchResultList = ({ results }: SearchResultSet) => { - const pageSize = 10; - const [page, setPage] = useState(1); - const changePage = (_: any, pageIndex: number) => { - setPage(pageIndex); - }; - const pageAmount = Math.ceil((results.length || 0) / pageSize); - return ( - <> - - {results - .slice(pageSize * (page - 1), pageSize * page) - .map(({ type, document }) => { - switch (type) { - case 'software-catalog': - return ( - - ); - case 'techdocs': - return ( - - ); - default: - return ( - - ); - } - })} - - {pageAmount > 1 && ( - - )} - - ); -}; - const SearchPage = () => { const classes = useStyles(); return ( @@ -129,7 +74,35 @@ const SearchPage = () => { - {({ results }) => } + {({ results }) => ( + + {results.map(({ type, document }) => { + switch (type) { + case 'software-catalog': + return ( + + ); + case 'techdocs': + return ( + + ); + default: + return ( + + ); + } + })} + + )} diff --git a/plugins/search/src/apis.test.ts b/plugins/search/src/apis.test.ts index cc59ca1a77..38f34bb91b 100644 --- a/plugins/search/src/apis.test.ts +++ b/plugins/search/src/apis.test.ts @@ -21,7 +21,7 @@ describe('apis', () => { term: '', filters: {}, types: [], - pageCursor: '', + page: {}, }; const baseUrl = 'https://base-url.com/'; @@ -53,7 +53,7 @@ describe('apis', () => { it('Fetch is called with expected URL (including stringified Q params)', async () => { await client.query(query); expect(getBaseUrl).toHaveBeenLastCalledWith('search/query'); - expect(fetch).toHaveBeenLastCalledWith(`${baseUrl}?term=&pageCursor=`, { + expect(fetch).toHaveBeenLastCalledWith(`${baseUrl}?term=`, { headers: {}, }); }); @@ -65,7 +65,7 @@ describe('apis', () => { }); await authedClient.query(query); expect(getBaseUrl).toHaveBeenLastCalledWith('search/query'); - expect(fetch).toHaveBeenLastCalledWith(`${baseUrl}?term=&pageCursor=`, { + expect(fetch).toHaveBeenLastCalledWith(`${baseUrl}?term=`, { headers: { Authorization: `Bearer ${token}` }, }); }); diff --git a/plugins/search/src/components/SearchBar/SearchBar.test.tsx b/plugins/search/src/components/SearchBar/SearchBar.test.tsx index b99b375252..acfeb2cb8b 100644 --- a/plugins/search/src/components/SearchBar/SearchBar.test.tsx +++ b/plugins/search/src/components/SearchBar/SearchBar.test.tsx @@ -30,7 +30,7 @@ jest.mock('@backstage/core-plugin-api', () => ({ describe('SearchBar', () => { const initialState = { term: '', - pageCursor: '', + page: {}, filters: {}, types: ['*'], }; diff --git a/plugins/search/src/components/SearchContext/SearchContext.test.tsx b/plugins/search/src/components/SearchContext/SearchContext.test.tsx index 4de2a8f0a6..c1618be5ec 100644 --- a/plugins/search/src/components/SearchContext/SearchContext.test.tsx +++ b/plugins/search/src/components/SearchContext/SearchContext.test.tsx @@ -38,7 +38,7 @@ describe('SearchContext', () => { const initialState = { term: '', - pageCursor: '', + page: {}, filters: {}, types: ['*'], }; @@ -93,22 +93,26 @@ describe('SearchContext', () => { initialProps: { initialState: { ...initialState, - pageCursor: '1', + page: { offset: 0, limit: 25 }, }, }, }); await waitForNextUpdate(); - expect(result.current.pageCursor).toBe('1'); + expect(result.current.page).toEqual({ offset: 0, limit: 25 }); act(() => { result.current.setTerm('first term'); }); + act(() => { + result.current.setPage({ offset: 75, limit: 25 }); + }); + await waitForNextUpdate(); - expect(result.current.pageCursor).toBe('1'); + expect(result.current.page).toEqual({ offset: 75, limit: 25 }); act(() => { result.current.setTerm('second term'); @@ -116,7 +120,7 @@ describe('SearchContext', () => { await waitForNextUpdate(); - expect(result.current.pageCursor).toBe(''); + expect(result.current.page).toEqual({ offset: 0, limit: 25 }); }); describe('Performs search (and sets results)', () => { @@ -139,7 +143,10 @@ describe('SearchContext', () => { await waitForNextUpdate(); expect(query).toHaveBeenLastCalledWith({ - ...initialState, + filters: {}, + types: ['*'], + limit: undefined, + offset: undefined, term, }); }); @@ -163,12 +170,15 @@ describe('SearchContext', () => { await waitForNextUpdate(); expect(query).toHaveBeenLastCalledWith({ - ...initialState, filters, + types: ['*'], + limit: undefined, + offset: undefined, + term: '', }); }); - it('When pageCursor is set', async () => { + it('When page is set', async () => { const { result, waitForNextUpdate } = renderHook(() => useSearch(), { wrapper, initialProps: { @@ -178,17 +188,20 @@ describe('SearchContext', () => { await waitForNextUpdate(); - const pageCursor = 'pageCursor'; + const page = { offset: 25, limit: 50 }; act(() => { - result.current.setPageCursor(pageCursor); + result.current.setPage(page); }); await waitForNextUpdate(); expect(query).toHaveBeenLastCalledWith({ - ...initialState, - pageCursor, + filters: {}, + types: ['*'], + limit: 50, + offset: 25, + term: '', }); }); @@ -211,8 +224,11 @@ describe('SearchContext', () => { await waitForNextUpdate(); expect(query).toHaveBeenLastCalledWith({ - ...initialState, types, + filters: {}, + limit: undefined, + offset: undefined, + term: '', }); }); }); diff --git a/plugins/search/src/components/SearchContext/SearchContext.tsx b/plugins/search/src/components/SearchContext/SearchContext.tsx index 45a74f5355..018496876b 100644 --- a/plugins/search/src/components/SearchContext/SearchContext.tsx +++ b/plugins/search/src/components/SearchContext/SearchContext.tsx @@ -14,19 +14,21 @@ * limitations under the License. */ -import React, { - PropsWithChildren, - createContext, - useContext, - useState, - useEffect, -} from 'react'; -import { useAsync, usePrevious } from 'react-use'; -import { SearchResultSet } from '@backstage/search-common'; -import { searchApiRef } from '../../apis'; -import { AsyncState } from 'react-use/lib/useAsync'; import { JsonObject } from '@backstage/config'; import { useApi } from '@backstage/core-plugin-api'; +import { SearchResultSet } from '@backstage/search-common'; +import React, { + createContext, + PropsWithChildren, + useContext, + useEffect, + useState, +} from 'react'; +import { useAsync, usePrevious } from 'react-use'; +import { AsyncState } from 'react-use/lib/useAsync'; +import { searchApiRef } from '../../apis'; + +type Page = { limit?: number; offset?: number }; type SearchContextValue = { result: AsyncState; @@ -36,13 +38,13 @@ type SearchContextValue = { setTypes: React.Dispatch>; filters: JsonObject; setFilters: React.Dispatch>; - pageCursor: string; - setPageCursor: React.Dispatch>; + page: Page; + setPage: React.Dispatch>; }; type SettableSearchContext = Omit< SearchContextValue, - 'result' | 'setTerm' | 'setTypes' | 'setFilters' | 'setPageCursor' + 'result' | 'setTerm' | 'setTypes' | 'setFilters' | 'setPage' >; export const SearchContext = createContext( @@ -52,14 +54,14 @@ export const SearchContext = createContext( export const SearchContextProvider = ({ initialState = { term: '', - pageCursor: '', + page: {}, filters: {}, types: [], }, children, }: PropsWithChildren<{ initialState?: SettableSearchContext }>) => { const searchApi = useApi(searchApiRef); - const [pageCursor, setPageCursor] = useState(initialState.pageCursor); + const [page, setPage] = useState(initialState.page); const [filters, setFilters] = useState(initialState.filters); const [term, setTerm] = useState(initialState.term); const [types, setTypes] = useState(initialState.types); @@ -70,18 +72,19 @@ export const SearchContextProvider = ({ searchApi.query({ term, filters, - pageCursor, + offset: page?.offset, + limit: page?.limit, types, }), - [term, filters, types, pageCursor], + [term, filters, types, page], ); useEffect(() => { // Any time a term is reset, we want to start from page 0. if (term && prevTerm && term !== prevTerm) { - setPageCursor(''); + setPage(initialState.page); } - }, [term, prevTerm]); + }, [term, prevTerm, initialState.page]); const value: SearchContextValue = { result, @@ -91,8 +94,8 @@ export const SearchContextProvider = ({ setTerm, types, setTypes, - pageCursor, - setPageCursor, + page, + setPage, }; return ; diff --git a/plugins/search/src/components/SearchFilter/SearchFilter.test.tsx b/plugins/search/src/components/SearchFilter/SearchFilter.test.tsx index e8c2f048b9..b5dd56bfb6 100644 --- a/plugins/search/src/components/SearchFilter/SearchFilter.test.tsx +++ b/plugins/search/src/components/SearchFilter/SearchFilter.test.tsx @@ -32,7 +32,7 @@ describe('SearchFilter', () => { term: '', filters: {}, types: [], - pageCursor: '', + page: {}, }; const name = 'field'; diff --git a/plugins/search/src/components/SearchPage/SearchPage.test.tsx b/plugins/search/src/components/SearchPage/SearchPage.test.tsx index c387d57b60..91568cddbe 100644 --- a/plugins/search/src/components/SearchPage/SearchPage.test.tsx +++ b/plugins/search/src/components/SearchPage/SearchPage.test.tsx @@ -74,21 +74,25 @@ describe('SearchPage', () => { const expectedTerm = 'justin bieber'; const expectedTypes = ['software-catalog']; const expectedFilters = { [expectedFilterField]: expectedFilterValue }; - const expectedPageCursor = 'page2-or-something'; + const expectedOffset = 25; + const expectedLimit = 50; - // e.g. ?query=petstore&pageCursor=1&filters[lifecycle][]=experimental&filters[kind]=Component + // e.g. ?query=petstore&offset=25&limit=50&filters[lifecycle][]=experimental&filters[kind]=Component (useLocation as jest.Mock).mockReturnValueOnce({ - search: `?query=${expectedTerm}&types[]=${expectedTypes[0]}&filters[${expectedFilterField}]=${expectedFilterValue}&pageCursor=${expectedPageCursor}`, + search: `?query=${expectedTerm}&types[]=${expectedTypes[0]}&filters[${expectedFilterField}]=${expectedFilterValue}&offset=${expectedOffset}&limit=${expectedLimit}`, }); // When we render the page... await renderInTestApp(); - // Then search context should be set with these values... - expect(setTermMock).toHaveBeenCalledWith(expectedTerm); - expect(setTypesMock).toHaveBeenCalledWith(expectedTypes); - expect(setPageCursorMock).toHaveBeenCalledWith(expectedPageCursor); - expect(setFiltersMock).toHaveBeenCalledWith(expectedFilters); + // Then search context should be initialized with these values... + const calls = (SearchContextProvider as jest.Mock).mock.calls[0]; + const actualInitialState = calls[0].initialState; + expect(actualInitialState.term).toEqual(expectedTerm); + expect(actualInitialState.types).toEqual(expectedTypes); + expect(actualInitialState.page.limit).toEqual(expectedLimit); + expect(actualInitialState.page.offset).toEqual(expectedOffset); + expect(actualInitialState.filters).toStrictEqual(expectedFilters); }); it('renders provided router element', async () => { @@ -108,7 +112,7 @@ describe('SearchPage', () => { (useSearch as jest.Mock).mockReturnValueOnce({ term: 'bieber', types: ['software-catalog'], - pageCursor: 'page2-or-something', + page: { offset: 25, limit: 50 }, filters: { anyKey: 'anyValue' }, setTerm: setTermMock, setTypes: setTypesMock, @@ -116,7 +120,7 @@ describe('SearchPage', () => { setPageCursor: setPageCursorMock, }); const expectedLocation = encodeURI( - '?query=bieber&types[]=software-catalog&pageCursor=page2-or-something&filters[anyKey]=anyValue', + '?query=bieber&types[]=software-catalog&offset=25&limit=50&filters[anyKey]=anyValue', ); await renderInTestApp(); diff --git a/plugins/search/src/components/SearchResult/SearchResult.test.tsx b/plugins/search/src/components/SearchResult/SearchResult.test.tsx index bb8f393d67..d28bcc9136 100644 --- a/plugins/search/src/components/SearchResult/SearchResult.test.tsx +++ b/plugins/search/src/components/SearchResult/SearchResult.test.tsx @@ -24,6 +24,7 @@ jest.mock('../SearchContext', () => ({ ...jest.requireActual('../SearchContext'), useSearch: jest.fn().mockReturnValue({ result: {}, + page: {}, }), })); @@ -93,16 +94,73 @@ describe('SearchResult', () => { it('Calls children with results set to result.value', async () => { (useSearch as jest.Mock).mockReturnValueOnce({ - result: { loading: false, error: '', value: { results: [] } }, + result: { + loading: false, + error: '', + value: { + totalCount: 1, + results: [ + { + type: 'some-type', + document: { + title: 'some-title', + text: 'some-text', + location: 'some-location', + }, + }, + ], + }, + }, + page: {}, }); - await renderInTestApp( + const { getByText } = await renderInTestApp( {({ results }) => { - expect(results).toEqual([]); - return <>; + return <>Results {results.length}; }} , ); + + expect(getByText('Results 1')).toBeInTheDocument(); + }); + + it('Starts on initial page if no offset is set', async () => { + (useSearch as jest.Mock).mockReturnValueOnce({ + page: {}, + result: { + loading: false, + error: '', + value: { results: [{}], totalCount: 100 }, + }, + }); + + const { getByLabelText } = await renderInTestApp( + {({}) => <>}, + ); + + expect(getByLabelText('page 1')).toHaveAttribute('aria-current', 'true'); + expect(getByLabelText('Go to page 2')).toBeInTheDocument(); + expect(getByLabelText('Go to page 3')).toBeInTheDocument(); + expect(getByLabelText('Go to page 4')).toBeInTheDocument(); + }); + + it('Shows the right page', async () => { + (useSearch as jest.Mock).mockReturnValueOnce({ + page: { offset: 25 }, + result: { + loading: false, + error: '', + value: { results: [{}], totalCount: 63 }, + }, + }); + + const { getByLabelText } = await renderInTestApp( + {({}) => <>}, + ); + + expect(getByLabelText('Go to page 1')).toBeInTheDocument(); + expect(getByLabelText('page 2')).toHaveAttribute('aria-current', 'true'); + expect(getByLabelText('Go to page 3')).toBeInTheDocument(); }); }); diff --git a/plugins/search/src/components/SearchResult/SearchResult.tsx b/plugins/search/src/components/SearchResult/SearchResult.tsx index 26a2484a86..5ac2c86a79 100644 --- a/plugins/search/src/components/SearchResult/SearchResult.tsx +++ b/plugins/search/src/components/SearchResult/SearchResult.tsx @@ -20,16 +20,20 @@ import { ResponseErrorPanel, } from '@backstage/core-components'; import { SearchResult } from '@backstage/search-common'; +import { Pagination } from '@material-ui/lab'; import React from 'react'; import { useSearch } from '../SearchContext'; type Props = { children: (results: { results: SearchResult[] }) => JSX.Element; + initialPageSize?: number; }; -const SearchResultComponent = ({ children }: Props) => { +const SearchResultComponent = ({ children, initialPageSize = 25 }: Props) => { const { result: { loading, error, value }, + page, + setPage, } = useSearch(); if (loading) { @@ -48,7 +52,25 @@ const SearchResultComponent = ({ children }: Props) => { return ; } - return children({ results: value.results }); + const pageSize = page.limit ?? initialPageSize; + const totalPages = Math.ceil(value.totalCount / pageSize); + const currentPage = page.offset ? Math.floor(page.offset / pageSize) + 1 : 1; + + const handlePageChange = (_: React.ChangeEvent, pageNum: number) => { + setPage({ offset: (pageNum - 1) * pageSize, limit: pageSize }); + window.scrollTo({ top: 0, left: 0, behavior: 'smooth' }); + }; + + return ( + <> + {children({ results: value.results })} + + + ); }; export { SearchResultComponent as SearchResult }; diff --git a/plugins/search/src/components/SearchType/SearchType.test.tsx b/plugins/search/src/components/SearchType/SearchType.test.tsx index b8bf59d13e..f8171b0779 100644 --- a/plugins/search/src/components/SearchType/SearchType.test.tsx +++ b/plugins/search/src/components/SearchType/SearchType.test.tsx @@ -31,7 +31,7 @@ describe('SearchType', () => { term: '', filters: {}, types: [], - pageCursor: '', + page: {}, }; const name = 'field';