Implement UI for Create Alert form #55232

YulNaumenko · 2020-01-17T20:49:14Z

Summary

Implement missing UI functionality for Create Alert form:

support for Action Groups - only one default action group is supported and is active by default
possibility to add connector from Create Alert form actions view
add variables to Slack and Email messages

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

~~- [ ] This was checked for cross-browser compatibility, including a check against IE11~~
~~- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support~~
~~- [ ] Documentation was added for features that require explanation or tutorials~~

Unit or functional tests were updated or added to match the most common scenarios
~~- [ ] This was checked for keyboard-only and screenreader accessibility~~

For maintainers

~~- [ ] This was checked for breaking API changes and was labeled appropriately~~
~~- [ ] This includes a feature addition or change that requires a release note and was labeled appropriately~~

elasticmachine · 2020-01-17T20:49:17Z

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

…m-ui-flyout # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.

…mponents, added some unit tests

…m-ui-flyout # Conflicts: # x-pack/legacy/plugins/triggers_actions_ui/np_ready/public/application/lib/alert_api.test.ts # x-pack/legacy/plugins/triggers_actions_ui/np_ready/public/application/sections/action_connector_form/action_connector_form.test.tsx # x-pack/legacy/plugins/triggers_actions_ui/np_ready/public/application/sections/action_connector_form/action_connector_form.tsx # x-pack/legacy/plugins/triggers_actions_ui/np_ready/public/application/sections/alert_add/alert_add.tsx # x-pack/legacy/plugins/triggers_actions_ui/np_ready/public/application/sections/alerts_list/components/alerts_list.tsx # x-pack/legacy/plugins/triggers_actions_ui/np_ready/public/types.ts # x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/connectors.ts

…m-ui-flyout # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.

…(temporary till design will be ready for support multiple groups)

…m-ui-flyout # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.

Some design fixes for alerting create UI

…trigger button

YulNaumenko · 2020-02-01T00:03:41Z

Overall looks like this is going in the right direction 👍

There are a couple of things I think we should do as part of this PR:

Just saw this is done in the other PR! Awesome :) We have some components (expression.tsx especially) that are many hundreds of lines long with multiple individual useState primitives and modifiers. This makes it hard to get a mental model of the different pieces of the puzzle, and this is going to make maintenance hard and error prone. Can we break this down into multiple smaller components? React makes this super easy, we should take advantage of that. :)

It seems all the Config and Param variabled in Actions are typed as any as we're extracting them out of Record<string,any>s. Seeing as these props are validatable and we have schema for them, we should type them to avoid mistakes and make it easier to visualise what data we have and how we can use it.

It doesn't seem we have many Functional tests around creating an alert (just one?). It feels like we should have more extensive testing of this as the kind of input users can make here can be varied.

I'd like to see these changes made before we merge but the rest looks good 👍

Done
Done
Partially done - planning to extend when index threshold alert type will be registered on the server side and watcher API will be replaced

…m-ui-flyout # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.

gmmorris · 2020-02-03T10:09:47Z

Sorry, approved too soon.
I tried the UI doing everything except for actually saving 🤦‍♂ which didn't work 😬

Saving doesn't work because we don't have a server API support fo Index Threshold alert type (which should come up with the other PR on which we are working with @pmuellr ) I don't think that it should be a blocker for merging current PR. The goal is to make UI available for other teams to start their work on Alert Type development.

Fair enough. If we 're waiting on functionality then yeah, definitely no need to hold off on merging, but we obviously need to make sure this is locked off if we can't get it done for 7.7 (just like the alerts details page :))

gmmorris

Now that I know it can't save by design - all good 👍

…m-ui-flyout # Conflicts: # x-pack/legacy/plugins/triggers_actions_ui/np_ready/public/application/sections/action_connector_form/action_connector_form.test.tsx # x-pack/legacy/plugins/triggers_actions_ui/np_ready/public/application/sections/action_connector_form/connector_add_flyout.test.tsx # x-pack/legacy/plugins/triggers_actions_ui/np_ready/public/application/sections/alerts_list/components/alerts_list.tsx # x-pack/legacy/plugins/triggers_actions_ui/np_ready/public/types.ts

...ins/triggers_actions_ui/np_ready/public/application/components/builtin_action_types/types.ts

pmuellr

I reviewed mainly the things specific to actions/alert data, and not so much the UI. LGTM - making some good forward progress. Made some notes about things that might need to be changed, but seems like non are critical, can be fixed later.

pmuellr · 2020-02-04T15:14:20Z

...ins/triggers_actions_ui/np_ready/public/application/components/builtin_action_types/types.ts

