From 727a039eb90fd25ecd25975d0d3306c9dc9cbfc6 Mon Sep 17 00:00:00 2001 From: Amlan Kumar Nandy <45410599+amlannandy@users.noreply.github.com> Date: Fri, 9 May 2025 11:34:32 +0700 Subject: [PATCH] fix: fix 'open in explorer' functionality in metrics explorer (#7873) --- .../MetricDetails/AllAttributes.tsx | 47 +++++++---- .../MetricDetails/MetricDetails.styles.scss | 5 ++ .../MetricDetails/MetricDetails.tsx | 78 ++++++++++-------- .../__tests__/AllAttributes.test.tsx | 82 +++++++++++++++++++ .../__tests__/MetricDetails.test.tsx | 45 +++++++++- 5 files changed, 204 insertions(+), 53 deletions(-) create mode 100644 frontend/src/container/MetricsExplorer/MetricDetails/__tests__/AllAttributes.test.tsx diff --git a/frontend/src/container/MetricsExplorer/MetricDetails/AllAttributes.tsx b/frontend/src/container/MetricsExplorer/MetricDetails/AllAttributes.tsx index b300c28791..151182ff76 100644 --- a/frontend/src/container/MetricsExplorer/MetricDetails/AllAttributes.tsx +++ b/frontend/src/container/MetricsExplorer/MetricDetails/AllAttributes.tsx @@ -3,28 +3,40 @@ import { ColumnsType } from 'antd/es/table'; import { ResizeTable } from 'components/ResizeTable'; import { DataType } from 'container/LogDetailedView/TableView'; import { Search } from 'lucide-react'; -import { useMemo, useState } from 'react'; +import { useCallback, useMemo, useState } from 'react'; import { AllAttributesProps } from './types'; +import { useHandleExplorerTabChange } from '../../../hooks/useHandleExplorerTabChange'; +import { getMetricDetailsQuery } from './utils'; +import ROUTES from '../../../constants/routes'; +import { PANEL_TYPES } from '../../../constants/queryBuilder'; -function AllAttributes({ attributes }: AllAttributesProps): JSX.Element { +function AllAttributes({ + metricName, + attributes, +}: AllAttributesProps): JSX.Element { const [searchString, setSearchString] = useState(''); const [activeKey, setActiveKey] = useState( 'all-attributes', ); - // const { safeNavigate } = useSafeNavigate(); + const { handleExplorerTabChange } = useHandleExplorerTabChange(); - // const goToMetricsExploreWithAppliedAttribute = useCallback( - // (key: string, value: string) => { - // const compositeQuery = getMetricDetailsQuery(metricName, { key, value }); - // const encodedCompositeQuery = JSON.stringify(compositeQuery); - // safeNavigate( - // `${ROUTES.METRICS_EXPLORER_EXPLORER}?compositeQuery=${encodedCompositeQuery}`, - // ); - // }, - // [metricName, safeNavigate], - // ); + const goToMetricsExploreWithAppliedAttribute = useCallback( + (key: string, value: string) => { + const compositeQuery = getMetricDetailsQuery(metricName, { key, value }); + handleExplorerTabChange( + PANEL_TYPES.TIME_SERIES, + { + query: compositeQuery, + name: metricName, + id: metricName, + }, + ROUTES.METRICS_EXPLORER_EXPLORER, + ); + }, + [metricName, handleExplorerTabChange], + ); const filteredAttributes = useMemo( () => @@ -81,10 +93,9 @@ function AllAttributes({ attributes }: AllAttributesProps): JSX.Element { @@ -93,7 +104,7 @@ function AllAttributes({ attributes }: AllAttributesProps): JSX.Element { ), }, ], - [], + [goToMetricsExploreWithAppliedAttribute], ); const items = useMemo( diff --git a/frontend/src/container/MetricsExplorer/MetricDetails/MetricDetails.styles.scss b/frontend/src/container/MetricsExplorer/MetricDetails/MetricDetails.styles.scss index 82fd4b77d0..79811fd9d0 100644 --- a/frontend/src/container/MetricsExplorer/MetricDetails/MetricDetails.styles.scss +++ b/frontend/src/container/MetricsExplorer/MetricDetails/MetricDetails.styles.scss @@ -14,6 +14,11 @@ } } + .metric-details-header-buttons { + display: flex; + gap: 8px; + } + .ant-btn { display: flex; align-items: center; diff --git a/frontend/src/container/MetricsExplorer/MetricDetails/MetricDetails.tsx b/frontend/src/container/MetricsExplorer/MetricDetails/MetricDetails.tsx index c0fc02761a..bb5adfbab0 100644 --- a/frontend/src/container/MetricsExplorer/MetricDetails/MetricDetails.tsx +++ b/frontend/src/container/MetricsExplorer/MetricDetails/MetricDetails.tsx @@ -13,8 +13,8 @@ import { } from 'antd'; import { useGetMetricDetails } from 'hooks/metricsExplorer/useGetMetricDetails'; import { useIsDarkMode } from 'hooks/useDarkMode'; -import { Compass, X } from 'lucide-react'; -import { useMemo } from 'react'; +import { Compass, Crosshair, X } from 'lucide-react'; +import { useCallback, useMemo } from 'react'; import { isInspectEnabled } from '../Inspect/utils'; import { formatNumberIntoHumanReadableFormat } from '../Summary/utils'; @@ -25,7 +25,11 @@ import { MetricDetailsProps } from './types'; import { formatNumberToCompactFormat, formatTimestampToReadableDate, + getMetricDetailsQuery, } from './utils'; +import { PANEL_TYPES } from '../../../constants/queryBuilder'; +import ROUTES from '../../../constants/routes'; +import { useHandleExplorerTabChange } from '../../../hooks/useHandleExplorerTabChange'; function MetricDetails({ onClose, @@ -34,7 +38,7 @@ function MetricDetails({ openInspectModal, }: MetricDetailsProps): JSX.Element { const isDarkMode = useIsDarkMode(); - // const { safeNavigate } = useSafeNavigate(); + const { handleExplorerTabChange } = useHandleExplorerTabChange(); const { data, @@ -76,15 +80,20 @@ function MetricDetails({ ); }, [metric]); - // const goToMetricsExplorerwithSelectedMetric = useCallback(() => { - // if (metricName) { - // const compositeQuery = getMetricDetailsQuery(metricName); - // const encodedCompositeQuery = JSON.stringify(compositeQuery); - // safeNavigate( - // `${ROUTES.METRICS_EXPLORER_EXPLORER}?compositeQuery=${encodedCompositeQuery}`, - // ); - // } - // }, [metricName, safeNavigate]); + const goToMetricsExplorerwithSelectedMetric = useCallback(() => { + if (metricName) { + const compositeQuery = getMetricDetailsQuery(metricName); + handleExplorerTabChange( + PANEL_TYPES.TIME_SERIES, + { + query: compositeQuery, + name: metricName, + id: metricName, + }, + ROUTES.METRICS_EXPLORER_EXPLORER, + ); + } + }, [metricName, handleExplorerTabChange]); const isMetricDetailsError = metricDetailsError || !metric; @@ -97,29 +106,30 @@ function MetricDetails({ {metric?.name} - {/* TODO: Enable this once we have fixed the redirect issue */} - {/* */} - {/* Show the based on the feature flag. Will remove before releasing the feature */} - {showInspectFeature && ( +
+ {/* Show the based on the feature flag. Will remove before releasing the feature */} + {showInspectFeature && ( +
} placement="right" diff --git a/frontend/src/container/MetricsExplorer/MetricDetails/__tests__/AllAttributes.test.tsx b/frontend/src/container/MetricsExplorer/MetricDetails/__tests__/AllAttributes.test.tsx new file mode 100644 index 0000000000..0a3e0629e9 --- /dev/null +++ b/frontend/src/container/MetricsExplorer/MetricDetails/__tests__/AllAttributes.test.tsx @@ -0,0 +1,82 @@ +import { fireEvent, render, screen } from '@testing-library/react'; + +import AllAttributes from '../AllAttributes'; +import { MetricDetailsAttribute } from '../../../../api/metricsExplorer/getMetricDetails'; +import ROUTES from '../../../../constants/routes'; +import * as useHandleExplorerTabChange from 'hooks/useHandleExplorerTabChange'; + +jest.mock('react-router-dom', () => ({ + ...jest.requireActual('react-router-dom'), + useLocation: (): { pathname: string } => ({ + pathname: `${ROUTES.METRICS_EXPLORER}`, + }), +})); +const mockHandleExplorerTabChange = jest.fn(); +jest + .spyOn(useHandleExplorerTabChange, 'useHandleExplorerTabChange') + .mockReturnValue({ + handleExplorerTabChange: mockHandleExplorerTabChange, + }); + +const mockMetricName = 'test-metric'; +const mockAttributes: MetricDetailsAttribute[] = [ + { + key: 'attribute1', + value: ['value1', 'value2'], + valueCount: 2, + }, + { + key: 'attribute2', + value: ['value3'], + valueCount: 1, + }, +]; + +describe('AllAttributes', () => { + it('renders attributes section with title', () => { + render( + , + ); + + expect(screen.getByText('All Attributes')).toBeInTheDocument(); + }); + + it('renders all attribute keys and values', () => { + render( + , + ); + + // Check attribute keys are rendered + expect(screen.getByText('attribute1')).toBeInTheDocument(); + expect(screen.getByText('attribute2')).toBeInTheDocument(); + + // Check attribute values are rendered + expect(screen.getByText('value1')).toBeInTheDocument(); + expect(screen.getByText('value2')).toBeInTheDocument(); + expect(screen.getByText('value3')).toBeInTheDocument(); + }); + + it('renders value counts correctly', () => { + render( + , + ); + + expect(screen.getByText('2')).toBeInTheDocument(); // For attribute1 + expect(screen.getByText('1')).toBeInTheDocument(); // For attribute2 + }); + + it('handles empty attributes array', () => { + render(); + + expect(screen.getByText('All Attributes')).toBeInTheDocument(); + expect(screen.queryByText('No data')).toBeInTheDocument(); + }); + + it('clicking on an attribute value opens the explorer with the attribute filter applied', () => { + render( + , + ); + fireEvent.click(screen.getByText('value1')); + expect(mockHandleExplorerTabChange).toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/container/MetricsExplorer/MetricDetails/__tests__/MetricDetails.test.tsx b/frontend/src/container/MetricsExplorer/MetricDetails/__tests__/MetricDetails.test.tsx index 5eb7e867ea..0fd0fdf29e 100644 --- a/frontend/src/container/MetricsExplorer/MetricDetails/__tests__/MetricDetails.test.tsx +++ b/frontend/src/container/MetricsExplorer/MetricDetails/__tests__/MetricDetails.test.tsx @@ -1,9 +1,10 @@ -import { render, screen } from '@testing-library/react'; +import { fireEvent, render, screen } from '@testing-library/react'; import { MetricDetails } from 'api/metricsExplorer/getMetricDetails'; import { MetricType } from 'api/metricsExplorer/getMetricsList'; import ROUTES from 'constants/routes'; import * as useGetMetricDetails from 'hooks/metricsExplorer/useGetMetricDetails'; import * as useUpdateMetricMetadata from 'hooks/metricsExplorer/useUpdateMetricMetadata'; +import * as useHandleExplorerTabChange from 'hooks/useHandleExplorerTabChange'; import MetricDetailsView from '../MetricDetails'; @@ -61,6 +62,13 @@ jest.spyOn(useUpdateMetricMetadata, 'useUpdateMetricMetadata').mockReturnValue({ error: null, } as any); +const mockHandleExplorerTabChange = jest.fn(); +jest + .spyOn(useHandleExplorerTabChange, 'useHandleExplorerTabChange') + .mockReturnValue({ + handleExplorerTabChange: mockHandleExplorerTabChange, + }); + jest.mock('react-router-dom', () => ({ ...jest.requireActual('react-router-dom'), useLocation: (): { pathname: string } => ({ @@ -90,6 +98,41 @@ describe('MetricDetails', () => { expect(screen.getByText(`${mockMetricData.unit}`)).toBeInTheDocument(); }); + it('renders the "open in explorer" and "inspect" buttons', () => { + jest.spyOn(useGetMetricDetails, 'useGetMetricDetails').mockReturnValueOnce({ + ...mockUseGetMetricDetailsData, + data: { + payload: { + data: { + ...mockMetricData, + metadata: { + ...mockMetricData.metadata, + metric_type: MetricType.GAUGE, + }, + }, + }, + }, + } as any); + render( + , + ); + + expect(screen.getByTestId('open-in-explorer-button')).toBeInTheDocument(); + expect(screen.getByTestId('inspect-metric-button')).toBeInTheDocument(); + + fireEvent.click(screen.getByTestId('open-in-explorer-button')); + expect(mockHandleExplorerTabChange).toHaveBeenCalled(); + + fireEvent.click(screen.getByTestId('inspect-metric-button')); + expect(mockOpenInspectModal).toHaveBeenCalled(); + }); + it('should render error state when metric details are not found', () => { jest.spyOn(useGetMetricDetails, 'useGetMetricDetails').mockReturnValue({ ...mockUseGetMetricDetailsData,