From f2bb46779f83ecfc01cc218583e324ddcb534ad7 Mon Sep 17 00:00:00 2001 From: SagarRajput-7 Date: Thu, 26 Dec 2024 10:44:55 +0530 Subject: [PATCH] feat: resolved comments and removed mount related handlings --- .../DashboardVariableSelection.tsx | 49 +++++++++----- .../VariableItem.tsx | 67 ++++--------------- .../DashboardVariablesSelection/util.ts | 13 ++-- 3 files changed, 56 insertions(+), 73 deletions(-) diff --git a/frontend/src/container/NewDashboard/DashboardVariablesSelection/DashboardVariableSelection.tsx b/frontend/src/container/NewDashboard/DashboardVariablesSelection/DashboardVariableSelection.tsx index 68b5759db3..5a3c48ad46 100644 --- a/frontend/src/container/NewDashboard/DashboardVariablesSelection/DashboardVariableSelection.tsx +++ b/frontend/src/container/NewDashboard/DashboardVariablesSelection/DashboardVariableSelection.tsx @@ -1,14 +1,18 @@ import { Row } from 'antd'; +import { isEmpty } from 'lodash-es'; import { useDashboard } from 'providers/Dashboard/Dashboard'; -import { memo, useEffect, useRef, useState } from 'react'; +import { memo, useEffect, useState } from 'react'; +import { useSelector } from 'react-redux'; +import { AppState } from 'store/reducers'; import { IDashboardVariable } from 'types/api/dashboard/getAll'; +import { GlobalReducer } from 'types/reducer/globalTime'; import { buildDependencies, buildDependencyGraph, buildParentDependencyGraph, + IDependencyData, onUpdateVariableNode, - VariableGraph, } from './util'; import VariableItem from './VariableItem'; @@ -27,11 +31,13 @@ function DashboardVariableSelection(): JSX.Element | null { const [variablesTableData, setVariablesTableData] = useState([]); - const [dependencyData, setDependencyData] = useState<{ - order: string[]; - graph: VariableGraph; - parentDependencyGraph: VariableGraph; - } | null>(null); + const [dependencyData, setDependencyData] = useState( + null, + ); + + const { maxTime, minTime } = useSelector( + (state) => state.globalTime, + ); useEffect(() => { if (variables) { @@ -55,10 +61,8 @@ function DashboardVariableSelection(): JSX.Element | null { } }, [variables]); - const initializationRef = useRef(false); - useEffect(() => { - if (variablesTableData.length > 0 && !initializationRef.current) { + if (variablesTableData.length > 0) { const depGrp = buildDependencies(variablesTableData); const { order, graph } = buildDependencyGraph(depGrp); const parentDependencyGraph = buildParentDependencyGraph(graph); @@ -67,16 +71,27 @@ function DashboardVariableSelection(): JSX.Element | null { graph, parentDependencyGraph, }); - initializationRef.current = true; } - }, [variablesTableData]); + }, [setVariablesToGetUpdated, variables, variablesTableData]); + + // this handles the case where the dependency order changes i.e. variable list updated via creation or deletion etc. and we need to refetch the variables + // also trigger when the global time changes + useEffect( + () => { + if (!isEmpty(dependencyData?.order)) { + setVariablesToGetUpdated(dependencyData?.order || []); + } + }, + // eslint-disable-next-line react-hooks/exhaustive-deps + [JSON.stringify(dependencyData?.order), minTime, maxTime], + ); const onValueUpdate = ( name: string, id: string, value: IDashboardVariable['selectedValue'], allSelected: boolean, - isMountedCall?: boolean, + // isMountedCall?: boolean, // eslint-disable-next-line sonarjs/cognitive-complexity ): void => { if (id) { @@ -116,7 +131,7 @@ function DashboardVariableSelection(): JSX.Element | null { }); } - if (dependencyData && !isMountedCall) { + if (dependencyData) { const updatedVariables: string[] = []; onUpdateVariableNode( name, @@ -124,8 +139,10 @@ function DashboardVariableSelection(): JSX.Element | null { dependencyData.order, (node) => updatedVariables.push(node), ); - setVariablesToGetUpdated(updatedVariables.filter((v) => v !== name)); - } else if (isMountedCall) { + setVariablesToGetUpdated((prev) => [ + ...new Set([...prev, ...updatedVariables.filter((v) => v !== name)]), + ]); + } else { setVariablesToGetUpdated((prev) => prev.filter((v) => v !== name)); } } diff --git a/frontend/src/container/NewDashboard/DashboardVariablesSelection/VariableItem.tsx b/frontend/src/container/NewDashboard/DashboardVariablesSelection/VariableItem.tsx index abc3cfc2f0..7e6c050653 100644 --- a/frontend/src/container/NewDashboard/DashboardVariablesSelection/VariableItem.tsx +++ b/frontend/src/container/NewDashboard/DashboardVariablesSelection/VariableItem.tsx @@ -24,7 +24,7 @@ import { commaValuesParser } from 'lib/dashbaordVariables/customCommaValuesParse import sortValues from 'lib/dashbaordVariables/sortVariableValues'; import { debounce, isArray, isString } from 'lodash-es'; import map from 'lodash-es/map'; -import { ChangeEvent, memo, useEffect, useMemo, useRef, useState } from 'react'; +import { ChangeEvent, memo, useEffect, useMemo, useState } from 'react'; import { useQuery } from 'react-query'; import { useSelector } from 'react-redux'; import { AppState } from 'store/reducers'; @@ -35,12 +35,7 @@ import { popupContainer } from 'utils/selectPopupContainer'; import { variablePropsToPayloadVariables } from '../utils'; import { SelectItemStyle } from './styles'; -import { - areArraysEqual, - checkAPIInvocation, - onUpdateVariableNode, - VariableGraph, -} from './util'; +import { areArraysEqual, checkAPIInvocation, IDependencyData } from './util'; const ALL_SELECT_VALUE = '__ALL__'; @@ -57,15 +52,10 @@ interface VariableItemProps { id: string, arg1: IDashboardVariable['selectedValue'], allSelected: boolean, - isMountedCall?: boolean, ) => void; variablesToGetUpdated: string[]; setVariablesToGetUpdated: React.Dispatch>; - dependencyData: { - order: string[]; - graph: VariableGraph; - parentDependencyGraph: VariableGraph; - } | null; + dependencyData: IDependencyData | null; } const getSelectValue = ( @@ -98,25 +88,16 @@ function VariableItem({ (state) => state.globalTime, ); - // logic to detect if its a rerender or a new render/mount - const isMounted = useRef(false); - - useEffect(() => { - isMounted.current = true; - }, []); - const validVariableUpdate = (): boolean => { if (!variableData.name) { return false; } - if (!isMounted.current) { - // variableData.name is present as the top element or next in the queue - variablesToGetUpdated - return Boolean( - variablesToGetUpdated.length && - variablesToGetUpdated[0] === variableData.name, - ); - } - return variablesToGetUpdated.includes(variableData.name); + + // variableData.name is present as the top element or next in the queue - variablesToGetUpdated + return Boolean( + variablesToGetUpdated.length && + variablesToGetUpdated[0] === variableData.name, + ); }; const [errorMessage, setErrorMessage] = useState(null); @@ -177,16 +158,7 @@ function VariableItem({ } if (variableData && variableData?.name && variableData?.id) { - onValueUpdate( - variableData.name, - variableData.id, - value, - allSelected, - isMounted.current, - ); - setVariablesToGetUpdated((prev) => - prev.filter((name) => name !== variableData.name), - ); + onValueUpdate(variableData.name, variableData.id, value, allSelected); } } @@ -216,6 +188,7 @@ function VariableItem({ variableData.name || '', `${minTime}`, `${maxTime}`, + JSON.stringify(dependencyData?.order), ], { enabled: @@ -234,21 +207,9 @@ function VariableItem({ refetchOnWindowFocus: false, onSuccess: (response) => { getOptions(response.payload); - if ( - dependencyData?.parentDependencyGraph[variableData.name || ''].length === 0 - ) { - const updatedVariables: string[] = []; - onUpdateVariableNode( - variableData.name || '', - dependencyData.graph, - dependencyData.order, - (node) => updatedVariables.push(node), - ); - setVariablesToGetUpdated((prev) => [ - ...prev, - ...updatedVariables.filter((v) => v !== variableData.name), - ]); - } + setVariablesToGetUpdated((prev) => + prev.filter((v) => v !== variableData.name), + ); }, onError: (error: { details: { diff --git a/frontend/src/container/NewDashboard/DashboardVariablesSelection/util.ts b/frontend/src/container/NewDashboard/DashboardVariablesSelection/util.ts index af44d339d6..ded7a80569 100644 --- a/frontend/src/container/NewDashboard/DashboardVariablesSelection/util.ts +++ b/frontend/src/container/NewDashboard/DashboardVariablesSelection/util.ts @@ -46,12 +46,11 @@ export type VariableGraph = Record; export const buildDependencies = ( variables: IDashboardVariable[], ): VariableGraph => { - console.log('buildDependencies', variables); const graph: VariableGraph = {}; // Initialize empty arrays for all variables first variables.forEach((variable) => { - if (variable.name) { + if (variable.name && variable.type === 'QUERY') { graph[variable.name] = []; } }); @@ -113,7 +112,7 @@ export const buildDependencyGraph = ( } if (topologicalOrder.length !== Object.keys(dependencies).length) { - throw new Error('Cycle detected in the dependency graph!'); + console.error('Cycle detected in the dependency graph!'); } return { order: topologicalOrder, graph: adjList }; @@ -170,7 +169,7 @@ export const checkAPIInvocation = ( } if (isEmpty(parentDependencyGraph)) { - return true; + return false; } // if no dependency then true @@ -186,3 +185,9 @@ export const checkAPIInvocation = ( variablesToGetUpdated[0] === variableData.name ); }; + +export interface IDependencyData { + order: string[]; + graph: VariableGraph; + parentDependencyGraph: VariableGraph; +}