Browse Source

ref(dashboards): Split dashboard warnings into components and hooks (#79382)

Dashboard widgets have various little badges and warnings. Each of them
is its own component that loads its own data. This works fine, but is
blocking a refactor I'm doing. In the _very near future_ I don't want to
use those little components, but I need to re-use their logic.

This PR takes each of those components and splits it into a hook that
returns its state, and a component that consumes that state and renders
the UI. This will make it easy for me to re-use that hook (and the
components) and render a different UI.

Best enjoyed with whitespace turned off.
George Gritsouk 4 months ago
parent
commit
bb0e71d2a7

+ 24 - 6
static/app/utils/performance/contexts/metricsEnhancedPerformanceDataContext.tsx

@@ -132,7 +132,11 @@ export function MEPTag() {
   return <Tag data-test-id="has-metrics-data-tag">{tagText}</Tag>;
 }
 
-export function ExtractedMetricsTag(props: {queryKey: MetricsResultsMetaMapKey}) {
+type ExtractionStatus = 'extracted' | 'not-extracted' | null;
+
+export function useExtractionStatus(props: {
+  queryKey: MetricsResultsMetaMapKey;
+}): ExtractionStatus {
   const resultsMeta = useMetricsResultsMeta();
   const organization = useOrganization();
   const _onDemandControl = useOnDemandControl();
@@ -156,11 +160,25 @@ export function ExtractedMetricsTag(props: {queryKey: MetricsResultsMetaMapKey})
   }
 
   if (!isMetricsExtractedData) {
+    return 'not-extracted';
+  }
+  return 'extracted';
+}
+
+export function ExtractedMetricsTag(props: {queryKey: MetricsResultsMetaMapKey}) {
+  const extractionStatus = useExtractionStatus(props);
+
+  if (extractionStatus === 'extracted') {
+    return (
+      <Tag type="info" data-test-id="has-metrics-data-tag">
+        {t('extracted')}
+      </Tag>
+    );
+  }
+
+  if (extractionStatus === 'not-extracted') {
     return <Tag style={{opacity: 0.25}}>{t('not extracted')}</Tag>;
   }
-  return (
-    <Tag type="info" data-test-id="has-metrics-data-tag">
-      {t('extracted')}
-    </Tag>
-  );
+
+  return null;
 }

+ 21 - 10
static/app/views/dashboards/discoverSplitAlert.tsx

@@ -8,19 +8,30 @@ interface DiscoverSplitAlertProps {
   widget: Widget;
 }
 
-export function DiscoverSplitAlert({widget}: DiscoverSplitAlertProps) {
+export function useDiscoverSplitAlert({widget}: DiscoverSplitAlertProps): string | null {
   if (widget?.datasetSource !== DatasetSource.FORCED) {
     return null;
   }
 
-  return (
-    <Tooltip
-      containerDisplayMode="inline-flex"
-      title={t(
-        "We're splitting our datasets up to make it a bit easier to digest. We defaulted this widget to Errors. Edit as you see fit."
-      )}
-    >
-      <IconWarning color="warningText" aria-label={t('Dataset split warning')} />
-    </Tooltip>
+  return t(
+    "We're splitting our datasets up to make it a bit easier to digest. We defaulted this widget to Errors. Edit as you see fit."
   );
 }
+
+export function DiscoverSplitAlert({widget}: DiscoverSplitAlertProps) {
+  const splitAlert = useDiscoverSplitAlert({widget});
+
+  if (widget?.datasetSource !== DatasetSource.FORCED) {
+    return null;
+  }
+
+  if (splitAlert) {
+    return (
+      <Tooltip containerDisplayMode="inline-flex" title={splitAlert}>
+        <IconWarning color="warningText" aria-label={t('Dataset split warning')} />
+      </Tooltip>
+    );
+  }
+
+  return null;
+}

+ 25 - 22
static/app/views/dashboards/widgetCard/index.tsx

@@ -55,13 +55,14 @@ import WidgetCardChartContainer from './widgetCardChartContainer';
 import WidgetCardContextMenu from './widgetCardContextMenu';
 
 const SESSION_DURATION_INGESTION_STOP_DATE = new Date('2023-01-12');
+
+export const SESSION_DURATION_ALERT_TEXT = t(
+  'session.duration is no longer being recorded as of %s. Data in this widget may be incomplete.',
+  getFormattedDate(SESSION_DURATION_INGESTION_STOP_DATE, 'MMM D, YYYY')
+);
+
 export const SESSION_DURATION_ALERT = (
-  <PanelAlert type="warning">
-    {t(
-      'session.duration is no longer being recorded as of %s. Data in this widget may be incomplete.',
-      getFormattedDate(SESSION_DURATION_INGESTION_STOP_DATE, 'MMM D, YYYY')
-    )}
-  </PanelAlert>
+  <PanelAlert type="warning">{SESSION_DURATION_ALERT_TEXT}</PanelAlert>
 );
 
 type DraggableProps = Pick<ReturnType<typeof useSortable>, 'attributes' | 'listeners'>;
@@ -358,8 +359,9 @@ function WidgetCard(props: Props) {
 
 export default withApi(withOrganization(withPageFilters(withSentryRouter(WidgetCard))));
 
-function DisplayOnDemandWarnings(props: {widget: Widget}) {
+function useOnDemandWarning(props: {widget: Widget}): string | null {
   const organization = useOrganization();
+
   if (!hasOnDemandMetricWidgetFeature(organization)) {
     return null;
   }
@@ -379,25 +381,26 @@ function DisplayOnDemandWarnings(props: {widget: Widget}) {
   );
 
   if (widgetContainsHighCardinality) {
-    return (
-      <Tooltip
-        containerDisplayMode="inline-flex"
-        title={t(
-          'This widget is using indexed data because it has a column with too many unique values.'
-        )}
-      >
-        <IconWarning color="warningText" />
-      </Tooltip>
+    return t(
+      'This widget is using indexed data because it has a column with too many unique values.'
     );
   }
+
   if (widgetReachedSpecLimit) {
+    return t(
+      "This widget is using indexed data because you've reached your organization limit for dynamically extracted metrics."
+    );
+  }
+
+  return null;
+}
+
+function DisplayOnDemandWarnings(props: {widget: Widget}) {
+  const warning = useOnDemandWarning(props);
+
+  if (warning) {
     return (
-      <Tooltip
-        containerDisplayMode="inline-flex"
-        title={t(
-          "This widget is using indexed data because you've reached your organization limit for dynamically extracted metrics."
-        )}
-      >
+      <Tooltip containerDisplayMode="inline-flex" title={warning}>
         <IconWarning color="warningText" />
       </Tooltip>
     );

+ 7 - 4
static/app/views/dashboards/widgetCard/issueWidgetCard.spec.tsx

@@ -12,6 +12,7 @@ import {
 } from 'sentry-test/reactTestingLibrary';
 
 import MemberListStore from 'sentry/stores/memberListStore';
+import {MEPSettingProvider} from 'sentry/utils/performance/contexts/metricsEnhancedSetting';
 import type {Widget} from 'sentry/views/dashboards/types';
 import {DisplayType, WidgetType} from 'sentry/views/dashboards/types';
 import WidgetCard from 'sentry/views/dashboards/widgetCard';
@@ -59,9 +60,11 @@ describe('Dashboards > IssueWidgetCard', function () {
   const BasicProvidersWrapper = makeAllTheProviders({organization, router});
   function Wrapper({children}: {children: React.ReactNode}) {
     return (
-      <DashboardsMEPProvider>
-        <BasicProvidersWrapper>{children}</BasicProvidersWrapper>
-      </DashboardsMEPProvider>
+      <BasicProvidersWrapper>
+        <DashboardsMEPProvider>
+          <MEPSettingProvider forceTransactions={false}>{children}</MEPSettingProvider>
+        </DashboardsMEPProvider>
+      </BasicProvidersWrapper>
     );
   }
 
@@ -156,7 +159,7 @@ describe('Dashboards > IssueWidgetCard', function () {
         widgetLimitReached={false}
         widgetLegendState={widgetLegendState}
       />,
-      {router, wrapper: Wrapper}
+      {wrapper: Wrapper}
     );
 
     await userEvent.click(await screen.findByLabelText('Widget actions'));

+ 38 - 35
static/app/views/dashboards/widgetCard/widgetCardContextMenu.spec.tsx

@@ -5,6 +5,7 @@ import {RouterFixture} from 'sentry-fixture/routerFixture';
 
 import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
 
+import {MEPSettingProvider} from 'sentry/utils/performance/contexts/metricsEnhancedSetting';
 import {DisplayType, WidgetType} from 'sentry/views/dashboards/types';
 import {performanceScoreTooltip} from 'sentry/views/dashboards/utils';
 import {DashboardsMEPProvider} from 'sentry/views/dashboards/widgetCard/dashboardsMEPContext';
@@ -13,41 +14,43 @@ import WidgetCardContextMenu from 'sentry/views/dashboards/widgetCard/widgetCard
 describe('WidgetCardContextMenu', () => {
   it('displays performance_score tooltip when widget uses performance_score', async () => {
     render(
-      <DashboardsMEPProvider>
-        <WidgetCardContextMenu
-          location={LocationFixture()}
-          organization={OrganizationFixture({
-            features: ['discover-basic'],
-          })}
-          router={RouterFixture()}
-          selection={PageFiltersFixture()}
-          widget={{
-            displayType: DisplayType.AREA,
-            interval: '',
-            queries: [
-              {
-                name: '',
-                fields: ['performance_score(measurements.score.total)'],
-                aggregates: ['performance_score(measurements.score.total)'],
-                conditions: '',
-                columns: [],
-                orderby: '',
-              },
-            ],
-            title: '',
-            datasetSource: undefined,
-            description: undefined,
-            id: undefined,
-            layout: undefined,
-            limit: undefined,
-            tempId: undefined,
-            thresholds: undefined,
-            widgetType: WidgetType.TRANSACTIONS,
-          }}
-          widgetLimitReached={false}
-          showContextMenu
-        />
-      </DashboardsMEPProvider>
+      <MEPSettingProvider>
+        <DashboardsMEPProvider>
+          <WidgetCardContextMenu
+            location={LocationFixture()}
+            organization={OrganizationFixture({
+              features: ['discover-basic'],
+            })}
+            router={RouterFixture()}
+            selection={PageFiltersFixture()}
+            widget={{
+              displayType: DisplayType.AREA,
+              interval: '',
+              queries: [
+                {
+                  name: '',
+                  fields: ['performance_score(measurements.score.total)'],
+                  aggregates: ['performance_score(measurements.score.total)'],
+                  conditions: '',
+                  columns: [],
+                  orderby: '',
+                },
+              ],
+              title: '',
+              datasetSource: undefined,
+              description: undefined,
+              id: undefined,
+              layout: undefined,
+              limit: undefined,
+              tempId: undefined,
+              thresholds: undefined,
+              widgetType: WidgetType.TRANSACTIONS,
+            }}
+            widgetLimitReached={false}
+            showContextMenu
+          />
+        </DashboardsMEPProvider>
+      </MEPSettingProvider>
     );
 
     await userEvent.click(await screen.findByLabelText('Widget actions'));

+ 124 - 135
static/app/views/dashboards/widgetCard/widgetCardContextMenu.tsx

@@ -21,9 +21,10 @@ import {trackAnalytics} from 'sentry/utils/analytics';
 import type {TableDataWithTitle} from 'sentry/utils/discover/discoverQuery';
 import type {AggregationOutputType} from 'sentry/utils/discover/fields';
 import {
-  MEPConsumer,
   MEPState,
+  useMEPSettingContext,
 } from 'sentry/utils/performance/contexts/metricsEnhancedSetting';
+import useOrganization from 'sentry/utils/useOrganization';
 import {
   getWidgetDiscoverUrl,
   getWidgetIssueUrl,
@@ -61,6 +62,19 @@ type Props = {
   totalIssuesCount?: string;
 };
 
+const useIndexedEventsWarning = (): string | null => {
+  const {isMetricsData} = useDashboardsMEPContext();
+  const organization = useOrganization();
+  const metricSettingContext = useMEPSettingContext();
+
+  return !organization.features.includes('performance-mep-bannerless-ui') &&
+    isMetricsData === false &&
+    metricSettingContext &&
+    metricSettingContext.metricSettingState !== MEPState.TRANSACTIONS_ONLY
+    ? t('Indexed')
+    : null;
+};
+
 function WidgetCardContextMenu({
   organization,
   selection,
@@ -82,6 +96,7 @@ function WidgetCardContextMenu({
   description,
   title,
 }: Props) {
+  const indexedEventsWarning = useIndexedEventsWarning();
   const {isMetricsData} = useDashboardsMEPContext();
 
   if (!showContextMenu) {
@@ -106,82 +121,68 @@ function WidgetCardContextMenu({
     return (
       <WidgetViewerContext.Consumer>
         {({setData}) => (
-          <MEPConsumer>
-            {metricSettingContext => (
-              <ContextWrapper>
-                {!organization.features.includes('performance-mep-bannerless-ui') &&
-                  isMetricsData === false &&
-                  metricSettingContext &&
-                  metricSettingContext.metricSettingState !==
-                    MEPState.TRANSACTIONS_ONLY && (
-                    <SampledTag
-                      tooltipText={t('This widget is only applicable to indexed events.')}
-                    >
-                      {t('Indexed')}
-                    </SampledTag>
-                  )}
-                {title && (
-                  <Tooltip
-                    title={
-                      <span>
-                        <WidgetTooltipTitle>{title}</WidgetTooltipTitle>
-                        {description && (
-                          <WidgetTooltipDescription>
-                            {description}
-                          </WidgetTooltipDescription>
-                        )}
-                      </span>
-                    }
-                    containerDisplayMode="grid"
-                    isHoverable
-                  >
-                    <WidgetTooltipButton
-                      aria-label={t('Widget description')}
-                      borderless
-                      size="xs"
-                      icon={<IconInfo />}
-                    />
-                  </Tooltip>
-                )}
-                <StyledDropdownMenuControl
-                  items={[
-                    {
-                      key: 'preview',
-                      label: t(
-                        'This is a preview only. To edit, you must add this dashboard.'
-                      ),
-                    },
-                  ]}
-                  triggerProps={{
-                    'aria-label': t('Widget actions'),
-                    size: 'xs',
-                    borderless: true,
-                    showChevron: false,
-                    icon: <IconEllipsis direction="down" size="sm" />,
-                  }}
-                  position="bottom-end"
-                  disabledKeys={[...disabledKeys, 'preview']}
-                />
-                <Button
-                  aria-label={t('Open Widget Viewer')}
+          <ContextWrapper>
+            {indexedEventsWarning ? (
+              <SampledTag tooltipText={indexedEventsWarning}>{t('Indexed')}</SampledTag>
+            ) : null}
+            {title && (
+              <Tooltip
+                title={
+                  <span>
+                    <WidgetTooltipTitle>{title}</WidgetTooltipTitle>
+                    {description && (
+                      <WidgetTooltipDescription>{description}</WidgetTooltipDescription>
+                    )}
+                  </span>
+                }
+                containerDisplayMode="grid"
+                isHoverable
+              >
+                <WidgetTooltipButton
+                  aria-label={t('Widget description')}
                   borderless
                   size="xs"
-                  icon={<IconExpand />}
-                  onClick={() => {
-                    (seriesData || tableData) &&
-                      setData({
-                        seriesData,
-                        tableData,
-                        pageLinks,
-                        totalIssuesCount,
-                        seriesResultsType,
-                      });
-                    openWidgetViewerPath(index);
-                  }}
+                  icon={<IconInfo />}
                 />
-              </ContextWrapper>
+              </Tooltip>
             )}
-          </MEPConsumer>
+            <StyledDropdownMenuControl
+              items={[
+                {
+                  key: 'preview',
+                  label: t(
+                    'This is a preview only. To edit, you must add this dashboard.'
+                  ),
+                },
+              ]}
+              triggerProps={{
+                'aria-label': t('Widget actions'),
+                size: 'xs',
+                borderless: true,
+                showChevron: false,
+                icon: <IconEllipsis direction="down" size="sm" />,
+              }}
+              position="bottom-end"
+              disabledKeys={[...disabledKeys, 'preview']}
+            />
+            <Button
+              aria-label={t('Open Widget Viewer')}
+              borderless
+              size="xs"
+              icon={<IconExpand />}
+              onClick={() => {
+                (seriesData || tableData) &&
+                  setData({
+                    seriesData,
+                    tableData,
+                    pageLinks,
+                    totalIssuesCount,
+                    seriesResultsType,
+                  });
+                openWidgetViewerPath(index);
+              }}
+            />
+          </ContextWrapper>
         )}
       </WidgetViewerContext.Consumer>
     );
@@ -296,72 +297,60 @@ function WidgetCardContextMenu({
   return (
     <WidgetViewerContext.Consumer>
       {({setData}) => (
-        <MEPConsumer>
-          {metricSettingContext => (
-            <ContextWrapper>
-              {!organization.features.includes('performance-mep-bannerless-ui') &&
-                isMetricsData === false &&
-                metricSettingContext &&
-                metricSettingContext.metricSettingState !==
-                  MEPState.TRANSACTIONS_ONLY && (
-                  <SampledTag
-                    tooltipText={t('This widget is only applicable to indexed events.')}
-                  >
-                    {t('Indexed')}
-                  </SampledTag>
-                )}
-              {title && (
-                <Tooltip
-                  title={
-                    <span>
-                      <WidgetTooltipTitle>{title}</WidgetTooltipTitle>
-                      {description && (
-                        <WidgetTooltipDescription>{description}</WidgetTooltipDescription>
-                      )}
-                    </span>
-                  }
-                  containerDisplayMode="grid"
-                  isHoverable
-                >
-                  <WidgetTooltipButton
-                    aria-label={t('Widget description')}
-                    borderless
-                    size="xs"
-                    icon={<IconInfo />}
-                  />
-                </Tooltip>
-              )}
-              <StyledDropdownMenuControl
-                items={menuOptions}
-                triggerProps={{
-                  'aria-label': t('Widget actions'),
-                  size: 'xs',
-                  borderless: true,
-                  showChevron: false,
-                  icon: <IconEllipsis direction="down" size="sm" />,
-                }}
-                position="bottom-end"
-                disabledKeys={[...disabledKeys]}
-              />
-              <Button
-                aria-label={t('Open Widget Viewer')}
+        <ContextWrapper>
+          {indexedEventsWarning ? (
+            <SampledTag tooltipText={indexedEventsWarning}>{t('Indexed')}</SampledTag>
+          ) : null}
+          {title && (
+            <Tooltip
+              title={
+                <span>
+                  <WidgetTooltipTitle>{title}</WidgetTooltipTitle>
+                  {description && (
+                    <WidgetTooltipDescription>{description}</WidgetTooltipDescription>
+                  )}
+                </span>
+              }
+              containerDisplayMode="grid"
+              isHoverable
+            >
+              <WidgetTooltipButton
+                aria-label={t('Widget description')}
                 borderless
                 size="xs"
-                icon={<IconExpand />}
-                onClick={() => {
-                  setData({
-                    seriesData,
-                    tableData,
-                    pageLinks,
-                    totalIssuesCount,
-                    seriesResultsType,
-                  });
-                  openWidgetViewerPath(widget.id ?? index);
-                }}
+                icon={<IconInfo />}
               />
-            </ContextWrapper>
+            </Tooltip>
           )}
-        </MEPConsumer>
+          <StyledDropdownMenuControl
+            items={menuOptions}
+            triggerProps={{
+              'aria-label': t('Widget actions'),
+              size: 'xs',
+              borderless: true,
+              showChevron: false,
+              icon: <IconEllipsis direction="down" size="sm" />,
+            }}
+            position="bottom-end"
+            disabledKeys={[...disabledKeys]}
+          />
+          <Button
+            aria-label={t('Open Widget Viewer')}
+            borderless
+            size="xs"
+            icon={<IconExpand />}
+            onClick={() => {
+              setData({
+                seriesData,
+                tableData,
+                pageLinks,
+                totalIssuesCount,
+                seriesResultsType,
+              });
+              openWidgetViewerPath(widget.id ?? index);
+            }}
+          />
+        </ContextWrapper>
       )}
     </WidgetViewerContext.Consumer>
   );