Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { waitFor } from '@/test-utils/rtl';
import { type GetWorkflowHistoryResponse } from '@/route-handlers/get-workflow-history/get-workflow-history.types';
import mswMockEndpoints from '@/test-utils/msw-mock-handlers/helper/msw-mock-endpoints';

import { scheduleActivityTaskEvent } from '../../__fixtures__/workflow-history-activity-events';
import workflowHistoryMultiPageFixture from '../../__fixtures__/workflow-history-multi-page-fixture';
import WorkflowHistoryFetcher from '../workflow-history-fetcher';

Expand Down Expand Up @@ -329,21 +330,110 @@ describe(WorkflowHistoryFetcher.name, () => {
jest.useRealTimers();
}
});

it('should send waitForNewEvent=true for all requests when param is set', async () => {
const emptyPageFixture: GetWorkflowHistoryResponse[] = [
// Page 1: has events
{
history: { events: [scheduleActivityTaskEvent] },
rawHistory: [],
archived: false,
nextPageToken: 'page2',
},
// Page 2: empty events (server keeps nextPageToken for long-polling)
{
history: { events: [] },
rawHistory: [],
archived: false,
nextPageToken: 'page3',
},
// Page 3: empty events (last page)
{
history: { events: [] },
rawHistory: [],
archived: false,
nextPageToken: '',
},
];

const { fetcher, getCapturedWaitForNewEvent } = setup(queryClient, {
waitForNewEvent: true,
responses: emptyPageFixture,
});

fetcher.start();

await waitFor(() => {
const state = fetcher.getCurrentState();
expect(state.hasNextPage).toBe(false);
expect(state.data?.pages).toHaveLength(3);
});

const waitForNewEventValues = getCapturedWaitForNewEvent();
// All requests should use waitForNewEvent=true when param is set
expect(waitForNewEventValues[0]).toBe('true');
expect(waitForNewEventValues[1]).toBe('true');
expect(waitForNewEventValues[2]).toBe('true');
});

it('should send waitForNewEvent=false for all requests when param is not set', async () => {
const emptyPageFixture: GetWorkflowHistoryResponse[] = [
{
history: { events: [scheduleActivityTaskEvent] },
rawHistory: [],
archived: false,
nextPageToken: 'page2',
},
{
history: { events: [] },
rawHistory: [],
archived: false,
nextPageToken: 'page3',
},
];

const { fetcher, getCapturedWaitForNewEvent } = setup(queryClient, {
responses: emptyPageFixture,
});

fetcher.start();

await waitFor(() => {
const state = fetcher.getCurrentState();
expect(state.hasNextPage).toBe(false);
});

const waitForNewEventValues = getCapturedWaitForNewEvent();
// All requests should use waitForNewEvent=false when param is not set
expect(waitForNewEventValues[0]).toBe('false');
expect(waitForNewEventValues[1]).toBe('false');
});
});

