From 57c71074a393a99893db5d7dff13ecd9719e5069 Mon Sep 17 00:00:00 2001 From: Mia Hsu Date: Tue, 6 Aug 2024 13:19:42 -0700 Subject: [PATCH 1/3] add logging + functional button component --- .../app/utils/analytics/integrations/index.ts | 1 + .../analytics/onboardingAnalyticsEvents.tsx | 18 +++ .../alerts/rules/issue/addIntegrationRow.tsx | 10 +- static/app/views/alerts/rules/issue/index.tsx | 60 +++++++--- .../rules/issue/messagingIntegrationModal.tsx | 12 +- .../setupAlertIntegrationButton.spec.tsx | 63 +--------- .../issue/setupAlertIntegrationButton.tsx | 60 +--------- .../setupMessagingIntegrationButton.spec.tsx | 68 +++++++++++ .../issue/setupMessagingIntegrationButton.tsx | 109 ++++++++++++++++++ .../addIntegration.tsx | 1 + .../integrationContext.tsx | 1 + 11 files changed, 264 insertions(+), 139 deletions(-) create mode 100644 static/app/views/alerts/rules/issue/setupMessagingIntegrationButton.spec.tsx create mode 100644 static/app/views/alerts/rules/issue/setupMessagingIntegrationButton.tsx diff --git a/static/app/utils/analytics/integrations/index.ts b/static/app/utils/analytics/integrations/index.ts index 042ce4f724b82e..94ea73b19a68b7 100644 --- a/static/app/utils/analytics/integrations/index.ts +++ b/static/app/utils/analytics/integrations/index.ts @@ -13,6 +13,7 @@ export type IntegrationView = { | 'stacktrace_link' | 'stacktrace_issue_details' | 'integration_configuration_detail' + | 'messaging_integration_onboarding' | 'onboarding' | 'project_creation' | 'developer_settings' diff --git a/static/app/utils/analytics/onboardingAnalyticsEvents.tsx b/static/app/utils/analytics/onboardingAnalyticsEvents.tsx index fa784ed8b00ccc..dcddc4401c8703 100644 --- a/static/app/utils/analytics/onboardingAnalyticsEvents.tsx +++ b/static/app/utils/analytics/onboardingAnalyticsEvents.tsx @@ -28,6 +28,13 @@ 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; @@ -46,6 +53,9 @@ 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; @@ -80,4 +90,12 @@ export const onboardingEventMap: Record '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', }; diff --git a/static/app/views/alerts/rules/issue/addIntegrationRow.tsx b/static/app/views/alerts/rules/issue/addIntegrationRow.tsx index 93542f8cc377aa..689ddef568fe5c 100644 --- a/static/app/views/alerts/rules/issue/addIntegrationRow.tsx +++ b/static/app/views/alerts/rules/issue/addIntegrationRow.tsx @@ -5,6 +5,7 @@ import Access from 'sentry/components/acl/access'; import PluginIcon from 'sentry/plugins/components/pluginIcon'; import {space} from 'sentry/styles/space'; import type {Organization} from 'sentry/types/organization'; +import {trackAnalytics} from 'sentry/utils/analytics'; import IntegrationButton from 'sentry/views/settings/organizationIntegrations/integrationButton'; import {IntegrationContext} from 'sentry/views/settings/organizationIntegrations/integrationContext'; @@ -23,6 +24,13 @@ function AddIntegrationRow({organization, onClick}: Props) { integration.onAddIntegration?.(); onClick(); }; + const onExternalClick = () => { + trackAnalytics('onboarding.messaging_integration_external_install_clicked', { + provider_key: provider.key, + organization, + }); + onClick(); + }; const buttonProps = { size: 'sm', @@ -43,7 +51,7 @@ function AddIntegrationRow({organization, onClick}: Props) { organization={organization} userHasAccess={hasAccess} onAddIntegration={onAddIntegration} - onExternalClick={onClick} + onExternalClick={onExternalClick} externalInstallText="Add Installation" buttonProps={buttonProps} /> diff --git a/static/app/views/alerts/rules/issue/index.tsx b/static/app/views/alerts/rules/issue/index.tsx index 291ef082ef0d96..d216b076fd409c 100644 --- a/static/app/views/alerts/rules/issue/index.tsx +++ b/static/app/views/alerts/rules/issue/index.tsx @@ -69,6 +69,7 @@ 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, @@ -1228,12 +1229,20 @@ class IssueRuleEditor extends DeprecatedAsyncView { - {t('Set conditions')} - + {t('Set conditions')}{' '} + {organization.features.includes('messaging-integration-onboarding') ? ( + + ) : ( + + )} @@ -1435,18 +1444,33 @@ class IssueRuleEditor extends DeprecatedAsyncView { ) } - 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, - }} + {...(organization.features.includes( + 'messaging-integration-onboarding' + ) + ? { + 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(); + }, + }, + } + : {})} /> - - ); - } - const {isSelfHosted} = ConfigStore.getState(); // link to docs to set up Slack for self-hosted folks const referrerQuery = '?referrer=issue-alert-builder'; @@ -132,8 +81,3 @@ export default class SetupAlertIntegrationButton extends DeprecatedAsyncComponen ); } } - -const IconWrapper = styled('div')` - display: flex; - gap: ${space(1)}; -`; diff --git a/static/app/views/alerts/rules/issue/setupMessagingIntegrationButton.spec.tsx b/static/app/views/alerts/rules/issue/setupMessagingIntegrationButton.spec.tsx new file mode 100644 index 00000000000000..3879a21e3d03ec --- /dev/null +++ b/static/app/views/alerts/rules/issue/setupMessagingIntegrationButton.spec.tsx @@ -0,0 +1,68 @@ +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 = () => ( + + ); + + it('renders when no integration is installed', async function () { + MockApiClient.addMockResponse({ + url: `/projects/${organization.slug}/${project.slug}/?expand=hasAlertIntegration`, + body: { + ...project, + hasAlertIntegrationInstalled: false, + }, + }); + render(getComponent()); + 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}/?expand=hasAlertIntegration`, + body: { + ...project, + hasAlertIntegrationInstalled: true, + }, + }); + const {container} = render(getComponent()); + expect(container).not.toHaveTextContent('Connect to messaging'); + }); + + it('opens modal when clicked', async function () { + MockApiClient.addMockResponse({ + url: `/projects/${organization.slug}/${project.slug}/?expand=hasAlertIntegration`, + body: { + ...project, + hasAlertIntegrationInstalled: false, + }, + }); + render( + + ); + const button = await screen.findByRole('button', {name: /connect to messaging/i}); + await userEvent.click(button); + expect(openModal).toHaveBeenCalled(); + }); +}); diff --git a/static/app/views/alerts/rules/issue/setupMessagingIntegrationButton.tsx b/static/app/views/alerts/rules/issue/setupMessagingIntegrationButton.tsx new file mode 100644 index 00000000000000..efb62b4c67d86d --- /dev/null +++ b/static/app/views/alerts/rules/issue/setupMessagingIntegrationButton.tsx @@ -0,0 +1,109 @@ +import {useEffect} from 'react'; +import styled from '@emotion/styled'; + +import {openModal} from 'sentry/actionCreators/modal'; +import {Button} from 'sentry/components/button'; +import {Tooltip} from 'sentry/components/tooltip'; +import {t} from 'sentry/locale'; +import PluginIcon from 'sentry/plugins/components/pluginIcon'; +import {space} from 'sentry/styles/space'; +import type {Organization} from 'sentry/types/organization'; +import type {Project} from 'sentry/types/project'; +import {trackAnalytics} from 'sentry/utils/analytics'; +import {useApiQuery} from 'sentry/utils/queryClient'; +import MessagingIntegrationModal from 'sentry/views/alerts/rules/issue/messagingIntegrationModal'; + +type Props = { + organization: Organization; + projectSlug: string; + refetchConfigs: () => void; +}; + +function SetupMessagingIntegrationButton({ + organization, + projectSlug, + refetchConfigs, +}: Props) { + const providerKeys = ['slack', 'discord', 'msteams']; + + const onAddIntegration = () => { + refetch(); + refetchConfigs(); + }; + + const { + data: project, + isLoading, + isError, + refetch, + } = useApiQuery<{}>( + [`/projects/${organization.slug}/${projectSlug}/?expand=hasAlertIntegration`], + {staleTime: Infinity} + ); + + const detailedProject = project as Project & { + hasAlertIntegrationInstalled: boolean; + }; + + useEffect(() => { + if (detailedProject && !detailedProject.hasAlertIntegrationInstalled) { + trackAnalytics('onboarding.messaging_integration_button_rendered', { + project_id: detailedProject.id, + organization, + }); + } + }, [detailedProject, organization]); + + if (isLoading || isError) { + return null; + } + + if (!detailedProject || detailedProject.hasAlertIntegrationInstalled) { + return null; + } + + // TODO(Mia): only render if organization has team plan and above + return ( + + + + ); +} + +const IconWrapper = styled('div')` + display: flex; + gap: ${space(1)}; +`; + +export default SetupMessagingIntegrationButton; diff --git a/static/app/views/settings/organizationIntegrations/addIntegration.tsx b/static/app/views/settings/organizationIntegrations/addIntegration.tsx index c65344e521efee..096bda585f0697 100644 --- a/static/app/views/settings/organizationIntegrations/addIntegration.tsx +++ b/static/app/views/settings/organizationIntegrations/addIntegration.tsx @@ -24,6 +24,7 @@ type Props = { view: | 'integrations_directory_integration_detail' | 'integrations_directory' + | 'messaging_integration_onboarding' | 'onboarding' | 'project_creation'; }; diff --git a/static/app/views/settings/organizationIntegrations/integrationContext.tsx b/static/app/views/settings/organizationIntegrations/integrationContext.tsx index 624d6ccc1b2aab..5ac204cbf34908 100644 --- a/static/app/views/settings/organizationIntegrations/integrationContext.tsx +++ b/static/app/views/settings/organizationIntegrations/integrationContext.tsx @@ -8,6 +8,7 @@ export type IntegrationContextProps = { view: | 'integrations_directory_integration_detail' | 'integrations_directory' + | 'messaging_integration_onboarding' | 'onboarding' | 'project_creation'; }; From 7258198bad7df84f314b0f2632605e4aaa00238f Mon Sep 17 00:00:00 2001 From: Mia Hsu Date: Wed, 7 Aug 2024 09:38:45 -0700 Subject: [PATCH 2/3] fixes --- .../rules/issue/addIntegrationRow.spec.tsx | 8 +-- .../alerts/rules/issue/addIntegrationRow.tsx | 7 ++- static/app/views/alerts/rules/issue/index.tsx | 53 +++++++++---------- .../issue/messagingIntegrationModal.spec.tsx | 5 +- .../rules/issue/messagingIntegrationModal.tsx | 22 +++----- .../setupMessagingIntegrationButton.spec.tsx | 21 +++----- .../issue/setupMessagingIntegrationButton.tsx | 49 +++++++++-------- .../integrationButton.spec.tsx | 7 ++- .../integrationButton.tsx | 5 +- .../integrationDetailedView.tsx | 2 - 10 files changed, 81 insertions(+), 98 deletions(-) diff --git a/static/app/views/alerts/rules/issue/addIntegrationRow.spec.tsx b/static/app/views/alerts/rules/issue/addIntegrationRow.spec.tsx index ad461426075d9a..6d261b388374b4 100644 --- a/static/app/views/alerts/rules/issue/addIntegrationRow.spec.tsx +++ b/static/app/views/alerts/rules/issue/addIntegrationRow.spec.tsx @@ -32,12 +32,12 @@ describe('AddIntegrationRow', function () { modalParams: {project: project.id}, }} > - + ); it('renders', async () => { - render(getComponent()); + render(getComponent(), {organization: org}); 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()); + render(getComponent(), {organization: org}); 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()); + render(getComponent(), {organization: org}); await screen.findByRole('button', {name: /request installation/i}); }); diff --git a/static/app/views/alerts/rules/issue/addIntegrationRow.tsx b/static/app/views/alerts/rules/issue/addIntegrationRow.tsx index 689ddef568fe5c..e2444fa3cd6e03 100644 --- a/static/app/views/alerts/rules/issue/addIntegrationRow.tsx +++ b/static/app/views/alerts/rules/issue/addIntegrationRow.tsx @@ -4,17 +4,17 @@ 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 type {Organization} from 'sentry/types/organization'; import {trackAnalytics} from 'sentry/utils/analytics'; +import useOrganization from 'sentry/utils/useOrganization'; import IntegrationButton from 'sentry/views/settings/organizationIntegrations/integrationButton'; import {IntegrationContext} from 'sentry/views/settings/organizationIntegrations/integrationContext'; type Props = { onClick: () => void; - organization: Organization; }; -function AddIntegrationRow({organization, onClick}: Props) { +function AddIntegrationRow({onClick}: Props) { + const organization = useOrganization(); const integration = useContext(IntegrationContext); if (!integration) { return null; @@ -48,7 +48,6 @@ function AddIntegrationRow({organization, onClick}: Props) { {({hasAccess}) => { return ( { 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 `
` 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 @@ -1230,10 +1233,9 @@ class IssueRuleEditor extends DeprecatedAsyncView { {t('Set conditions')}{' '} - {organization.features.includes('messaging-integration-onboarding') ? ( + {hasMessagingIntegrationOnboarding ? ( ) : ( @@ -1444,33 +1446,28 @@ class IssueRuleEditor extends DeprecatedAsyncView { ) } - {...(organization.features.includes( - 'messaging-integration-onboarding' - ) - ? { - 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(); - }, + {...(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(); + }, + }, + })} /> diff --git a/static/app/views/settings/organizationIntegrations/integrationButton.spec.tsx b/static/app/views/settings/organizationIntegrations/integrationButton.spec.tsx index a5d7765b0b4d79..a93a763ff20e49 100644 --- a/static/app/views/settings/organizationIntegrations/integrationButton.spec.tsx +++ b/static/app/views/settings/organizationIntegrations/integrationButton.spec.tsx @@ -37,7 +37,6 @@ describe('AddIntegrationButton', function () { }} > void; onExternalClick: () => void; - organization: Organization; userHasAccess: boolean; externalInstallText?: string; }; @@ -25,13 +24,13 @@ type ButtonProps = { } | null; function IntegrationButton({ - organization, userHasAccess, onAddIntegration, onExternalClick, externalInstallText, buttonProps, }: Props) { + const organization = useOrganization(); const {provider, type, installStatus, analyticsParams, modalParams} = useContext(IntegrationContext) ?? {}; if (!provider || !type) return null; diff --git a/static/app/views/settings/organizationIntegrations/integrationDetailedView.tsx b/static/app/views/settings/organizationIntegrations/integrationDetailedView.tsx index 05a7cab9125c10..b3c0721d168df6 100644 --- a/static/app/views/settings/organizationIntegrations/integrationDetailedView.tsx +++ b/static/app/views/settings/organizationIntegrations/integrationDetailedView.tsx @@ -239,7 +239,6 @@ class IntegrationDetailedView extends AbstractIntegrationDetailedView< } renderTopButton(disabledFromFeatures: boolean, userHasAccess: boolean) { - const {organization} = this.props; const provider = this.provider; const buttonProps = { @@ -262,7 +261,6 @@ class IntegrationDetailedView extends AbstractIntegrationDetailedView< }} > Date: Wed, 7 Aug 2024 10:31:44 -0700 Subject: [PATCH 3/3] more fix --- .../rules/issue/messagingIntegrationModal.tsx | 4 ++-- .../issue/setupMessagingIntegrationButton.tsx | 17 +++++++---------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/static/app/views/alerts/rules/issue/messagingIntegrationModal.tsx b/static/app/views/alerts/rules/issue/messagingIntegrationModal.tsx index 9cca07cd8c5872..0363c6881daa57 100644 --- a/static/app/views/alerts/rules/issue/messagingIntegrationModal.tsx +++ b/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: string; + headerContent: React.ReactNode; organization: Organization; project: Project; providerKeys: string[]; - bodyContent?: string; + bodyContent?: React.ReactNode; onAddIntegration?: () => void; }; diff --git a/static/app/views/alerts/rules/issue/setupMessagingIntegrationButton.tsx b/static/app/views/alerts/rules/issue/setupMessagingIntegrationButton.tsx index 2b63c9510f3a14..d8ca2ff493cd56 100644 --- a/static/app/views/alerts/rules/issue/setupMessagingIntegrationButton.tsx +++ b/static/app/views/alerts/rules/issue/setupMessagingIntegrationButton.tsx @@ -1,4 +1,3 @@ -import {useEffect} from 'react'; import styled from '@emotion/styled'; import {openModal} from 'sentry/actionCreators/modal'; @@ -10,6 +9,7 @@ import {space} from 'sentry/styles/space'; import type {Project} from 'sentry/types/project'; import {trackAnalytics} from 'sentry/utils/analytics'; import {useApiQuery} from 'sentry/utils/queryClient'; +import useRouteAnalyticsParams from 'sentry/utils/routeAnalytics/useRouteAnalyticsParams'; import useOrganization from 'sentry/utils/useOrganization'; import MessagingIntegrationModal from 'sentry/views/alerts/rules/issue/messagingIntegrationModal'; @@ -44,20 +44,17 @@ function SetupMessagingIntegrationButton({projectSlug, refetchConfigs}: Props) { {staleTime: Infinity} ); - useEffect(() => { - if (project && !project.hasAlertIntegrationInstalled) { - trackAnalytics('onboarding.messaging_integration_button_rendered', { - project_id: project.id, - organization, - }); - } - }, [project, organization]); + const shouldRenderSetupButton = project && !project.hasAlertIntegrationInstalled; + + useRouteAnalyticsParams({ + setup_message_integration_button_shown: shouldRenderSetupButton, + }); if (isLoading || isError) { return null; } - if (!project || project.hasAlertIntegrationInstalled) { + if (!shouldRenderSetupButton) { return null; }