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.

Sign up for GitHub

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

Jump to bottom

[maps][alerting] fix ES query rule boundary field changed when editing the rule #165155

Merged
merged 34 commits into from
Sep 6, 2023
Merged

[maps][alerting] fix ES query rule boundary field changed when editing the rule #165155

merged 34 commits into from
Sep 6, 2023

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Aug 29, 2023 •
edited
Loading

Fixes #163959

While digging into the original issue, it was determined that the existing components were unsalvageable. Fixing all of the issues would have required more work than just starting over. Problems with original components include:

  1. updating rule state on component load. This is the cause of the reported bug.
  2. lack of loading state when performing async tasks, like loading data views.
  3. not displaying validation errors. When users clicked "save" with missing configuration, no UI notifications were displayed
  4. Heavy use of EuiExpression made it impossible to view all configuration in a single time

Now, geo containment form:

  1. Only updates rule state when users interact with inputs.
  2. Displays loading state when performing async tasks, like loading data views.
  3. Displays validation errors
  4. Has a simpler UI that allows users to see all configuration information at the same time.
Screen Shot 2023-08-30 at 5 34 00 PM Screen Shot 2023-08-30 at 5 34 48 PM

@nreese nreese force-pushed the geo_containment_form_refactor branch from 4a04f64 to d14f6b1 Compare August 29, 2023 20:21
@nreese nreese force-pushed the geo_containment_form_refactor branch from 8e88120 to bd66277 Compare August 30, 2023 18:34
@nreese nreese force-pushed the geo_containment_form_refactor branch from 8d2c5f4 to 92d0dab Compare August 30, 2023 23:11
@nreese nreese force-pushed the geo_containment_form_refactor branch from df2ea41 to 7c23118 Compare August 30, 2023 23:17
@nreese
Copy link
Contributor Author

nreese commented Aug 31, 2023

@elasticmachine merge upstream

@nreese nreese marked this pull request as ready for review August 31, 2023 20:44
@nreese nreese requested review from a team as code owners August 31, 2023 20:44
@nreese nreese added release_note:fix Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.11.0 labels Aug 31, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

stratoula
stratoula approved these changes Sep 1, 2023
View reviewed changes
Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

Unified search changes LGTM, code review only

@nreese
Copy link
Contributor Author

nreese commented Sep 5, 2023

@elasticmachine merge upstream

@nreese
Copy link
Contributor Author

nreese commented Sep 5, 2023

@elasticmachine merge upstream

@nickpeihl nickpeihl self-requested a review September 5, 2023 14:54
@nreese
Copy link
Contributor Author

nreese commented Sep 5, 2023

@elasticmachine merge upstream

nickpeihl
nickpeihl approved these changes Sep 5, 2023
View reviewed changes
Copy link
Member

@nickpeihl nickpeihl left a comment

Choose a reason for hiding this comment

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

lgtm! code review and tested in on-prem.

This looks so much cleaner and easier to use. Great to have the validation, too!

@nreese
Copy link
Contributor Author

nreese commented Sep 5, 2023

@elasticmachine merge upstream

@nreese
Copy link
Contributor Author

nreese commented Sep 5, 2023

@elasticmachine merge upstream

ymao1
ymao1 approved these changes Sep 6, 2023
View reviewed changes
Copy link
Contributor

@ymao1 ymao1 left a comment

Choose a reason for hiding this comment

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

response ops changes LGTM!

@nreese
Copy link
Contributor Author

nreese commented Sep 6, 2023

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

kibana-ci commented Sep 6, 2023 •
edited
Loading

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] Serverless Security Cypress Tests #8 / Add endpoint exception from rule details edits an endpoint exception item edits an endpoint exception item
  • [job] [logs] Serverless Security Cypress Tests #11 / Rules table: sorting "before all" hook for "Sorts by enabled rules" "before all" hook for "Sorts by enabled rules"
  • [job] [logs] Serverless Observability Tests / serverless common UI Data View Management Scripted fields tab is missing
  • [job] [logs] Serverless Security Investigations Cypress Tests #6 / Toggle full screen "before all" hook for "Should hide timeline header and tab list area" "before all" hook for "Should hide timeline header and tab list area"

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
stackAlerts 188.0KB 184.8KB -3.2KB
unifiedSearch 214.8KB 214.8KB +22.0B
total -3.2KB

Page load bundle

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

id before after diff
stackAlerts 21.0KB 21.0KB +13.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
stackAlerts 28 24 -4

References to deprecated APIs

id before after diff
stackAlerts 63 33 -30

Total ESLint disabled count

id before after diff
stackAlerts 28 24 -4

History

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

@nreese nreese merged commit 118ea87 into elastic:main Sep 6, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stratoula stratoula stratoula approved these changes

@ymao1 ymao1 ymao1 approved these changes

@nickpeihl nickpeihl nickpeihl approved these changes

Assignees

@nreese nreese

Labels
backport:skip This commit does not require backporting release_note:fix Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.11.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] ES query rule boundary field changed when editing the rule
7 participants
@nreese @elasticmachine @kibana-ci @nickpeihl @ymao1 @stratoula @kibanamachine