[SIEM] [Detections] Fix open close signal on detail page #56757
Conversation
XavierM
added
bug
|
Pinging @elastic/siem (Team:SIEM) |
| filters: globalFilters, | ||
| filters: isEmpty(defaultFilters) | ||
| ? globalFilters | ||
| : [...(defaultFilters ?? []), ...globalFilters], |
There was a problem hiding this comment.
nit: Doesn't isEmpty catch the null and undefined, do you need the ?? []
There was a problem hiding this comment.
yes but typescript is still yelling at me
| queryObject = query; | ||
| queryObject = { | ||
| bool: { | ||
| filter: query, |
There was a problem hiding this comment.
I am surprised you are locking down the backend to only work with a filter query? This does make the backend more limited with this change where every query is now a filter?
If @dhurley14 is ok with the API change then this is good. Just pointing out I typically would try to keep the API as flexible as we can and push the logic more front end when it does the query.
But...Like I said if @dhurley14 sees this as the correct path I am ok because sometimes things like this is a bit of an "art form" where you do want to lock things down more compared to keeping them "loose".
There was a problem hiding this comment.
Yeah I thought the same thing. The query for open / close is copied across a couple different areas of the frontend so this was the "quicker" fix but long term we definitely should keep this more flexible. There might be a larger discussion around exposing crud operations on the signals index itself. Just a thought.
But yes I agree I would like to keep the API flexible. Quick fix for now though. I'll open an issue to come back to this in the near future.
There was a problem hiding this comment.
LGTM Checked it out, tested it locally, ran @dhurley14 's command line tests and those looked good. I think this is great! Appreciate the speed of this.
💚 Build SucceededTo update your PR or re-run it, just comment with: |
* master: (23 commits) Properly handle password change for users authenticated with provider other than `basic`. (elastic#55206) Improve pull request template proposal (elastic#56756) Only change handlers as the element changes (elastic#56782) [SIEM][Detection Engine] Final final rule changes (elastic#56806) [SIEM][Detection Engine] critical blocker, wrong ilm policy, need to match beats ilm policy Move ui/agg_types in to shim data plugin (elastic#56353) [SIEM] Fixes Signals count spinner (elastic#56797) [docs] Update upgrade version path (elastic#56658) [Canvas] Use unique Id for Canvas Embeddables (elastic#56783) [Rollups] Adjust max width for job detail panel (elastic#56674) Prevent http client from converting our form data (elastic#56772) Disable creating alerts client instances when ESO plugin is using an ephemeral encryption key (elastic#56676) Bumps terser-webpack-plugin to 2.3.4 (elastic#56662) Advanced settings component registry ⇒ kibana platform plugin (elastic#55940) [Endpoint] EMT-67: add kql support for endpoint list (elastic#56328) Implement UI for Create Alert form (elastic#55232) Fix: Filter pill base coloring (elastic#56761) fix open close signal on detail page (elastic#56757) [Search service] Move loadingCount to sync search strategy (elastic#56335) Rollup TSVB integration: Add test and fix warning text (elastic#56639) ...
Summary
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.For maintainers