Skip to content

Commit

Permalink
[Security Solutions][Detection Engine] Removes dead duplicated code a…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
FrankHassanabad authored Jul 13, 2021
1 parent ef06cf7 commit bdf1069
Show file tree
Hide file tree
Showing 26 changed files with 63 additions and 1,439 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ import {
ListOperatorEnum as OperatorEnum,
ListOperatorTypeEnum as OperatorTypeEnum,
} from '@kbn/securitysolution-io-ts-list-types';

import { OperatorOption } from './types';
import { OperatorOption } from '../types';

export const isOperator: OperatorOption = {
message: i18n.translate('lists.exceptions.isOperatorLabel', {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import {
isOneOfOperator,
isOperator,
} from '../autocomplete_operators';
import { OperatorOption } from '../autocomplete_operators/types';

import {
BuilderEntry,
Expand All @@ -52,6 +51,7 @@ import {
EmptyNestedEntry,
ExceptionsBuilderExceptionItem,
FormattedBuilderEntry,
OperatorOption,
} from '../types';

export const isEntryNested = (item: BuilderEntry): item is EntryNested => {
Expand Down
7 changes: 6 additions & 1 deletion packages/kbn-securitysolution-list-utils/src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,12 @@ import {
EXCEPTION_LIST_NAMESPACE_AGNOSTIC,
} from '@kbn/securitysolution-list-constants';

import type { OperatorOption } from '../autocomplete_operators/types';
export interface OperatorOption {
message: string;
value: string;
operator: OperatorEnum;
type: OperatorTypeEnum;
}

/**
* @deprecated Use the one from core once it is in its own package which will be from:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ interface OperatorProps {
selectedField: IFieldType | undefined;
}

/**
* There is a copy within:
* x-pack/plugins/security_solution/public/common/components/autocomplete/field.tsx
*
* TODO: This should be in its own packaged and not copied, https://github.com/elastic/kibana/issues/105378
* NOTE: This has deviated from the copy and will have to be reconciled.
*/
export const FieldComponent: React.FC<OperatorProps> = ({
fieldInputWidth,
fieldTypeFilter = [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ interface AutocompleteFieldMatchProps {
onError?: (arg: boolean) => void;
}

/**
* There is a copy of this within:
* x-pack/plugins/security_solution/public/common/components/autocomplete/field_value_match.tsx
* TODO: This should be in its own packaged and not copied, https://github.com/elastic/kibana/issues/105378
*/
export const AutocompleteFieldMatchComponent: React.FC<AutocompleteFieldMatchProps> = ({
placeholder,
rowLabel,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { EuiComboBoxOptionOption } from '@elastic/eui';
import type { ListSchema, Type } from '@kbn/securitysolution-io-ts-list-types';
import {
EXCEPTION_OPERATORS,
OperatorOption,
doesNotExistOperator,
existsOperator,
isNotOperator,
Expand All @@ -18,7 +19,7 @@ import {

import { IFieldType } from '../../../../../../../src/plugins/data/common';

import { GetGenericComboBoxPropsReturn, OperatorOption } from './types';
import { GetGenericComboBoxPropsReturn } from './types';
import * as i18n from './translations';

/**
Expand Down Expand Up @@ -72,6 +73,10 @@ export const checkEmptyValue = (

/**
* Very basic validation for values
* There is a copy within:
* x-pack/plugins/security_solution/public/common/components/autocomplete/helpers.ts
*
* TODO: This should be in its own packaged and not copied, https://github.com/elastic/kibana/issues/105378
*
* @param param the value being checked
* @param field the selected field
Expand Down Expand Up @@ -109,7 +114,10 @@ export const paramIsValid = (

/**
* Determines the options, selected values and option labels for EUI combo box
* There is a copy within:
* x-pack/plugins/security_solution/public/common/components/autocomplete/helpers.ts
*
* TODO: This should be in its own packaged and not copied, https://github.com/elastic/kibana/issues/105378
* @param options options user can select from
* @param selectedOptions user selection if any
* @param getLabel helper function to know which property to use for labels
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ export interface UseFieldValueAutocompleteProps {
}
/**
* Hook for using the field value autocomplete service
* There is a copy within:
* x-pack/plugins/security_solution/public/common/components/autocomplete/hooks/use_field_value_autocomplete.ts
*
* TODO: This should be in its own packaged and not copied, https://github.com/elastic/kibana/issues/105378
*/
export const useFieldValueAutocomplete = ({
selectedField,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@

import React, { useCallback, useMemo } from 'react';
import { EuiComboBox, EuiComboBoxOptionOption } from '@elastic/eui';
import { OperatorOption } from '@kbn/securitysolution-list-utils';

import { IFieldType } from '../../../../../../../src/plugins/data/common';

import { getGenericComboBoxProps, getOperators } from './helpers';
import { GetGenericComboBoxPropsReturn, OperatorOption } from './types';
import { GetGenericComboBoxPropsReturn } from './types';

const AS_PLAIN_TEXT = { asPlainText: true };

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,9 @@
*/

import { EuiComboBoxOptionOption } from '@elastic/eui';
import type {
ListOperatorEnum as OperatorEnum,
ListOperatorTypeEnum as OperatorTypeEnum,
} from '@kbn/securitysolution-io-ts-list-types';

export interface GetGenericComboBoxPropsReturn {
comboOptions: EuiComboBoxOptionOption[];
labels: string[];
selectedComboOptions: EuiComboBoxOptionOption[];
}

export interface OperatorOption {
message: string;
value: string;
operator: OperatorEnum;
type: OperatorTypeEnum;
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
BuilderEntry,
EXCEPTION_OPERATORS_ONLY_LISTS,
FormattedBuilderEntry,
OperatorOption,
getEntryOnFieldChange,
getEntryOnListChange,
getEntryOnMatchAnyChange,
Expand All @@ -32,7 +33,6 @@ import { IFieldType, IIndexPattern } from '../../../../../../../src/plugins/data
import { HttpStart } from '../../../../../../../src/core/public';
import { FieldComponent } from '../autocomplete/field';
import { OperatorComponent } from '../autocomplete/operator';
import { OperatorOption } from '../autocomplete/types';
import { AutocompleteFieldExistsComponent } from '../autocomplete/field_value_exists';
import { AutocompleteFieldMatchComponent } from '../autocomplete/field_value_match';
import { AutocompleteFieldMatchAnyComponent } from '../autocomplete/field_value_match_any';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
EmptyEntry,
ExceptionsBuilderExceptionItem,
FormattedBuilderEntry,
OperatorOption,
doesNotExistOperator,
existsOperator,
filterExceptionItems,
Expand Down Expand Up @@ -64,7 +65,6 @@ import { getEntryNestedMock } from '../../../../common/schemas/types/entry_neste
import { getEntryMatchMock } from '../../../../common/schemas/types/entry_match.mock';
import { getEntryMatchAnyMock } from '../../../../common/schemas/types/entry_match_any.mock';
import { getListResponseMock } from '../../../../common/schemas/response/list_schema.mock';
import { OperatorOption } from '../autocomplete/types';
import { getEntryListMock } from '../../../../common/schemas/types/entry_list.mock';

// TODO: ALL THESE TESTS SHOULD BE MOVED TO @kbn/securitysolution-list-utils for its helper. The only reason why they're here is due to missing other packages we hae to create or missing things from kbn packages such as mocks from kibana core
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ interface OperatorProps {
onChange: (a: IFieldType[]) => void;
}

/**
* There is a copy within:
* x-pack/plugins/lists/public/exceptions/components/autocomplete/field.tsx
*
* TODO: This should be in its own packaged and not copied, https://github.com/elastic/kibana/issues/105378
* NOTE: This has deviated from the copy and will have to be reconciled.
*/
export const FieldComponent: React.FC<OperatorProps> = ({
placeholder,
selectedField,
Expand Down

This file was deleted.

This file was deleted.

Loading

0 comments on commit bdf1069

Please sign in to comment.