[Response Ops][Alerting] Fixing bug with agg building for ES query rule when there are multi-terms and a group by field #182865
Conversation
| @@ -793,6 +793,51 @@ describe('buildAgg', () => { | |||
| }); | |||
| }); | |||
|
|
|||
| it('should create correct aggregation when condition params are defined and multi terms selected', async () => { | |||
There was a problem hiding this comment.
if you remove the fix and run this test, you'll see the same TypeError: Cannot set properties of undefined (setting 'order') error that we see in the logs
ymao1
added
Feature:Alerting
Team:ResponseOps
|
Pinging @elastic/response-ops (Team:ResponseOps) |
There was a problem hiding this comment.
LGTM
I noticed that aggParent is an any - and I checked and can't use any in an alerting plugin server plugin. So, wondering a couple things:
- should we disallow
anyin general for the TA_UI code? - as a simpler exercise, could we do typing on the
aggParentitself?
Not sure it's worth the effort ...
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 |
💚 Build Succeeded
Metrics [docs]Page load bundle
History
To update your PR or re-run it, just comment with: cc @ymao1 |
…le when there are multi-terms and a group by field (elastic#182865) (cherry picked from commit 3f7731c)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…uery rule when there are multi-terms and a group by field (#182865) (#182898) # Backport This will backport the following commits from `main` to `8.14`: - [[Response Ops][Alerting] Fixing bug with agg building for ES query rule when there are multi-terms and a group by field (#182865)](#182865) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Ying Mao","email":"ying.mao@elastic.co"},"sourceCommit":{"committedDate":"2024-05-08T02:58:19Z","message":"[Response Ops][Alerting] Fixing bug with agg building for ES query rule when there are multi-terms and a group by field (#182865)","sha":"3f7731c8b79dc349111a843e39247f11b946b228","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Feature:Alerting","Team:ResponseOps","v8.14.0","v8.15.0"],"title":"[Response Ops][Alerting] Fixing bug with agg building for ES query rule when there are multi-terms and a group by field","number":182865,"url":"#182865 Ops][Alerting] Fixing bug with agg building for ES query rule when there are multi-terms and a group by field (#182865)","sha":"3f7731c8b79dc349111a843e39247f11b946b228"}},"sourceBranch":"main","suggestedTargetBranches":["8.14"],"targetPullRequestStates":[{"branch":"8.14","label":"v8.14.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.15.0","branchLabelMappingKey":"^v8.15.0$","isSourceBranch":true,"state":"MERGED","url":"#182865 Ops][Alerting] Fixing bug with agg building for ES query rule when there are multi-terms and a group by field (#182865)","sha":"3f7731c8b79dc349111a843e39247f11b946b228"}}]}] BACKPORT--> Co-authored-by: Ying Mao <ying.mao@elastic.co>
Towards #182755
Summary
Aggregation builder was updated to accommodate multiple term fields but the group-by portion of the builder does not properly handle multi-term aggs.