feat: added error preview and warning text with info on cyclic dependency detected (#7893)

* feat: added error preview and warning text with info on cyclic dependency detected

* feat: removed console.log

* feat: restricted save when cycle found

* feat: added test cases for variableitem flow and update test cases

* feat: updated test cases

* feat: corrected the recent resolved title text

frontend/src/container/NewDashboard/DashboardSettings/Variables/VariableItem/VariableItem.test.tsx

@@ -0,0 +1,474 @@
+import { QueryClient, QueryClientProvider } from 'react-query';
+import { fireEvent, render, screen, waitFor } from 'tests/test-utils';
+import {
+	IDashboardVariable,
+	TSortVariableValuesType,
+	VariableSortTypeArr,
+} from 'types/api/dashboard/getAll';
+
+import VariableItem from './VariableItem';
+
+// Mock modules
+jest.mock('api/dashboard/variables/dashboardVariablesQuery', () => ({
+	__esModule: true,
+	default: jest.fn().mockResolvedValue({
+		payload: {
+			variableValues: ['value1', 'value2', 'value3'],
+		},
+	}),
+}));
+
+jest.mock('uuid', () => ({
+	v4: jest.fn().mockReturnValue('test-uuid'),
+}));
+
+// Mock functions
+const onCancel = jest.fn();
+const onSave = jest.fn();
+const validateName = jest.fn(() => true);
+
+// Mode constant
+const VARIABLE_MODE = 'ADD';
+
+// Common text constants
+const TEXT = {
+	INCLUDE_ALL_VALUES: 'Include an option for ALL values',
+	ENABLE_MULTI_VALUES: 'Enable multiple values to be checked',
+	VARIABLE_EXISTS: 'Variable name already exists',
+	SORT_VALUES: 'Sort Values',
+	DEFAULT_VALUE: 'Default Value',
+	ALL_VARIABLES: 'All variables',
+	DISCARD: 'Discard',
+	OPTIONS: 'Options',
+	QUERY: 'Query',
+	TEXTBOX: 'Textbox',
+	CUSTOM: 'Custom',
+};
+
+// Common test constants
+const VARIABLE_DEFAULTS = {
+	sort: VariableSortTypeArr[0] as TSortVariableValuesType,
+	multiSelect: false,
+	showALLOption: false,
+};
+
+// Common variable properties
+const TEST_VAR_NAMES = {
+	VAR1: 'variable1',
+	VAR2: 'variable2',
+	VAR3: 'variable3',
+};
+
+const TEST_VAR_IDS = {
+	VAR1: 'var1',
+	VAR2: 'var2',
+	VAR3: 'var3',
+};
+
+const TEST_VAR_DESCRIPTIONS = {
+	VAR1: 'Variable 1',
+	VAR2: 'Variable 2',
+	VAR3: 'Variable 3',
+};
+
+// Common UI elements
+const SAVE_BUTTON_TEXT = 'Save Variable';
+const UNIQUE_NAME_PLACEHOLDER = 'Unique name of the variable';
+
+// Create QueryClient for wrapping the component
+const createTestQueryClient = (): QueryClient =>
+	new QueryClient({
+		defaultOptions: {
+			queries: {
+				retry: false,
+			},
+		},
+	});
+
+// Wrapper component with QueryClientProvider
+const wrapper = ({ children }: { children: React.ReactNode }): JSX.Element => (
+	<QueryClientProvider client={createTestQueryClient()}>
+		{children}
+	</QueryClientProvider>
+);
+
+// Basic variable data for testing
+const basicVariableData: IDashboardVariable = {
+	id: TEST_VAR_IDS.VAR1,
+	name: TEST_VAR_NAMES.VAR1,
+	description: 'Test Variable 1',
+	type: 'QUERY',
+	queryValue: 'SELECT * FROM test',
+	...VARIABLE_DEFAULTS,
+	order: 0,
+};
+
+// Helper function to render VariableItem with common props
+const renderVariableItem = (
+	variableData: IDashboardVariable = basicVariableData,
+	existingVariables: Record<string, IDashboardVariable> = {},
+	validateNameFn = validateName,
+): void => {
+	render(
+		<VariableItem
+			variableData={variableData}
+			existingVariables={existingVariables}
+			onCancel={onCancel}
+			onSave={onSave}
+			validateName={validateNameFn}
+			mode={VARIABLE_MODE}
+		/>,
+		{ wrapper } as any,
+	);
+};
+
+// Helper function to find button by text within its span
+const findButtonByText = (text: string): HTMLElement | null => {
+	const buttons = screen.getAllByRole('button');
+	return buttons.find((button) => button.textContent?.includes(text)) || null;
+};
+
+describe('VariableItem Component', () => {
+	// Test SQL query patterns
+	const SQL_PATTERN_DOT = 'SELECT * FROM test WHERE env = {{.variable2}}';
+	const SQL_PATTERN_DOLLAR = 'SELECT * FROM test WHERE env = $variable2';
+	const SQL_PATTERN_BRACKET = 'SELECT * FROM test WHERE service = [[variable3]]';
+	const SQL_PATTERN_BRACES = 'SELECT * FROM test WHERE app = {{variable1}}';
+	const SQL_PATTERN_NO_VARS = 'SELECT * FROM test WHERE env = "prod"';
+	const SQL_PATTERN_DOT_VAR1 =
+		'SELECT * FROM test WHERE service = {{.variable1}}';
+
+	// Error message text constant
+	const CIRCULAR_DEPENDENCY_ERROR = /Cannot save: Circular dependency detected/;
+
+	// Test functions and utilities
+	const createVariable = (
+		id: string,
+		name: string,
+		description: string,
+		queryValue: string,
+		order: number,
+	): IDashboardVariable => ({
+		id,
+		name,
+		description,
+		type: 'QUERY',
+		queryValue,
+		...VARIABLE_DEFAULTS,
+		order,
+	});
+
+	beforeEach(() => {
+		jest.clearAllMocks();
+	});
+
+	test('renders without crashing', () => {
+		renderVariableItem();
+
+		expect(screen.getByText(TEXT.ALL_VARIABLES)).toBeInTheDocument();
+		expect(screen.getByText('Name')).toBeInTheDocument();
+		expect(screen.getByText('Description')).toBeInTheDocument();
+		expect(screen.getByText('Variable Type')).toBeInTheDocument();
+	});
+
+	describe('Variable Name Validation', () => {
+		test('shows error when variable name already exists', () => {
+			// Set validateName to return false (name exists)
+			const mockValidateName = jest.fn().mockReturnValue(false);
+
+			renderVariableItem({ ...basicVariableData, name: '' }, {}, mockValidateName);
+
+			// Enter a name that already exists
+			const nameInput = screen.getByPlaceholderText(UNIQUE_NAME_PLACEHOLDER);
+			fireEvent.change(nameInput, { target: { value: 'existingVariable' } });
+
+			// Error message should be displayed
+			expect(screen.getByText(TEXT.VARIABLE_EXISTS)).toBeInTheDocument();
+
+			// We won't check for button disabled state as it might be inconsistent in tests
+		});
+
+		test('allows save when current variable name is used', () => {
+			// Mock validate to return false for all other names but true for own name
+			const mockValidateName = jest
+				.fn()
+				.mockImplementation((name) => name === TEST_VAR_NAMES.VAR1);
+
+			renderVariableItem(basicVariableData, {}, mockValidateName);
+
+			// Enter the current variable name
+			const nameInput = screen.getByPlaceholderText(UNIQUE_NAME_PLACEHOLDER);
+			fireEvent.change(nameInput, { target: { value: TEST_VAR_NAMES.VAR1 } });
+
+			// Error should not be visible
+			expect(screen.queryByText(TEXT.VARIABLE_EXISTS)).not.toBeInTheDocument();
+		});
+	});
+
+	describe('Variable Type Switching', () => {
+		test('switches to CUSTOM variable type correctly', () => {
+			renderVariableItem();
+
+			// Find the Query button
+			const queryButton = findButtonByText(TEXT.QUERY);
+			expect(queryButton).toBeInTheDocument();
+			expect(queryButton).toHaveClass('selected');
+
+			// Find and click Custom button
+			const customButton = findButtonByText(TEXT.CUSTOM);
+			expect(customButton).toBeInTheDocument();
+
+			if (customButton) {
+				fireEvent.click(customButton);
+			}
+
+			// Custom button should now be selected
+			expect(customButton).toHaveClass('selected');
+			expect(queryButton).not.toHaveClass('selected');
+
+			// Custom options input should appear
+			expect(screen.getByText(TEXT.OPTIONS)).toBeInTheDocument();
+		});
+
+		test('switches to TEXTBOX variable type correctly', () => {
+			renderVariableItem();
+
+			// Find and click Textbox button
+			const textboxButton = findButtonByText(TEXT.TEXTBOX);
+			expect(textboxButton).toBeInTheDocument();
+
+			if (textboxButton) {
+				fireEvent.click(textboxButton);
+			}
+
+			// Textbox button should now be selected
+			expect(textboxButton).toHaveClass('selected');
+
+			// Default Value input should appear
+			expect(screen.getByText(TEXT.DEFAULT_VALUE)).toBeInTheDocument();
+			expect(
+				screen.getByPlaceholderText('Enter a default value (if any)...'),
+			).toBeInTheDocument();
+		});
+	});
+
+	describe('MultiSelect and ALL Option', () => {
+		test('enables ALL option only when multiSelect is enabled', async () => {
+			renderVariableItem();
+
+			// Initially, ALL option should not be visible
+			expect(screen.queryByText(TEXT.INCLUDE_ALL_VALUES)).not.toBeInTheDocument();
+
+			// Enable multiple values
+			const multipleValuesSwitch = screen
+				.getByText(TEXT.ENABLE_MULTI_VALUES)
+				.closest('.multiple-values-section')
+				?.querySelector('button');
+
+			expect(multipleValuesSwitch).toBeInTheDocument();
+			if (multipleValuesSwitch) {
+				fireEvent.click(multipleValuesSwitch);
+			}
+
+			// Now ALL option should be visible
+			await waitFor(() => {
+				expect(screen.getByText(TEXT.INCLUDE_ALL_VALUES)).toBeInTheDocument();
+			});
+
+			// Disable multiple values
+			if (multipleValuesSwitch) {
+				fireEvent.click(multipleValuesSwitch);
+			}
+
+			// ALL option should be hidden again
+			await waitFor(() => {
+				expect(screen.queryByText(TEXT.INCLUDE_ALL_VALUES)).not.toBeInTheDocument();
+			});
+		});
+
+		test('disables ALL option when multiSelect is disabled', async () => {
+			// Create variable with multiSelect and showALLOption both enabled
+			const variable: IDashboardVariable = {
+				...basicVariableData,
+				multiSelect: true,
+				showALLOption: true,
+			};
+
+			renderVariableItem(variable);
+
+			// ALL option should be visible initially
+			expect(screen.getByText(TEXT.INCLUDE_ALL_VALUES)).toBeInTheDocument();
+
+			// Disable multiple values
+			const multipleValuesSwitch = screen
+				.getByText(TEXT.ENABLE_MULTI_VALUES)
+				.closest('.multiple-values-section')
+				?.querySelector('button');
+
+			if (multipleValuesSwitch) {
+				fireEvent.click(multipleValuesSwitch);
+			}
+
+			// ALL option should be hidden
+			await waitFor(() => {
+				expect(screen.queryByText(TEXT.INCLUDE_ALL_VALUES)).not.toBeInTheDocument();
+			});
+
+			// Check that when saving, showALLOption is set to false
+			const saveButton = screen.getByText(SAVE_BUTTON_TEXT);
+			fireEvent.click(saveButton);
+
+			expect(onSave).toHaveBeenCalledWith(
+				expect.anything(),
+				expect.objectContaining({
+					multiSelect: false,
+					showALLOption: false,
+				}),
+			);
+		});
+	});
+
+	describe('Cancel and Navigation', () => {
+		test('calls onCancel when clicking All Variables button', () => {
+			renderVariableItem();
+
+			// Click All variables button
+			const allVariablesButton = screen.getByText(TEXT.ALL_VARIABLES);
+			fireEvent.click(allVariablesButton);
+
+			expect(onCancel).toHaveBeenCalledTimes(1);
+		});
+
+		test('calls onCancel when clicking Discard button', () => {
+			renderVariableItem();
+
+			// Click Discard button
+			const discardButton = screen.getByText(TEXT.DISCARD);
+			fireEvent.click(discardButton);
+
+			expect(onCancel).toHaveBeenCalledTimes(1);
+		});
+	});
+
+	describe('Cyclic Dependency Detection', () => {
+		// Common function to render the component with variables and click save
+		const renderAndSave = async (
+			variableData: IDashboardVariable,
+			existingVariables: Record<string, IDashboardVariable>,
+		): Promise<void> => {
+			renderVariableItem(variableData, existingVariables);
+
+			// Fill in the variable name if it's not already populated
+			const nameInput = screen.getByPlaceholderText(UNIQUE_NAME_PLACEHOLDER);
+			if (nameInput.getAttribute('value') === '') {
+				fireEvent.change(nameInput, { target: { value: variableData.name || '' } });
+			}
+
+			// Click save button to trigger the dependency check
+			const saveButton = screen.getByText(SAVE_BUTTON_TEXT);
+			fireEvent.click(saveButton);
+		};
+
+		// Common expectations for finding circular dependency error
+		const expectCircularDependencyError = async (): Promise<void> => {
+			await waitFor(() => {
+				expect(screen.getByText(CIRCULAR_DEPENDENCY_ERROR)).toBeInTheDocument();
+				expect(onSave).not.toHaveBeenCalled();
+			});
+		};
+
+		// Test for cyclic dependency detection
+		test('detects circular dependency and shows error message', async () => {
+			// Create variables with circular dependency
+			const variable1 = createVariable(
+				TEST_VAR_IDS.VAR1,
+				TEST_VAR_NAMES.VAR1,
+				TEST_VAR_DESCRIPTIONS.VAR1,
+				SQL_PATTERN_DOT,
+				0,
+			);
+
+			const variable2 = createVariable(
+				TEST_VAR_IDS.VAR2,
+				TEST_VAR_NAMES.VAR2,
+				TEST_VAR_DESCRIPTIONS.VAR2,
+				SQL_PATTERN_DOT_VAR1,
+				1,
+			);
+
+			const existingVariables = {
+				[TEST_VAR_IDS.VAR2]: variable2,
+			};
+
+			await renderAndSave(variable1, existingVariables);
+			await expectCircularDependencyError();
+		});
+
+		// Test for saving with no circular dependency
+		test('allows saving when no circular dependency exists', async () => {
+			// Create variables without circular dependency
+			const variable1 = createVariable(
+				TEST_VAR_IDS.VAR1,
+				TEST_VAR_NAMES.VAR1,
+				TEST_VAR_DESCRIPTIONS.VAR1,
+				SQL_PATTERN_NO_VARS,
+				0,
+			);
+
+			const variable2 = createVariable(
+				TEST_VAR_IDS.VAR2,
+				TEST_VAR_NAMES.VAR2,
+				TEST_VAR_DESCRIPTIONS.VAR2,
+				SQL_PATTERN_DOT_VAR1,
+				1,
+			);
+
+			const existingVariables = {
+				[TEST_VAR_IDS.VAR2]: variable2,
+			};
+
+			await renderAndSave(variable1, existingVariables);
+
+			// Verify the onSave function was called
+			await waitFor(() => {
+				expect(onSave).toHaveBeenCalled();
+			});
+		});
+
+		// Test with multiple variable formats in query
+		test('detects circular dependency with different variable formats', async () => {
+			// Create variables with circular dependency using different formats
+			const variable1 = createVariable(
+				TEST_VAR_IDS.VAR1,
+				TEST_VAR_NAMES.VAR1,
+				TEST_VAR_DESCRIPTIONS.VAR1,
+				SQL_PATTERN_DOLLAR,
+				0,
+			);
+
+			const variable2 = createVariable(
+				TEST_VAR_IDS.VAR2,
+				TEST_VAR_NAMES.VAR2,
+				TEST_VAR_DESCRIPTIONS.VAR2,
+				SQL_PATTERN_BRACKET,
+				1,
+			);
+
+			const variable3 = createVariable(
+				TEST_VAR_IDS.VAR3,
+				TEST_VAR_NAMES.VAR3,
+				TEST_VAR_DESCRIPTIONS.VAR3,
+				SQL_PATTERN_BRACES,
+				2,
+			);
+
+			const existingVariables = {
+				[TEST_VAR_IDS.VAR2]: variable2,
+				[TEST_VAR_IDS.VAR3]: variable3,
+			};
+
+			await renderAndSave(variable1, existingVariables);
+			await expectCircularDependencyError();
+		});
+	});
+});

frontend/src/container/NewDashboard/DashboardSettings/Variables/VariableItem/VariableItem.tsx

@@ -28,6 +28,10 @@ import {
 } from 'types/api/dashboard/getAll';
 import { v4 as generateUUID } from 'uuid';
 
+import {
+	buildDependencies,
+	buildDependencyGraph,
+} from '../../../DashboardVariablesSelection/util';
 import { variablePropsToPayloadVariables } from '../../../utils';
 import { TVariableMode } from '../types';
 import { LabelContainer, VariableItemRow } from './styles';
@@ -107,7 +111,8 @@ function VariableItem({
 	]);
 
 	const handleSave = (): void => {
-		const variable: IDashboardVariable = {
+		// Check for cyclic dependencies
+		const newVariable = {
 			name: variableName,
 			description: variableDescription,
 			type: queryType,
@@ -126,7 +131,21 @@ function VariableItem({
 			order: variableData.order,
 		};
 
-		onSave(mode, variable);
+		const allVariables = [...Object.values(existingVariables), newVariable];
+
+		const dependencies = buildDependencies(allVariables);
+		const { hasCycle, cycleNodes } = buildDependencyGraph(dependencies);
+
+		if (hasCycle) {
+			setErrorPreview(
+				`Cannot save: Circular dependency detected between variables: ${cycleNodes?.join(
+					' → ',
+				)}`,
+			);
+			return;
+		}
+
+		onSave(mode, newVariable);
 	};
 
 	// Fetches the preview values for the SQL variable query

frontend/src/container/NewDashboard/DashboardVariablesSelection/DashboardVariableSelection.styles.scss

@@ -106,3 +106,9 @@
 		}
 	}
 }
+
+.cycle-error-alert {
+	margin-bottom: 12px;
+	padding: 4px 12px;
+	font-size: 12px;
+}

frontend/src/container/NewDashboard/DashboardVariablesSelection/DashboardVariableSelection.tsx

@@ -1,4 +1,6 @@
-import { Row } from 'antd';
+import './DashboardVariableSelection.styles.scss';
+
+import { Alert, Row } from 'antd';
 import { isEmpty } from 'lodash-es';
 import { useDashboard } from 'providers/Dashboard/Dashboard';
 import { memo, useEffect, useState } from 'react';
@@ -64,7 +66,7 @@ function DashboardVariableSelection(): JSX.Element | null {
 	useEffect(() => {
 		if (variablesTableData.length > 0) {
 			const depGrp = buildDependencies(variablesTableData);
-			const { order, graph } = buildDependencyGraph(depGrp);
+			const { order, graph, hasCycle, cycleNodes } = buildDependencyGraph(depGrp);
 			const parentDependencyGraph = buildParentDependencyGraph(graph);
 
 			// cleanup order to only include variables that are of type 'QUERY'
@@ -79,6 +81,8 @@ function DashboardVariableSelection(): JSX.Element | null {
 				order: cleanedOrder,
 				graph,
 				parentDependencyGraph,
+				hasCycle,
+				cycleNodes,
 			});
 		}
 	}, [setVariablesToGetUpdated, variables, variablesTableData]);
@@ -166,25 +170,37 @@ function DashboardVariableSelection(): JSX.Element | null {
 	);
 
 	return (
-		<Row style={{ display: 'flex', gap: '12px' }}>
+		<>
-			{orderBasedSortedVariables &&
+			{dependencyData?.hasCycle && (
-				Array.isArray(orderBasedSortedVariables) &&
+				<Alert
-				orderBasedSortedVariables.length > 0 &&
+					message={`Circular dependency detected: ${dependencyData?.cycleNodes?.join(
-				orderBasedSortedVariables.map((variable) => (
+						' → ',
-					<VariableItem
+					)}`}
-						key={`${variable.name}${variable.id}}${variable.order}`}
+					type="error"
-						existingVariables={variables}
+					showIcon
-						variableData={{
+					className="cycle-error-alert"
-							name: variable.name,
+				/>
-							...variable,
+			)}
-						}}
+			<Row style={{ display: 'flex', gap: '12px' }}>
-						onValueUpdate={onValueUpdate}
+				{orderBasedSortedVariables &&
-						variablesToGetUpdated={variablesToGetUpdated}
+					Array.isArray(orderBasedSortedVariables) &&
-						setVariablesToGetUpdated={setVariablesToGetUpdated}
+					orderBasedSortedVariables.length > 0 &&
-						dependencyData={dependencyData}
+					orderBasedSortedVariables.map((variable) => (
-					/>
+						<VariableItem
-				))}
+							key={`${variable.name}${variable.id}}${variable.order}`}
-		</Row>
+							existingVariables={variables}
+							variableData={{
+								name: variable.name,
+								...variable,
+							}}
+							onValueUpdate={onValueUpdate}
+							variablesToGetUpdated={variablesToGetUpdated}
+							setVariablesToGetUpdated={setVariablesToGetUpdated}
+							dependencyData={dependencyData}
+						/>
+					))}
+			</Row>
+		</>
 	);
 }
 

