ref(alerts): Move issue alert preview into subcomponent (#57875)

static/app/views/alerts/rules/issue/index.spec.tsx

@@ -1,7 +1,6 @@
 import {browserHistory, PlainRoute} from 'react-router';
 import selectEvent from 'react-select-event';
 import moment from 'moment';
-import {MetricRule} from 'sentry-fixture/metricRule';
 import {ProjectAlertRule} from 'sentry-fixture/projectAlertRule';
 import {ProjectAlertRuleConfiguration} from 'sentry-fixture/projectAlertRuleConfiguration';
 
@@ -158,39 +157,34 @@ describe('IssueRuleEditor', function () {
   });
 
   describe('Viewing the rule', () => {
-    const rule = MetricRule();
-
-    it('is visible without org-level alerts:write', () => {
+    it('is visible without org-level alerts:write', async () => {
       createWrapper({
         organization: {access: []},
         project: {access: []},
-        rule,
       });
 
-      expect(screen.queryByText(permissionAlertText)).toBeInTheDocument();
+      expect(await screen.findByText(permissionAlertText)).toBeInTheDocument();
       expect(screen.queryByLabelText('Save Rule')).toBeDisabled();
     });
 
-    it('is enabled with org-level alerts:write', () => {
+    it('is enabled with org-level alerts:write', async () => {
       createWrapper({
         organization: {access: ['alerts:write']},
         project: {access: []},
-        rule,
       });
 
+      expect(await screen.findByLabelText('Save Rule')).toBeEnabled();
       expect(screen.queryByText(permissionAlertText)).not.toBeInTheDocument();
-      expect(screen.queryByLabelText('Save Rule')).toBeEnabled();
     });
 
-    it('is enabled with project-level alerts:write', () => {
+    it('is enabled with project-level alerts:write', async () => {
       createWrapper({
         organization: {access: []},
         project: {access: ['alerts:write']},
-        rule,
       });
 
+      expect(await screen.findByLabelText('Save Rule')).toBeEnabled();
       expect(screen.queryByText(permissionAlertText)).not.toBeInTheDocument();
-      expect(screen.queryByLabelText('Save Rule')).toBeEnabled();
     });
   });
 
@@ -205,7 +199,7 @@ describe('IssueRuleEditor', function () {
       });
     });
 
-    it('gets correct rule name', function () {
+    it('gets correct rule name', async function () {
       const rule = ProjectAlertRule();
       mock = MockApiClient.addMockResponse({
         url: endpoint,
@@ -213,7 +207,7 @@ describe('IssueRuleEditor', function () {
         body: rule,
       });
       const {onChangeTitleMock} = createWrapper();
-      expect(mock).toHaveBeenCalled();
+      await waitFor(() => expect(mock).toHaveBeenCalled());
       expect(onChangeTitleMock).toHaveBeenCalledWith(rule.name);
     });
 
@@ -464,7 +458,7 @@ describe('IssueRuleEditor', function () {
       });
     });
 
-    it('gets correct rule to duplicate and renders fields correctly', function () {
+    it('gets correct rule to duplicate and renders fields correctly', async function () {
       createWrapper({
         organization: {
           access: ['alerts:write'],
@@ -478,8 +472,9 @@ describe('IssueRuleEditor', function () {
           },
         },
       });
+
+      expect(await screen.findByTestId('alert-name')).toHaveValue(`${rule.name} copy`);
       expect(mock).toHaveBeenCalled();
-      expect(screen.getByTestId('alert-name')).toHaveValue(`${rule.name} copy`);
     });
   });
 });

static/app/views/alerts/rules/issue/index.tsx

@@ -21,6 +21,7 @@ import AlertLink from 'sentry/components/alertLink';
 import {Button} from 'sentry/components/button';
 import Checkbox from 'sentry/components/checkbox';
 import Confirm from 'sentry/components/confirm';
+import ErrorBoundary from 'sentry/components/errorBoundary';
 import SelectControl from 'sentry/components/forms/controls/selectControl';
 import FieldGroup from 'sentry/components/forms/fieldGroup';
 import FieldHelp from 'sentry/components/forms/fieldGroup/fieldHelp';
@@ -34,15 +35,12 @@ import ExternalLink from 'sentry/components/links/externalLink';
 import List from 'sentry/components/list';
 import ListItem from 'sentry/components/list/listItem';
 import LoadingMask from 'sentry/components/loadingMask';
-import {CursorHandler} from 'sentry/components/pagination';
 import Panel from 'sentry/components/panels/panel';
 import PanelBody from 'sentry/components/panels/panelBody';
 import TeamSelector from 'sentry/components/teamSelector';
-import {Tooltip} from 'sentry/components/tooltip';
 import {ALL_ENVIRONMENTS_KEY} from 'sentry/constants';
 import {IconChevron, IconNot} from 'sentry/icons';
 import {t, tct, tn} from 'sentry/locale';
-import GroupStore from 'sentry/stores/groupStore';
 import {space} from 'sentry/styles/space';
 import {
   Environment,
@@ -71,7 +69,7 @@ import routeTitleGen from 'sentry/utils/routeTitle';
 import {normalizeUrl} from 'sentry/utils/withDomainRequired';
 import withOrganization from 'sentry/utils/withOrganization';
 import withProjects from 'sentry/utils/withProjects';
-import PreviewTable from 'sentry/views/alerts/rules/issue/previewTable';
+import {PreviewIssues} from 'sentry/views/alerts/rules/issue/previewIssues';
 import {
   CHANGE_ALERT_CONDITION_IDS,
   CHANGE_ALERT_PLACEHOLDERS_LABELS,
@@ -121,9 +119,6 @@ const defaultRule: UnsavedIssueAlertRule = {
 
 const POLLING_MAX_TIME_LIMIT = 3 * 60000;
 
-const SENTRY_ISSUE_ALERT_DOCS_URL =
-  'https://docs.sentry.io/product/alerts/alert-types/#issue-alerts';
-
 type ConditionOrActionProperty = 'conditions' | 'actions' | 'filters';
 
 type RuleTaskResponse = {
@@ -162,13 +157,6 @@ type State = DeprecatedAsyncView['state'] & {
   environments: Environment[] | null;
   incompatibleConditions: number[] | null;
   incompatibleFilters: number[] | null;
-  issueCount: number;
-  loadingPreview: boolean;
-  previewCursor: string | null | undefined;
-  previewEndpoint: null | string;
-  previewError: null | string;
-  previewGroups: string[] | null;
-  previewPage: number;
   project: Project;
   sendingNotification: boolean;
   uuid: null | string;
@@ -201,28 +189,21 @@ class IssueRuleEditor extends DeprecatedAsyncView<Props, State> {
 
   componentDidMount() {
     super.componentDidMount();
-    this.fetchPreview();
   }
 
   componentWillUnmount() {
     super.componentWillUnmount();
     this.isUnmounted = true;
-    GroupStore.reset();
     window.clearTimeout(this.pollingTimeout);
     this.checkIncompatibleRuleDebounced.cancel();
-    this.fetchPreviewDebounced.cancel();
   }
 
   componentDidUpdate(_prevProps: Props, prevState: State) {
-    if (prevState.previewCursor !== this.state.previewCursor) {
-      this.fetchPreview();
-    } else if (this.isRuleStateChange(prevState)) {
+    if (this.isRuleStateChange(prevState)) {
       this.setState({
-        loadingPreview: true,
         incompatibleConditions: null,
         incompatibleFilters: null,
       });
-      this.fetchPreviewDebounced();
       this.checkIncompatibleRuleDebounced();
     }
     if (prevState.project.id === this.state.project.id) {
@@ -268,16 +249,9 @@ class IssueRuleEditor extends DeprecatedAsyncView<Props, State> {
       environments: [],
       uuid: null,
       project,
-      previewGroups: null,
-      previewCursor: null,
-      previewError: null,
-      issueCount: 0,
-      previewPage: 0,
-      loadingPreview: false,
       sendingNotification: false,
       incompatibleConditions: null,
       incompatibleFilters: null,
-      previewEndpoint: null,
     };
 
     const projectTeamIds = new Set(project.teams.map(({id}) => id));
@@ -409,73 +383,6 @@ class IssueRuleEditor extends DeprecatedAsyncView<Props, State> {
     }
   };
 
-  fetchPreview = (resetCursor = false) => {
-    const {organization} = this.props;
-    const {project, rule, previewCursor, previewEndpoint} = this.state;
-
-    if (!rule) {
-      return;
-    }
-
-    this.setState({loadingPreview: true});
-    if (resetCursor) {
-      this.setState({previewCursor: null, previewPage: 0});
-    }
-    // we currently don't have a way to parse objects from query params, so this method is POST for now
-    this.api
-      .requestPromise(`/projects/${organization.slug}/${project.slug}/rules/preview/`, {
-        method: 'POST',
-        includeAllArgs: true,
-        query: {
-          cursor: resetCursor ? null : previewCursor,
-          per_page: 5,
-        },
-        data: {
-          conditions: rule?.conditions || [],
-          filters: rule?.filters || [],
-          actionMatch: rule?.actionMatch || 'all',
-          filterMatch: rule?.filterMatch || 'all',
-          frequency: rule?.frequency || 60,
-          endpoint: previewEndpoint,
-        },
-      })
-      .then(([data, _, resp]) => {
-        if (this.isUnmounted) {
-          return;
-        }
-
-        GroupStore.add(data);
-
-        const pageLinks = resp?.getResponseHeader('Link');
-        const hits = resp?.getResponseHeader('X-Hits');
-        const endpoint = resp?.getResponseHeader('Endpoint');
-        const issueCount =
-          typeof hits !== 'undefined' && hits ? parseInt(hits, 10) || 0 : 0;
-        this.setState({
-          previewGroups: data.map(g => g.id),
-          previewError: null,
-          pageLinks: pageLinks ?? '',
-          issueCount,
-          loadingPreview: false,
-          previewEndpoint: endpoint ?? null,
-        });
-      })
-      .catch(_ => {
-        const errorMessage =
-          rule?.conditions.length || rule?.filters.length
-            ? t('Preview is not supported for these conditions')
-            : t('Select a condition to generate a preview');
-        this.setState({
-          previewError: errorMessage,
-          loadingPreview: false,
-        });
-      });
-  };
-
-  fetchPreviewDebounced = debounce(() => {
-    this.fetchPreview(true);
-  }, 1000);
-
   // As more incompatible combinations are added, we will need a more generic way to check for incompatibility.
   checkIncompatibleRuleDebounced = debounce(() => {
     const {conditionIndices, filterIndices} = findIncompatibleRules(this.state.rule);
@@ -494,13 +401,6 @@ class IssueRuleEditor extends DeprecatedAsyncView<Props, State> {
     });
   }, 500);
 
-  onPreviewCursor: CursorHandler = (cursor, _1, _2, direction) => {
-    this.setState({
-      previewCursor: cursor,
-      previewPage: this.state.previewPage + direction,
-    });
-  };
-
   fetchEnvironments() {
     const {organization} = this.props;
     const {project} = this.state;
@@ -1116,52 +1016,6 @@ class IssueRuleEditor extends DeprecatedAsyncView<Props, State> {
     );
   }
 
-  renderPreviewText() {
-    const {issueCount, previewError} = this.state;
-    if (previewError) {
-      return t(
-        "Select a condition above to see which issues would've triggered this alert"
-      );
-    }
-    return tct(
-      "[issueCount] issues would have triggered this rule in the past 14 days [approximately:approximately]. If you're looking to reduce noise then make sure to [link:read the docs].",
-      {
-        issueCount,
-        approximately: (
-          <Tooltip
-            title={t('Previews that include issue frequency conditions are approximated')}
-            showUnderline
-          />
-        ),
-        link: <ExternalLink href={SENTRY_ISSUE_ALERT_DOCS_URL} />,
-      }
-    );
-  }
-
-  renderPreviewTable() {
-    const {members} = this.props;
-    const {
-      previewGroups,
-      previewError,
-      pageLinks,
-      issueCount,
-      previewPage,
-      loadingPreview,
-    } = this.state;
-    return (
-      <PreviewTable
-        previewGroups={previewGroups}
-        members={members}
-        pageLinks={pageLinks}
-        onCursor={this.onPreviewCursor}
-        issueCount={issueCount}
-        page={previewPage}
-        loading={loadingPreview}
-        error={previewError}
-      />
-    );
-  }
-
   renderProjectSelect(disabled: boolean) {
     const {project: _selectedProject, projects, organization} = this.props;
     const {rule} = this.state;
@@ -1269,7 +1123,7 @@ class IssueRuleEditor extends DeprecatedAsyncView<Props, State> {
   }
 
   renderBody() {
-    const {organization} = this.props;
+    const {organization, members} = this.props;
     const {
       project,
       rule,
@@ -1570,15 +1424,9 @@ class IssueRuleEditor extends DeprecatedAsyncView<Props, State> {
               </StyledFieldHelp>
             </StyledListItem>
             <ContentIndent>{this.renderActionInterval(disabled)}</ContentIndent>
-            <StyledListItem>
-              <StyledListItemSpaced>
-                <div>
-                  <StepHeader>{t('Preview')}</StepHeader>
-                  <StyledFieldHelp>{this.renderPreviewText()}</StyledFieldHelp>
-                </div>
-              </StyledListItemSpaced>
-            </StyledListItem>
-            <ContentIndent>{this.renderPreviewTable()}</ContentIndent>
+            <ErrorBoundary mini>
+              <PreviewIssues members={members} rule={rule} project={project} />
+            </ErrorBoundary>
             <StyledListItem>
               <StepHeader>{t('Add a name and owner')}</StepHeader>
               <StyledFieldHelp>
@@ -1719,11 +1567,6 @@ const StyledListItem = styled(ListItem)`
   font-size: ${p => p.theme.fontSizeExtraLarge};
 `;
 
-const StyledListItemSpaced = styled('div')`
-  display: flex;
-  justify-content: space-between;
-`;
-
 const StyledFieldHelp = styled(FieldHelp)`
   margin-top: 0;
   @media (max-width: ${p => p.theme.breakpoints.small}) {

static/app/views/alerts/rules/issue/previewIssues.tsx

@@ -0,0 +1,215 @@
+import {Fragment, useCallback, useEffect, useMemo, useRef, useState} from 'react';
+import styled from '@emotion/styled';
+import debounce from 'lodash/debounce';
+
+import FieldHelp from 'sentry/components/forms/fieldGroup/fieldHelp';
+import ExternalLink from 'sentry/components/links/externalLink';
+import ListItem from 'sentry/components/list/listItem';
+import type {CursorHandler} from 'sentry/components/pagination';
+import {Tooltip} from 'sentry/components/tooltip';
+import {t, tct} from 'sentry/locale';
+import GroupStore from 'sentry/stores/groupStore';
+import {space} from 'sentry/styles/space';
+import type {Member, Project} from 'sentry/types';
+import {IssueAlertRule, UnsavedIssueAlertRule} from 'sentry/types/alerts';
+import useApi from 'sentry/utils/useApi';
+import {useIsMountedRef} from 'sentry/utils/useIsMountedRef';
+import useOrganization from 'sentry/utils/useOrganization';
+
+import PreviewTable from './previewTable';
+
+const SENTRY_ISSUE_ALERT_DOCS_URL =
+  'https://docs.sentry.io/product/alerts/alert-types/#issue-alerts';
+
+function PreviewText({issueCount, previewError}) {
+  if (previewError) {
+    return (
+      <Fragment>
+        {t("Select a condition above to see which issues would've triggered this alert")}
+      </Fragment>
+    );
+  }
+
+  return tct(
+    "[issueCount] issues would have triggered this rule in the past 14 days [approximately:approximately]. If you're looking to reduce noise then make sure to [link:read the docs].",
+    {
+      issueCount,
+      approximately: (
+        <Tooltip
+          title={t('Previews that include issue frequency conditions are approximated')}
+          showUnderline
+        />
+      ),
+      link: <ExternalLink href={SENTRY_ISSUE_ALERT_DOCS_URL} />,
+    }
+  );
+}
+
+interface PreviewIssuesProps {
+  members: Member[] | undefined;
+  project: Project;
+  rule?: UnsavedIssueAlertRule | IssueAlertRule | null;
+}
+
+export function PreviewIssues({members, rule, project}: PreviewIssuesProps) {
+  const api = useApi();
+  const organization = useOrganization();
+  const isMounted = useIsMountedRef();
+  const [isLoading, setIsLoading] = useState<boolean>(false);
+  const [previewError, setPreviewError] = useState<boolean>(false);
+  const [previewGroups, setPreviewGroups] = useState<string[]>([]);
+  const [previewPage, setPreviewPage] = useState<number>(0);
+  const [pageLinks, setPageLinks] = useState<string>('');
+  const [issueCount, setIssueCount] = useState<number>(0);
+  const endDateRef = useRef<string | null>(null);
+
+  /**
+   * If any of this data changes we'll need to re-fetch the preview
+   */
+  const relevantRuleData = useMemo(
+    () =>
+      rule
+        ? {
+            conditions: rule.conditions || [],
+            filters: rule.filters || [],
+            actionMatch: rule.actionMatch || 'all',
+            filterMatch: rule.filterMatch || 'all',
+            frequency: rule.frequency || 60,
+          }
+        : {},
+    [rule]
+  );
+
+  /**
+   * Not using useApiQuery because it makes a post request
+   */
+  const fetchApiData = useCallback(
+    (ruleFields: any, cursor?: string | null, resetCursor?: boolean) => {
+      setIsLoading(true);
+      if (resetCursor) {
+        setPreviewPage(0);
+      }
+
+      // we currently don't have a way to parse objects from query params, so this method is POST for now
+      api
+        .requestPromise(`/projects/${organization.slug}/${project.slug}/rules/preview/`, {
+          method: 'POST',
+          includeAllArgs: true,
+          query: {
+            cursor,
+            per_page: 5,
+          },
+          data: {
+            ...ruleFields,
+            // so the end date doesn't change? Not sure.
+            endpoint: endDateRef.current,
+          },
+        })
+        .then(([data, _, resp]) => {
+          if (!isMounted.current) {
+            return;
+          }
+
+          GroupStore.add(data);
+
+          const hits = resp?.getResponseHeader('X-Hits');
+          const count = typeof hits !== 'undefined' && hits ? parseInt(hits, 10) : 0;
+          setPreviewGroups(data.map(g => g.id));
+          setPreviewError(false);
+          setPageLinks(resp?.getResponseHeader('Link') ?? '');
+          setIssueCount(count);
+          setIsLoading(false);
+          endDateRef.current = resp?.getResponseHeader('Endpoint') ?? null;
+        })
+        .catch(_ => {
+          setPreviewError(true);
+          setIsLoading(false);
+        });
+    },
+    [
+      setIsLoading,
+      setPreviewError,
+      setPreviewGroups,
+      setIssueCount,
+      api,
+      project.slug,
+      organization.slug,
+      isMounted,
+    ]
+  );
+
+  const debouncedFetchApiData = useMemo(
+    () => debounce(fetchApiData, 500),
+    [fetchApiData]
+  );
+
+  useEffect(() => {
+    debouncedFetchApiData(relevantRuleData, null, true);
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [JSON.stringify(relevantRuleData), debouncedFetchApiData]);
+
+  useEffect(() => {
+    return () => {
+      debouncedFetchApiData.cancel();
+      // Reset the group store when leaving
+      GroupStore.reset();
+    };
+  }, [debouncedFetchApiData]);
+
+  const onPreviewCursor: CursorHandler = (cursor, _1, _2, direction) => {
+    setPreviewPage(previewPage + direction);
+    debouncedFetchApiData.cancel();
+    fetchApiData(relevantRuleData, cursor);
+  };
+
+  const errorMessage = previewError
+    ? rule?.conditions.length || rule?.filters.length
+      ? t('Preview is not supported for these conditions')
+      : t('Select a condition to generate a preview')
+    : null;
+
+  return (
+    <Fragment>
+      <StyledListItem>
+        <StepHeader>{t('Preview')}</StepHeader>
+        <StyledFieldHelp>
+          <PreviewText issueCount={issueCount} previewError={previewError} />
+        </StyledFieldHelp>
+      </StyledListItem>
+      <ContentIndent>
+        <PreviewTable
+          previewGroups={previewGroups}
+          members={members}
+          pageLinks={pageLinks}
+          onCursor={onPreviewCursor}
+          issueCount={issueCount}
+          page={previewPage}
+          isLoading={isLoading}
+          error={errorMessage}
+        />
+      </ContentIndent>
+    </Fragment>
+  );
+}
+
+const StyledListItem = styled(ListItem)`
+  margin: ${space(2)} 0 ${space(1)} 0;
+  font-size: ${p => p.theme.fontSizeExtraLarge};
+`;
+
+const StepHeader = styled('h5')`
+  margin-bottom: ${space(1)};
+`;
+
+const StyledFieldHelp = styled(FieldHelp)`
+  margin-top: 0;
+  @media (max-width: ${p => p.theme.breakpoints.small}) {
+    margin-left: -${space(4)};
+  }
+`;
+
+const ContentIndent = styled('div')`
+  @media (min-width: ${p => p.theme.breakpoints.small}) {
+    margin-left: ${space(4)};
+  }
+`;

static/app/views/alerts/rules/issue/previewTable.tsx

@@ -15,13 +15,13 @@ import {Group, Member} from 'sentry/types';
 
 type Props = {
   error: string | null;
+  isLoading: boolean;
   issueCount: number;
-  loading: boolean;
   members: Member[] | undefined;
   onCursor: CursorHandler;
   page: number;
   pageLinks: string;
-  previewGroups: string[] | null;
+  previewGroups: string[];
 };
 
 function PreviewTable({
@@ -31,11 +31,11 @@ function PreviewTable({
   onCursor,
   issueCount,
   page,
-  loading,
+  isLoading,
   error,
 }: Props) {
   const renderBody = () => {
-    if (loading) {
+    if (isLoading) {
       return <LoadingIndicator />;
     }
 
@@ -54,7 +54,7 @@ function PreviewTable({
       );
     }
     const memberList = indexMembersByProject(members);
-    return previewGroups?.map((id, index) => {
+    return previewGroups.map((id, index) => {
       const group = GroupStore.get(id) as Group | undefined;
 
       return (
@@ -75,7 +75,7 @@ function PreviewTable({
   };
 
   const renderCaption = () => {
-    if (loading || error || !previewGroups) {
+    if (isLoading || error || !previewGroups) {
       return null;
     }
     const pageIssues = page * 5 + previewGroups.length;
@@ -91,7 +91,7 @@ function PreviewTable({
         pageLinks={pageLinks}
         onCursor={onCursor}
         caption={renderCaption()}
-        disabled={loading}
+        disabled={isLoading}
       />
     );
   };