[Alerting UI] Don't wait for health check before showing Create Alert flyout #80996

ymao1 · 2020-10-19T14:34:46Z

Resolves #77472

Summary

Added optional input prop waitForCheck to HealthCheck component to determine if it should immediately render its children or wait for the health check to complete. I thought it only made sense to use the waitForCheck override in the AlertAdd component, but I can update the other components (AlertEdit, AlertList, ActionConnectorList) if we feel like it also makes sense there.

Created a HealthContextProvider to hold the state of whether or not the health check has returned. This allows the AlertAdd component to disable the 'Save' button until the health check has completed.

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

… check

…ing/slow-alert-flyout

elasticmachine · 2020-10-19T22:53:22Z

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

sorenlouv

This is awesome. Thank you so much!

YulNaumenko

Changes LGTM! Tested it locally with adding delay to the health API route and it works as expected - flyout is loaded immediately and Save button is disabled till health check is completed.
I have a couple of nits, but it shouldn't be a blocker. Great work!

x-pack/plugins/triggers_actions_ui/public/application/components/health_check.tsx

YulNaumenko · 2020-10-20T23:45:51Z

x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.tsx

@@ -149,6 +149,10 @@ export const AlertForm = ({
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, []);

+  useEffect(() => {


Is this changes related to the current issue? Did you want to fix something by adding it?
I suppose it can replace the default state from this line

This change was to handle a pre-selected alert being passed into the alert form (as in the case of uptime alerts). Previously, there was a delay in rendering the alert form because the health check loading indicator rendered first (however briefly). That allows the pre-selected alert to render when the alert form was first rendered. With the change to show the alert form immediately in the add alert flyout, this useEffect is needed to re-render the form to show the selected alert type.

ymao1 · 2020-10-21T12:04:03Z

@elasticmachine merge upstream

ymao1 · 2020-10-26T12:51:43Z

@elasticmachine merge upstream

mikecote

Changes LGTM, great work!

Regarding the waitForCheck in other areas, I think I'm good waiting and seeing if they're needed. I have a feeling there's some UX scenarios to think through (Ex: save button disabled in edit form until request completes, TLS banner displaying after a few seconds, etc).

x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_add_footer.tsx

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>

kibanamachine · 2020-10-26T17:18:47Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: aaf710d

Metrics [docs]

@kbn/optimizer bundle module count

id	before	after	diff
`triggersActionsUi`	284	286	+2

async chunks size

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

History

💔 Build #83678 failed 0c2412a
💚 Build #83627 succeeded c9701ea
💚 Build #83049 succeeded 8092495
💚 Build #82972 succeeded f483526
💚 Build #82601 succeeded 821e050

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

… flyout (elastic#80996) * wip * Adding health context provider and option to block waiting for health check * Adding tests * Removing forced lag * Fixing action form not rendering pre selected action * PR fixes * Updating i18n ids Co-authored-by: Mike Côté <mikecote@users.noreply.github.com> * Applying i18n fix Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>

… flyout (#80996) (#81665) * wip * Adding health context provider and option to block waiting for health check * Adding tests * Removing forced lag * Fixing action form not rendering pre selected action * PR fixes * Updating i18n ids Co-authored-by: Mike Côté <mikecote@users.noreply.github.com> * Applying i18n fix Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Mike Côté <mikecote@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>

* master: (37 commits) [ILM] Migrate Warm phase to Form Lib (elastic#81323) [Security Solutions][Detection Engine] Fixes critical bug with error reporting that was doing a throw (elastic#81549) [Detection Rules] Add 7.10 rules (elastic#81676) [kbn/optimizer] ignore missing metrics when updating limits with --focus (elastic#81696) [SECURITY SOLUTIONS] Bugs overview page + investigate eql in timeline (elastic#81550) [Maps] fix unable to edit cluster vector styles styled by count when switching to super fine grid resolution (elastic#81525) Fixed migration issue for case specific actions, by extending email action migrator checks (elastic#81673) [CI] Preparation for APM tracking on CI (elastic#80399) [Home] Fixes Kibana app description order on home page and updates Canvas copy (elastic#80057) Make sure `to` is 'now' and not the same as `from` (elastic#81524) Nitpicking the 8.0 Breaking Change issue template (elastic#81678) [SECURITY_SOLUTION] Fix text on onboarding screen (elastic#81672) [data.search] Skip async search tests in build candidates and production builds (elastic#81547) Fix previousStartedAt by not changing when execution fails (elastic#81388) [Monitoring] Fix a couple of issues with the cpu usage alert (elastic#80737) Telemetry collection xpack to ts project references (elastic#81269) Elasticsearch: don't use url authentication for new client (elastic#81564) [App Search] Credentials: implement working flyout form (elastic#81541) Properly encode links to edit user page (elastic#81562) [Alerting UI] Don't wait for health check before showing Create Alert flyout (elastic#80996) ...

ymao1 added 6 commits October 16, 2020 13:22

wip

d17b38d

Adding health context provider and option to block waiting for health…

b6c4f38

… check

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

12ef7cf

…ing/slow-alert-flyout

Adding tests

c2b785d

Removing forced lag

9dff0db

Fixing action form not rendering pre selected action

821e050

ymao1 changed the title ~~Alerting/slow alert flyout~~ [Alerting UI] Don't wait for health check before showing Create Alert flyout Oct 19, 2020

ymao1 self-assigned this Oct 19, 2020

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 19, 2020

ymao1 marked this pull request as ready for review October 19, 2020 22:53

ymao1 requested a review from a team as a code owner October 19, 2020 22:53

ymao1 requested a review from sorenlouv October 19, 2020 22:55

sorenlouv approved these changes Oct 20, 2020

View reviewed changes

YulNaumenko approved these changes Oct 20, 2020

View reviewed changes

PR fixes

f483526

Merge branch 'master' into alerting/slow-alert-flyout

8092495

Merge branch 'master' into alerting/slow-alert-flyout

c9701ea

mikecote self-requested a review October 26, 2020 14:15

mikecote approved these changes Oct 26, 2020

View reviewed changes

x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_add_footer.tsx Outdated Show resolved Hide resolved

x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_add_footer.tsx Outdated Show resolved Hide resolved

ymao1 and others added 2 commits October 26, 2020 11:08

Updating i18n ids

0c2412a

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>

Applying i18n fix

aaf710d

ymao1 merged commit 7e34bf2 into elastic:master Oct 26, 2020

ymao1 mentioned this pull request Oct 26, 2020

[7.x] [Alerting UI] Don't wait for health check before showing Create Alert flyout (#80996) #81665

Merged

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

ymao1 deleted the alerting/slow-alert-flyout branch February 4, 2021 15:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Alerting UI] Don't wait for health check before showing Create Alert flyout #80996

[Alerting UI] Don't wait for health check before showing Create Alert flyout #80996

ymao1 commented Oct 19, 2020 •

edited

Loading

elasticmachine commented Oct 19, 2020

sorenlouv left a comment

YulNaumenko left a comment

YulNaumenko Oct 20, 2020

ymao1 Oct 21, 2020

ymao1 commented Oct 21, 2020

ymao1 commented Oct 26, 2020

mikecote left a comment

kibanamachine commented Oct 26, 2020

[Alerting UI] Don't wait for health check before showing Create Alert flyout #80996

[Alerting UI] Don't wait for health check before showing Create Alert flyout #80996

Conversation

ymao1 commented Oct 19, 2020 • edited Loading

Summary

Checklist

elasticmachine commented Oct 19, 2020

sorenlouv left a comment

Choose a reason for hiding this comment

YulNaumenko left a comment

Choose a reason for hiding this comment

YulNaumenko Oct 20, 2020

Choose a reason for hiding this comment

ymao1 Oct 21, 2020

Choose a reason for hiding this comment

ymao1 commented Oct 21, 2020

ymao1 commented Oct 26, 2020

mikecote left a comment

Choose a reason for hiding this comment

kibanamachine commented Oct 26, 2020

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

async chunks size

History

ymao1 commented Oct 19, 2020 •

edited

Loading