+  secrets: PagerDutySecrets;
+}
+
+interface SlackSecrets {


I'd rather see the action config/secrets/params all defined close to each other, probably in that order; and I guess the action connector defined after params. Keep all the action-specific stuff grouped together.

x-pack/legacy/plugins/triggers_actions_ui/np_ready/public/application/lib/alert_api.test.ts

...ions_ui/np_ready/public/application/sections/action_connector_form/action_connector_form.tsx

pmuellr · 2020-02-04T15:48:32Z

...ions_ui/np_ready/public/application/sections/action_connector_form/action_connector_form.tsx

+            <p>
+              <FormattedMessage
+                id="xpack.triggersActionsUI.sections.actionConnectorForm.actions.actionConfigurationWarningDescriptionText"
+                defaultMessage="To create this connector, you must configure at least one {actionType} account. {docLink}"


It seems like the case we run into here is that somehow we've got an action type which hasn't been registered. An example case would be if you created an action previously with an action defined in some plugin, and then later run Kibana WITHOUT that plugin available/enabled, and so the action type is no longer registered.

If so, then the message should be something like "the action type Foo is not a registered action type". I'm not sure what kind of remediation we'd want in the message, but could be something like "the action type is no longer available" with a pointer to some doc link describing the situation and remediation further.

x-pack/legacy/plugins/triggers_actions_ui/np_ready/public/types.ts

snide

LGTM. We might have more design PRs later, but they can be iterative.

kibanamachine · 2020-02-04T19:04:46Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 5ba502c

History

💚 Build #24230 succeeded 9e52760
💔 Build #24128 failed c0b3195
💔 Build #24098 failed 8d332db
💔 Build #24074 failed 21a0631
💔 Build #23929 failed 87d6a9b

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

* master: (23 commits) Properly handle password change for users authenticated with provider other than `basic`. (elastic#55206) Improve pull request template proposal (elastic#56756) Only change handlers as the element changes (elastic#56782) [SIEM][Detection Engine] Final final rule changes (elastic#56806) [SIEM][Detection Engine] critical blocker, wrong ilm policy, need to match beats ilm policy Move ui/agg_types in to shim data plugin (elastic#56353) [SIEM] Fixes Signals count spinner (elastic#56797) [docs] Update upgrade version path (elastic#56658) [Canvas] Use unique Id for Canvas Embeddables (elastic#56783) [Rollups] Adjust max width for job detail panel (elastic#56674) Prevent http client from converting our form data (elastic#56772) Disable creating alerts client instances when ESO plugin is using an ephemeral encryption key (elastic#56676) Bumps terser-webpack-plugin to 2.3.4 (elastic#56662) Advanced settings component registry ⇒ kibana platform plugin (elastic#55940) [Endpoint] EMT-67: add kql support for endpoint list (elastic#56328) Implement UI for Create Alert form (elastic#55232) Fix: Filter pill base coloring (elastic#56761) fix open close signal on detail page (elastic#56757) [Search service] Move loadingCount to sync search strategy (elastic#56335) Rollup TSVB integration: Add test and fix warning text (elastic#56639) ...

* Updated Alert ui model, fixed form validation and small issues * Fixed error messages and validation for action params forms * Fixed typecheck error * Moved alert add/edit common fields to alert form * Fixed type checks * Refactored alert add flyout by splitting it to the form and flyout components, added some unit tests * Refactored connector add/edit flyouts and created add connector modal * Refactored add/edit flyout tests * Fixed test * Removed orig files * Removed orig file * Added unit tests for add connector modal dialog * Action Groups idea of implementation * Removed action group tabs and set only first action group as default (temporary till design will be ready for support multiple groups) * Added missing unit tests * Changed design of the email params form * Fixed actions params forms according to latest mockups * Fixed options list for available actions connectors * Fixed modal dialog update on action delete * fixed build fail * Added functionality for action types with Message field to Add variables * Added alertReducer unit tests * Added create alert functional test * Added types for Params * Some design fixes * alerts empty prompt * Fixed failing app on save alert and added possibility to hide Change trigger button * Fixed type check issues * Added connector config types * fixed type check * Fixed merge issues * Fixed type checks * Fixed functional tests and error expression message * Fixed jest tests * Review changes Co-authored-by: dave.snider@gmail.com <dave.snider@gmail.com>

* Updated Alert ui model, fixed form validation and small issues * Fixed error messages and validation for action params forms * Fixed typecheck error * Moved alert add/edit common fields to alert form * Fixed type checks * Refactored alert add flyout by splitting it to the form and flyout components, added some unit tests * Refactored connector add/edit flyouts and created add connector modal * Refactored add/edit flyout tests * Fixed test * Removed orig files * Removed orig file * Added unit tests for add connector modal dialog * Action Groups idea of implementation * Removed action group tabs and set only first action group as default (temporary till design will be ready for support multiple groups) * Added missing unit tests * Changed design of the email params form * Fixed actions params forms according to latest mockups * Fixed options list for available actions connectors * Fixed modal dialog update on action delete * fixed build fail * Added functionality for action types with Message field to Add variables * Added alertReducer unit tests * Added create alert functional test * Added types for Params * Some design fixes * alerts empty prompt * Fixed failing app on save alert and added possibility to hide Change trigger button * Fixed type check issues * Added connector config types * fixed type check * Fixed merge issues * Fixed type checks * Fixed functional tests and error expression message * Fixed jest tests * Review changes Co-authored-by: dave.snider@gmail.com <dave.snider@gmail.com> Co-authored-by: dave.snider@gmail.com <dave.snider@gmail.com>

Updated Alert ui model, fixed form validation and small issues

7de96c7

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

YulNaumenko self-assigned this Jan 17, 2020

YulNaumenko added 22 commits January 17, 2020 15:54

Fixed error messages and validation for action params forms

1aade15

Fixed typecheck error

3c68e68

Moved alert add/edit common fields to alert form

9bab832

Fixed type checks

1c7ccfa

Refactored alert add flyout by splitting it to the form and flyout co…

a332b90

…mponents, added some unit tests

Refactored connector add/edit flyouts and created add connector modal

ab87477

Refactored add/edit flyout tests

d2d7aeb

Fixed test

a3b1e71

Removed orig files

b9c508a

Removed orig file

34cf05c

Added unit tests for add connector modal dialog

a619f76

Action Groups idea of implementation

9a27d24

Removed action group tabs and set only first action group as default …

69f9a46

…(temporary till design will be ready for support multiple groups)

Added missing unit tests

0a23205

Changed design of the email params form

552da20

Fixed actions params forms according to latest mockups

6c75b78

Fixed options list for available actions connectors

bbc87cc

YulNaumenko and others added 3 commits January 31, 2020 14:24

Merge pull request #1 from snide/design/alert-create

5fa78d6

Some design fixes for alerting create UI

Fixed failing app on save alert and added possibility to hide Change …

b99f14e

…trigger button

Fixed type check issues

9992f40

Added connector config types

42b41fe

YulNaumenko requested a review from gmmorris February 1, 2020 01:15

YulNaumenko added 2 commits February 2, 2020 20:35

fixed type check

87d6a9b

gmmorris approved these changes Feb 3, 2020

View reviewed changes

YulNaumenko added 5 commits February 3, 2020 08:48

Fixed merge issues

8d332db

Fixed type checks

c0b3195

Fixed functional tests and error expression message

5734040

Fixed jest tests

9e52760

pmuellr reviewed Feb 4, 2020

View reviewed changes

...ins/triggers_actions_ui/np_ready/public/application/components/builtin_action_types/types.ts Outdated Show resolved Hide resolved

pmuellr reviewed Feb 4, 2020

View reviewed changes

...ins/triggers_actions_ui/np_ready/public/application/components/builtin_action_types/types.ts Outdated Show resolved Hide resolved

pmuellr approved these changes Feb 4, 2020

View reviewed changes

snide approved these changes Feb 4, 2020

View reviewed changes

Review changes

5ba502c

YulNaumenko merged commit b458b3b into elastic:master Feb 4, 2020

YulNaumenko mentioned this pull request Feb 6, 2020

[7.x] Implement UI for Create Alert form (#55232) #57000

Merged

YulNaumenko added the backported label Feb 18, 2020

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement UI for Create Alert form #55232

Implement UI for Create Alert form #55232

YulNaumenko commented Jan 17, 2020 •

edited

Loading

elasticmachine commented Jan 17, 2020

YulNaumenko commented Feb 1, 2020

gmmorris commented Feb 3, 2020

gmmorris left a comment

pmuellr left a comment

pmuellr Feb 4, 2020

pmuellr Feb 4, 2020

snide left a comment

kibanamachine commented Feb 4, 2020

Implement UI for Create Alert form #55232

Implement UI for Create Alert form #55232

Conversation

YulNaumenko commented Jan 17, 2020 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Jan 17, 2020

YulNaumenko commented Feb 1, 2020

gmmorris commented Feb 3, 2020

gmmorris left a comment

Choose a reason for hiding this comment

pmuellr left a comment

Choose a reason for hiding this comment

pmuellr Feb 4, 2020

Choose a reason for hiding this comment

pmuellr Feb 4, 2020

Choose a reason for hiding this comment

snide left a comment

Choose a reason for hiding this comment

kibanamachine commented Feb 4, 2020

💚 Build Succeeded

History

YulNaumenko commented Jan 17, 2020 •

edited

Loading