feat: display change details (#2327)

* feat: display change details

* refactor: reorganize components

* feat: display deleted strategy name if present

* feat: UI tweaks

* fix: types

* refactor: remove unnecessary checks for types

frontend/src/component/changeRequest/ChangeRequest/ChangeRequest.tsx

@@ -2,15 +2,11 @@ import { VFC } from 'react';
 import { Box } from '@mui/material';
 import { ChangeRequestFeatureToggleChange } from '../ChangeRequestOverview/ChangeRequestFeatureToggleChange/ChangeRequestFeatureToggleChange';
 import { objectId } from 'utils/objectId';
-import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
 import { ToggleStatusChange } from '../ChangeRequestOverview/ChangeRequestFeatureToggleChange/ToggleStatusChange';
 import { useChangeRequestApi } from 'hooks/api/actions/useChangeRequestApi/useChangeRequestApi';
 import { formatUnknownError } from 'utils/formatUnknownError';
 import useToast from 'hooks/useToast';
-import type {
+import type { IChangeRequest } from '../changeRequest.types';
-    IChangeRequest,
-    IChangeRequestAddStrategy,
-} from '../changeRequest.types';
 import {
     StrategyAddedChange,
     StrategyDeletedChange,
@@ -20,7 +16,7 @@ import {
     formatStrategyName,
     GetFeatureStrategyIcon,
 } from 'utils/strategyNames';
-import { IChangeRequestEnabled } from '../changeRequest.types';
+import { hasNameField } from '../changeRequest.types';
 
 interface IChangeRequestProps {
     changeRequest: IChangeRequest;
@@ -62,46 +58,60 @@ export const ChangeRequest: VFC<IChangeRequestProps> = ({
                     onNavigate={onNavigate}
                 >
                     {featureToggleChange.changes.map(change => (
-                        <Box key={objectId(change)}>
+                        <Box
-                            <ConditionallyRender
+                            key={objectId(change)}
-                                condition={change.action === 'updateEnabled'}
+                            sx={theme => ({
-                                show={
+                                padding: 2,
+                                borderTop: '1px solid',
+                                borderColor: theme =>
+                                    theme.palette.dividerAlternative,
+                            })}
+                        >
+                            {change.action === 'updateEnabled' && (
                                 <ToggleStatusChange
-                                        enabled={
+                                    enabled={change.payload.enabled}
-                                            (change as IChangeRequestEnabled)
+                                    onDiscard={onDiscard(change.id)}
-                                                ?.payload?.enabled
-                                        }
-                                        onDiscard={onDiscard(change.id!)}
                                 />
-                                }
+                            )}
+                            {change.action === 'addStrategy' && (
+                                <StrategyAddedChange
+                                    onDiscard={onDiscard(change.id)}
+                                >
+                                    <GetFeatureStrategyIcon
+                                        strategyName={change.payload.name}
                                     />
-                            <ConditionallyRender
+
-                                condition={change.action === 'addStrategy'}
+                                    {formatStrategyName(change.payload.name)}
-                                show={
+                                </StrategyAddedChange>
-                                    <StrategyAddedChange>
+                            )}
+                            {change.action === 'deleteStrategy' && (
+                                <StrategyDeletedChange
+                                    onDiscard={onDiscard(change.id)}
+                                >
+                                    {hasNameField(change.payload) && (
+                                        <>
                                             <GetFeatureStrategyIcon
                                                 strategyName={
-                                                (
+                                                    change.payload.name
-                                                    change as IChangeRequestAddStrategy
-                                                )?.payload.name!
                                                 }
                                             />
                                             {formatStrategyName(
-                                            (
+                                                change.payload.name
-                                                change as IChangeRequestAddStrategy
-                                            )?.payload.name!
                                             )}
-                                    </StrategyAddedChange>
+                                        </>
-                                }
+                                    )}
-                            />
+                                </StrategyDeletedChange>
-                            <ConditionallyRender
+                            )}
-                                condition={change.action === 'deleteStrategy'}
+                            {change.action === 'updateStrategy' && (
-                                show={<StrategyDeletedChange />}
+                                <StrategyEditedChange
-                            />
+                                    onDiscard={onDiscard(change.id)}
-                            <ConditionallyRender
+                                >
-                                condition={change.action === 'updateStrategy'}
+                                    <GetFeatureStrategyIcon
-                                show={<StrategyEditedChange />}
+                                        strategyName={change.payload.name}
                                     />
+                                    {formatStrategyName(change.payload.name)}
+                                </StrategyEditedChange>
+                            )}
                         </Box>
                     ))}
                 </ChangeRequestFeatureToggleChange>

frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestFeatureToggleChange/ChangeRequestFeatureToggleChange.tsx

@@ -26,7 +26,7 @@ export const ChangeRequestFeatureToggleChange: FC<
             <Box
                 sx={theme => ({
                     backgroundColor: theme.palette.tableHeaderBackground,
-                    p: 2,
+                    padding: theme.spacing(3),
                 })}
             >
                 <Box sx={{ display: 'flex', gap: 1 }}>
@@ -42,7 +42,7 @@ export const ChangeRequestFeatureToggleChange: FC<
                     </Typography>
                 </Box>
             </Box>
-            <Box sx={{ p: 2 }}>{children}</Box>
+            <Box>{children}</Box>
         </Card>
     );
 };

frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestFeatureToggleChange/StrategyChange.tsx

@@ -1,27 +1,74 @@
-import { Box, Typography } from '@mui/material';
+import { Box, Link, styled, Typography } from '@mui/material';
 import { FC } from 'react';
 
-export const StrategyAddedChange: FC = ({ children }) => {
+interface IStrategyChangeProps {
+    onDiscard: () => void;
+}
+
+export const ChangeItemWrapper = styled(Box)(({ theme }) => ({
+    display: 'flex',
+    justifyContent: 'space-between',
+    padding: theme.spacing(1),
+}));
+
+const ChangeItemInfo: FC = styled(Box)(({ theme }) => ({
+    display: 'flex',
+    gap: theme.spacing(1),
+}));
+
+const Discard: FC<IStrategyChangeProps> = ({ onDiscard }) => (
+    <Box>
+        <Link onClick={onDiscard}>Discard</Link>
+    </Box>
+);
+
+export const StrategyAddedChange: FC<IStrategyChangeProps> = ({
+    children,
+    onDiscard,
+}) => {
     return (
-        <Box sx={{ p: 1, display: 'flex', gap: 1 }}>
+        <ChangeItemWrapper>
-            <Typography sx={theme => ({ color: theme.palette.success.main })}>
+            <ChangeItemInfo>
-                + Strategy Added:
+                <Typography
+                    sx={theme => ({ color: theme.palette.success.main })}
+                >
+                    + Adding strategy:
                 </Typography>
                 {children}
-        </Box>
+            </ChangeItemInfo>
+            <Discard onDiscard={onDiscard} />
+        </ChangeItemWrapper>
     );
 };
 
-export const StrategyEditedChange: FC = () => {
+export const StrategyEditedChange: FC<IStrategyChangeProps> = ({
-    return <Box sx={{ p: 1 }}>Strategy Edited</Box>;
+    children,
-};
+    onDiscard,
-
+}) => {
-export const StrategyDeletedChange: FC = () => {
     return (
-        <Box sx={{ p: 1 }}>
+        <ChangeItemWrapper>
-            <Typography sx={theme => ({ color: theme.palette.error.main })}>
+            <ChangeItemInfo>
-                - Strategy Deleted
+                <Typography>Editing strategy:</Typography>
-            </Typography>
+                {children}
-        </Box>
+            </ChangeItemInfo>
+            <Discard onDiscard={onDiscard} />
+        </ChangeItemWrapper>
+    );
+};
+
+export const StrategyDeletedChange: FC<IStrategyChangeProps> = ({
+    onDiscard,
+    children,
+}) => {
+    return (
+        <ChangeItemWrapper>
+            <ChangeItemInfo>
+                <Typography sx={theme => ({ color: theme.palette.error.main })}>
+                    - Deleting strategy
+                </Typography>
+                {children}
+            </ChangeItemInfo>
+            <Discard onDiscard={onDiscard} />
+        </ChangeItemWrapper>
     );
 };

frontend/src/component/changeRequest/ChangeRequestOverview/ChangeRequestFeatureToggleChange/ToggleStatusChange.tsx

@@ -2,6 +2,7 @@ import { VFC } from 'react';
 import { Link, Box } from '@mui/material';
 import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
 import { Badge } from 'component/common/Badge/Badge';
+import { ChangeItemWrapper } from './StrategyChange';
 
 interface IPlaygroundResultsTable {
     enabled: boolean;
@@ -13,7 +14,7 @@ export const ToggleStatusChange: VFC<IPlaygroundResultsTable> = ({
     onDiscard,
 }) => {
     return (
-        <Box sx={{ p: 1, display: 'flex', justifyContent: 'space-between' }}>
+        <ChangeItemWrapper>
             <Box>
                 New status:{' '}
                 <Badge color={enabled ? 'success' : 'error'}>
@@ -28,6 +29,6 @@ export const ToggleStatusChange: VFC<IPlaygroundResultsTable> = ({
                     </Box>
                 }
             />
-        </Box>
+        </ChangeItemWrapper>
     );
 };

frontend/src/component/changeRequest/ChangeRequestSidebar/ChangeRequestSidebar.tsx

@@ -6,6 +6,7 @@ import {
     styled,
     Tooltip,
     Divider,
+    IconButton,
 } from '@mui/material';
 import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
 import { SidebarModal } from 'component/common/SidebarModal/SidebarModal';
@@ -17,6 +18,7 @@ import { ChangeRequest } from '../ChangeRequest/ChangeRequest';
 import { useChangeRequestOpen } from 'hooks/api/getters/useChangeRequestOpen/useChangeRequestOpen';
 import { useChangeRequestApi } from 'hooks/api/actions/useChangeRequestApi/useChangeRequestApi';
 import { ChangeRequestStatusBadge } from '../ChangeRequestStatusBadge/ChangeRequestStatusBadge';
+import CloseIcon from '@mui/icons-material/Close';
 
 interface IChangeRequestSidebarProps {
     open: boolean;
@@ -112,8 +114,19 @@ export const ChangeRequestSidebar: VFC<IChangeRequestSidebarProps> = ({
                 header={
                     <PageHeader
                         secondary
+                        actions={
+                            <IconButton onClick={onClose}>
+                                <CloseIcon />
+                            </IconButton>
+                        }
                         titleElement={
                             <>
+                                <Box
+                                    sx={{
+                                        display: 'flex',
+                                        alignItems: 'center',
+                                    }}
+                                >
                                     Review your changes
                                     <Tooltip
                                         title="You can review your changes from this page.
@@ -122,6 +135,7 @@ export const ChangeRequestSidebar: VFC<IChangeRequestSidebarProps> = ({
                                     >
                                         <StyledHelpOutline />
                                     </Tooltip>
+                                </Box>
                                 <StyledHeaderHint>
                                     Make sure you are sending the right changes
                                     to be reviewed
@@ -228,7 +242,6 @@ export const ChangeRequestSidebar: VFC<IChangeRequestSidebarProps> = ({
                         </Box>
                     </Box>
                 ))}
-                <BackButton onClick={onClose}>Close</BackButton>
             </StyledPageContent>
         </SidebarModal>
     );

frontend/src/component/changeRequest/changeRequest.types.ts

@@ -18,7 +18,7 @@ export interface IChangeRequestFeature {
 }
 
 export interface IChangeRequestBase {
-    id?: number;
+    id: number;
     action: ChangeRequestAction;
     payload: ChangeRequestPayload;
     conflict?: string;
@@ -83,3 +83,6 @@ export type ChangeRequestAction =
     | 'addStrategy'
     | 'updateStrategy'
     | 'deleteStrategy';
+
+export const hasNameField = (payload: unknown): payload is { name: string } =>
+    typeof payload === 'object' && payload !== null && 'name' in payload;

frontend/src/component/feature/FeatureStrategy/FeatureStrategyRemove/FeatureStrategyRemove.tsx

@@ -15,6 +15,7 @@ import PermissionIconButton from 'component/common/PermissionIconButton/Permissi
 import { Delete } from '@mui/icons-material';
 import { useChangeRequestApi } from 'hooks/api/actions/useChangeRequestApi/useChangeRequestApi';
 import { useChangeRequestsEnabled } from 'hooks/useChangeRequestsEnabled';
+import { useChangeRequestOpen } from 'hooks/api/getters/useChangeRequestOpen/useChangeRequestOpen';
 
 interface IFeatureStrategyRemoveProps {
     projectId: string;
@@ -130,6 +131,7 @@ const useOnSuggestRemove = ({
     strategyId,
 }: IRemoveProps) => {
     const { addChangeRequest } = useChangeRequestApi();
+    const { refetch: refetchChangeRequests } = useChangeRequestOpen(projectId);
     const { setToastData, setToastApiError } = useToast();
     const onSuggestRemove = async (event: React.FormEvent) => {
         try {
@@ -145,6 +147,7 @@ const useOnSuggestRemove = ({
                 title: 'Changes added to the draft!',
                 type: 'success',
             });
+            await refetchChangeRequests();
         } catch (error: unknown) {
             setToastApiError(formatUnknownError(error));
         }