1
0
mirror of https://github.com/Unleash/unleash.git synced 2025-07-31 13:47:02 +02:00

Show change request stage timestamps in UI (#10388)

Adds a timestamp for each state we have time (and that isn't a state
downstream from the current state) in the CR timeline.

<img width="437" height="318" alt="image"
src="https://github.com/user-attachments/assets/a499e73f-c506-46a0-8d1a-7e4eb5ec4f7d"
/>

The timestamp respects the user's preferred locale and uses the `time`
element.

I've used the current name of the API payload's timestamps as it stands
on the enterprise main branch for now. This name is not set in any
schemas yet, so it is likely to change. Because it's not currently
exposed to any users, that will not cause any issues. Name suggestions
are welcome if you have them.

We only show timestamps for states up to and including the current
state. Timestamps for downstream states (such as "approved" if you're in
the "in review" state), will not be shown, even if they exist in the
payload. (There are some decisions to make on whether to include these
in the payload at all or not.)

There's no flags in this PR. They're not necessary If the API payload
doesn't contain the timestamp, we just don't render the timestamp, and
the timeline looks the way it always did:
<img width="447" height="399" alt="image"
src="https://github.com/user-attachments/assets/0062393a-190c-4099-bc16-29f9da82e7ea"
/>


## Bonus work

In the `ChangeRequestTimeline.tsx` file, I've made a few extra changes:
- `createTimelineItem` and `createScheduledTimelineItem` have become
normal React components (`TimelineItem` and `ScheduledTimelineItem`) and
are being called as such (in accordance with [React
recommendations](https://react.dev/reference/rules/react-calls-components-and-hooks#never-call-component-functions-directly)).
- I've updated the subtitles for schedules to also use the time element,
to improve HTML structure.

## Outstanding work

There's a few things that still need to be sorted out (primarily with
UX). Mainly about how we handle scheduled items, which already have time
information. For now, it looks like this:

<img width="426" height="394" alt="image"
src="https://github.com/user-attachments/assets/4bfc4ca2-c738-4954-9251-8d063143371e"
/>

<img width="700" height="246" alt="image"
src="https://github.com/user-attachments/assets/fe688b08-c5c8-40f8-a9d0-fe455e44665f"
/>
This commit is contained in:
Thomas Heartman 2025-07-24 14:42:29 +02:00 committed by GitHub
parent 8943cc0a3d
commit d2e2378481
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 138 additions and 82 deletions

View File

@ -349,7 +349,10 @@ export const ChangeRequestOverview: FC = () => {
<ChangeRequestHeader changeRequest={changeRequest} /> <ChangeRequestHeader changeRequest={changeRequest} />
<ChangeRequestBody> <ChangeRequestBody>
<StyledAsideBox> <StyledAsideBox>
<ChangeRequestTimeline {...timelineProps} /> <ChangeRequestTimeline
{...timelineProps}
timestamps={changeRequest.stateTransitionTimestamps}
/>
<ConditionallyRender <ConditionallyRender
condition={approversEnabled} condition={approversEnabled}
show={ show={

View File

@ -111,60 +111,63 @@ test('returns success for stages other than Rejected in Rejected state', () => {
describe('changeRequestScheduleProps', () => { describe('changeRequestScheduleProps', () => {
test('returns correct props for a pending schedule', () => { test('returns correct props for a pending schedule', () => {
const date = new Date();
const schedule = { const schedule = {
scheduledAt: new Date().toISOString(), scheduledAt: date.toISOString(),
status: 'pending' as const, status: 'pending' as const,
}; };
const time = 'some time string'; const { title, subtitle, color, reason } = getScheduleProps(schedule);
const { title, subtitle, color, reason } = getScheduleProps(
schedule,
time,
);
expect(title).toBe('Scheduled'); expect(title).toBe('Scheduled');
expect(subtitle).toBe(`for ${time}`);
expect(color).toBe('warning'); expect(color).toBe('warning');
expect(reason).toBeNull(); expect(reason).toBeNull();
render(subtitle);
screen.getByText('for');
const timeElement = screen.getByRole('time');
const datetime = timeElement.getAttribute('datetime');
expect(new Date(datetime || 1)).toEqual(date);
}); });
test('returns correct props for a failed schedule', () => { test('returns correct props for a failed schedule', () => {
const date = new Date();
const schedule = { const schedule = {
scheduledAt: new Date().toISOString(), scheduledAt: date.toISOString(),
status: 'failed' as const, status: 'failed' as const,
reason: 'reason', reason: 'reason',
failureReason: 'failure reason', failureReason: 'failure reason',
}; };
const time = 'some time string'; const { title, subtitle, color, reason } = getScheduleProps(schedule);
const { title, subtitle, color, reason } = getScheduleProps(
schedule,
time,
);
expect(title).toBe('Schedule failed'); expect(title).toBe('Schedule failed');
expect(subtitle).toBe(`at ${time}`);
expect(color).toBe('error'); expect(color).toBe('error');
expect(reason).toBeTruthy(); expect(reason).toBeTruthy();
render(subtitle);
screen.getByText('at');
const timeElement = screen.getByRole('time');
const datetime = timeElement.getAttribute('datetime');
expect(new Date(datetime || 1)).toEqual(date);
}); });
test('returns correct props for a suspended schedule', () => { test('returns correct props for a suspended schedule', () => {
const date = new Date();
const schedule = { const schedule = {
scheduledAt: new Date().toISOString(), scheduledAt: date.toISOString(),
status: 'suspended' as const, status: 'suspended' as const,
reason: 'reason', reason: 'reason',
}; };
const time = 'some time string'; const { title, subtitle, color, reason } = getScheduleProps(schedule);
const { title, subtitle, color, reason } = getScheduleProps(
schedule,
time,
);
expect(title).toBe('Schedule suspended'); expect(title).toBe('Schedule suspended');
expect(subtitle).toBe(`was ${time}`);
expect(color).toBe('grey'); expect(color).toBe('grey');
expect(reason).toBeTruthy(); expect(reason).toBeTruthy();
render(subtitle);
screen.getByText('was');
const timeElement = screen.getByRole('time');
const datetime = timeElement.getAttribute('datetime');
expect(new Date(datetime || 1)).toEqual(date);
}); });
}); });

View File

@ -1,7 +1,7 @@
import type { FC } from 'react'; import type { FC, ReactNode } from 'react';
import { Box, Paper, styled, Typography } from '@mui/material'; import { Box, Paper, styled, Typography } from '@mui/material';
import Timeline from '@mui/lab/Timeline'; import Timeline from '@mui/lab/Timeline';
import TimelineItem, { timelineItemClasses } from '@mui/lab/TimelineItem'; import MuiTimelineItem, { timelineItemClasses } from '@mui/lab/TimelineItem';
import TimelineSeparator from '@mui/lab/TimelineSeparator'; import TimelineSeparator from '@mui/lab/TimelineSeparator';
import TimelineDot from '@mui/lab/TimelineDot'; import TimelineDot from '@mui/lab/TimelineDot';
import TimelineConnector from '@mui/lab/TimelineConnector'; import TimelineConnector from '@mui/lab/TimelineConnector';
@ -9,16 +9,16 @@ import TimelineContent from '@mui/lab/TimelineContent';
import type { import type {
ChangeRequestSchedule, ChangeRequestSchedule,
ChangeRequestState, ChangeRequestState,
ChangeRequestType,
} from '../../changeRequest.types'; } from '../../changeRequest.types';
import { HtmlTooltip } from '../../../common/HtmlTooltip/HtmlTooltip.tsx'; import { HtmlTooltip } from '../../../common/HtmlTooltip/HtmlTooltip.tsx';
import ErrorIcon from '@mui/icons-material/Error'; import ErrorIcon from '@mui/icons-material/Error';
import { import { useLocationSettings } from 'hooks/useLocationSettings';
type ILocationSettings, import { formatDateYMDHM } from 'utils/formatDate.ts';
useLocationSettings,
} from 'hooks/useLocationSettings';
import { formatDateYMDHMS } from 'utils/formatDate';
export type ISuggestChangeTimelineProps = export type ISuggestChangeTimelineProps = {
timestamps?: ChangeRequestType['stateTransitionTimestamps']; // todo: update with flag `timestampsInChangeRequestTimeline`
} & (
| { | {
state: Exclude<ChangeRequestState, 'Scheduled'>; state: Exclude<ChangeRequestState, 'Scheduled'>;
schedule?: undefined; schedule?: undefined;
@ -26,7 +26,13 @@ export type ISuggestChangeTimelineProps =
| { | {
state: 'Scheduled'; state: 'Scheduled';
schedule: ChangeRequestSchedule; schedule: ChangeRequestSchedule;
}; }
);
const StyledTimelineContent = styled(TimelineContent)(({ theme }) => ({
display: 'flex',
flexDirection: 'column',
}));
const StyledPaper = styled(Paper)(({ theme }) => ({ const StyledPaper = styled(Paper)(({ theme }) => ({
marginTop: theme.spacing(2), marginTop: theme.spacing(2),
@ -89,6 +95,7 @@ export const determineColor = (
export const ChangeRequestTimeline: FC<ISuggestChangeTimelineProps> = ({ export const ChangeRequestTimeline: FC<ISuggestChangeTimelineProps> = ({
state, state,
schedule, schedule,
timestamps,
}) => { }) => {
let data: ChangeRequestState[]; let data: ChangeRequestState[];
switch (state) { switch (state) {
@ -102,17 +109,24 @@ export const ChangeRequestTimeline: FC<ISuggestChangeTimelineProps> = ({
data = steps; data = steps;
} }
const activeIndex = data.findIndex((item) => item === state); const activeIndex = data.findIndex((item) => item === state);
const { locationSettings } = useLocationSettings();
return ( return (
<StyledPaper elevation={0}> <StyledPaper elevation={0}>
<StyledBox> <StyledBox>
<StyledTimeline> <StyledTimeline>
{data.map((title, index) => { {data.map((title, index) => {
const timestampComponent =
index <= activeIndex && timestamps?.[title] ? (
<Time dateTime={timestamps?.[title]} />
) : undefined;
if (schedule && title === 'Scheduled') { if (schedule && title === 'Scheduled') {
return createTimelineScheduleItem( return (
schedule, <TimelineScheduleItem
locationSettings, key={title}
schedule={schedule}
timestamp={timestampComponent}
/>
); );
} }
@ -132,11 +146,17 @@ export const ChangeRequestTimeline: FC<ISuggestChangeTimelineProps> = ({
timelineDotProps = { variant: 'outlined' }; timelineDotProps = { variant: 'outlined' };
} }
return createTimelineItem( return (
color, <TimelineItem
title, key={title}
index < data.length - 1, color={color}
timelineDotProps, title={title}
shouldConnectToNextItem={
index < data.length - 1
}
timestamp={timestampComponent}
timelineDotProps={timelineDotProps}
/>
); );
})} })}
</StyledTimeline> </StyledTimeline>
@ -145,30 +165,61 @@ export const ChangeRequestTimeline: FC<ISuggestChangeTimelineProps> = ({
); );
}; };
const createTimelineItem = ( const Time = styled(({ dateTime, ...props }: { dateTime: string }) => {
color: 'primary' | 'success' | 'grey' | 'error' | 'warning', const { locationSettings } = useLocationSettings();
title: string, const displayTime = formatDateYMDHM(
shouldConnectToNextItem: boolean, new Date(dateTime || ''),
timelineDotProps: { [key: string]: string | undefined } = {}, locationSettings.locale,
) => ( );
<TimelineItem key={title}> return (
<time {...props} dateTime={dateTime}>
{displayTime}
</time>
);
})(({ theme }) => ({
color: theme.palette.text.secondary,
fontSize: theme.typography.body2.fontSize,
}));
const TimelineItem = ({
color,
title,
shouldConnectToNextItem,
timestamp,
timelineDotProps = {},
}: {
color: 'primary' | 'success' | 'grey' | 'error' | 'warning';
title: string;
shouldConnectToNextItem: boolean;
timestamp?: ReactNode;
timelineDotProps: { [key: string]: string | undefined };
}) => {
return (
<MuiTimelineItem key={title}>
<TimelineSeparator> <TimelineSeparator>
<TimelineDot color={color} {...timelineDotProps} /> <TimelineDot color={color} {...timelineDotProps} />
{shouldConnectToNextItem && <TimelineConnector />} {shouldConnectToNextItem && <TimelineConnector />}
</TimelineSeparator> </TimelineSeparator>
<TimelineContent>{title}</TimelineContent> <StyledTimelineContent>
</TimelineItem> {title}
); {timestamp}
</StyledTimelineContent>
</MuiTimelineItem>
);
};
export const getScheduleProps = (schedule: ChangeRequestSchedule) => {
const Subtitle = ({ prefix }: { prefix: string }) => (
<>
{prefix} <Time dateTime={schedule.scheduledAt} />
</>
);
export const getScheduleProps = (
schedule: ChangeRequestSchedule,
formattedTime: string,
) => {
switch (schedule.status) { switch (schedule.status) {
case 'suspended': case 'suspended':
return { return {
title: 'Schedule suspended', title: 'Schedule suspended',
subtitle: `was ${formattedTime}`, subtitle: <Subtitle prefix='was' />,
color: 'grey' as const, color: 'grey' as const,
reason: ( reason: (
<HtmlTooltip title={schedule.reason} arrow> <HtmlTooltip title={schedule.reason} arrow>
@ -179,7 +230,7 @@ export const getScheduleProps = (
case 'failed': case 'failed':
return { return {
title: 'Schedule failed', title: 'Schedule failed',
subtitle: `at ${formattedTime}`, subtitle: <Subtitle prefix='at' />,
color: 'error' as const, color: 'error' as const,
reason: ( reason: (
<HtmlTooltip <HtmlTooltip
@ -195,40 +246,38 @@ export const getScheduleProps = (
default: default:
return { return {
title: 'Scheduled', title: 'Scheduled',
subtitle: `for ${formattedTime}`, subtitle: <Subtitle prefix='for' />,
color: 'warning' as const, color: 'warning' as const,
reason: null, reason: null,
}; };
} }
}; };
const createTimelineScheduleItem = ( const TimelineScheduleItem = ({
schedule: ChangeRequestSchedule, schedule,
locationSettings: ILocationSettings, timestamp,
) => { }: {
const time = formatDateYMDHMS( schedule: ChangeRequestSchedule;
new Date(schedule.scheduledAt), timestamp: ReactNode;
locationSettings?.locale, }) => {
); const { title, subtitle, color, reason } = getScheduleProps(schedule);
const { title, subtitle, color, reason } = getScheduleProps(schedule, time);
return ( return (
<TimelineItem key={title}> <MuiTimelineItem key={title}>
<TimelineSeparator> <TimelineSeparator>
<TimelineDot color={color} /> <TimelineDot color={color} />
<TimelineConnector /> <TimelineConnector />
</TimelineSeparator> </TimelineSeparator>
<TimelineContent> <StyledTimelineContent>
{title} {title}
{timestamp}
<StyledSubtitle> <StyledSubtitle>
<Typography <Typography color={'text.secondary'} sx={{ mr: 1 }}>
color={'text.secondary'} ({subtitle})
sx={{ mr: 1 }} </Typography>
>{`(${subtitle})`}</Typography>
{reason} {reason}
</StyledSubtitle> </StyledSubtitle>
</TimelineContent> </StyledTimelineContent>
</TimelineItem> </MuiTimelineItem>
); );
}; };

View File

@ -19,6 +19,7 @@ type BaseChangeRequest = {
rejections: IChangeRequestApproval[]; rejections: IChangeRequestApproval[];
comments: IChangeRequestComment[]; comments: IChangeRequestComment[];
conflict?: string; conflict?: string;
stateTransitionTimestamps?: Partial<Record<ChangeRequestState, string>>; // todo(timestampsInChangeRequestTimeline): make sure this matches the model and what we return from the API
}; };
export type UnscheduledChangeRequest = BaseChangeRequest & { export type UnscheduledChangeRequest = BaseChangeRequest & {