From 39145e261738da8dcb96610babd63b9f19109509 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Fri, 12 Jan 2024 13:15:43 +0530 Subject: [PATCH] refactor: use union types for change request types (#5870) This changes the two interfaces IChangeRequest and IChangeRequestSchedule to be union types instead of interfaces. It also extracts the constituents of those union types into proper types themselves (so that they can be used in function type signatures etc). It also updates the type names. This turned out to be more work than I had imagined, but I think the end result pays off, giving us more type safety and control. I wanted to use just `ChangeRequest` for the IChangeRequest type, but that caused issues due to naming collisions with the `ChangeRequest` component that we have, causing tests to fail. I've named it `ChangeRequestType` as a potential solution, but suggestions are welcome. The relevant changes are in `frontend/src/component/changeRequest/changeRequest.types.ts`. Everything else is updated references and some necessary refactoring to respect the new types. --- .../ChangeRequest/ChangeRequest.test.tsx | 6 +- .../ChangeRequest/ChangeRequest.tsx | 4 +- .../Changes/Change/ChangeActions.tsx | 6 +- .../Changes/Change/FeatureChange.test.tsx | 49 +++++++++++----- .../Changes/Change/FeatureChange.tsx | 4 +- .../ChangeRequestHeader.tsx | 4 +- .../ChangeRequestOverview.test.tsx | 41 ++++++++----- .../ChangeRequestOverview.tsx | 58 +++++++++++++------ .../ChangeRequestReviewStatus.tsx | 30 +++++----- .../ChangeRequestReviewers.tsx | 4 +- .../ChangeRequestTitle.tsx | 4 +- .../EnvironmentChangeRequest.tsx | 4 +- .../EnvironmentChangeRequestTitle.test.tsx | 6 +- .../ChangeRequestStatusBadge.tsx | 22 ++++--- .../ChangeRequestStatusCell.tsx | 4 +- .../changeRequest/changeRequest.types.ts | 44 +++++++++++--- .../component/changeRequest/changesCount.ts | 4 +- .../StrategyDraggableItem.test.tsx | 4 +- .../FeatureSettingsProjectConfirm.tsx | 4 +- .../EnvironmentVariantsCard.test.tsx | 4 +- .../MainLayout/DraftBanner/DraftBanner.tsx | 6 +- .../ProjectInfo/ChangeRequestsWidget.tsx | 8 ++- .../useChangeRequest/useChangeRequest.ts | 4 +- .../usePendingChangeRequests.ts | 4 +- .../usePendingChangeRequestsForFeature.ts | 4 +- .../hooks/useChangeRequestInReviewWarning.tsx | 4 +- 26 files changed, 214 insertions(+), 122 deletions(-) diff --git a/frontend/src/component/changeRequest/ChangeRequest/ChangeRequest.test.tsx b/frontend/src/component/changeRequest/ChangeRequest/ChangeRequest.test.tsx index 78b1c16224..8523ae94c7 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/ChangeRequest.test.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/ChangeRequest.test.tsx @@ -5,7 +5,7 @@ import { Route, Routes } from 'react-router-dom'; import { render } from 'utils/testRenderer'; import { ChangeRequest } from './ChangeRequest'; import { - IChangeRequest, + ChangeRequestType, IChangeRequestAddStrategy, IChangeRequestEnabled, } from '../changeRequest.types'; @@ -98,7 +98,7 @@ const feature = () => const changeRequestWithDefaultChange = ( defaultChange: IChangeRequestEnabled | IChangeRequestAddStrategy, ) => { - const changeRequest: IChangeRequest = { + const changeRequest: ChangeRequestType = { approvals: [], rejections: [], comments: [], @@ -142,7 +142,7 @@ const changeRequestWithDefaultChange = ( }; const changeRequest = (variants: StrategyVariantSchema[]) => { - const changeRequest: IChangeRequest = { + const changeRequest: ChangeRequestType = { approvals: [], rejections: [], comments: [], diff --git a/frontend/src/component/changeRequest/ChangeRequest/ChangeRequest.tsx b/frontend/src/component/changeRequest/ChangeRequest/ChangeRequest.tsx index 77bee1aa54..4833ddfc88 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/ChangeRequest.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/ChangeRequest.tsx @@ -1,6 +1,6 @@ import { VFC } from 'react'; import { Box, Typography } from '@mui/material'; -import type { IChangeRequest } from '../changeRequest.types'; +import type { ChangeRequestType } from '../changeRequest.types'; import { FeatureToggleChanges } from './Changes/FeatureToggleChanges'; import { FeatureChange } from './Changes/Change/FeatureChange'; import { ChangeActions } from './Changes/Change/ChangeActions'; @@ -8,7 +8,7 @@ import { ConditionallyRender } from 'component/common/ConditionallyRender/Condit import { SegmentChange } from './Changes/Change/SegmentChange'; interface IChangeRequestProps { - changeRequest: IChangeRequest; + changeRequest: ChangeRequestType; onRefetch?: () => void; onNavigate?: () => void; } diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ChangeActions.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ChangeActions.tsx index a506224075..6b8dae7783 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ChangeActions.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ChangeActions.tsx @@ -1,6 +1,6 @@ import React, { FC, useState } from 'react'; import { - IChangeRequest, + ChangeRequestType, IChangeRequestAddStrategy, IChangeRequestUpdateStrategy, IChange, @@ -28,7 +28,7 @@ import { EditChange } from './EditChange'; import { useUiFlag } from 'hooks/useUiFlag'; import { NewEditChange } from './NewEditChange'; -const useShowActions = (changeRequest: IChangeRequest, change: IChange) => { +const useShowActions = (changeRequest: ChangeRequestType, change: IChange) => { const { isChangeRequestConfigured } = useChangeRequestsEnabled( changeRequest.project, ); @@ -60,7 +60,7 @@ const StyledPopover = styled(Popover)(({ theme }) => ({ })); export const ChangeActions: FC<{ - changeRequest: IChangeRequest; + changeRequest: ChangeRequestType; feature: string; change: IChange; onRefetch?: () => void; diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/FeatureChange.test.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/FeatureChange.test.tsx index 80c2b167a4..57626069f4 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/FeatureChange.test.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/FeatureChange.test.tsx @@ -3,6 +3,7 @@ import { screen } from '@testing-library/react'; import { FeatureChange } from './FeatureChange'; import { ChangeRequestState, + ChangeRequestType, IChangeRequestFeature, IFeatureChange, } from 'component/changeRequest/changeRequest.types'; @@ -40,21 +41,39 @@ describe('Schedule conflicts', () => { }); const changeRequest = - (feature: IChangeRequestFeature) => (state: ChangeRequestState) => ({ - id: 1, - state, - title: '', - project: 'default', - environment: 'default', - minApprovals: 1, - createdBy: { id: 1, username: 'user1', imageUrl: '' }, - createdAt: new Date(), - features: [feature], - segments: [], - approvals: [], - rejections: [], - comments: [], - }); + (feature: IChangeRequestFeature) => + (state: ChangeRequestState): ChangeRequestType => { + const shared = { + id: 1, + title: '', + project: 'default', + environment: 'default', + minApprovals: 1, + createdBy: { id: 1, username: 'user1', imageUrl: '' }, + createdAt: new Date(), + features: [feature], + segments: [], + approvals: [], + rejections: [], + comments: [], + }; + + if (state === 'Scheduled') { + return { + ...shared, + state, + schedule: { + scheduledAt: '2024-01-12T09:46:51+05:30', + status: 'pending', + }, + }; + } + + return { + ...shared, + state, + }; + }; it.each(['Draft', 'Scheduled', 'In review', 'Approved'])( 'should show schedule conflicts (when they exist) for change request in the %s state', diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/FeatureChange.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/FeatureChange.tsx index 7ec9b32a18..d111f3218a 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/FeatureChange.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/FeatureChange.tsx @@ -1,7 +1,7 @@ import { FC, ReactNode } from 'react'; import { IFeatureChange, - IChangeRequest, + ChangeRequestType, IChangeRequestFeature, } from '../../../changeRequest.types'; import { objectId } from 'utils/objectId'; @@ -69,7 +69,7 @@ const InlineList = styled('ul')(({ theme }) => ({ export const FeatureChange: FC<{ actions: ReactNode; index: number; - changeRequest: IChangeRequest; + changeRequest: ChangeRequestType; change: IFeatureChange; feature: IChangeRequestFeature; onNavigate?: () => void; diff --git a/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestHeader/ChangeRequestHeader.tsx b/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestHeader/ChangeRequestHeader.tsx index 6e2eaa75a1..1e0ceb88ea 100644 --- a/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestHeader/ChangeRequestHeader.tsx +++ b/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestHeader/ChangeRequestHeader.tsx @@ -2,7 +2,7 @@ import { Box } from '@mui/material'; import { FC, useState } from 'react'; import { Typography, Tooltip } from '@mui/material'; import TimeAgo from 'react-timeago'; -import { IChangeRequest } from 'component/changeRequest/changeRequest.types'; +import { ChangeRequestType } from 'component/changeRequest/changeRequest.types'; import { ChangeRequestStatusBadge } from 'component/changeRequest/ChangeRequestStatusBadge/ChangeRequestStatusBadge'; import { StyledPaper, @@ -15,7 +15,7 @@ import { Separator } from '../../ChangeRequestSidebar/ChangeRequestSidebar'; import { ChangeRequestTitle } from '../../ChangeRequestSidebar/EnvironmentChangeRequest/ChangeRequestTitle'; import { UpdateCount } from 'component/changeRequest/UpdateCount'; -export const ChangeRequestHeader: FC<{ changeRequest: IChangeRequest }> = ({ +export const ChangeRequestHeader: FC<{ changeRequest: ChangeRequestType }> = ({ changeRequest, }) => { const [title, setTitle] = useState(changeRequest.title); diff --git a/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestOverview.test.tsx b/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestOverview.test.tsx index 71f64c1dae..7398f39a02 100644 --- a/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestOverview.test.tsx +++ b/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestOverview.test.tsx @@ -1,6 +1,6 @@ import { fireEvent, screen, waitFor, within } from '@testing-library/react'; import { testServerRoute, testServerSetup } from 'utils/testServer'; -import { ChangeRequestState, IChangeRequest } from '../changeRequest.types'; +import { ChangeRequestState, ChangeRequestType } from '../changeRequest.types'; import { render } from 'utils/testRenderer'; import { ChangeRequestOverview } from './ChangeRequestOverview'; import { @@ -14,11 +14,10 @@ const mockChangeRequest = ( featureName: string, state: ChangeRequestState, failureReason?: string, -): IChangeRequest => { - const result: IChangeRequest = { +): ChangeRequestType => { + const shared: Omit = { id: 1, environment: 'production', - state: state, minApprovals: 1, project: 'default', createdBy: { @@ -60,26 +59,40 @@ const mockChangeRequest = ( }; if (state === 'Scheduled') { - result.schedule = { - scheduledAt: '2022-12-02T09:19:12.242Z', - status: 'pending', + if (failureReason) { + return { + ...shared, + state, + schedule: { + scheduledAt: '2022-12-02T09:19:12.242Z', + status: 'failed', + reason: failureReason, + }, + }; + } + return { + ...shared, + state, + schedule: { + scheduledAt: '2022-12-02T09:19:12.242Z', + status: 'pending', + }, }; } - if (failureReason) { - result.schedule!.failureReason = failureReason; - } - - return result; + return { + ...shared, + state, + }; }; -const pendingChangeRequest = (changeRequest: IChangeRequest) => +const pendingChangeRequest = (changeRequest: ChangeRequestType) => testServerRoute( server, '/api/admin/projects/default/change-requests/pending', [changeRequest], ); -const changeRequest = (changeRequest: IChangeRequest) => +const changeRequest = (changeRequest: ChangeRequestType) => testServerRoute( server, '/api/admin/projects/default/change-requests/1', diff --git a/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestOverview.tsx b/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestOverview.tsx index b0a8fbec67..0305e47080 100644 --- a/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestOverview.tsx +++ b/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestOverview.tsx @@ -111,7 +111,9 @@ export const ChangeRequestOverview: FC = () => { changeRequest.environment, ); - const hasSchedule = Boolean(changeRequest.schedule?.scheduledAt); + const hasSchedule = Boolean( + 'schedule' in changeRequest && changeRequest.schedule?.scheduledAt, + ); const onApplyChanges = async () => { try { @@ -259,6 +261,30 @@ export const ChangeRequestOverview: FC = () => { const countOfChanges = changesCount(changeRequest); + const reason = (() => { + if (!('schedule' in changeRequest)) { + return undefined; + } + + switch (changeRequest.schedule.status) { + case 'failed': + return ( + (changeRequest.schedule.reason || + changeRequest.schedule.failureReason) ?? + undefined + ); + case 'suspended': + return changeRequest.schedule.reason; + default: + return undefined; + } + })(); + + const scheduledAt = + 'schedule' in changeRequest + ? changeRequest.schedule.scheduledAt + : undefined; + return ( <> @@ -266,8 +292,12 @@ export const ChangeRequestOverview: FC = () => { @@ -417,10 +447,7 @@ export const ChangeRequestOverview: FC = () => { { projectId={projectId} environment={changeRequest.environment} primaryButtonText={ - changeRequest?.schedule?.scheduledAt + changeRequest.state === 'Scheduled' ? 'Update scheduled time' : 'Schedule changes' } title={ - changeRequest?.schedule?.scheduledAt + changeRequest.state === 'Scheduled' ? 'Update schedule' : 'Schedule changes' } - scheduledAt={ - changeRequest?.schedule?.scheduledAt || - undefined - } + scheduledAt={scheduledAt} /> { open={showRejectScheduledDialog} onConfirm={onReject} onClose={onRejectScheduledAbort} - scheduledTime={ - changeRequest?.schedule?.scheduledAt - } + scheduledTime={scheduledAt} /> } diff --git a/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestReviewStatus/ChangeRequestReviewStatus.tsx b/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestReviewStatus/ChangeRequestReviewStatus.tsx index 67e2d64b24..efa6b8be4a 100644 --- a/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestReviewStatus/ChangeRequestReviewStatus.tsx +++ b/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestReviewStatus/ChangeRequestReviewStatus.tsx @@ -26,15 +26,16 @@ import { } from './ChangeRequestReviewStatus.styles'; import { ChangeRequestState, - IChangeRequest, - IChangeRequestSchedule, + ChangeRequestType, + ChangeRequestSchedule, + ChangeRequestScheduleFailed, } from 'component/changeRequest/changeRequest.types'; import { getBrowserTimezone } from './utils'; import { ConditionallyRender } from '../../../common/ConditionallyRender/ConditionallyRender'; import { formatDateYMDHMS } from 'utils/formatDate'; interface ISuggestChangeReviewsStatusProps { - changeRequest: IChangeRequest; + changeRequest: ChangeRequestType; onEditClick?: () => void; } const resolveBorder = (state: ChangeRequestState, theme: Theme) => { @@ -103,7 +104,7 @@ export const ChangeRequestReviewStatus: FC = }; interface IResolveComponentProps { - changeRequest: IChangeRequest; + changeRequest: ChangeRequestType; onEditClick?: () => void; } @@ -111,7 +112,7 @@ const ResolveComponent = ({ changeRequest, onEditClick, }: IResolveComponentProps) => { - const { state, schedule } = changeRequest; + const { state } = changeRequest; if (!state) { return null; @@ -134,6 +135,7 @@ const ResolveComponent = ({ } if (state === 'Scheduled') { + const { schedule } = changeRequest; return ; } @@ -228,13 +230,11 @@ const StyledIconButton = styled(IconButton)({ }); interface IScheduledProps { - schedule?: IChangeRequest['schedule']; + schedule?: ChangeRequestSchedule; onEditClick?: () => any; } const Scheduled = ({ schedule, onEditClick }: IScheduledProps) => { const theme = useTheme(); - const timezone = getBrowserTimezone(); - const { locationSettings } = useLocationSettings(); if (!schedule?.scheduledAt) { return null; @@ -258,9 +258,13 @@ const Scheduled = ({ schedule, onEditClick }: IScheduledProps) => { } - elseShow={} + elseShow={ + + } /> @@ -273,7 +277,7 @@ const Scheduled = ({ schedule, onEditClick }: IScheduledProps) => { const ScheduledFailed = ({ schedule, -}: { schedule: IChangeRequestSchedule }) => { +}: { schedule: ChangeRequestScheduleFailed }) => { const theme = useTheme(); const timezone = getBrowserTimezone(); const { locationSettings } = useLocationSettings(); @@ -293,7 +297,7 @@ const ScheduledFailed = ({ Changes failed to be applied on {scheduledTime} because of{' '} - {schedule?.failureReason} + {schedule.reason ?? schedule.failureReason} Your timezone is {timezone} @@ -303,7 +307,7 @@ const ScheduledFailed = ({ const ScheduledPending = ({ schedule, -}: { schedule: IChangeRequestSchedule }) => { +}: { schedule: ChangeRequestSchedule }) => { const theme = useTheme(); const timezone = getBrowserTimezone(); const { locationSettings } = useLocationSettings(); diff --git a/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestReviewers/ChangeRequestReviewers.tsx b/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestReviewers/ChangeRequestReviewers.tsx index 20a8e380df..c9b516b8f3 100644 --- a/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestReviewers/ChangeRequestReviewers.tsx +++ b/frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestReviewers/ChangeRequestReviewers.tsx @@ -3,7 +3,7 @@ import { FC, ReactNode } from 'react'; import { ConditionallyRender } from '../../../common/ConditionallyRender/ConditionallyRender'; import { ChangeRequestRejections } from './ChangeRequestRejections'; import { ChangeRequestApprovals } from './ChangeRequestApprovals'; -import { IChangeRequest } from '../../changeRequest.types'; +import { ChangeRequestType } from '../../changeRequest.types'; const StyledBox = styled(Box)(({ theme }) => ({ marginBottom: theme.spacing(2), @@ -44,7 +44,7 @@ export const ChangeRequestReviewersWrapper: FC<{ header: ReactNode }> = ({ export const ChangeRequestReviewers: FC<{ changeRequest: Pick< - IChangeRequest, + ChangeRequestType, 'approvals' | 'rejections' | 'state' | 'minApprovals' >; }> = ({ changeRequest }) => ( diff --git a/frontend/src/component/changeRequest/ChangeRequestSidebar/EnvironmentChangeRequest/ChangeRequestTitle.tsx b/frontend/src/component/changeRequest/ChangeRequestSidebar/EnvironmentChangeRequest/ChangeRequestTitle.tsx index 0159b7ae3e..399bdd7c00 100644 --- a/frontend/src/component/changeRequest/ChangeRequestSidebar/EnvironmentChangeRequest/ChangeRequestTitle.tsx +++ b/frontend/src/component/changeRequest/ChangeRequestSidebar/EnvironmentChangeRequest/ChangeRequestTitle.tsx @@ -1,7 +1,7 @@ import React, { FC, useState } from 'react'; import { Box, Button, IconButton, styled, Typography } from '@mui/material'; import Input from 'component/common/Input/Input'; -import { IChangeRequest } from '../../changeRequest.types'; +import { ChangeRequestType } from '../../changeRequest.types'; import { Edit } from '@mui/icons-material'; import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; import { useChangeRequestApi } from 'hooks/api/actions/useChangeRequestApi/useChangeRequestApi'; @@ -25,7 +25,7 @@ export const StyledHeader = styled(Typography)(({ theme }) => ({ })); export const ChangeRequestTitle: FC<{ - environmentChangeRequest: IChangeRequest; + environmentChangeRequest: ChangeRequestType; title: string; setTitle: React.Dispatch>; }> = ({ environmentChangeRequest, title, setTitle, children }) => { diff --git a/frontend/src/component/changeRequest/ChangeRequestSidebar/EnvironmentChangeRequest/EnvironmentChangeRequest.tsx b/frontend/src/component/changeRequest/ChangeRequestSidebar/EnvironmentChangeRequest/EnvironmentChangeRequest.tsx index 77edd563bc..7a38a6fe14 100644 --- a/frontend/src/component/changeRequest/ChangeRequestSidebar/EnvironmentChangeRequest/EnvironmentChangeRequest.tsx +++ b/frontend/src/component/changeRequest/ChangeRequestSidebar/EnvironmentChangeRequest/EnvironmentChangeRequest.tsx @@ -7,7 +7,7 @@ import { Typography, useTheme, } from '@mui/material'; -import { IChangeRequest } from '../../changeRequest.types'; +import { ChangeRequestType } from '../../changeRequest.types'; import { useNavigate } from 'react-router-dom'; import { ChangeRequestStatusBadge } from '../../ChangeRequestStatusBadge/ChangeRequestStatusBadge'; import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; @@ -54,7 +54,7 @@ const ChangeRequestContent = styled(Box)(({ theme }) => ({ })); export const EnvironmentChangeRequest: FC<{ - environmentChangeRequest: IChangeRequest; + environmentChangeRequest: ChangeRequestType; onClose: () => void; onReview: (id: number, comment?: string) => void; onDiscard: (id: number) => void; diff --git a/frontend/src/component/changeRequest/ChangeRequestSidebar/EnvironmentChangeRequest/EnvironmentChangeRequestTitle.test.tsx b/frontend/src/component/changeRequest/ChangeRequestSidebar/EnvironmentChangeRequest/EnvironmentChangeRequestTitle.test.tsx index a5e25a1f24..95f24d511a 100644 --- a/frontend/src/component/changeRequest/ChangeRequestSidebar/EnvironmentChangeRequest/EnvironmentChangeRequestTitle.test.tsx +++ b/frontend/src/component/changeRequest/ChangeRequestSidebar/EnvironmentChangeRequest/EnvironmentChangeRequestTitle.test.tsx @@ -1,14 +1,14 @@ import { FC, useState } from 'react'; import { screen } from '@testing-library/react'; import { ChangeRequestTitle } from './ChangeRequestTitle'; -import { ChangeRequestState } from 'component/changeRequest/changeRequest.types'; +import { UnscheduledChangeRequest } from 'component/changeRequest/changeRequest.types'; import userEvent from '@testing-library/user-event'; import { testServerRoute, testServerSetup } from 'utils/testServer'; import { render } from 'utils/testRenderer'; -const changeRequest = { +const changeRequest: UnscheduledChangeRequest = { id: 3, - state: 'Draft' as ChangeRequestState, + state: 'Draft' as const, title: 'My title', project: 'default', environment: 'default', diff --git a/frontend/src/component/changeRequest/ChangeRequestStatusBadge/ChangeRequestStatusBadge.tsx b/frontend/src/component/changeRequest/ChangeRequestStatusBadge/ChangeRequestStatusBadge.tsx index 977ddc9d15..5c5d320616 100644 --- a/frontend/src/component/changeRequest/ChangeRequestStatusBadge/ChangeRequestStatusBadge.tsx +++ b/frontend/src/component/changeRequest/ChangeRequestStatusBadge/ChangeRequestStatusBadge.tsx @@ -1,5 +1,5 @@ import { VFC } from 'react'; -import { IChangeRequest } from '../changeRequest.types'; +import { ChangeRequestType } from '../changeRequest.types'; import { Badge } from 'component/common/Badge/Badge'; import { AccessTime, @@ -11,7 +11,7 @@ import { import { HtmlTooltip } from 'component/common/HtmlTooltip/HtmlTooltip'; interface IChangeRequestStatusBadgeProps { - changeRequest: IChangeRequest | undefined; + changeRequest: ChangeRequestType | undefined; } const ReviewRequiredBadge: VFC = () => ( @@ -71,12 +71,18 @@ export const ChangeRequestStatusBadge: VFC = ({ schedule!.scheduledAt, ).toLocaleString(); - const tooltipTitle = - schedule?.status === 'pending' - ? `Scheduled for ${scheduledAt}` - : `Failed on ${scheduledAt} because of ${ - schedule!.failureReason - }`; + const tooltipTitle = (() => { + switch (schedule.status) { + case 'failed': + return `Failed on ${scheduledAt} because of ${ + schedule.reason || schedule.failureReason + }`; + case 'suspended': + return schedule.reason; + default: + return `Scheduled for ${scheduledAt}`; + } + })(); return ( diff --git a/frontend/src/component/changeRequest/ProjectChangeRequests/ChangeRequestsTabs/ChangeRequestStatusCell.tsx b/frontend/src/component/changeRequest/ProjectChangeRequests/ChangeRequestsTabs/ChangeRequestStatusCell.tsx index d497cf0a80..398c963602 100644 --- a/frontend/src/component/changeRequest/ProjectChangeRequests/ChangeRequestsTabs/ChangeRequestStatusCell.tsx +++ b/frontend/src/component/changeRequest/ProjectChangeRequests/ChangeRequestsTabs/ChangeRequestStatusCell.tsx @@ -1,11 +1,11 @@ import { VFC } from 'react'; import { TextCell } from 'component/common/Table/cells/TextCell/TextCell'; -import { IChangeRequest } from 'component/changeRequest/changeRequest.types'; +import { ChangeRequestType } from 'component/changeRequest/changeRequest.types'; import { ChangeRequestStatusBadge } from 'component/changeRequest/ChangeRequestStatusBadge/ChangeRequestStatusBadge'; interface IChangeRequestStatusCellProps { value?: string | null; // FIXME: proper type - row: { original: IChangeRequest }; + row: { original: ChangeRequestType }; } export const ChangeRequestStatusCell: VFC = ({ diff --git a/frontend/src/component/changeRequest/changeRequest.types.ts b/frontend/src/component/changeRequest/changeRequest.types.ts index e90bfd6a5b..e46e9f11ab 100644 --- a/frontend/src/component/changeRequest/changeRequest.types.ts +++ b/frontend/src/component/changeRequest/changeRequest.types.ts @@ -3,7 +3,7 @@ import { IFeatureStrategy } from '../../interfaces/strategy'; import { IUser } from '../../interfaces/user'; import { SetStrategySortOrderSchema } from '../../openapi'; -export interface IChangeRequest { +type BaseChangeRequest = { id: number; title: string; project: string; @@ -17,15 +17,43 @@ export interface IChangeRequest { rejections: IChangeRequestApproval[]; comments: IChangeRequestComment[]; conflict?: string; - state: ChangeRequestState; - schedule?: IChangeRequestSchedule; -} +}; -export interface IChangeRequestSchedule { +export type UnscheduledChangeRequest = BaseChangeRequest & { + state: Exclude; +}; + +export type ScheduledChangeRequest = BaseChangeRequest & { + state: 'Scheduled'; + schedule: ChangeRequestSchedule; +}; + +export type ChangeRequestType = + | UnscheduledChangeRequest + | ScheduledChangeRequest; + +export type ChangeRequestSchedulePending = { + status: 'pending'; scheduledAt: string; - status: 'pending' | 'failed'; - failureReason?: string; -} +}; + +export type ChangeRequestScheduleFailed = { + status: 'failed'; + scheduledAt: string; + failureReason?: string | null; + reason: string; +}; + +export type ChangeRequestScheduleSuspended = { + status: 'suspended'; + scheduledAt: string; + reason: string; +}; + +export type ChangeRequestSchedule = + | ChangeRequestSchedulePending + | ChangeRequestScheduleFailed + | ChangeRequestScheduleSuspended; export interface IChangeRequestEnvironmentConfig { environment: string; diff --git a/frontend/src/component/changeRequest/changesCount.ts b/frontend/src/component/changeRequest/changesCount.ts index 391ed9dc12..9b15f8d2e4 100644 --- a/frontend/src/component/changeRequest/changesCount.ts +++ b/frontend/src/component/changeRequest/changesCount.ts @@ -1,5 +1,5 @@ -import { IChangeRequest } from './changeRequest.types'; +import { ChangeRequestType } from './changeRequest.types'; -export const changesCount = (changeRequest: IChangeRequest) => +export const changesCount = (changeRequest: ChangeRequestType) => changeRequest.features.flatMap((feature) => feature.changes).length + changeRequest.segments.length; diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyDraggableItem.test.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyDraggableItem.test.tsx index ab091f565f..6b35012d50 100644 --- a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyDraggableItem.test.tsx +++ b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyDraggableItem.test.tsx @@ -6,7 +6,7 @@ import { ADMIN } from 'component/providers/AccessProvider/permissions'; import { screen } from '@testing-library/dom'; import { Route, Routes } from 'react-router-dom'; import { - IChangeRequest, + ChangeRequestType, ChangeRequestAction, } from 'component/changeRequest/changeRequest.types'; @@ -30,7 +30,7 @@ const strategy = { const draftRequest = ( action: Omit = 'updateStrategy', createdBy = 1, -): IChangeRequest => { +): ChangeRequestType => { return { id: 71, title: 'Change request #71', diff --git a/frontend/src/component/feature/FeatureView/FeatureSettings/FeatureSettingsProject/FeatureSettingsProjectConfirm/FeatureSettingsProjectConfirm.tsx b/frontend/src/component/feature/FeatureView/FeatureSettings/FeatureSettingsProject/FeatureSettingsProjectConfirm/FeatureSettingsProjectConfirm.tsx index e21511a910..500ce225bc 100644 --- a/frontend/src/component/feature/FeatureView/FeatureSettings/FeatureSettingsProject/FeatureSettingsProjectConfirm/FeatureSettingsProjectConfirm.tsx +++ b/frontend/src/component/feature/FeatureView/FeatureSettings/FeatureSettingsProject/FeatureSettingsProjectConfirm/FeatureSettingsProjectConfirm.tsx @@ -6,7 +6,7 @@ import { Dialogue } from 'component/common/Dialogue/Dialogue'; import { arraysHaveSameItems } from 'utils/arraysHaveSameItems'; import { Alert, List, ListItem, styled } from '@mui/material'; import { Link } from 'react-router-dom'; -import { IChangeRequest } from 'component/changeRequest/changeRequest.types'; +import { ChangeRequestType } from 'component/changeRequest/changeRequest.types'; import { useRequiredPathParam } from 'hooks/useRequiredPathParam'; import { useChangeRequestsEnabled } from 'hooks/useChangeRequestsEnabled'; @@ -29,7 +29,7 @@ interface IFeatureSettingsProjectConfirm { onClose: () => void; onClick: (args: any) => void; feature: IFeatureToggle; - changeRequests: IChangeRequest[] | undefined; + changeRequests: ChangeRequestType[] | undefined; } const FeatureSettingsProjectConfirm = ({ diff --git a/frontend/src/component/feature/FeatureView/FeatureVariants/FeatureEnvironmentVariants/EnvironmentVariantsCard/EnvironmentVariantsCard.test.tsx b/frontend/src/component/feature/FeatureView/FeatureVariants/FeatureEnvironmentVariants/EnvironmentVariantsCard/EnvironmentVariantsCard.test.tsx index 583086a6ff..2478703c6e 100644 --- a/frontend/src/component/feature/FeatureView/FeatureVariants/FeatureEnvironmentVariants/EnvironmentVariantsCard/EnvironmentVariantsCard.test.tsx +++ b/frontend/src/component/feature/FeatureView/FeatureVariants/FeatureEnvironmentVariants/EnvironmentVariantsCard/EnvironmentVariantsCard.test.tsx @@ -5,7 +5,7 @@ import { screen } from '@testing-library/dom'; import { Route, Routes } from 'react-router-dom'; import { ChangeRequestAction, - IChangeRequest, + ChangeRequestType, } from 'component/changeRequest/changeRequest.types'; import { EnvironmentVariantsCard } from './EnvironmentVariantsCard'; import { IFeatureEnvironment } from 'interfaces/featureToggle'; @@ -30,7 +30,7 @@ const strategy = { const scheduledRequest = ( action: Omit = 'updateStrategy', createdBy = 1, -): IChangeRequest => { +): ChangeRequestType => { return { id: 71, title: 'Change request #71', diff --git a/frontend/src/component/layout/MainLayout/DraftBanner/DraftBanner.tsx b/frontend/src/component/layout/MainLayout/DraftBanner/DraftBanner.tsx index 334973c73c..b708294551 100644 --- a/frontend/src/component/layout/MainLayout/DraftBanner/DraftBanner.tsx +++ b/frontend/src/component/layout/MainLayout/DraftBanner/DraftBanner.tsx @@ -3,7 +3,7 @@ import { Box, Button, styled, Typography } from '@mui/material'; import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; import { ChangeRequestSidebar } from 'component/changeRequest/ChangeRequestSidebar/ChangeRequestSidebar'; import { usePendingChangeRequests } from 'hooks/api/getters/usePendingChangeRequests/usePendingChangeRequests'; -import { IChangeRequest } from 'component/changeRequest/changeRequest.types'; +import { ChangeRequestType } from 'component/changeRequest/changeRequest.types'; import { changesCount } from 'component/changeRequest/changesCount'; import { Sticky } from 'component/common/Sticky/Sticky'; import { useUiFlag } from 'hooks/useUiFlag'; @@ -61,7 +61,7 @@ const StyledSpaciousDraftBanner = styled(Box)(({ theme }) => ({ })); const DraftBannerContent: FC<{ - changeRequests: IChangeRequest[]; + changeRequests: ChangeRequestType[]; onClick: () => void; }> = ({ changeRequests, onClick }) => { const environments = changeRequests.map(({ environment }) => environment); @@ -165,7 +165,7 @@ export const DraftBanner: VFC = ({ project }) => { show={ { setIsSidebarOpen(true); diff --git a/frontend/src/component/project/Project/ProjectInfo/ChangeRequestsWidget.tsx b/frontend/src/component/project/Project/ProjectInfo/ChangeRequestsWidget.tsx index 41f372451e..190888bd3a 100644 --- a/frontend/src/component/project/Project/ProjectInfo/ChangeRequestsWidget.tsx +++ b/frontend/src/component/project/Project/ProjectInfo/ChangeRequestsWidget.tsx @@ -1,7 +1,7 @@ import { FC } from 'react'; import useLoading from 'hooks/useLoading'; import { Box, styled, Typography } from '@mui/material'; -import { IChangeRequest } from 'component/changeRequest/changeRequest.types'; +import { ChangeRequestType } from 'component/changeRequest/changeRequest.types'; import { StyledCount, @@ -74,10 +74,12 @@ export const ChangeRequestsWidget: FC = ({ const { changeRequests, loading } = useProjectChangeRequests(projectId); const loadingRef = useLoading(loading, `[data-loading="${LOADING_LABEL}"]`); const toBeApplied = changeRequests?.filter( - (changeRequest: IChangeRequest) => changeRequest?.state === 'Approved', + (changeRequest: ChangeRequestType) => + changeRequest?.state === 'Approved', ).length; const toBeReviewed = changeRequests?.filter( - (changeRequest: IChangeRequest) => changeRequest?.state === 'In review', + (changeRequest: ChangeRequestType) => + changeRequest?.state === 'In review', ).length; return ( diff --git a/frontend/src/hooks/api/getters/useChangeRequest/useChangeRequest.ts b/frontend/src/hooks/api/getters/useChangeRequest/useChangeRequest.ts index 025ba95e2a..e97ddebd30 100644 --- a/frontend/src/hooks/api/getters/useChangeRequest/useChangeRequest.ts +++ b/frontend/src/hooks/api/getters/useChangeRequest/useChangeRequest.ts @@ -1,10 +1,10 @@ import useSWR from 'swr'; import { formatApiPath } from 'utils/formatPath'; import handleErrorResponses from '../httpErrorResponseHandler'; -import { IChangeRequest } from 'component/changeRequest/changeRequest.types'; +import { ChangeRequestType } from 'component/changeRequest/changeRequest.types'; export const useChangeRequest = (projectId: string, id: string) => { - const { data, error, mutate } = useSWR( + const { data, error, mutate } = useSWR( formatApiPath(`api/admin/projects/${projectId}/change-requests/${id}`), fetcher, { refreshInterval: 15000 }, diff --git a/frontend/src/hooks/api/getters/usePendingChangeRequests/usePendingChangeRequests.ts b/frontend/src/hooks/api/getters/usePendingChangeRequests/usePendingChangeRequests.ts index 12ff757304..15cb9bfc81 100644 --- a/frontend/src/hooks/api/getters/usePendingChangeRequests/usePendingChangeRequests.ts +++ b/frontend/src/hooks/api/getters/usePendingChangeRequests/usePendingChangeRequests.ts @@ -1,6 +1,6 @@ import { formatApiPath } from 'utils/formatPath'; import handleErrorResponses from '../httpErrorResponseHandler'; -import { IChangeRequest } from 'component/changeRequest/changeRequest.types'; +import { ChangeRequestType } from 'component/changeRequest/changeRequest.types'; import { useEnterpriseSWR } from '../useEnterpriseSWR/useEnterpriseSWR'; const fetcher = (path: string) => { @@ -10,7 +10,7 @@ const fetcher = (path: string) => { }; export const usePendingChangeRequests = (project: string) => { - const { data, error, mutate } = useEnterpriseSWR( + const { data, error, mutate } = useEnterpriseSWR( [], formatApiPath(`api/admin/projects/${project}/change-requests/pending`), fetcher, diff --git a/frontend/src/hooks/api/getters/usePendingChangeRequestsForFeature/usePendingChangeRequestsForFeature.ts b/frontend/src/hooks/api/getters/usePendingChangeRequestsForFeature/usePendingChangeRequestsForFeature.ts index f341add4df..a666d66766 100644 --- a/frontend/src/hooks/api/getters/usePendingChangeRequestsForFeature/usePendingChangeRequestsForFeature.ts +++ b/frontend/src/hooks/api/getters/usePendingChangeRequestsForFeature/usePendingChangeRequestsForFeature.ts @@ -1,6 +1,6 @@ import { formatApiPath } from 'utils/formatPath'; import handleErrorResponses from '../httpErrorResponseHandler'; -import { IChangeRequest } from 'component/changeRequest/changeRequest.types'; +import { ChangeRequestType } from 'component/changeRequest/changeRequest.types'; import { useEnterpriseSWR } from '../useEnterpriseSWR/useEnterpriseSWR'; const fetcher = (path: string) => { @@ -13,7 +13,7 @@ export const usePendingChangeRequestsForFeature = ( project: string, featureName: string, ) => { - const { data, error, mutate } = useEnterpriseSWR( + const { data, error, mutate } = useEnterpriseSWR( [], formatApiPath( `api/admin/projects/${project}/change-requests/pending/${featureName}`, diff --git a/frontend/src/hooks/useChangeRequestInReviewWarning.tsx b/frontend/src/hooks/useChangeRequestInReviewWarning.tsx index 3f42a5299c..b2b063719a 100644 --- a/frontend/src/hooks/useChangeRequestInReviewWarning.tsx +++ b/frontend/src/hooks/useChangeRequestInReviewWarning.tsx @@ -1,9 +1,9 @@ import { Alert } from '@mui/material'; import { oneOf } from 'utils/oneOf'; -import { IChangeRequest } from '../component/changeRequest/changeRequest.types'; +import { ChangeRequestType } from '../component/changeRequest/changeRequest.types'; export const useChangeRequestInReviewWarning = ( - draft: IChangeRequest[] | undefined, + draft: ChangeRequestType[] | undefined, ) => { const changeRequestInReviewOrApproved = (environment: string) => { if (!draft) return false;