Browse Source

ref(dashboards): Split out indexed events alert (#79537)

This little alert shows up in some rare circumstances. It _used_ to live
inside `WidgetCard`, and controlled by the `showStoredAlert` prop. That
prop is only true inside _modals_! Awkward. This PR takes that alert,
moved it into its own component, and then adds that component to the two
places where it's actually used.

This removes irrelevant code from the already bloated `WidgetCard` and
simplifies it a bit. It's generally not a great pattern to conditionally
render tangentially related content this way.
George Gritsouk 4 months ago
parent
commit
a345553f97

+ 3 - 1
static/app/components/modals/widgetBuilder/addToDashboardModal.tsx

@@ -22,6 +22,7 @@ import {MetricsCardinalityProvider} from 'sentry/utils/performance/contexts/metr
 import {MEPSettingProvider} from 'sentry/utils/performance/contexts/metricsEnhancedSetting';
 import normalizeUrl from 'sentry/utils/url/normalizeUrl';
 import useApi from 'sentry/utils/useApi';
+import {IndexedEventsSelectionAlert} from 'sentry/views/dashboards/indexedEventsSelectionAlert';
 import type {
   DashboardDetails,
   DashboardListItem,
@@ -301,7 +302,6 @@ function AddToDashboardModal({
                       getDashboardFiltersFromURL(location) ?? selectedDashboard?.filters
                     }
                     widget={widget}
-                    showStoredAlert
                     shouldResize={false}
                     widgetLegendState={widgetLegendState}
                     onLegendSelectChanged={() => {}}
@@ -312,6 +312,8 @@ function AddToDashboardModal({
                         : undefined
                     }
                   />
+
+                  <IndexedEventsSelectionAlert widget={widget} />
                 </MEPSettingProvider>
               </DashboardsMEPProvider>
             )}

+ 45 - 0
static/app/views/dashboards/indexedEventsSelectionAlert.spec.tsx

@@ -0,0 +1,45 @@
+import {WidgetFixture} from 'sentry-fixture/widget';
+
+import {render, screen} from 'sentry-test/reactTestingLibrary';
+
+import {MEPSettingProvider} from 'sentry/utils/performance/contexts/metricsEnhancedSetting';
+
+import {DashboardsMEPContext} from './widgetCard/dashboardsMEPContext';
+import {IndexedEventsSelectionAlert} from './indexedEventsSelectionAlert';
+import {WidgetType} from './types';
+
+describe('IndexedEventsSelectionAlert', () => {
+  const widget = WidgetFixture({
+    widgetType: WidgetType.DISCOVER,
+  });
+
+  it('Shows warning if falling through to indexed events', async () => {
+    render(
+      <MEPSettingProvider forceTransactions>
+        <DashboardsMEPContext.Provider
+          value={{isMetricsData: false, setIsMetricsData: () => {}}}
+        >
+          <IndexedEventsSelectionAlert widget={widget} />
+        </DashboardsMEPContext.Provider>
+      </MEPSettingProvider>
+    );
+
+    await screen.findByText(/we've automatically adjusted your results/i);
+  });
+
+  it('Does not show warning if using metrics successfully', () => {
+    render(
+      <MEPSettingProvider>
+        <DashboardsMEPContext.Provider
+          value={{isMetricsData: true, setIsMetricsData: () => {}}}
+        >
+          <IndexedEventsSelectionAlert widget={widget} />
+        </DashboardsMEPContext.Provider>
+      </MEPSettingProvider>
+    );
+
+    expect(
+      screen.queryByText(/we've automatically adjusted your results/i)
+    ).not.toBeInTheDocument();
+  });
+});

+ 92 - 0
static/app/views/dashboards/indexedEventsSelectionAlert.tsx

@@ -0,0 +1,92 @@
+import styled from '@emotion/styled';
+
+import Alert from 'sentry/components/alert';
+import ExternalLink from 'sentry/components/links/externalLink';
+import {parseSearch} from 'sentry/components/searchSyntax/parser';
+import {tct} from 'sentry/locale';
+import {space} from 'sentry/styles/space';
+import {parseFunction} from 'sentry/utils/discover/fields';
+import {
+  MEPConsumer,
+  MEPState,
+} from 'sentry/utils/performance/contexts/metricsEnhancedSetting';
+import useOrganization from 'sentry/utils/useOrganization';
+
+import {DashboardsMEPConsumer} from './widgetCard/dashboardsMEPContext';
+import {type Widget, WidgetType} from './types';
+
+type SearchFilterKey = {key?: {value: string}};
+
+interface IndexedEventsSelectionAlertProps {
+  widget: Widget;
+}
+
+const ERROR_FIELDS = [
+  'error.handled',
+  'error.unhandled',
+  'error.mechanism',
+  'error.type',
+  'error.value',
+];
+
+export function IndexedEventsSelectionAlert({widget}: IndexedEventsSelectionAlertProps) {
+  const organization = useOrganization();
+
+  if (organization.features.includes('performance-mep-bannerless-ui')) {
+    return null;
+  }
+
+  const widgetContainsErrorFields = widget.queries.some(
+    ({columns, aggregates, conditions}) =>
+      ERROR_FIELDS.some(
+        errorField =>
+          columns.includes(errorField) ||
+          aggregates.some(aggregate =>
+            parseFunction(aggregate)?.arguments.includes(errorField)
+          ) ||
+          parseSearch(conditions)?.some(
+            filter => (filter as SearchFilterKey).key?.value === errorField
+          )
+      )
+  );
+
+  return (
+    <MEPConsumer>
+      {metricSettingContext => {
+        return (
+          <DashboardsMEPConsumer>
+            {({isMetricsData}) => {
+              if (
+                isMetricsData === false &&
+                widget.widgetType === WidgetType.DISCOVER &&
+                metricSettingContext &&
+                metricSettingContext.metricSettingState !== MEPState.TRANSACTIONS_ONLY
+              ) {
+                if (!widgetContainsErrorFields) {
+                  return (
+                    <StoredDataAlert showIcon>
+                      {tct(
+                        "Your selection is only applicable to [indexedData: indexed event data]. We've automatically adjusted your results.",
+                        {
+                          indexedData: (
+                            <ExternalLink href="https://docs.sentry.io/product/dashboards/widget-builder/#transactions" />
+                          ),
+                        }
+                      )}
+                    </StoredDataAlert>
+                  );
+                }
+              }
+              return null;
+            }}
+          </DashboardsMEPConsumer>
+        );
+      }}
+    </MEPConsumer>
+  );
+}
+
+const StoredDataAlert = styled(Alert)`
+  margin-top: ${space(1)};
+  margin-bottom: 0;
+`;

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

@@ -21,6 +21,7 @@ import type {DashboardFilters, Widget, WidgetType} from 'sentry/views/dashboards
 import {DisplayType} from 'sentry/views/dashboards/types';
 import WidgetLegendNameEncoderDecoder from 'sentry/views/dashboards/widgetLegendNameEncoderDecoder';
 
+import {IndexedEventsSelectionAlert} from '../../indexedEventsSelectionAlert';
 import {getDashboardFiltersFromURL} from '../../utils';
 import WidgetCard, {WidgetCardPanel} from '../../widgetCard';
 import type WidgetLegendSelectionState from '../../widgetLegendSelectionState';
@@ -136,7 +137,6 @@ export function VisualizationStep({
             )
           }
           noLazyLoad
-          showStoredAlert
           isWidgetInvalid={isWidgetInvalid}
           onDataFetched={onDataFetched}
           onWidgetSplitDecision={onWidgetSplitDecision}
@@ -150,6 +150,8 @@ export function VisualizationStep({
           }
           widgetLegendState={widgetLegendState}
         />
+
+        <IndexedEventsSelectionAlert widget={widget} />
       </VisualizationWrapper>
     </StyledBuildStep>
   );

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

@@ -583,49 +583,6 @@ describe('Dashboards > WidgetCard', function () {
     );
   });
 
-  it('renders stored data disclaimer', async function () {
-    MockApiClient.addMockResponse({
-      url: '/organizations/org-slug/events/',
-      body: {
-        meta: {title: 'string', isMetricsData: false},
-        data: [{title: 'title'}],
-      },
-    });
-
-    renderWithProviders(
-      <WidgetCard
-        api={api}
-        organization={{
-          ...organization,
-          features: [...organization.features, 'dashboards-mep'],
-        }}
-        widget={{
-          ...multipleQueryWidget,
-          displayType: DisplayType.TABLE,
-          queries: [{...multipleQueryWidget.queries[0]}],
-        }}
-        selection={selection}
-        isEditingDashboard={false}
-        onDelete={() => undefined}
-        onEdit={() => undefined}
-        onDuplicate={() => undefined}
-        renderErrorMessage={() => undefined}
-        showContextMenu
-        widgetLimitReached={false}
-        showStoredAlert
-        widgetLegendState={widgetLegendState}
-      />
-    );
-
-    // Badge in the widget header
-    expect(await screen.findByText('Indexed')).toBeInTheDocument();
-
-    expect(
-      // Alert below the widget
-      await screen.findByText(/we've automatically adjusted your results/i)
-    ).toBeInTheDocument();
-  });
-
   it('renders chart using axis and tooltip formatters from custom measurement meta', async function () {
     const spy = jest.spyOn(LineChart, 'LineChart');
     const eventsStatsMock = MockApiClient.addMockResponse({

+ 1 - 76
static/app/views/dashboards/widgetCard/index.tsx

@@ -4,19 +4,16 @@ import type {LegendComponentOption} from 'echarts';
 import type {Location} from 'history';
 
 import type {Client} from 'sentry/api';
-import {Alert} from 'sentry/components/alert';
 import ErrorPanel from 'sentry/components/charts/errorPanel';
 import {HeaderTitle} from 'sentry/components/charts/styles';
 import ErrorBoundary from 'sentry/components/errorBoundary';
 import {LazyRender} from 'sentry/components/lazyRender';
-import ExternalLink from 'sentry/components/links/externalLink';
 import Panel from 'sentry/components/panels/panel';
 import PanelAlert from 'sentry/components/panels/panelAlert';
 import Placeholder from 'sentry/components/placeholder';
-import {parseSearch} from 'sentry/components/searchSyntax/parser';
 import {Tooltip} from 'sentry/components/tooltip';
 import {IconWarning} from 'sentry/icons';
-import {t, tct} from 'sentry/locale';
+import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import type {PageFilters} from 'sentry/types/core';
 import type {Series} from 'sentry/types/echarts';
@@ -25,13 +22,8 @@ import type {Organization} from 'sentry/types/organization';
 import {getFormattedDate} from 'sentry/utils/dates';
 import type {TableDataWithTitle} from 'sentry/utils/discover/discoverQuery';
 import type {AggregationOutputType} from 'sentry/utils/discover/fields';
-import {parseFunction} from 'sentry/utils/discover/fields';
 import {hasOnDemandMetricWidgetFeature} from 'sentry/utils/onDemandMetrics/features';
 import {ExtractedMetricsTag} from 'sentry/utils/performance/contexts/metricsEnhancedPerformanceDataContext';
-import {
-  MEPConsumer,
-  MEPState,
-} from 'sentry/utils/performance/contexts/metricsEnhancedSetting';
 import {VisuallyCompleteWithData} from 'sentry/utils/performanceForSentry';
 import useOrganization from 'sentry/utils/useOrganization';
 import withApi from 'sentry/utils/withApi';
@@ -49,7 +41,6 @@ import {DisplayType, OnDemandExtractionState, WidgetType} from '../types';
 import {DEFAULT_RESULTS_LIMIT} from '../widgetBuilder/utils';
 import type WidgetLegendSelectionState from '../widgetLegendSelectionState';
 
-import {DashboardsMEPConsumer} from './dashboardsMEPContext';
 import WidgetCardChartContainer from './widgetCardChartContainer';
 import WidgetCardContextMenu from './widgetCardContextMenu';
 
@@ -96,16 +87,6 @@ type Props = WithRouterProps & {
   windowWidth?: number;
 };
 
-type SearchFilterKey = {key?: {value: string}};
-
-const ERROR_FIELDS = [
-  'error.handled',
-  'error.unhandled',
-  'error.mechanism',
-  'error.type',
-  'error.value',
-];
-
 type Data = {
   pageLinks?: string;
   tableResults?: TableDataWithTitle[];
@@ -135,7 +116,6 @@ function WidgetCard(props: Props) {
     tableItemLimit,
     windowWidth,
     noLazyLoad,
-    showStoredAlert,
     dashboardFilters,
     isWidgetInvalid,
     location,
@@ -162,21 +142,6 @@ function WidgetCard(props: Props) {
     query.aggregates.some(aggregate => aggregate.includes('session.duration'))
   );
 
-  // prettier-ignore
-  const widgetContainsErrorFields = widget.queries.some(
-    ({columns, aggregates, conditions}) =>
-      ERROR_FIELDS.some(
-        errorField =>
-          columns.includes(errorField) ||
-          aggregates.some(
-            aggregate => parseFunction(aggregate)?.arguments.includes(errorField)
-          ) ||
-          parseSearch(conditions)?.some(
-            filter => (filter as SearchFilterKey).key?.value === errorField
-          )
-      )
-  );
-
   if (widget.widgetType === WidgetType.METRICS) {
     return (
       <MetricWidgetCard
@@ -311,41 +276,6 @@ function WidgetCard(props: Props) {
           )}
         </WidgetCardPanel>
       </VisuallyCompleteWithData>
-      {!organization.features.includes('performance-mep-bannerless-ui') && (
-        <MEPConsumer>
-          {metricSettingContext => {
-            return (
-              <DashboardsMEPConsumer>
-                {({isMetricsData}) => {
-                  if (
-                    showStoredAlert &&
-                    isMetricsData === false &&
-                    widget.widgetType === WidgetType.DISCOVER &&
-                    metricSettingContext &&
-                    metricSettingContext.metricSettingState !== MEPState.TRANSACTIONS_ONLY
-                  ) {
-                    if (!widgetContainsErrorFields) {
-                      return (
-                        <StoredDataAlert showIcon>
-                          {tct(
-                            "Your selection is only applicable to [indexedData: indexed event data]. We've automatically adjusted your results.",
-                            {
-                              indexedData: (
-                                <ExternalLink href="https://docs.sentry.io/product/dashboards/widget-builder/#errors--transactions" />
-                              ),
-                            }
-                          )}
-                        </StoredDataAlert>
-                      );
-                    }
-                  }
-                  return null;
-                }}
-              </DashboardsMEPConsumer>
-            );
-          }}
-        </MEPConsumer>
-      )}
     </ErrorBoundary>
   );
 }
@@ -447,11 +377,6 @@ export const WidgetCardPanel = styled(Panel, {
   }
 `;
 
-const StoredDataAlert = styled(Alert)`
-  margin-top: ${space(1)};
-  margin-bottom: 0;
-`;
-
 const StyledErrorPanel = styled(ErrorPanel)`
   padding: ${space(2)};
 `;