[Security Solution][Detections] Fixes show alerts for execution action on Rule Execution Log #128843

spong · 2022-03-29T23:06:37Z

Summary

One-liner fix for the Show alerts for execution action on the Rule Execution Log table. Had the wrong key after changing the response interface. Working on the follow-up feedback PR from #126215, and will be including additional test coverage there, but wanted to get this in before the first BC for testing purposes.

elasticmachine · 2022-03-29T23:06:39Z

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

elasticmachine · 2022-03-29T23:06:39Z

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

FrankHassanabad · 2022-03-29T23:41:57Z

It is additive to the filter, so when I click on multiple rows i get multiples of it.

Was expecting constancy where an animation would follow up to the tool bar so I could see what was happening. But I think this is acceptable for now.

FrankHassanabad

LGTM,

Verified the bug in my local env.
Changed the code locally to match this
Verified the fix
Left a comment about no constancy (I didn't see an animation to let me know a filter is being added)
Noticed it adds multiple filters but I think this is the intent of the UI/UX
Ate an orange 🟠 along the way as I was slightly hungry.

FrankHassanabad · 2022-03-29T23:44:09Z

.../detections/pages/detection_engine/rules/details/execution_log_table/execution_log_table.tsx

@@ -223,7 +223,7 @@ const ExecutionLogTableComponent: React.FC<ExecutionLogTableProps> = ({
            icon: 'filter',
            type: 'icon',
            onClick: (value: object) => {


You should type this eventually other than object and remove the ugly get below as that is just going to break later down the road.

Yeah, that's no bueno -- will try to type out the columns and then this can get cleaned up 👍

Already had a type for this! 🙂 Addressed here: https://github.com/elastic/kibana/pull/129003/files#diff-81a89a66909bd6b3a6836daa93e7cef8100675de09e94244319ee60426a4c361R230-R232

spong · 2022-03-29T23:46:55Z

Left a comment about no constancy (I didn't see an animation to let me know a filter is being added)

Yeah we don't do that anymore. That's a relic of the drag and drop days my friend 😅

FrankHassanabad · 2022-03-29T23:51:07Z

Left a comment about no constancy (I didn't see an animation to let me know a filter is being added)

Yeah we don't do that anymore. That's a relic of the drag and drop days my friend 😅

Thanks for the response and update on this PR. I would say the more appropriate statement I should make is, I was expecting a form of positive UI/UX feedback that something was added to the filter bar. This could have been one of the following or maybe something else:

Filter bar shaking (to show something moving and attract my eye movement)
A flashing indicator somewhere such as the maybe the filter flashing blue to white to blue to white a few times
Size increase of the filter bar and then shrinking back down possibly
Other ideas I'm sure...

I honestly missed the filter had been added on the click.

spong · 2022-03-29T23:54:31Z

Left a comment about no constancy (I didn't see an animation to let me know a filter is being added)

Yeah we don't do that anymore. That's a relic of the drag and drop days my friend 😅

Thanks for the response and update on this PR. I would say the more appropriate statement I should make is, I was expecting a form of positive UI/UX feedback that something was added to the filter bar. This could have been one of the following or maybe something else:

Filter bar shaking (to show something moving and attract my eye movement)

A flashing indicator somewhere such as the maybe the filter flashing blue to white to blue to white a few times

Size increase of the filter bar and then shrinking back down possibly

Other ideas I'm sure...

I honestly missed the filter had been added on the click.

You're welcome @FrankHassanabad! Thanks for the feedback! 🙂

I agree, it's definitely easy to miss and hard to tell that a filter has been added. I'm using the core provided global KQL addFilter mechanism, and there doesn't appear to be any functionality like you describe, so this would additional custom development. I can open an enhancement request to track this if you think that would be a good idea?

kibana-ci · 2022-03-30T01:58:50Z

💚 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`	4.8MB	4.8MB	+14.0B

Unknown metric groups

ESLint disabled in files

id	before	after	diff
`apm`	15	14	-1
`osquery`	5	4	-1
`securitySolution`	69	68	-1
`uptime`	7	6	-1
total			-4

ESLint disabled line counts

id	before	after	diff
`apm`	88	85	-3
`enterpriseSearch`	9	7	-2
`fleet`	47	46	-1
`osquery`	122	119	-3
`uptime`	49	43	-6
total			-15

References to deprecated APIs

id	before	after	diff
`canvas`	70	64	-6
`dashboard`	78	72	-6
`data`	475	465	-10
`dataEnhanced`	55	49	-6
`discover`	26	20	-6
`fleet`	20	19	-1
`lens`	18	14	-4
`management`	2	1	-1
`maps`	456	330	-126
`monitoring`	40	28	-12
`upgradeAssistant`	12	7	-5
`visDefaultEditor`	205	155	-50
`visTypeVega`	4	3	-1
`visualizations`	17	13	-4
total			-238

Total ESLint disabled count

id	before	after	diff
`apm`	103	99	-4
`enterpriseSearch`	9	7	-2
`fleet`	55	54	-1
`osquery`	127	123	-4
`securitySolution`	510	509	-1
`uptime`	56	49	-7
total			-19

History

💔 Build #34725 failed 9b14c21

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

cc @spong

FrankHassanabad · 2022-03-30T16:41:49Z

I can open an enhancement request to track this if you think that would be a good idea?

Yeah, I don't know if that goes to them or UI/UX, but I think something to indicate that we indeed add a filter would be helpful.

spong · 2022-03-30T20:09:19Z

I can open an enhancement request to track this if you think that would be a good idea?

Yeah, I don't know if that goes to them or UI/UX, but I think something to indicate that we indeed add a filter would be helpful.

I can update the tooltip on the action as an immediate fix so the user at least has the potential to be aware of what's about to happen. Will also clear out all other filters when the action fires to prevent duplicate filters, although that has the undesired side effect of wiping any specific user filters, and there's no easy way for the user to undo. We discussed this in relation to the datepicker, which has last 5 values when you open it, but there's nothing currently for capturing historical KQL state and restoring. I can raise with design and we can discuss further, but this is at least an okay experience for an MVP, and can help raise the priority of these much needed UX enhancements around adding/modifying/undoing the state of the global KQL bar. Thanks! 👍

kibanamachine · 2022-04-01T02:06:19Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 128843 or prevent reminders by adding the backport:skip label.

spong · 2022-04-12T21:01:11Z

@FrankHassanabad, I have implemented the above changes and this action will now show a toast informing the user that their global query state has been updated, and the toast also includes a button to revert to their previous query so as to not interrupt their workflow. I've also created this enhancement request (#129542) to add support for a query history to the new unified search component to improve this UX even further.

Thanks again for the UX feedback here, really appreciate it! This action UX now feels waaaay better for the user 🙂

Fixes show alerts for execution action

9b14c21

spong self-assigned this Mar 29, 2022

spong requested a review from a team as a code owner March 29, 2022 23:06

spong enabled auto-merge (squash) March 29, 2022 23:13

FrankHassanabad approved these changes Mar 29, 2022

View reviewed changes

FrankHassanabad reviewed Mar 29, 2022

View reviewed changes

Merge branch 'main' into rule-execution-log-action-fix

7be390a

spong merged commit ace270f into elastic:main Mar 30, 2022

spong deleted the rule-execution-log-action-fix branch March 30, 2022 02:53

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Apr 1, 2022

spalger added the backport:skip This commit does not require backporting label Apr 1, 2022

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Apr 1, 2022

spong mentioned this pull request Apr 12, 2022

[Security Solution][Platform] Standardize & Document methods for updating Global Query State #130068

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Detections] Fixes show alerts for execution action on Rule Execution Log #128843

[Security Solution][Detections] Fixes show alerts for execution action on Rule Execution Log #128843

spong commented Mar 29, 2022

elasticmachine commented Mar 29, 2022

elasticmachine commented Mar 29, 2022

FrankHassanabad commented Mar 29, 2022

FrankHassanabad left a comment

FrankHassanabad Mar 29, 2022

spong Mar 29, 2022

spong Mar 31, 2022

spong commented Mar 29, 2022

FrankHassanabad commented Mar 29, 2022

spong commented Mar 29, 2022

kibana-ci commented Mar 30, 2022

ESLint disabled in files

ESLint disabled line counts

References to deprecated APIs

Total ESLint disabled count

FrankHassanabad commented Mar 30, 2022

spong commented Mar 30, 2022

kibanamachine commented Apr 1, 2022

spong commented Apr 12, 2022

[Security Solution][Detections] Fixes show alerts for execution action on Rule Execution Log #128843

[Security Solution][Detections] Fixes show alerts for execution action on Rule Execution Log #128843

Conversation

spong commented Mar 29, 2022

Summary

elasticmachine commented Mar 29, 2022

elasticmachine commented Mar 29, 2022

FrankHassanabad commented Mar 29, 2022

FrankHassanabad left a comment

Choose a reason for hiding this comment

FrankHassanabad Mar 29, 2022

Choose a reason for hiding this comment

spong Mar 29, 2022

Choose a reason for hiding this comment

spong Mar 31, 2022

Choose a reason for hiding this comment

spong commented Mar 29, 2022

FrankHassanabad commented Mar 29, 2022

spong commented Mar 29, 2022

kibana-ci commented Mar 30, 2022

💚 Build Succeeded

Metrics [docs]

Async chunks

ESLint disabled in files

ESLint disabled line counts

References to deprecated APIs

Total ESLint disabled count

History

FrankHassanabad commented Mar 30, 2022

spong commented Mar 30, 2022

kibanamachine commented Apr 1, 2022

spong commented Apr 12, 2022