Removed Alerting & Event Log deprecated fields that should not be using#85652
Conversation
|
Pinging @elastic/kibana-alerting-services (Team:Alerting Services) |
gmmorris
left a comment
There was a problem hiding this comment.
LGTM,
My only request is that we replace the casting to unknown as it breaks the typing and is unsafe.
Thanks! :D
| @@ -286,15 +286,13 @@ export class ClusterClientAdapter { | |||
| hits: { hits, total }, | |||
| }: SearchResponse<unknown> = await this.callEs('search', { | |||
There was a problem hiding this comment.
If you use the ESSearchResponse type you won't need to cast to unknown.
At the top add:
import { ESSearchResponse } from '../../../../typings/elasticsearch';
Then you can replace this with:
| }: SearchResponse<unknown> = await this.callEs('search', { | |
| }: ESSearchResponse<unknown, {}> = await this.callEs('search', { |
Then below, replace the cast with:
total: total.value,
pmuellr
left a comment
There was a problem hiding this comment.
LGTM
I'm curious if/how we do function tests on these. Looked in x-pack/test/alerting_api_integration, and didn't see anything, but maybe I didn't know what to search for, or maybe it's somewhere else. Or we just don't have any.
I don't see any existing. How do you think we should test it? Just mock ESSearchResponse? |
💚 Build SucceededMetrics [docs]Distributable file count
History
To update your PR or re-run it, just comment with: |
…ng (elastic#85652) * Removed Alerting & Event Log deprecated fields that should not be using * fixed due to comments
I was thinking more about an E2E test, as FTS/FTR tests. I don't think unit tests with mocks is something worth the effort. And I'm not sure how this would work as FTS/FTR tests. Just curious ... :-) |
* master: (66 commits) [Alerting] fixes broken Alerting Example plugin (elastic#85774) [APM] Service overview instances table (elastic#85770) [Security Solution] Unskip timeline creation Cypress test (elastic#85871) properly recognize enterprise licenses (elastic#85849) [SecuritySolution][Detections] Adds SavedObject persistence to Signals Migrations (elastic#85690) [TSVB] Fix functional tests flakiness and unskip them (elastic#85388) [Fleet] Change permissions for Fleet enroll role (elastic#85802) Gauge visualization can no longer be clicked to filter on values since Kibana 7.10.0 (elastic#84768) [Security Solution][Detections] Add alert source to detection rule action context (elastic#85488) [Discover] Don't display hide/show button for histogram when there's no time filter (elastic#85424) skip flaky suite (elastic#78553) License checks for alerts plugin (elastic#85649) skip flaky suite (elastic#84992) skip 'query return results valid for scripted field' elastic#78553 Allow action types to perform their own mustache variable escaping in parameter templates (elastic#83919) [ML] More machine learning links in doc_links_service.ts (elastic#85365) Removed Alerting & Event Log deprecated fields that should not be using (elastic#85652) Closes elastic#79995 by adding new tab in transaction details to show related trace logs. (elastic#85859) Fix outdated jest snapshot [Maps] Surface on prem EMS (elastic#85729) ...
rest_total_hits_as_intfrom alerts and actions telemetry cluster request.rest_total_hits_as_intwith the new proper query optiontrack_total_hitsfor Event Log plugin.