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][Detection Engine] Pull out duplicated code from lists and security_solution into a package #105378

Closed
FrankHassanabad opened this issue Jul 13, 2021 · 3 comments · Fixed by #105382
Labels
bug Fixes for quality problems that affect the customer experience Feature:Rule Exceptions Security Solution Detection Rule Exceptions area fixed Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc.

Comments

@FrankHassanabad
Copy link
Contributor

FrankHassanabad commented Jul 13, 2021

Describe the bug:
We have duplicated code from lists and security_solution around a autocomplete feature. Some of these files have bug fixes and modifications that are not the same between the copies and will have to reviewed for the hopes we don't introduce a regression.

We should try and pull out autocomplete feature into its own package and then re-ingest and share those between lists and security_solution.

Duplicates:

field.tsx (Copied code is different, ref PR #87004)

  • x-pack/plugins/security_solution/public/common/components/autocomplete/field.tsx
  • x-pack/plugins/lists/public/exceptions/components/autocomplete/field.tsx

field_value_match.tsx

  • x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_match.tsx
  • x-pack/plugins/lists/public/exceptions/components/autocomplete/field_value_match.tsx

helpers.ts (some functions, not all)

  • x-pack/plugins/security_solution/public/common/components/autocomplete/helpers.ts
  • x-pack/plugins/lists/public/exceptions/components/autocomplete/helpers.ts

use_field_value_autocomplete.ts (Copied code is different, ref PR #94515)

  • x-pack/plugins/security_solution/public/common/components/autocomplete/hooks/use_field_value_autocomplete.ts
  • x-pack/lists/public/exceptions/components/autocomplete/hooks/use_field_value_autocomplete.ts

Kibana/Elasticsearch Stack version:
7.14.0

@FrankHassanabad FrankHassanabad added bug Fixes for quality problems that affect the customer experience Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Feature:Rule Exceptions Security Solution Detection Rule Exceptions area labels Jul 13, 2021
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

FrankHassanabad added a commit that referenced this issue Jul 13, 2021
…nd marks other duplicated code (#105374)

## Summary

* Removes dead duplicated code from `security_solution` and  `lists`
* Adds notes and TODO's where we still have duplicated logic
* Adds notes where I saw that the original deviated from the copy from modifications in one file but not the other.
* DOES NOT fix the bugs existing in one copy but not the other. That should be done when the copied chunks are collapsed into a package. Instead see this issue where I marked those areas: #105378

See these two files where things have deviated from our duplications as an example:
[security_solution/public/common/components/autocomplete/field.tsx](https://github.com/elastic/kibana/blob/master/x-pack/plugins/security_solution/public/common/components/autocomplete/field.tsx
)
[lists/public/exceptions/components/autocomplete/field.tsx](https://github.com/elastic/kibana/blob/master/x-pack/plugins/lists/public/exceptions/components/autocomplete/field.tsx)

Ref PR where fixes are applied to one of the files but not the other (could be other PR's in addition to this one):
#87004

### Checklist

Delete any items that are not applicable to this PR.

- [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 issue Jul 13, 2021
…nd marks other duplicated code (elastic#105374)

## Summary

* Removes dead duplicated code from `security_solution` and  `lists`
* Adds notes and TODO's where we still have duplicated logic
* Adds notes where I saw that the original deviated from the copy from modifications in one file but not the other.
* DOES NOT fix the bugs existing in one copy but not the other. That should be done when the copied chunks are collapsed into a package. Instead see this issue where I marked those areas: elastic#105378

See these two files where things have deviated from our duplications as an example:
[security_solution/public/common/components/autocomplete/field.tsx](https://github.com/elastic/kibana/blob/master/x-pack/plugins/security_solution/public/common/components/autocomplete/field.tsx
)
[lists/public/exceptions/components/autocomplete/field.tsx](https://github.com/elastic/kibana/blob/master/x-pack/plugins/lists/public/exceptions/components/autocomplete/field.tsx)

Ref PR where fixes are applied to one of the files but not the other (could be other PR's in addition to this one):
elastic#87004

### Checklist

Delete any items that are not applicable to this PR.

- [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 issue Jul 14, 2021
…nd marks other duplicated code (#105374) (#105524)

## Summary

* Removes dead duplicated code from `security_solution` and  `lists`
* Adds notes and TODO's where we still have duplicated logic
* Adds notes where I saw that the original deviated from the copy from modifications in one file but not the other.
* DOES NOT fix the bugs existing in one copy but not the other. That should be done when the copied chunks are collapsed into a package. Instead see this issue where I marked those areas: #105378

See these two files where things have deviated from our duplications as an example:
[security_solution/public/common/components/autocomplete/field.tsx](https://github.com/elastic/kibana/blob/master/x-pack/plugins/security_solution/public/common/components/autocomplete/field.tsx
)
[lists/public/exceptions/components/autocomplete/field.tsx](https://github.com/elastic/kibana/blob/master/x-pack/plugins/lists/public/exceptions/components/autocomplete/field.tsx)

Ref PR where fixes are applied to one of the files but not the other (could be other PR's in addition to this one):
#87004

### Checklist

Delete any items that are not applicable to this PR.

- [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 <[email protected]>
@peluja1012
Copy link
Contributor

Fixed by this PR #105382

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Rule Exceptions Security Solution Detection Rule Exceptions area fixed Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc.
Projects
None yet
3 participants