Skip to content

Commit a2af92e

Browse files
Assem-UberCopilot
andauthored
feat: Fetcher updates (#1090)
* fetcher updates * add placeholder for fetcher * update fetcher mock * fetcher start update * update todo * Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix copilot comments * remove extra comment * lint fix * change lastFlattented initial value to -1 * move lastFlattenedPagesCount to a ref and remove shouldContinue condition * fix typo and add mock for configs * update page size test case --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 58356cd commit a2af92e

File tree

8 files changed

+97
-22
lines changed

8 files changed

+97
-22
lines changed

src/views/workflow-history-v2/workflow-history-v2.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import usePageFilters from '@/components/page-filters/hooks/use-page-filters';
44
import decodeUrlParams from '@/utils/decode-url-params';
55

66
import workflowHistoryFiltersConfig from '../workflow-history/config/workflow-history-filters.config';
7-
import WORKFLOW_HISTORY_PAGE_SIZE_CONFIG from '../workflow-history/config/workflow-history-page-size.config';
7+
import { WORKFLOW_HISTORY_PAGE_SIZE_CONFIG } from '../workflow-history/config/workflow-history-page-size.config';
88
import { WorkflowHistoryContext } from '../workflow-history/workflow-history-context-provider/workflow-history-context-provider';
99
import workflowPageQueryParamsConfig from '../workflow-page/config/workflow-page-query-params.config';
1010
import { type WorkflowPageTabContentParams } from '../workflow-page/workflow-page-tab-content/workflow-page-tab-content.types';

src/views/workflow-history/__tests__/workflow-history.test.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@ jest.mock('../hooks/use-workflow-history-fetcher', () => {
3333
const actual = jest.requireActual('../hooks/use-workflow-history-fetcher');
3434
return {
3535
__esModule: true,
36-
default: jest.fn((params) => actual.default(params, 0)), // 0ms throttle for tests
36+
default: jest.fn((params, onEventsChange) =>
37+
actual.default(params, onEventsChange, 0)
38+
), // 0ms throttle for tests
3739
};
3840
});
3941

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,2 @@
1-
const WORKFLOW_HISTORY_PAGE_SIZE_CONFIG = 200;
2-
3-
export default WORKFLOW_HISTORY_PAGE_SIZE_CONFIG;
1+
export const WORKFLOW_HISTORY_PAGE_SIZE_CONFIG = 1000;
2+
export const WORKFLOW_HISTORY_FIRST_PAGE_SIZE_CONFIG = 200;

src/views/workflow-history/helpers/__tests__/workflow-history-fetcher.test.tsx

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,15 @@ const RETRY_COUNT = 3;
1515
let queryClient: QueryClient;
1616
let hoistedFetcher: WorkflowHistoryFetcher;
1717

18+
jest.mock(
19+
'@/views/workflow-history/config/workflow-history-page-size.config',
20+
() => ({
21+
__esModule: true,
22+
WORKFLOW_HISTORY_FIRST_PAGE_SIZE_CONFIG: 200,
23+
WORKFLOW_HISTORY_PAGE_SIZE_CONFIG: 1000,
24+
})
25+
);
26+
1827
describe(WorkflowHistoryFetcher.name, () => {
1928
beforeEach(() => {
2029
queryClient = new QueryClient({
@@ -262,6 +271,35 @@ describe(WorkflowHistoryFetcher.name, () => {
262271
const state = fetcher.getCurrentState();
263272
expect(state.data?.pages.length).toBe(pageCountBefore);
264273
});
274+
275+
it('should use WORKFLOW_HISTORY_FIRST_PAGE_SIZE_CONFIG for the first page', async () => {
276+
const { fetcher, getCapturedPageSizes } = setup(queryClient);
277+
278+
fetcher.start((state) => !state?.data?.pages?.length);
279+
280+
await waitFor(() => {
281+
const state = fetcher.getCurrentState();
282+
expect(state.data?.pages).toHaveLength(1);
283+
});
284+
285+
const pageSizes = getCapturedPageSizes();
286+
expect(pageSizes[0]).toBe('200');
287+
});
288+
289+
it('should use WORKFLOW_HISTORY_PAGE_SIZE_CONFIG for subsequent pages', async () => {
290+
const { fetcher, getCapturedPageSizes } = setup(queryClient);
291+
292+
fetcher.start((state) => (state.data?.pages.length || 0) < 3);
293+
294+
await waitFor(() => {
295+
const state = fetcher.getCurrentState();
296+
expect(state.data?.pages).toHaveLength(3);
297+
});
298+
299+
const pageSizes = getCapturedPageSizes();
300+
expect(pageSizes[1]).toBe('1000');
301+
expect(pageSizes[2]).toBe('1000');
302+
});
265303
});
266304

267305
function setup(client: QueryClient, options: { failOnPages?: number[] } = {}) {
@@ -273,7 +311,10 @@ function setup(client: QueryClient, options: { failOnPages?: number[] } = {}) {
273311
pageSize: 10,
274312
};
275313

276-
mockHistoryEndpoint(workflowHistoryMultiPageFixture, options.failOnPages);
314+
const { getCapturedPageSizes } = mockHistoryEndpoint(
315+
workflowHistoryMultiPageFixture,
316+
options.failOnPages
317+
);
277318
const fetcher = new WorkflowHistoryFetcher(client, params);
278319
hoistedFetcher = fetcher;
279320

@@ -292,13 +333,16 @@ function setup(client: QueryClient, options: { failOnPages?: number[] } = {}) {
292333
fetcher,
293334
params,
294335
waitForData,
336+
getCapturedPageSizes,
295337
};
296338
}
297339

298340
function mockHistoryEndpoint(
299341
responses: GetWorkflowHistoryResponse[],
300342
failOnPages: number[] = []
301343
) {
344+
const capturedPageSizes: string[] = [];
345+
302346
mswMockEndpoints([
303347
{
304348
path: '/api/domains/:domain/:cluster/workflows/:workflowId/:runId/history',
@@ -307,6 +351,9 @@ function mockHistoryEndpoint(
307351
httpResolver: async ({ request }) => {
308352
const url = new URL(request.url);
309353
const nextPage = url.searchParams.get('nextPage');
354+
const pageSize = url.searchParams.get('pageSize');
355+
356+
capturedPageSizes.push(pageSize ?? '');
310357

311358
// Determine current page number based on nextPage param
312359
let pageNumber = 1;
@@ -334,4 +381,8 @@ function mockHistoryEndpoint(
334381
},
335382
},
336383
]);
384+
385+
return {
386+
getCapturedPageSizes: () => capturedPageSizes,
387+
};
337388
}

src/views/workflow-history/helpers/workflow-history-fetcher.ts

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ import {
77
} from '@/route-handlers/get-workflow-history/get-workflow-history.types';
88
import request from '@/utils/request';
99

10+
import {
11+
WORKFLOW_HISTORY_FIRST_PAGE_SIZE_CONFIG,
12+
WORKFLOW_HISTORY_PAGE_SIZE_CONFIG,
13+
} from '../config/workflow-history-page-size.config';
14+
1015
import {
1116
type WorkflowHistoryQueryResult,
1217
type QueryResultOnChangeCallback,
@@ -41,16 +46,18 @@ export default class WorkflowHistoryFetcher {
4146
}
4247

4348
start(shouldContinue: ShouldContinueCallback = () => true): void {
44-
if (shouldContinue) {
45-
this.shouldContinue = shouldContinue;
46-
}
47-
// If already started, return
48-
if (this.isStarted) return;
49+
this.shouldContinue = shouldContinue;
50+
51+
// remove current listener (if exists) to have fresh emits only
52+
this.unsubscribe?.();
53+
this.unsubscribe = null;
54+
4955
this.isStarted = true;
5056
let emitCount = 0;
5157
const currentState = this.observer.getCurrentResult();
5258
const fetchedFirstPage = currentState.status !== 'pending';
53-
const shouldEnableQuery = !fetchedFirstPage && shouldContinue(currentState);
59+
const shouldEnableQuery =
60+
!fetchedFirstPage && this.shouldContinue(currentState);
5461

5562
if (shouldEnableQuery) {
5663
this.observer.setOptions({
@@ -81,8 +88,6 @@ export default class WorkflowHistoryFetcher {
8188
emit(currentState);
8289
}
8390

84-
// remove current listener (if exists) and add new one
85-
this.unsubscribe?.();
8691
this.unsubscribe = this.observer.subscribe((res) => emit(res));
8792
}
8893

@@ -126,7 +131,9 @@ export default class WorkflowHistoryFetcher {
126131
url: `/api/domains/${params.domain}/${params.cluster}/workflows/${params.workflowId}/${params.runId}/history`,
127132
query: {
128133
nextPage: pageParam,
129-
pageSize: params.pageSize,
134+
pageSize: pageParam
135+
? WORKFLOW_HISTORY_PAGE_SIZE_CONFIG
136+
: WORKFLOW_HISTORY_FIRST_PAGE_SIZE_CONFIG,
130137
waitForNewEvent: params.waitForNewEvent ?? false,
131138
} satisfies WorkflowHistoryQueryParams,
132139
})

src/views/workflow-history/hooks/__tests__/use-workflow-history-fetcher.test.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,17 @@ let mockOnChangeCallback: jest.Mock;
1919
let mockUnsubscribe: jest.Mock;
2020

2121
function setup() {
22-
const hookResult = renderHook(() => useWorkflowHistoryFetcher(mockParams));
22+
const mockOnEventsChange = jest.fn();
23+
const hookResult = renderHook(() =>
24+
useWorkflowHistoryFetcher(mockParams, mockOnEventsChange)
25+
);
2326

2427
return {
2528
...hookResult,
2629
mockFetcherInstance,
2730
mockOnChangeCallback,
2831
mockUnsubscribe,
32+
mockOnEventsChange,
2933
};
3034
}
3135

src/views/workflow-history/hooks/use-workflow-history-fetcher.ts

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
useQueryClient,
77
} from '@tanstack/react-query';
88

9+
import { type HistoryEvent } from '@/__generated__/proto-ts/uber/cadence/api/v1/HistoryEvent';
910
import useThrottledState from '@/hooks/use-throttled-state';
1011
import {
1112
type WorkflowHistoryQueryParams,
@@ -19,13 +20,18 @@ import { type ShouldContinueCallback } from '../helpers/workflow-history-fetcher
1920

2021
export default function useWorkflowHistoryFetcher(
2122
params: WorkflowHistoryQueryParams & RouteParams,
23+
onEventsChange: (events: HistoryEvent[]) => void,
2224
throttleMs: number = 2000
2325
) {
2426
const queryClient = useQueryClient();
2527
const fetcherRef = useRef<WorkflowHistoryFetcher | null>(null);
28+
const lastFlattenedPagesCountRef = useRef<number>(-1);
2629

2730
if (!fetcherRef.current) {
2831
fetcherRef.current = new WorkflowHistoryFetcher(queryClient, params);
32+
33+
// Fetch first page
34+
fetcherRef.current.start((state) => !state?.data?.pages?.length);
2935
}
3036

3137
const [historyQuery, setHistoryQuery] = useThrottledState<
@@ -40,21 +46,25 @@ export default function useWorkflowHistoryFetcher(
4046

4147
useEffect(() => {
4248
if (!fetcherRef.current) return;
43-
4449
const unsubscribe = fetcherRef.current.onChange((state) => {
4550
const pagesCount = state.data?.pages?.length || 0;
51+
// If the pages count is greater than the last flattened pages count, then we need to flatten the pages and call the onEventsChange callback
52+
// Depending on ref variable instead of historyQuery is because historyQuery is throttled.
53+
if (pagesCount > lastFlattenedPagesCountRef.current) {
54+
lastFlattenedPagesCountRef.current = pagesCount;
55+
onEventsChange(
56+
state.data?.pages?.flatMap((page) => page.history?.events || []) || []
57+
);
58+
}
4659
// immediately set if there is the first page without throttling other wise throttle
4760
const executeImmediately = pagesCount <= 1;
4861
setHistoryQuery(() => state, executeImmediately);
4962
});
5063

51-
// Fetch first page
52-
fetcherRef.current.start((state) => !state?.data?.pages?.length);
53-
5464
return () => {
5565
unsubscribe();
5666
};
57-
}, [setHistoryQuery]);
67+
}, [setHistoryQuery, onEventsChange]);
5868

5969
useEffect(() => {
6070
return () => {

src/views/workflow-history/workflow-history.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import workflowPageQueryParamsConfig from '../workflow-page/config/workflow-page
2525
import { useSuspenseDescribeWorkflow } from '../workflow-page/hooks/use-describe-workflow';
2626

2727
import workflowHistoryFiltersConfig from './config/workflow-history-filters.config';
28-
import WORKFLOW_HISTORY_PAGE_SIZE_CONFIG from './config/workflow-history-page-size.config';
28+
import { WORKFLOW_HISTORY_PAGE_SIZE_CONFIG } from './config/workflow-history-page-size.config';
2929
import compareUngroupedEvents from './helpers/compare-ungrouped-events';
3030
import getSortableEventId from './helpers/get-sortable-event-id';
3131
import getVisibleGroupsHasMissingEvents from './helpers/get-visible-groups-has-missing-events';
@@ -73,6 +73,8 @@ export default function WorkflowHistory({ params }: Props) {
7373
pageSize: wfHistoryRequestArgs.pageSize,
7474
waitForNewEvent: wfHistoryRequestArgs.waitForNewEvent,
7575
},
76+
//TODO: @assem.hafez replace this with grouper callback
77+
() => {},
7678
2000
7779
);
7880

0 commit comments

Comments
 (0)