Skip to content

Commit 439e9fd

Browse files
committed
ref(project-creation): Add SCM-styled notification options
The SCM alert-frequency section renders the messaging-integration notification options in an SCM-styled layout instead of the classic inline card. Extracts useMessagingIntegrationAlertRule (channels query, channel validation, the provider/integration/channel options and change handlers) plus a shared ChannelSelect, so the classic MessagingIntegrationAlertRule and the new ScmMessagingIntegrationAlertRule build identical controls from one source. The classic inline layout is unchanged. ScmMessagingIntegrationAlertRule reuses the provider sentence but renders each segment, text or input, on its own full-width row in makeSentence's order, with no grey card background. Rendering segments independently keeps it correct regardless of how a translation reorders the placeholders. ScmIssueAlertNotificationOptions lifts the Notify via wording into a shared header so the checkboxes read Email and Integration.
1 parent 7f153aa commit 439e9fd

6 files changed

Lines changed: 384 additions & 75 deletions

static/app/views/onboarding/components/scmAlertFrequencySection.spec.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,10 @@ describe('ScmAlertFrequencySection', () => {
6969
it('shows the notification options when alerts are enabled', () => {
7070
renderSection({analyticsFlow: 'onboarding'});
7171

72-
expect(screen.getByText('Notify via email')).toBeInTheDocument();
72+
expect(screen.getByText('Notify via')).toBeInTheDocument();
73+
expect(
74+
screen.getByText('Integration (Slack, Discord, MS Teams, etc.)')
75+
).toBeInTheDocument();
7376
});
7477

7578
it('hides the notification options when alerts are turned off', () => {
@@ -81,6 +84,6 @@ describe('ScmAlertFrequencySection', () => {
8184
},
8285
});
8386

84-
expect(screen.queryByText('Notify via email')).not.toBeInTheDocument();
87+
expect(screen.queryByText('Notify via')).not.toBeInTheDocument();
8588
});
8689
});

