[Security Solutions][Detection Engine] Creates an autocomplete package and moves duplicate code between lists and security_solution there #105382

FrankHassanabad · 2021-07-13T03:36:30Z

Summary

Fixes #105378

Creates an autocomplete package from lists and removes duplicate code between lists and security_solutions

Consolidates different PR's where we were changing different parts of autocomplete in different ways.
Existing Cypress tests should cover any mistakes hopefully

Manual Testing:

Ensure this bug does not crop up again [Security Solution][Detections] Fix severity and risk score overrides when field mapping exists but the mapped fields do not #87004
Make sure that the exception list autocomplete looks alright

Checklist

Unit or functional tests were updated or added to match the most common scenarios

…ibana tickets to all the TODO blocks

elasticmachine · 2021-07-20T22:33:42Z

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

FrankHassanabad · 2021-07-21T14:10:23Z

@elasticmachine merge upstream

banderror

Looks great, love single-purpose libraries like this package! I'm only nitpicking in the comments.

What I did:

Tested locally, went through the steps to reproduce the related bugfix ([Security Solution] The detection rules have defined source values for severity overridden but there is no Source field selected for the same. #85951 (comment)), made sure the bug is still fixed.
Looked briefly at the code of the package and how it's used from security_solution.

What I didn't do:

Thorough line-by-line review of the package.
Review of changes in lists.
Testing if it works with exceptions.

banderror · 2021-07-22T13:03:02Z

packages/kbn-securitysolution-autocomplete/src/check_empty_value/index.ts

+// TODO: I have to use any here for now, but once this is available below, we should use the correct types, https://github.com/elastic/kibana/issues/105731
+// import { IFieldType } from '../../../../../../../src/plugins/data/common';
+type IFieldType = any;


Nit: #103530 is merged

Thanks, for letting me know that merged. They created a Kibana ticket for me I assigned it to myself. I have to fix these across all the packages and that will be a follow up all at once for these which I included in the comments of this PR:
#105731

banderror · 2021-07-22T13:08:57Z

packages/kbn-securitysolution-autocomplete/src/check_empty_value/index.ts

@@ -0,0 +1,37 @@
+/*


Nit: I'd bring more structure to this package by separating components from utils etc. For example:

src/components/autocomplete_field
src/components/autocomplete_field_match
src/utils/fields/check_empty_value
etc

The current flat structure works since there's not too many files, but still a little bit hard to navigate.

Also, file names don't always correspond to component names, for example.

I think from this comment and the comment below, maybe we can do a follow up code sweep or discussion around file name changes, conventions?

I have been following mostly patterns seen within these folders to make things portable if we collapse between this package and the others, but I'm open if we want to do a broad cleanup/sweep of just naming changes in 1 faster PR.

Yeah, that sounds good, if more people from the team say this kind of restructure/renaming makes sense, doing it in a separate PR either for only this one or across more packages (kbn-securitysolution-*?) would be a good idea.

banderror · 2021-07-22T13:12:56Z

packages/kbn-securitysolution-autocomplete/src/field/index.tsx

@@ -1,17 +1,23 @@
 /*
 * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
 * or more contributor license agreements. Licensed under the Elastic License
- * 2.0; you may not use this file except in compliance with the Elastic License
- * 2.0.
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except


Nit: personally I'm not a fan of folder-per-thing structure where inside of each folder you have a single index.ts or something like that. Makes it harder to use inside VS Code:

I'd lean towards less folders and more uniquely named files.

I think from this comment and the comment below, maybe we can do a follow up code sweep or discussion around file name changes, conventions?

I have been following mostly patterns seen within these folders to make things portable if we collapse between this package and the others, but I'm open if we want to do a broad cleanup/sweep of just naming changes in 1 faster PR.

…bad/kibana into create-autocomplete-package

kibanamachine · 2021-07-22T18:47:57Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

…e and moves duplicate code between lists and security_solution there (elastic#105382) ## Summary Creates an autocomplete package from `lists` and removes duplicate code between `lists` and `security_solutions` * Consolidates different PR's where we were changing different parts of autocomplete in different ways. * Existing Cypress tests should cover any mistakes hopefully Manual Testing: * Ensure this bug does not crop up again elastic#87004 * Make sure that the exception list autocomplete looks alright ### 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

…e and moves duplicate code between lists and security_solution there (elastic#105382) ## Summary Creates an autocomplete package from `lists` and removes duplicate code between `lists` and `security_solutions` * Consolidates different PR's where we were changing different parts of autocomplete in different ways. * Existing Cypress tests should cover any mistakes hopefully Manual Testing: * Ensure this bug does not crop up again elastic#87004 * Make sure that the exception list autocomplete looks alright ### 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 # Conflicts: # x-pack/plugins/translations/translations/ja-JP.json

…e and moves duplicate code between lists and security_solution there (#105382) (#106612) ## Summary Creates an autocomplete package from `lists` and removes duplicate code between `lists` and `security_solutions` * Consolidates different PR's where we were changing different parts of autocomplete in different ways. * Existing Cypress tests should cover any mistakes hopefully Manual Testing: * Ensure this bug does not crop up again #87004 * Make sure that the exception list autocomplete looks alright ### 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 # Conflicts: # x-pack/plugins/translations/translations/ja-JP.json

kibanamachine · 2021-07-27T04:19:35Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`lists`	222	229	+7
`securitySolution`	2260	2271	+11
total			+18

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`actions`	-	117	+117
`advancedSettings`	-	22	+22
`alerting`	-	234	+234
`apm`	-	39	+39
`apmOss`	-	4	+4
`banners`	-	9	+9
`bfetch`	-	62	+62
`canvas`	-	5	+5
`cases`	-	407	+407
`charts`	-	159	+159
`cloud`	-	21	+21
`core`	-	1080	+1080
`dashboard`	-	137	+137
`dashboardEnhanced`	-	50	+50
`dashboardMode`	-	11	+11
`data`	-	3162	+3162
`dataEnhanced`	-	16	+16
`dataVisualizer`	-	104	+104
`devTools`	-	8	+8
`discover`	-	55	+55
`discoverEnhanced`	-	37	+37
`embeddable`	-	384	+384
`embeddableEnhanced`	-	14	+14
`encryptedSavedObjects`	-	28	+28
`enterpriseSearch`	-	2	+2
`esUiShared`	-	90	+90
`eventLog`	-	70	+70
`expressionError`	-	12	+12
`expressionRevealImage`	-	4	+4
`expressions`	-	1567	+1567
`expressionShape`	-	90	+90
`features`	-	97	+97
`fileUpload`	-	128	+128
`fleet`	-	1033	+1033
`globalSearch`	-	14	+14
`home`	-	70	+70
`indexLifecycleManagement`	-	4	+4
`indexManagement`	-	157	+157
`indexPatternFieldEditor`	-	29	+29
`infra`	-	22	+22
`inspector`	-	78	+78
`kibanaLegacy`	-	62	+62
`kibanaReact`	-	230	+230
`kibanaUtils`	-	359	+359
`lens`	-	169	+169
`licenseApiGuard`	-	8	+8
`licenseManagement`	-	3	+3
`licensing`	-	42	+42
`lists`	-	143	+143
`management`	-	40	+40
`maps`	-	203	+203
`mapsEms`	-	75	+75
`metricsEntities`	-	6	+6
`ml`	-	274	+274
`monitoring`	-	10	+10
`navigation`	-	31	+31
`newsfeed`	-	17	+17
`observability`	-	219	+219
`osquery`	-	11	+11
`presentationUtil`	-	136	+136
`remoteClusters`	-	4	+4
`reporting`	-	132	+132
`rollup`	-	20	+20
`ruleRegistry`	-	60	+60
`runtimeFields`	-	19	+19
`savedObjects`	-	199	+199
`savedObjectsManagement`	-	85	+85
`savedObjectsTagging`	-	50	+50
`savedObjectsTaggingOss`	-	50	+50
`screenshotMode`	-	17	+17
`security`	-	51	+51
`securityOss`	-	9	+9
`securitySolution`	-	1130	+1130
`share`	-	83	+83
`snapshotRestore`	-	22	+22
`spacesOss`	-	5	+5
`stackAlerts`	-	4	+4
`taskManager`	-	25	+25
`telemetryCollectionManager`	-	29	+29
`telemetryCollectionXpack`	-	1	+1
`telemetryManagementSection`	-	13	+13
`timelines`	-	763	+763
`triggersActionsUi`	-	228	+228
`uiActions`	-	88	+88
`uiActionsEnhanced`	-	147	+147
`urlForwarding`	-	15	+15
`usageCollection`	-	16	+16
`visTypeTimeseries`	-	10	+10
`visualizations`	-	229	+229
total			+15174

Any counts in public APIs

Total count of every any typed public API. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats any for more detailed information.

id	before	after	diff
`bfetch`	-	1	+1
`charts`	-	2	+2
`core`	-	148	+148
`dashboard`	-	1	+1
`data`	-	98	+98
`dataVisualizer`	-	3	+3
`embeddable`	-	4	+4
`esUiShared`	-	4	+4
`expressions`	-	58	+58
`fileUpload`	-	4	+4
`fleet`	-	15	+15
`indexManagement`	-	12	+12
`indexPatternFieldEditor`	-	1	+1
`inspector`	-	6	+6
`kibanaLegacy`	-	3	+3
`kibanaReact`	-	5	+5
`kibanaUtils`	-	3	+3
`maps`	-	2	+2
`mapsEms`	-	1	+1
`ml`	-	10	+10
`presentationUtil`	-	3	+3
`reporting`	-	1	+1
`savedObjects`	-	3	+3
`savedObjectsTaggingOss`	-	3	+3
`securitySolution`	-	8	+8
`share`	-	1	+1
`snapshotRestore`	-	1	+1
`timelines`	-	6	+6
`triggersActionsUi`	-	1	+1
`uiActionsEnhanced`	-	2	+2
`visTypeTimeseries`	-	1	+1
`visualizations`	-	13	+13
total			+424

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`lists`	272.4KB	275.4KB	+3.0KB
`securitySolution`	6.3MB	6.3MB	+4.6KB
total			+7.6KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`actions`	-	7	+7
`advancedSettings`	-	1	+1
`alerting`	-	16	+16
`apm`	-	30	+30
`bfetch`	-	2	+2
`canvas`	-	3	+3
`cases`	-	14	+14
`charts`	-	1	+1
`core`	-	31	+31
`dashboard`	-	9	+9
`data`	-	64	+64
`dataEnhanced`	-	2	+2
`devTools`	-	2	+2
`discover`	-	6	+6
`discoverEnhanced`	-	2	+2
`embeddable`	-	3	+3
`encryptedSavedObjects`	-	3	+3
`esUiShared`	-	1	+1
`eventLog`	-	4	+4
`expressionError`	-	2	+2
`expressionRevealImage`	-	1	+1
`expressions`	-	5	+5
`features`	-	2	+2
`fileUpload`	-	1	+1
`fleet`	-	8	+8
`globalSearch`	-	5	+5
`home`	-	5	+5
`indexManagement`	-	3	+3
`indexPatternFieldEditor`	-	4	+4
`infra`	-	3	+3
`inspector`	-	4	+4
`kibanaLegacy`	-	1	+1
`kibanaReact`	-	4	+4
`kibanaUtils`	-	8	+8
`lens`	-	14	+14
`licensing`	-	8	+8
`lists`	-	38	+38
`management`	-	5	+5
`maps`	-	11	+11
`metricsEntities`	-	1	+1
`ml`	-	33	+33
`monitoring`	-	2	+2
`navigation`	-	2	+2
`observability`	-	10	+10
`presentationUtil`	-	5	+5
`reporting`	-	14	+14
`ruleRegistry`	-	9	+9
`runtimeFields`	-	2	+2
`savedObjects`	-	5	+5
`screenshotMode`	-	1	+1
`security`	-	6	+6
`securityOss`	-	3	+3
`securitySolution`	-	28	+28
`share`	-	8	+8
`snapshotRestore`	-	1	+1
`taskManager`	-	8	+8
`telemetryCollectionManager`	-	4	+4
`timelines`	-	25	+25
`triggersActionsUi`	-	19	+19
`uiActions`	-	11	+11
`uiActionsEnhanced`	-	10	+10
`usageCollection`	-	2	+2
`visTypeTimeseries`	-	3	+3
`visualizations`	-	12	+12
total			+557

Unknown metric groups

API count

id	before	after	diff
`actions`	-	117	+117
`advancedSettings`	-	23	+23
`alerting`	-	242	+242
`apm`	-	39	+39
`apmOss`	-	4	+4
`banners`	-	9	+9
`bfetch`	-	73	+73
`canvas`	-	6	+6
`cases`	-	445	+445
`charts`	-	190	+190
`cloud`	-	21	+21
`core`	-	2359	+2359
`dashboard`	-	160	+160
`dashboardEnhanced`	-	51	+51
`dashboardMode`	-	11	+11
`data`	-	3716	+3716
`dataEnhanced`	-	16	+16
`dataVisualizer`	-	104	+104
`devTools`	-	10	+10
`discover`	-	81	+81
`discoverEnhanced`	-	39	+39
`embeddable`	-	456	+456
`embeddableEnhanced`	-	14	+14
`encryptedSavedObjects`	-	30	+30
`enterpriseSearch`	-	2	+2
`esUiShared`	-	92	+92
`eventLog`	-	70	+70
`expressionError`	-	12	+12
`expressionRevealImage`	-	4	+4
`expressions`	-	2003	+2003
`expressionShape`	-	90	+90
`features`	-	215	+215
`fileUpload`	-	128	+128
`fleet`	-	1128	+1128
`globalSearch`	-	68	+68
`home`	-	94	+94
`indexLifecycleManagement`	-	4	+4
`indexManagement`	-	162	+162
`indexPatternFieldEditor`	-	31	+31
`infra`	-	25	+25
`inspector`	-	101	+101
`kibanaLegacy`	-	66	+66
`kibanaReact`	-	260	+260
`kibanaUtils`	-	551	+551
`lens`	-	185	+185
`licenseApiGuard`	-	8	+8
`licenseManagement`	-	3	+3
`licensing`	-	117	+117
`lists`	-	150	+150
`management`	-	40	+40
`maps`	-	204	+204
`mapsEms`	-	75	+75
`metricsEntities`	-	9	+9
`ml`	-	278	+278
`monitoring`	-	10	+10
`navigation`	-	31	+31
`newsfeed`	-	17	+17
`observability`	-	219	+219
`osquery`	-	11	+11
`presentationUtil`	-	163	+163
`remoteClusters`	-	4	+4
`reporting`	-	133	+133
`rollup`	-	20	+20
`ruleRegistry`	-	60	+60
`runtimeFields`	-	24	+24
`savedObjects`	-	213	+213
`savedObjectsManagement`	-	96	+96
`savedObjectsTagging`	-	54	+54
`savedObjectsTaggingOss`	-	89	+89
`screenshotMode`	-	22	+22
`security`	-	112	+112
`securityOss`	-	12	+12
`securitySolution`	-	1181	+1181
`share`	-	123	+123
`snapshotRestore`	-	23	+23
`spaces`	-	106	+106
`spacesOss`	-	72	+72
`stackAlerts`	-	4	+4
`taskManager`	-	52	+52
`telemetry`	-	42	+42
`telemetryCollectionManager`	-	29	+29
`telemetryCollectionXpack`	-	1	+1
`telemetryManagementSection`	-	14	+14
`timelines`	-	882	+882
`triggersActionsUi`	-	237	+237
`uiActions`	-	127	+127
`uiActionsEnhanced`	-	205	+205
`urlForwarding`	-	15	+15
`usageCollection`	-	57	+57
`visTypeTimeseries`	-	10	+10
`visualizations`	-	247	+247
total			+19108

References to deprecated APIs

id	before	after	diff
`actions`	-	8	+8
`alerting`	-	32	+32
`apm`	-	7	+7
`canvas`	-	55	+55
`cases`	-	151	+151
`crossClusterReplication`	-	2	+2
`dashboard`	-	128	+128
`dashboardEnhanced`	-	10	+10
`dataEnhanced`	-	53	+53
`dataVisualizer`	-	16	+16
`discover`	-	102	+102
`discoverEnhanced`	-	19	+19
`embeddable`	-	2	+2
`encryptedSavedObjects`	-	2	+2
`fleet`	-	89	+89
`globalSearch`	-	4	+4
`graph`	-	2	+2
`indexLifecycleManagement`	-	2	+2
`indexManagement`	-	12	+12
`infra`	-	292	+292
`lens`	-	168	+168
`lists`	-	103	+103
`maps`	-	592	+592
`ml`	-	140	+140
`observability`	-	34	+34
`presentationUtil`	-	2	+2
`savedObjects`	-	6	+6
`savedObjectsManagement`	-	18	+18
`savedObjectsTaggingOss`	-	5	+5
`security`	-	2	+2
`securitySolution`	-	832	+832
`stackAlerts`	-	104	+104
`timelines`	-	76	+76
`transform`	-	16	+16
`uptime`	-	11	+11
`urlDrilldown`	-	18	+18
`visTypeTimeseries`	-	10	+10
`visualizations`	-	32	+32
total			+3157

History

💚 Build #140424 succeeded 3b86db2
💚 Build #140054 succeeded 08f7308
💚 Build #139874 succeeded e9a32f4
💔 Build #139853 failed 70fd0cb
💔 Build #139843 failed cd203f0

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

cc @FrankHassanabad

FrankHassanabad added 2 commits July 12, 2021 21:26

Creates an auto-complete packaage

b2567e7

Fixed i18n keys

efa6d3b

FrankHassanabad self-assigned this Jul 13, 2021

FrankHassanabad added 12 commits July 13, 2021 09:48

Merge branch 'master' into create-autocomplete-package

31e9a2a

Merge branch 'master' into create-autocomplete-package

0d3ea89

Copied a few other patterns to try and get ?? to work

57b65e0

Merge branch 'master' into create-autocomplete-package

377061b

Changed out the lists implementation for the autocomplete from packages

c08a370

Integrated more with packages, some issues with mocks and jest tests

fb0a549

Merge branch 'master' into create-autocomplete-package

1c6c6aa

Moves the rest of the files and tests into the kbn-package and adds K…

cd203f0

…ibana tickets to all the TODO blocks

Merge branch 'master' into create-autocomplete-package

70fd0cb

Merge branch 'master' into create-autocomplete-package

1d88059

Merge branch 'master' into create-autocomplete-package

80aa9bf

Fixed i81n keys

e9a32f4

FrankHassanabad added v8.0.0 v7.15.0 release_note:skip Skip the PR/issue when compiling release notes Team:Detections and Resp Security Detection Response Team Feature:Rule Exceptions Security Solution Rule Exceptions feature labels Jul 20, 2021

FrankHassanabad changed the title ~~[Security Solutions][Detection Engine] Creates an autocomplete package~~ [Security Solutions][Detection Engine] Creates an autocomplete package and moves duplicate code between lists and security_solution there Jul 20, 2021

FrankHassanabad requested review from yctercero and banderror July 20, 2021 22:33

FrankHassanabad marked this pull request as ready for review July 20, 2021 22:33

FrankHassanabad requested review from a team as code owners July 20, 2021 22:33

FrankHassanabad requested review from mistic and jbudz July 20, 2021 22:40

Merge branch 'master' into create-autocomplete-package

08f7308

jbudz approved these changes Jul 21, 2021

View reviewed changes

banderror approved these changes Jul 22, 2021

View reviewed changes

FrankHassanabad added 3 commits July 22, 2021 09:27

Merge branch 'master' into create-autocomplete-package

68a107d

Fixing those i18n keys (looks like I am going to conflict for a while)

cbc133f

Merge branch 'create-autocomplete-package' of github.com:FrankHassana…

3b86db2

…bad/kibana into create-autocomplete-package

FrankHassanabad added the auto-backport Deprecated - use backport:version if exact versions are needed label Jul 22, 2021

FrankHassanabad merged commit cd667d0 into elastic:master Jul 22, 2021

FrankHassanabad deleted the create-autocomplete-package branch July 22, 2021 18:44

kibanamachine mentioned this pull request Jul 22, 2021

[7.x] [Security Solutions][Detection Engine] Creates an autocomplete package and moves duplicate code between lists and security_solution there (#105382) #106592

Closed

FrankHassanabad mentioned this pull request Jul 22, 2021

[7.x] [Security Solutions][Detection Engine] Creates an autocomplete package and moves duplicate code between lists and security_solution there (#105382) #106612

Merged

peluja1012 mentioned this pull request Jul 27, 2021

[Security Solution][Detection Engine] Pull out duplicated code from lists and security_solution into a package #105378

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solutions][Detection Engine] Creates an autocomplete package and moves duplicate code between lists and security_solution there #105382

[Security Solutions][Detection Engine] Creates an autocomplete package and moves duplicate code between lists and security_solution there #105382

FrankHassanabad commented Jul 13, 2021 •

edited

Loading

elasticmachine commented Jul 20, 2021

FrankHassanabad commented Jul 21, 2021

banderror left a comment •

edited

Loading

banderror Jul 22, 2021

FrankHassanabad Jul 22, 2021

banderror Jul 22, 2021

FrankHassanabad Jul 22, 2021

banderror Jul 22, 2021

banderror Jul 22, 2021

FrankHassanabad Jul 22, 2021

kibanamachine commented Jul 22, 2021

kibanamachine commented Jul 27, 2021

API count

References to deprecated APIs

[Security Solutions][Detection Engine] Creates an autocomplete package and moves duplicate code between lists and security_solution there #105382

[Security Solutions][Detection Engine] Creates an autocomplete package and moves duplicate code between lists and security_solution there #105382

Conversation

FrankHassanabad commented Jul 13, 2021 • edited Loading

Summary

Checklist

elasticmachine commented Jul 20, 2021

FrankHassanabad commented Jul 21, 2021

banderror left a comment • edited Loading

Choose a reason for hiding this comment

banderror Jul 22, 2021

Choose a reason for hiding this comment

FrankHassanabad Jul 22, 2021

Choose a reason for hiding this comment

banderror Jul 22, 2021

Choose a reason for hiding this comment

FrankHassanabad Jul 22, 2021

Choose a reason for hiding this comment

banderror Jul 22, 2021

Choose a reason for hiding this comment

banderror Jul 22, 2021

Choose a reason for hiding this comment

FrankHassanabad Jul 22, 2021

Choose a reason for hiding this comment

kibanamachine commented Jul 22, 2021

💚 Backport successful

kibanamachine commented Jul 27, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Any counts in public APIs

Async chunks

Public APIs missing exports

API count

References to deprecated APIs

History

FrankHassanabad commented Jul 13, 2021 •

edited

Loading

banderror left a comment •

edited

Loading