Browse Source

Revert "feat(onboarding): add logging (#75679)"

This reverts commit 26cb3d10861f1165b17809031e027bd4f8fe375c.

Co-authored-by: ameliahsu <55610339+ameliahsu@users.noreply.github.com>
getsentry-bot 7 months ago
parent
commit
064a0e087d

+ 0 - 1
static/app/utils/analytics/integrations/index.ts

@@ -13,7 +13,6 @@ export type IntegrationView = {
     | 'stacktrace_link'
     | 'stacktrace_issue_details'
     | 'integration_configuration_detail'
-    | 'messaging_integration_onboarding'
     | 'onboarding'
     | 'project_creation'
     | 'developer_settings'

+ 0 - 18
static/app/utils/analytics/onboardingAnalyticsEvents.tsx

@@ -28,13 +28,6 @@ export type OnboardingEventParameters = {
     platform: string;
     project_id: string;
   };
-  'onboarding.messaging_integration_external_install_clicked': {
-    provider_key: string;
-  };
-  'onboarding.messaging_integration_modal_rendered': {
-    project_id: string;
-  };
-  'onboarding.messaging_integration_steps_refreshed': {};
   'onboarding.nextjs-dsn-copied': {};
   'onboarding.select_framework_modal_close_button_clicked': {
     platform: string;
@@ -53,9 +46,6 @@ export type OnboardingEventParameters = {
     platform: string;
     project_id: string;
   };
-  'onboarding.setup_messaging_integration_button_rendered': {
-    project_id: string;
-  };
   'onboarding.source_maps_wizard_button_copy_clicked': {
     platform: string;
     project_id: string;
@@ -90,12 +80,4 @@ export const onboardingEventMap: Record<keyof OnboardingEventParameters, string>
   'onboarding.source_maps_wizard_selected_and_copied':
     'Onboarding: Source Maps Wizard Selected and Copied',
   'onboarding.nextjs-dsn-copied': 'Onboarding: NextJS DSN Copied',
-  'onboarding.setup_messaging_integration_button_rendered':
-    'Onboarding: Setup Messaging Integration Button Rendered',
-  'onboarding.messaging_integration_modal_rendered':
-    'Onboarding: Messaging Integration Modal Rendered',
-  'onboarding.messaging_integration_external_install_clicked':
-    'Onboarding: Messaging Integration External Install Clicked',
-  'onboarding.messaging_integration_steps_refreshed':
-    'Onboarding: Messaging Integration Steps Refreshed',
 };

+ 4 - 4
static/app/views/alerts/rules/issue/addIntegrationRow.spec.tsx

@@ -32,12 +32,12 @@ describe('AddIntegrationRow', function () {
         modalParams: {project: project.id},
       }}
     >
-      <AddIntegrationRow onClick={jest.fn()} />
+      <AddIntegrationRow organization={org} onClick={jest.fn()} />
     </IntegrationContext.Provider>
   );
 
   it('renders', async () => {
-    render(getComponent(), {organization: org});
+    render(getComponent());
 
     const button = await screen.findByRole('button', {name: /add integration/i});
     expect(button).toBeInTheDocument();
@@ -49,7 +49,7 @@ describe('AddIntegrationRow', function () {
     // any is needed here because getSentry has different types for global
     (global as any).open = open;
 
-    render(getComponent(), {organization: org});
+    render(getComponent());
 
     const button = await screen.findByRole('button', {name: /add integration/i});
     await userEvent.click(button);
@@ -63,7 +63,7 @@ describe('AddIntegrationRow', function () {
   it('renders request button when user does not have access', async () => {
     org.access = ['org:read'];
 
-    render(getComponent(), {organization: org});
+    render(getComponent());
 
     await screen.findByRole('button', {name: /request installation/i});
   });

+ 5 - 12
static/app/views/alerts/rules/issue/addIntegrationRow.tsx

@@ -4,17 +4,16 @@ import styled from '@emotion/styled';
 import Access from 'sentry/components/acl/access';
 import PluginIcon from 'sentry/plugins/components/pluginIcon';
 import {space} from 'sentry/styles/space';
-import {trackAnalytics} from 'sentry/utils/analytics';
-import useOrganization from 'sentry/utils/useOrganization';
+import type {Organization} from 'sentry/types/organization';
 import IntegrationButton from 'sentry/views/settings/organizationIntegrations/integrationButton';
 import {IntegrationContext} from 'sentry/views/settings/organizationIntegrations/integrationContext';
 
 type Props = {
   onClick: () => void;
+  organization: Organization;
 };
 
-function AddIntegrationRow({onClick}: Props) {
-  const organization = useOrganization();
+function AddIntegrationRow({organization, onClick}: Props) {
   const integration = useContext(IntegrationContext);
   if (!integration) {
     return null;
@@ -24,13 +23,6 @@ function AddIntegrationRow({onClick}: Props) {
     integration.onAddIntegration?.();
     onClick();
   };
-  const onExternalClick = () => {
-    trackAnalytics('onboarding.messaging_integration_external_install_clicked', {
-      provider_key: provider.key,
-      organization,
-    });
-    onClick();
-  };
 
   const buttonProps = {
     size: 'sm',
@@ -48,9 +40,10 @@ function AddIntegrationRow({onClick}: Props) {
         {({hasAccess}) => {
           return (
             <StyledButton
+              organization={organization}
               userHasAccess={hasAccess}
               onAddIntegration={onAddIntegration}
-              onExternalClick={onExternalClick}
+              onExternalClick={onClick}
               externalInstallText="Add Installation"
               buttonProps={buttonProps}
             />

+ 16 - 37
static/app/views/alerts/rules/issue/index.tsx

@@ -69,7 +69,6 @@ import normalizeUrl from 'sentry/utils/url/normalizeUrl';
 import withOrganization from 'sentry/utils/withOrganization';
 import withProjects from 'sentry/utils/withProjects';
 import {PreviewIssues} from 'sentry/views/alerts/rules/issue/previewIssues';
-import SetupMessagingIntegrationButton from 'sentry/views/alerts/rules/issue/setupMessagingIntegrationButton';
 import {
   CHANGE_ALERT_CONDITION_IDS,
   CHANGE_ALERT_PLACEHOLDERS_LABELS,
@@ -1168,9 +1167,6 @@ class IssueRuleEditor extends DeprecatedAsyncView<Props, State> {
     const disabled = loading || !(canCreateAlert || isActiveSuperuser());
     const displayDuplicateError =
       detailedError?.name?.some(str => isExactDuplicateExp.test(str)) ?? false;
-    const hasMessagingIntegrationOnboarding = organization.features.includes(
-      'messaging-integration-onboarding'
-    );
 
     // Note `key` on `<Form>` below is so that on initial load, we show
     // the form with a loading mask on top of it, but force a re-render by using
@@ -1223,19 +1219,12 @@ class IssueRuleEditor extends DeprecatedAsyncView<Props, State> {
               </SettingsContainer>
             </ContentIndent>
             <SetConditionsListItem>
-              <StepHeader>{t('Set conditions')}</StepHeader>{' '}
-              {hasMessagingIntegrationOnboarding ? (
-                <SetupMessagingIntegrationButton
-                  projectSlug={project.slug}
-                  refetchConfigs={this.refetchConfigs}
-                />
-              ) : (
-                <SetupAlertIntegrationButton
-                  projectSlug={project.slug}
-                  organization={organization}
-                  refetchConfigs={this.refetchConfigs}
-                />
-              )}
+              <StepHeader>{t('Set conditions')}</StepHeader>
+              <SetupAlertIntegrationButton
+                projectSlug={project.slug}
+                organization={organization}
+                refetchConfigs={this.refetchConfigs}
+              />
             </SetConditionsListItem>
             <ContentIndent>
               <ConditionsPanel>
@@ -1437,28 +1426,18 @@ class IssueRuleEditor extends DeprecatedAsyncView<Props, State> {
                               </StyledAlert>
                             )
                           }
-                          {...(hasMessagingIntegrationOnboarding && {
-                            additionalAction: {
-                              label: 'Notify integration\u{2026}',
-                              option: {
-                                label: 'Missing an integration? Click here to refresh',
-                                value: {
-                                  enabled: true,
-                                  id: 'refresh_configs',
-                                  label: 'Refresh Integration List',
-                                },
-                              },
-                              onClick: () => {
-                                trackAnalytics(
-                                  'onboarding.messaging_integration_steps_refreshed',
-                                  {
-                                    organization: this.props.organization,
-                                  }
-                                );
-                                this.refetchConfigs();
+                          additionalAction={{
+                            label: 'Notify integration\u{2026}',
+                            option: {
+                              label: 'Missing an integration? Click here to refresh',
+                              value: {
+                                enabled: true,
+                                id: 'refresh_configs',
+                                label: 'Refresh Integration List',
                               },
                             },
-                          })}
+                            onClick: this.refetchConfigs,
+                          }}
                         />
                         <TestButtonWrapper>
                           <Button

+ 2 - 3
static/app/views/alerts/rules/issue/messagingIntegrationModal.spec.tsx

@@ -11,7 +11,6 @@ import {
   ModalBody,
   ModalFooter,
 } from 'sentry/components/globalModal/components';
-import {t} from 'sentry/locale';
 import MessagingIntegrationModal from 'sentry/views/alerts/rules/issue/messagingIntegrationModal';
 
 jest.mock('sentry/actionCreators/modal');
@@ -37,8 +36,8 @@ describe('MessagingIntegrationModal', function () {
       closeModal={closeModal ? closeModal : jest.fn()}
       Header={makeClosableHeader(() => {})}
       Body={ModalBody}
-      headerContent={t('Connect with a messaging tool')}
-      bodyContent={t('Receive alerts and digests right where you work.')}
+      headerContent={<h1>Connect with a messaging tool</h1>}
+      bodyContent={<p>Receive alerts and digests right where you work.</p>}
       providerKeys={providerKeys}
       organization={org}
       project={project}

+ 6 - 8
static/app/views/alerts/rules/issue/messagingIntegrationModal.tsx

@@ -13,11 +13,11 @@ import AddIntegrationRow from 'sentry/views/alerts/rules/issue/addIntegrationRow
 import {IntegrationContext} from 'sentry/views/settings/organizationIntegrations/integrationContext';
 
 type Props = ModalRenderProps & {
-  headerContent: React.ReactNode;
+  headerContent: React.ReactElement<any, any>;
   organization: Organization;
   project: Project;
   providerKeys: string[];
-  bodyContent?: React.ReactNode;
+  bodyContent?: React.ReactElement<any, any>;
   onAddIntegration?: () => void;
 };
 
@@ -51,11 +51,9 @@ function MessagingIntegrationModal({
 
   return (
     <Fragment>
-      <Header closeButton>
-        <h1>{headerContent}</h1>
-      </Header>
+      <Header closeButton>{headerContent}</Header>
       <Body>
-        <p>{bodyContent}</p>
+        {bodyContent}
         <IntegrationsWrapper>
           {queryResults.map(result => {
             const provider = result.data?.providers[0];
@@ -72,13 +70,13 @@ function MessagingIntegrationModal({
                   installStatus: 'Not Installed',
                   analyticsParams: {
                     already_installed: false,
-                    view: 'messaging_integration_onboarding',
+                    view: 'onboarding',
                   },
                   onAddIntegration: onAddIntegration,
                   modalParams: {projectId: project.id},
                 }}
               >
-                <AddIntegrationRow onClick={closeModal} />
+                <AddIntegrationRow organization={organization} onClick={closeModal} />
               </IntegrationContext.Provider>
             );
           })}

+ 60 - 3
static/app/views/alerts/rules/issue/setupAlertIntegrationButton.spec.tsx

@@ -1,17 +1,21 @@
 import {OrganizationFixture} from 'sentry-fixture/organization';
 import {ProjectFixture} from 'sentry-fixture/project';
 
-import {render} from 'sentry-test/reactTestingLibrary';
+import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
 
+import {openModal} from 'sentry/actionCreators/modal';
 import SetupAlertIntegrationButton from 'sentry/views/alerts/rules/issue/setupAlertIntegrationButton';
 
 jest.mock('sentry/actionCreators/modal');
 
 describe('SetupAlertIntegrationButton', function () {
   const organization = OrganizationFixture();
+  const featureOrg = OrganizationFixture({
+    features: ['messaging-integration-onboarding'],
+  });
   const project = ProjectFixture();
 
-  it('renders slack button if no alert integrations are installed', function () {
+  it('renders slack button if no alert integrations when feature flag is off', function () {
     MockApiClient.addMockResponse({
       url: `/projects/${organization.slug}/${project.slug}/?expand=hasAlertIntegration`,
       body: {
@@ -28,7 +32,7 @@ describe('SetupAlertIntegrationButton', function () {
     );
     expect(container).toHaveTextContent('Set Up Slack Now');
   });
-  it('does not render button if alert integration is installed', function () {
+  it('does not render button if alert integration installed when feature flag is off', function () {
     MockApiClient.addMockResponse({
       url: `/projects/${organization.slug}/${project.slug}/?expand=hasAlertIntegration`,
       body: {
@@ -45,4 +49,57 @@ describe('SetupAlertIntegrationButton', function () {
     );
     expect(container).not.toHaveTextContent('Set Up Slack Now');
   });
+  it('renders connect to messaging button when feature flag is on', function () {
+    MockApiClient.addMockResponse({
+      url: `/projects/${featureOrg.slug}/${project.slug}/?expand=hasAlertIntegration`,
+      body: {
+        ...project,
+        hasAlertIntegrationInstalled: false,
+      },
+    });
+    const {container} = render(
+      <SetupAlertIntegrationButton
+        projectSlug={project.slug}
+        organization={featureOrg}
+        refetchConfigs={jest.fn()}
+      />
+    );
+    expect(container).toHaveTextContent('Connect to messaging');
+  });
+  it('does not render button if alert integration installed when feature flag is on', function () {
+    MockApiClient.addMockResponse({
+      url: `/projects/${featureOrg.slug}/${project.slug}/?expand=hasAlertIntegration`,
+      body: {
+        ...project,
+        hasAlertIntegrationInstalled: true,
+      },
+    });
+    const {container} = render(
+      <SetupAlertIntegrationButton
+        projectSlug={project.slug}
+        organization={featureOrg}
+        refetchConfigs={jest.fn()}
+      />
+    );
+    expect(container).not.toHaveTextContent('Connect to messaging');
+  });
+  it('opens modal when clicked', async () => {
+    MockApiClient.addMockResponse({
+      url: `/projects/${featureOrg.slug}/${project.slug}/?expand=hasAlertIntegration`,
+      body: {
+        ...project,
+        hasAlertIntegrationInstalled: false,
+      },
+    });
+    render(
+      <SetupAlertIntegrationButton
+        projectSlug={project.slug}
+        organization={featureOrg}
+        refetchConfigs={jest.fn()}
+      />
+    );
+    await userEvent.click(screen.getByLabelText('Connect to messaging'));
+
+    expect(openModal).toHaveBeenCalled();
+  });
 });

+ 58 - 2
static/app/views/alerts/rules/issue/setupAlertIntegrationButton.tsx

@@ -1,11 +1,16 @@
+import styled from '@emotion/styled';
+
+import {openModal} from 'sentry/actionCreators/modal';
 import {Button} from 'sentry/components/button';
 import DeprecatedAsyncComponent from 'sentry/components/deprecatedAsyncComponent';
 import {Tooltip} from 'sentry/components/tooltip';
 import {t} from 'sentry/locale';
 import PluginIcon from 'sentry/plugins/components/pluginIcon';
 import ConfigStore from 'sentry/stores/configStore';
+import {space} from 'sentry/styles/space';
 import type {Organization} from 'sentry/types/organization';
 import type {Project} from 'sentry/types/project';
+import MessagingIntegrationModal from 'sentry/views/alerts/rules/issue/messagingIntegrationModal';
 
 type Props = DeprecatedAsyncComponent['props'] & {
   organization: Organization;
@@ -21,7 +26,7 @@ type State = DeprecatedAsyncComponent['state'] & {
 
 /**
  * This component renders a button to Set up an alert integration (just Slack for now)
- * if the project has no alerting integrations setup already. To be replaced by SetupMessagingIntegrationButton.
+ * if the project has no alerting integrations setup already.
  */
 export default class SetupAlertIntegrationButton extends DeprecatedAsyncComponent<
   Props,
@@ -47,15 +52,61 @@ export default class SetupAlertIntegrationButton extends DeprecatedAsyncComponen
   }
 
   renderBody(): React.ReactNode {
+    const headerContent = <h1>Connect with a messaging tool</h1>;
+    const bodyContent = <p>Receive alerts and digests right where you work.</p>;
+    const providerKeys = ['slack', 'discord', 'msteams'];
     const {organization} = this.props;
     const {detailedProject} = this.state;
-
+    const onAddIntegration = () => {
+      this.reloadData();
+      this.props.refetchConfigs();
+    };
     // don't render anything if we don't have the project yet or if an alert integration
     // is installed
     if (!detailedProject || detailedProject.hasAlertIntegrationInstalled) {
       return null;
     }
 
+    if (organization.features.includes('messaging-integration-onboarding')) {
+      // TODO(Mia): only render if organization has team plan and above
+      return (
+        <Tooltip
+          title={t('Send alerts to your messaging service. Install the integration now.')}
+        >
+          <Button
+            size="sm"
+            icon={
+              <IconWrapper>
+                {providerKeys.map((value: string) => {
+                  return <PluginIcon key={value} pluginId={value} size={16} />;
+                })}
+              </IconWrapper>
+            }
+            onClick={() =>
+              openModal(
+                deps => (
+                  <MessagingIntegrationModal
+                    {...deps}
+                    headerContent={headerContent}
+                    bodyContent={bodyContent}
+                    providerKeys={providerKeys}
+                    organization={organization}
+                    project={detailedProject}
+                    onAddIntegration={onAddIntegration}
+                  />
+                ),
+                {
+                  closeEvents: 'escape-key',
+                }
+              )
+            }
+          >
+            {t('Connect to messaging')}
+          </Button>
+        </Tooltip>
+      );
+    }
+
     const {isSelfHosted} = ConfigStore.getState();
     // link to docs to set up Slack for self-hosted folks
     const referrerQuery = '?referrer=issue-alert-builder';
@@ -81,3 +132,8 @@ export default class SetupAlertIntegrationButton extends DeprecatedAsyncComponen
     );
   }
 }
+
+const IconWrapper = styled('div')`
+  display: flex;
+  gap: ${space(1)};
+`;

+ 0 - 61
static/app/views/alerts/rules/issue/setupMessagingIntegrationButton.spec.tsx

@@ -1,61 +0,0 @@
-import {OrganizationFixture} from 'sentry-fixture/organization';
-import {ProjectFixture} from 'sentry-fixture/project';
-
-import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
-
-import {openModal} from 'sentry/actionCreators/modal';
-import SetupMessagingIntegrationButton from 'sentry/views/alerts/rules/issue/setupMessagingIntegrationButton';
-
-jest.mock('sentry/actionCreators/modal');
-
-describe('SetupAlertIntegrationButton', function () {
-  const organization = OrganizationFixture({
-    features: ['messaging-integration-onboarding'],
-  });
-  const project = ProjectFixture();
-
-  const getComponent = () => (
-    <SetupMessagingIntegrationButton
-      projectSlug={project.slug}
-      refetchConfigs={jest.fn()}
-    />
-  );
-
-  it('renders when no integration is installed', async function () {
-    MockApiClient.addMockResponse({
-      url: `/projects/${organization.slug}/${project.slug}/`,
-      body: {
-        ...project,
-        hasAlertIntegrationInstalled: false,
-      },
-    });
-    render(getComponent(), {organization: organization});
-    await screen.findByRole('button', {name: /connect to messaging/i});
-  });
-
-  it('does not render button if alert integration installed when feature flag is on', function () {
-    MockApiClient.addMockResponse({
-      url: `/projects/${organization.slug}/${project.slug}/`,
-      body: {
-        ...project,
-        hasAlertIntegrationInstalled: true,
-      },
-    });
-    render(getComponent(), {organization: organization});
-    expect(screen.queryByRole('button')).not.toBeInTheDocument();
-  });
-
-  it('opens modal when clicked', async function () {
-    MockApiClient.addMockResponse({
-      url: `/projects/${organization.slug}/${project.slug}/`,
-      body: {
-        ...project,
-        hasAlertIntegrationInstalled: false,
-      },
-    });
-    render(getComponent(), {organization: organization});
-    const button = await screen.findByRole('button', {name: /connect to messaging/i});
-    await userEvent.click(button);
-    expect(openModal).toHaveBeenCalled();
-  });
-});

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