Просмотр исходного кода

fix(dashboards): Fix dashboard field options (#24964)

Co-authored-by: Mark Story <mark@sentry.io>
Alberto Leal 3 лет назад
Родитель
Сommit
1265af5372

+ 65 - 43
src/sentry/static/sentry/app/components/dashboards/widgetQueryFields.tsx

@@ -101,10 +101,11 @@ function WidgetQueryFields({
     (['line', 'area', 'stacked_area', 'bar'].includes(displayType) &&
       fields.length === 3);
 
-  // Any column choice for World Map and Big Number widgets is legal since the
+  // Any function/field choice for Big Number widgets is legal since the
   // data source is from an endpoint that is not timeseries-based.
+  // The function/field choice for World Map widget will need to be numeric-like.
   // Column builder for Table widget is already handled above.
-  const doNotValidateYAxis = ['world_map', 'big_number'].includes(displayType);
+  const doNotValidateYAxis = displayType === 'big_number';
 
   return (
     <Field
@@ -117,53 +118,74 @@ function WidgetQueryFields({
       required
       stacked
     >
-      {fields.map((field, i) => (
-        <QueryFieldWrapper key={`${field}:${i}`}>
-          <QueryField
-            fieldValue={explodeField({field})}
-            fieldOptions={fieldOptions}
-            onChange={value => handleChangeField(value, i)}
-            filterPrimaryOptions={option => {
-              // Only validate functions for timeseries widgets.
-              if (!doNotValidateYAxis && option.value.kind === FieldValueKind.FUNCTION) {
+      {fields.map((field, i) => {
+        const fieldValue = explodeField({field});
+        return (
+          <QueryFieldWrapper key={`${field}:${i}`}>
+            <QueryField
+              fieldValue={fieldValue}
+              fieldOptions={fieldOptions}
+              onChange={value => handleChangeField(value, i)}
+              filterPrimaryOptions={option => {
+                // Only validate function names for timeseries widgets and
+                // world map widgets.
+                if (
+                  !doNotValidateYAxis &&
+                  option.value.kind === FieldValueKind.FUNCTION
+                ) {
+                  const primaryOutput = aggregateFunctionOutputType(
+                    option.value.meta.name,
+                    undefined
+                  );
+                  if (primaryOutput) {
+                    // If a function returns a specific type, then validate it.
+                    return isLegalYAxisType(primaryOutput);
+                  }
+                }
+
+                return option.value.kind === FieldValueKind.FUNCTION;
+              }}
+              filterAggregateParameters={option => {
+                // Only validate function parameters for timeseries widgets and
+                // world map widgets.
+                if (doNotValidateYAxis) {
+                  return true;
+                }
+
+                if (fieldValue.kind !== 'function') {
+                  return true;
+                }
+
+                const functionName = fieldValue.function[0];
                 const primaryOutput = aggregateFunctionOutputType(
-                  option.value.meta.name,
-                  undefined
+                  functionName as string,
+                  option.value.meta.name
                 );
                 if (primaryOutput) {
-                  // If a function returns a specific type, then validate it.
                   return isLegalYAxisType(primaryOutput);
                 }
-              }
-
-              return option.value.kind === FieldValueKind.FUNCTION;
-            }}
-            filterAggregateParameters={option => {
-              // Only validate function parameters for timeseries widgets.
-              if (doNotValidateYAxis) {
-                return true;
-              }
-
-              if (option.value.kind === FieldValueKind.FUNCTION) {
-                // Functions are not legal options as an aggregate/function parameter.
-                return false;
-              }
-
-              return isLegalYAxisType(option.value.meta.dataType);
-            }}
-          />
-          {fields.length > 1 && (
-            <Button
-              size="zero"
-              borderless
-              onClick={event => handleRemove(event, i)}
-              icon={<IconDelete />}
-              title={t('Remove this Y-Axis')}
-              label={t('Remove this Y-Axis')}
+
+                if (option.value.kind === FieldValueKind.FUNCTION) {
+                  // Functions are not legal options as an aggregate/function parameter.
+                  return false;
+                }
+
+                return isLegalYAxisType(option.value.meta.dataType);
+              }}
             />
-          )}
-        </QueryFieldWrapper>
-      ))}
+            {fields.length > 1 && (
+              <Button
+                size="zero"
+                borderless
+                onClick={event => handleRemove(event, i)}
+                icon={<IconDelete />}
+                title={t('Remove this Y-Axis')}
+                label={t('Remove this Y-Axis')}
+              />
+            )}
+          </QueryFieldWrapper>
+        );
+      })}
       {!hideAddYAxisButton && (
         <div>
           <Button size="small" icon={<IconAdd isCircled />} onClick={handleAdd}>

+ 10 - 1
src/sentry/static/sentry/app/components/modals/addDashboardWidgetModal.tsx

@@ -18,7 +18,11 @@ import {PanelAlert} from 'app/components/panels';
 import {t} from 'app/locale';
 import space from 'app/styles/space';
 import {GlobalSelection, Organization, TagCollection} from 'app/types';
-import {isAggregateField} from 'app/utils/discover/fields';
+import {
+  aggregateOutputType,
+  isAggregateField,
+  isLegalYAxisType,
+} from 'app/utils/discover/fields';
 import Measurements from 'app/utils/measurements/measurements';
 import withApi from 'app/utils/withApi';
 import withGlobalSelection from 'app/utils/withGlobalSelection';
@@ -112,6 +116,11 @@ function normalizeQueries(
   queries = queries.map(query => {
     let fields = query.fields.filter(isAggregateField);
 
+    if (isTimeseriesChart || displayType === 'world_map') {
+      // Filter out fields that will not generate numeric output types
+      fields = fields.filter(field => isLegalYAxisType(aggregateOutputType(field)));
+    }
+
     if (isTimeseriesChart && fields.length && fields.length > 3) {
       // Timeseries charts supports at most 3 fields.
       fields = fields.slice(0, 3);

+ 80 - 3
tests/js/spec/components/modals/addDashboardWidgetModal.spec.jsx

@@ -494,7 +494,7 @@ describe('Modals -> AddDashboardWidgetModal', function () {
     expect(widget.queries[0].fields).toEqual(['count_unique(user.display)']);
   });
 
-  it('should not filter y-axis choices for world map widget charts', async function () {
+  it('should filter y-axis choices for world map widget charts', async function () {
     let widget = undefined;
     const wrapper = mountModal({
       initialData,
@@ -507,23 +507,100 @@ describe('Modals -> AddDashboardWidgetModal', function () {
     selectByLabel(wrapper, 'World Map', {name: 'displayType', at: 0, control: true});
     expect(getDisplayType(wrapper).props().value).toEqual('world_map');
 
+    // Choose any()
+    selectByLabel(wrapper, 'any(\u2026)', {
+      name: 'field',
+      at: 0,
+      control: true,
+    });
+
+    // user.display should be filtered out for any()
+    const option = getOptionByLabel(wrapper, 'user.display', {
+      name: 'parameter',
+      at: 0,
+      control: true,
+    });
+    expect(option.exists()).toEqual(false);
+
+    selectByLabel(wrapper, 'measurements.lcp', {
+      name: 'parameter',
+      at: 0,
+      control: true,
+    });
+
+    // Choose count_unique()
     selectByLabel(wrapper, 'count_unique(\u2026)', {
       name: 'field',
       at: 0,
       control: true,
     });
 
-    // Be able to choose a non numeric-like option for count_unique()
+    // user.display not should be filtered out for count_unique()
     selectByLabel(wrapper, 'user.display', {
       name: 'parameter',
       at: 0,
       control: true,
     });
 
+    // Be able to choose a numeric-like option
+    selectByLabel(wrapper, 'measurements.lcp', {
+      name: 'parameter',
+      at: 0,
+      control: true,
+    });
+
     await clickSubmit(wrapper);
 
     expect(widget.displayType).toEqual('world_map');
     expect(widget.queries).toHaveLength(1);
-    expect(widget.queries[0].fields).toEqual(['count_unique(user.display)']);
+    expect(widget.queries[0].fields).toEqual(['count_unique(measurements.lcp)']);
+  });
+
+  it('should filter y-axis choices by output type when switching from big number to line chart', async function () {
+    let widget = undefined;
+    const wrapper = mountModal({
+      initialData,
+      onAddWidget: data => (widget = data),
+    });
+    // No delete button as there is only one field.
+    expect(wrapper.find('IconDelete')).toHaveLength(0);
+
+    // Select Big Number display
+    selectByLabel(wrapper, 'Big Number', {name: 'displayType', at: 0, control: true});
+    expect(getDisplayType(wrapper).props().value).toEqual('big_number');
+
+    // Choose any()
+    selectByLabel(wrapper, 'any(\u2026)', {
+      name: 'field',
+      at: 0,
+      control: true,
+    });
+
+    selectByLabel(wrapper, 'id', {
+      name: 'parameter',
+      at: 0,
+      control: true,
+    });
+
+    // Select Line chart display
+    selectByLabel(wrapper, 'Line Chart', {name: 'displayType', at: 0, control: true});
+    expect(getDisplayType(wrapper).props().value).toEqual('line');
+
+    // Expect event.type field to be converted to count()
+    const fieldColumn = wrapper.find('input[name="field"]');
+    expect(fieldColumn.length).toEqual(1);
+    expect(fieldColumn.props().value).toMatchObject({
+      kind: 'function',
+      meta: {
+        name: 'count',
+        parameters: [],
+      },
+    });
+
+    await clickSubmit(wrapper);
+
+    expect(widget.displayType).toEqual('line');
+    expect(widget.queries).toHaveLength(1);
+    expect(widget.queries[0].fields).toEqual(['count()']);
   });
 });