diff --git a/static/app/utils/fields/index.ts b/static/app/utils/fields/index.ts index 875247232cfb3f..71502027bfd1b1 100644 --- a/static/app/utils/fields/index.ts +++ b/static/app/utils/fields/index.ts @@ -829,6 +829,7 @@ export const ALLOWED_EXPLORE_VISUALIZE_FIELDS: SpanIndexedField[] = [ export const ALLOWED_EXPLORE_VISUALIZE_AGGREGATES: AggregationKey[] = [ AggregationKey.COUNT, // DO NOT RE-ORDER: the first element is used as the default + AggregationKey.COUNT_UNIQUE, AggregationKey.AVG, AggregationKey.P50, AggregationKey.P75, diff --git a/static/app/views/alerts/rules/metric/eapField.spec.tsx b/static/app/views/alerts/rules/metric/eapField.spec.tsx index 6bf05128f18b53..041cfcfd9e07a0 100644 --- a/static/app/views/alerts/rules/metric/eapField.spec.tsx +++ b/static/app/views/alerts/rules/metric/eapField.spec.tsx @@ -82,18 +82,6 @@ describe('EAPField', () => { } render(); - expect(fieldsMock).toHaveBeenCalledWith( - `/organizations/${organization.slug}/spans/fields/`, - expect.objectContaining({ - query: expect.objectContaining({type: 'number'}), - }) - ); - expect(fieldsMock).toHaveBeenCalledWith( - `/organizations/${organization.slug}/spans/fields/`, - expect.objectContaining({ - query: expect.objectContaining({type: 'string'}), - }) - ); // switch from count(spans) -> max(span.duration) await userEvent.click(screen.getByText('count')); @@ -111,4 +99,37 @@ describe('EAPField', () => { expect(screen.getByText('count')).toBeInTheDocument(); expect(screen.getByText('spans')).toBeInTheDocument(); }); + + it('defaults count_unique argument to span.op', async function () { + function Component() { + const [aggregate, setAggregate] = useState('count(span.duration)'); + return ( + + + + ); + } + + render(); + + // switch from count(spans) -> count_unique(span.op) + await userEvent.click(screen.getByText('count')); + await userEvent.click(await screen.findByText('count_unique')); + expect(screen.getByText('count_unique')).toBeInTheDocument(); + expect(screen.getByText('span.op')).toBeInTheDocument(); + + // switch from count_unique(span.op) -> avg(span.self_time) + await userEvent.click(screen.getByText('count_unique')); + await userEvent.click(await screen.findByText('avg')); + await userEvent.click(screen.getByText('span.duration')); + await userEvent.click(await screen.findByText('span.self_time')); + expect(screen.getByText('avg')).toBeInTheDocument(); + expect(screen.getByText('span.self_time')).toBeInTheDocument(); + + // switch from avg(span.self_time) -> count_unique(span.op) + await userEvent.click(screen.getByText('avg')); + await userEvent.click(await screen.findByText('count_unique')); + expect(screen.getByText('count_unique')).toBeInTheDocument(); + expect(screen.getByText('span.op')).toBeInTheDocument(); + }); }); diff --git a/static/app/views/alerts/rules/metric/eapField.tsx b/static/app/views/alerts/rules/metric/eapField.tsx index 024a038c42498e..09c193b35f1225 100644 --- a/static/app/views/alerts/rules/metric/eapField.tsx +++ b/static/app/views/alerts/rules/metric/eapField.tsx @@ -7,16 +7,17 @@ import {space} from 'sentry/styles/space'; import type {TagCollection} from 'sentry/types/group'; import {defined} from 'sentry/utils'; import {parseFunction, prettifyTagKey} from 'sentry/utils/discover/fields'; -import {ALLOWED_EXPLORE_VISUALIZE_AGGREGATES} from 'sentry/utils/fields'; +import {AggregationKey, ALLOWED_EXPLORE_VISUALIZE_AGGREGATES} from 'sentry/utils/fields'; import { DEFAULT_VISUALIZATION, - DEFAULT_VISUALIZATION_AGGREGATE, DEFAULT_VISUALIZATION_FIELD, + updateVisualizeAggregate, } from 'sentry/views/explore/contexts/pageParamsContext/visualizes'; import {useSpanTags} from 'sentry/views/explore/contexts/spanTagsContext'; +export const DEFAULT_EAP_AGGREGATION = 'count'; export const DEFAULT_EAP_FIELD = 'span.duration'; -export const DEFAULT_EAP_METRICS_ALERT_FIELD = `count(${DEFAULT_EAP_FIELD})`; +export const DEFAULT_EAP_METRICS_ALERT_FIELD = `${DEFAULT_EAP_AGGREGATION}(${DEFAULT_EAP_FIELD})`; interface Props { aggregate: string; @@ -48,7 +49,10 @@ function EAPField({aggregate, onChange}: Props) { // compatibility, we don't want to lock down all `count` queries immediately. const lockOptions = aggregate === DEFAULT_VISUALIZATION; - const {tags: storedTags} = useSpanTags('number'); + const {tags: storedStringTags} = useSpanTags('string'); + const {tags: storedNumberTags} = useSpanTags('number'); + const storedTags = + aggregation === AggregationKey.COUNT_UNIQUE ? storedStringTags : storedNumberTags; const numberTags: TagCollection = useMemo(() => { const availableTags: TagCollection = storedTags; if (field && !defined(storedTags[field])) { @@ -84,13 +88,14 @@ function EAPField({aggregate, onChange}: Props) { const handleOperationChange = useCallback( (option: any) => { - const newAggregate = - option.value === DEFAULT_VISUALIZATION_AGGREGATE - ? DEFAULT_VISUALIZATION - : `${option.value}(${field || DEFAULT_EAP_FIELD})`; + const newAggregate = updateVisualizeAggregate({ + newAggregate: option.value, + oldAggregate: aggregation || DEFAULT_EAP_AGGREGATION, + oldArgument: field || DEFAULT_EAP_FIELD, + }); onChange(newAggregate, {}); }, - [field, onChange] + [aggregation, field, onChange] ); // As SelectControl does not support an options size limit out of the box diff --git a/static/app/views/dashboards/datasetConfig/spans.tsx b/static/app/views/dashboards/datasetConfig/spans.tsx index 3a53d4254346cc..e1a99ad6b1c4f3 100644 --- a/static/app/views/dashboards/datasetConfig/spans.tsx +++ b/static/app/views/dashboards/datasetConfig/spans.tsx @@ -21,7 +21,7 @@ import { doDiscoverQuery, } from 'sentry/utils/discover/genericDiscoverQuery'; import {DiscoverDatasets} from 'sentry/utils/discover/types'; -import {ALLOWED_EXPLORE_VISUALIZE_AGGREGATES} from 'sentry/utils/fields'; +import {AggregationKey, ALLOWED_EXPLORE_VISUALIZE_AGGREGATES} from 'sentry/utils/fields'; import type {MEPState} from 'sentry/utils/performance/contexts/metricsEnhancedSetting'; import type {OnDemandControlContext} from 'sentry/utils/performance/contexts/onDemandControl'; import { @@ -177,15 +177,22 @@ function _isNotNumericTag(option: FieldValueOption) { return true; } -function filterAggregateParams(option: FieldValueOption) { +function filterAggregateParams(option: FieldValueOption, fieldValue?: QueryFieldValue) { // Allow for unknown values to be used for aggregate functions // This supports showing the tag value even if it's not in the current // set of tags. if ('unknown' in option.value.meta && option.value.meta.unknown) { return true; } + + const expectedDataType = + fieldValue?.kind === 'function' && + fieldValue?.function[0] === AggregationKey.COUNT_UNIQUE + ? 'string' + : 'number'; + if ('dataType' in option.value.meta) { - return option.value.meta.dataType === 'number'; + return option.value.meta.dataType === expectedDataType; } return true; } diff --git a/static/app/views/dashboards/widgetBuilder/components/visualize/index.spec.tsx b/static/app/views/dashboards/widgetBuilder/components/visualize/index.spec.tsx index add95f9183243e..09dc3f1d78d6de 100644 --- a/static/app/views/dashboards/widgetBuilder/components/visualize/index.spec.tsx +++ b/static/app/views/dashboards/widgetBuilder/components/visualize/index.spec.tsx @@ -42,8 +42,19 @@ describe('Visualize', () => { return {tags, isLoading: false}; } + const tags: TagCollection = { + 'span.op': { + key: 'span.op', + name: 'span.op', + }, + 'span.description': { + key: 'span.description', + name: 'span.description', + }, + }; + return { - tags: {}, + tags, isLoading: false, }; }); @@ -1379,4 +1390,70 @@ describe('Visualize', () => { 'spans' ); }); + + it('defaults count_unique argument to span.op', async function () { + render( + + + , + { + organization, + router: RouterFixture({ + location: LocationFixture({ + query: { + dataset: WidgetType.SPANS, + displayType: DisplayType.LINE, + yAxis: ['count(span.duration)'], + }, + }), + }), + } + ); + + expect( + await screen.findByRole('button', {name: 'Aggregate Selection'}) + ).toBeEnabled(); + + expect( + await screen.findByRole('button', {name: 'Aggregate Selection'}) + ).toHaveTextContent('count'); + expect( + await screen.findByRole('button', {name: 'Column Selection'}) + ).toHaveTextContent('spans'); + + await userEvent.click( + await screen.findByRole('button', {name: 'Aggregate Selection'}) + ); + await userEvent.click(await screen.findByRole('option', {name: 'count_unique'})); + + expect( + await screen.findByRole('button', {name: 'Aggregate Selection'}) + ).toHaveTextContent('count_unique'); + expect( + await screen.findByRole('button', {name: 'Column Selection'}) + ).toHaveTextContent('span.op'); + + await userEvent.click( + await screen.findByRole('button', {name: 'Aggregate Selection'}) + ); + await userEvent.click(await screen.findByRole('option', {name: 'avg'})); + + expect( + await screen.findByRole('button', {name: 'Aggregate Selection'}) + ).toHaveTextContent('avg'); + expect( + await screen.findByRole('button', {name: 'Column Selection'}) + ).toHaveTextContent('span.duration'); + + await userEvent.click( + await screen.findByRole('button', {name: 'Aggregate Selection'}) + ); + await userEvent.click(await screen.findByRole('option', {name: 'count_unique'})); + expect( + await screen.findByRole('button', {name: 'Aggregate Selection'}) + ).toHaveTextContent('count_unique'); + expect( + await screen.findByRole('button', {name: 'Column Selection'}) + ).toHaveTextContent('span.op'); + }); }); diff --git a/static/app/views/dashboards/widgetBuilder/components/visualize/selectRow.tsx b/static/app/views/dashboards/widgetBuilder/components/visualize/selectRow.tsx index 931d7f5055b4bb..f054dea5d39a0b 100644 --- a/static/app/views/dashboards/widgetBuilder/components/visualize/selectRow.tsx +++ b/static/app/views/dashboards/widgetBuilder/components/visualize/selectRow.tsx @@ -16,6 +16,7 @@ import { parseFunction, type QueryFieldValue, } from 'sentry/utils/discover/fields'; +import {AggregationKey} from 'sentry/utils/fields'; import useOrganization from 'sentry/utils/useOrganization'; import {getDatasetConfig} from 'sentry/views/dashboards/datasetConfig/base'; import {DisplayType, WidgetType} from 'sentry/views/dashboards/types'; @@ -36,6 +37,7 @@ import { DEFAULT_VISUALIZATION_AGGREGATE, DEFAULT_VISUALIZATION_FIELD, } from 'sentry/views/explore/contexts/pageParamsContext/visualizes'; +import {SpanIndexedField} from 'sentry/views/insights/types'; type AggregateFunction = [ AggregationKeyWithAlias, @@ -266,6 +268,7 @@ export function SelectRow({ openColumnSelect(); } else { if (currentField.kind === FieldValueKind.FUNCTION) { + const originalFunction = currentField.function[0]; // Handle setting an aggregate from an aggregate currentField.function[0] = parseAggregateFromValueKey( dropdownSelection.value as string @@ -279,6 +282,30 @@ export function SelectRow({ ) { currentField.function[1] = DEFAULT_VISUALIZATION_FIELD; + // Wipe out the remaining parameters that are unnecessary + for (let i = 1; i < MAX_FUNCTION_PARAMETERS; i++) { + currentField.function[i + 1] = undefined; + } + } else if ( + // when switching to the count_unique aggregate, we want to reset the + // field to the default + state.dataset === WidgetType.SPANS && + currentField.function[0] === AggregationKey.COUNT_UNIQUE + ) { + currentField.function[1] = SpanIndexedField.SPAN_OP; + + // Wipe out the remaining parameters that are unnecessary + for (let i = 1; i < MAX_FUNCTION_PARAMETERS; i++) { + currentField.function[i + 1] = undefined; + } + } else if ( + // when switching away from the count_unique aggregate, we want to reset the + // field to the default + state.dataset === WidgetType.SPANS && + originalFunction === AggregationKey.COUNT_UNIQUE + ) { + currentField.function[1] = DEFAULT_VISUALIZATION_FIELD; + // Wipe out the remaining parameters that are unnecessary for (let i = 1; i < MAX_FUNCTION_PARAMETERS; i++) { currentField.function[i + 1] = undefined; diff --git a/static/app/views/dashboards/widgetBuilder/widgetBuilderSortBy.spec.tsx b/static/app/views/dashboards/widgetBuilder/widgetBuilderSortBy.spec.tsx index abf3b659c8bbe5..6f97414144f5cc 100644 --- a/static/app/views/dashboards/widgetBuilder/widgetBuilderSortBy.spec.tsx +++ b/static/app/views/dashboards/widgetBuilder/widgetBuilderSortBy.spec.tsx @@ -968,8 +968,8 @@ describe('WidgetBuilder', function () { await screen.findByText('Sort by a y-axis'); await selectEvent.openMenu(screen.getAllByText('count(\u2026)')[1]!); - // 12 options in the dropdown - expect(screen.queryAllByTestId('menu-list-item-label')).toHaveLength(12); + // Check the number of options in the dropdown + expect(screen.queryAllByTestId('menu-list-item-label')).toHaveLength(13); // Appears once in the y-axis section, dropdown, and in the sort by field expect(await screen.findAllByText('count(\u2026)')).toHaveLength(3); diff --git a/static/app/views/explore/contexts/pageParamsContext/visualizes.tsx b/static/app/views/explore/contexts/pageParamsContext/visualizes.tsx index fb5f584be92264..0b9c30fd3e549a 100644 --- a/static/app/views/explore/contexts/pageParamsContext/visualizes.tsx +++ b/static/app/views/explore/contexts/pageParamsContext/visualizes.tsx @@ -5,11 +5,13 @@ import type {Organization} from 'sentry/types/organization'; import {defined} from 'sentry/utils'; import {parseFunction} from 'sentry/utils/discover/fields'; import { + AggregationKey, ALLOWED_EXPLORE_VISUALIZE_AGGREGATES, ALLOWED_EXPLORE_VISUALIZE_FIELDS, } from 'sentry/utils/fields'; import {decodeList} from 'sentry/utils/queryString'; import {ChartType} from 'sentry/views/insights/common/components/chart'; +import {SpanIndexedField} from 'sentry/views/insights/types'; export const MAX_VISUALIZES = 4; @@ -100,3 +102,34 @@ export function updateLocationWithVisualizes( delete location.query.visualize; } } + +export function updateVisualizeAggregate({ + newAggregate, + oldAggregate, + oldArgument, +}: { + newAggregate: string; + oldAggregate: string; + oldArgument: string; +}): string { + // the default aggregate only has 1 allowed field + if (newAggregate === DEFAULT_VISUALIZATION_AGGREGATE) { + return DEFAULT_VISUALIZATION; + } + + // count_unique uses a different set of fields + if (newAggregate === AggregationKey.COUNT_UNIQUE) { + // The general thing to do here is to valid the the argument type + // and carry the argument if it's the same type, reset to a default + // if it's not the same type. Just hard coding it for now for simplicity + // as `count_unique` is the only aggregate that takes a string. + return `${AggregationKey.COUNT_UNIQUE}(${SpanIndexedField.SPAN_OP})`; + } + + // switching away from count_unique means we need to reset the field + if (oldAggregate === AggregationKey.COUNT_UNIQUE) { + return `${newAggregate}(${DEFAULT_VISUALIZATION_FIELD})`; + } + + return `${newAggregate}(${oldArgument})`; +} diff --git a/static/app/views/explore/hooks/useVisualizeFields.tsx b/static/app/views/explore/hooks/useVisualizeFields.tsx index c4223fe19d6baa..02cfc37047d038 100644 --- a/static/app/views/explore/hooks/useVisualizeFields.tsx +++ b/static/app/views/explore/hooks/useVisualizeFields.tsx @@ -7,13 +7,31 @@ import { parseFunction, prettifyTagKey, } from 'sentry/utils/discover/fields'; +import {AggregationKey} from 'sentry/utils/fields'; import {useSpanTags} from 'sentry/views/explore/contexts/spanTagsContext'; -type Props = {yAxes: string[]}; +interface Props { + /** + * All the aggregates that are in use. The arguments will be extracted + * and injected as options if they are compatible. + */ + yAxes: string[]; + /** + * The current aggregate in use. Used to determine what the argument + * types will be compatible. + */ + yAxis?: string; +} -export function useVisualizeFields({yAxes}: Props) { +export function useVisualizeFields({yAxis, yAxes}: Props) { + const {tags: stringTags} = useSpanTags('string'); const {tags: numberTags} = useSpanTags('number'); + const parsedYAxis = useMemo(() => (yAxis ? parseFunction(yAxis) : undefined), [yAxis]); + + const tags = + parsedYAxis?.name === AggregationKey.COUNT_UNIQUE ? stringTags : numberTags; + const parsedYAxes: ParsedFunction[] = useMemo(() => { return yAxes.map(parseFunction).filter(defined); }, [yAxes]); @@ -24,7 +42,7 @@ export function useVisualizeFields({yAxes}: Props) { return entry.arguments; }) .filter(option => { - return !numberTags.hasOwnProperty(option); + return !tags.hasOwnProperty(option); }); const options = [ @@ -33,7 +51,7 @@ export function useVisualizeFields({yAxes}: Props) { value: option, textValue: option, })), - ...Object.values(numberTags).map(tag => { + ...Object.values(tags).map(tag => { return {label: tag.name, value: tag.key, textValue: tag.name}; }), ]; @@ -51,7 +69,7 @@ export function useVisualizeFields({yAxes}: Props) { }); return options; - }, [numberTags, parsedYAxes]); + }, [tags, parsedYAxes]); return fieldOptions; } diff --git a/static/app/views/explore/multiQueryMode/content.spec.tsx b/static/app/views/explore/multiQueryMode/content.spec.tsx index 7f61ff5cab0f4e..71c650d613e710 100644 --- a/static/app/views/explore/multiQueryMode/content.spec.tsx +++ b/static/app/views/explore/multiQueryMode/content.spec.tsx @@ -189,6 +189,100 @@ describe('MultiQueryModeContent', function () { ]); }); + it('defaults count_unique argument to span.op', async function () { + let queries: any; + function Component() { + queries = useReadQueriesFromLocation(); + return ; + } + + render( + + + + + , + {enableRouterMocks: false} + ); + + const section = await screen.findByTestId('section-visualize-0'); + + expect(queries).toEqual([ + { + chartType: 1, + yAxes: ['count(span.duration)'], + sortBys: [ + { + field: 'span.duration', + kind: 'desc', + }, + ], + fields: ['id', 'span.duration'], + groupBys: [], + query: '', + }, + ]); + + await userEvent.click(within(section).getByRole('button', {name: 'count'})); + await userEvent.click(within(section).getByRole('option', {name: 'count_unique'})); + + expect(queries).toEqual([ + { + chartType: 1, + yAxes: ['count_unique(span.op)'], + sortBys: [ + { + field: 'id', + kind: 'desc', + }, + ], + fields: ['id', 'span.op'], + groupBys: [], + query: '', + }, + ]); + + await userEvent.click(within(section).getByRole('button', {name: 'count_unique'})); + await userEvent.click(within(section).getByRole('option', {name: 'avg'})); + await userEvent.click(within(section).getByRole('button', {name: 'span.duration'})); + await userEvent.click(within(section).getByRole('option', {name: 'span.self_time'})); + + expect(queries).toEqual([ + { + chartType: 1, + yAxes: ['avg(span.self_time)'], + sortBys: [ + { + field: 'id', + kind: 'desc', + }, + ], + fields: ['id', 'span.self_time'], + groupBys: [], + query: '', + }, + ]); + + await userEvent.click(within(section).getByRole('button', {name: 'avg'})); + await userEvent.click(within(section).getByRole('option', {name: 'count_unique'})); + + expect(queries).toEqual([ + { + chartType: 1, + yAxes: ['count_unique(span.op)'], + sortBys: [ + { + field: 'id', + kind: 'desc', + }, + ], + fields: ['id', 'span.op'], + groupBys: [], + query: '', + }, + ]); + }); + it('updates visualization and outdated sorts', async function () { let queries: any; function Component() { diff --git a/static/app/views/explore/multiQueryMode/queryConstructors/visualize.tsx b/static/app/views/explore/multiQueryMode/queryConstructors/visualize.tsx index a249bdecc1fd74..2dcaadd1a4171b 100644 --- a/static/app/views/explore/multiQueryMode/queryConstructors/visualize.tsx +++ b/static/app/views/explore/multiQueryMode/queryConstructors/visualize.tsx @@ -6,12 +6,13 @@ import PageFilterBar from 'sentry/components/organizations/pageFilterBar'; import {Tooltip} from 'sentry/components/tooltip'; import {t} from 'sentry/locale'; import {defined} from 'sentry/utils'; +import type {ParsedFunction} from 'sentry/utils/discover/fields'; import {parseFunction} from 'sentry/utils/discover/fields'; import {ALLOWED_EXPLORE_VISUALIZE_AGGREGATES} from 'sentry/utils/fields'; import { - DEFAULT_VISUALIZATION, DEFAULT_VISUALIZATION_AGGREGATE, DEFAULT_VISUALIZATION_FIELD, + updateVisualizeAggregate, } from 'sentry/views/explore/contexts/pageParamsContext/visualizes'; import {useVisualizeFields} from 'sentry/views/explore/hooks/useVisualizeFields'; import { @@ -30,7 +31,7 @@ type Props = { }; export function VisualizeSection({query, index}: Props) { - const parsedFunction = query.yAxes.map(parseFunction).find(defined); + const [yAxis, parsedFunction] = findFirstFunction(query.yAxes); // We want to lock down the fields dropdown when using count so that we can // render `count(spans)` for better legibility. However, for backwards @@ -53,6 +54,7 @@ export function VisualizeSection({query, index}: Props) { ); const defaultFieldOptions: Array> = useVisualizeFields({ yAxes: query.yAxes, + yAxis, }); const fieldOptions = lockOptions ? countFieldOptions : defaultFieldOptions; @@ -85,10 +87,11 @@ export function VisualizeSection({query, index}: Props) { options={aggregateOptions} value={parsedFunction?.name} onChange={newAggregate => { - const newYAxis = - newAggregate.value === DEFAULT_VISUALIZATION_AGGREGATE - ? DEFAULT_VISUALIZATION - : `${newAggregate.value}(${parsedFunction!.arguments[0]})`; + const newYAxis = updateVisualizeAggregate({ + newAggregate: newAggregate.value, + oldAggregate: parsedFunction!.name, + oldArgument: parsedFunction!.arguments[0]!, + }); updateYAxis({yAxes: [newYAxis]}); }} /> @@ -108,6 +111,19 @@ export function VisualizeSection({query, index}: Props) { ); } +function findFirstFunction( + yAxes: ReadableExploreQueryParts['yAxes'] +): [string | undefined, ParsedFunction | undefined] { + for (const yAxis of yAxes) { + const parsed = parseFunction(yAxis); + if (defined(parsed)) { + return [yAxis, parsed]; + } + } + + return [undefined, undefined]; +} + const StyledPageFilterBar = styled(PageFilterBar)` & > * { min-width: 0; diff --git a/static/app/views/explore/toolbar/index.spec.tsx b/static/app/views/explore/toolbar/index.spec.tsx index 529153958cd38b..4876b063e566f8 100644 --- a/static/app/views/explore/toolbar/index.spec.tsx +++ b/static/app/views/explore/toolbar/index.spec.tsx @@ -226,6 +226,74 @@ describe('ExploreToolbar', function () { ]); }); + it('defaults count_unique argument to span.op', async function () { + let visualizes: any; + function Component() { + visualizes = useExploreVisualizes(); + return ; + } + + render( + + + + + , + {enableRouterMocks: false} + ); + + const section = screen.getByTestId('section-visualizes'); + + // this is the default + expect(visualizes).toEqual([ + { + chartType: ChartType.BAR, + label: 'A', + yAxes: ['count(span.duration)'], + }, + ]); + + // try changing the aggregate + await userEvent.click(within(section).getByRole('button', {name: 'count'})); + await userEvent.click(within(section).getByRole('option', {name: 'count_unique'})); + + expect(visualizes).toEqual([ + { + chartType: ChartType.BAR, + label: 'A', + yAxes: ['count_unique(span.op)'], + }, + ]); + + // try changing the aggregate + field + await userEvent.click(within(section).getByRole('button', {name: 'count_unique'})); + await userEvent.click(within(section).getByRole('option', {name: 'avg'})); + + // try changing the field + await userEvent.click(within(section).getByRole('button', {name: 'span.duration'})); + await userEvent.click(within(section).getByRole('option', {name: 'span.self_time'})); + + expect(visualizes).toEqual([ + { + chartType: ChartType.BAR, + label: 'A', + yAxes: ['avg(span.self_time)'], + }, + ]); + // + // try changing the aggregate back to count_unique + await userEvent.click(within(section).getByRole('button', {name: 'avg'})); + await userEvent.click(within(section).getByRole('option', {name: 'count_unique'})); + + expect(visualizes).toEqual([ + { + chartType: ChartType.BAR, + label: 'A', + yAxes: ['count_unique(span.op)'], + }, + ]); + }); + it('allows changing visualizes', async function () { let fields, visualizes: any; function Component() { diff --git a/static/app/views/explore/toolbar/toolbarVisualize.tsx b/static/app/views/explore/toolbar/toolbarVisualize.tsx index de96d5e6c5e6a4..fb8acb812f00b8 100644 --- a/static/app/views/explore/toolbar/toolbarVisualize.tsx +++ b/static/app/views/explore/toolbar/toolbarVisualize.tsx @@ -20,9 +20,9 @@ import { import type {Visualize} from 'sentry/views/explore/contexts/pageParamsContext/visualizes'; import { DEFAULT_VISUALIZATION, - DEFAULT_VISUALIZATION_AGGREGATE, DEFAULT_VISUALIZATION_FIELD, MAX_VISUALIZES, + updateVisualizeAggregate, } from 'sentry/views/explore/contexts/pageParamsContext/visualizes'; import {useVisualizeFields} from 'sentry/views/explore/hooks/useVisualizeFields'; import {ChartType} from 'sentry/views/insights/common/components/chart'; @@ -200,7 +200,10 @@ function VisualizeDropdown({ ], [] ); - const defaultFieldOptions: Array> = useVisualizeFields({yAxes}); + const defaultFieldOptions: Array> = useVisualizeFields({ + yAxes, + yAxis, + }); const fieldOptions = lockOptions ? countFieldOptions : defaultFieldOptions; const aggregateOptions: Array> = useMemo(() => { @@ -225,10 +228,11 @@ function VisualizeDropdown({ const setChartAggregate = useCallback( ({value}: SelectOption) => { const newVisualizes = visualizes.slice(); - newVisualizes[group]!.yAxes[index] = - value === DEFAULT_VISUALIZATION_AGGREGATE - ? DEFAULT_VISUALIZATION - : `${value}(${parsedVisualize.arguments[0]})`; + newVisualizes[group]!.yAxes[index] = updateVisualizeAggregate({ + newAggregate: value as string, + oldAggregate: parsedVisualize.name, + oldArgument: parsedVisualize.arguments[0]!, + }); setVisualizes(newVisualizes); }, [group, index, parsedVisualize, setVisualizes, visualizes] @@ -310,7 +314,7 @@ function VisualizeEquation({ return visualizes.flatMap(visualize => visualize.yAxes); }, [visualizes]); - const fieldOptions: Array> = useVisualizeFields({yAxes}); + const fieldOptions: Array> = useVisualizeFields({yAxes, yAxis}); const functionArguments = useMemo(() => { return fieldOptions.map(o => {