feat(project-creation): Notification options in the SCM alert section#118394
feat(project-creation): Notification options in the SCM alert section#118394jaydgoss wants to merge 6 commits into
Conversation
github-actions
Bot
added
the
Scope: Frontend
|
@jaydgoss manually test project creation locally with mock messaging integrations, test normal create, create then edit. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d196660. Configure here.
| !missingFields.projectName && | ||
| !missingFields.team && | ||
| !missingFields.platform && | ||
| !missingFields.notificationChannel && |
There was a problem hiding this comment.
Reuse path skips notification rules
High Severity
The unchanged back-navigation reuse check compares project name, team, and alert fields but not persisted notificationActions. After a project exists, adding or changing a messaging integration and submitting can take the reuse shortcut, so createNotificationAction never runs and the integration alert rule is not created.
Reviewed by Cursor Bugbot for commit d196660. Configure here.
| } | ||
| if (notificationChannel) { | ||
| return t('Please provide an integration channel for alert notifications'); | ||
| } |
There was a problem hiding this comment.
Tooltip omits channel from count
Medium Severity
getSubmitTooltipText counts only platform, projectName, and team when deciding whether several fields are missing. A missing integration channel together with another required field can show the wrong single-field message instead of the channel prompt or the generic “fill all fields” text.
Reviewed by Cursor Bugbot for commit d196660. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| case 'discord': | ||
| return { | ||
| id: IssueAlertActionType.DISCORD, | ||
| server: integration?.id, | ||
| channel_id: channel?.value, | ||
| }; |
There was a problem hiding this comment.
Bug: The logic to restore a notification channel checks for a channel property, but Discord actions use channel_id, causing the selected channel not to be restored upon navigation.
Severity: HIGH
Suggested Fix
Update the useEffect in useCreateNotificationAction to also check for firstAction.channel_id. If channel_id is present, use it to find the correct channel object from the list of available channels and call setChannel to restore the selection in the UI.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: static/app/views/projectInstall/issueAlertNotificationOptions.tsx#L110-L115
Potential issue: When a Discord integration channel is configured, the action is stored
with a `channel_id` property. However, the `useEffect` in the
`useCreateNotificationAction` hook, which is responsible for re-populating the form
state, only checks for a `channel` property on the action object. Since Discord actions
lack a `channel` property, the check `if (firstAction.channel)` evaluates to false.
Consequently, `setChannel` is never called for Discord integrations when the component
re-renders, such as when a user navigates away and then returns. This causes the channel
selection to appear blank, blocking project creation until the user manually re-selects
the channel.
Did we get this right? 👍 / 👎 to inform future reviews.
…ction Render IssueAlertNotificationOptions (the messaging-integration notification picker) in ScmAlertFrequencySection, gated on the alert setting, and wire the real createNotificationAction through useScmProjectDetails so the chosen notification rule is created on project creation (it was previously a no-op). Reuses the classic component as-is.
When 'Notify via integration' is selected, block project creation until a channel is chosen, mirroring the classic flow's active gate (its useValidateChannel is wired with enabled:false, so live channel validation is off there too; the real gate is channel-required). Surfaces the reason in the create tooltip.
… nav The messaging-integration selection lived only in useCreateNotificationAction's local state, so it reset when the onboarding step remounted on navigation (and was dropped from the createProject return). Persist it in projectDetailsForm (seeding the hook from it and carrying it in the submitted form), the same way alertRuleConfig already persists. Extract getMessagingIntegrationAction so the selection can be serialized; persistence is driven by user setter calls so re-seeding can't loop. Known limitation: when several messaging providers are connected, the hook re-selects the first available on load, so a non-first provider is not faithfully restored (the channel is).
…options The 'You can always change alerts after project creation' note moves from ScmAlertFrequency into ScmAlertFrequencySection, so it renders below both the alert cards and the notification options instead of between them.
Hoist the 'You can always change alerts after project creation' note into a single element reused by the collapsible and onboarding branches, matching how the notification options are shared.
…n across nav" Reverts 99112b1. The wrapped-setter persistence clobbered provider selection: the provider dropdown's onChange fires setProvider/setIntegration/setChannel together, and each wrapped setter rebuilt the persisted action from stale state, so the last write reverted the provider on re-seed. Cross-provider restore was also unreliable (the re-seed reads the Slack-only channel field, not Discord's channel_id). Persisting the uncontrolled useCreateNotificationAction state via projectDetailsForm is too fragile for the narrow back-nav-on-active-project scenario it served. The notification options, channel-required gate, and core wiring remain.
jaydgoss
force-pushed
the
jaygoss/vdy-77-scm-alert-frequency-polish
branch
from
aba9239 to
6e8ef78
Compare
jaydgoss
force-pushed
the
jaygoss/scm-alert-notification-options
branch
from
4534ccd to
ca9b1b8
Compare
1 participant
TLDR
Brings the messaging-integration notification options into the SCM alert-frequency section, makes the chosen notification rule actually get created on submit (it was stubbed as a no-op), requires a channel before submit, and persists the selection across navigation.
Details
IssueAlertNotificationOptions(reused as-is, restyle to follow) inScmAlertFrequencySection, gated onalertSetting !== CREATE_ALERT_LATER, in both the collapsible project-creation and always-expanded onboarding layouts.useScmProjectDetailsnow callsuseCreateNotificationAction, returnsnotificationProps, and passes the realcreateNotificationActioninto project creation (replacing the no-op), so a chosen Slack/Discord/MS Teams rule is created with the project.useValidateChannelis wiredenabled: false, so live channel validation is off there too, this is the real gate.projectDetailsForm(seeds the hook from it and carries it in the submitted form) so it survives navigation, the wayalertRuleConfigalready does. ExtractedgetMessagingIntegrationActionto serialize the selection.Notes