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 Solutions] Critical bug fix to make error messages about missing connections clearer for the end user. #116490

Merged
merged 16 commits into from
Oct 29, 2021

Conversation

FrankHassanabad
Copy link
Contributor

@FrankHassanabad FrankHassanabad commented Oct 27, 2021

Summary

Fixes issue see on this comment:
#116336 (comment)

  • Removes legacy toaster component
  • Adds newer toaster component
  • Removes issue with the deps array within ReactJS
  • Adds utility to give a better network error message to the end user.
  • This does effect the timeline component since it shares the same import common component.
  • Adds a count of how many rules/timeline items have failed imports
  • These error toasters mimic Kibana core's error toaster error message and UI/UX
  • Adds e2e tests for imports with actions and error messages for them.

Rules import error messages now

Before for small toaster:
Screen Shot 2021-10-26 at 6 03 25 PM

After for small toaster for different error conditions:
Screen Shot 2021-10-26 at 6 00 24 PM

Screen Shot 2021-10-26 at 6 01 00 PM

Screen Shot 2021-10-26 at 6 02 29 PM

Before for when you click "See the full error":
Screen Shot 2021-10-26 at 5 58 47 PM

After for when you click "See the full error":
Screen Shot 2021-10-27 at 1 48 16 PM

Screen Shot 2021-10-27 at 1 26 31 PM

timeline

Before:
Screen Shot 2021-10-27 at 1 19 00 PM

Screen Shot 2021-10-27 at 1 19 08 PM

After:
Screen Shot 2021-10-27 at 1 49 45 PM

Screen Shot 2021-10-27 at 1 49 50 PM

Checklist

@FrankHassanabad FrankHassanabad self-assigned this Oct 27, 2021
@FrankHassanabad FrankHassanabad added v8.0.0 v7.16.0 auto-backport Deprecated: Automatically backport this PR after it's merged Team:Security Solution Platform Security Solution Platform Team labels Oct 27, 2021
@FrankHassanabad FrankHassanabad changed the title [Security Solutions] Make error messages about missing connectors more clear to the end user. [Security Solutions] Critical bug fix where we need to make error messages about missing connectors more clear to the end user. Oct 27, 2021
@FrankHassanabad FrankHassanabad marked this pull request as ready for review October 27, 2021 19:53
@FrankHassanabad FrankHassanabad requested a review from a team as a code owner October 27, 2021 19:53
@FrankHassanabad FrankHassanabad added the Team:Detections and Resp Security Detection Response Team label Oct 27, 2021
@elasticmachine
Copy link
Contributor

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

@FrankHassanabad FrankHassanabad changed the title [Security Solutions] Critical bug fix where we need to make error messages about missing connectors more clear to the end user. [Security Solutions] Critical bug fix where we need to make error messages about missing clear to the end user. Oct 27, 2021
@FrankHassanabad FrankHassanabad added release_note:skip Skip the PR/issue when compiling release notes and removed release_note:fix labels Oct 27, 2021
@FrankHassanabad FrankHassanabad changed the title [Security Solutions] Critical bug fix where we need to make error messages about missing clear to the end user. [Security Solutions] Critical bug fix where make error messages about missing connections clearer for the end user. Oct 27, 2021
@FrankHassanabad
Copy link
Contributor Author

@elasticmachine merge upstream

@FrankHassanabad FrankHassanabad changed the title [Security Solutions] Critical bug fix where make error messages about missing connections clearer for the end user. [Security Solutions] Critical bug fix to make error messages about missing connections clearer for the end user. Oct 28, 2021
@FrankHassanabad
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💛 Build succeeded, but was flaky

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.5MB 4.5MB +49.0B

History

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

cc @FrankHassanabad

