feat: resolved comments and removed mount related handlings

SigNoz · Dec 26, 2024 · f2bb467 · f2bb467
1 parent 0c3282e
commit f2bb467
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 73 deletions.
diff --git a/...end/src/container/NewDashboard/DashboardVariablesSelection/DashboardVariableSelection.tsx b/...end/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<any>([]);
 
-	const [dependencyData, setDependencyData] = useState<{
-		order: string[];
-		graph: VariableGraph;
-		parentDependencyGraph: VariableGraph;
-	} | null>(null);
+	const [dependencyData, setDependencyData] = useState<IDependencyData | null>(
+		null,
+	);
+
+	const { maxTime, minTime } = useSelector<AppState, GlobalReducer>(
+		(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,16 +131,18 @@ function DashboardVariableSelection(): JSX.Element | null {
 				});
 			}
 
-			if (dependencyData && !isMountedCall) {
+			if (dependencyData) {
 				const updatedVariables: string[] = [];
 				onUpdateVariableNode(
 					name,
 					dependencyData.graph,
 					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
@@ -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<React.SetStateAction<string[]>>;
-	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 | string>(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
@@ -46,12 +46,11 @@ export type VariableGraph = Record<string, string[]>;
 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;
+}