[Response Ops][Alerting] Fixing bug with agg building for ES query rule when there are multi-terms and a group by field

elastic / kibana

Your window into the Elastic Stack

https://www.elastic.co/products/kibana

Other

19.35k stars 7.98k forks source link

Closed ymao1 closed 1 week ago

ymao1 commented 1 week ago

Aggregation builder was updated to accommodate multiple term fields but the group-by portion of the builder does not properly handle multi-term aggs.

elasticmachine commented 1 week ago

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

ymao1 commented 1 week ago

The ES type is a little weird because aggs is optional, but I didn't want to make up my own type. WDYT.

should we disallow any in general for the TA_UI code?

I can make a followup issue for this. ETA: followup issue: https://github.com/elastic/kibana/issues/182895

pmuellr commented 1 week ago

The ES type is a little weird because aggs is optional, but I didn't want to make up my own type. WDYT.

meh, it's no weirder than other ES types :-)

LGTM. I should have been more specific that I was thinking about this as a followup. Thanks for the extra effort!

And thanks for the followup issue on the use of any in general!

kibana-ci commented 1 week ago

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

id	before	after	diff
`triggersActionsUi`	120.9KB	121.2KB	+230.0B

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

cc @ymao1

kibanamachine commented 1 week ago

Status	Branch	Result
✅	8.14

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