Comment on lines +211 to +219
const actionIds = actionsFind.map((action) => action.id);
const { errors, rulesAcc } = rules.reduce(
(acc, parsedRule) => {
if (parsedRule instanceof Error) {
acc.rulesAcc.set(uuid.v4(), parsedRule);
} else {
const { rule_id: ruleId, actions } = parsedRule;
const missingActionIds = actions.flatMap((action) => {
if (actionIds.find((actionsId) => actionsId === action.id) == null) {
Copy link
Contributor

@dhurley14 dhurley14 Oct 29, 2021

Choose a reason for hiding this comment

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

I'm wondering if Line 211 should use a Set (could be rewritten as const actionIds = new Set(actionsFind.map((action) => action.id)); which would allow us to replace the actionIds.find with the faster Set.has operation

if (!actionIds.has(action.id)) {

which operates in sublinear time compared to the Array.find operation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me do this in a follow up PR since the rename from master to main just in case ... So I don't mess up this branch. Good advice, thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Follow up is here: #116860

Copy link
Contributor

@dhurley14 dhurley14 left a comment

Choose a reason for hiding this comment

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

This is a great improvement for our users. We might be able to remove an additional nested iteration in the getInvalidConnectors but everything else looks great!

@FrankHassanabad FrankHassanabad merged commit 29ac558 into elastic:main Oct 29, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 29, 2021
…ssing connections clearer for the end user. (elastic#116490)

## Summary

Fixes issue see on this comment:
elastic#116336 (comment)

* Removes legacy toaster component
* Adds newer toaster component
* Removes issue with the deps array within ReactJS
* Adds utility to give a better network error message to the end user.
* This does effect the timeline component since it shares the same import common component.
* Adds a count of how many rules/timeline items have failed imports
* These error toasters mimic Kibana core's error toaster error message and UI/UX
* Adds e2e tests for imports with actions and error messages for them.

## Rules import error messages now

Before for small toaster:
<img width="417" alt="Screen Shot 2021-10-26 at 6 03 25 PM" src="https://user-images.githubusercontent.com/1151048/139132586-3cf77c73-53ac-4066-b01f-2e91ef2da111.png">

After for small toaster for different error conditions:
<img width="358" alt="Screen Shot 2021-10-26 at 6 00 24 PM" src="https://user-images.githubusercontent.com/1151048/139132679-2eeb1ed3-9f6e-4766-a8ed-8804ce3e6963.png">

<img width="396" alt="Screen Shot 2021-10-26 at 6 01 00 PM" src="https://user-images.githubusercontent.com/1151048/139132742-750cd937-f401-44e8-9a10-c21410073b5d.png">

<img width="379" alt="Screen Shot 2021-10-26 at 6 02 29 PM" src="https://user-images.githubusercontent.com/1151048/139132766-21b58bea-7f46-43a6-a0e9-f01632958eab.png">

Before for when you click "See the full error":
<img width="817" alt="Screen Shot 2021-10-26 at 5 58 47 PM" src="https://user-images.githubusercontent.com/1151048/139132980-de1942d6-7b03-4c08-b34a-1fc4a22d5207.png">

After for when you click "See the full error":
<img width="838" alt="Screen Shot 2021-10-27 at 1 48 16 PM" src="https://user-images.githubusercontent.com/1151048/139136581-af1e331e-ed77-4338-8fb0-c2457acd135f.png">

<img width="802" alt="Screen Shot 2021-10-27 at 1 26 31 PM" src="https://user-images.githubusercontent.com/1151048/139135083-9ca56940-30a8-4f83-9355-312307172834.png">

## timeline

Before:
<img width="441" alt="Screen Shot 2021-10-27 at 1 19 00 PM" src="https://user-images.githubusercontent.com/1151048/139136614-8360d6a6-d182-413e-b5d9-b18e3d70dc24.png">

<img width="827" alt="Screen Shot 2021-10-27 at 1 19 08 PM" src="https://user-images.githubusercontent.com/1151048/139136637-f9203ac2-0eea-4a77-9c53-ac2c20ab32e0.png">

After:
<img width="408" alt="Screen Shot 2021-10-27 at 1 49 45 PM" src="https://user-images.githubusercontent.com/1151048/139136758-7532a8ba-6d73-45e2-adbb-6756ee997289.png">

<img width="820" alt="Screen Shot 2021-10-27 at 1 49 50 PM" src="https://user-images.githubusercontent.com/1151048/139136774-26d4a8a2-caf0-4c6f-94d3-a6cd92b79f5f.png">

### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 29, 2021
…ssing connections clearer for the end user. (elastic#116490)

## Summary

Fixes issue see on this comment:
elastic#116336 (comment)

* Removes legacy toaster component
* Adds newer toaster component
* Removes issue with the deps array within ReactJS
* Adds utility to give a better network error message to the end user.
* This does effect the timeline component since it shares the same import common component.
* Adds a count of how many rules/timeline items have failed imports
* These error toasters mimic Kibana core's error toaster error message and UI/UX
* Adds e2e tests for imports with actions and error messages for them.

## Rules import error messages now

Before for small toaster:
<img width="417" alt="Screen Shot 2021-10-26 at 6 03 25 PM" src="https://user-images.githubusercontent.com/1151048/139132586-3cf77c73-53ac-4066-b01f-2e91ef2da111.png">

After for small toaster for different error conditions:
<img width="358" alt="Screen Shot 2021-10-26 at 6 00 24 PM" src="https://user-images.githubusercontent.com/1151048/139132679-2eeb1ed3-9f6e-4766-a8ed-8804ce3e6963.png">

<img width="396" alt="Screen Shot 2021-10-26 at 6 01 00 PM" src="https://user-images.githubusercontent.com/1151048/139132742-750cd937-f401-44e8-9a10-c21410073b5d.png">

<img width="379" alt="Screen Shot 2021-10-26 at 6 02 29 PM" src="https://user-images.githubusercontent.com/1151048/139132766-21b58bea-7f46-43a6-a0e9-f01632958eab.png">

Before for when you click "See the full error":
<img width="817" alt="Screen Shot 2021-10-26 at 5 58 47 PM" src="https://user-images.githubusercontent.com/1151048/139132980-de1942d6-7b03-4c08-b34a-1fc4a22d5207.png">

After for when you click "See the full error":
<img width="838" alt="Screen Shot 2021-10-27 at 1 48 16 PM" src="https://user-images.githubusercontent.com/1151048/139136581-af1e331e-ed77-4338-8fb0-c2457acd135f.png">

<img width="802" alt="Screen Shot 2021-10-27 at 1 26 31 PM" src="https://user-images.githubusercontent.com/1151048/139135083-9ca56940-30a8-4f83-9355-312307172834.png">

## timeline

Before:
<img width="441" alt="Screen Shot 2021-10-27 at 1 19 00 PM" src="https://user-images.githubusercontent.com/1151048/139136614-8360d6a6-d182-413e-b5d9-b18e3d70dc24.png">

<img width="827" alt="Screen Shot 2021-10-27 at 1 19 08 PM" src="https://user-images.githubusercontent.com/1151048/139136637-f9203ac2-0eea-4a77-9c53-ac2c20ab32e0.png">

After:
<img width="408" alt="Screen Shot 2021-10-27 at 1 49 45 PM" src="https://user-images.githubusercontent.com/1151048/139136758-7532a8ba-6d73-45e2-adbb-6756ee997289.png">

<img width="820" alt="Screen Shot 2021-10-27 at 1 49 50 PM" src="https://user-images.githubusercontent.com/1151048/139136774-26d4a8a2-caf0-4c6f-94d3-a6cd92b79f5f.png">

### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
8.0
7.16

The backport PRs will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Oct 29, 2021
…ssing connections clearer for the end user. (#116490) (#116804)

## Summary

Fixes issue see on this comment:
#116336 (comment)

* Removes legacy toaster component
* Adds newer toaster component
* Removes issue with the deps array within ReactJS
* Adds utility to give a better network error message to the end user.
* This does effect the timeline component since it shares the same import common component.
* Adds a count of how many rules/timeline items have failed imports
* These error toasters mimic Kibana core's error toaster error message and UI/UX
* Adds e2e tests for imports with actions and error messages for them.

## Rules import error messages now

Before for small toaster:
<img width="417" alt="Screen Shot 2021-10-26 at 6 03 25 PM" src="https://user-images.githubusercontent.com/1151048/139132586-3cf77c73-53ac-4066-b01f-2e91ef2da111.png">

After for small toaster for different error conditions:
<img width="358" alt="Screen Shot 2021-10-26 at 6 00 24 PM" src="https://user-images.githubusercontent.com/1151048/139132679-2eeb1ed3-9f6e-4766-a8ed-8804ce3e6963.png">

<img width="396" alt="Screen Shot 2021-10-26 at 6 01 00 PM" src="https://user-images.githubusercontent.com/1151048/139132742-750cd937-f401-44e8-9a10-c21410073b5d.png">

<img width="379" alt="Screen Shot 2021-10-26 at 6 02 29 PM" src="https://user-images.githubusercontent.com/1151048/139132766-21b58bea-7f46-43a6-a0e9-f01632958eab.png">

Before for when you click "See the full error":
<img width="817" alt="Screen Shot 2021-10-26 at 5 58 47 PM" src="https://user-images.githubusercontent.com/1151048/139132980-de1942d6-7b03-4c08-b34a-1fc4a22d5207.png">

After for when you click "See the full error":
<img width="838" alt="Screen Shot 2021-10-27 at 1 48 16 PM" src="https://user-images.githubusercontent.com/1151048/139136581-af1e331e-ed77-4338-8fb0-c2457acd135f.png">

<img width="802" alt="Screen Shot 2021-10-27 at 1 26 31 PM" src="https://user-images.githubusercontent.com/1151048/139135083-9ca56940-30a8-4f83-9355-312307172834.png">

## timeline

Before:
<img width="441" alt="Screen Shot 2021-10-27 at 1 19 00 PM" src="https://user-images.githubusercontent.com/1151048/139136614-8360d6a6-d182-413e-b5d9-b18e3d70dc24.png">

<img width="827" alt="Screen Shot 2021-10-27 at 1 19 08 PM" src="https://user-images.githubusercontent.com/1151048/139136637-f9203ac2-0eea-4a77-9c53-ac2c20ab32e0.png">

After:
<img width="408" alt="Screen Shot 2021-10-27 at 1 49 45 PM" src="https://user-images.githubusercontent.com/1151048/139136758-7532a8ba-6d73-45e2-adbb-6756ee997289.png">

<img width="820" alt="Screen Shot 2021-10-27 at 1 49 50 PM" src="https://user-images.githubusercontent.com/1151048/139136774-26d4a8a2-caf0-4c6f-94d3-a6cd92b79f5f.png">

### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

Co-authored-by: Frank Hassanabad <frank.hassanabad@elastic.co>
kibanamachine added a commit that referenced this pull request Oct 29, 2021
…ssing connections clearer for the end user. (#116490) (#116803)

## Summary

Fixes issue see on this comment:
#116336 (comment)

* Removes legacy toaster component
* Adds newer toaster component
* Removes issue with the deps array within ReactJS
* Adds utility to give a better network error message to the end user.
* This does effect the timeline component since it shares the same import common component.
* Adds a count of how many rules/timeline items have failed imports
* These error toasters mimic Kibana core's error toaster error message and UI/UX
* Adds e2e tests for imports with actions and error messages for them.

## Rules import error messages now

Before for small toaster:
<img width="417" alt="Screen Shot 2021-10-26 at 6 03 25 PM" src="https://user-images.githubusercontent.com/1151048/139132586-3cf77c73-53ac-4066-b01f-2e91ef2da111.png">

After for small toaster for different error conditions:
<img width="358" alt="Screen Shot 2021-10-26 at 6 00 24 PM" src="https://user-images.githubusercontent.com/1151048/139132679-2eeb1ed3-9f6e-4766-a8ed-8804ce3e6963.png">

<img width="396" alt="Screen Shot 2021-10-26 at 6 01 00 PM" src="https://user-images.githubusercontent.com/1151048/139132742-750cd937-f401-44e8-9a10-c21410073b5d.png">

<img width="379" alt="Screen Shot 2021-10-26 at 6 02 29 PM" src="https://user-images.githubusercontent.com/1151048/139132766-21b58bea-7f46-43a6-a0e9-f01632958eab.png">

Before for when you click "See the full error":
<img width="817" alt="Screen Shot 2021-10-26 at 5 58 47 PM" src="https://user-images.githubusercontent.com/1151048/139132980-de1942d6-7b03-4c08-b34a-1fc4a22d5207.png">

After for when you click "See the full error":
<img width="838" alt="Screen Shot 2021-10-27 at 1 48 16 PM" src="https://user-images.githubusercontent.com/1151048/139136581-af1e331e-ed77-4338-8fb0-c2457acd135f.png">

<img width="802" alt="Screen Shot 2021-10-27 at 1 26 31 PM" src="https://user-images.githubusercontent.com/1151048/139135083-9ca56940-30a8-4f83-9355-312307172834.png">

## timeline

Before:
<img width="441" alt="Screen Shot 2021-10-27 at 1 19 00 PM" src="https://user-images.githubusercontent.com/1151048/139136614-8360d6a6-d182-413e-b5d9-b18e3d70dc24.png">

<img width="827" alt="Screen Shot 2021-10-27 at 1 19 08 PM" src="https://user-images.githubusercontent.com/1151048/139136637-f9203ac2-0eea-4a77-9c53-ac2c20ab32e0.png">

After:
<img width="408" alt="Screen Shot 2021-10-27 at 1 49 45 PM" src="https://user-images.githubusercontent.com/1151048/139136758-7532a8ba-6d73-45e2-adbb-6756ee997289.png">

<img width="820" alt="Screen Shot 2021-10-27 at 1 49 50 PM" src="https://user-images.githubusercontent.com/1151048/139136774-26d4a8a2-caf0-4c6f-94d3-a6cd92b79f5f.png">

### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

Co-authored-by: Frank Hassanabad <frank.hassanabad@elastic.co>
FrankHassanabad added a commit that referenced this pull request Oct 30, 2021
## Summary

One line follow up from #116490 from @dhurley14 here: #116490 (comment)


### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 30, 2021
## Summary

One line follow up from elastic#116490 from @dhurley14 here: elastic#116490 (comment)


### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 30, 2021
## Summary

One line follow up from elastic#116490 from @dhurley14 here: elastic#116490 (comment)


### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
kibanamachine added a commit that referenced this pull request Oct 30, 2021
## Summary

One line follow up from #116490 from @dhurley14 here: #116490 (comment)


### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

Co-authored-by: Frank Hassanabad <frank.hassanabad@elastic.co>
kibanamachine added a commit that referenced this pull request Oct 30, 2021
## Summary

One line follow up from #116490 from @dhurley14 here: #116490 (comment)


### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

Co-authored-by: Frank Hassanabad <frank.hassanabad@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated: Automatically backport this PR after it's merged release_note:skip Skip the PR/issue when compiling release notes Team:Detections and Resp Security Detection Response Team Team:Security Solution Platform Security Solution Platform Team v7.16.0 v8.0.0 v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants