fix(logs): centralize time range handling in DateTimeSelectionV2

- Remove custom time range handling logic from logs components - Use unified time range selection through DateTimeSelectionV2 component
SigNoz · Feb 23, 2025 · f8c2788 · f8c2788
1 parent e18bda8
commit f8c2788
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 74 deletions.
diff --git a/frontend/src/container/LogsExplorerViews/index.tsx b/frontend/src/container/LogsExplorerViews/index.tsx
@@ -29,7 +29,6 @@ import TimeSeriesView from 'container/TimeSeriesView/TimeSeriesView';
 import dayjs from 'dayjs';
 import { useUpdateDashboard } from 'hooks/dashboard/useUpdateDashboard';
 import { addEmptyWidgetInDashboardJSONWithQuery } from 'hooks/dashboard/utils';
-import { LogTimeRange } from 'hooks/logs/types';
 import { useCopyLogLink } from 'hooks/logs/useCopyLogLink';
 import { useGetExplorerQueryRange } from 'hooks/queryBuilder/useGetExplorerQueryRange';
 import { useGetPanelTypesQueryParam } from 'hooks/queryBuilder/useGetPanelTypesQueryParam';
@@ -103,7 +102,7 @@ function LogsExplorerViews({
 	// this is to respect the panel type present in the URL rather than defaulting it to list always.
 	const panelTypes = useGetPanelTypesQueryParam(PANEL_TYPES.LIST);
 
-	const { activeLogId, onTimeRangeChange } = useCopyLogLink();
+	const { activeLogId } = useCopyLogLink();
 
 	const { queryData: pageSize } = useUrlQueryData(
 		QueryParams.pageSize,
@@ -537,7 +536,6 @@ function LogsExplorerViews({
 	}, [handleSetConfig, panelTypes]);
 
 	useEffect(() => {
-		const currentParams = data?.params as Omit<LogTimeRange, 'pageSize'>;
 		const currentData = data?.payload?.data?.newResult?.data?.result || [];
 		if (currentData.length > 0 && currentData[0].list) {
 			const currentLogs: ILog[] = currentData[0].list.map((item) => ({
@@ -547,11 +545,6 @@ function LogsExplorerViews({
 			const newLogs = [...logs, ...currentLogs];
 
 			setLogs(newLogs);
-			onTimeRangeChange({
-				start: currentParams?.start,
-				end: currentParams?.end,
-				pageSize: newLogs.length,
-			});
 		}
 
 		// eslint-disable-next-line react-hooks/exhaustive-deps
@@ -587,7 +580,6 @@ function LogsExplorerViews({
 		pageSize,
 		minTime,
 		activeLogId,
-		onTimeRangeChange,
 		panelType,
 		selectedView,
 	]);

diff --git a/frontend/src/container/TopNav/DateTimeSelectionV2/index.tsx b/frontend/src/container/TopNav/DateTimeSelectionV2/index.tsx
@@ -30,7 +30,7 @@ import getTimeString from 'lib/getTimeString';
 import { isObject } from 'lodash-es';
 import { Check, Copy, Info, Send, Undo } from 'lucide-react';
 import { useTimezone } from 'providers/Timezone';
-import { useCallback, useEffect, useMemo, useState } from 'react';
+import { useCallback, useEffect, useState } from 'react';
 import { useQueryClient } from 'react-query';
 import { connect, useDispatch, useSelector } from 'react-redux';
 import { RouteComponentProps, withRouter } from 'react-router-dom';
@@ -314,11 +314,6 @@ function DateTimeSelection({
 		return `Refreshed ${secondsDiff} sec ago`;
 	}, [maxTime, minTime, selectedTime]);
 
-	const isLogsExplorerPage = useMemo(
-		() => location.pathname === ROUTES.LOGS_EXPLORER,
-		[location.pathname],
-	);
-
 	const onSelectHandler = useCallback(
 		(value: Time | CustomTimeType): void => {
 			if (isModalTimeSelection) {
@@ -347,15 +342,13 @@ function DateTimeSelection({
 				return;
 			}
 
-			if (!isLogsExplorerPage) {
-				urlQuery.delete('startTime');
-				urlQuery.delete('endTime');
+			urlQuery.delete('startTime');
+			urlQuery.delete('endTime');
 
-				urlQuery.set(QueryParams.relativeTime, value);
+			urlQuery.set(QueryParams.relativeTime, value);
 
-				const generatedUrl = `${location.pathname}?${urlQuery.toString()}`;
-				safeNavigate(generatedUrl);
-			}
+			const generatedUrl = `${location.pathname}?${urlQuery.toString()}`;
+			safeNavigate(generatedUrl);
 
 			// For logs explorer - time range handling is managed in useCopyLogLink.ts:52
 
@@ -368,7 +361,6 @@ function DateTimeSelection({
 		},
 		[
 			initQueryBuilderData,
-			isLogsExplorerPage,
 			isModalTimeSelection,
 			location.pathname,
 			onTimeChange,
@@ -438,16 +430,14 @@ function DateTimeSelection({
 
 				updateLocalStorageForRoutes(JSON.stringify({ startTime, endTime }));
 
-				if (!isLogsExplorerPage) {
-					urlQuery.set(
-						QueryParams.startTime,
-						startTime?.toDate().getTime().toString(),
-					);
-					urlQuery.set(QueryParams.endTime, endTime?.toDate().getTime().toString());
-					urlQuery.delete(QueryParams.relativeTime);
-					const generatedUrl = `${location.pathname}?${urlQuery.toString()}`;
-					safeNavigate(generatedUrl);
-				}
+				urlQuery.set(
+					QueryParams.startTime,
+					startTime?.toDate().getTime().toString(),
+				);
+				urlQuery.set(QueryParams.endTime, endTime?.toDate().getTime().toString());
+				urlQuery.delete(QueryParams.relativeTime);
+				const generatedUrl = `${location.pathname}?${urlQuery.toString()}`;
+				safeNavigate(generatedUrl);
 			}
 		}
 	};
@@ -466,15 +456,13 @@ function DateTimeSelection({
 
 		setIsValidteRelativeTime(true);
 
-		if (!isLogsExplorerPage) {
-			urlQuery.delete('startTime');
-			urlQuery.delete('endTime');
+		urlQuery.delete('startTime');
+		urlQuery.delete('endTime');
 
-			urlQuery.set(QueryParams.relativeTime, dateTimeStr);
+		urlQuery.set(QueryParams.relativeTime, dateTimeStr);
 
-			const generatedUrl = `${location.pathname}?${urlQuery.toString()}`;
-			safeNavigate(generatedUrl);
-		}
+		const generatedUrl = `${location.pathname}?${urlQuery.toString()}`;
+		safeNavigate(generatedUrl);
 
 		if (!stagedQuery) {
 			return;

diff --git a/frontend/src/hooks/logs/types.ts b/frontend/src/hooks/logs/types.ts
@@ -5,15 +5,13 @@ import { DataTypes } from 'types/api/queryBuilder/queryAutocompleteResponse';
 export type LogTimeRange = {
 	start: number;
 	end: number;
-	pageSize: number;
 };
 
 export type UseCopyLogLink = {
 	isHighlighted: boolean;
 	isLogsExplorerPage: boolean;
 	activeLogId: string | null;
 	onLogCopy: MouseEventHandler<HTMLElement>;
-	onTimeRangeChange: (newTimeRange: LogTimeRange | null) => void;
 };
 
 export type UseActiveLog = {

diff --git a/frontend/src/hooks/logs/useCopyLogLink.ts b/frontend/src/hooks/logs/useCopyLogLink.ts
@@ -3,7 +3,6 @@ import ROUTES from 'constants/routes';
 import { useNotifications } from 'hooks/useNotifications';
 import useUrlQuery from 'hooks/useUrlQuery';
 import useUrlQueryData from 'hooks/useUrlQueryData';
-import history from 'lib/history';
 import {
 	MouseEventHandler,
 	useCallback,
@@ -18,7 +17,7 @@ import { AppState } from 'store/reducers';
 import { GlobalReducer } from 'types/reducer/globalTime';
 
 import { HIGHLIGHTED_DELAY } from './configs';
-import { LogTimeRange, UseCopyLogLink } from './types';
+import { UseCopyLogLink } from './types';
 
 export const useCopyLogLink = (logId?: string): UseCopyLogLink => {
 	const urlQuery = useUrlQuery();
@@ -31,27 +30,8 @@ export const useCopyLogLink = (logId?: string): UseCopyLogLink => {
 		null,
 	);
 
-	const { selectedTime, minTime, maxTime } = useSelector<
-		AppState,
-		GlobalReducer
-	>((state) => state.globalTime);
-
-	const onTimeRangeChange = useCallback(
-		(newTimeRange: LogTimeRange | null): void => {
-			if (selectedTime !== 'custom') {
-				urlQuery.delete(QueryParams.startTime);
-				urlQuery.delete(QueryParams.endTime);
-				urlQuery.set(QueryParams.relativeTime, selectedTime);
-			} else {
-				urlQuery.set(QueryParams.startTime, newTimeRange?.start.toString() || '');
-				urlQuery.set(QueryParams.endTime, newTimeRange?.end.toString() || '');
-				urlQuery.delete(QueryParams.relativeTime);
-			}
-
-			const generatedUrl = `${pathname}?${urlQuery.toString()}`;
-			history.replace(generatedUrl);
-		},
-		[pathname, urlQuery, selectedTime],
+	const { minTime, maxTime } = useSelector<AppState, GlobalReducer>(
+		(state) => state.globalTime,
 	);
 
 	const isActiveLog = useMemo(() => activeLogId === logId, [activeLogId, logId]);
@@ -101,6 +81,5 @@ export const useCopyLogLink = (logId?: string): UseCopyLogLink => {
 		isLogsExplorerPage,
 		activeLogId,
 		onLogCopy,
-		onTimeRangeChange,
 	};
 };
diff --git a/frontend/src/hooks/useLogsData.ts b/frontend/src/hooks/useLogsData.ts
@@ -19,7 +19,6 @@ import {
 import { QueryDataV3 } from 'types/api/widgets/getQuery';
 import { GlobalReducer } from 'types/reducer/globalTime';
 
-import { LogTimeRange } from './logs/types';
 import { useCopyLogLink } from './logs/useCopyLogLink';
 import { useGetExplorerQueryRange } from './queryBuilder/useGetExplorerQueryRange';
 import useUrlQueryData from './useUrlQueryData';
@@ -129,7 +128,7 @@ export const useLogsData = ({
 		return data;
 	};
 
-	const { activeLogId, onTimeRangeChange } = useCopyLogLink();
+	const { activeLogId } = useCopyLogLink();
 
 	const { data, isFetching } = useGetExplorerQueryRange(
 		requestData,
@@ -150,7 +149,6 @@ export const useLogsData = ({
 	);
 
 	useEffect(() => {
-		const currentParams = data?.params as Omit<LogTimeRange, 'pageSize'>;
 		const currentData = data?.payload?.data?.newResult?.data?.result || [];
 		if (currentData.length > 0 && currentData[0].list) {
 			const currentLogs: ILog[] = currentData[0].list.map((item) => ({
@@ -160,11 +158,6 @@ export const useLogsData = ({
 			const newLogs = [...logs, ...currentLogs];
 
 			setLogs(newLogs);
-			onTimeRangeChange({
-				start: currentParams?.start,
-				end: currentParams?.end,
-				pageSize: newLogs.length,
-			});
 		}
 
 		// eslint-disable-next-line react-hooks/exhaustive-deps