diff --git a/static/app/views/dashboards/widgetBuilder/components/sortBySelector.spec.tsx b/static/app/views/dashboards/widgetBuilder/components/sortBySelector.spec.tsx index 31c51c0661c454..6200d2aba258eb 100644 --- a/static/app/views/dashboards/widgetBuilder/components/sortBySelector.spec.tsx +++ b/static/app/views/dashboards/widgetBuilder/components/sortBySelector.spec.tsx @@ -22,6 +22,7 @@ const {organization, router} = initializeOrg({ query: { displayType: 'line', fields: ['transaction.duration', 'count()', 'id'], + yAxis: ['count()', 'count_unique(transaction.duration)'], }, }, params: {}, @@ -123,4 +124,63 @@ describe('WidgetBuilderSortBySelector', function () { }) ); }); + + it('renders the correct limit options', async function () { + render( + + + + + , + {router, organization} + ); + + // default limit is 5 + expect(await screen.findByText('Limit to 5 results')).toBeInTheDocument(); + + const moreAggregatesRouter = RouterFixture({ + ...router, + location: { + ...router.location, + query: { + ...router.location.query, + yAxis: ['count()', 'count_unique(transaction.duration)', 'eps()'], + }, + }, + }); + + render( + + + + + , + {router: moreAggregatesRouter, organization} + ); + + // default limit changes to 3 since its the max limit for 3 aggregates + expect(await screen.findByText('Limit to 3 results')).toBeInTheDocument(); + }); + + it('correctly handles limit changes', async function () { + const mockNavigate = jest.fn(); + mockUseNavigate.mockReturnValue(mockNavigate); + + render( + + + + + , + {router, organization} + ); + + const limitSelector = await screen.findByText('Limit to 5 results'); + await userEvent.click(limitSelector); + await userEvent.click(await screen.findByText('Limit to 3 results')); + + expect(mockNavigate).toHaveBeenLastCalledWith( + expect.objectContaining({query: expect.objectContaining({limit: 3})}) + ); + }); }); diff --git a/static/app/views/dashboards/widgetBuilder/components/sortBySelector.tsx b/static/app/views/dashboards/widgetBuilder/components/sortBySelector.tsx index fb519389871f1b..bb8fe24c3f4a49 100644 --- a/static/app/views/dashboards/widgetBuilder/components/sortBySelector.tsx +++ b/static/app/views/dashboards/widgetBuilder/components/sortBySelector.tsx @@ -1,4 +1,4 @@ -import {Fragment, useState} from 'react'; +import {Fragment, useEffect} from 'react'; import styled from '@emotion/styled'; import SelectControl from 'sentry/components/forms/controls/selectControl'; @@ -16,7 +16,6 @@ import {SectionHeader} from 'sentry/views/dashboards/widgetBuilder/components/co import {useWidgetBuilderContext} from 'sentry/views/dashboards/widgetBuilder/contexts/widgetBuilderContext'; import {BuilderStateAction} from 'sentry/views/dashboards/widgetBuilder/hooks/useWidgetBuilderState'; import { - DEFAULT_RESULTS_LIMIT, getResultsLimit, SortDirection, } from 'sentry/views/dashboards/widgetBuilder/utils'; @@ -27,8 +26,6 @@ function WidgetBuilderSortBySelector() { const {state, dispatch} = useWidgetBuilderContext(); const widget = convertBuilderStateToWidget(state); - const [limit, setLimit] = useState(DEFAULT_RESULTS_LIMIT); - const datasetConfig = getDatasetConfig(state.dataset); let tags: TagCollection = useTags(); @@ -60,6 +57,19 @@ function WidgetBuilderSortBySelector() { widget.queries[0].aggregates.length ); + // handles when the maxLimit changes to a value less than the current selected limit + useEffect(() => { + if (!state.limit) { + return; + } + if (state.limit > maxLimit) { + dispatch({ + type: BuilderStateAction.SET_LIMIT, + payload: maxLimit, + }); + } + }, [state.limit, maxLimit, dispatch]); + function handleSortByChange(newSortBy: string, sortDirection: 'asc' | 'desc') { dispatch({ type: BuilderStateAction.SET_SORT, @@ -83,7 +93,7 @@ function WidgetBuilderSortBySelector() { flexibleControlStateSize stacked > - {isTimeseriesChart && limit && ( + {isTimeseriesChart && state.limit && ( ) => { - // TODO: implement this when limit is implemented in widget builder state - setLimit(option.value); + dispatch({ + type: BuilderStateAction.SET_LIMIT, + payload: option.value, + }); }} /> )} diff --git a/static/app/views/dashboards/widgetBuilder/hooks/useWidgetBuilderState.tsx b/static/app/views/dashboards/widgetBuilder/hooks/useWidgetBuilderState.tsx index 4926dd86dd61a0..8f17c11588cc10 100644 --- a/static/app/views/dashboards/widgetBuilder/hooks/useWidgetBuilderState.tsx +++ b/static/app/views/dashboards/widgetBuilder/hooks/useWidgetBuilderState.tsx @@ -21,6 +21,7 @@ export type WidgetBuilderStateQueryParams = { description?: string; displayType?: DisplayType; field?: (string | undefined)[]; + limit?: number; query?: string[]; sort?: string[]; title?: string; diff --git a/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToWidget.spec.tsx b/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToWidget.spec.tsx index 563e27c814516a..ab2f226a1a83fa 100644 --- a/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToWidget.spec.tsx +++ b/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToWidget.spec.tsx @@ -10,6 +10,7 @@ describe('convertBuilderStateToWidget', function () { description: 'Test Description', dataset: WidgetType.ERRORS, displayType: DisplayType.TABLE, + limit: 5, fields: [], yAxis: [], }; @@ -22,6 +23,7 @@ describe('convertBuilderStateToWidget', function () { widgetType: WidgetType.ERRORS, displayType: DisplayType.TABLE, interval: '1h', + limit: 5, queries: [ { ...getDatasetConfig(WidgetType.ERRORS).defaultWidgetQuery, @@ -36,6 +38,7 @@ describe('convertBuilderStateToWidget', function () { description: 'test description for an issues widget', dataset: WidgetType.ISSUE, displayType: DisplayType.TABLE, + limit: 5, fields: [], yAxis: [], }; @@ -48,6 +51,7 @@ describe('convertBuilderStateToWidget', function () { widgetType: WidgetType.ISSUE, displayType: DisplayType.TABLE, interval: '1h', + limit: 5, queries: [ { ...getDatasetConfig(WidgetType.ISSUE).defaultWidgetQuery, @@ -62,6 +66,7 @@ describe('convertBuilderStateToWidget', function () { description: 'Test Description', dataset: WidgetType.ERRORS, displayType: DisplayType.TABLE, + limit: 5, fields: [ {kind: 'field', field: 'geo.country'}, { @@ -84,6 +89,7 @@ describe('convertBuilderStateToWidget', function () { widgetType: WidgetType.ERRORS, displayType: DisplayType.TABLE, interval: '1h', + limit: 5, queries: [ { fields: ['geo.country', 'count()', 'count_unique(user)'], diff --git a/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToWidget.ts b/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToWidget.ts index 5ec914b8cc3620..1f3e3961b8a6f8 100644 --- a/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToWidget.ts +++ b/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToWidget.ts @@ -51,5 +51,6 @@ export function convertBuilderStateToWidget(state: WidgetBuilderState): Widget { interval: '1h', // TODO: Not sure what to put here yet queries: widgetQueries, widgetType: state.dataset, + limit: state.limit, }; } diff --git a/static/app/views/dashboards/widgetBuilder/utils/convertWidgetToBuilderStateParams.ts b/static/app/views/dashboards/widgetBuilder/utils/convertWidgetToBuilderStateParams.ts index 64424e45792cf1..676529168f1805 100644 --- a/static/app/views/dashboards/widgetBuilder/utils/convertWidgetToBuilderStateParams.ts +++ b/static/app/views/dashboards/widgetBuilder/utils/convertWidgetToBuilderStateParams.ts @@ -27,6 +27,7 @@ export function convertWidgetToBuilderStateParams( description: widget.description ?? '', dataset: widget.widgetType ?? WidgetType.ERRORS, displayType: widget.displayType ?? DisplayType.TABLE, + limit: widget.limit, field, yAxis, query,