Skip to content

[ResponseOps][Alerting] Error when submit rule form when using AddFilterPopover in actions#194600

Merged
guskovaue merged 9 commits intoelastic:mainfrom
guskovaue:MX-192847-error-when-submit-rule-form-with-add-filetr-popover
Oct 7, 2024
Merged

[ResponseOps][Alerting] Error when submit rule form when using AddFilterPopover in actions#194600
guskovaue merged 9 commits intoelastic:mainfrom
guskovaue:MX-192847-error-when-submit-rule-form-with-add-filetr-popover

Conversation

@guskovaue
Copy link
Copy Markdown
Contributor

@guskovaue guskovaue commented Oct 1, 2024

Resolve: #192847

When user try to save the rule which has a conditional action with a filter which contains AND or OR, it'll fail.
Error raises when a new rule SO object is going to be created. Validation fails because schema is wrong.
I fixed it in this PR.

Checklist

How to test

  1. Go to Rules and try creating a new rule of any type
  2. Add an action to the rule
  3. Check the option If alert matches a query
  4. Click the + icon to add a filter
  5. Create a filter in the popover
  6. Click AND or OR
  7. Create another filter
  8. Click Add filter
  9. Try saving the rule
  10. Saving should be successful

@guskovaue guskovaue added bug Fixes for quality problems that affect the customer experience backport This PR is a backport of another PR Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// v9.0.0 backport:prev-minor v8.16.0 labels Oct 1, 2024
@guskovaue guskovaue self-assigned this Oct 1, 2024
@guskovaue guskovaue requested a review from a team as a code owner October 1, 2024 14:51
@elasticmachine
Copy link
Copy Markdown
Contributor

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

@guskovaue guskovaue marked this pull request as draft October 1, 2024 14:52
@guskovaue guskovaue marked this pull request as ready for review October 2, 2024 12:17
@guskovaue guskovaue changed the title [MX] Error when submit rule form when using AddFilterPopover in actions [ResponseOps][Alerting] Error when submit rule form when using AddFilterPopover in actions Oct 2, 2024
type: schema.maybe(schema.string()),
key: schema.maybe(schema.string()),
params: schema.maybe(schema.recordOf(schema.string(), schema.any())), // better type?
params: schema.maybe(schema.any()),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this one, shouldn't we replicate the type instead of opening up?

Copy link
Copy Markdown
Contributor Author

@guskovaue guskovaue Oct 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cnasikas We talked about it. But you see, there is the type in kbn-es-query package. Maybe we can use it for creating params schema? But it looks huge and can change with time, so we'll not know. Maybe we can create some sort of test, which will correlate between type and schema?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, we should not persist with the filters as the schema is only related to the UI and is not needed for the backend or a consumer of the API. I am not sure we can efficiently catch up with all variants of the params schema. An alternative would be to do schema.maybe(schema.oneOf([schema.recordOf(schema.string(), schema.any()), schema.arrayOf(schema.recordOf(schema.string(), schema.any())])) but maybe it is not worth the effort. What do you both think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sound reasonable!

Copy link
Copy Markdown
Member

@umbopepato umbopepato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, works as expected ✅
LGTM!

image image

@kibanamachine
Copy link
Copy Markdown
Contributor

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#7086

[✅] x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/config.ts: 100/100 tests passed.

see run history

@cnasikas cnasikas requested a review from jcger October 7, 2024 07:34
@kibana-ci
Copy link
Copy Markdown

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Page load bundle

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

id before after diff
triggersActionsUi 123.3KB 123.3KB +19.0B

History

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

cc @guskovaue

