From 5ef05891ce5b2ba05ffe7d3c61ed7c5820fc5ba9 Mon Sep 17 00:00:00 2001 From: Shaheer Kochai Date: Mon, 7 Oct 2024 09:57:46 +0430 Subject: [PATCH] fix: fix incorrect alert history state (#5898) * fix: on unmount remove the alert disabled state * fix: get updated alert state from response and fix the alert state mismatch issue --- .../ActionButtons/ActionButtons.tsx | 27 ++++++++++++++++--- .../AlertDetails/AlertHeader/AlertHeader.tsx | 14 +++------- frontend/src/pages/AlertDetails/hooks.tsx | 20 +++++++------- frontend/src/providers/Alert.tsx | 19 ++++++------- 4 files changed, 45 insertions(+), 35 deletions(-) diff --git a/frontend/src/pages/AlertDetails/AlertHeader/ActionButtons/ActionButtons.tsx b/frontend/src/pages/AlertDetails/AlertHeader/ActionButtons/ActionButtons.tsx index 186a34676b..2f37c4fc9d 100644 --- a/frontend/src/pages/AlertDetails/AlertHeader/ActionButtons/ActionButtons.tsx +++ b/frontend/src/pages/AlertDetails/AlertHeader/ActionButtons/ActionButtons.tsx @@ -15,7 +15,7 @@ import { } from 'pages/AlertDetails/hooks'; import CopyToClipboard from 'periscope/components/CopyToClipboard'; import { useAlertRule } from 'providers/Alert'; -import React from 'react'; +import React, { useEffect, useState } from 'react'; import { CSSProperties } from 'styled-components'; import { AlertDef } from 'types/api/alerts/def'; @@ -32,7 +32,7 @@ function AlertActionButtons({ ruleId: string; alertDetails: AlertHeaderProps['alertDetails']; }): JSX.Element { - const { isAlertRuleDisabled } = useAlertRule(); + const { alertRuleState, setAlertRuleState } = useAlertRule(); const { handleAlertStateToggle } = useAlertRuleStatusToggle({ ruleId }); const { handleAlertDuplicate } = useAlertRuleDuplicate({ @@ -79,13 +79,32 @@ function AlertActionButtons({ ); const isDarkMode = useIsDarkMode(); + // state for immediate UI feedback rather than waiting for onSuccess of handleAlertStateTiggle to updating the alertRuleState + const [isAlertRuleDisabled, setIsAlertRuleDisabled] = useState< + undefined | boolean + >(undefined); + + useEffect(() => { + if (alertRuleState === undefined) { + setAlertRuleState(alertDetails.state); + setIsAlertRuleDisabled(alertDetails.state === 'disabled'); + } + }, [setAlertRuleState, alertRuleState, alertDetails.state]); + + // on unmount remove the alert state + // eslint-disable-next-line react-hooks/exhaustive-deps + useEffect(() => (): void => setAlertRuleState(undefined), []); + return (
- + {isAlertRuleDisabled !== undefined && ( { + setIsAlertRuleDisabled((prev) => !prev); + handleAlertStateToggle(); + }} checked={!isAlertRuleDisabled} /> )} diff --git a/frontend/src/pages/AlertDetails/AlertHeader/AlertHeader.tsx b/frontend/src/pages/AlertDetails/AlertHeader/AlertHeader.tsx index 073b84382b..04edd6a8b0 100644 --- a/frontend/src/pages/AlertDetails/AlertHeader/AlertHeader.tsx +++ b/frontend/src/pages/AlertDetails/AlertHeader/AlertHeader.tsx @@ -2,7 +2,7 @@ import './AlertHeader.styles.scss'; import LineClampedText from 'periscope/components/LineClampedText/LineClampedText'; import { useAlertRule } from 'providers/Alert'; -import { useEffect, useMemo } from 'react'; +import { useMemo } from 'react'; import AlertActionButtons from './ActionButtons/ActionButtons'; import AlertLabels from './AlertLabels/AlertLabels'; @@ -19,7 +19,7 @@ export type AlertHeaderProps = { }; }; function AlertHeader({ alertDetails }: AlertHeaderProps): JSX.Element { - const { state, alert, labels, disabled } = alertDetails; + const { state, alert, labels } = alertDetails; const labelsWithoutSeverity = useMemo( () => @@ -29,20 +29,14 @@ function AlertHeader({ alertDetails }: AlertHeaderProps): JSX.Element { [labels], ); - const { isAlertRuleDisabled, setIsAlertRuleDisabled } = useAlertRule(); - - useEffect(() => { - if (isAlertRuleDisabled === undefined) { - setIsAlertRuleDisabled(disabled); - } - }, [disabled, setIsAlertRuleDisabled, isAlertRuleDisabled]); + const { alertRuleState } = useAlertRule(); return (
- +
diff --git a/frontend/src/pages/AlertDetails/hooks.tsx b/frontend/src/pages/AlertDetails/hooks.tsx index c9257ad47b..da02f10b40 100644 --- a/frontend/src/pages/AlertDetails/hooks.tsx +++ b/frontend/src/pages/AlertDetails/hooks.tsx @@ -161,7 +161,6 @@ export const useGetAlertRuleDetails = (): Props => { id: parseInt(ruleId || '', 10), }), enabled: isValidRuleId, - refetchOnMount: false, refetchOnWindowFocus: false, }); @@ -369,9 +368,9 @@ export const useAlertRuleStatusToggle = ({ }: { ruleId: string; }): { - handleAlertStateToggle: (state: boolean) => void; + handleAlertStateToggle: () => void; } => { - const { isAlertRuleDisabled, setIsAlertRuleDisabled } = useAlertRule(); + const { alertRuleState, setAlertRuleState } = useAlertRule(); const { notifications } = useNotifications(); const queryClient = useQueryClient(); @@ -381,16 +380,17 @@ export const useAlertRuleStatusToggle = ({ [REACT_QUERY_KEY.TOGGLE_ALERT_STATE, ruleId], patchAlert, { - onMutate: () => { - setIsAlertRuleDisabled((prev) => !prev); - }, - onSuccess: () => { + onSuccess: (data) => { + setAlertRuleState(data?.payload?.state); + notifications.success({ - message: `Alert has been ${isAlertRuleDisabled ? 'enabled' : 'disabled'}.`, + message: `Alert has been ${ + data?.payload?.state === 'disabled' ? 'disabled' : 'enabled' + }.`, }); }, onError: (error) => { - queryClient.refetchQueries([REACT_QUERY_KEY.ALERT_RULE_DETAILS]); + queryClient.refetchQueries([REACT_QUERY_KEY.ALERT_RULE_DETAILS, ruleId]); handleError(error); }, }, @@ -399,7 +399,7 @@ export const useAlertRuleStatusToggle = ({ const handleAlertStateToggle = (): void => { const args = { id: parseInt(ruleId, 10), - data: { disabled: !isAlertRuleDisabled }, + data: { disabled: alertRuleState !== 'disabled' }, }; toggleAlertState(args); }; diff --git a/frontend/src/providers/Alert.tsx b/frontend/src/providers/Alert.tsx index 337eec9ba5..328c8e8aee 100644 --- a/frontend/src/providers/Alert.tsx +++ b/frontend/src/providers/Alert.tsx @@ -1,10 +1,8 @@ import React, { createContext, useContext, useState } from 'react'; interface AlertRuleContextType { - isAlertRuleDisabled: boolean | undefined; - setIsAlertRuleDisabled: React.Dispatch< - React.SetStateAction - >; + alertRuleState: string | undefined; + setAlertRuleState: React.Dispatch>; } const AlertRuleContext = createContext( @@ -16,15 +14,14 @@ function AlertRuleProvider({ }: { children: React.ReactNode; }): JSX.Element { - const [isAlertRuleDisabled, setIsAlertRuleDisabled] = useState< - boolean | undefined - >(undefined); - - const value = React.useMemo( - () => ({ isAlertRuleDisabled, setIsAlertRuleDisabled }), - [isAlertRuleDisabled], + const [alertRuleState, setAlertRuleState] = useState( + undefined, ); + const value = React.useMemo(() => ({ alertRuleState, setAlertRuleState }), [ + alertRuleState, + ]); + return ( {children}