frontend/src/container/NewDashboard/DashboardVariablesSelection/VariableItem.test.tsx

@@ -53,6 +53,7 @@ describe('VariableItem', () => {
 						order: [],
 						graph: {},
 						parentDependencyGraph: {},
+						hasCycle: false,
 					}}
 				/>
 			</MockQueryClientProvider>,
@@ -74,6 +75,7 @@ describe('VariableItem', () => {
 						order: [],
 						graph: {},
 						parentDependencyGraph: {},
+						hasCycle: false,
 					}}
 				/>
 			</MockQueryClientProvider>,
@@ -94,6 +96,7 @@ describe('VariableItem', () => {
 						order: [],
 						graph: {},
 						parentDependencyGraph: {},
+						hasCycle: false,
 					}}
 				/>
 			</MockQueryClientProvider>,
@@ -128,6 +131,7 @@ describe('VariableItem', () => {
 						order: [],
 						graph: {},
 						parentDependencyGraph: {},
+						hasCycle: false,
 					}}
 				/>
 			</MockQueryClientProvider>,
@@ -157,6 +161,7 @@ describe('VariableItem', () => {
 						order: [],
 						graph: {},
 						parentDependencyGraph: {},
+						hasCycle: false,
 					}}
 				/>
 			</MockQueryClientProvider>,
