From 95122774b1e6397756453e18287f155a02a34e51 Mon Sep 17 00:00:00 2001 From: Samir Benzenine Date: Thu, 23 Jan 2025 11:55:48 +0000 Subject: [PATCH 1/2] Newswires UI: add pagination --- newswires/client/src/Feed.tsx | 4 +- newswires/client/src/WireItemTable.tsx | 67 ++++++++++++------- .../client/src/context/SearchContext.tsx | 40 ++++++++++- .../client/src/context/SearchReducer.test.ts | 43 ++++++++++++ newswires/client/src/context/SearchReducer.ts | 37 ++++++++++ .../client/src/context/fetchResults.test.ts | 26 +++++-- newswires/client/src/context/fetchResults.ts | 10 +-- newswires/client/src/urlState.ts | 13 +++- 8 files changed, 200 insertions(+), 40 deletions(-) diff --git a/newswires/client/src/Feed.tsx b/newswires/client/src/Feed.tsx index 6a45fa81..44bbe7c9 100644 --- a/newswires/client/src/Feed.tsx +++ b/newswires/client/src/Feed.tsx @@ -25,7 +25,9 @@ export const Feed = () => { titleSize="s" /> )} - {(status == 'success' || status == 'offline') && + {(status == 'success' || + status == 'offline' || + status == 'loading-more') && queryData.results.length > 0 && ( )} diff --git a/newswires/client/src/WireItemTable.tsx b/newswires/client/src/WireItemTable.tsx index a56d1cc9..fefbf611 100644 --- a/newswires/client/src/WireItemTable.tsx +++ b/newswires/client/src/WireItemTable.tsx @@ -1,4 +1,5 @@ import { + EuiButton, EuiFlexGroup, euiScreenReaderOnly, EuiTable, @@ -31,38 +32,54 @@ const fadeOutBackground = css` `; export const WireItemTable = ({ wires }: { wires: WireData[] }) => { - const { config, handleSelectItem } = useSearch(); + const { + config, + handleSelectItem, + state: { status }, + loadMoreResults, + } = useSearch(); const selectedWireId = config.itemId; return ( - - + + + Headline + Version Created + + + {wires.map(({ id, supplier, content, isFromRefresh, highlight }) => ( + + ))} + + + - Headline - Version Created - - - {wires.map(({ id, supplier, content, isFromRefresh, highlight }) => ( - - ))} - - + {status === 'loading-more' ? 'Loading' : 'Load more'} + + ); }; diff --git a/newswires/client/src/context/SearchContext.tsx b/newswires/client/src/context/SearchContext.tsx index f6332a13..b7d25840 100644 --- a/newswires/client/src/context/SearchContext.tsx +++ b/newswires/client/src/context/SearchContext.tsx @@ -43,6 +43,13 @@ const StateSchema = z.discriminatedUnion('status', [ successfulQueryHistory: SearchHistorySchema, autoUpdate: z.boolean().default(true), }), + z.object({ + status: z.literal('loading-more'), + error: z.string().optional(), + queryData: WiresQueryResponseSchema, + successfulQueryHistory: SearchHistorySchema, + autoUpdate: z.boolean().default(true), + }), z.object({ status: z.literal('success'), error: z.string().optional(), @@ -72,11 +79,16 @@ export type State = z.infer; // Action Schema const ActionSchema = z.discriminatedUnion('type', [ z.object({ type: z.literal('ENTER_QUERY') }), + z.object({ type: z.literal('LOAD_MORE_RESULTS') }), z.object({ type: z.literal('FETCH_SUCCESS'), query: QuerySchema, data: WiresQueryResponseSchema, }), + z.object({ + type: z.literal('APPEND_RESULTS'), + data: WiresQueryResponseSchema, + }), z.object({ type: z.literal('FETCH_ERROR'), error: z.string() }), z.object({ type: z.literal('RETRY') }), z.object({ type: z.literal('SELECT_ITEM'), item: z.string().optional() }), @@ -100,6 +112,7 @@ export type SearchContextShape = { handleNextItem: () => void; handlePreviousItem: () => void; toggleAutoUpdate: () => void; + loadMoreResults: () => void; }; export const SearchContext: Context = createContext(null); @@ -167,7 +180,27 @@ export function SearchContextProvider({ children }: PropsWithChildren) { }); } - if (state.status === 'success' || state.status === 'offline') { + if (state.status === 'loading-more') { + fetchResults(currentConfig.query, { + beforeId: Math.min( + ...state.queryData.results.map((wire) => wire.id), + ).toString(), + }) + .then((data) => { + dispatch({ type: 'APPEND_RESULTS', data }); + }) + .catch((error) => { + const errorMessage = + error instanceof Error ? error.message : 'unknown error'; + dispatch({ type: 'FETCH_ERROR', error: errorMessage }); + }); + } + + if ( + state.status === 'success' || + state.status === 'offline' || + state.status === 'loading-more' + ) { pollingInterval = setInterval(() => { if (state.autoUpdate) { const sinceId = @@ -274,6 +307,10 @@ export function SearchContextProvider({ children }: PropsWithChildren) { dispatch({ type: 'TOGGLE_AUTO_UPDATE' }); }; + const loadMoreResults = () => { + dispatch({ type: 'LOAD_MORE_RESULTS' }); + }; + return ( {children} diff --git a/newswires/client/src/context/SearchReducer.test.ts b/newswires/client/src/context/SearchReducer.test.ts index 37285280..c6011c85 100644 --- a/newswires/client/src/context/SearchReducer.test.ts +++ b/newswires/client/src/context/SearchReducer.test.ts @@ -15,6 +15,12 @@ describe('SearchReducer', () => { queryData: { results: [sampleWireData] }, }; + const loadingMoreState: State = { + ...initialState, + status: 'loading-more', + queryData: { results: [sampleWireData] }, + }; + const offlineState: State = { status: 'offline', queryData: { results: [sampleWireData] }, @@ -98,6 +104,31 @@ describe('SearchReducer', () => { }); }); + it(`should handle APPEND_RESULTS action in loading-more state`, () => { + const state: State = { + ...loadingMoreState, + queryData: { results: [{ ...sampleWireData, id: 2 }] }, + }; + + const action: Action = { + type: 'APPEND_RESULTS', + data: { results: [{ ...sampleWireData, id: 1 }] }, + }; + + const newState = SearchReducer(state, action); + + expect(newState.status).toBe('success'); + expect(newState.queryData?.results).toHaveLength(2); + expect(newState.queryData?.results).toContainEqual({ + ...sampleWireData, + id: 1, + }); + expect(newState.queryData?.results).toContainEqual({ + ...sampleWireData, + id: 2, + }); + }); + [ { state: initialState, expectedStatus: 'error' }, { state: successState, expectedStatus: 'offline' }, @@ -136,4 +167,16 @@ describe('SearchReducer', () => { expect(newState.status).toBe('loading'); }); }); + + [successState, offlineState].forEach((state) => { + it(`should handle LOAD_MORE_RESULTS action in ${state.status} state`, () => { + const action: Action = { + type: 'LOAD_MORE_RESULTS', + }; + + const newState = SearchReducer(state, action); + + expect(newState.status).toBe('loading-more'); + }); + }); }); diff --git a/newswires/client/src/context/SearchReducer.ts b/newswires/client/src/context/SearchReducer.ts index 5e5e6ad4..d9cb7803 100644 --- a/newswires/client/src/context/SearchReducer.ts +++ b/newswires/client/src/context/SearchReducer.ts @@ -29,6 +29,20 @@ function mergeQueryData( } } +function appendQueryData( + existing: WiresQueryResponse | undefined, + newData: WiresQueryResponse, +): WiresQueryResponse { + if (existing) { + return { + ...newData, + results: [...existing.results, ...newData.results], + }; + } else { + return newData; + } +} + function getUpdatedHistory( previousHistory: SearchHistory, newQuery: Query, @@ -85,6 +99,17 @@ export const SearchReducer = (state: State, action: Action): State => { default: return state; } + case 'APPEND_RESULTS': + switch (state.status) { + case 'loading-more': + return { + ...state, + status: 'success', + queryData: appendQueryData(state.queryData, action.data), + }; + default: + return state; + } case 'FETCH_ERROR': switch (state.status) { case 'loading': @@ -117,6 +142,18 @@ export const SearchReducer = (state: State, action: Action): State => { ...state, status: 'loading', }; + case 'LOAD_MORE_RESULTS': + switch (state.status) { + case 'success': + case 'offline': + return { + ...state, + status: 'loading-more', + }; + default: + return state; + } + default: return state; } diff --git a/newswires/client/src/context/fetchResults.test.ts b/newswires/client/src/context/fetchResults.test.ts index ae735f42..288c0222 100644 --- a/newswires/client/src/context/fetchResults.test.ts +++ b/newswires/client/src/context/fetchResults.test.ts @@ -24,7 +24,7 @@ describe('fetchResults', () => { const mockQuery = { q: 'value' }; await fetchResults(mockQuery); - expect(paramsToQuerystring).toHaveBeenCalledWith(mockQuery); + expect(paramsToQuerystring).toHaveBeenCalledWith(mockQuery, {}); expect(pandaFetch).toHaveBeenCalledWith('/api/search?queryString', { headers: { Accept: 'application/json' }, }); @@ -70,11 +70,25 @@ describe('fetchResults', () => { it('should append sinceId to the query if provided', async () => { const mockQuery = { q: 'value' }; - await fetchResults(mockQuery, '123'); + await fetchResults(mockQuery, { sinceId: '123' }); - expect(paramsToQuerystring).toHaveBeenCalledWith({ - ...mockQuery, - sinceId: '123', - }); + expect(paramsToQuerystring).toHaveBeenCalledWith( + { + ...mockQuery, + }, + { sinceId: '123' }, + ); + }); + + it('should append beforeId to the query if provided', async () => { + const mockQuery = { q: 'value' }; + await fetchResults(mockQuery, { beforeId: '123' }); + + expect(paramsToQuerystring).toHaveBeenCalledWith( + { + ...mockQuery, + }, + { beforeId: '123' }, + ); }); }); diff --git a/newswires/client/src/context/fetchResults.ts b/newswires/client/src/context/fetchResults.ts index e25ee8d7..caa76655 100644 --- a/newswires/client/src/context/fetchResults.ts +++ b/newswires/client/src/context/fetchResults.ts @@ -5,12 +5,12 @@ import { paramsToQuerystring } from '../urlState.ts'; export const fetchResults = async ( query: Query, - sinceId: string | undefined = undefined, + additionalParams: { + sinceId?: string; + beforeId?: string; + } = {}, ): Promise => { - const queryToSerialise = sinceId - ? { ...query, sinceId: sinceId.toString() } - : query; - const queryString = paramsToQuerystring(queryToSerialise); + const queryString = paramsToQuerystring(query, additionalParams); const response = await pandaFetch(`/api/search${queryString}`, { headers: { Accept: 'application/json', diff --git a/newswires/client/src/urlState.ts b/newswires/client/src/urlState.ts index 6d975027..b5863ecb 100644 --- a/newswires/client/src/urlState.ts +++ b/newswires/client/src/urlState.ts @@ -70,7 +70,13 @@ export const configToUrl = (config: Config): string => { export const paramsToQuerystring = ( config: Query, - sinceId: number | undefined = undefined, + { + sinceId, + beforeId, + }: { + sinceId?: string; + beforeId?: string; + } = {}, ): string => { const params = Object.entries(config).reduce>( (acc, [k, v]) => { @@ -89,7 +95,10 @@ export const paramsToQuerystring = ( [], ); if (sinceId !== undefined) { - params.push(['sinceId', sinceId.toString()]); + params.push(['sinceId', sinceId]); + } + if (beforeId !== undefined) { + params.push(['beforeId', beforeId]); } const querystring = new URLSearchParams(params).toString(); return querystring.length !== 0 ? `?${querystring}` : ''; From 0fe356da2020c3e16815f75f712c959771379a63 Mon Sep 17 00:00:00 2001 From: Samir Benzenine Date: Fri, 24 Jan 2025 14:27:35 +0000 Subject: [PATCH 2/2] Newswires UI: Update pagination logic --- newswires/client/src/Feed.tsx | 4 +- newswires/client/src/WireItemTable.tsx | 29 ++++++++---- .../client/src/context/SearchContext.tsx | 46 ++++++------------- .../client/src/context/SearchReducer.test.ts | 22 +-------- newswires/client/src/context/SearchReducer.ts | 27 ++--------- 5 files changed, 41 insertions(+), 87 deletions(-) diff --git a/newswires/client/src/Feed.tsx b/newswires/client/src/Feed.tsx index 44bbe7c9..6a45fa81 100644 --- a/newswires/client/src/Feed.tsx +++ b/newswires/client/src/Feed.tsx @@ -25,9 +25,7 @@ export const Feed = () => { titleSize="s" /> )} - {(status == 'success' || - status == 'offline' || - status == 'loading-more') && + {(status == 'success' || status == 'offline') && queryData.results.length > 0 && ( )} diff --git a/newswires/client/src/WireItemTable.tsx b/newswires/client/src/WireItemTable.tsx index fefbf611..536ab025 100644 --- a/newswires/client/src/WireItemTable.tsx +++ b/newswires/client/src/WireItemTable.tsx @@ -14,6 +14,7 @@ import { useEuiTheme, } from '@elastic/eui'; import { css } from '@emotion/react'; +import { useState } from 'react'; import sanitizeHtml from 'sanitize-html'; import { useSearch } from './context/SearchContext.tsx'; import { formatTimestamp } from './formatTimestamp'; @@ -32,15 +33,24 @@ const fadeOutBackground = css` `; export const WireItemTable = ({ wires }: { wires: WireData[] }) => { - const { - config, - handleSelectItem, - state: { status }, - loadMoreResults, - } = useSearch(); + const { config, handleSelectItem, loadMoreResults } = useSearch(); + + const [isLoadingMore, setIsLoadingMore] = useState(false); const selectedWireId = config.itemId; + const handleLoadMoreResults = () => { + if (wires.length > 0) { + setIsLoadingMore(true); + + const beforeId = Math.min(...wires.map((wire) => wire.id)).toString(); + + void loadMoreResults(beforeId).finally(() => { + setIsLoadingMore(false); + }); + } + }; + return ( <> { - {status === 'loading-more' ? 'Loading' : 'Load more'} + {isLoadingMore ? 'Loading' : 'Load more'} ); @@ -118,6 +128,7 @@ const WireDataRow = ({ background-color: ${primaryBgColor}; border-left: 4px solid ${theme.euiTheme.colors.accent}; } + border-left: 4px solid ${selected ? theme.euiTheme.colors.primary : 'transparent'}; ${isFromRefresh ? fadeOutBackground : ''} diff --git a/newswires/client/src/context/SearchContext.tsx b/newswires/client/src/context/SearchContext.tsx index b7d25840..85a493e5 100644 --- a/newswires/client/src/context/SearchContext.tsx +++ b/newswires/client/src/context/SearchContext.tsx @@ -43,13 +43,6 @@ const StateSchema = z.discriminatedUnion('status', [ successfulQueryHistory: SearchHistorySchema, autoUpdate: z.boolean().default(true), }), - z.object({ - status: z.literal('loading-more'), - error: z.string().optional(), - queryData: WiresQueryResponseSchema, - successfulQueryHistory: SearchHistorySchema, - autoUpdate: z.boolean().default(true), - }), z.object({ status: z.literal('success'), error: z.string().optional(), @@ -79,7 +72,6 @@ export type State = z.infer; // Action Schema const ActionSchema = z.discriminatedUnion('type', [ z.object({ type: z.literal('ENTER_QUERY') }), - z.object({ type: z.literal('LOAD_MORE_RESULTS') }), z.object({ type: z.literal('FETCH_SUCCESS'), query: QuerySchema, @@ -112,7 +104,7 @@ export type SearchContextShape = { handleNextItem: () => void; handlePreviousItem: () => void; toggleAutoUpdate: () => void; - loadMoreResults: () => void; + loadMoreResults: (beforeId: string) => Promise; }; export const SearchContext: Context = createContext(null); @@ -180,27 +172,7 @@ export function SearchContextProvider({ children }: PropsWithChildren) { }); } - if (state.status === 'loading-more') { - fetchResults(currentConfig.query, { - beforeId: Math.min( - ...state.queryData.results.map((wire) => wire.id), - ).toString(), - }) - .then((data) => { - dispatch({ type: 'APPEND_RESULTS', data }); - }) - .catch((error) => { - const errorMessage = - error instanceof Error ? error.message : 'unknown error'; - dispatch({ type: 'FETCH_ERROR', error: errorMessage }); - }); - } - - if ( - state.status === 'success' || - state.status === 'offline' || - state.status === 'loading-more' - ) { + if (state.status === 'success' || state.status === 'offline') { pollingInterval = setInterval(() => { if (state.autoUpdate) { const sinceId = @@ -209,7 +181,7 @@ export function SearchContextProvider({ children }: PropsWithChildren) { ...state.queryData.results.map((wire) => wire.id), ).toString() : undefined; - fetchResults(currentConfig.query, sinceId) + fetchResults(currentConfig.query, { sinceId }) .then((data) => { dispatch({ type: 'UPDATE_RESULTS', data }); }) @@ -307,8 +279,16 @@ export function SearchContextProvider({ children }: PropsWithChildren) { dispatch({ type: 'TOGGLE_AUTO_UPDATE' }); }; - const loadMoreResults = () => { - dispatch({ type: 'LOAD_MORE_RESULTS' }); + const loadMoreResults = async (beforeId: string): Promise => { + return fetchResults(currentConfig.query, { beforeId }) + .then((data) => { + dispatch({ type: 'APPEND_RESULTS', data }); + }) + .catch((error) => { + const errorMessage = + error instanceof Error ? error.message : 'unknown error'; + dispatch({ type: 'FETCH_ERROR', error: errorMessage }); + }); }; return ( diff --git a/newswires/client/src/context/SearchReducer.test.ts b/newswires/client/src/context/SearchReducer.test.ts index c6011c85..0e9ccef1 100644 --- a/newswires/client/src/context/SearchReducer.test.ts +++ b/newswires/client/src/context/SearchReducer.test.ts @@ -15,12 +15,6 @@ describe('SearchReducer', () => { queryData: { results: [sampleWireData] }, }; - const loadingMoreState: State = { - ...initialState, - status: 'loading-more', - queryData: { results: [sampleWireData] }, - }; - const offlineState: State = { status: 'offline', queryData: { results: [sampleWireData] }, @@ -104,9 +98,9 @@ describe('SearchReducer', () => { }); }); - it(`should handle APPEND_RESULTS action in loading-more state`, () => { + it(`should handle APPEND_RESULTS action in success state`, () => { const state: State = { - ...loadingMoreState, + ...successState, queryData: { results: [{ ...sampleWireData, id: 2 }] }, }; @@ -167,16 +161,4 @@ describe('SearchReducer', () => { expect(newState.status).toBe('loading'); }); }); - - [successState, offlineState].forEach((state) => { - it(`should handle LOAD_MORE_RESULTS action in ${state.status} state`, () => { - const action: Action = { - type: 'LOAD_MORE_RESULTS', - }; - - const newState = SearchReducer(state, action); - - expect(newState.status).toBe('loading-more'); - }); - }); }); diff --git a/newswires/client/src/context/SearchReducer.ts b/newswires/client/src/context/SearchReducer.ts index d9cb7803..265cabb1 100644 --- a/newswires/client/src/context/SearchReducer.ts +++ b/newswires/client/src/context/SearchReducer.ts @@ -100,16 +100,11 @@ export const SearchReducer = (state: State, action: Action): State => { return state; } case 'APPEND_RESULTS': - switch (state.status) { - case 'loading-more': - return { - ...state, - status: 'success', - queryData: appendQueryData(state.queryData, action.data), - }; - default: - return state; - } + return { + ...state, + status: 'success', + queryData: appendQueryData(state.queryData, action.data), + }; case 'FETCH_ERROR': switch (state.status) { case 'loading': @@ -142,18 +137,6 @@ export const SearchReducer = (state: State, action: Action): State => { ...state, status: 'loading', }; - case 'LOAD_MORE_RESULTS': - switch (state.status) { - case 'success': - case 'offline': - return { - ...state, - status: 'loading-more', - }; - default: - return state; - } - default: return state; }