From 6e3789097f6371606b8e53fa37520d7932f2bf1c Mon Sep 17 00:00:00 2001 From: Kev Date: Thu, 20 Nov 2025 12:12:58 -0500 Subject: [PATCH] fix(tracemetrics): Add 'new query' for save-as behaviour Closes LOGS-511 --- .../explore/logs/useSaveAsItems.spec.tsx | 83 +++++++- .../app/views/explore/logs/useSaveAsItems.tsx | 15 +- .../metrics/useSaveAsMetricItems.spec.tsx | 183 ++++++++++++++++++ .../explore/metrics/useSaveAsMetricItems.tsx | 22 ++- 4 files changed, 287 insertions(+), 16 deletions(-) create mode 100644 static/app/views/explore/metrics/useSaveAsMetricItems.spec.tsx diff --git a/static/app/views/explore/logs/useSaveAsItems.spec.tsx b/static/app/views/explore/logs/useSaveAsItems.spec.tsx index 32f11880569a25..195b2280d3ba09 100644 --- a/static/app/views/explore/logs/useSaveAsItems.spec.tsx +++ b/static/app/views/explore/logs/useSaveAsItems.spec.tsx @@ -103,7 +103,7 @@ describe('useSaveAsItems', () => { }); }); - it('should open save query modal when save as query is clicked', () => { + it('should open save query modal when save as new query is clicked', () => { const {result} = renderHook( () => useSaveAsItems({ @@ -132,6 +132,87 @@ describe('useSaveAsItems', () => { }); }); + it('should show both existing and new query options when saved query exists', async () => { + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/explore/saved/test-query-id/`, + body: { + id: 'test-query-id', + name: 'Test Query', + isPrebuilt: false, + query: [{}], + dateAdded: '2024-01-01T00:00:00.000Z', + dateUpdated: '2024-01-01T00:00:00.000Z', + interval: '5m', + lastVisited: '2024-01-01T00:00:00.000Z', + position: null, + projects: [1], + dataset: 'logs', + starred: false, + }, + }); + + mockedUseLocation.mockReturnValue( + LocationFixture({ + query: { + id: 'test-query-id', + logsFields: ['timestamp', 'message'], + logsQuery: 'message:"test"', + mode: 'aggregate', + }, + }) + ); + + const {result} = renderHook( + () => + useSaveAsItems({ + visualizes: [new VisualizeFunction('count()')], + groupBys: ['message.template'], + interval: '5m', + mode: Mode.AGGREGATE, + search: new MutableSearch('message:"test"'), + sortBys: [{field: 'timestamp', kind: 'desc'}], + }), + {wrapper: createWrapper()} + ); + + await waitFor(() => { + expect(result.current.some(item => item.key === 'update-query')).toBe(true); + }); + + const saveAsItems = result.current; + expect(saveAsItems.some(item => item.key === 'save-query')).toBe(true); + }); + + it('should show only new query option when no saved query exists', () => { + mockedUseLocation.mockReturnValue( + LocationFixture({ + query: { + logsFields: ['timestamp', 'message'], + logsQuery: 'message:"test"', + mode: 'aggregate', + }, + }) + ); + + const {result} = renderHook( + () => + useSaveAsItems({ + visualizes: [new VisualizeFunction('count()')], + groupBys: ['message.template'], + interval: '5m', + mode: Mode.AGGREGATE, + search: new MutableSearch('message:"test"'), + sortBys: [{field: 'timestamp', kind: 'desc'}], + }), + {wrapper: createWrapper()} + ); + + const saveAsItems = result.current; + + expect(saveAsItems.some(item => item.key === 'update-query')).toBe(false); + expect(saveAsItems.some(item => item.key === 'save-query')).toBe(true); + }); + it('should call saveQuery with correct parameters when modal saves', async () => { const {result} = renderHook( () => diff --git a/static/app/views/explore/logs/useSaveAsItems.tsx b/static/app/views/explore/logs/useSaveAsItems.tsx index f51ee55c3aeab5..e51e5935f3d26d 100644 --- a/static/app/views/explore/logs/useSaveAsItems.tsx +++ b/static/app/views/explore/logs/useSaveAsItems.tsx @@ -77,9 +77,10 @@ export function useSaveAsItems({ ); const saveAsQuery = useMemo(() => { - // Show "Existing Query" if we have a non-prebuilt saved query, otherwise "A New Query" + const items = []; + if (defined(id) && savedQuery?.isPrebuilt === false) { - return { + items.push({ key: 'update-query', textValue: t('Existing Query'), label: {t('Existing Query')}, @@ -98,10 +99,10 @@ export function useSaveAsItems({ Sentry.captureException(error); } }, - }; + }); } - return { + items.push({ key: 'save-query', label: {t('A New Query')}, textValue: t('A New Query'), @@ -119,7 +120,9 @@ export function useSaveAsItems({ traceItemDataset: TraceItemDataset.LOGS, }); }, - }; + }); + + return items; }, [id, savedQuery?.isPrebuilt, updateQuery, saveQuery, organization]); const saveAsAlert = useMemo(() => { @@ -235,7 +238,7 @@ export function useSaveAsItems({ return useMemo(() => { const saveAs = []; if (isLogsEnabled(organization)) { - saveAs.push(saveAsQuery); + saveAs.push(...saveAsQuery); saveAs.push(saveAsAlert); saveAs.push(saveAsDashboard); } diff --git a/static/app/views/explore/metrics/useSaveAsMetricItems.spec.tsx b/static/app/views/explore/metrics/useSaveAsMetricItems.spec.tsx new file mode 100644 index 00000000000000..03f8667d566e02 --- /dev/null +++ b/static/app/views/explore/metrics/useSaveAsMetricItems.spec.tsx @@ -0,0 +1,183 @@ +import {LocationFixture} from 'sentry-fixture/locationFixture'; +import {OrganizationFixture} from 'sentry-fixture/organization'; +import {ProjectFixture} from 'sentry-fixture/project'; + +import {makeTestQueryClient} from 'sentry-test/queryClient'; +import {renderHook, waitFor} from 'sentry-test/reactTestingLibrary'; + +import * as modal from 'sentry/actionCreators/modal'; +import ProjectsStore from 'sentry/stores/projectsStore'; +import {QueryClientProvider} from 'sentry/utils/queryClient'; +import {useLocation} from 'sentry/utils/useLocation'; +import {useNavigate} from 'sentry/utils/useNavigate'; +import {MockMetricQueryParamsContext} from 'sentry/views/explore/metrics/hooks/testUtils'; +import {useSaveAsMetricItems} from 'sentry/views/explore/metrics/useSaveAsMetricItems'; +import {OrganizationContext} from 'sentry/views/organizationContext'; + +jest.mock('sentry/utils/useLocation'); +jest.mock('sentry/utils/useNavigate'); +jest.mock('sentry/actionCreators/modal'); + +const mockedUseLocation = jest.mocked(useLocation); +const mockUseNavigate = jest.mocked(useNavigate); +const mockOpenSaveQueryModal = jest.mocked(modal.openSaveQueryModal); + +describe('useSaveAsMetricItems', () => { + const organization = OrganizationFixture({ + features: ['tracemetrics-enabled', 'tracemetrics-saved-queries'], + }); + const project = ProjectFixture({id: '1'}); + const queryClient = makeTestQueryClient(); + ProjectsStore.loadInitialData([project]); + + function createWrapper() { + return function ({children}: {children?: React.ReactNode}) { + return ( + + + {children} + + + ); + }; + } + + beforeEach(() => { + jest.resetAllMocks(); + MockApiClient.clearMockResponses(); + queryClient.clear(); + + mockedUseLocation.mockReturnValue( + LocationFixture({ + query: { + interval: '5m', + }, + }) + ); + mockUseNavigate.mockReturnValue(jest.fn()); + + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/explore/saved/`, + method: 'POST', + body: {id: 'new-query-id', name: 'Test Query'}, + }); + }); + + it('should open save query modal when save as new query is clicked', () => { + const {result} = renderHook( + () => + useSaveAsMetricItems({ + interval: '5m', + }), + {wrapper: createWrapper()} + ); + + const saveAsItems = result.current; + const saveAsQuery = saveAsItems.find(item => item.key === 'save-query') as { + onAction: () => void; + }; + + saveAsQuery?.onAction?.(); + + expect(mockOpenSaveQueryModal).toHaveBeenCalledWith({ + organization, + saveQuery: expect.any(Function), + source: 'table', + traceItemDataset: 'tracemetrics', + }); + }); + + it('should show both existing and new query options when saved query exists', async () => { + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/explore/saved/test-query-id/`, + body: { + id: 'test-query-id', + name: 'Test Metrics Query', + isPrebuilt: false, + query: [{}], + dateAdded: '2024-01-01T00:00:00.000Z', + dateUpdated: '2024-01-01T00:00:00.000Z', + interval: '5m', + lastVisited: '2024-01-01T00:00:00.000Z', + position: null, + projects: [1], + dataset: 'tracemetrics', + starred: false, + }, + }); + + mockedUseLocation.mockReturnValue( + LocationFixture({ + query: { + id: 'test-query-id', + interval: '5m', + }, + }) + ); + + const {result} = renderHook( + () => + useSaveAsMetricItems({ + interval: '5m', + }), + {wrapper: createWrapper()} + ); + + await waitFor(() => { + expect(result.current.some(item => item.key === 'update-query')).toBe(true); + }); + + const saveAsItems = result.current; + expect(saveAsItems.some(item => item.key === 'save-query')).toBe(true); + }); + + it('should show only new query option when no saved query exists', () => { + mockedUseLocation.mockReturnValue( + LocationFixture({ + query: { + interval: '5m', + }, + }) + ); + + const {result} = renderHook( + () => + useSaveAsMetricItems({ + interval: '5m', + }), + {wrapper: createWrapper()} + ); + + const saveAsItems = result.current; + + expect(saveAsItems.some(item => item.key === 'update-query')).toBe(false); + expect(saveAsItems.some(item => item.key === 'save-query')).toBe(true); + }); + + it('should return empty array when metrics saved queries UI is not enabled', () => { + const orgWithoutFeature = OrganizationFixture({ + features: [], + }); + + const {result} = renderHook( + () => + useSaveAsMetricItems({ + interval: '5m', + }), + { + wrapper: function ({children}: {children?: React.ReactNode}) { + return ( + + + {children} + + + ); + }, + } + ); + + const saveAsItems = result.current; + expect(saveAsItems).toEqual([]); + }); +}); diff --git a/static/app/views/explore/metrics/useSaveAsMetricItems.tsx b/static/app/views/explore/metrics/useSaveAsMetricItems.tsx index 196587508aa9f4..bdfb04a3c97299 100644 --- a/static/app/views/explore/metrics/useSaveAsMetricItems.tsx +++ b/static/app/views/explore/metrics/useSaveAsMetricItems.tsx @@ -7,7 +7,6 @@ import { addSuccessMessage, } from 'sentry/actionCreators/indicator'; import {openSaveQueryModal} from 'sentry/actionCreators/modal'; -import type {MenuItemProps} from 'sentry/components/dropdownMenu'; import {t} from 'sentry/locale'; import {defined} from 'sentry/utils'; import {trackAnalytics} from 'sentry/utils/analytics'; @@ -31,12 +30,15 @@ export function useSaveAsMetricItems(_options: UseSaveAsMetricItemsOptions) { const id = getIdFromLocation(location); const {data: savedQuery} = useGetSavedQuery(id); - const saveAsQuery = useMemo(() => { + const saveAsItems = useMemo(() => { if (!canUseMetricsSavedQueriesUI(organization)) { - return null; + return []; } + + const items = []; + if (defined(id) && savedQuery?.isPrebuilt === false) { - return { + items.push({ key: 'update-query', textValue: t('Existing Query'), label: {t('Existing Query')}, @@ -55,10 +57,10 @@ export function useSaveAsMetricItems(_options: UseSaveAsMetricItemsOptions) { Sentry.captureException(error); } }, - }; + }); } - return { + items.push({ key: 'save-query', label: {t('A New Query')}, textValue: t('A New Query'), @@ -76,7 +78,9 @@ export function useSaveAsMetricItems(_options: UseSaveAsMetricItemsOptions) { traceItemDataset: TraceItemDataset.TRACEMETRICS, }); }, - }; + }); + + return items; }, [id, savedQuery?.isPrebuilt, updateQuery, saveQuery, organization]); // TODO: Implement alert functionality when organizations:tracemetrics-alerts flag is enabled @@ -84,6 +88,6 @@ export function useSaveAsMetricItems(_options: UseSaveAsMetricItemsOptions) { // TODO: Implement dashboard functionality when organizations:tracemetrics-dashboards flag is enabled return useMemo(() => { - return [saveAsQuery].filter(Boolean) as MenuItemProps[]; - }, [saveAsQuery]); + return saveAsItems; + }, [saveAsItems]); }