-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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][Detection Engine] Creates an autocomplete package and moves duplicate code between lists and security_solution there #105382
Changes from all commits
b2567e7
efa6d3b
31e9a2a
0d3ea89
57b65e0
377061b
c08a370
fb0a549
1c6c6aa
cd203f0
70fd0cb
1d88059
80aa9bf
e9a32f4
08f7308
68a107d
cbc133f
3b86db2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,125 @@ | ||
load("@npm//@bazel/typescript:index.bzl", "ts_config", "ts_project") | ||
load("@build_bazel_rules_nodejs//:index.bzl", "js_library", "pkg_npm") | ||
|
||
PKG_BASE_NAME = "kbn-securitysolution-autocomplete" | ||
|
||
PKG_REQUIRE_NAME = "@kbn/securitysolution-autocomplete" | ||
|
||
SOURCE_FILES = glob( | ||
[ | ||
"src/**/*.ts", | ||
"src/**/*.tsx" | ||
], | ||
exclude = [ | ||
"**/*.test.*", | ||
"**/*.mock.*", | ||
"**/*.mocks.*", | ||
], | ||
) | ||
|
||
SRCS = SOURCE_FILES | ||
|
||
filegroup( | ||
name = "srcs", | ||
srcs = SRCS, | ||
) | ||
|
||
NPM_MODULE_EXTRA_FILES = [ | ||
"react/package.json", | ||
"package.json", | ||
"README.md", | ||
] | ||
|
||
SRC_DEPS = [ | ||
"//packages/kbn-babel-preset", | ||
"//packages/kbn-dev-utils", | ||
"//packages/kbn-i18n", | ||
"//packages/kbn-securitysolution-io-ts-list-types", | ||
"//packages/kbn-securitysolution-list-hooks", | ||
"@npm//@babel/core", | ||
"@npm//babel-loader", | ||
"@npm//@elastic/eui", | ||
"@npm//react", | ||
"@npm//resize-observer-polyfill", | ||
"@npm//rxjs", | ||
"@npm//tslib", | ||
] | ||
|
||
TYPES_DEPS = [ | ||
"@npm//typescript", | ||
"@npm//@types/jest", | ||
"@npm//@types/node", | ||
"@npm//@types/react", | ||
] | ||
|
||
DEPS = SRC_DEPS + TYPES_DEPS | ||
|
||
ts_config( | ||
name = "tsconfig", | ||
src = "tsconfig.json", | ||
deps = [ | ||
"//:tsconfig.base.json", | ||
], | ||
) | ||
|
||
ts_config( | ||
name = "tsconfig_browser", | ||
src = "tsconfig.browser.json", | ||
deps = [ | ||
"//:tsconfig.base.json", | ||
"//:tsconfig.browser.json", | ||
], | ||
) | ||
|
||
ts_project( | ||
name = "tsc", | ||
args = ["--pretty"], | ||
srcs = SRCS, | ||
deps = DEPS, | ||
allow_js = True, | ||
declaration = True, | ||
declaration_dir = "target_types", | ||
declaration_map = True, | ||
incremental = True, | ||
out_dir = "target_node", | ||
root_dir = "src", | ||
source_map = True, | ||
tsconfig = ":tsconfig", | ||
) | ||
|
||
ts_project( | ||
name = "tsc_browser", | ||
args = ['--pretty'], | ||
srcs = SRCS, | ||
deps = DEPS, | ||
allow_js = True, | ||
declaration = False, | ||
incremental = True, | ||
out_dir = "target_web", | ||
source_map = True, | ||
root_dir = "src", | ||
tsconfig = ":tsconfig_browser", | ||
) | ||
|
||
js_library( | ||
name = PKG_BASE_NAME, | ||
package_name = PKG_REQUIRE_NAME, | ||
srcs = NPM_MODULE_EXTRA_FILES, | ||
visibility = ["//visibility:public"], | ||
deps = [":tsc", ":tsc_browser"] + DEPS, | ||
) | ||
|
||
pkg_npm( | ||
name = "npm_module", | ||
deps = [ | ||
":%s" % PKG_BASE_NAME, | ||
] | ||
) | ||
|
||
filegroup( | ||
name = "build", | ||
srcs = [ | ||
":npm_module", | ||
], | ||
visibility = ["//visibility:public"], | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
/* | ||
* 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 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
module.exports = { | ||
env: { | ||
web: { | ||
presets: ['@kbn/babel-preset/webpack_preset'], | ||
}, | ||
node: { | ||
presets: ['@kbn/babel-preset/node_preset'], | ||
}, | ||
}, | ||
ignore: ['**/*.test.ts', '**/*.test.tsx'], | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
/* | ||
* 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 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
module.exports = { | ||
preset: '@kbn/test', | ||
rootDir: '../..', | ||
roots: ['<rootDir>/packages/kbn-securitysolution-autocomplete'], | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{ | ||
"name": "@kbn/securitysolution-autocomplete", | ||
"version": "1.0.0", | ||
"description": "Security Solution auto complete", | ||
"license": "SSPL-1.0 OR Elastic License 2.0", | ||
"browser": "./target_web/index.js", | ||
"main": "./target_node/index.js", | ||
"types": "./target_types/index.d.ts", | ||
"private": true | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
{ | ||
"browser": "../target_web/react", | ||
"main": "../target_node/react", | ||
"types": "../target_types/react/index.d.ts" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
/* | ||
* 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 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
// Copied from "src/plugins/data/public/mocks.ts" but without any type information | ||
// TODO: Remove this in favor of the data/public/mocks if/when they become available, https://github.com/elastic/kibana/issues/100715 | ||
export const autocompleteStartMock = { | ||
getQuerySuggestions: jest.fn(), | ||
getValueSuggestions: jest.fn(), | ||
hasQuerySuggestions: jest.fn(), | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
/* | ||
* 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 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
import { checkEmptyValue } from '.'; | ||
import { getField } from '../fields/index.mock'; | ||
import * as i18n from '../translations'; | ||
|
||
describe('check_empty_value', () => { | ||
test('returns no errors if no field has been selected', () => { | ||
const isValid = checkEmptyValue('', undefined, true, false); | ||
|
||
expect(isValid).toBeUndefined(); | ||
}); | ||
|
||
test('returns error string if user has touched a required input and left empty', () => { | ||
const isValid = checkEmptyValue(undefined, getField('@timestamp'), true, true); | ||
|
||
expect(isValid).toEqual(i18n.FIELD_REQUIRED_ERR); | ||
}); | ||
|
||
test('returns no errors if required input is empty but user has not yet touched it', () => { | ||
const isValid = checkEmptyValue(undefined, getField('@timestamp'), true, false); | ||
|
||
expect(isValid).toBeUndefined(); | ||
}); | ||
|
||
test('returns no errors if user has touched an input that is not required and left empty', () => { | ||
const isValid = checkEmptyValue(undefined, getField('@timestamp'), false, true); | ||
|
||
expect(isValid).toBeUndefined(); | ||
}); | ||
|
||
test('returns no errors if user has touched an input that is not required and left empty string', () => { | ||
const isValid = checkEmptyValue('', getField('@timestamp'), false, true); | ||
|
||
expect(isValid).toBeUndefined(); | ||
}); | ||
|
||
test('returns null if input value is not empty string or undefined', () => { | ||
const isValid = checkEmptyValue('hellooo', getField('@timestamp'), false, true); | ||
|
||
expect(isValid).toBeNull(); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
/* | ||
* 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 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
import * as i18n from '../translations'; | ||
|
||
// 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; | ||
Comment on lines
+11
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: #103530 is merged There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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: |
||
|
||
/** | ||
* Determines if empty value is ok | ||
*/ | ||
export const checkEmptyValue = ( | ||
param: string | undefined, | ||
field: IFieldType | undefined, | ||
isRequired: boolean, | ||
touched: boolean | ||
): string | undefined | null => { | ||
if (isRequired && touched && (param == null || param.trim() === '')) { | ||
return i18n.FIELD_REQUIRED_ERR; | ||
} | ||
|
||
if ( | ||
field == null || | ||
(isRequired && !touched) || | ||
(!isRequired && (param == null || param === '')) | ||
) { | ||
return undefined; | ||
} | ||
|
||
return null; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
import React, { useCallback, useMemo, useState } from 'react'; | ||
import { EuiComboBox, EuiComboBoxOptionOption } from '@elastic/eui'; | ||
|
||
import { IFieldType, IIndexPattern } from '../../../../../../../src/plugins/data/common'; | ||
// 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, IIndexPattern } from '../../../../../../../../src/plugins/data/common'; | ||
type IFieldType = any; | ||
type IIndexPattern = any; | ||
|
||
import { getGenericComboBoxProps } from './helpers'; | ||
import { GetGenericComboBoxPropsReturn } from './types'; | ||
import { | ||
getGenericComboBoxProps, | ||
GetGenericComboBoxPropsReturn, | ||
} from '../get_generic_combo_box_props'; | ||
|
||
const AS_PLAIN_TEXT = { asPlainText: true }; | ||
|
||
|
@@ -28,13 +34,6 @@ 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 = [], | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.