Browse Source

feat(dashboards): Update the widget builder state on split decision (#74864)

Update the widget builder dataset state when a split decision has been made.
Nar Saynorath 7 months ago
parent
commit
adba3a1232

+ 2 - 0
static/app/types/organization.tsx

@@ -5,6 +5,7 @@ import type {
   DiscoverDatasets,
   SavedQueryDatasets,
 } from 'sentry/utils/discover/types';
+import type {WidgetType} from 'sentry/views/dashboards/types';
 
 import type {Actor, Avatar, ObjectStatus, Scope} from './core';
 import type {OrgExperiments} from './experiments';
@@ -293,6 +294,7 @@ export type EventsStats = {
     isMetricsData: boolean;
     tips: {columns?: string; query?: string};
     units: Record<string, string>;
+    discoverSplitDecision?: WidgetType;
     isMetricsExtractedData?: boolean;
   };
   order?: number;

+ 2 - 0
static/app/utils/discover/eventView.tsx

@@ -43,6 +43,7 @@ import {
 import {statsPeriodToDays} from 'sentry/utils/duration/statsPeriodToDays';
 import {decodeList, decodeScalar, decodeSorts} from 'sentry/utils/queryString';
 import normalizeUrl from 'sentry/utils/url/normalizeUrl';
+import type {WidgetType} from 'sentry/views/dashboards/types';
 import {getSavedQueryDatasetFromLocationOrDataset} from 'sentry/views/discover/savedQuery/utils';
 import type {TableColumn, TableColumnSort} from 'sentry/views/discover/table/types';
 import {FieldValueKind} from 'sentry/views/discover/table/types';
@@ -65,6 +66,7 @@ export type MetaType = Record<string, any> & {
 export type EventsMetaType = {fields: Record<string, ColumnType>} & {
   units: Record<string, string>;
 } & {
+  discoverSplitDecision?: WidgetType;
   isMetricsData?: boolean;
   isMetricsExtractedData?: boolean;
 };

+ 7 - 1
static/app/views/dashboards/addWidget.tsx

@@ -43,6 +43,12 @@ function AddWidget({onAddWidget}: Props) {
 
   const organization = useOrganization();
 
+  const defaultDataset = organization.features.includes(
+    'performance-discover-dataset-selector'
+  )
+    ? DataSet.ERRORS
+    : DataSet.EVENTS;
+
   return (
     <Feature features="dashboards-edit">
       <WidgetWrapper
@@ -74,7 +80,7 @@ function AddWidget({onAddWidget}: Props) {
             />
           </InnerWrapper>
         ) : (
-          <InnerWrapper onClick={() => onAddWidget(DataSet.EVENTS)}>
+          <InnerWrapper onClick={() => onAddWidget(defaultDataset)}>
             <AddButton
               data-test-id="widget-add"
               icon={<IconAdd size="lg" isCircled color="inactive" />}

+ 7 - 1
static/app/views/dashboards/controls.tsx

@@ -131,6 +131,12 @@ function Controls({
     );
   }
 
+  const defaultDataset = organization.features.includes(
+    'performance-discover-dataset-selector'
+  )
+    ? DataSet.ERRORS
+    : DataSet.EVENTS;
+
   return (
     <StyledButtonBar gap={1} key="controls">
       <DashboardEditFeature>
@@ -190,7 +196,7 @@ function Controls({
                       trackAnalytics('dashboards_views.widget_library.opened', {
                         organization,
                       });
-                      onAddWidget(DataSet.EVENTS);
+                      onAddWidget(defaultDataset);
                     }}
                   >
                     {t('Add Widget')}

+ 28 - 6
static/app/views/dashboards/datasetConfig/errorsAndTransactions.tsx

@@ -635,6 +635,13 @@ function getEventsSeriesRequest(
     return doOnDemandMetricsRequest(api, requestData);
   }
 
+  if (organization.features.includes('performance-discover-dataset-selector')) {
+    requestData.queryExtras = {
+      ...requestData.queryExtras,
+      ...getQueryExtraForSplittingDiscover(widget, organization, false),
+    };
+  }
+
   return doEventsRequest<true>(api, requestData);
 }
 
@@ -687,20 +694,35 @@ export function filterAggregateParams(
   return true;
 }
 
-const shouldSendWidgetForSplittingDiscover = (organization: Organization) => {
-  return organization.features.includes('performance-discover-widget-split-ui');
-};
-
 const getQueryExtraForSplittingDiscover = (
   widget: Widget,
   organization: Organization,
   useOnDemandMetrics: boolean
 ) => {
-  if (!useOnDemandMetrics || !shouldSendWidgetForSplittingDiscover(organization)) {
+  // We want to send the dashboardWidgetId on the request if we're in the Widget
+  // Builder with the selector feature flag
+  const isEditing = location.pathname.endsWith('/edit/');
+  const hasDiscoverSelector = organization.features.includes(
+    'performance-discover-dataset-selector'
+  );
+
+  if (!hasDiscoverSelector) {
+    if (
+      !useOnDemandMetrics ||
+      !organization.features.includes('performance-discover-widget-split-ui')
+    ) {
+      return {};
+    }
+    if (widget.id) {
+      return {dashboardWidgetId: widget.id};
+    }
+
     return {};
   }
-  if (widget.id) {
+
+  if (isEditing && widget.id) {
     return {dashboardWidgetId: widget.id};
   }
+
   return {};
 };

+ 3 - 1
static/app/views/dashboards/widgetBuilder/buildSteps/dataSetStep.tsx

@@ -43,7 +43,9 @@ export function DataSetStep({
     datasetChoices.set(DataSet.TRANSACTIONS, t('Transactions'));
   }
 
-  datasetChoices.set(DataSet.EVENTS, t('Errors and Transactions'));
+  if (!organization.features.includes('performance-discover-dataset-selector')) {
+    datasetChoices.set(DataSet.EVENTS, t('Errors and Transactions'));
+  }
   datasetChoices.set(DataSet.ISSUES, t('Issues (States, Assignment, Time, etc.)'));
 
   if (hasReleaseHealthFeature) {

+ 4 - 1
static/app/views/dashboards/widgetBuilder/buildSteps/visualizationStep.tsx

@@ -17,7 +17,7 @@ import type {PageFilters, SelectValue} from 'sentry/types/core';
 import type {Organization} from 'sentry/types/organization';
 import type {TableDataWithTitle} from 'sentry/utils/discover/discoverQuery';
 import usePrevious from 'sentry/utils/usePrevious';
-import type {DashboardFilters, Widget} from 'sentry/views/dashboards/types';
+import type {DashboardFilters, Widget, WidgetType} from 'sentry/views/dashboards/types';
 import {DisplayType} from 'sentry/views/dashboards/types';
 
 import {getDashboardFiltersFromURL} from '../../utils';
@@ -38,6 +38,7 @@ interface Props {
   error?: string;
   noDashboardsMEPProvider?: boolean;
   onDataFetched?: (results: TableDataWithTitle[]) => void;
+  onWidgetSplitDecision?: (splitDecision: WidgetType) => void;
 }
 
 export function VisualizationStep({
@@ -52,6 +53,7 @@ export function VisualizationStep({
   dashboardFilters,
   location,
   isWidgetInvalid,
+  onWidgetSplitDecision,
 }: Props) {
   const [debouncedWidget, setDebouncedWidget] = useState(widget);
 
@@ -129,6 +131,7 @@ export function VisualizationStep({
           noDashboardsMEPProvider={noDashboardsMEPProvider}
           isWidgetInvalid={isWidgetInvalid}
           onDataFetched={onDataFetched}
+          onWidgetSplitDecision={onWidgetSplitDecision}
         />
       </VisualizationWrapper>
     </StyledBuildStep>

+ 94 - 0
static/app/views/dashboards/widgetBuilder/widgetBuilder.spec.tsx

@@ -1687,6 +1687,100 @@ describe('WidgetBuilder', function () {
     expect(eventsStatsMock).toHaveBeenCalledTimes(1);
   });
 
+  describe('discover dataset split', function () {
+    describe('events', function () {
+      it('selects the error discover split type as the dataset when the events request completes', async function () {
+        eventsMock = MockApiClient.addMockResponse({
+          url: '/organizations/org-slug/events/',
+          method: 'GET',
+          statusCode: 200,
+          body: {
+            meta: {discoverSplitDecision: WidgetType.ERRORS},
+            data: [],
+          },
+        });
+        renderTestComponent({
+          orgFeatures: [...defaultOrgFeatures, 'performance-discover-dataset-selector'],
+        });
+
+        await waitFor(() => {
+          expect(eventsMock).toHaveBeenCalled();
+        });
+
+        expect(screen.getByRole('radio', {name: /errors/i})).toBeChecked();
+      });
+
+      it('selects the transaction discover split type as the dataset when the events request completes', async function () {
+        eventsMock = MockApiClient.addMockResponse({
+          url: '/organizations/org-slug/events/',
+          method: 'GET',
+          statusCode: 200,
+          body: {
+            meta: {discoverSplitDecision: WidgetType.TRANSACTIONS},
+            data: [],
+          },
+        });
+        renderTestComponent({
+          orgFeatures: [...defaultOrgFeatures, 'performance-discover-dataset-selector'],
+        });
+
+        await waitFor(() => {
+          expect(eventsMock).toHaveBeenCalled();
+        });
+
+        expect(screen.getByRole('radio', {name: /transactions/i})).toBeChecked();
+      });
+    });
+
+    describe('events-stats', function () {
+      it('selects the error discover split type as the dataset when the request completes', async function () {
+        eventsStatsMock = MockApiClient.addMockResponse({
+          url: '/organizations/org-slug/events-stats/',
+          method: 'GET',
+          statusCode: 200,
+          body: {
+            meta: {discoverSplitDecision: WidgetType.ERRORS},
+            data: [],
+          },
+        });
+        renderTestComponent({
+          orgFeatures: [...defaultOrgFeatures, 'performance-discover-dataset-selector'],
+          // Triggers an events-stats request because line charts are for timeseries
+          query: {displayType: 'line'},
+        });
+
+        await waitFor(() => {
+          expect(eventsStatsMock).toHaveBeenCalled();
+        });
+
+        expect(screen.getByRole('radio', {name: /errors/i})).toBeChecked();
+      });
+
+      it('selects the transaction discover split type as the dataset when the request completes', async function () {
+        eventsStatsMock = MockApiClient.addMockResponse({
+          url: '/organizations/org-slug/events-stats/',
+          method: 'GET',
+          statusCode: 200,
+          body: {
+            meta: {discoverSplitDecision: WidgetType.TRANSACTIONS},
+            data: [],
+          },
+        });
+        renderTestComponent({
+          orgFeatures: [...defaultOrgFeatures, 'performance-discover-dataset-selector'],
+          // Triggers an events-stats request because line charts are for timeseries
+          query: {displayType: 'line'},
+        });
+
+        await waitFor(() => {
+          expect(eventsStatsMock).toHaveBeenCalled();
+        });
+
+        expect(screen.getByRole('radio', {name: /transactions/i})).toBeChecked();
+      });
+    });
+  });
+
   describe('Widget Library', function () {
     it('only opens the modal when the query data is changed', async function () {
       const mockModal = jest.spyOn(modals, 'openWidgetBuilderOverwriteModal');

+ 39 - 14
static/app/views/dashboards/widgetBuilder/widgetBuilder.tsx

@@ -154,6 +154,7 @@ interface State {
   dataUnit?: string;
   description?: string;
   errors?: Record<string, any>;
+  id?: string;
   selectedDashboard?: DashboardDetails['id'];
   thresholds?: ThresholdsConfig | null;
   widgetToBeUpdated?: Widget;
@@ -217,7 +218,17 @@ function WidgetBuilder({
     DashboardWidgetSource.ISSUE_DETAILS,
   ].includes(source);
 
-  const dataSet = dataset ? dataset : DataSet.EVENTS;
+  const defaultWidgetType = organization.features.includes(
+    'performance-discover-dataset-selector'
+  )
+    ? WidgetType.ERRORS
+    : WidgetType.DISCOVER;
+  const defaultDataset = organization.features.includes(
+    'performance-discover-dataset-selector'
+  )
+    ? DataSet.ERRORS
+    : DataSet.EVENTS;
+  const dataSet = dataset ? dataset : defaultDataset;
 
   const api = useApi();
 
@@ -314,7 +325,7 @@ function WidgetBuilder({
         queries = normalizeQueries({
           displayType: newDisplayType,
           queries: widgetFromDashboard.queries,
-          widgetType: widgetFromDashboard.widgetType ?? WidgetType.DISCOVER,
+          widgetType: widgetFromDashboard.widgetType ?? defaultWidgetType,
         }).map(query => ({
           ...query,
           // Use the last aggregate because that's where the y-axis is stored
@@ -326,11 +337,12 @@ function WidgetBuilder({
         queries = normalizeQueries({
           displayType: newDisplayType,
           queries: widgetFromDashboard.queries,
-          widgetType: widgetFromDashboard.widgetType ?? WidgetType.DISCOVER,
+          widgetType: widgetFromDashboard.widgetType ?? defaultWidgetType,
         });
       }
 
       setState({
+        id: widgetFromDashboard.id,
         title: widgetFromDashboard.title,
         description: widgetFromDashboard.description,
         displayType: newDisplayType,
@@ -342,7 +354,7 @@ function WidgetBuilder({
         thresholds: widgetFromDashboard.thresholds ?? defaultThresholds,
         dataSet: widgetFromDashboard.widgetType
           ? WIDGET_TYPE_TO_DATA_SET[widgetFromDashboard.widgetType]
-          : DataSet.EVENTS,
+          : defaultDataset,
         limit: newLimit,
         prebuiltWidgetId: null,
         queryConditionsValid: true,
@@ -372,6 +384,7 @@ function WidgetBuilder({
   const widgetType = DATA_SET_TO_WIDGET_TYPE[state.dataSet];
 
   const currentWidget = {
+    id: state.id,
     title: state.title,
     description: state.description,
     displayType: state.displayType,
@@ -417,12 +430,12 @@ function WidgetBuilder({
           'queries',
           normalizeQueries({
             displayType: newDisplayType,
-            queries: [{...getDatasetConfig(WidgetType.DISCOVER).defaultWidgetQuery}],
-            widgetType: WidgetType.DISCOVER,
+            queries: [{...getDatasetConfig(defaultWidgetType).defaultWidgetQuery}],
+            widgetType: defaultWidgetType,
           })
         );
-        set(newState, 'dataSet', DataSet.EVENTS);
-        setDataSetConfig(getDatasetConfig(WidgetType.DISCOVER));
+        set(newState, 'dataSet', defaultDataset);
+        setDataSetConfig(getDatasetConfig(defaultWidgetType));
         return {...newState, errors: undefined};
       }
 
@@ -717,7 +730,7 @@ function WidgetBuilder({
         newQuery.orderby = '';
       } else if (!newQuery.orderby) {
         const orderOptions = generateOrderOptions({
-          widgetType: widgetType ?? WidgetType.DISCOVER,
+          widgetType: widgetType ?? defaultWidgetType,
           columns: query.columns,
           aggregates: query.aggregates,
         });
@@ -827,7 +840,10 @@ function WidgetBuilder({
     if (widgetToBeUpdated) {
       let nextWidgetList = [...dashboard.widgets];
       const updateWidgetIndex = getUpdateWidgetIndex();
-      const nextWidgetData = {...widgetData, id: widgetToBeUpdated.id};
+      const nextWidgetData = {
+        ...widgetData,
+        id: widgetToBeUpdated.id,
+      };
 
       // Only modify and re-compact if the default height has changed
       if (
@@ -845,7 +861,7 @@ function WidgetBuilder({
       goToDashboards(dashboardId ?? NEW_DASHBOARD_ID);
       trackAnalytics('dashboards_views.widget_builder.save', {
         organization,
-        data_set: widgetData.widgetType ?? WidgetType.DISCOVER,
+        data_set: widgetData.widgetType ?? defaultWidgetType,
         new_widget: false,
       });
       return;
@@ -856,7 +872,7 @@ function WidgetBuilder({
     goToDashboards(dashboardId ?? NEW_DASHBOARD_ID);
     trackAnalytics('dashboards_views.widget_builder.save', {
       organization,
-      data_set: widgetData.widgetType ?? WidgetType.DISCOVER,
+      data_set: widgetData.widgetType ?? defaultWidgetType,
       new_widget: true,
     });
   }
@@ -995,6 +1011,12 @@ function WidgetBuilder({
     });
   }
 
+  function handleUpdateWidgetSplitDecision(splitDecision: WidgetType) {
+    setState(prevState => {
+      return {...cloneDeep(prevState), dataSet: WIDGET_TYPE_TO_DATA_SET[splitDecision]};
+    });
+  }
+
   function isFormInvalid() {
     if (
       (notDashboardsOrigin && !state.selectedDashboard) ||
@@ -1143,6 +1165,9 @@ function WidgetBuilder({
                                     }}
                                     noDashboardsMEPProvider
                                     isWidgetInvalid={!state.queryConditionsValid}
+                                    onWidgetSplitDecision={
+                                      handleUpdateWidgetSplitDecision
+                                    }
                                   />
                                   <DataSetStep
                                     dataSet={state.dataSet}
@@ -1265,7 +1290,7 @@ function WidgetBuilder({
                                   setLatestLibrarySelectionTitle(prebuiltWidget.title);
                                   setDataSetConfig(
                                     getDatasetConfig(
-                                      prebuiltWidget.widgetType || WidgetType.DISCOVER
+                                      prebuiltWidget.widgetType || defaultWidgetType
                                     )
                                   );
                                   const {id, ...prebuiltWidgetProps} = prebuiltWidget;
@@ -1274,7 +1299,7 @@ function WidgetBuilder({
                                     ...prebuiltWidgetProps,
                                     dataSet: prebuiltWidget.widgetType
                                       ? WIDGET_TYPE_TO_DATA_SET[prebuiltWidget.widgetType]
-                                      : DataSet.EVENTS,
+                                      : defaultDataset,
                                     userHasModified: false,
                                     prebuiltWidgetId: id || null,
                                   });

+ 4 - 0
static/app/views/dashboards/widgetCard/index.tsx

@@ -91,6 +91,7 @@ type Props = WithRouterProps & {
   onDuplicate?: () => void;
   onEdit?: () => void;
   onUpdate?: (widget: Widget | null) => void;
+  onWidgetSplitDecision?: (splitDecision: WidgetType) => void;
   renderErrorMessage?: (errorMessage?: string) => React.ReactNode;
   showContextMenu?: boolean;
   showStoredAlert?: boolean;
@@ -236,6 +237,7 @@ class WidgetCard extends Component<Props, State> {
       dashboardFilters,
       isWidgetInvalid,
       location,
+      onWidgetSplitDecision,
     } = this.props;
 
     if (widget.displayType === DisplayType.TOP_N) {
@@ -385,6 +387,7 @@ class WidgetCard extends Component<Props, State> {
                     onDataFetched={this.setData}
                     dashboardFilters={dashboardFilters}
                     chartGroup={DASHBOARD_CHART_GROUP}
+                    onWidgetSplitDecision={onWidgetSplitDecision}
                   />
                 ) : (
                   <LazyRender containerHeight={200} withoutContainer>
@@ -401,6 +404,7 @@ class WidgetCard extends Component<Props, State> {
                       onDataFetched={this.setData}
                       dashboardFilters={dashboardFilters}
                       chartGroup={DASHBOARD_CHART_GROUP}
+                      onWidgetSplitDecision={onWidgetSplitDecision}
                     />
                   </LazyRender>
                 )}

Some files were not shown because too many files changed in this diff