feat(dashboard): Split column and yaxis change handlers (#35635)

Simplify callbacks that are fired when table column
or yaxis is changed. This PR also removes code
paths for non-widget builder new design conditions

static/app/views/dashboardsV2/datasetConfig/base.tsx

@@ -1,11 +1,16 @@
+import trimStart from 'lodash/trimStart';
+
 import {OrganizationSummary, PageFilters, SelectValue, TagCollection} from 'sentry/types';
 import {Series} from 'sentry/types/echarts';
 import {TableData} from 'sentry/utils/discover/discoverQuery';
 import {MetaType} from 'sentry/utils/discover/eventView';
 import {getFieldRenderer} from 'sentry/utils/discover/fieldRenderers';
+import {isEquation} from 'sentry/utils/discover/fields';
+import {FieldValueOption} from 'sentry/views/eventsV2/table/queryField';
 import {FieldValue} from 'sentry/views/eventsV2/table/types';
 
 import {DisplayType, WidgetQuery, WidgetType} from '../types';
+import {getNumEquations} from '../utils';
 
 import {ErrorsAndTransactionsConfig} from './errorsAndTransactions';
 import {IssuesConfig} from './issues';
@@ -48,6 +53,17 @@ export interface DatasetConfig<SeriesResponse, TableResponse> {
    * values in tables.
    */
   fieldHeaderMap?: Record<string, string>;
+  /**
+   * Filter the options available to the parameters list
+   * of an aggregate function in a table widget column on the
+   * Widget Builder.
+   */
+  filterTableAggregateParams?: (option: FieldValueOption) => boolean;
+  /**
+   * Filter the primary options available in a table widget
+   * columns on the Widget Builder.
+   */
+  filterTableOptions?: (option: FieldValueOption) => boolean;
   /**
    * Used to select custom renderers for field types.
    */
@@ -56,6 +72,16 @@ export interface DatasetConfig<SeriesResponse, TableResponse> {
     meta: MetaType,
     contextualProps?: ContextualProps
   ) => ReturnType<typeof getFieldRenderer> | null;
+  /**
+   * Apply dataset specific overrides to the logic that handles
+   * column updates for tables in the Widget Builder.
+   */
+  handleColumnFieldChangeOverride?: (widgetQuery: WidgetQuery) => WidgetQuery;
+  /**
+   * Called on column or y-axis change in the Widget Builder
+   * to reset the orderby of the widget query.
+   */
+  handleOrderByReset?: (widgetQuery: WidgetQuery, newFields: string[]) => WidgetQuery;
   /**
    * Transforms timeseries API results into series data that is
    * ingestable by echarts for timeseries visualizations.
@@ -88,3 +114,32 @@ export function getDatasetConfig(
       return ErrorsAndTransactionsConfig;
   }
 }
+
+/**
+ * A generic orderby reset helper function that updates the query's
+ * orderby based on new selected fields.
+ */
+export function handleOrderByReset(
+  widgetQuery: WidgetQuery,
+  newFields: string[]
+): WidgetQuery {
+  const rawOrderby = trimStart(widgetQuery.orderby, '-');
+  const isDescending = widgetQuery.orderby.startsWith('-');
+  const orderbyPrefix = isDescending ? '-' : '';
+
+  if (!newFields.includes(rawOrderby) && widgetQuery.orderby !== '') {
+    const isFromAggregates = widgetQuery.aggregates.includes(rawOrderby);
+    const isCustomEquation = isEquation(rawOrderby);
+    const isUsedInGrouping = widgetQuery.columns.includes(rawOrderby);
+
+    const keepCurrentOrderby = isFromAggregates || isCustomEquation || isUsedInGrouping;
+    const firstAggregateAlias = isEquation(widgetQuery.aggregates[0] ?? '')
+      ? `equation[${getNumEquations(widgetQuery.aggregates) - 1}]`
+      : newFields[0];
+
+    widgetQuery.orderby =
+      (keepCurrentOrderby && widgetQuery.orderby) ||
+      `${orderbyPrefix}${firstAggregateAlias}`;
+  }
+  return widgetQuery;
+}

static/app/views/dashboardsV2/datasetConfig/errorsAndTransactions.tsx

@@ -27,7 +27,7 @@ import {
   transformSeries,
 } from '../widgetCard/widgetQueries';
 
-import {ContextualProps, DatasetConfig} from './base';
+import {ContextualProps, DatasetConfig, handleOrderByReset} from './base';
 
 const DEFAULT_WIDGET_QUERY: WidgetQuery = {
   name: '',
@@ -48,6 +48,7 @@ export const ErrorsAndTransactionsConfig: DatasetConfig<
   defaultWidgetQuery: DEFAULT_WIDGET_QUERY,
   getCustomFieldRenderer: getCustomEventsFieldRenderer,
   getTableFieldOptions: getEventsTableFieldOptions,
+  handleOrderByReset,
   supportedDisplayTypes: [
     DisplayType.AREA,
     DisplayType.BAR,

static/app/views/dashboardsV2/datasetConfig/releases.tsx

@@ -6,6 +6,8 @@ import {Series} from 'sentry/types/echarts';
 import {defined} from 'sentry/utils';
 import {TableData} from 'sentry/utils/discover/discoverQuery';
 import {getFieldRenderer} from 'sentry/utils/discover/fieldRenderers';
+import {FieldValueOption} from 'sentry/views/eventsV2/table/queryField';
+import {FieldValueKind} from 'sentry/views/eventsV2/table/types';
 
 import {DisplayType, WidgetQuery} from '../types';
 import {
@@ -25,7 +27,7 @@ import {
   mapDerivedMetricsToFields,
 } from '../widgetCard/transformSessionsResponseToTable';
 
-import {DatasetConfig} from './base';
+import {DatasetConfig, handleOrderByReset} from './base';
 
 const DEFAULT_WIDGET_QUERY: WidgetQuery = {
   name: '',
@@ -42,8 +44,12 @@ export const ReleasesConfig: DatasetConfig<
   SessionApiResponse | MetricsApiResponse
 > = {
   defaultWidgetQuery: DEFAULT_WIDGET_QUERY,
+  filterTableOptions: filterPrimaryReleaseTableOptions,
+  filterTableAggregateParams: filterAggregateParams,
   getCustomFieldRenderer: (field, meta) => getFieldRenderer(field, meta, false),
   getTableFieldOptions: getReleasesTableFieldOptions,
+  handleColumnFieldChangeOverride,
+  handleOrderByReset: handleReleasesTableOrderByReset,
   supportedDisplayTypes: [
     DisplayType.AREA,
     DisplayType.BAR,
@@ -56,6 +62,38 @@ export const ReleasesConfig: DatasetConfig<
   transformTable: transformSessionsResponseToTable,
 };
 
+function filterPrimaryReleaseTableOptions(option: FieldValueOption) {
+  return [
+    FieldValueKind.FUNCTION,
+    FieldValueKind.FIELD,
+    FieldValueKind.NUMERIC_METRICS,
+  ].includes(option.value.kind);
+}
+
+function filterAggregateParams(option: FieldValueOption) {
+  return option.value.kind === FieldValueKind.METRICS;
+}
+
+function handleReleasesTableOrderByReset(widgetQuery: WidgetQuery, newFields: string[]) {
+  const disableSortBy = widgetQuery.columns.includes('session.status');
+  if (disableSortBy) {
+    widgetQuery.orderby = '';
+  }
+  return handleOrderByReset(widgetQuery, newFields);
+}
+
+function handleColumnFieldChangeOverride(widgetQuery: WidgetQuery): WidgetQuery {
+  if (widgetQuery.aggregates.length === 0) {
+    // Release Health widgets require an aggregate in tables
+    const defaultReleaseHealthAggregate = `crash_free_rate(${SessionField.SESSION})`;
+    widgetQuery.aggregates = [defaultReleaseHealthAggregate];
+    widgetQuery.fields = widgetQuery.fields
+      ? [...widgetQuery.fields, defaultReleaseHealthAggregate]
+      : [defaultReleaseHealthAggregate];
+  }
+  return widgetQuery;
+}
+
 function getReleasesTableFieldOptions() {
   return generateReleaseWidgetFieldOptions(Object.values(SESSIONS_FIELDS), SESSIONS_TAGS);
 }

static/app/views/dashboardsV2/widgetBuilder/buildSteps/columnsStep/index.tsx

@@ -1,13 +1,7 @@
-import cloneDeep from 'lodash/cloneDeep';
-
 import ExternalLink from 'sentry/components/links/externalLink';
 import {t, tct} from 'sentry/locale';
 import {Organization, TagCollection} from 'sentry/types';
-import {
-  generateFieldAsString,
-  getColumnsAndAggregatesAsStrings,
-  QueryFieldValue,
-} from 'sentry/utils/discover/fields';
+import {QueryFieldValue} from 'sentry/utils/discover/fields';
 import {getDatasetConfig} from 'sentry/views/dashboardsV2/datasetConfig/base';
 import {DisplayType, WidgetQuery, WidgetType} from 'sentry/views/dashboardsV2/types';
 
@@ -15,14 +9,13 @@ import {DataSet} from '../../utils';
 import {BuildStep} from '../buildStep';
 
 import {ColumnFields} from './columnFields';
-import {ReleaseColumnFields} from './releaseColumnFields';
 
 interface Props {
   dataSet: DataSet;
   displayType: DisplayType;
   explodedFields: QueryFieldValue[];
+  handleColumnFieldChange: (newFields: QueryFieldValue[]) => void;
   onQueryChange: (queryIndex: number, newQuery: WidgetQuery) => void;
-  onYAxisOrColumnFieldChange: (newFields: QueryFieldValue[]) => void;
   organization: Organization;
   queries: WidgetQuery[];
   tags: TagCollection;
@@ -33,11 +26,9 @@ interface Props {
 export function ColumnsStep({
   dataSet,
   displayType,
-  onQueryChange,
   organization,
-  queries,
   widgetType,
-  onYAxisOrColumnFieldChange,
+  handleColumnFieldChange,
   queryErrors,
   explodedFields,
   tags,
@@ -81,45 +72,17 @@ export function ColumnsStep({
             )
       }
     >
-      {dataSet === DataSet.EVENTS ? (
-        <ColumnFields
-          displayType={displayType}
-          organization={organization}
-          widgetType={widgetType}
-          fields={explodedFields}
-          errors={queryErrors}
-          fieldOptions={datasetConfig.getTableFieldOptions({organization}, tags)}
-          onChange={onYAxisOrColumnFieldChange}
-        />
-      ) : dataSet === DataSet.ISSUES ? (
-        <ColumnFields
-          displayType={displayType}
-          organization={organization}
-          widgetType={widgetType}
-          fields={explodedFields}
-          errors={queryErrors?.[0] ? [queryErrors?.[0]] : undefined}
-          fieldOptions={datasetConfig.getTableFieldOptions({organization}, tags)}
-          onChange={newFields => {
-            const fieldStrings = newFields.map(generateFieldAsString);
-            const splitFields = getColumnsAndAggregatesAsStrings(newFields);
-            const newQuery = cloneDeep(queries[0]);
-            newQuery.fields = fieldStrings;
-            newQuery.aggregates = splitFields.aggregates;
-            newQuery.columns = splitFields.columns;
-            newQuery.fieldAliases = splitFields.fieldAliases;
-            onQueryChange(0, newQuery);
-          }}
-        />
-      ) : (
-        <ReleaseColumnFields
-          displayType={displayType}
-          organization={organization}
-          widgetType={widgetType}
-          explodedFields={explodedFields}
-          queryErrors={queryErrors}
-          onYAxisOrColumnFieldChange={onYAxisOrColumnFieldChange}
-        />
-      )}
+      <ColumnFields
+        displayType={displayType}
+        organization={organization}
+        widgetType={widgetType}
+        fields={explodedFields}
+        errors={queryErrors}
+        fieldOptions={datasetConfig.getTableFieldOptions({organization}, tags)}
+        filterAggregateParameters={datasetConfig.filterTableAggregateParams}
+        filterPrimaryOptions={datasetConfig.filterTableOptions}
+        onChange={handleColumnFieldChange}
+      />
     </BuildStep>
   );
 }

static/app/views/dashboardsV2/widgetBuilder/buildSteps/columnsStep/releaseColumnFields.tsx

@@ -1,51 +0,0 @@
-import {Organization} from 'sentry/types';
-import {QueryFieldValue} from 'sentry/utils/discover/fields';
-import {getDatasetConfig} from 'sentry/views/dashboardsV2/datasetConfig/base';
-import {DisplayType, WidgetType} from 'sentry/views/dashboardsV2/types';
-import {filterPrimaryOptions} from 'sentry/views/dashboardsV2/widgetBuilder/utils';
-import {FieldValueOption} from 'sentry/views/eventsV2/table/queryField';
-import {FieldValueKind} from 'sentry/views/eventsV2/table/types';
-
-import {ColumnFields} from './columnFields';
-
-interface Props {
-  displayType: DisplayType;
-  explodedFields: QueryFieldValue[];
-  onYAxisOrColumnFieldChange: (newFields: QueryFieldValue[]) => void;
-  organization: Organization;
-  widgetType: WidgetType;
-  queryErrors?: Record<string, any>[];
-}
-
-export function ReleaseColumnFields({
-  displayType,
-  organization,
-  widgetType,
-  explodedFields,
-  queryErrors,
-  onYAxisOrColumnFieldChange,
-}: Props) {
-  const datasetConfig = getDatasetConfig(WidgetType.RELEASE);
-  const filterAggregateParameters = (option: FieldValueOption) => {
-    return option.value.kind === FieldValueKind.METRICS;
-  };
-  return (
-    <ColumnFields
-      displayType={displayType}
-      organization={organization}
-      widgetType={widgetType}
-      fields={explodedFields}
-      errors={queryErrors?.[0] ? [queryErrors?.[0]] : undefined}
-      fieldOptions={datasetConfig.getTableFieldOptions()}
-      filterAggregateParameters={filterAggregateParameters}
-      filterPrimaryOptions={option =>
-        filterPrimaryOptions({
-          option,
-          widgetType,
-          displayType,
-        })
-      }
-      onChange={onYAxisOrColumnFieldChange}
-    />
-  );
-}

static/app/views/dashboardsV2/widgetBuilder/widgetBuilder.tsx

@@ -30,7 +30,6 @@ import {
   Organization,
   PageFilters,
   SelectValue,
-  SessionField,
   TagCollection,
 } from 'sentry/types';
 import {defined, objectIsEmpty} from 'sentry/utils';
@@ -43,8 +42,6 @@ import {
   getColumnsAndAggregatesAsStrings,
   isEquation,
   QueryFieldValue,
-  stripDerivedMetricsPrefix,
-  stripEquationPrefix,
 } from 'sentry/utils/discover/fields';
 import handleXhrErrorResponse from 'sentry/utils/handleXhrErrorResponse';
 import useApi from 'sentry/utils/useApi';
@@ -611,50 +608,38 @@ function WidgetBuilder({
     });
   }
 
-  function handleYAxisOrColumnFieldChange(
-    newFields: QueryFieldValue[],
-    isColumn = false
-  ) {
-    const fieldStrings = newFields
-      .map(generateFieldAsString)
-      .map(field =>
-        state.dataSet === DataSet.RELEASES ? stripDerivedMetricsPrefix(field) : field
-      );
+  function handleColumnFieldChange(newFields: QueryFieldValue[]) {
+    const fieldStrings = newFields.map(generateFieldAsString);
+    const splitFields = getColumnsAndAggregatesAsStrings(newFields);
+    const newState = cloneDeep(state);
+    let newQuery = cloneDeep(newState.queries[0]);
 
-    const columnsAndAggregates = isColumn
-      ? getColumnsAndAggregatesAsStrings(newFields)
-      : undefined;
+    newQuery.fields = fieldStrings;
+    newQuery.aggregates = splitFields.aggregates;
+    newQuery.columns = splitFields.columns;
+    newQuery.fieldAliases = splitFields.fieldAliases;
 
-    const newState = cloneDeep(state);
+    if (datasetConfig.handleColumnFieldChangeOverride) {
+      newQuery = datasetConfig.handleColumnFieldChangeOverride(newQuery);
+    }
 
-    const disableSortBy =
-      widgetType === WidgetType.RELEASE && fieldStrings.includes('session.status');
+    if (datasetConfig.handleOrderByReset) {
+      newQuery = datasetConfig.handleOrderByReset(newQuery, fieldStrings);
+    }
 
-    const newQueries = state.queries.map(query => {
-      const isDescending = query.orderby.startsWith('-');
-      const orderbyPrefix = isDescending ? '-' : '';
-      const rawOrderby = trimStart(query.orderby, '-');
-      const prevAggregateFieldStrings = query.aggregates.map(aggregate =>
-        state.dataSet === DataSet.RELEASES
-          ? stripDerivedMetricsPrefix(aggregate)
-          : aggregate
-      );
-      const newQuery = cloneDeep(query);
+    set(newState, 'queries', [newQuery]);
+    set(newState, 'userHasModified', true);
+    setState(newState);
+  }
 
-      if (disableSortBy) {
-        newQuery.orderby = '';
-      }
+  function handleYAxisChange(newFields: QueryFieldValue[]) {
+    const fieldStrings = newFields.map(generateFieldAsString);
+    const newState = cloneDeep(state);
 
-      if (isColumn) {
-        newQuery.fields = fieldStrings;
-        newQuery.aggregates = columnsAndAggregates?.aggregates ?? [];
-        if (state.dataSet === DataSet.RELEASES && newQuery.aggregates.length === 0) {
-          // Release Health widgets require an aggregate in tables
-          const defaultReleaseHealthAggregate = `crash_free_rate(${SessionField.SESSION})`;
-          newQuery.aggregates = [defaultReleaseHealthAggregate];
-          newQuery.fields = [...newQuery.fields, defaultReleaseHealthAggregate];
-        }
-      } else if (state.displayType === DisplayType.TOP_N) {
+    const newQueries = state.queries.map(query => {
+      let newQuery = cloneDeep(query);
+
+      if (state.displayType === DisplayType.TOP_N) {
         // Top N queries use n-1 fields for columns and the nth field for y-axis
         newQuery.fields = [
           ...(newQuery.fields?.slice(0, newQuery.fields.length - 1) ?? []),
@@ -669,46 +654,8 @@ function WidgetBuilder({
         newQuery.aggregates = fieldStrings;
       }
 
-      // Prevent overwriting columns when setting y-axis for time series
-      if (!(widgetBuilderNewDesign && isTimeseriesChart) && isColumn) {
-        newQuery.columns = columnsAndAggregates?.columns ?? [];
-      }
-
-      if (!fieldStrings.includes(rawOrderby) && query.orderby !== '') {
-        if (
-          !widgetBuilderNewDesign &&
-          prevAggregateFieldStrings.length === newFields.length &&
-          prevAggregateFieldStrings.includes(rawOrderby)
-        ) {
-          // The aggregate that was used in orderby has changed. Get the new field.
-          let newOrderByValue =
-            fieldStrings[prevAggregateFieldStrings.indexOf(rawOrderby)];
-
-          if (!stripEquationPrefix(newOrderByValue ?? '')) {
-            newOrderByValue = '';
-          }
-
-          newQuery.orderby = `${orderbyPrefix}${newOrderByValue}`;
-        } else {
-          const isFromAggregates = newQuery.aggregates.includes(rawOrderby);
-          const isCustomEquation = isEquation(rawOrderby);
-          const isUsedInGrouping = newQuery.columns.includes(rawOrderby);
-
-          const keepCurrentOrderby =
-            isFromAggregates || isCustomEquation || isUsedInGrouping;
-          const firstAggregateAlias = isEquation(newQuery.aggregates[0] ?? '')
-            ? `equation[${getNumEquations(newQuery.aggregates) - 1}]`
-            : fieldStrings[0];
-
-          newQuery.orderby = widgetBuilderNewDesign
-            ? (keepCurrentOrderby && newQuery.orderby) ||
-              `${orderbyPrefix}${firstAggregateAlias}`
-            : '';
-        }
-      }
-
-      if (widgetBuilderNewDesign) {
-        newQuery.fieldAliases = columnsAndAggregates?.fieldAliases ?? [];
+      if (datasetConfig.handleOrderByReset) {
+        newQuery = datasetConfig.handleOrderByReset(newQuery, fieldStrings);
       }
 
       return newQuery;
@@ -717,22 +664,20 @@ function WidgetBuilder({
     set(newState, 'queries', newQueries);
     set(newState, 'userHasModified', true);
 
-    if (widgetBuilderNewDesign && isTimeseriesChart) {
-      const groupByFields = newState.queries[0].columns.filter(
-        field => !(field === 'equation|')
+    const groupByFields = newState.queries[0].columns.filter(
+      field => !(field === 'equation|')
+    );
+    if (groupByFields.length === 0) {
+      set(newState, 'limit', undefined);
+    } else {
+      set(
+        newState,
+        'limit',
+        Math.min(
+          newState.limit ?? DEFAULT_RESULTS_LIMIT,
+          getResultsLimit(newQueries.length, newQueries[0].aggregates.length)
+        )
       );
-      if (groupByFields.length === 0) {
-        set(newState, 'limit', undefined);
-      } else {
-        set(
-          newState,
-          'limit',
-          Math.min(
-            newState.limit ?? DEFAULT_RESULTS_LIMIT,
-            getResultsLimit(newQueries.length, newQueries[0].aggregates.length)
-          )
-        );
-      }
     }
 
     setState(newState);
@@ -1123,9 +1068,7 @@ function WidgetBuilder({
                       widgetType={widgetType}
                       queryErrors={state.errors?.queries}
                       onQueryChange={handleQueryChange}
-                      onYAxisOrColumnFieldChange={newFields => {
-                        handleYAxisOrColumnFieldChange(newFields, true);
-                      }}
+                      handleColumnFieldChange={handleColumnFieldChange}
                       explodedFields={explodedFields}
                       tags={tags}
                       organization={organization}
@@ -1138,7 +1081,7 @@ function WidgetBuilder({
                       widgetType={widgetType}
                       queryErrors={state.errors?.queries}
                       onYAxisChange={newFields => {
-                        handleYAxisOrColumnFieldChange(newFields);
+                        handleYAxisChange(newFields);
                       }}
                       aggregates={explodedAggregates}
                       tags={tags}