Skip to content

Commit 63a4939

Browse files
authored
Fix duration to read correct startTime (#916)
1 parent e94713c commit 63a4939

18 files changed

+126
-44
lines changed

src/views/workflow-history/__fixtures__/workflow-history-event-groups.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ export const mockActivityEventGroup: ActivityHistoryGroup = {
1414
status: 'COMPLETED',
1515
eventsMetadata: [],
1616
hasMissingEvents: false,
17-
timeMs: 1725747380000,
17+
timeMs: 1725747370632,
18+
startTimeMs: 1725747370599,
1819
timeLabel: 'Mock time label',
1920
events: completedActivityTaskEvents,
2021
};
@@ -26,6 +27,7 @@ export const mockTimerEventGroup: TimerHistoryGroup = {
2627
eventsMetadata: [],
2728
hasMissingEvents: false,
2829
timeMs: 1725747380000,
30+
startTimeMs: 1725747380000,
2931
timeLabel: 'Mock time label',
3032
events: [startTimerTaskEvent],
3133
};
@@ -37,6 +39,7 @@ export const mockSingleEventGroup: SingleEventHistoryGroup = {
3739
eventsMetadata: [],
3840
hasMissingEvents: false,
3941
timeMs: 1725747380000,
42+
startTimeMs: 1725747380000,
4043
timeLabel: 'Mock time label',
4144
events: [startWorkflowExecutionEvent],
4245
};

src/views/workflow-history/helpers/__tests__/get-common-history-group-fields.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,11 @@ describe('getCommonHistoryGroupFields', () => {
9292
expect(group.closeTimeMs).toEqual(null);
9393
});
9494

95+
it('should return group with startTimeMs equal to first event timeMs', () => {
96+
const group = setup({});
97+
expect(group.startTimeMs).toEqual(group.eventsMetadata[0].timeMs);
98+
});
99+
95100
const groupFieldsExtractedFromEventsMetadaTests: {
96101
name: string;
97102
groupField: 'events' | 'eventsMetadata' | 'status' | 'timeMs' | 'timeLabel';

src/views/workflow-history/helpers/get-common-history-group-fields.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ export default function getCommonHistoryGroupFields<
2424
| 'timeMs'
2525
| 'timeLabel'
2626
| 'closeTimeMs'
27+
| 'startTimeMs'
2728
> {
2829
const eventsMetadata = events.map((event, index) => {
2930
const attrs = event.attributes as GroupT['events'][number]['attributes'];
@@ -47,6 +48,7 @@ export default function getCommonHistoryGroupFields<
4748

4849
const groupStatus = eventsMetadata[eventsMetadata.length - 1].status;
4950
const groupTimeMs = eventsMetadata[eventsMetadata.length - 1].timeMs;
51+
const groupStartTimeMs = eventsMetadata[0].timeMs;
5052
const groupTimeLabel = eventsMetadata[eventsMetadata.length - 1].timeLabel;
5153
const groupCloseTimeMs = closeEvent?.eventTime
5254
? parseGrpcTimestamp(closeEvent.eventTime)
@@ -57,6 +59,7 @@ export default function getCommonHistoryGroupFields<
5759
events,
5860
status: groupStatus,
5961
timeMs: groupTimeMs,
62+
startTimeMs: groupStartTimeMs,
6063
closeTimeMs: groupCloseTimeMs,
6164
timeLabel: groupTimeLabel,
6265
};

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

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,9 @@ describe('WorkflowHistoryCompactEventCard', () => {
7171
expect(mockOnClick).toHaveBeenCalled();
7272
});
7373

74-
it('should render duration badge passing showOngoingOnly as true when timeMs is provided', () => {
74+
it('should render duration badge passing showOngoingOnly as true when startTimeMs is provided', () => {
7575
setup({
76-
timeMs: 1726652232190.7927,
76+
startTimeMs: 1726652232190.7927,
7777
});
7878
expect(WorkflowHistoryEventsDurationBadge).toHaveBeenCalledWith(
7979
expect.objectContaining({
@@ -83,11 +83,10 @@ describe('WorkflowHistoryCompactEventCard', () => {
8383
);
8484
});
8585

86-
it('does not render duration badge when timeMs is not provided', () => {
86+
it('should not render duration badge when startTimeMs is not provided', () => {
8787
setup({
88-
timeMs: null,
88+
startTimeMs: null,
8989
});
90-
9190
expect(screen.queryByText('Duration Badge')).not.toBeInTheDocument();
9291
});
9392
});
@@ -102,7 +101,7 @@ function setup(props: Partial<Props>) {
102101
status="COMPLETED"
103102
label="test label"
104103
onClick={mockOnClick}
105-
timeMs={null}
104+
startTimeMs={null}
106105
closeTimeMs={null}
107106
events={[startWorkflowExecutionEvent]}
108107
workflowCloseTimeMs={null}

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export default function WorkflowHistoryCompactEventCard({
2323
showLabelPlaceholder,
2424
selected,
2525
disabled,
26-
timeMs,
26+
startTimeMs,
2727
badges,
2828
closeTimeMs,
2929
events,
@@ -65,13 +65,14 @@ export default function WorkflowHistoryCompactEventCard({
6565
color="primary"
6666
/>
6767
))}
68-
{timeMs && (
68+
{startTimeMs && (
6969
<span className={cls.durationContainer}>
7070
<WorkflowHistoryEventsDurationBadge
71-
startTime={timeMs}
71+
startTime={startTimeMs}
7272
closeTime={closeTimeMs}
7373
eventsCount={events.length}
7474
hasMissingEvents={hasMissingEvents}
75+
loadingMoreEvents={!statusReady}
7576
workflowCloseTime={workflowCloseTimeMs}
7677
workflowIsArchived={workflowIsArchived}
7778
workflowCloseStatus={workflowCloseStatus}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ export type Props = Pick<
1212
| 'status'
1313
| 'badges'
1414
| 'resetToDecisionEventId'
15-
| 'timeMs'
15+
| 'startTimeMs'
1616
| 'closeTimeMs'
1717
> & {
1818
statusReady: boolean;

src/views/workflow-history/workflow-history-events-duration-badge/__tests__/workflow-history-events-duration-badge.test.tsx

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,14 @@ describe('WorkflowHistoryEventsDurationBadge', () => {
6464
expect(screen.getByText('Duration: 120')).toBeInTheDocument();
6565
});
6666

67+
it('does not render badge when loading more events', () => {
68+
setup({
69+
loadingMoreEvents: true,
70+
});
71+
72+
expect(screen.queryByText(/Duration:/)).not.toBeInTheDocument();
73+
});
74+
6775
it('does not render badge when workflow is archived without close time', () => {
6876
setup({
6977
closeTime: null,
@@ -122,7 +130,8 @@ describe('WorkflowHistoryEventsDurationBadge', () => {
122130

123131
expect(getFormattedEventsDuration).toHaveBeenCalledWith(
124132
mockStartTime,
125-
mockCloseTime
133+
mockCloseTime,
134+
expect.any(Boolean)
126135
);
127136
expect(screen.getByText('Duration: 60')).toBeInTheDocument();
128137
});
@@ -133,6 +142,7 @@ function setup({
133142
closeTime,
134143
eventsCount = 2,
135144
hasMissingEvents = false,
145+
loadingMoreEvents = false,
136146
workflowIsArchived = false,
137147
workflowCloseStatus = WorkflowExecutionCloseStatus.WORKFLOW_EXECUTION_CLOSE_STATUS_INVALID,
138148
workflowCloseTime = null,
@@ -143,6 +153,7 @@ function setup({
143153
closeTime={closeTime}
144154
eventsCount={eventsCount}
145155
hasMissingEvents={hasMissingEvents}
156+
loadingMoreEvents={loadingMoreEvents}
146157
workflowIsArchived={workflowIsArchived}
147158
workflowCloseStatus={workflowCloseStatus}
148159
workflowCloseTime={workflowCloseTime}

src/views/workflow-history/workflow-history-events-duration-badge/helpers/__tests__/get-formatted-events-duration.test.ts

Lines changed: 43 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,45 +2,81 @@ import getFormattedEventsDuration from '../get-formatted-events-duration';
22

33
jest.mock('@/utils/data-formatters/format-duration', () => ({
44
__esModule: true,
5-
default: jest.fn((duration) => `mocked: ${duration.seconds}s`),
5+
default: jest.fn(
6+
(duration) => `mocked: ${duration.seconds}s ${duration.nanos / 1000000}ms`
7+
),
68
}));
79

10+
const mockNow = new Date('2024-01-01T10:02:00Z');
11+
812
describe('getFormattedEventsDuration', () => {
13+
beforeEach(() => {
14+
jest.useFakeTimers();
15+
jest.setSystemTime(mockNow);
16+
});
17+
18+
afterEach(() => {
19+
jest.useRealTimers();
20+
});
21+
922
it('should return 0s for identical start and end times', () => {
1023
const duration = getFormattedEventsDuration(
1124
'2021-01-01T00:00:00Z',
1225
'2021-01-01T00:00:00Z'
1326
);
14-
expect(duration).toEqual('mocked: 0s');
27+
expect(duration).toEqual('mocked: 0s 0ms');
1528
});
1629

1730
it('should return correct duration for 1 minute', () => {
1831
const duration = getFormattedEventsDuration(
1932
'2021-01-01T00:00:00Z',
2033
'2021-01-01T00:01:00Z'
2134
);
22-
expect(duration).toEqual('mocked: 60s');
35+
expect(duration).toEqual('mocked: 60s 0ms');
2336
});
2437

2538
it('should return correct duration for 1 hour, 2 minutes, 3 seconds', () => {
2639
const duration = getFormattedEventsDuration(
2740
'2021-01-01T01:02:03Z',
2841
'2021-01-01T02:04:06Z'
2942
);
30-
expect(duration).toEqual(`mocked: ${60 * 60 + 2 * 60 + 3}s`);
43+
expect(duration).toEqual(`mocked: ${60 * 60 + 2 * 60 + 3}s 0ms`);
3144
});
3245

3346
it('should handle endTime as null (use current time)', () => {
34-
const start = new Date(Date.now() - 60000).toISOString(); // 1 minute ago
47+
const start = new Date(mockNow.getTime() - 60000).toISOString(); // 1 minute ago
3548
const duration = getFormattedEventsDuration(start, null);
36-
expect(duration).toEqual('mocked: 60s');
49+
expect(duration).toEqual('mocked: 60s 0ms');
3750
});
3851

3952
it('should handle negative durations (start after end)', () => {
4053
const duration = getFormattedEventsDuration(
4154
'2021-01-01T01:00:00Z',
4255
'2021-01-01T00:00:00Z'
4356
);
44-
expect(duration).toEqual('mocked: -3600s');
57+
expect(duration).toEqual('mocked: -3600s 0ms');
58+
});
59+
60+
it('should handle numeric durations', () => {
61+
const duration = getFormattedEventsDuration(1726652232190, 1726652292194);
62+
expect(duration).toEqual('mocked: 60s 4ms');
63+
});
64+
65+
it('should remove ms from duration when hideMs is true', () => {
66+
const duration = getFormattedEventsDuration(
67+
1726652232190,
68+
1726652292194,
69+
true
70+
);
71+
expect(duration).toEqual('mocked: 60s');
72+
});
73+
74+
it('should not hide ms if there are no bigger units', () => {
75+
const duration = getFormattedEventsDuration(
76+
1726652232190,
77+
1726652232194,
78+
true
79+
);
80+
expect(duration).toEqual('mocked: 0s 4ms');
4581
});
4682
});

src/views/workflow-history/workflow-history-events-duration-badge/helpers/get-formatted-events-duration.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,27 @@ import formatDuration from '@/utils/data-formatters/format-duration';
22
import dayjs from '@/utils/datetime/dayjs';
33

44
export default function getFormattedEventsDuration(
5-
startTime: Date | string | number,
6-
endTime: Date | string | number | null | undefined
5+
startTime: Date | string | number | null,
6+
endTime: Date | string | number | null | undefined,
7+
hideMs: boolean = false
78
) {
89
const end = endTime ? dayjs(endTime) : dayjs();
910
const start = dayjs(startTime);
1011
const diff = end.diff(start);
1112
const durationObj = dayjs.duration(diff);
12-
return formatDuration(
13+
const seconds = Math.floor(durationObj.asSeconds());
14+
15+
const duration = formatDuration(
1316
{
14-
seconds: durationObj.asSeconds().toString(),
15-
nanos: 0,
17+
seconds: seconds.toString(),
18+
nanos: (durationObj.asMilliseconds() - seconds * 1000) * 1000000,
1619
},
1720
{ separator: ' ' }
1821
);
22+
// TODO: add this functionality to formatDuration in more reusable way
23+
if (hideMs && seconds > 0) {
24+
return duration.replace(/ \d+ms/i, '');
25+
}
26+
27+
return duration;
1928
}

src/views/workflow-history/workflow-history-events-duration-badge/workflow-history-events-duration-badge.tsx

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ export default function WorkflowHistoryEventsDurationBadge({
1212
workflowCloseStatus,
1313
eventsCount,
1414
hasMissingEvents,
15+
loadingMoreEvents,
1516
workflowCloseTime,
1617
showOngoingOnly,
1718
}: Props) {
@@ -20,23 +21,25 @@ export default function WorkflowHistoryEventsDurationBadge({
2021
workflowIsArchived ||
2122
workflowCloseStatus !== 'WORKFLOW_EXECUTION_CLOSE_STATUS_INVALID';
2223
const singleEvent = eventsCount === 1 && !hasMissingEvents;
23-
const noDuration = singleEvent || (workflowEnded && !endTime);
24+
const noDuration =
25+
loadingMoreEvents || singleEvent || (workflowEnded && !endTime);
2426
const hideDuration = (showOngoingOnly && endTime) || noDuration;
27+
const isOngoing = !endTime && !hideDuration;
2528

2629
const [duration, setDuration] = useState<string>(() =>
27-
getFormattedEventsDuration(startTime, endTime)
30+
getFormattedEventsDuration(startTime, endTime, isOngoing)
2831
);
2932

3033
useEffect(() => {
31-
setDuration(getFormattedEventsDuration(startTime, endTime));
32-
if (!endTime && !hideDuration) {
34+
setDuration(getFormattedEventsDuration(startTime, endTime, isOngoing));
35+
if (isOngoing) {
3336
const interval = setInterval(() => {
34-
setDuration(getFormattedEventsDuration(startTime, endTime));
37+
setDuration(getFormattedEventsDuration(startTime, endTime, true));
3538
}, 1000);
3639

3740
return () => clearInterval(interval);
3841
}
39-
}, [startTime, endTime, hideDuration]);
42+
}, [startTime, endTime, isOngoing]);
4043

4144
if (hideDuration) {
4245
return null;

0 commit comments

Comments
 (0)