static/app/views/onboarding/components/scmAlertFrequencySection.tsx

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,7 @@ import {Text} from '@sentry/scraps/text';
55
import {IconInfo} from 'sentry/icons';
66
import {t} from 'sentry/locale';
77
import type {TagVariant} from 'sentry/utils/theme';
8-
import {
9-
IssueAlertNotificationOptions,
10-
type IssueAlertNotificationProps,
11-
} from 'sentry/views/projectInstall/issueAlertNotificationOptions';
8+
import {type IssueAlertNotificationProps} from 'sentry/views/projectInstall/issueAlertNotificationOptions';
129
import {
1310
type AlertRuleOptions,
1411
RuleAction,
@@ -17,6 +14,7 @@ import {
1714
import {ScmAlertFrequency} from './scmAlertFrequency';
1815
import type {ScmAnalyticsFlow} from './scmAnalyticsFlow';
1916
import {ScmCollapsibleSection} from './scmCollapsibleSection';
17+
import {ScmIssueAlertNotificationOptions} from './scmIssueAlertNotificationOptions';
2018

2119
interface ScmAlertFrequencySectionProps {
2220
alertRuleConfig: AlertRuleOptions;
@@ -47,7 +45,7 @@ export function ScmAlertFrequencySection({
4745
// hide them for "create alerts later" (mirrors issueAlertOptions).
4846
const notificationOptions =
4947
alertRuleConfig.alertSetting === RuleAction.CREATE_ALERT_LATER ? null : (
50-
<IssueAlertNotificationOptions {...notificationProps} />
48+
<ScmIssueAlertNotificationOptions {...notificationProps} />
5149
);
5250

5351
const footer = (
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import {Stack} from '@sentry/scraps/layout';
2+
import {Text} from '@sentry/scraps/text';
3+
4+
import {MultipleCheckbox} from 'sentry/components/forms/controls/multipleCheckbox';
5+
import {t} from 'sentry/locale';
6+
import {useRouteAnalyticsParams} from 'sentry/utils/routeAnalytics/useRouteAnalyticsParams';
7+
import {
8+
MessagingIntegrationAnalyticsView,
9+
SetupMessagingIntegrationButton,
10+
} from 'sentry/views/alerts/rules/issue/setupMessagingIntegrationButton';
11+
import {
12+
type IssueAlertNotificationProps,
13+
MultipleCheckboxOptions,
14+
} from 'sentry/views/projectInstall/issueAlertNotificationOptions';
15+
16+
import {ScmMessagingIntegrationAlertRule} from './scmMessagingIntegrationAlertRule';
17+
18+
/**
19+
* SCM-styled notification options for the alert-frequency section. Mirrors
20+
* `IssueAlertNotificationOptions` but lifts the "Notify via" wording into a
21+
* shared header (so the checkboxes read just "Email" / "Integration ..."), and
22+
* renders the messaging rule stacked (`ScmMessagingIntegrationAlertRule`)
23+
* instead of the classic inline card.
24+
*/
25+
export function ScmIssueAlertNotificationOptions(props: IssueAlertNotificationProps) {
26+
const {actions, setActions, querySuccess, shouldRenderSetupButton} = props;
27+
28+
const shouldRenderNotificationConfigs = actions.some(
29+
v => v !== MultipleCheckboxOptions.EMAIL
30+
);
31+
32+
useRouteAnalyticsParams({
33+
setup_message_integration_button_shown: shouldRenderSetupButton,
34+
});
35+
36+
if (!querySuccess) {
37+
return null;
38+
}
39+
40+
return (
41+
<Stack gap="lg" padding="lg 0">
42+
<Text size="sm" bold variant="secondary" uppercase>
43+
{t('Notify via')}
44+
</Text>
45+
<MultipleCheckbox
46+
name="notification"
47+
value={actions}
48+
onChange={values => setActions(values)}
49+
>
50+
<Stack gap="md" width="100%">
51+
<Stack>
52+
<MultipleCheckbox.Item value={MultipleCheckboxOptions.EMAIL} disabled>
53+
{t('Email')}
54+
</MultipleCheckbox.Item>
55+
{shouldRenderSetupButton ? null : (
56+
<MultipleCheckbox.Item value={MultipleCheckboxOptions.INTEGRATION}>
57+
{t('Integration (Slack, Discord, MS Teams, etc.)')}
58+
</MultipleCheckbox.Item>
59+
)}
60+
</Stack>
61+
{!shouldRenderSetupButton && shouldRenderNotificationConfigs ? (
62+
<ScmMessagingIntegrationAlertRule {...props} />
63+
) : null}
64+
</Stack>
65+
</MultipleCheckbox>
66+
{shouldRenderSetupButton && (
67+
<SetupMessagingIntegrationButton
68+
analyticsView={MessagingIntegrationAnalyticsView.PROJECT_CREATION}
69+
/>
70+
)}
71+
</Stack>
72+
);
73+
}
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import {OrganizationFixture} from 'sentry-fixture/organization';
2+
import {OrganizationIntegrationsFixture} from 'sentry-fixture/organizationIntegrations';
3+
4+
import {render, screen} from 'sentry-test/reactTestingLibrary';
5+
6+
import type {IssueAlertNotificationProps} from 'sentry/views/projectInstall/issueAlertNotificationOptions';
7+
8+
import {ScmMessagingIntegrationAlertRule} from './scmMessagingIntegrationAlertRule';
9+
10+
describe('ScmMessagingIntegrationAlertRule', () => {
11+
const organization = OrganizationFixture();
12+
const slackIntegrations = [
13+
OrganizationIntegrationsFixture({name: "Moo Deng's Workspace"}),
14+
];
15+
16+
const notificationProps: IssueAlertNotificationProps = {
17+
actions: [],
18+
channel: {label: 'channel', value: 'channel'},
19+
integration: slackIntegrations[0],
20+
provider: 'slack',
21+
providersToIntegrations: {slack: slackIntegrations},
22+
querySuccess: true,
23+
shouldRenderSetupButton: false,
24+
setActions: jest.fn(),
25+
setChannel: jest.fn(),
26+
setIntegration: jest.fn(),
27+
setProvider: jest.fn(),
28+
};
29+
30+
beforeEach(() => {
31+
MockApiClient.addMockResponse({
32+
url: `/organizations/${organization.slug}/integrations/${slackIntegrations[0]!.id}/channels/`,
33+
body: {results: []},
34+
});
35+
});
36+
37+
it('renders the reused provider sentence with its three controls', () => {
38+
render(<ScmMessagingIntegrationAlertRule {...notificationProps} />, {organization});
39+
40+
// The same three controls as the classic rule render.
41+
expect(screen.getByLabelText('provider')).toBeInTheDocument();
42+
expect(screen.getByLabelText('integration')).toBeInTheDocument();
43+
expect(screen.getByLabelText('channel')).toBeInTheDocument();
44+
45+
// The provider sentence text is reused verbatim. The fragments are the
46+
// column's direct text nodes, so they read as one normalized string
47+
// ("Send [provider] notification to the [integration] workspace to [channel]").
48+
expect(screen.getByText('Send notification to the workspace to')).toBeInTheDocument();
49+
});
50+
});
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
import styled from '@emotion/styled';
2+
3+
import {Flex} from '@sentry/scraps/layout';
4+
import {Select} from '@sentry/scraps/select';
5+
6+
import {t} from 'sentry/locale';
7+
import {
8+
type IssueAlertNotificationProps,
9+
providerDetails,
10+
} from 'sentry/views/projectInstall/issueAlertNotificationOptions';
11+
import {
12+
ChannelField,
13+
ChannelSelect,
14+
useMessagingIntegrationAlertRule,
15+
} from 'sentry/views/projectInstall/messagingIntegrationAlertRule';
16+
17+
/**
18+
* SCM-styled variant of `MessagingIntegrationAlertRule`. Instead of the classic
19+
* inline sentence inside a grey card, the provider sentence is rendered into a
20+
* flex column: each input is its own flex item and each contiguous run of text
21+
* becomes an anonymous flex item, so every text block and input lands on its
22+
* own full-width row in `makeSentence`'s order. Letting the column do the
23+
* splitting (rather than parsing the sentence) keeps it correct no matter how a
24+
* translation reorders the placeholders.
25+
*
26+
* Reuses `useMessagingIntegrationAlertRule` (the channel query, validation, and
27+
* option lists) and the same provider sentence as the classic layout, so copy
28+
* and behavior stay in lockstep; only the presentation differs.
29+
*/
30+
export function ScmMessagingIntegrationAlertRule(props: IssueAlertNotificationProps) {
31+
const {
32+
provider,
33+
integration,
34+
channel,
35+
providerOptions,
36+
integrationOptions,
37+
channelOptions,
38+
isChannelLoading,
39+
channelError,
40+
providerDisabled,
41+
integrationDisabled,
42+
onProviderChange,
43+
onIntegrationChange,
44+
onChannelChange,
45+
onCreateChannel,
46+
} = useMessagingIntegrationAlertRule(props);
47+
48+
if (!provider) {
49+
return null;
50+
}
51+
52+
return (
53+
<Flex direction="column" gap="md">
54+
{providerDetails[provider as keyof typeof providerDetails]?.makeSentence({
55+
providerName: (
56+
<FullWidthSelect
57+
aria-label={t('provider')}
58+
disabled={providerDisabled}
59+
value={provider}
60+
options={providerOptions}
61+
onChange={onProviderChange}
62+
/>
63+
),
64+
integrationName: (
65+
<FullWidthSelect
66+
aria-label={t('integration')}
67+
disabled={integrationDisabled}
68+
value={integration}
69+
options={integrationOptions}
70+
onChange={onIntegrationChange}
71+
/>
72+
),
73+
target: (
74+
// flexibleControlStateSize collapses the (empty) control-state column
75+
// that otherwise reserves a fixed 24px, so the full-width select can
76+
// fill the row.
77+
<ChannelField
78+
name="channel"
79+
error={channelError}
80+
inline={false}
81+
flexibleControlStateSize
82+
>
83+
{() => (
84+
<FullWidthChannelSelect
85+
provider={provider}
86+
options={channelOptions}
87+
value={channel}
88+
isLoading={isChannelLoading}
89+
disabled={!integration}
90+
onChange={onChannelChange}
91+
onCreateOption={onCreateChannel}
92+
/>
93+
)}
94+
</ChannelField>
95+
),
96+
})}
97+
</Flex>
98+
);
99+
}
100+
101+
const FullWidthSelect = styled(Select)`
102+
width: 100%;
103+
`;
104+
105+
const FullWidthChannelSelect = styled(ChannelSelect)`
106+
width: 100%;
107+
`;

0 commit comments

Comments
 (0)