function setup(client: QueryClient, options: { failOnPages?: number[] } = {}) {
function setup(
client: QueryClient,
options: {
failOnPages?: number[];
waitForNewEvent?: boolean;
responses?: GetWorkflowHistoryResponse[];
} = {}
) {
const params = {
domain: 'test-domain',
cluster: 'test-cluster',
workflowId: 'test-workflow-id',
runId: 'test-run-id',
pageSize: 10,
...(options.waitForNewEvent !== undefined && {
waitForNewEvent: options.waitForNewEvent,
}),
};

const { getCapturedPageSizes } = mockHistoryEndpoint(
workflowHistoryMultiPageFixture,
options.failOnPages
);
const { getCapturedPageSizes, getCapturedWaitForNewEvent } =
mockHistoryEndpoint(
options.responses ?? workflowHistoryMultiPageFixture,
options.failOnPages
);
const fetcher = new WorkflowHistoryFetcher(client, params);
hoistedFetcher = fetcher;

Expand All @@ -363,6 +453,7 @@ function setup(client: QueryClient, options: { failOnPages?: number[] } = {}) {
params,
waitForData,
getCapturedPageSizes,
getCapturedWaitForNewEvent,
};
}

Expand All @@ -371,6 +462,7 @@ function mockHistoryEndpoint(
failOnPages: number[] = []
) {
const capturedPageSizes: string[] = [];
const capturedWaitForNewEvent: string[] = [];

mswMockEndpoints([
{
Expand All @@ -381,8 +473,10 @@ function mockHistoryEndpoint(
const url = new URL(request.url);
const nextPage = url.searchParams.get('nextPage');
const pageSize = url.searchParams.get('pageSize');
const waitForNewEvent = url.searchParams.get('waitForNewEvent');

capturedPageSizes.push(pageSize ?? '');
capturedWaitForNewEvent.push(waitForNewEvent ?? '');

// Determine current page number based on nextPage param
let pageNumber = 1;
Expand All @@ -406,12 +500,21 @@ function mockHistoryEndpoint(
const responseIndex = pageNumber - 1;
const response =
responses[responseIndex] || responses[responses.length - 1];

// Simulate real server behavior: when waitForNewEvent is false
// and the page is empty, the server doesn't return nextPageToken
const isEmpty = response.history?.events?.length === 0;
if (isEmpty && waitForNewEvent === 'false') {
return HttpResponse.json({ ...response, nextPageToken: '' });
}

return HttpResponse.json(response);
},
},
]);

return {
getCapturedPageSizes: () => capturedPageSizes,
getCapturedWaitForNewEvent: () => capturedWaitForNewEvent,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,21 @@ describe('WorkflowHistoryEventsDurationBadge', () => {
expect(screen.getByText('Duration: 120')).toBeInTheDocument();
});

it('does not render badge when loading more events', () => {
it('renders badge when loading more events for running workflow', () => {
setup({
loadingMoreEvents: true,
});

expect(screen.getByText(/Duration:/)).toBeInTheDocument();
});

it('does not render badge when loading more events for completed workflow', () => {
setup({
loadingMoreEvents: true,
workflowCloseStatus:
WorkflowExecutionCloseStatus.WORKFLOW_EXECUTION_CLOSE_STATUS_COMPLETED,
});

expect(screen.queryByText(/Duration:/)).not.toBeInTheDocument();
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ export default function WorkflowHistoryEventsDurationBadge({
workflowCloseStatus !== 'WORKFLOW_EXECUTION_CLOSE_STATUS_INVALID';
const singleEvent = eventsCount === 1 && !hasMissingEvents;
const noDuration =
loadingMoreEvents || singleEvent || (workflowEnded && !endTime);
(loadingMoreEvents && workflowEnded) ||
singleEvent ||
(workflowEnded && !endTime);
const hideDuration = (showOngoingOnly && endTime) || noDuration;
const isOngoing = !endTime && !hideDuration;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ describe('WorkflowHistoryRemainingDurationBadge', () => {
expect(screen.getByText('Remaining: 5m 30s')).toBeInTheDocument();
});

it('does not render badge when loading more events', () => {
it('renders badge when loading more events for running workflow', () => {
setup({
loadingMoreEvents: true,
});

expect(screen.queryByText(/Remaining:/)).not.toBeInTheDocument();
expect(screen.getByText('Remaining: 5m 30s')).toBeInTheDocument();
});

it('does not render badge when workflow is archived', () => {
Expand Down Expand Up @@ -151,11 +151,11 @@ describe('WorkflowHistoryRemainingDurationBadge', () => {
startTime={mockStartTime}
expectedEndTime={new Date('2024-01-01T10:07:00Z').getTime()}
prefix="Remaining:"
workflowIsArchived={false}
workflowIsArchived={true}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good catch

workflowCloseStatus={
WorkflowExecutionCloseStatus.WORKFLOW_EXECUTION_CLOSE_STATUS_INVALID
}
loadingMoreEvents={true}
loadingMoreEvents={false}
/>
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export default function WorkflowHistoryRemainingDurationBadge({
workflowIsArchived ||
workflowCloseStatus !== 'WORKFLOW_EXECUTION_CLOSE_STATUS_INVALID';

const shouldHide = loadingMoreEvents || workflowEnded;
const shouldHide = workflowEnded;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Quality: Unused loadingMoreEvents prop in remaining-duration-badge

After the change on line 21 of workflow-history-remaining-duration-badge.tsx, shouldHide no longer references loadingMoreEvents. The prop is still declared in the Props type (line 9 of the types file) and destructured (line 15 of the component) but never used. This is dead code that will confuse future readers into thinking loading state still affects this badge.

Suggested fix:

Remove `loadingMoreEvents` from the `Props` type in the `.types.ts` file, remove it from the destructured props in the component, and remove it from all call sites passing it to this component.

Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion


const [remainingDuration, setRemainingDuration] = useState<string | null>(
null
Expand Down
Loading