Skip to content

Commit e6269dd

Browse files
Assem-Uberpandeyanshumanadhityamamallan
authored
Bugfix/12323/fix-wrong-status-for-history-groups (#845)
* change calculations for hasMissingEvents * lint fix * Make time select in date filter editable (#838) The datepicker used allows users to select time range. We populate the time select with time interval of 15 minutes. Adding more granular time options hampers the performance of the component. However, the users often need to specify time intervals that does not align with the options present. This change adds the `creatable` flag to the time select to enable this. Please note that the `creatable` flag in the underlying BaseWeb component is not quite intuitive. * fix hasMissingEvents for timeouts * Create useThrottledState * use defaultThrottleSettings * fix failing test * Show loading indicator and load missing events * fix lint * fix types * fix lint * revert status changes * rename helper * rename method * remove statusReady from WorkflowHistoryTimelineGroup --------- Co-authored-by: Pandey Anshuman Kishore <[email protected]> Co-authored-by: Adhitya Mamallan <[email protected]>
1 parent ec849bb commit e6269dd

17 files changed

+267
-21
lines changed

src/config/dynamic/resolvers/clusters.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export default function clusters() {
2727
['cadence-frontend']
2828
);
2929
if (
30-
clusterNames.length !== peers.length &&
30+
clusterNames.length !== peers.length ||
3131
clusterNames.length !== serviceNames.length
3232
) {
3333
throw new Error(
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
import getVisibleGroupsHasMissingEvents from '../get-visible-groups-has-missing-events';
2+
3+
describe('getVisibleGroupsHasMissingEvents', () => {
4+
const groupEntries: [string, { hasMissingEvents: boolean }][] = [
5+
['group1', { hasMissingEvents: false }],
6+
['group2', { hasMissingEvents: true }],
7+
['group3', { hasMissingEvents: false }],
8+
['group4', { hasMissingEvents: true }],
9+
];
10+
11+
it('should return true if any event in the main visible range has missing events', () => {
12+
const visibleRanges = {
13+
startIndex: 0,
14+
endIndex: 2,
15+
compactStartIndex: 2,
16+
compactEndIndex: 3,
17+
};
18+
expect(getVisibleGroupsHasMissingEvents(groupEntries, visibleRanges)).toBe(
19+
true
20+
);
21+
});
22+
23+
it('should return true if any event in the compact visible range has missing events', () => {
24+
const visibleRanges = {
25+
startIndex: 0,
26+
endIndex: 1,
27+
compactStartIndex: 2,
28+
compactEndIndex: 3,
29+
};
30+
expect(getVisibleGroupsHasMissingEvents(groupEntries, visibleRanges)).toBe(
31+
true
32+
);
33+
});
34+
35+
it('should return false if no events in the visible range have missing events', () => {
36+
const visibleRanges = {
37+
startIndex: 0,
38+
endIndex: 0,
39+
compactStartIndex: 2,
40+
compactEndIndex: 2,
41+
};
42+
expect(getVisibleGroupsHasMissingEvents(groupEntries, visibleRanges)).toBe(
43+
false
44+
);
45+
});
46+
47+
it('should handle an empty groupEntries array and return false', () => {
48+
const visibleRanges = {
49+
startIndex: 0,
50+
endIndex: 0,
51+
compactStartIndex: 0,
52+
compactEndIndex: 0,
53+
};
54+
expect(getVisibleGroupsHasMissingEvents([], visibleRanges)).toBe(false);
55+
});
56+
57+
it('should handle out of range numbers and return false', () => {
58+
const visibleRanges = {
59+
startIndex: -1,
60+
endIndex: -1,
61+
compactStartIndex: 100,
62+
compactEndIndex: 200,
63+
};
64+
expect(getVisibleGroupsHasMissingEvents(groupEntries, visibleRanges)).toBe(
65+
false
66+
);
67+
});
68+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import type {
2+
HistoryEventsGroup,
3+
VisibleHistoryGroupRanges,
4+
} from '../workflow-history.types';
5+
6+
export default function getVisibleGroupsHasMissingEvents(
7+
groupEntries: Array<[string, Pick<HistoryEventsGroup, 'hasMissingEvents'>]>,
8+
visibleRanges: VisibleHistoryGroupRanges
9+
): boolean {
10+
const { startIndex, endIndex, compactStartIndex, compactEndIndex } =
11+
visibleRanges;
12+
const visibleHasMissing = groupEntries
13+
.slice(startIndex, endIndex + 1)
14+
.some(([_, { hasMissingEvents }]) => hasMissingEvents);
15+
16+
if (visibleHasMissing) return true;
17+
const compactVisibleHasMissing = groupEntries
18+
.slice(compactStartIndex, compactEndIndex + 1)
19+
.some(([_, { hasMissingEvents }]) => hasMissingEvents);
20+
21+
if (compactVisibleHasMissing) return true;
22+
return false;
23+
}

src/views/workflow-history/hooks/__tests__/use-keep-loading-events.test.ts

+22
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,28 @@ describe('useKeepLoadingEvents', () => {
4040
expect(fetchNextPageMock).not.toHaveBeenCalled();
4141
});
4242

43+
it('should not call fetchNextPage after error when continueLoadingAfterError is false', () => {
44+
const { fetchNextPageMock, rerender } = setup({
45+
isFetchNextPageError: true,
46+
continueLoadingAfterError: false,
47+
});
48+
49+
rerender({ isFetchNextPageError: false });
50+
51+
expect(fetchNextPageMock).not.toHaveBeenCalled();
52+
});
53+
54+
it('should call fetchNextPage after error when continueLoadingAfterError is true', () => {
55+
const { fetchNextPageMock, rerender } = setup({
56+
isFetchNextPageError: true,
57+
continueLoadingAfterError: true,
58+
});
59+
60+
rerender({ isFetchNextPageError: false });
61+
62+
expect(fetchNextPageMock).toHaveBeenCalled();
63+
});
64+
4365
it('should set stoppedDueToError to true when isFetchNextPageError is true', () => {
4466
const { result, rerender } = setup({
4567
isFetchNextPageError: false,

src/views/workflow-history/hooks/use-keep-loading-events.ts

+13-8
Original file line numberDiff line numberDiff line change
@@ -10,24 +10,29 @@ export default function useKeepLoadingEvents({
1010
isFetchingNextPage,
1111
stopAfterEndReached,
1212
isFetchNextPageError,
13+
continueLoadingAfterError,
1314
}: UseKeepLoadingEventsParams) {
1415
const reachedAvailableHistoryEnd = useRef(false);
1516

16-
const stoppedDueToError = useRef(isFetchNextPageError);
17-
17+
const hadErrorOnce = useRef(isFetchNextPageError);
1818
// update reachedAvailableHistoryEnd
19-
const reached = !hasNextPage || (hasNextPage && isLastPageEmpty);
19+
const reached =
20+
!hasNextPage || (hasNextPage && isLastPageEmpty && !isFetchNextPageError);
2021
if (reached && !reachedAvailableHistoryEnd.current)
2122
reachedAvailableHistoryEnd.current = true;
2223

23-
// update stoppedDueToError
24-
if (isFetchNextPageError && !stoppedDueToError.current)
25-
stoppedDueToError.current = true;
24+
// update hadErrorOnce
25+
if (isFetchNextPageError && !hadErrorOnce.current)
26+
hadErrorOnce.current = true;
27+
28+
const stopDueToError =
29+
isFetchNextPageError ||
30+
(hadErrorOnce.current && !continueLoadingAfterError);
2631

2732
const canLoadMore =
2833
shouldKeepLoading &&
2934
!(stopAfterEndReached && reachedAvailableHistoryEnd.current) &&
30-
!stoppedDueToError.current &&
35+
!stopDueToError &&
3136
hasNextPage;
3237

3338
useEffect(() => {
@@ -36,7 +41,7 @@ export default function useKeepLoadingEvents({
3641

3742
return {
3843
reachedAvailableHistoryEnd: reachedAvailableHistoryEnd.current,
39-
stoppedDueToError: stoppedDueToError.current,
44+
stoppedDueToError: stopDueToError,
4045
isLoadingMore: canLoadMore,
4146
};
4247
}

src/views/workflow-history/hooks/use-keep-loading-events.types.ts

+1
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,5 @@ export type UseKeepLoadingEventsParams = {
66
isFetchingNextPage: boolean;
77
isFetchNextPageError: boolean;
88
stopAfterEndReached?: boolean;
9+
continueLoadingAfterError?: boolean;
910
};

src/views/workflow-history/workflow-history-compact-event-card/__tests__/workflow-history-compact-event-card.test.tsx

+1
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ function setup(props: Partial<Props>) {
6767
return {
6868
...render(
6969
<WorkflowHistoryCompactEventCard
70+
statusReady
7071
status="COMPLETED"
7172
label="test label"
7273
secondaryLabel="test secondaryLabel"

src/views/workflow-history/workflow-history-compact-event-card/workflow-history-compact-event-card.tsx

+6-1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { type Props } from './workflow-history-compact-event-card.types';
1717

1818
export default function WorkflowHistoryCompactEventCard({
1919
status,
20+
statusReady,
2021
label,
2122
badges,
2223
showLabelPlaceholder,
@@ -37,7 +38,11 @@ export default function WorkflowHistoryCompactEventCard({
3738
disabled={disabled}
3839
onClick={onClick}
3940
>
40-
<WorkflowHistoryEventStatusBadge status={status} size="small" />
41+
<WorkflowHistoryEventStatusBadge
42+
status={status}
43+
statusReady={statusReady}
44+
size="small"
45+
/>
4146
<div className={cls.textContainer}>
4247
{label && !showLabelPlaceholder && (
4348
<div className={cls.label}>

src/views/workflow-history/workflow-history-compact-event-card/workflow-history-compact-event-card.types.ts

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { type HistoryGroupBadge } from '../workflow-history.types';
55

66
export type Props = {
77
status: WorkflowEventStatus;
8+
statusReady: boolean;
89
label: string;
910
secondaryLabel: string;
1011
showLabelPlaceholder?: boolean;

src/views/workflow-history/workflow-history-event-status-badge/__tests__/workflow-history-tab-event-status-badge.test.tsx

+26-5
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,44 @@
11
import React from 'react';
22

3-
import { render } from '@/test-utils/rtl';
3+
import { render, screen } from '@/test-utils/rtl';
44

55
import WorkflowHistoryEventStatusBadge from '../workflow-history-event-status-badge';
66
import {
77
WORKFLOW_EVENT_STATUS,
88
WORKFLOW_EVENT_STATUS_BADGE_SIZES,
99
} from '../workflow-history-event-status-badge.constants';
1010

11+
jest.mock('baseui/skeleton', () => ({
12+
Skeleton: jest.fn(() => <div>Loading Skeleton</div>),
13+
}));
14+
1115
describe('WorkflowHistoryEventStatusBadge', () => {
16+
it('should show loading skeleton when status is not ready', () => {
17+
render(
18+
<WorkflowHistoryEventStatusBadge status="CANCELED" statusReady={false} />
19+
);
20+
21+
expect(screen.getByText('Loading Skeleton')).toBeInTheDocument();
22+
});
23+
1224
it('should match snapshot when status is not valid and badge should not be rendered', () => {
1325
const { container } = render(
1426
// @ts-expect-error invalid status
15-
<WorkflowHistoryEventStatusBadge status="INVALID_STATUS" />,
27+
<WorkflowHistoryEventStatusBadge status="INVALID_STATUS" statusReady />,
1628
{ isSnapshotTest: true }
1729
);
1830
expect(container).toMatchSnapshot();
1931
});
2032

2133
it('should match snapshot when size is not valid and icon should not be rendered', () => {
2234
const { container } = render(
23-
// @ts-expect-error invalid size
24-
<WorkflowHistoryEventStatusBadge status="COMPLETE" size="invalid" />
35+
<WorkflowHistoryEventStatusBadge
36+
// @ts-expect-error invalid status
37+
status="COMPLETE"
38+
statusReady
39+
// @ts-expect-error invalid size
40+
size="invalid"
41+
/>
2542
);
2643
expect(container).toMatchSnapshot();
2744
});
@@ -30,7 +47,11 @@ describe('WorkflowHistoryEventStatusBadge', () => {
3047
for (const size of Object.values(WORKFLOW_EVENT_STATUS_BADGE_SIZES)) {
3148
it(`should match snapshot when status is ${status} and size is ${size}`, () => {
3249
const { container } = render(
33-
<WorkflowHistoryEventStatusBadge status={status} size={size} />,
50+
<WorkflowHistoryEventStatusBadge
51+
status={status}
52+
statusReady
53+
size={size}
54+
/>,
3455
{ isSnapshotTest: true }
3556
);
3657
expect(container).toMatchSnapshot();

src/views/workflow-history/workflow-history-event-status-badge/workflow-history-event-status-badge.styles.ts

+12
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { styled as createStyled } from 'baseui';
2+
import { type SkeletonOverrides } from 'baseui/skeleton/types';
23

34
import type { StyletronCSSObjectValue } from '@/hooks/use-styletron-classes';
45

@@ -9,6 +10,17 @@ import type {
910
WorkflowEventStatusBadgeSize,
1011
} from './workflow-history-event-status-badge.types';
1112

13+
export const overrides = {
14+
circularSkeleton: {
15+
Root: {
16+
style: {
17+
borderRadius: '50%',
18+
flexShrink: 0, // prevent badge from shrinking in flex containers
19+
},
20+
},
21+
} satisfies SkeletonOverrides,
22+
};
23+
1224
export const styled = {
1325
BadgeContainer: createStyled<
1426
'div',

src/views/workflow-history/workflow-history-event-status-badge/workflow-history-event-status-badge.tsx

+20-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import React from 'react';
22

33
import { useStyletron } from 'baseui';
4+
import { Skeleton } from 'baseui/skeleton';
45
import { Spinner } from 'baseui/spinner';
56
import {
67
MdCheck,
@@ -9,19 +10,37 @@ import {
910
MdReportGmailerrorred,
1011
} from 'react-icons/md';
1112

13+
import getBadgeContainerSize from './helpers/get-badge-container-size';
1214
import getBadgeIconSize from './helpers/get-badge-icon-size';
1315
import {
1416
WORKFLOW_EVENT_STATUS,
1517
WORKFLOW_EVENT_STATUS_BADGE_SIZES,
1618
} from './workflow-history-event-status-badge.constants';
17-
import { styled } from './workflow-history-event-status-badge.styles';
19+
import {
20+
styled,
21+
overrides,
22+
} from './workflow-history-event-status-badge.styles';
1823
import type { Props } from './workflow-history-event-status-badge.types';
1924

2025
export default function WorkflowHistoryEventStatusBadge({
2126
status,
27+
statusReady,
2228
size = WORKFLOW_EVENT_STATUS_BADGE_SIZES.medium,
2329
}: Props) {
2430
const [_, theme] = useStyletron();
31+
32+
if (!statusReady) {
33+
const skeletonSize = getBadgeContainerSize(theme, size);
34+
return (
35+
<Skeleton
36+
height={skeletonSize}
37+
width={skeletonSize}
38+
overrides={overrides.circularSkeleton}
39+
animation
40+
/>
41+
);
42+
}
43+
2544
if (!WORKFLOW_EVENT_STATUS[status]) return null;
2645

2746
const renderIcon = () => {

src/views/workflow-history/workflow-history-event-status-badge/workflow-history-event-status-badge.types.ts

+1
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,6 @@ export type WorkflowEventStatus =
88

99
export type Props = {
1010
status: WorkflowEventStatus;
11+
statusReady: boolean;
1112
size?: WorkflowEventStatusBadgeSize;
1213
};

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

+3
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ export default function WorkflowHistoryEventsCard({
4242
title={
4343
<>
4444
<WorkflowHistoryEventStatusBadge
45+
statusReady={true}
4546
size="small"
4647
status={eventMetadata.status}
4748
/>
@@ -66,11 +67,13 @@ export default function WorkflowHistoryEventsCard({
6667
width={getBadgeContainerSize(theme, 'small')}
6768
height={getBadgeContainerSize(theme, 'small')}
6869
overrides={overrides.circularSkeleton}
70+
animation
6971
/>
7072
<Skeleton
7173
rows={0}
7274
width="100px"
7375
height={theme.typography.LabelSmall.lineHeight.toString()}
76+
animation
7477
/>
7578
</div>
7679
}

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

+5-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,11 @@ export default function WorkflowHistoryTimelineGroup({
3333
return (
3434
<div className={cls.groupContainer}>
3535
<div className={cls.timelineEventHeader}>
36-
<WorkflowHistoryEventStatusBadge status={status} size="medium" />
36+
<WorkflowHistoryEventStatusBadge
37+
status={status}
38+
statusReady={!hasMissingEvents}
39+
size="medium"
40+
/>
3741
<div className={cls.timelineEventLabelAndTime}>
3842
<div className={cls.timelineEventsLabel}>{label}</div>
3943
{hasBadges && (

0 commit comments

Comments
 (0)