Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.x] [Alerting] fixes buggy default message behaviour (#84202) #84553

Merged
merged 5 commits into from
Dec 1, 2020

Conversation

gmmorris
Copy link
Contributor

Backports the following commits to 7.x:

This PR addresses some weird UX we've identified with default values in Action Params components and their inferred defaults when placed inside of an Alerts flyout.
Key changes:

1. Typing of these components has been corrected to reflect that we expect these parameters to only be _partial_, as the form is used to set these values (for example, the `message` field of the Server Log action, might or might not be set, so it should be nullable, but in the typing we treated it as the _final_ valid state, which is message not being nullable).
2. When a default message is set by the params components, the are tracked against the value of the default, which means that if the default changes, then so will the value in the field. Custom values provided by the user will not be overridden when the default changes. This has to be handled by the component itself at the moment (hopefully in the future we can make this a concern of the flyout and not each component).
3. The concept of the "Recovered" action group has been removed from these components - that's an Alerting concern, not actions, and shouldn't appear in the action components' code.
# Conflicts:
#	x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.test.tsx
@gmmorris
Copy link
Contributor Author

gmmorris commented Dec 1, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
triggersActionsUi 1.5MB 1.5MB +3.3KB

History

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

@gmmorris gmmorris merged commit a260c44 into elastic:7.x Dec 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants