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.

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

[Security Solution][Alerts] saved query UX changes follow-up #141747

Merged
merged 4 commits into from
Sep 27, 2022

Conversation

vitaliidm
Copy link
Contributor

@vitaliidm vitaliidm commented Sep 26, 2022

Summary

addresses feedback left during code review for #140064

  • Saved query checkbox label shows saved query name
  • createSavedQuery moved to x-pack/plugins/security_solution/cypress/tasks/api_calls/saved_queries.ts
  • useGetSavedQuery moved to x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/details
  • additional comments and minor code changes

Before

Screenshot 2022-09-26 at 11 38 05

After

Screenshot 2022-09-26 at 11 37 23

@vitaliidm vitaliidm self-assigned this Sep 26, 2022
@vitaliidm vitaliidm added Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detections and Resp Security Detection Response Team Team:Detection Alerts Security Detection Alerts Area Team labels Sep 26, 2022
@vitaliidm vitaliidm added backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) v8.5.0 v8.6.0 release_note:skip Skip the PR/issue when compiling release notes labels Sep 26, 2022
@vitaliidm vitaliidm changed the title [Security Solution][Alerts] fixes saved_query feedback [Security Solution][Alerts] saved_query UX changes follow-up Sep 26, 2022
@kibana-ci
Copy link
Collaborator

💚 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
securitySolution 6.6MB 6.6MB -1.8KB

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

cc @vitaliidm

@vitaliidm vitaliidm marked this pull request as ready for review September 26, 2022 12:06
@vitaliidm vitaliidm requested review from a team as code owners September 26, 2022 12:06
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@vitaliidm vitaliidm changed the title [Security Solution][Alerts] saved_query UX changes follow-up [Security Solution][Alerts] saved query UX changes follow-up Sep 26, 2022
Copy link
Contributor

@banderror banderror left a comment

Choose a reason for hiding this comment

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

The diff LGTM 👍 Thanks for addressing my comments in the previous PR @vitaliidm!

Comment on lines +310 to +311
// Rule schema allows to save any rule with saved_id property, but it only used for saved_query rule type
// In future we might look in possibility to restrict rule schema (breaking change!) and remove saved_id from the rest of rules through migration
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, that makes it a lot more clear.

@vitaliidm vitaliidm merged commit f585dd6 into elastic:main Sep 27, 2022
@vitaliidm vitaliidm deleted the alerts/saved_query-follow-up branch September 27, 2022 15:14
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 27, 2022
…#141747)

## Summary
addresses feedback left during code review for elastic#140064

- Saved query checkbox label shows saved query name
- `createSavedQuery` moved to `x-pack/plugins/security_solution/cypress/tasks/api_calls/saved_queries.ts`
- `useGetSavedQuery` moved to `x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/details`
- additional comments and minor code changes

### Before
<img width="762" alt="Screenshot 2022-09-26 at 11 38 05" src="https://user-images.githubusercontent.com/92328789/192256482-2bfc29c9-305a-4ff6-b9cd-b0505e887bd1.png">

### After
<img width="989" alt="Screenshot 2022-09-26 at 11 37 23" src="https://user-images.githubusercontent.com/92328789/192256378-290183b1-44e2-47bb-9d64-c46d0819cff4.png">

(cherry picked from commit f585dd6)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.5

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

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Sep 27, 2022
#141962)

## Summary
addresses feedback left during code review for #140064

- Saved query checkbox label shows saved query name
- `createSavedQuery` moved to `x-pack/plugins/security_solution/cypress/tasks/api_calls/saved_queries.ts`
- `useGetSavedQuery` moved to `x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/details`
- additional comments and minor code changes

### Before
<img width="762" alt="Screenshot 2022-09-26 at 11 38 05" src="https://user-images.githubusercontent.com/92328789/192256482-2bfc29c9-305a-4ff6-b9cd-b0505e887bd1.png">

### After
<img width="989" alt="Screenshot 2022-09-26 at 11 37 23" src="https://user-images.githubusercontent.com/92328789/192256378-290183b1-44e2-47bb-9d64-c46d0819cff4.png">

(cherry picked from commit f585dd6)

Co-authored-by: Vitalii Dmyterko <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes Team:Detection Alerts Security Detection Alerts Area Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.5.0 v8.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants