diff --git a/static/app/views/dashboards/widgetBuilder/components/visualize.spec.tsx b/static/app/views/dashboards/widgetBuilder/components/visualize.spec.tsx index 176595fe51cca8..32e8fd92df0b51 100644 --- a/static/app/views/dashboards/widgetBuilder/components/visualize.spec.tsx +++ b/static/app/views/dashboards/widgetBuilder/components/visualize.spec.tsx @@ -648,6 +648,68 @@ describe('Visualize', () => { expect(screen.getAllByText('epm')).toHaveLength(1); }); + it('shows appropriate error messages for non-chart widget queries', async () => { + render( + + + , + { + organization, + router: RouterFixture({ + location: LocationFixture({ + query: { + dataset: WidgetType.TRANSACTIONS, + displayType: DisplayType.BIG_NUMBER, + }, + }), + }), + } + ); + + expect(await screen.findByText('this field has an error')).toBeInTheDocument(); + expect(screen.queryByText('this aggregate has an error')).not.toBeInTheDocument(); + }); + + it('shows appropriate error messages for chart type widget queries', async () => { + render( + + + , + { + organization, + router: RouterFixture({ + location: LocationFixture({ + query: { + dataset: WidgetType.TRANSACTIONS, + displayType: DisplayType.LINE, + }, + }), + }), + } + ); + + expect(await screen.findByText('this aggregate has an error')).toBeInTheDocument(); + expect(screen.queryByText('this field has an error')).not.toBeInTheDocument(); + }); + describe('spans', () => { beforeEach(() => { jest.mocked(useSpanTags).mockImplementation((type?: 'string' | 'number') => { diff --git a/static/app/views/dashboards/widgetBuilder/components/visualize.tsx b/static/app/views/dashboards/widgetBuilder/components/visualize.tsx index 993d1d0b4a871a..57faf9d59e5d03 100644 --- a/static/app/views/dashboards/widgetBuilder/components/visualize.tsx +++ b/static/app/views/dashboards/widgetBuilder/components/visualize.tsx @@ -5,6 +5,7 @@ import cloneDeep from 'lodash/cloneDeep'; import {Button} from 'sentry/components/button'; import {CompactSelect} from 'sentry/components/compactSelect'; import SelectControl from 'sentry/components/forms/controls/selectControl'; +import FieldGroup from 'sentry/components/forms/fieldGroup'; import Input from 'sentry/components/input'; import {IconDelete} from 'sentry/icons'; import {t} from 'sentry/locale'; @@ -51,7 +52,12 @@ const NONE_AGGREGATE = { value: NONE, }; -function Visualize() { +interface VisualizeProps { + error?: Record; + setError?: (error: Record) => void; +} + +function Visualize({error, setError}: VisualizeProps) { const organization = useOrganization(); const {state, dispatch} = useWidgetBuilderContext(); let tags = useTags(); @@ -147,6 +153,11 @@ function Visualize() { // Used to extract selected aggregates and parameters from the fields const stringFields = fields?.map(generateFieldAsString); + const fieldErrors = error?.queries?.find(queryError => queryError?.fields)?.fields; + const aggregateErrors = error?.queries?.find( + aggregateError => aggregateError?.aggregates + )?.aggregates; + return ( - - {fields?.map((field, index) => { - // Depending on the dataset and the display type, we use different options for - // displaying in the column select. - // For charts, we show aggregate parameter options for the y-axis as primary options. - // For tables, we show all string tags and fields as primary options, as well - // as aggregates that don't take parameters. - const columnFilterMethod = isChartWidget - ? datasetConfig.filterYAxisAggregateParams?.( - field, - state.displayType ?? DisplayType.LINE - ) - : field.kind === FieldValueKind.FUNCTION - ? datasetConfig.filterAggregateParams - : datasetConfig.filterTableOptions; - const columnOptions = Object.values(fieldOptions) - .filter(option => { - // Don't show any aggregates under the columns, and if - // there isn't a filter method, just show the option - return ( - option.value.kind !== FieldValueKind.FUNCTION && - (columnFilterMethod?.(option, field) ?? true) - ); - }) - .map(option => ({ + + + {fields?.map((field, index) => { + // Depending on the dataset and the display type, we use different options for + // displaying in the column select. + // For charts, we show aggregate parameter options for the y-axis as primary options. + // For tables, we show all string tags and fields as primary options, as well + // as aggregates that don't take parameters. + const columnFilterMethod = isChartWidget + ? datasetConfig.filterYAxisAggregateParams?.( + field, + state.displayType ?? DisplayType.LINE + ) + : field.kind === FieldValueKind.FUNCTION + ? datasetConfig.filterAggregateParams + : datasetConfig.filterTableOptions; + const columnOptions = Object.values(fieldOptions) + .filter(option => { + // Don't show any aggregates under the columns, and if + // there isn't a filter method, just show the option + return ( + option.value.kind !== FieldValueKind.FUNCTION && + (columnFilterMethod?.(option, field) ?? true) + ); + }) + .map(option => ({ + value: option.value.meta.name, + label: + state.dataset === WidgetType.SPANS + ? prettifyTagKey(option.value.meta.name) + : option.value.meta.name, + + // For the spans dataset, all of the options are measurements, + // so we force the number badge to show + trailingItems: + state.dataset === WidgetType.SPANS ? ( + + ) : null, + })); + + let aggregateOptions = aggregates.map(option => ({ value: option.value.meta.name, - label: - state.dataset === WidgetType.SPANS - ? prettifyTagKey(option.value.meta.name) - : option.value.meta.name, - - // For the spans dataset, all of the options are measurements, - // so we force the number badge to show - trailingItems: - state.dataset === WidgetType.SPANS ? ( - - ) : null, + label: option.value.meta.name, })); + aggregateOptions = + isChartWidget || isBigNumberWidget + ? aggregateOptions + : [NONE_AGGREGATE, ...aggregateOptions]; + + let matchingAggregate; + if ( + fields[index]!.kind === FieldValueKind.FUNCTION && + FieldValueKind.FUNCTION in fields[index]! + ) { + matchingAggregate = aggregates.find( + option => + option.value.meta.name === + parseFunction(stringFields?.[index] ?? '')?.name + ); + } - let aggregateOptions = aggregates.map(option => ({ - value: option.value.meta.name, - label: option.value.meta.name, - })); - aggregateOptions = - isChartWidget || isBigNumberWidget - ? aggregateOptions - : [NONE_AGGREGATE, ...aggregateOptions]; - - let matchingAggregate; - if ( - fields[index]!.kind === FieldValueKind.FUNCTION && - FieldValueKind.FUNCTION in fields[index]! - ) { - matchingAggregate = aggregates.find( - option => - option.value.meta.name === - parseFunction(stringFields?.[index] ?? '')?.name - ); - } - - const parameterRefinements = - matchingAggregate?.value.meta.parameters.length > 1 - ? matchingAggregate?.value.meta.parameters.slice(1) - : []; - - return ( - - - {field.kind === FieldValueKind.EQUATION ? ( - - dispatch({ - type: updateAction, - payload: fields.map((_field, i) => - i === index ? {..._field, field: value} : _field - ), - }) - } - options={fields} - placeholder={t('Equation')} - aria-label={t('Equation')} - /> - ) : ( - - - { - const newFields = cloneDeep(fields); - const currentField = newFields[index]!; - // Update the current field's aggregate with the new aggregate - if (currentField.kind === FieldValueKind.FUNCTION) { - currentField.function[1] = newField.value as string; + const parameterRefinements = + matchingAggregate?.value.meta.parameters.length > 1 + ? matchingAggregate?.value.meta.parameters.slice(1) + : []; + + return ( + + + {field.kind === FieldValueKind.EQUATION ? ( + { + dispatch({ + type: updateAction, + payload: fields.map((_field, i) => + i === index ? {..._field, field: value} : _field + ), + }); + setError?.({...error, queries: []}); + }} + options={fields} + placeholder={t('Equation')} + aria-label={t('Equation')} + /> + ) : ( + + + - { - const isNone = aggregateSelection.value === NONE; - const newFields = cloneDeep(fields); - const currentField = newFields[index]!; - const newAggregate = aggregates.find( - option => option.value.meta.name === aggregateSelection.value - ); - // Update the current field's aggregate with the new aggregate - if (!isNone) { + onChange={newField => { + const newFields = cloneDeep(fields); + const currentField = newFields[index]!; + // Update the current field's aggregate with the new aggregate if (currentField.kind === FieldValueKind.FUNCTION) { - // Handle setting an aggregate from an aggregate - currentField.function[0] = - aggregateSelection.value as AggregationKeyWithAlias; - if ( - newAggregate?.value.meta && - 'parameters' in newAggregate.value.meta - ) { - // There are aggregates that have no parameters, so wipe out the argument - // if it's supposed to be empty - if (newAggregate.value.meta.parameters.length === 0) { - currentField.function[1] = ''; - } else { - currentField.function[1] = - (currentField.function[1] || - newAggregate.value.meta.parameters[0]! - .defaultValue) ?? - ''; - // Set the remaining parameters for the new aggregate + currentField.function[1] = newField.value as string; + } + if (currentField.kind === FieldValueKind.FIELD) { + currentField.field = newField.value as string; + } + dispatch({ + type: updateAction, + payload: newFields, + }); + setError?.({...error, queries: []}); + }} + triggerProps={{ + 'aria-label': t('Column Selection'), + }} + disabled={ + fields[index]!.kind === FieldValueKind.FUNCTION && + matchingAggregate?.value.meta.parameters.length === 0 + } + /> + { + const isNone = aggregateSelection.value === NONE; + const newFields = cloneDeep(fields); + const currentField = newFields[index]!; + const newAggregate = aggregates.find( + option => + option.value.meta.name === aggregateSelection.value + ); + // Update the current field's aggregate with the new aggregate + if (!isNone) { + if (currentField.kind === FieldValueKind.FUNCTION) { + // Handle setting an aggregate from an aggregate + currentField.function[0] = + aggregateSelection.value as AggregationKeyWithAlias; + if ( + newAggregate?.value.meta && + 'parameters' in newAggregate.value.meta + ) { + // There are aggregates that have no parameters, so wipe out the argument + // if it's supposed to be empty + if (newAggregate.value.meta.parameters.length === 0) { + currentField.function[1] = ''; + } else { + currentField.function[1] = + (currentField.function[1] || + newAggregate.value.meta.parameters[0]! + .defaultValue) ?? + ''; + // Set the remaining parameters for the new aggregate + for ( + let i = 1; // The first parameter is the column selection + i < newAggregate.value.meta.parameters.length; + i++ + ) { + // Increment by 1 to skip past the aggregate name + currentField.function[i + 1] = + newAggregate.value.meta.parameters[ + i + ]!.defaultValue; + } + } + + // Wipe out the remaining parameters that are unnecessary + // This is necessary for transitioning between aggregates that have + // more parameters to ones of fewer parameters for ( - let i = 1; // The first parameter is the column selection - i < newAggregate.value.meta.parameters.length; + let i = newAggregate.value.meta.parameters.length; + i < MAX_FUNCTION_PARAMETERS; i++ ) { - // Increment by 1 to skip past the aggregate name - currentField.function[i + 1] = - newAggregate.value.meta.parameters[i]!.defaultValue; + currentField.function[i + 1] = undefined; } } + } else { + if ( + !newAggregate || + !('parameters' in newAggregate.value.meta) + ) { + return; + } - // Wipe out the remaining parameters that are unnecessary - // This is necessary for transitioning between aggregates that have - // more parameters to ones of fewer parameters - for ( - let i = newAggregate.value.meta.parameters.length; - i < MAX_FUNCTION_PARAMETERS; - i++ + // Handle setting an aggregate from a field + const newFunction: AggregateFunction = [ + aggregateSelection.value as AggregationKeyWithAlias, + (currentField.field || + newAggregate?.value.meta?.parameters?.[0] + ?.defaultValue) ?? + '', + newAggregate?.value.meta?.parameters?.[1] + ?.defaultValue ?? undefined, + newAggregate?.value.meta?.parameters?.[2] + ?.defaultValue ?? undefined, + ]; + if ( + newAggregate?.value.meta && + 'parameters' in newAggregate.value.meta ) { - currentField.function[i + 1] = undefined; + newAggregate?.value.meta.parameters.forEach( + (parameter, parameterIndex) => { + // Increment by 1 to skip past the aggregate name + newFunction[parameterIndex + 1] = + newFunction[parameterIndex + 1] ?? + parameter.defaultValue; + } + ); } + newFields[index] = { + kind: FieldValueKind.FUNCTION, + function: newFunction, + }; } } else { - if ( - !newAggregate || - !('parameters' in newAggregate.value.meta) - ) { - return; - } - - // Handle setting an aggregate from a field - const newFunction: AggregateFunction = [ - aggregateSelection.value as AggregationKeyWithAlias, - (currentField.field || - newAggregate?.value.meta?.parameters?.[0] - ?.defaultValue) ?? - '', - newAggregate?.value.meta?.parameters?.[1]?.defaultValue ?? - undefined, - newAggregate?.value.meta?.parameters?.[2]?.defaultValue ?? - undefined, - ]; - if ( - newAggregate?.value.meta && - 'parameters' in newAggregate.value.meta - ) { - newAggregate?.value.meta.parameters.forEach( - (parameter, parameterIndex) => { - // Increment by 1 to skip past the aggregate name - newFunction[parameterIndex + 1] = - newFunction[parameterIndex + 1] ?? - parameter.defaultValue; - } - ); - } + // Handle selecting None so we can select just a field, e.g. for samples + // If none is selected, set the field to a field value newFields[index] = { - kind: FieldValueKind.FUNCTION, - function: newFunction, + kind: FieldValueKind.FIELD, + field: + 'function' in currentField + ? (currentField.function[1] as string) ?? + columnOptions[0]!.value + : '', }; } - } else { - // Handle selecting None so we can select just a field, e.g. for samples - // If none is selected, set the field to a field value - newFields[index] = { - kind: FieldValueKind.FIELD, - field: - 'function' in currentField - ? (currentField.function[1] as string) ?? - columnOptions[0]!.value - : '', - }; - } - dispatch({ - type: updateAction, - payload: newFields, - }); - }} - triggerProps={{ - 'aria-label': t('Aggregate Selection'), - }} - /> - - {field.kind === FieldValueKind.FUNCTION && - parameterRefinements.length > 0 && ( - - {parameterRefinements.map((parameter, parameterIndex) => { - // The current value is displaced by 2 because the first two parameters - // are the aggregate name and the column selection - const currentValue = field.function[parameterIndex + 2] || ''; - const key = `${field.function.join('_')}-${parameterIndex}`; - return ( - { - const newFields = cloneDeep(fields); - if ( - newFields[index]!.kind !== FieldValueKind.FUNCTION - ) { - return; - } - newFields[index]!.function[parameterIndex + 2] = value; - dispatch({ - type: updateAction, - payload: newFields, - }); - }} - /> - ); - })} - - )} - - )} - - - {!isChartWidget && !isBigNumberWidget && ( - { - const newFields = cloneDeep(fields); - newFields[index]!.alias = e.target.value; + dispatch({ + type: updateAction, + payload: newFields, + }); + setError?.({...error, queries: []}); + }} + triggerProps={{ + 'aria-label': t('Aggregate Selection'), + }} + /> + + {field.kind === FieldValueKind.FUNCTION && + parameterRefinements.length > 0 && ( + + {parameterRefinements.map((parameter, parameterIndex) => { + // The current value is displaced by 2 because the first two parameters + // are the aggregate name and the column selection + const currentValue = + field.function[parameterIndex + 2] || ''; + const key = `${field.function.join('_')}-${parameterIndex}`; + return ( + { + const newFields = cloneDeep(fields); + if ( + newFields[index]!.kind !== FieldValueKind.FUNCTION + ) { + return; + } + newFields[index]!.function[parameterIndex + 2] = + value; + dispatch({ + type: updateAction, + payload: newFields, + }); + setError?.({...error, queries: []}); + }} + /> + ); + })} + + )} + + )} + + + {!isChartWidget && !isBigNumberWidget && ( + { + const newFields = cloneDeep(fields); + newFields[index]!.alias = e.target.value; + dispatch({ + type: updateAction, + payload: newFields, + }); + }} + /> + )} + } + size="zero" + disabled={fields.length <= 1} + onClick={() => dispatch({ type: updateAction, - payload: newFields, - }); - }} + payload: fields?.filter((_field, i) => i !== index) ?? [], + }) + } + aria-label={t('Remove field')} /> - )} - } - size="zero" - disabled={fields.length <= 1} - onClick={() => - dispatch({ - type: updateAction, - payload: fields?.filter((_field, i) => i !== index) ?? [], - }) - } - aria-label={t('Remove field')} - /> - - - ); - })} - + + + ); + })} + +
- +