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] Add query bar inputs to geo threshold alerts tracked points & boundaries #80871

Merged
merged 32 commits into from
Nov 18, 2020
Merged

[Maps] Add query bar inputs to geo threshold alerts tracked points & boundaries #80871

merged 32 commits into from
Nov 18, 2020

Conversation

kindsun
Copy link
Contributor

@kindsun kindsun commented Oct 16, 2020 •
edited
Loading

Adds KQL query bars to filter both the tracked geo_points and the tracking geo_shapes

image

@kindsun kindsun added [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation Feature:Alerting v8.0.0 v7.11.0 labels Oct 16, 2020
@kindsun kindsun mentioned this pull request Oct 16, 2020
Closed
20 tasks
Aaron Caldwell added 11 commits October 19, 2020 15:29
Verify syntax and send to server. Add handling for shapes only logic …
fa4f0ef 
…on server
Merge remote-tracking branch 'upstream/master' into add-query-inputs-…
2349822 
…to-geo-tracker
Merge remote-tracking branch 'upstream/master' into add-query-inputs-…
cb7d3b8 
…to-geo-tracker
Construct es queries filters server side with interval dates
6680db0
Fix how query bars repopulate for alert edit
ed259c6
Add parenthesis to query conditions to structure conditions correctly
8f9fce2
Merge remote-tracking branch 'upstream/master' into add-query-inputs-…
29f1b30 
…to-geo-tracker
Type fixes and clean up
4f692c4
More type fixes and safety checks on the server side
5f5df0c
Merge remote-tracking branch 'upstream/master' into add-query-inputs-…
56d646d 
…to-geo-tracker
Type fix
8e86d59
@kindsun kindsun marked this pull request as ready for review November 5, 2020 14:55
@kindsun kindsun requested a review from a team as a code owner November 5, 2020 14:55
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis (Team:Geo)

thomasneirynck
thomasneirynck reviewed Nov 9, 2020
View reviewed changes
Copy link
Contributor

@thomasneirynck thomasneirynck left a comment

Choose a reason for hiding this comment

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

thx for adding this important functionality. it's a clean way for users to filter down. imho, it works well, even better than e.g. a filter-pill approach which is very cumbersome to work with.

only naming nits at this point 😅

With this new addition, I'd add unit test coverage as well. It's an opportunity to introduce:

  • snapshot tests for the components (GeoThresholdAlertTypeExpression and sub-components). This will help close the gap on the UX-side.
  • unit tests on some logic. here specifically, introduce a unit test for getEsFormattedQuery. This will help us to give more confidence when refactoring later-on

...s_ui/public/application/components/builtin_alert_types/geo_threshold/query_builder/index.tsx Outdated Show resolved Hide resolved
YulNaumenko
YulNaumenko approved these changes Nov 10, 2020
View reviewed changes
Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

LGTM 👍 . Anyway we have this issue coming soon in alerting for switch our deps into KibanaContext

@kindsun
Copy link
Contributor Author

kindsun commented Nov 12, 2020

Thanks @thomasneirynck! Just some quick follow up:

snapshot tests for the components (GeoThresholdAlertTypeExpression and sub-components). This will help close the gap on the UX-side.

I was able to add snapshot tests for the boundary and index sub-components. Trying to do a shallow snapshot copy of the top-level threshold alerting component was a litte trickier (you run into some strange transitive dependency errors that are difficult to debug) so I've omitted this for now.

unit tests on some logic. here specifically, introduce a unit test for getEsFormattedQuery. This will help us to give more confidence when refactoring later-on

Added unit tests for this logic covering both lucene and kuery style queries

thomasneirynck
thomasneirynck approved these changes Nov 12, 2020
View reviewed changes
Copy link
Contributor

@thomasneirynck thomasneirynck left a comment

Choose a reason for hiding this comment

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

lgtm when green, thx for adding the test-coverage!

@kindsun
Copy link
Contributor Author

kindsun commented Nov 16, 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
stackAlerts 79.6KB 81.6KB +2.0KB
triggersActionsUi 1.4MB 1.4MB +351.0B
total +2.4KB

Page load bundle

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

id before after diff
stackAlerts 14.3KB 14.6KB +231.0B
triggersActionsUi 142.2KB 143.0KB +760.0B
total +991.0B

History

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

@kindsun kindsun merged commit 0546f98 into elastic:master Nov 18, 2020
@kindsun kindsun deleted the add-query-inputs-to-geo-tracker branch November 18, 2020 21:47
@kindsun kindsun mentioned this pull request Nov 18, 2020
Merged
gmmorris added a commit to gmmorris/kibana that referenced this pull request Nov 19, 2020
@gmmorris
Merge branch 'master' into task-manager/retry-by-schedule
6452100 
* master: (60 commits)
  Forward any registry cache-control header for files (elastic#83680)
  Revert "[Alerting] Add `alert.updatedAt` field to represent date of last user edit (elastic#83578)"
  [Security Solution][Detections] Fix adding an action to detection rules (elastic#83722)
  Make expectSnapshot available in all functional test runs (elastic#82932)
  Skip failing cypress test
  Increase bulk request timeout during esArchiver load (elastic#83657)
  [data.search] Server-side background session service (elastic#81099)
  [maps] convert VectorStyleEditor to TS (elastic#83582)
  Revert "[App Search] Engine overview layout stub (elastic#83504)"
  Adding documentation for global action configuration options (elastic#83557)
  [Metrics UI] Optimizations for Snapshot and Inventory Metadata (elastic#83596)
  chore(NA): update lmdb store to v0.8.15 (elastic#83726)
  [App Search] Engine overview layout stub (elastic#83504)
  [Workplace Search] Update SourceIcon to match latest changes in ent-search (elastic#83714)
  [Enterprise Search] Rename React Router helpers (elastic#83718)
  [Maps] Add 'crossed' & 'exited' events to tracking alert (elastic#82463)
  Updating code-owners to use new core/app-services team names (elastic#83731)
  Add Managed label to data streams and a view switch for the table (elastic#83049)
  [Maps] Add query bar inputs to geo threshold alerts tracked points & boundaries (elastic#80871)
  fix(NA): search examples kibana version declaration (elastic#83182)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Nov 19, 2020
@gmmorris
Merge branch 'master' into alerts/action-groups-as-conditions
3bf8f60 
* master: (60 commits)
  Forward any registry cache-control header for files (elastic#83680)
  Revert "[Alerting] Add `alert.updatedAt` field to represent date of last user edit (elastic#83578)"
  [Security Solution][Detections] Fix adding an action to detection rules (elastic#83722)
  Make expectSnapshot available in all functional test runs (elastic#82932)
  Skip failing cypress test
  Increase bulk request timeout during esArchiver load (elastic#83657)
  [data.search] Server-side background session service (elastic#81099)
  [maps] convert VectorStyleEditor to TS (elastic#83582)
  Revert "[App Search] Engine overview layout stub (elastic#83504)"
  Adding documentation for global action configuration options (elastic#83557)
  [Metrics UI] Optimizations for Snapshot and Inventory Metadata (elastic#83596)
  chore(NA): update lmdb store to v0.8.15 (elastic#83726)
  [App Search] Engine overview layout stub (elastic#83504)
  [Workplace Search] Update SourceIcon to match latest changes in ent-search (elastic#83714)
  [Enterprise Search] Rename React Router helpers (elastic#83718)
  [Maps] Add 'crossed' & 'exited' events to tracking alert (elastic#82463)
  Updating code-owners to use new core/app-services team names (elastic#83731)
  Add Managed label to data streams and a view switch for the table (elastic#83049)
  [Maps] Add query bar inputs to geo threshold alerts tracked points & boundaries (elastic#80871)
  fix(NA): search examples kibana version declaration (elastic#83182)
  ...
phillipb added a commit to phillipb/kibana that referenced this pull request Nov 19, 2020
@phillipb
Merge branch 'add-logs-to-node-details' of github.com:phillipb/kibana…
2249140 
… into add-logs-to-node-details

* 'add-logs-to-node-details' of github.com:phillipb/kibana: (87 commits)
  [Maps] Add 'crossed' & 'exited' events to tracking alert (elastic#82463)
  Updating code-owners to use new core/app-services team names (elastic#83731)
  Add Managed label to data streams and a view switch for the table (elastic#83049)
  [Maps] Add query bar inputs to geo threshold alerts tracked points & boundaries (elastic#80871)
  fix(NA): search examples kibana version declaration (elastic#83182)
  Fixed console error, which appears when saving changes in Edit Alert flyout (elastic#83610)
  [Alerting] Add `alert.updatedAt` field to represent date of last user edit (elastic#83578)
  Not resetting server log level if level is defined (elastic#83651)
  disable incremenetal build for legacy tsconfig.json (elastic#82986)
  [Workplace Search] Migrate SourceLogic from ent-search (elastic#83593)
  [Workplace Search] Port Box changes from ent-search (elastic#83675)
  [APM] Improve router types (elastic#83620)
  Bump flat to v4.1.1 (elastic#83647)
  Bump y18n@5 to v5.0.5 (elastic#83644)
  Bump jsonpointer to v4.1.0 (elastic#83641)
  Bump is-my-json-valid to v2.20.5 (elastic#83642)
  [Telemetry] Move Monitoring collection strategy to a collector (elastic#82638)
  Update typescript eslint to v4.8 (elastic#83520)
  [ML] Persist URL state for Anomaly detection jobs using metric function (elastic#83507)
  [ML] Performance improvements to annotations editing in Single Metric Viewer & buttons placement (elastic#83216)
  ...
kindsun pushed a commit that referenced this pull request Nov 19, 2020
@kibanamachine
[7.x] [Maps] Add query bar inputs to geo threshold alerts tracked poi…
d6c5323 
…nts & boundaries (#80871) (#83727)

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
chrisronline pushed a commit to chrisronline/kibana that referenced this pull request Nov 19, 2020
@kibanamachine @chrisronline
[Maps] Add query bar inputs to geo threshold alerts tracked points & …
380a38f 
…boundaries (elastic#80871)

Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YulNaumenko YulNaumenko YulNaumenko approved these changes

@thomasneirynck thomasneirynck thomasneirynck approved these changes

Assignees
No one assigned
Labels
[Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation Feature:Alerting release_note:enhancement v7.11.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kindsun @elasticmachine @kibanamachine @thomasneirynck @YulNaumenko