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

[RAM] [Rule Form V2] Add full-page create/edit form behind feature flag to Stack Management #180539

Open
wants to merge 73 commits into
base: main
Choose a base branch
from

Conversation

Zacqary
Copy link
Contributor

@Zacqary Zacqary commented Apr 10, 2024

Summary

Closes #179105

Rule Form V2 is now accessible from Stack Management by adding this to kibana.yml:

xpack.trigger_actions_ui.enableExperimental: ['ruleFormV2']

Actions cannot yet be configured. These will be added in #179106 and #179107

Screenshot 2024-04-10 at 3 29 36 PM

Create and Edit

This full page form works both for creating new rules and editing existing ones. It can be accessed using the existing UI, or using the new routes rule/create/:ruleTypeId and rule/edit/:ruleId.

Expression validation

Error messages from the expression component affect the overall form validation.
Screenshot 2024-04-10 at 3 31 51 PM
Screenshot 2024-04-10 at 3 31 42 PM

Known broken rule types

V2 contains breaking changes for the following rule type expression components:

  • Log threshold
  • Inventory threshold
  • Uptime monitor status
  • Synthetics TLS certificate

These should be fairly simple errors to fix, and we can include them in a further PR where we add this rule form to Observability.

Checklist

Delete any items that are not applicable to this PR.

@Zacqary Zacqary added Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Alerting/RulesManagement Issues related to the Rules Management UX v8.14.0 labels Apr 10, 2024
@Zacqary Zacqary requested a review from a team as a code owner April 10, 2024 20:49
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@Zacqary
Copy link
Contributor Author

Zacqary commented Apr 17, 2024

@JiaweiWu Re: field level validation

fieldvalidation.mov

It still happens, it just doesn't turn red and throw a warning at you unless you actually close the field without finishing it.

@Zacqary
Copy link
Contributor Author

Zacqary commented Apr 17, 2024

For the Elasticsearch query rule type, it seems like we keep the validation around when selection which query to use, which seems incorrect.

Fixed. Using the new IncompleteError rule types for missing information, the validation errors no longer harass the user.

Also when i type in -34 in the rule schedule it goes into an unrecoverable state. Seems to work as intended in the rule flyout

There was an uncecessary regex in the duration parsing functions I ported over from triggersActionsUI. Removing this fixes the issue.

For the existing rule flyout, we default alert delay to 1, but the new form we default it to empty, not sure if this matters or not.

It's not an issue, the alert delay isn't a required value, though I can fix this for a cleaner UX

@Zacqary
Copy link
Contributor Author

Zacqary commented Apr 17, 2024

I noticed we have this new show api request button, do we want to port this over too?

Ported over in latest commit

@Zacqary Zacqary requested a review from JiaweiWu April 17, 2024 20:30
@Zacqary
Copy link
Contributor Author

Zacqary commented Apr 18, 2024

@elasticmachine merge upstream

@Zacqary
Copy link
Contributor Author

Zacqary commented Apr 23, 2024

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
alerting 175 231 +56
observability 510 566 +56
securitySolution 5452 5508 +56
stackAlerts 145 242 +97
triggersActionsUi 733 792 +59
total +324

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/alerts-ui-shared 98 122 +24

Async chunks

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

id before after diff
alerting 91.5KB 95.7KB +4.1KB
observability 287.3KB 291.5KB +4.2KB
securitySolution 14.6MB 14.7MB +4.3KB
stackAlerts 82.5KB 72.2KB -10.3KB
triggersActionsUi 1.6MB 1.6MB +3.0KB
total +5.2KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
@kbn/alerts-ui-shared 2 3 +1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
alerting 23.6KB 23.8KB +112.0B
observability 150.7KB 150.9KB +269.0B
stackAlerts 24.7KB 40.0KB +15.3KB
triggersActionsUi 120.7KB 153.1KB +32.4KB
total +48.1KB
Unknown metric groups

API count

id before after diff
@kbn/alerts-ui-shared 110 134 +24

ESLint disabled line counts

id before after diff
@kbn/alerts-ui-shared 6 9 +3

References to deprecated APIs

id before after diff
infra 4 28 +24
ml 27 28 +1
observability 0 13 +13
stackAlerts 17 30 +13
stackConnectors 0 5 +5
total +56

Total ESLint disabled count

id before after diff
@kbn/alerts-ui-shared 6 9 +3

History

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

JiaweiWu added a commit that referenced this pull request May 31, 2024
## Summary
Issue: #179105
Related PR: #180539

Part 1 of 3 PRs of new rule form. This PR extracts the first section of
the rule form, the rule definition, from the original PR. The purpose is
to fix a few bugs (Such as improving the alert delay and the rule
schedule input validation), and also try to make the PR much smaller for
review. The design philosophy in the PR is to create components that are
devoid of any fetching or form logic. These are simply dumb components.

I have also created a example plugin to demonstrate this PR. To access: 

1. Run the branch with `yarn start --run-examples`
2. Navigate to
`http://localhost:5601/app/triggersActionsUiExample/rule_definition`

And you should be able to play around with the components in this PR:

<img width="1257" alt="Screenshot 2024-05-13 at 10 10 51 AM"
src="https://github.com/elastic/kibana/assets/74562234/a1ab6d96-946d-4bf6-94e2-6aa903d0b8f5">

### Checklist
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Zacqary <zacqary.xeper@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
JiaweiWu added a commit that referenced this pull request Jun 5, 2024
## Summary
Issue: #179105
Related PR: #180539

Part 1: #183325

Part 2 of 3 PRs of new rule form. This PR depends on the code from part
1, so only merge this when part 1 has been merged. This PR extracts the
last section of the rule form, the rule details, from the original PR.
The design philosophy in the PR is to create components that are devoid
of any fetching or form logic. These are simply dumb components.

I have also created a example plugin to demonstrate this PR. To access:

1. Run the branch with yarn start --run-examples
2. Navigate to
http://localhost:5601/app/triggersActionsUiExample/rule_details

And you should be able to play around with the components in this PR:

<img width="1281" alt="Screenshot 2024-05-13 at 9 44 14 PM"
src="https://github.com/elastic/kibana/assets/74562234/7ca900e3-ca9a-4810-8b24-7c3ea41055d6">

### Checklist
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------
Co-authored-by: Zacqary <zacqary.xeper@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Alerting/RulesManagement Issues related to the Rules Management UX release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RAM] [Rule Form V2] Add full-page create/edit form
7 participants