[Alerting] Prompt for confirmation when saving alert with no action #79892

ymao1 · 2020-10-07T18:45:27Z

Resolves #61522

Summary

When saving an alert, show confirmation modal if user is trying to save an alert with no actions. This will only occur for users with the ability to create actions. Users who don't have the ability to create actions will not be prompted.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

…re/save-alert-no-action

…ing/save-alert-no-action

elasticmachine · 2020-10-08T00:38:35Z

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

...ck/plugins/triggers_actions_ui/public/application/sections/alert_form/confirm_alert_save.tsx

x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alerts.ts

YulNaumenko

Code LGTM. I left a few comments about the e2e test, but the rest looks awesome and works as expected 👍 . I will rely on @gchaps decision about the text for the confirmation modal.

gmmorris

Works as expected so LGTM 👍
Worth considering the comments on the e2e tests but no blockers as far as I'm concerned. :)

...ck/plugins/triggers_actions_ui/public/application/sections/alert_form/confirm_alert_save.tsx

x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alerts.ts

gmmorris · 2020-10-08T08:41:27Z

x-pack/test/functional_with_es_ssl/apps/uptime/alert_flyout.ts


      it('can save alert', async () => {
        await alerts.clickSaveAlertButton();
+        await alerts.clickSaveAlertsConfirmButton();
        await pageObjects.common.closeToast();
      });



It might be worth pinging the Uptime team just so they know about the changes to their tests.
I don't think they will get pinged automatically because we own the test suite.

elasticmachine · 2020-10-08T11:39:04Z

Pinging @elastic/uptime (Team:uptime)

pmuellr · 2020-10-08T16:51:09Z

I noticed this oddity, when I did a save with the "index" prompter open, and then pressed the save button which was enabled at the time. I wasn't even trying it, I think I fat-fingered enter there or something :-)

Seems like this is unrelated to this PR, it feels like that "index" prompter should be dismissed upon attempted Save, or Save should be disabled until the "index" prompter is dismissed. I think we have some other issues regarding these "expression" prompters, specifically around when they are dismissed, perhaps such scenarios are already being addressed in the expression editor bits. I'd kinda hate to even make a z-order change, just for this, as it feels like that could be brittle over time.

before hitting save

after hitting save

pmuellr

LGTM, works as expected when creating a new alert.

I did notice this doesn't prompt when editing an existing alert. I think that's fine, but ... dunno. We're certainly looking at catching the "you didn't add any actions" upon create. And if for some reason, you had a reason to have an action-less alert, and got that prompter every time you saved it, it would be frustrating.

ymao1 · 2020-10-08T17:03:21Z

I did notice this doesn't prompt when editing an existing alert. I think that's fine, but ... dunno. We're certainly looking at catching the "you didn't add any actions" upon create. And if for some reason, you had a reason to have an action-less alert, and got that prompter every time you saved it, it would be frustrating.

Agreed. I thought about adding it to the Edit flyout as well but I wasn't sure what the expected behavior should be when editing.

ymao1 · 2020-10-08T17:04:06Z

I noticed this oddity, when I did a save with the "index" prompter open, and then pressed the save button which was enabled at the time. I wasn't even trying it, I think I fat-fingered enter there or something :-)

Should I make a separate ticket to address this?

YulNaumenko · 2020-10-08T19:33:39Z

LGTM, works as expected when creating a new alert.

I did notice this doesn't prompt when editing an existing alert. I think that's fine, but ... dunno. We're certainly looking at catching the "you didn't add any actions" upon create. And if for some reason, you had a reason to have an action-less alert, and got that prompter every time you saved it, it would be frustrating.

I don't think that we should be prompted again in Edit, because it is something with what user have already agreed on and saved before. So +1 to left it as it is now.

pmuellr · 2020-10-12T14:23:11Z

I noticed this oddity, when I did a save with the "index" prompter open, and then pressed the save button which was enabled at the time. I wasn't even trying it, I think I fat-fingered enter there or something :-)

Should I make a separate ticket to address this?

Ya, I think so.

ymao1 · 2020-10-12T14:29:03Z

@elasticmachine merge upstream

ymao1 · 2020-10-12T14:37:09Z

@elasticmachine merge upstream

kibanamachine · 2020-10-12T20:10:08Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 46ff3f8

Metrics [docs]

@kbn/optimizer bundle module count

id	before	after	diff
`triggersActionsUi`	283	284	+1

async chunks size

id	before	after	diff
`triggersActionsUi`	1.5MB	1.5MB	+1.7KB

History

💚 Build #81089 succeeded ab39c83
💚 Build #80705 succeeded ea7019c
💚 Build #80630 succeeded 7558d34
💔 Build #80607 failed 78bd6b2
💔 Build #80601 failed af2963f

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…lastic#79892) * Adding save confirmation for new alerts with no associated actions * Adding functional tests * Fixing uptime alert test * Fixing uptime alert test * PR fixes * Updating confirmation modal wording Co-authored-by: Kibana Machine <[email protected]>

…79892) (#80314) * Adding save confirmation for new alerts with no associated actions * Adding functional tests * Fixing uptime alert test * Fixing uptime alert test * PR fixes * Updating confirmation modal wording Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>

ymao1 added 7 commits October 7, 2020 08:33

Adding save confirmation for new alerts with no associated actions

b6d2637

Merge branch 'master' of https://github.com/elastic/kibana into featu…

3ad0964

…re/save-alert-no-action

Adding functional tests

c383970

Merge branch 'master' of https://github.com/elastic/kibana into featu…

f2dd930

…re/save-alert-no-action

Fixing uptime alert test

af2963f

Merge branch 'master' of https://github.com/elastic/kibana into alert…

78bd6b2

…ing/save-alert-no-action

Fixing uptime alert test

7558d34

ymao1 added Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.11.0 v8.0.0 labels Oct 8, 2020

ymao1 marked this pull request as ready for review October 8, 2020 00:38

ymao1 requested a review from a team as a code owner October 8, 2020 00:38

YulNaumenko reviewed Oct 8, 2020

View reviewed changes

...ck/plugins/triggers_actions_ui/public/application/sections/alert_form/confirm_alert_save.tsx Outdated Show resolved Hide resolved

YulNaumenko reviewed Oct 8, 2020

View reviewed changes

x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alerts.ts Outdated Show resolved Hide resolved

YulNaumenko reviewed Oct 8, 2020

View reviewed changes

x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alerts.ts Outdated Show resolved Hide resolved

YulNaumenko reviewed Oct 8, 2020

View reviewed changes

gmmorris approved these changes Oct 8, 2020

View reviewed changes

PR fixes

ea7019c

ymao1 added the Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability label Oct 8, 2020

ymao1 self-assigned this Oct 8, 2020

pmuellr reviewed Oct 8, 2020

View reviewed changes

YulNaumenko approved these changes Oct 8, 2020

View reviewed changes

kibanamachine and others added 2 commits October 12, 2020 10:37

Merge branch 'master' into alerting/save-alert-no-action

ab39c83

Updating confirmation modal wording

46ff3f8

ymao1 merged commit 7b00052 into elastic:master Oct 13, 2020

ymao1 mentioned this pull request Oct 13, 2020

[7.x] [Alerting] Prompt for confirmation when saving alert with no action (#79892) #80314

Merged

mikecote added release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes labels Dec 16, 2020

ymao1 deleted the alerting/save-alert-no-action branch February 4, 2021 15:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Alerting] Prompt for confirmation when saving alert with no action #79892

[Alerting] Prompt for confirmation when saving alert with no action #79892

ymao1 commented Oct 7, 2020

elasticmachine commented Oct 8, 2020

YulNaumenko left a comment

gmmorris left a comment

gmmorris Oct 8, 2020

elasticmachine commented Oct 8, 2020

pmuellr commented Oct 8, 2020

pmuellr left a comment

ymao1 commented Oct 8, 2020

ymao1 commented Oct 8, 2020

YulNaumenko commented Oct 8, 2020

pmuellr commented Oct 12, 2020

ymao1 commented Oct 12, 2020

ymao1 commented Oct 12, 2020

kibanamachine commented Oct 12, 2020

[Alerting] Prompt for confirmation when saving alert with no action #79892

[Alerting] Prompt for confirmation when saving alert with no action #79892

Conversation

ymao1 commented Oct 7, 2020

Summary

Checklist

elasticmachine commented Oct 8, 2020

YulNaumenko left a comment

Choose a reason for hiding this comment

gmmorris left a comment

Choose a reason for hiding this comment

gmmorris Oct 8, 2020

Choose a reason for hiding this comment

elasticmachine commented Oct 8, 2020

pmuellr commented Oct 8, 2020

pmuellr left a comment

Choose a reason for hiding this comment

ymao1 commented Oct 8, 2020

ymao1 commented Oct 8, 2020

YulNaumenko commented Oct 8, 2020

pmuellr commented Oct 12, 2020

ymao1 commented Oct 12, 2020

ymao1 commented Oct 12, 2020

kibanamachine commented Oct 12, 2020

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

async chunks size

History