chore: remove usage of updatedTimeRef in dashboard provider

* chore: remove selectedRowWidgetId from provider

* chore: rename helper

* chore: add comment

frontend/src/container/DashboardContainer/DashboardDescription/__tests__/DashboardDescription.test.tsx

@@ -193,20 +193,16 @@ describe('Dashboard landing page actions header tests', () => {
 			handleDashboardLockToggle: jest.fn(),
 			dashboardResponse: {} as IDashboardContext['dashboardResponse'],
 			selectedDashboard: (getDashboardById.data as unknown) as Dashboard,
-			dashboardId: '4',
 			layouts: [],
 			panelMap: {},
 			setPanelMap: jest.fn(),
 			setLayouts: jest.fn(),
 			setSelectedDashboard: jest.fn(),
-			updatedTimeRef: { current: null },
 			toScrollWidgetId: '',
 			setToScrollWidgetId: jest.fn(),
 			updateLocalStorageDashboardVariables: jest.fn(),
 			dashboardQueryRangeCalled: false,
 			setDashboardQueryRangeCalled: jest.fn(),
-			selectedRowWidgetId: null,
-			setSelectedRowWidgetId: jest.fn(),
 			isDashboardFetching: false,
 			columnWidths: {},
 			setColumnWidths: jest.fn(),

frontend/src/container/DashboardContainer/DashboardDescription/index.tsx

@@ -78,7 +78,6 @@ function DashboardDescription(props: DashboardDescriptionProps): JSX.Element {
 		isDashboardLocked,
 		setSelectedDashboard,
 		handleToggleDashboardSlider,
-		setSelectedRowWidgetId,
 		handleDashboardLockToggle,
 	} = useDashboard();
 
@@ -146,7 +145,6 @@ function DashboardDescription(props: DashboardDescriptionProps): JSX.Element {
 	const [addPanelPermission] = useComponentPermission(permissions, userRole);
 
 	const onEmptyWidgetHandler = useCallback(() => {
-		setSelectedRowWidgetId(null);
 		handleToggleDashboardSlider(true);
 		logEvent('Dashboard Detail: Add new panel clicked', {
 			dashboardId: selectedDashboard?.id,

frontend/src/container/DashboardContainer/DashboardVariablesSelection/useDashboardVariableUpdate.ts

@@ -67,17 +67,18 @@ export const useDashboardVariableUpdate = (): UseDashboardVariableUpdateReturn =
 							const oldVariables = prev?.data.variables;
 							// this is added to handle case where we have two different
 							// schemas for variable response
-							if (oldVariables?.[id]) {
-								oldVariables[id] = {
-									...oldVariables[id],
+							const updatedVariables = { ...oldVariables };
+							if (updatedVariables?.[id]) {
+								updatedVariables[id] = {
+									...updatedVariables[id],
 									selectedValue: value,
 									allSelected,
 									haveCustomValuesSelected,
 								};
 							}
-							if (oldVariables?.[name]) {
-								oldVariables[name] = {
-									...oldVariables[name],
+							if (updatedVariables?.[name]) {
+								updatedVariables[name] = {
+									...updatedVariables[name],
 									selectedValue: value,
 									allSelected,
 									haveCustomValuesSelected,
@@ -87,9 +88,7 @@ export const useDashboardVariableUpdate = (): UseDashboardVariableUpdateReturn =
 								...prev,
 								data: {
 									...prev?.data,
-									variables: {
-										...oldVariables,
-									},
+									variables: updatedVariables,
 								},
 							};
 						}

frontend/src/container/GridCardLayout/DashboardEmptyState/DashboardEmptyState.tsx

@@ -19,7 +19,6 @@ export default function DashboardEmptyState(): JSX.Element {
 		selectedDashboard,
 		isDashboardLocked,
 		handleToggleDashboardSlider,
-		setSelectedRowWidgetId,
 	} = useDashboard();
 
 	const variablesSettingsTabHandle = useRef<VariablesSettingsTab>(null);
@@ -42,7 +41,6 @@ export default function DashboardEmptyState(): JSX.Element {
 	const [addPanelPermission] = useComponentPermission(permissions, userRole);
 
 	const onEmptyWidgetHandler = useCallback(() => {
-		setSelectedRowWidgetId(null);
 		handleToggleDashboardSlider(true);
 		logEvent('Dashboard Detail: Add new panel clicked', {
 			dashboardId: selectedDashboard?.id,

frontend/src/container/GridCardLayout/GridCardLayout.tsx

@@ -71,7 +71,6 @@ function GraphLayout(props: GraphLayoutProps): JSX.Element {
 		isDashboardLocked,
 		dashboardQueryRangeCalled,
 		setDashboardQueryRangeCalled,
-		setSelectedRowWidgetId,
 		isDashboardFetching,
 		columnWidths,
 	} = useDashboard();
@@ -195,7 +194,6 @@ function GraphLayout(props: GraphLayoutProps): JSX.Element {
 
 		updateDashboardMutation.mutate(updatedDashboard, {
 			onSuccess: (updatedDashboard) => {
-				setSelectedRowWidgetId(null);
 				if (updatedDashboard.data) {
 					if (updatedDashboard.data.data.layout) {
 						setLayouts(sortLayout(updatedDashboard.data.data.layout));

frontend/src/container/GridCardLayout/WidgetRow.tsx

@@ -5,6 +5,7 @@ import useComponentPermission from 'hooks/useComponentPermission';
 import { EllipsisIcon, PenLine, Plus, X } from 'lucide-react';
 import { useAppContext } from 'providers/App/App';
 import { useDashboard } from 'providers/Dashboard/Dashboard';
+import { setSelectedRowWidgetId } from 'providers/Dashboard/helpers/selectedRowWidgetIdHelper';
 import { ROLES, USER_ROLES } from 'types/roles';
 import { ComponentTypes } from 'utils/permission';
 
@@ -37,7 +38,6 @@ export function WidgetRowHeader(props: WidgetRowHeaderProps): JSX.Element {
 		handleToggleDashboardSlider,
 		selectedDashboard,
 		isDashboardLocked,
-		setSelectedRowWidgetId,
 	} = useDashboard();
 
 	const permissions: ComponentTypes[] = ['add_panel'];
@@ -81,7 +81,12 @@ export function WidgetRowHeader(props: WidgetRowHeaderProps): JSX.Element {
 							disabled={!editWidget && addPanelPermission && !isDashboardLocked}
 							icon={<Plus size={14} />}
 							onClick={(): void => {
-								setSelectedRowWidgetId(id);
+								// TODO: @AshwinBhatkal Simplify this check in cleanup of https://github.com/SigNoz/engineering-pod/issues/3953
+								if (!selectedDashboard?.id) {
+									return;
+								}
+
+								setSelectedRowWidgetId(selectedDashboard.id, id);
 								handleToggleDashboardSlider(true);
 							}}
 						>

frontend/src/container/NewWidget/index.tsx

@@ -34,6 +34,10 @@ import { cloneDeep, defaultTo, isEmpty, isUndefined } from 'lodash-es';
 import { Check, X } from 'lucide-react';
 import { DashboardWidgetPageParams } from 'pages/DashboardWidget';
 import { useDashboard } from 'providers/Dashboard/Dashboard';
+import {
+	clearSelectedRowWidgetId,
+	getSelectedRowWidgetId,
+} from 'providers/Dashboard/helpers/selectedRowWidgetIdHelper';
 import {
 	getNextWidgets,
 	getPreviousWidgets,
@@ -86,8 +90,6 @@ function NewWidget({
 		selectedDashboard,
 		setSelectedDashboard,
 		setToScrollWidgetId,
-		selectedRowWidgetId,
-		setSelectedRowWidgetId,
 		columnWidths,
 	} = useDashboard();
 
@@ -450,6 +452,8 @@ function NewWidget({
 		const widgetId = query.get('widgetId') || '';
 		let updatedLayout = selectedDashboard.data.layout || [];
 
+		const selectedRowWidgetId = getSelectedRowWidgetId(dashboardId);
+
 		if (isNewDashboard && isEmpty(selectedRowWidgetId)) {
 			const newLayoutItem = placeWidgetAtBottom(widgetId, updatedLayout);
 			updatedLayout = [...updatedLayout, newLayoutItem];
@@ -554,7 +558,6 @@ function NewWidget({
 
 		updateDashboardMutation.mutateAsync(dashboard, {
 			onSuccess: (updatedDashboard) => {
-				setSelectedRowWidgetId(null);
 				setSelectedDashboard(updatedDashboard.data);
 				setToScrollWidgetId(selectedWidget?.id || '');
 				safeNavigate({
@@ -566,7 +569,6 @@ function NewWidget({
 		selectedDashboard,
 		query,
 		isNewDashboard,
-		selectedRowWidgetId,
 		afterWidgets,
 		selectedWidget,
 		selectedTime.enum,
@@ -577,7 +579,6 @@ function NewWidget({
 		widgets,
 		setSelectedDashboard,
 		setToScrollWidgetId,
-		setSelectedRowWidgetId,
 		safeNavigate,
 		dashboardId,
 	]);
@@ -681,6 +682,10 @@ function NewWidget({
 		 * on mount here with the currentQuery in the begining itself
 		 */
 		setSupersetQuery(currentQuery);
+
+		return (): void => {
+			clearSelectedRowWidgetId(dashboardId);
+		};
 		// eslint-disable-next-line react-hooks/exhaustive-deps
 	}, []);
 

frontend/src/hooks/dashboard/useUpdateDashboard.tsx

@@ -1,6 +1,5 @@
 import { useMutation, UseMutationResult } from 'react-query';
 import update from 'api/v1/dashboards/id/update';
-import dayjs from 'dayjs';
 import { useDashboard } from 'providers/Dashboard/Dashboard';
 import { useErrorModal } from 'providers/ErrorModalProvider';
 import { SuccessResponseV2 } from 'types/api';
@@ -9,12 +8,12 @@ import { Props } from 'types/api/dashboard/update';
 import APIError from 'types/api/error';
 
 export const useUpdateDashboard = (): UseUpdateDashboard => {
-	const { updatedTimeRef } = useDashboard();
+	const { setSelectedDashboard } = useDashboard();
 	const { showErrorModal } = useErrorModal();
 	return useMutation(update, {
 		onSuccess: (data) => {
 			if (data.data) {
-				updatedTimeRef.current = dayjs(data.data.updatedAt);
+				setSelectedDashboard(data.data);
 			}
 		},
 		onError: (error) => {

frontend/src/providers/Dashboard/Dashboard.tsx

@@ -21,7 +21,7 @@ import locked from 'api/v1/dashboards/id/lock';
 import { ALL_SELECTED_VALUE } from 'components/NewSelect/utils';
 import { REACT_QUERY_KEY } from 'constants/reactQueryKeys';
 import ROUTES from 'constants/routes';
-import dayjs, { Dayjs } from 'dayjs';
+import dayjs from 'dayjs';
 import { useDashboardVariablesFromLocalStorage } from 'hooks/dashboard/useDashboardFromLocalStorage';
 import useVariablesFromUrl from 'hooks/dashboard/useVariablesFromUrl';
 import useTabVisibility from 'hooks/useTabFocus';
@@ -68,21 +68,17 @@ export const DashboardContext = createContext<IDashboardContext>({
 		APIError
 	>,
 	selectedDashboard: {} as Dashboard,
-	dashboardId: '',
 	layouts: [],
 	panelMap: {},
 	setPanelMap: () => {},
 
 	setLayouts: () => {},
 	setSelectedDashboard: () => {},
-	updatedTimeRef: {} as React.MutableRefObject<Dayjs | null>,
 	toScrollWidgetId: '',
 	setToScrollWidgetId: () => {},
 	updateLocalStorageDashboardVariables: () => {},
 	dashboardQueryRangeCalled: false,
 	setDashboardQueryRangeCalled: () => {},
-	selectedRowWidgetId: '',
-	setSelectedRowWidgetId: () => {},
 	isDashboardFetching: false,
 	columnWidths: {},
 	setColumnWidths: () => {},
@@ -102,10 +98,6 @@ export function DashboardProvider({
 
 	const [isDashboardLocked, setIsDashboardLocked] = useState<boolean>(false);
 
-	const [selectedRowWidgetId, setSelectedRowWidgetId] = useState<string | null>(
-		null,
-	);
-
 	const [
 		dashboardQueryRangeCalled,
 		setDashboardQueryRangeCalled,
@@ -172,13 +164,11 @@ export function DashboardProvider({
 
 	const { getUrlVariables, updateUrlVariable } = useVariablesFromUrl();
 
-	const updatedTimeRef = useRef<Dayjs | null>(null); // Using ref to store the updated time
 	const modalRef = useRef<any>(null);
 
 	const isVisible = useTabVisibility();
 
 	const { t } = useTranslation(['dashboard']);
-	const dashboardRef = useRef<Dashboard>();
 
 	const [isDashboardFetching, setIsDashboardFetching] = useState<boolean>(false);
 
@@ -311,101 +301,94 @@ export function DashboardProvider({
 				if (variables) {
 					initializeDefaultVariables(variables, getUrlVariables, updateUrlVariable);
 				}
+			},
+		},
+	);
 
-				const updatedDashboardData = transformDashboardVariables(data?.data);
-				const updatedDate = dayjs(updatedDashboardData?.updatedAt);
+	// Handle dashboard data updates in a useEffect so that selectedDashboard state
+	// is always current when comparing timestamps (avoids stale closure issues in onSuccess)
+	useEffect(() => {
+		if (!dashboardResponse.data?.data) {
+			return;
+		}
 
-				setIsDashboardLocked(updatedDashboardData?.locked || false);
+		const updatedDashboardData = transformDashboardVariables(
+			dashboardResponse.data.data,
+		);
+		const updatedDate = dayjs(updatedDashboardData?.updatedAt);
 
-				// on first render
-				if (updatedTimeRef.current === null) {
+		setIsDashboardLocked(updatedDashboardData?.locked || false);
+
+		// on first render
+		if (!selectedDashboard) {
+			setSelectedDashboard(updatedDashboardData);
+			setLayouts(sortLayout(getUpdatedLayout(updatedDashboardData?.data.layout)));
+			setPanelMap(defaultTo(updatedDashboardData?.data?.panelMap, {}));
+			return;
+		}
+
+		if (
+			updatedDate.isAfter(dayjs(selectedDashboard.updatedAt)) &&
+			isVisible &&
+			selectedDashboard.id === updatedDashboardData?.id
+		) {
+			// show modal when state is out of sync
+			const modal = onModal.confirm({
+				centered: true,
+				title: t('dashboard_has_been_updated'),
+				content: t('do_you_want_to_refresh_the_dashboard'),
+				onOk() {
 					setSelectedDashboard(updatedDashboardData);
 
-					updatedTimeRef.current = updatedDate;
+					const { maxTime, minTime } = getMinMaxForSelectedTime(
+						globalTime.selectedTime,
+						globalTime.minTime,
+						globalTime.maxTime,
+					);
 
-					dashboardRef.current = updatedDashboardData;
+					dispatch({
+						type: UPDATE_TIME_INTERVAL,
+						payload: {
+							maxTime,
+							minTime,
+							selectedTime: globalTime.selectedTime,
+						},
+					});
 
 					setLayouts(
 						sortLayout(getUpdatedLayout(updatedDashboardData?.data.layout)),
 					);
 
-					setPanelMap(defaultTo(updatedDashboardData?.data?.panelMap, {}));
-				}
+					setPanelMap(defaultTo(updatedDashboardData?.data.panelMap, {}));
+				},
+			});
 
-				if (
-					updatedTimeRef.current !== null &&
-					updatedDate.isAfter(updatedTimeRef.current) &&
-					isVisible &&
-					dashboardRef.current?.id === updatedDashboardData?.id
-				) {
-					// show modal when state is out of sync
-					const modal = onModal.confirm({
-						centered: true,
-						title: t('dashboard_has_been_updated'),
-						content: t('do_you_want_to_refresh_the_dashboard'),
-						onOk() {
-							setSelectedDashboard(updatedDashboardData);
+			modalRef.current = modal;
+		} else {
+			// normal flow
+			if (!isEqual(selectedDashboard, updatedDashboardData)) {
+				setSelectedDashboard(updatedDashboardData);
+			}
 
-							const { maxTime, minTime } = getMinMaxForSelectedTime(
-								globalTime.selectedTime,
-								globalTime.minTime,
-								globalTime.maxTime,
-							);
+			if (
+				!isEqual(
+					[omitBy(layouts, (value): boolean => isUndefined(value))[0]],
+					updatedDashboardData?.data.layout,
+				)
+			) {
+				setLayouts(sortLayout(getUpdatedLayout(updatedDashboardData?.data.layout)));
 
-							dispatch({
-								type: UPDATE_TIME_INTERVAL,
-								payload: {
-									maxTime,
-									minTime,
-									selectedTime: globalTime.selectedTime,
-								},
-							});
-
-							dashboardRef.current = updatedDashboardData;
-
-							updatedTimeRef.current = dayjs(updatedDashboardData?.updatedAt);
-
-							setLayouts(
-								sortLayout(getUpdatedLayout(updatedDashboardData?.data.layout)),
-							);
-
-							setPanelMap(defaultTo(updatedDashboardData?.data.panelMap, {}));
-						},
-					});
-
-					modalRef.current = modal;
-				} else {
-					// normal flow
-					updatedTimeRef.current = dayjs(updatedDashboardData?.updatedAt);
-
-					dashboardRef.current = updatedDashboardData;
-
-					if (!isEqual(selectedDashboard, updatedDashboardData)) {
-						setSelectedDashboard(updatedDashboardData);
-					}
-
-					if (
-						!isEqual(
-							[omitBy(layouts, (value): boolean => isUndefined(value))[0]],
-							updatedDashboardData?.data.layout,
-						)
-					) {
-						setLayouts(
-							sortLayout(getUpdatedLayout(updatedDashboardData?.data.layout)),
-						);
-
-						setPanelMap(defaultTo(updatedDashboardData?.data.panelMap, {}));
-					}
-				}
-			},
-		},
-	);
+				setPanelMap(defaultTo(updatedDashboardData?.data.panelMap, {}));
+			}
+		}
+		// eslint-disable-next-line react-hooks/exhaustive-deps
+	}, [dashboardResponse.data]);
 
 	useEffect(() => {
 		// make the call on tab visibility only if the user is on dashboard / widget page
 		if (
 			isVisible &&
-			updatedTimeRef.current &&
+			!!selectedDashboard &&
 			(!!isDashboardPage || !!isDashboardWidgetPage)
 		) {
 			dashboardResponse.refetch();
@@ -463,13 +446,10 @@ export function DashboardProvider({
 			setLayouts,
 			setPanelMap,
 			setSelectedDashboard,
-			updatedTimeRef,
 			setToScrollWidgetId,
 			updateLocalStorageDashboardVariables,
 			dashboardQueryRangeCalled,
 			setDashboardQueryRangeCalled,
-			selectedRowWidgetId,
-			setSelectedRowWidgetId,
 			isDashboardFetching,
 			columnWidths,
 			setColumnWidths,
@@ -488,8 +468,6 @@ export function DashboardProvider({
 			currentDashboard,
 			dashboardQueryRangeCalled,
 			setDashboardQueryRangeCalled,
-			selectedRowWidgetId,
-			setSelectedRowWidgetId,
 			isDashboardFetching,
 			columnWidths,
 			setColumnWidths,

frontend/src/providers/Dashboard/__tests__/Dashboard.test.tsx

@@ -58,8 +58,9 @@ jest.mock('react-redux', () => ({
 jest.mock('uuid', () => ({ v4: jest.fn(() => 'mock-uuid') }));
 
 function TestComponent(): JSX.Element {
-	const { dashboardResponse, dashboardId, selectedDashboard } = useDashboard();
+	const { dashboardResponse, selectedDashboard } = useDashboard();
 	const { dashboardVariables } = useDashboardVariables();
+	const dashboardId = selectedDashboard?.id;
 
 	return (
 		<div>

frontend/src/providers/Dashboard/helpers/selectedRowWidgetIdHelper.ts

@@ -0,0 +1,28 @@
+const PREFIX = 'dashboard_row_widget_';
+
+function getKey(dashboardId: string): string {
+	return `${PREFIX}${dashboardId}`;
+}
+
+export function setSelectedRowWidgetId(
+	dashboardId: string,
+	widgetId: string,
+): void {
+	const key = getKey(dashboardId);
+
+	// remove all other selected widget ids for the dashboard before setting the new one
+	// to ensure only one widget is selected at a time. Helps out in weird navigate and refresh scenarios
+	Object.keys(sessionStorage)
+		.filter((k) => k.startsWith(PREFIX) && k !== key)
+		.forEach((k) => sessionStorage.removeItem(k));
+
+	sessionStorage.setItem(key, widgetId);
+}
+
+export function getSelectedRowWidgetId(dashboardId: string): string | null {
+	return sessionStorage.getItem(getKey(dashboardId));
+}
+
+export function clearSelectedRowWidgetId(dashboardId: string): void {
+	sessionStorage.removeItem(getKey(dashboardId));
+}

frontend/src/providers/Dashboard/types.ts

@@ -1,6 +1,5 @@
 import { Layout } from 'react-grid-layout';
 import { UseQueryResult } from 'react-query';
-import dayjs from 'dayjs';
 import { SuccessResponseV2 } from 'types/api';
 import { Dashboard } from 'types/api/dashboard/getAll';
 
@@ -15,7 +14,6 @@ export interface IDashboardContext {
 	handleDashboardLockToggle: (value: boolean) => void;
 	dashboardResponse: UseQueryResult<SuccessResponseV2<Dashboard>, unknown>;
 	selectedDashboard: Dashboard | undefined;
-	dashboardId: string;
 	layouts: Layout[];
 	panelMap: Record<string, { widgets: Layout[]; collapsed: boolean }>;
 	setPanelMap: React.Dispatch<React.SetStateAction<Record<string, any>>>;
@@ -23,7 +21,6 @@ export interface IDashboardContext {
 	setSelectedDashboard: React.Dispatch<
 		React.SetStateAction<Dashboard | undefined>
 	>;
-	updatedTimeRef: React.MutableRefObject<dayjs.Dayjs | null>;
 	toScrollWidgetId: string;
 	setToScrollWidgetId: React.Dispatch<React.SetStateAction<string>>;
 	updateLocalStorageDashboardVariables: (
@@ -40,8 +37,6 @@ export interface IDashboardContext {
 	) => void;
 	dashboardQueryRangeCalled: boolean;
 	setDashboardQueryRangeCalled: (value: boolean) => void;
-	selectedRowWidgetId: string | null;
-	setSelectedRowWidgetId: React.Dispatch<React.SetStateAction<string | null>>;
 	isDashboardFetching: boolean;
 	columnWidths: WidgetColumnWidths;
 	setColumnWidths: React.Dispatch<React.SetStateAction<WidgetColumnWidths>>;