Skip to content

feat(aci): Require at least one connected monitor in the alert builder#113246

Merged
malwilley merged 1 commit intomasterfrom
malwilley/alert-require-detectors
Apr 17, 2026
Merged

feat(aci): Require at least one connected monitor in the alert builder#113246
malwilley merged 1 commit intomasterfrom
malwilley/alert-require-detectors

Conversation

@malwilley
Copy link
Copy Markdown
Member

@malwilley malwilley commented Apr 16, 2026

Closes ISWF-2235

  • Adds validation to prevent saving alerts without a connected project/monitor
  • When creating a new alert, automatically selects a project from your page filters (or if "my projects" is selected, chooses one of the projects you belong to)
CleanShot.2026-04-16.at.15.16.00.mp4

@malwilley malwilley requested a review from a team as a code owner April 16, 2026 22:15
@linear-code
Copy link
Copy Markdown

linear-code bot commented Apr 16, 2026

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Apr 16, 2026
Copy link
Copy Markdown
Contributor

@saponifi3d saponifi3d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, just a few small things / nits.

function EditConnectedMonitorsContent({
initialMode,
connectedIds,
setConnectedIds,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not related to this pr and a bit of a nit.. but might be nice to get this method from context or something so it's clearer where the connected id's are being stored.

});
});

it('shows validation error when submitting with no projects or monitors selected', async () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we also add that it's cleared if something has been added?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes good catch!

}

// If any specific projects are selected, use the first one
const intitialSelectedProject = selection.projects.find(p => p > 0);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const intitialSelectedProject = selection.projects.find(p => p > 0);
const initialSelectedProject = selection.projects.find(p => p > 0);

^ typo in initial

async (data, onSubmitSuccess, onSubmitError, _event, formModel) => {
const errors = validateAutomationBuilderState(state);
const errors = validateAutomationBuilderState(state, data as AutomationFormData, {
validateConnectedMonitors: false,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be nice to have a comment as to why we don't validate in the drawer; (seems like because we're enforcing on to be there, but it's a little unclear why we don't want to validate that)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, will add. We don't validate it in the drawer because we actually do want to create it without any connections (because when creating inline for a new monitor, the monitor hasn't been created yet)

@malwilley malwilley merged commit e36d84d into master Apr 17, 2026
68 checks passed
@malwilley malwilley deleted the malwilley/alert-require-detectors branch April 17, 2026 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants