Skip to content

Commit

Permalink
Revert "feat(onboarding): add logging (#75679)"
Browse files Browse the repository at this point in the history
This reverts commit 26cb3d1.

Co-authored-by: ameliahsu <55610339+ameliahsu@users.noreply.github.com>
  • Loading branch information
getsentry-bot and ameliahsu committed Aug 7, 2024
1 parent 913036d commit 064a0e0
Show file tree
Hide file tree
Showing 16 changed files with 160 additions and 265 deletions.
1 change: 0 additions & 1 deletion static/app/utils/analytics/integrations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ export type IntegrationView = {
| 'stacktrace_link'
| 'stacktrace_issue_details'
| 'integration_configuration_detail'
| 'messaging_integration_onboarding'
| 'onboarding'
| 'project_creation'
| 'developer_settings'
Expand Down
18 changes: 0 additions & 18 deletions static/app/utils/analytics/onboardingAnalyticsEvents.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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',
};
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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);
Expand All @@ -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});
});
Expand Down
17 changes: 5 additions & 12 deletions static/app/views/alerts/rules/issue/addIntegrationRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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',
Expand All @@ -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}
/>
Expand Down
53 changes: 16 additions & 37 deletions static/app/views/alerts/rules/issue/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down Expand Up @@ -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];
Expand All @@ -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>
);
})}
Expand Down
Original file line number Diff line number Diff line change
@@ -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: {
Expand All @@ -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: {
Expand All @@ -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();
});
});
Loading

0 comments on commit 064a0e0

Please sign in to comment.