@guskovaue guskovaue merged commit 02d0c98 into elastic:main Oct 7, 2024
@kibanamachine
Copy link
Copy Markdown
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11214470836

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 7, 2024
…terPopover in actions (elastic#194600)

Resolve: elastic#192847

When user try to save the rule which has a conditional action with a
filter which contains AND or OR, it'll fail.
Error raises when a new rule SO object is going to be created.
Validation fails because schema is wrong.
I fixed it in this PR.

### 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

### How to test

1. Go to Rules and try creating a new rule of any type
2. Add an action to the rule
3. Check the option If alert matches a query
4. Click the + icon to add a filter
5. Create a filter in the popover
6. Click AND or OR
7. Create another filter
8. Click Add filter
9. Try saving the rule
10. Saving should be successful

(cherry picked from commit 02d0c98)
@kibanamachine
Copy link
Copy Markdown
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Oct 7, 2024
…AddFilterPopover in actions (#194600) (#195228)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[ResponseOps][Alerting] Error when submit rule form when using
AddFilterPopover in actions
(#194600)](#194600)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"Julia","email":"iuliia.guskova@elastic.co"},"sourceCommit":{"committedDate":"2024-10-07T11:23:25Z","message":"[ResponseOps][Alerting]
Error when submit rule form when using AddFilterPopover in actions
(#194600)\n\nResolve:
https://github.com/elastic/kibana/issues/192847\r\n\r\nWhen user try to
save the rule which has a conditional action with a\r\nfilter which
contains AND or OR, it'll fail.\r\nError raises when a new rule SO
object is going to be created.\r\nValidation fails because schema is
wrong.\r\nI fixed it in this PR.\r\n\r\n\r\n### Checklist\r\n\r\n- [x]
[Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n\r\n### How
to test\r\n\r\n1. Go to Rules and try creating a new rule of any
type\r\n2. Add an action to the rule\r\n3. Check the option If alert
matches a query\r\n4. Click the + icon to add a filter\r\n5. Create a
filter in the popover\r\n6. Click AND or OR\r\n7. Create another
filter\r\n8. Click Add filter\r\n9. Try saving the rule\r\n10. Saving
should be
successful","sha":"02d0c9852f0efb15b67c06e240e1525220a701ec","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","backport","Feature:Alerting","release_note:skip","Team:ResponseOps","v9.0.0","backport:prev-minor","v8.16.0"],"title":"[ResponseOps][Alerting]
Error when submit rule form when using AddFilterPopover in
actions","number":194600,"url":"https://github.com/elastic/kibana/pull/194600","mergeCommit":{"message":"[ResponseOps][Alerting]
Error when submit rule form when using AddFilterPopover in actions
(#194600)\n\nResolve:
https://github.com/elastic/kibana/issues/192847\r\n\r\nWhen user try to
save the rule which has a conditional action with a\r\nfilter which
contains AND or OR, it'll fail.\r\nError raises when a new rule SO
object is going to be created.\r\nValidation fails because schema is
wrong.\r\nI fixed it in this PR.\r\n\r\n\r\n### Checklist\r\n\r\n- [x]
[Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n\r\n### How
to test\r\n\r\n1. Go to Rules and try creating a new rule of any
type\r\n2. Add an action to the rule\r\n3. Check the option If alert
matches a query\r\n4. Click the + icon to add a filter\r\n5. Create a
filter in the popover\r\n6. Click AND or OR\r\n7. Create another
filter\r\n8. Click Add filter\r\n9. Try saving the rule\r\n10. Saving
should be
successful","sha":"02d0c9852f0efb15b67c06e240e1525220a701ec"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/194600","number":194600,"mergeCommit":{"message":"[ResponseOps][Alerting]
Error when submit rule form when using AddFilterPopover in actions
(#194600)\n\nResolve:
https://github.com/elastic/kibana/issues/192847\r\n\r\nWhen user try to
save the rule which has a conditional action with a\r\nfilter which
contains AND or OR, it'll fail.\r\nError raises when a new rule SO
object is going to be created.\r\nValidation fails because schema is
wrong.\r\nI fixed it in this PR.\r\n\r\n\r\n### Checklist\r\n\r\n- [x]
[Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n\r\n### How
to test\r\n\r\n1. Go to Rules and try creating a new rule of any
type\r\n2. Add an action to the rule\r\n3. Check the option If alert
matches a query\r\n4. Click the + icon to add a filter\r\n5. Create a
filter in the popover\r\n6. Click AND or OR\r\n7. Create another
filter\r\n8. Click Add filter\r\n9. Try saving the rule\r\n10. Saving
should be
successful","sha":"02d0c9852f0efb15b67c06e240e1525220a701ec"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Julia <iuliia.guskova@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport This PR is a backport of another PR bug Fixes for quality problems that affect the customer experience Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// v8.16.0 v9.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ResponseOps][Alerts] Error submiting rule form when using AddFilterPopover in actions

7 participants