fix(logs): preserve active viewKey in log-details filter/group/replace (#12757)

#### Description

- Fixes the issue where applying filter for value / filter out (also
group by and replace filter) from the log details drawer was removing
the added columns from the table.
- As part of this fix, we rename `id` and `name` fields in
explorerTabChange input type. Now it reads `viewName` and `viewKey` and
avoids confusion
- Now consumers of explorerTabChange need to send viewName and viewKey
only if needed. As now this is an optional field

#### Screenshots/Recording

Before


https://github.com/user-attachments/assets/4c6145e3-4979-4386-9230-f74eb00721c8



After


https://github.com/user-attachments/assets/52bc3083-40a2-4345-aebd-63770882b06c



#### Issues closed by this PR

Closes https://github.com/SigNoz/engineering-pod/issues/6012

#### Additional Information

- same issue was present in the old drawer (group by, replace filter)
and the metrics explorer detail (passed the metric name)..both fixed
here. metrics never surfaced as a bug since it opens in time series with
no columns to collapse
- filter for/out in the old drawer was never affected, it uses a
different add to query path

frontend/src/components/ExplorerCard/MenuItemGenerator.tsx

@@ -57,8 +57,8 @@ function MenuItemGenerator({
 
 			handleExplorerTabChange(currentPanelType, {
 				query,
-				name,
-				id,
+				viewName: name,
+				viewKey: id,
 			});
 		},
 		[viewData, handleExplorerTabChange],

frontend/src/components/QuickFilters/FilterRenderers/Checkbox/checkboxFilterQuery.test.ts

@@ -1,249 +0,0 @@
-import { convertFiltersToExpressionWithExistingQuery } from 'components/QueryBuilderV2/utils';
-import {
-	FiltersType,
-	IQuickFiltersConfig,
-	QuickFiltersSource,
-} from 'components/QuickFilters/types';
-import { Query, TagFilterItem } from 'types/api/queryBuilder/queryBuilderData';
-import { DataSource } from 'types/common/queryBuilder';
-
-import { applyCheckboxToggle } from './checkboxFilterQuery';
-import { CheckedState } from '../../types';
-import { SectionType } from './v2/itemRules';
-
-const ATTRIBUTE_KEY = 'k8s.cluster.name';
-
-const filter = {
-	type: FiltersType.CHECKBOX,
-	title: 'Cluster',
-	attributeKey: {
-		key: ATTRIBUTE_KEY,
-		dataType: 'string',
-		type: 'tag',
-		isColumn: false,
-	},
-	dataSource: DataSource.METRICS,
-	defaultOpen: true,
-} as unknown as IQuickFiltersConfig;
-
-function makeQuery(expression: string, items: TagFilterItem[]): Query {
-	return {
-		builder: {
-			queryData: [{ filter: { expression }, filters: { items, op: 'AND' } }],
-		},
-	} as unknown as Query;
-}
-
-/**
- * Quick filters dispatch through the URL, and `useGetCompositeQueryParam` merges
- * `filters.items` into `filter.expression` on the way back in — a clause left in
- * the expression resurrects a filter the user just removed. Every assertion here
- * runs through that round-trip.
- */
-function roundTrip(query: Query): Query {
-	const queryData = query.builder.queryData[0];
-	const converted = convertFiltersToExpressionWithExistingQuery(
-		queryData.filters || { items: [], op: 'AND' },
-		queryData.filter?.expression || '',
-	);
-	return makeQuery(converted.filter.expression, converted.filters.items);
-}
-
-function toggle(
-	query: Query,
-	{
-		value,
-		checked,
-		previousState,
-		sectionType,
-		isOnlyOrAllClicked = false,
-		attributeValues = ['A', 'B', 'C'],
-	}: {
-		value: string;
-		checked: boolean;
-		previousState?: CheckedState;
-		sectionType?: SectionType;
-		isOnlyOrAllClicked?: boolean;
-		attributeValues?: string[];
-	},
-): Query {
-	return roundTrip(
-		applyCheckboxToggle({
-			currentQuery: query,
-			activeQueryIndex: 0,
-			filter,
-			source: QuickFiltersSource.INFRA_MONITORING,
-			attributeValues,
-			value,
-			checked,
-			isOnlyOrAllClicked,
-			previousState,
-			sectionType,
-		}),
-	);
-}
-
-const expressionOf = (query: Query): string =>
-	query.builder.queryData[0].filter?.expression ?? '';
-
-const itemsOf = (query: Query): TagFilterItem[] =>
-	query.builder.queryData[0].filters?.items ?? [];
-
-describe('applyCheckboxToggle expression sync', () => {
-	it('unchecking a value excludes it, re-checking it clears the filter', () => {
-		let query = makeQuery('', []);
-
-		query = toggle(query, {
-			value: 'A',
-			checked: false,
-			previousState: 'checked',
-			sectionType: SectionType.SELECTED,
-		});
-		expect(expressionOf(query)).toBe(`${ATTRIBUTE_KEY} not in ['A']`);
-
-		query = toggle(query, {
-			value: 'A',
-			checked: true,
-			previousState: 'unchecked',
-			sectionType: SectionType.SELECTED,
-		});
-		expect(expressionOf(query)).toBe('');
-		expect(itemsOf(query)).toHaveLength(0);
-	});
-
-	it('toggling the same value repeatedly stays a two-state cycle', () => {
-		let query = makeQuery('', []);
-
-		for (let i = 0; i < 3; i += 1) {
-			query = toggle(query, {
-				value: 'A',
-				checked: false,
-				previousState: 'checked',
-				sectionType: SectionType.SELECTED,
-			});
-			expect(expressionOf(query)).toBe(`${ATTRIBUTE_KEY} not in ['A']`);
-
-			query = toggle(query, {
-				value: 'A',
-				checked: true,
-				previousState: 'unchecked',
-				sectionType: SectionType.SELECTED,
-			});
-			expect(expressionOf(query)).toBe('');
-		}
-	});
-
-	it('re-including one of several excluded values leaves the rest excluded', () => {
-		let query = makeQuery(`${ATTRIBUTE_KEY} not in ['A', 'B']`, []);
-		query = roundTrip(query);
-
-		query = toggle(query, {
-			value: 'A',
-			checked: true,
-			previousState: 'unchecked',
-			sectionType: SectionType.SELECTED,
-		});
-
-		expect(expressionOf(query)).toBe(`${ATTRIBUTE_KEY} not in ['B']`);
-	});
-
-	it('unchecking the last selected value clears the filter', () => {
-		let query = makeQuery(`${ATTRIBUTE_KEY} in ['A']`, []);
-		query = roundTrip(query);
-
-		query = toggle(query, {
-			value: 'A',
-			checked: false,
-			previousState: 'checked',
-			sectionType: SectionType.SELECTED,
-		});
-
-		expect(expressionOf(query)).toBe('');
-		expect(itemsOf(query)).toHaveLength(0);
-	});
-
-	it('unchecking one of several selected values keeps the others', () => {
-		let query = makeQuery(`${ATTRIBUTE_KEY} in ['A', 'B']`, []);
-		query = roundTrip(query);
-
-		query = toggle(query, {
-			value: 'A',
-			checked: false,
-			previousState: 'checked',
-			sectionType: SectionType.SELECTED,
-		});
-
-		expect(expressionOf(query)).toBe(`${ATTRIBUTE_KEY} in ['B']`);
-	});
-
-	it('checking a value that is not excluded narrows the filter to it', () => {
-		let query = makeQuery(`${ATTRIBUTE_KEY} not in ['A']`, []);
-		query = roundTrip(query);
-
-		query = toggle(query, {
-			value: 'C',
-			checked: true,
-			previousState: 'unchecked',
-			sectionType: SectionType.ALL_VALUES,
-		});
-
-		expect(expressionOf(query)).toBe(`${ATTRIBUTE_KEY} in ['C']`);
-	});
-
-	it('excluding a related value replaces the selection with a NOT IN clause', () => {
-		let query = makeQuery(`${ATTRIBUTE_KEY} in ['A']`, []);
-		query = roundTrip(query);
-
-		query = toggle(query, {
-			value: 'B',
-			checked: false,
-			previousState: 'checked',
-			sectionType: SectionType.RELATED,
-		});
-
-		expect(expressionOf(query)).toBe(`${ATTRIBUTE_KEY} not in ['B']`);
-	});
-
-	it('Only narrows to the clicked value and All clears the filter', () => {
-		let query = makeQuery('', []);
-
-		query = toggle(query, {
-			value: 'A',
-			checked: true,
-			isOnlyOrAllClicked: true,
-		});
-		expect(expressionOf(query)).toBe(`${ATTRIBUTE_KEY} in ['A']`);
-
-		query = toggle(query, {
-			value: 'A',
-			checked: true,
-			isOnlyOrAllClicked: true,
-		});
-		expect(expressionOf(query)).toBe('');
-	});
-
-	it('leaves clauses for other keys untouched', () => {
-		let query = makeQuery(`k8s.namespace.name = 'default'`, []);
-		query = roundTrip(query);
-
-		query = toggle(query, {
-			value: 'A',
-			checked: false,
-			previousState: 'checked',
-			sectionType: SectionType.SELECTED,
-		});
-		expect(expressionOf(query)).toContain(`k8s.namespace.name = 'default'`);
-		// consecutive clauses with no AND/OR are an implicit AND in the filter grammar
-		expect(expressionOf(query)).toMatch(
-			new RegExp(`${ATTRIBUTE_KEY} not in \\['A'\\]`, 'i'),
-		);
-
-		query = toggle(query, {
-			value: 'A',
-			checked: true,
-			previousState: 'unchecked',
-			sectionType: SectionType.SELECTED,
-		});
-		expect(expressionOf(query)).toBe(`k8s.namespace.name = 'default'`);
-	});
-});

frontend/src/components/QuickFilters/FilterRenderers/Checkbox/checkboxFilterQuery.ts

@@ -1,8 +1,5 @@
 /* eslint-disable sonarjs/no-identical-functions */
-import {
-	convertFiltersToExpressionWithExistingQuery,
-	removeKeysFromExpression,
-} from 'components/QueryBuilderV2/utils';
+import { removeKeysFromExpression } from 'components/QueryBuilderV2/utils';
 import {
 	IQuickFiltersConfig,
 	QuickFiltersSource,
@@ -197,6 +194,12 @@ export function applyCheckboxToggle({
 			(q) => !isKeyMatch(q.key?.key, filter.attributeKey.key),
 		);
 
+		if (query.filter?.expression) {
+			query.filter.expression = removeKeysFromExpression(query.filter.expression, [
+				filter.attributeKey.key,
+			]);
+		}
+
 		if (isOnlyOrAll === 'Only') {
 			const newFilterItem: TagFilterItem = {
 				id: uuid(),
@@ -264,6 +267,12 @@ export function applyCheckboxToggle({
 									}
 									return item;
 								});
+								if (query.filter?.expression) {
+									query.filter.expression = removeKeysFromExpression(
+										query.filter.expression,
+										[filter.attributeKey.key],
+									);
+								}
 							} else if (isArray(currentFilter.value)) {
 								// if we are removing some value when the running operator is IN we filter.
 								// example - key IN [value1,currentSelectedValue] becomes key IN [value1] in case of array
@@ -300,10 +309,9 @@ export function applyCheckboxToggle({
 							? currentFilter.value.includes(value)
 							: currentFilter.value === value;
 
-						// When clicking an unchecked value that is not itself excluded, the user
-						// wants to SELECT it: replace the NOT IN filter with IN [value]. A value
-						// that IS in the exclusion list falls through to the removal branch below.
-						if (previousState === 'unchecked' && checked && !isValueInFilter) {
+						// When clicking unchecked "Other" item, user wants to SELECT it
+						// Replace NOT IN filter with IN [value]
+						if (previousState === 'unchecked' && checked) {
 							const newFilter: TagFilterItem = {
 								id: uuid(),
 								op: getOperatorValue(OPERATORS.IN),
@@ -316,6 +324,12 @@ export function applyCheckboxToggle({
 								}
 								return item;
 							});
+							if (query.filter?.expression) {
+								query.filter.expression = removeKeysFromExpression(
+									query.filter.expression,
+									[filter.attributeKey.key],
+								);
+							}
 						} else if (!checked || !isValueInFilter) {
 							// Add to NOT IN when:
 							// - checked=false (user explicitly unchecked to exclude)
@@ -355,6 +369,12 @@ export function applyCheckboxToggle({
 									query.filters.items = query.filters.items.filter(
 										(item) => !isKeyMatch(item.key?.key, filter.attributeKey.key),
 									);
+									if (query.filter?.expression) {
+										query.filter.expression = removeKeysFromExpression(
+											query.filter.expression,
+											[filter.attributeKey.key],
+										);
+									}
 								} else {
 									query.filters.items = query.filters.items.map((item) => {
 										if (isKeyMatch(item.key?.key, filter.attributeKey.key)) {
@@ -364,6 +384,16 @@ export function applyCheckboxToggle({
 									});
 								}
 							} else {
+								const newFilter = {
+									...currentFilter,
+									value: currentFilter.value === value ? null : currentFilter.value,
+								};
+								if (newFilter.value === null && query.filter?.expression) {
+									query.filter.expression = removeKeysFromExpression(
+										query.filter.expression,
+										[filter.attributeKey.key],
+									);
+								}
 								query.filters.items = query.filters.items.filter(
 									(item) => !isKeyMatch(item.key?.key, filter.attributeKey.key),
 								);
@@ -426,17 +456,6 @@ export function applyCheckboxToggle({
 		}
 	}
 
-	if (query) {
-		const synced = convertFiltersToExpressionWithExistingQuery(
-			query.filters ?? { items: [], op: 'AND' },
-			removeKeysFromExpression(query.filter?.expression ?? '', [
-				filter.attributeKey.key,
-			]),
-		);
-		query.filter = synced.filter;
-		query.filters = synced.filters;
-	}
-
 	return {
 		...currentQuery,
 		builder: {

frontend/src/container/ExplorerOptions/ExplorerOptions.tsx

@@ -452,15 +452,15 @@ function ExplorerOptions({
 			if (handleChangeSelectedView) {
 				handleChangeSelectedView(panelTypeToExplorerView[currentPanelType], {
 					query,
-					name,
-					id,
+					viewName: name,
+					viewKey: id,
 				});
 			} else {
 				// to remove this after traces cleanup
 				handleExplorerTabChange(currentPanelType, {
 					query,
-					name,
-					id,
+					viewName: name,
+					viewKey: id,
 				});
 			}
 		},

frontend/src/container/Home/SavedViews/SavedViews.tsx

@@ -115,8 +115,8 @@ export default function SavedViews({
 				currentPanelType,
 				{
 					query,
-					name,
-					id,
+					viewName: name,
+					viewKey: id,
 				},
 				SOURCEPAGE_VS_ROUTES[selectedEntity],
 			);

frontend/src/container/LogDetailedView/BodyTitleRenderer.tsx

@@ -16,7 +16,6 @@ import {
 } from 'constants/antlrQueryConstants';
 import { FeatureKeys } from 'constants/features';
 import { useActiveLog } from 'hooks/logs/useActiveLog';
-import { useGetSavedViewParams } from 'hooks/saveViews/useGetSavedViewParams';
 import { useQueryBuilder } from 'hooks/queryBuilder/useQueryBuilder';
 import { ICurrentQueryData } from 'hooks/useHandleExplorerTabChange';
 import { useNotifications } from 'hooks/useNotifications';
@@ -49,7 +48,6 @@ function BodyTitleRenderer({
 	const { featureFlags } = useAppContext();
 	const [, setCopy] = useCopyToClipboard();
 	const { notifications } = useNotifications();
-	const { viewName } = useGetSavedViewParams();
 
 	const cleanedNodeKey = removeObjectFromString(nodeKey);
 	const isBodyJsonQueryEnabled =
@@ -123,8 +121,6 @@ function BodyTitleRenderer({
 		);
 
 		const queryData: ICurrentQueryData = {
-			name: viewName,
-			id: updatedQuery.id,
 			query: updatedQuery,
 		};
 
@@ -137,7 +133,6 @@ function BodyTitleRenderer({
 		stagedQuery,
 		updateQueriesData,
 		value,
-		viewName,
 	]);
 
 	const onClickHandler = (key: string): void => {

frontend/src/container/LogDetailedView/TableView/TableViewActions.tsx

@@ -12,7 +12,6 @@ import ROUTES from 'constants/routes';
 import { ChangeViewFunctionType } from 'container/ExplorerOptions/types';
 import { RESTRICTED_SELECTED_FIELDS } from 'container/LogsFilters/config';
 import { MetricsType } from 'container/MetricsApplication/constant';
-import { useGetSavedViewParams } from 'hooks/saveViews/useGetSavedViewParams';
 import { useQueryBuilder } from 'hooks/queryBuilder/useQueryBuilder';
 import { ICurrentQueryData } from 'hooks/useHandleExplorerTabChange';
 import {
@@ -140,7 +139,6 @@ export default function TableViewActions(
 
 	const { pathname } = useLocation();
 	const { stagedQuery, updateQueriesData } = useQueryBuilder();
-	const { viewName } = useGetSavedViewParams();
 	const { dataType, logType: fieldType } = getFieldAttributes(record.field);
 
 	// there is no option for where clause in old logs explorer and live logs page or infra monitoring
@@ -201,8 +199,6 @@ export default function TableViewActions(
 		);
 
 		const queryData: ICurrentQueryData = {
-			name: viewName,
-			id: updatedQuery.id,
 			query: updatedQuery,
 		};
 
@@ -214,7 +210,6 @@ export default function TableViewActions(
 		fieldType,
 		dataType,
 		handleChangeSelectedView,
-		viewName,
 	]);
 
 	const handleReplaceFilter = useCallback((): void => {
@@ -264,8 +259,6 @@ export default function TableViewActions(
 		);
 
 		const queryData: ICurrentQueryData = {
-			name: viewName,
-			id: updatedQuery.id,
 			query: updatedQuery,
 		};
 
@@ -278,7 +271,6 @@ export default function TableViewActions(
 		dataType,
 		fieldData,
 		handleChangeSelectedView,
-		viewName,
 	]);
 
 	// Memoize textToCopy computation

frontend/src/container/LogDetailedView/TableView/__test__/TableViewActions.test.tsx

@@ -272,8 +272,6 @@ describe('TableViewActions', () => {
 		expect(defaultProps.handleChangeSelectedView).toHaveBeenCalledWith(
 			ExplorerViews.TIMESERIES,
 			expect.objectContaining({
-				name: '',
-				id: 'test-query-id',
 				query: expect.objectContaining({
 					builder: expect.objectContaining({
 						queryData: expect.arrayContaining([

frontend/src/container/LogDetailedView/hooks/useLogAttributeActions.tsx

@@ -5,7 +5,6 @@ import { convertFiltersToExpression } from 'components/QueryBuilderV2/utils';
 import { FeatureKeys } from 'constants/features';
 import ROUTES from 'constants/routes';
 import { ChangeViewFunctionType } from 'container/ExplorerOptions/types';
-import { useGetSavedViewParams } from 'hooks/saveViews/useGetSavedViewParams';
 import { useQueryBuilder } from 'hooks/queryBuilder/useQueryBuilder';
 import { ICurrentQueryData } from 'hooks/useHandleExplorerTabChange';
 import { ExplorerViews } from 'pages/LogsExplorer/utils';
@@ -57,7 +56,6 @@ export function useLogAttributeActions({
 	const { pathname } = useLocation();
 	const { stagedQuery, updateQueriesData } = useQueryBuilder();
 	const { featureFlags } = useAppContext();
-	const { viewName } = useGetSavedViewParams();
 
 	const isBodyJsonQueryEnabled =
 		featureFlags?.find((flag) => flag.name === FeatureKeys.USE_JSON_BODY)
@@ -110,8 +108,6 @@ export function useLogAttributeActions({
 			);
 
 			const queryData: ICurrentQueryData = {
-				name: viewName,
-				id: updatedQuery.id,
 				query: updatedQuery,
 			};
 			handleChangeSelectedView?.(ExplorerViews.LIST, queryData);
@@ -120,7 +116,6 @@ export function useLogAttributeActions({
 			stagedQuery,
 			isBodyJsonQueryEnabled,
 			updateQueriesData,
-			viewName,
 			handleChangeSelectedView,
 			onApplyLogFilter,
 		],
@@ -147,8 +142,6 @@ export function useLogAttributeActions({
 			);
 
 			const queryData: ICurrentQueryData = {
-				name: viewName,
-				id: updatedQuery.id,
 				query: updatedQuery,
 			};
 			handleChangeSelectedView?.(ExplorerViews.TIMESERIES, queryData);
@@ -157,7 +150,6 @@ export function useLogAttributeActions({
 			stagedQuery,
 			isBodyJsonQueryEnabled,
 			updateQueriesData,
-			viewName,
 			handleChangeSelectedView,
 		],
 	);
@@ -183,8 +175,6 @@ export function useLogAttributeActions({
 			);
 
 			const queryData: ICurrentQueryData = {
-				name: viewName,
-				id: updatedQuery.id,
 				query: updatedQuery,
 			};
 			handleChangeSelectedView?.(ExplorerViews.LIST, queryData);
@@ -193,7 +183,6 @@ export function useLogAttributeActions({
 			stagedQuery,
 			isBodyJsonQueryEnabled,
 			updateQueriesData,
-			viewName,
 			handleChangeSelectedView,
 		],
 	);

frontend/src/container/MetricsExplorer/MetricDetails/AllAttributes.tsx

@@ -78,8 +78,6 @@ function AllAttributes({
 				PANEL_TYPES.TIME_SERIES,
 				{
 					query: compositeQuery,
-					name: metricName,
-					id: metricName,
 				},
 				ROUTES.METRICS_EXPLORER_EXPLORER,
 				true,
@@ -109,8 +107,6 @@ function AllAttributes({
 				PANEL_TYPES.TIME_SERIES,
 				{
 					query: compositeQuery,
-					name: metricName,
-					id: metricName,
 				},
 				ROUTES.METRICS_EXPLORER_EXPLORER,
 				true,

frontend/src/container/MetricsExplorer/MetricDetails/MetricDetails.tsx

@@ -92,8 +92,6 @@ function MetricDetails({
 				PANEL_TYPES.TIME_SERIES,
 				{
 					query: compositeQuery,
-					name: metricName,
-					id: metricName,
 				},
 				ROUTES.METRICS_EXPLORER_EXPLORER,
 				true,

frontend/src/hooks/useHandleExplorerTabChange.ts

@@ -10,8 +10,8 @@ import { useGetSavedViewParams } from './saveViews/useGetSavedViewParams';
 import { useQueryBuilder } from './queryBuilder/useQueryBuilder';
 
 export interface ICurrentQueryData {
-	name: string;
-	id: string;
+	viewName?: string;
+	viewKey?: string;
 	query: Query;
 }
 
@@ -57,6 +57,8 @@ export const useHandleExplorerTabChange = (): {
 		[currentQuery, updateAllQueriesOperators, updateQueriesData],
 	);
 
+	//TODO: this util is used not just to change explorer tab but also
+	// for changing just the query or saved view. consider renaming this.
 	const handleExplorerTabChange = useCallback(
 		(
 			type: string,
@@ -77,8 +79,8 @@ export const useHandleExplorerTabChange = (): {
 					query,
 					{
 						[QueryParams.panelTypes]: newPanelType,
-						[QueryParams.viewName]: currentQueryData?.name || viewName,
-						[QueryParams.viewKey]: currentQueryData?.id || viewKey,
+						[QueryParams.viewName]: currentQueryData?.viewName || viewName,
+						[QueryParams.viewKey]: currentQueryData?.viewKey || viewKey,
 					},
 					redirectToUrl,
 					undefined,
@@ -89,8 +91,8 @@ export const useHandleExplorerTabChange = (): {
 					query,
 					{
 						[QueryParams.panelTypes]: newPanelType,
-						[QueryParams.viewName]: currentQueryData?.name || viewName,
-						[QueryParams.viewKey]: currentQueryData?.id || viewKey,
+						[QueryParams.viewName]: currentQueryData?.viewName || viewName,
+						[QueryParams.viewKey]: currentQueryData?.viewKey || viewKey,
 					},
 					undefined,
 					undefined,

frontend/src/pages/SaveView/index.tsx

@@ -209,8 +209,8 @@ function SaveView(): JSX.Element {
 				currentPanelType,
 				{
 					query,
-					name,
-					id,
+					viewName: name,
+					viewKey: id,
 				},
 				SOURCEPAGE_VS_ROUTES[sourcepage],
 			);