@@ -178,6 +183,7 @@ describe('VariableItem', () => {
 						order: [],
 						graph: {},
 						parentDependencyGraph: {},
+						hasCycle: false,
 					}}
 				/>
 			</MockQueryClientProvider>,

frontend/src/container/NewDashboard/DashboardVariablesSelection/__test__/dashboardVariables.test.tsx

@@ -191,16 +191,6 @@ describe('dashboardVariables - utilities and processors', () => {
 		describe('buildDependencyGraph', () => {
 			it('should build complete dependency graph with correct structure and order', () => {
 				const expected = {
-					graph: {
-						deployment_environment: ['service_name', 'endpoint'],
-						service_name: ['endpoint'],
-						endpoint: ['http_status_code'],
-						http_status_code: [],
-						k8s_cluster_name: ['k8s_node_name', 'k8s_namespace_name'],
-						k8s_node_name: ['k8s_namespace_name'],
-						k8s_namespace_name: [],
-						environment: [],
-					},
 					order: [
 						'deployment_environment',
 						'k8s_cluster_name',
@@ -211,6 +201,28 @@ describe('dashboardVariables - utilities and processors', () => {
 						'k8s_namespace_name',
 						'http_status_code',
 					],
+					graph: {
+						deployment_environment: ['service_name', 'endpoint'],
+						service_name: ['endpoint'],
+						endpoint: ['http_status_code'],
+						http_status_code: [],
+						k8s_cluster_name: ['k8s_node_name', 'k8s_namespace_name'],
+						k8s_node_name: ['k8s_namespace_name'],
+						k8s_namespace_name: [],
+						environment: [],
+					},
+					parentDependencyGraph: {
+						deployment_environment: [],
+						service_name: ['deployment_environment'],
+						endpoint: ['deployment_environment', 'service_name'],
+						http_status_code: ['endpoint'],
+						k8s_cluster_name: [],
+						k8s_node_name: ['k8s_cluster_name'],
+						k8s_namespace_name: ['k8s_cluster_name', 'k8s_node_name'],
+						environment: [],
+					},
+					hasCycle: false,
+					cycleNodes: undefined,
 				};
 
 				expect(buildDependencyGraph(graph)).toEqual(expected);

frontend/src/container/NewDashboard/DashboardVariablesSelection/util.ts

@@ -95,10 +95,96 @@ export const buildDependencies = (
 	return graph;
 };
 
-// Function to build the dependency graph
+export interface IDependencyData {
+	order: string[];
+	graph: VariableGraph;
+	parentDependencyGraph: VariableGraph;
+	hasCycle: boolean;
+	cycleNodes?: string[];
+}
+
+export const buildParentDependencyGraph = (
+	graph: VariableGraph,
+): VariableGraph => {
+	const parentGraph: VariableGraph = {};
+
+	// Initialize empty arrays for all nodes
+	Object.keys(graph).forEach((node) => {
+		parentGraph[node] = [];
+	});
+
+	// For each node and its children in the original graph
+	Object.entries(graph).forEach(([node, children]) => {
+		// For each child, add the current node as its parent
+		children.forEach((child) => {
+			parentGraph[child].push(node);
+		});
+	});
+
+	return parentGraph;
+};
+
+const collectCyclePath = (
+	graph: VariableGraph,
+	start: string,
+	end: string,
+): string[] => {
+	const path: string[] = [];
+	let current = start;
+
+	const findParent = (node: string): string | undefined =>
+		Object.keys(graph).find((key) => graph[key]?.includes(node));
+
+	while (current !== end) {
+		const parent = findParent(current);
+		if (!parent) break;
+		path.push(parent);
+		current = parent;
+	}
+
+	return [start, ...path];
+};
+
+const detectCycle = (
+	graph: VariableGraph,
+	node: string,
+	visited: Set<string>,
+	recStack: Set<string>,
+): string[] | null => {
+	if (!visited.has(node)) {
+		visited.add(node);
+		recStack.add(node);
+
+		const neighbors = graph[node] || [];
+		let cycleNodes: string[] | null = null;
+
+		neighbors.some((neighbor) => {
+			if (!visited.has(neighbor)) {
+				const foundCycle = detectCycle(graph, neighbor, visited, recStack);
+				if (foundCycle) {
+					cycleNodes = foundCycle;
+					return true;
+				}
+			} else if (recStack.has(neighbor)) {
+				// Found a cycle, collect the cycle nodes
+				cycleNodes = collectCyclePath(graph, node, neighbor);
+				return true;
+			}
+			return false;
+		});
+
+		if (cycleNodes) {
+			return cycleNodes;
+		}
+	}
+	recStack.delete(node);
+	return null;
+};
+
 export const buildDependencyGraph = (
 	dependencies: VariableGraph,
-): { order: string[]; graph: VariableGraph } => {
+	// eslint-disable-next-line sonarjs/cognitive-complexity
+): IDependencyData => {
 	const inDegree: Record<string, number> = {};
 	const adjList: VariableGraph = {};
 
@@ -113,6 +199,22 @@ export const buildDependencyGraph = (
 		});
 	});
 
+	// Detect cycles
+	const visited = new Set<string>();
+	const recStack = new Set<string>();
+	let cycleNodes: string[] | undefined;
+
+	Object.keys(dependencies).some((node) => {
+		if (!visited.has(node)) {
+			const foundCycle = detectCycle(dependencies, node, visited, recStack);
+			if (foundCycle) {
+				cycleNodes = foundCycle;
+				return true;
+			}
+		}
+		return false;
+	});
+
 	// Topological sort using Kahn's Algorithm
 	const queue: string[] = Object.keys(inDegree).filter(
 		(node) => inDegree[node] === 0,
@@ -132,11 +234,15 @@ export const buildDependencyGraph = (
 		});
 	}
 
-	if (topologicalOrder.length !== Object.keys(dependencies)?.length) {
+	const hasCycle = topologicalOrder.length !== Object.keys(dependencies)?.length;
-		console.error('Cycle detected in the dependency graph!');
-	}
 
-	return { order: topologicalOrder, graph: adjList };
+	return {
+		order: topologicalOrder,
+		graph: adjList,
+		parentDependencyGraph: buildParentDependencyGraph(adjList),
+		hasCycle,
+		cycleNodes,
+	};
 };
 
 export const onUpdateVariableNode = (
@@ -159,27 +265,6 @@ export const onUpdateVariableNode = (
 	});
 };
 
-export const buildParentDependencyGraph = (
-	graph: VariableGraph,
-): VariableGraph => {
-	const parentGraph: VariableGraph = {};
-
-	// Initialize empty arrays for all nodes
-	Object.keys(graph).forEach((node) => {
-		parentGraph[node] = [];
-	});
-
-	// For each node and its children in the original graph
-	Object.entries(graph).forEach(([node, children]) => {
-		// For each child, add the current node as its parent
-		children.forEach((child) => {
-			parentGraph[child].push(node);
-		});
-	});
-
-	return parentGraph;
-};
-
 export const checkAPIInvocation = (
 	variablesToGetUpdated: string[],
 	variableData: IDashboardVariable,
@@ -206,9 +291,3 @@ export const checkAPIInvocation = (
 		variablesToGetUpdated[0] === variableData.name
 	);
 };
-
-export interface IDependencyData {
-	order: string[];
-	graph: VariableGraph;
-	parentDependencyGraph: VariableGraph;
-}

frontend/src/hooks/dashboard/__test__/useGetResolvedText.test.tsx

@@ -73,18 +73,20 @@ describe('useGetResolvedText', () => {
 	});
 
 	it('should handle different variable formats', () => {
-		const text = 'Logs in $service.name, {{service.name}}, [[service.name]]';
+		const text =
+			'Logs in $service.name, {{service.name}}, [[service.name]] - $dyn-service.name';
 		const variables = {
 			'service.name': SERVICE_VAR,
+			'$dyn-service.name': 'dyn-1, dyn-2',
 		};
 
 		const { result } = renderHookWithProps({ text, variables });
 
 		expect(result.current.truncatedText).toBe(
-			'Logs in test, app +2, test, app +2, test, app +2',
+			'Logs in test, app +2, test, app +2, test, app +2 - dyn-1, dyn-2',
 		);
 		expect(result.current.fullText).toBe(
-			'Logs in test, app, frontend, env, test, app, frontend, env, test, app, frontend, env',
+			'Logs in test, app, frontend, env, test, app, frontend, env, test, app, frontend, env - dyn-1, dyn-2',
 		);
 	});
 

frontend/src/hooks/dashboard/useGetResolvedText.tsx

@@ -82,11 +82,12 @@ function useGetResolvedText({
 
 	const combinedPattern = useMemo(() => {
 		const escapedMatcher = matcher.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+		const varNamePattern = '[a-zA-Z_\\-][a-zA-Z0-9_.\\-]*';
 		const variablePatterns = [
-			`\\{\\{\\s*?\\.([^\\s}]+)\\s*?\\}\\}`, // {{.var}}
+			`\\{\\{\\s*?\\.(${varNamePattern})\\s*?\\}\\}`, // {{.var}}
-			`\\{\\{\\s*([^\\s}]+)\\s*\\}\\}`, // {{var}}
+			`\\{\\{\\s*(${varNamePattern})\\s*\\}\\}`, // {{var}}
-			`${escapedMatcher}([\\w.]+)`, // matcher + var.name
+			`${escapedMatcher}(${varNamePattern})`, // matcher + var.name
-			`\\[\\[\\s*([^\\s\\]]+)\\s*\\]\\]`, // [[var]]
+			`\\[\\[\\s*(${varNamePattern})\\s*\\]\\]`, // [[var]]
 		];
 		return new RegExp(variablePatterns.join('|'), 'g');
 	}, [matcher]);
@@ -94,20 +95,38 @@ function useGetResolvedText({
 	const extractVarName = useCallback(
 		(match: string): string => {
 			// Extract variable name from different formats
+			const varNamePattern = '[a-zA-Z_\\-][a-zA-Z0-9_.\\-]*';
 			if (match.startsWith('{{')) {
-				const dotMatch = match.match(/\{\{\s*\.([^}]+)\}\}/);
+				const dotMatch = match.match(
+					new RegExp(`\\{\\{\\s*\\.(${varNamePattern})\\s*\\}\\}`),
+				);
 				if (dotMatch) return dotMatch[1].trim();
-				const normalMatch = match.match(/\{\{\s*([^}]+)\}\}/);
+				const normalMatch = match.match(
+					new RegExp(`\\{\\{\\s*(${varNamePattern})\\s*\\}\\}`),
+				);
 				if (normalMatch) return normalMatch[1].trim();
 			} else if (match.startsWith('[[')) {
-				const bracketMatch = match.match(/\[\[\s*([^\]]+)\]\]/);
+				const bracketMatch = match.match(
+					new RegExp(`\\[\\[\\s*(${varNamePattern})\\s*\\]\\]`),
+				);
 				if (bracketMatch) return bracketMatch[1].trim();
 			} else if (match.startsWith(matcher)) {
-				return match.substring(matcher.length);
+				// For $ variables, we always want to strip the prefix
+				// unless the full match exists in processedVariables
+				const withoutPrefix = match.substring(matcher.length).trim();
+				const fullMatch = match.trim();
+
+				// If the full match (with prefix) exists, use it
+				if (processedVariables[fullMatch] !== undefined) {
+					return fullMatch;
+				}
+
+				// Otherwise return without prefix
+				return withoutPrefix;
 			}
 			return match;
 		},
-		[matcher],
+		[matcher, processedVariables],
 	);
 
 	const fullText = useMemo(() => {