diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts index 333f5caf72525..a8c5df8d64630 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts @@ -21,28 +21,64 @@ import { esKuery } from '../../../es_query'; type KueryNode = any; -import { typeRegistryMock } from '../../../saved_objects_type_registry.mock'; +import { SavedObjectTypeRegistry } from '../../../saved_objects_type_registry'; import { ALL_NAMESPACES_STRING } from '../utils'; import { getQueryParams, getClauseForReference } from './query_params'; -const registry = typeRegistryMock.create(); +const registerTypes = (registry: SavedObjectTypeRegistry) => { + registry.registerType({ + name: 'pending', + hidden: false, + namespaceType: 'single', + mappings: { + properties: { title: { type: 'text' } }, + }, + management: { + defaultSearchField: 'title', + }, + }); -const MAPPINGS = { - properties: { - pending: { properties: { title: { type: 'text' } } }, - saved: { + registry.registerType({ + name: 'saved', + hidden: false, + namespaceType: 'single', + mappings: { properties: { title: { type: 'text', fields: { raw: { type: 'keyword' } } }, obj: { properties: { key1: { type: 'text' } } }, }, }, - // mock registry returns isMultiNamespace=true for 'shared' type - shared: { properties: { name: { type: 'keyword' } } }, - // mock registry returns isNamespaceAgnostic=true for 'global' type - global: { properties: { name: { type: 'keyword' } } }, - }, + management: { + defaultSearchField: 'title', + }, + }); + + registry.registerType({ + name: 'shared', + hidden: false, + namespaceType: 'multiple', + mappings: { + properties: { name: { type: 'keyword' } }, + }, + management: { + defaultSearchField: 'name', + }, + }); + + registry.registerType({ + name: 'global', + hidden: false, + namespaceType: 'agnostic', + mappings: { + properties: { name: { type: 'keyword' } }, + }, + management: { + defaultSearchField: 'name', + }, + }); }; -const ALL_TYPES = Object.keys(MAPPINGS.properties); + +const ALL_TYPES = ['pending', 'saved', 'shared', 'global']; // get all possible subsets (combination) of all types const ALL_TYPE_SUBSETS = ALL_TYPES.reduce( (subsets, value) => subsets.concat(subsets.map((set) => [...set, value])), @@ -51,48 +87,53 @@ const ALL_TYPE_SUBSETS = ALL_TYPES.reduce( .filter((x) => x.length) // exclude empty set .map((x) => (x.length === 1 ? x[0] : x)); // if a subset is a single string, destructure it -const createTypeClause = (type: string, namespaces?: string[]) => { - if (registry.isMultiNamespace(type)) { - const array = [...(namespaces ?? ['default']), ALL_NAMESPACES_STRING]; - return { - bool: { - must: expect.arrayContaining([{ terms: { namespaces: array } }]), - must_not: [{ exists: { field: 'namespace' } }], - }, - }; - } else if (registry.isSingleNamespace(type)) { - const nonDefaultNamespaces = namespaces?.filter((n) => n !== 'default') ?? []; - const should: any = []; - if (nonDefaultNamespaces.length > 0) { - should.push({ terms: { namespace: nonDefaultNamespaces } }); - } - if (namespaces?.includes('default')) { - should.push({ bool: { must_not: [{ exists: { field: 'namespace' } }] } }); - } - return { - bool: { - must: [{ term: { type } }], - should: expect.arrayContaining(should), - minimum_should_match: 1, - must_not: [{ exists: { field: 'namespaces' } }], - }, - }; - } - // isNamespaceAgnostic - return { - bool: expect.objectContaining({ - must_not: [{ exists: { field: 'namespace' } }, { exists: { field: 'namespaces' } }], - }), - }; -}; - /** * Note: these tests cases are defined in the order they appear in the source code, for readability's sake */ describe('#getQueryParams', () => { - const mappings = MAPPINGS; + let registry: SavedObjectTypeRegistry; type Result = ReturnType; + beforeEach(() => { + registry = new SavedObjectTypeRegistry(); + registerTypes(registry); + }); + + const createTypeClause = (type: string, namespaces?: string[]) => { + if (registry.isMultiNamespace(type)) { + const array = [...(namespaces ?? ['default']), ALL_NAMESPACES_STRING]; + return { + bool: { + must: expect.arrayContaining([{ terms: { namespaces: array } }]), + must_not: [{ exists: { field: 'namespace' } }], + }, + }; + } else if (registry.isSingleNamespace(type)) { + const nonDefaultNamespaces = namespaces?.filter((n) => n !== 'default') ?? []; + const should: any = []; + if (nonDefaultNamespaces.length > 0) { + should.push({ terms: { namespace: nonDefaultNamespaces } }); + } + if (namespaces?.includes('default')) { + should.push({ bool: { must_not: [{ exists: { field: 'namespace' } }] } }); + } + return { + bool: { + must: [{ term: { type } }], + should: expect.arrayContaining(should), + minimum_should_match: 1, + must_not: [{ exists: { field: 'namespaces' } }], + }, + }; + } + // isNamespaceAgnostic + return { + bool: expect.objectContaining({ + must_not: [{ exists: { field: 'namespace' } }, { exists: { field: 'namespaces' } }], + }), + }; + }; + describe('kueryNode filter clause', () => { const expectResult = (result: Result, expected: any) => { expect(result.query.bool.filter).toEqual(expect.arrayContaining([expected])); @@ -100,13 +141,13 @@ describe('#getQueryParams', () => { describe('`kueryNode` parameter', () => { it('does not include the clause when `kueryNode` is not specified', () => { - const result = getQueryParams({ mappings, registry, kueryNode: undefined }); + const result = getQueryParams({ registry, kueryNode: undefined }); expect(result.query.bool.filter).toHaveLength(1); }); it('includes the specified Kuery clause', () => { const test = (kueryNode: KueryNode) => { - const result = getQueryParams({ mappings, registry, kueryNode }); + const result = getQueryParams({ registry, kueryNode }); const expected = esKuery.toElasticsearchQuery(kueryNode); expect(result.query.bool.filter).toHaveLength(2); expectResult(result, expected); @@ -165,7 +206,6 @@ describe('#getQueryParams', () => { it('does not include the clause when `hasReference` is not specified', () => { const result = getQueryParams({ - mappings, registry, hasReference: undefined, }); @@ -176,7 +216,6 @@ describe('#getQueryParams', () => { it('creates a should clause for specified reference when operator is `OR`', () => { const hasReference = { id: 'foo', type: 'bar' }; const result = getQueryParams({ - mappings, registry, hasReference, hasReferenceOperator: 'OR', @@ -192,7 +231,6 @@ describe('#getQueryParams', () => { it('creates a must clause for specified reference when operator is `AND`', () => { const hasReference = { id: 'foo', type: 'bar' }; const result = getQueryParams({ - mappings, registry, hasReference, hasReferenceOperator: 'AND', @@ -210,7 +248,6 @@ describe('#getQueryParams', () => { { id: 'hello', type: 'dolly' }, ]; const result = getQueryParams({ - mappings, registry, hasReference, hasReferenceOperator: 'OR', @@ -229,7 +266,6 @@ describe('#getQueryParams', () => { { id: 'hello', type: 'dolly' }, ]; const result = getQueryParams({ - mappings, registry, hasReference, hasReferenceOperator: 'AND', @@ -244,7 +280,6 @@ describe('#getQueryParams', () => { it('defaults to `OR` when operator is not specified', () => { const hasReference = { id: 'foo', type: 'bar' }; const result = getQueryParams({ - mappings, registry, hasReference, }); @@ -278,14 +313,13 @@ describe('#getQueryParams', () => { }; it('searches for all known types when `type` is not specified', () => { - const result = getQueryParams({ mappings, registry, type: undefined }); + const result = getQueryParams({ registry, type: undefined }); expectResult(result, ...ALL_TYPES); }); it('searches for specified type/s', () => { const test = (typeOrTypes: string | string[]) => { const result = getQueryParams({ - mappings, registry, type: typeOrTypes, }); @@ -309,18 +343,17 @@ describe('#getQueryParams', () => { const test = (namespaces?: string[]) => { for (const typeOrTypes of ALL_TYPE_SUBSETS) { - const result = getQueryParams({ mappings, registry, type: typeOrTypes, namespaces }); + const result = getQueryParams({ registry, type: typeOrTypes, namespaces }); const types = Array.isArray(typeOrTypes) ? typeOrTypes : [typeOrTypes]; expectResult(result, ...types.map((x) => createTypeClause(x, namespaces))); } // also test with no specified type/s - const result = getQueryParams({ mappings, registry, type: undefined, namespaces }); + const result = getQueryParams({ registry, type: undefined, namespaces }); expectResult(result, ...ALL_TYPES.map((x) => createTypeClause(x, namespaces))); }; it('normalizes and deduplicates provided namespaces', () => { const result = getQueryParams({ - mappings, registry, search: '*', namespaces: ['foo', '*', 'foo', 'bar', 'default'], @@ -360,7 +393,6 @@ describe('#getQueryParams', () => { it('supersedes `type` and `namespaces` parameters', () => { const result = getQueryParams({ - mappings, registry, type: ['pending', 'saved', 'shared', 'global'], namespaces: ['foo', 'bar', 'default'], @@ -381,148 +413,266 @@ describe('#getQueryParams', () => { }); }); - describe('search clause (query.bool.must.simple_query_string)', () => { - const search = 'foo*'; + describe('search clause (query.bool)', () => { + describe('when using simple search (query.bool.must.simple_query_string)', () => { + const search = 'foo'; - const expectResult = (result: Result, sqsClause: any) => { - expect(result.query.bool.must).toEqual([{ simple_query_string: sqsClause }]); - }; + const expectResult = (result: Result, sqsClause: any) => { + expect(result.query.bool.must).toEqual([{ simple_query_string: sqsClause }]); + }; - describe('`search` parameter', () => { - it('does not include clause when `search` is not specified', () => { - const result = getQueryParams({ - mappings, - registry, - search: undefined, + describe('`search` parameter', () => { + it('does not include clause when `search` is not specified', () => { + const result = getQueryParams({ + registry, + search: undefined, + }); + expect(result.query.bool.must).toBeUndefined(); }); - expect(result.query.bool.must).toBeUndefined(); - }); - it('creates a clause with query for specified search', () => { - const result = getQueryParams({ - mappings, - registry, - search, + it('creates a clause with query for specified search', () => { + const result = getQueryParams({ + registry, + search, + }); + expectResult(result, expect.objectContaining({ query: search })); }); - expectResult(result, expect.objectContaining({ query: search })); }); - }); - describe('`searchFields` and `rootSearchFields` parameters', () => { - const getExpectedFields = (searchFields: string[], typeOrTypes: string | string[]) => { - const types = Array.isArray(typeOrTypes) ? typeOrTypes : [typeOrTypes]; - return searchFields.map((x) => types.map((y) => `${y}.${x}`)).flat(); - }; + describe('`searchFields` and `rootSearchFields` parameters', () => { + const getExpectedFields = (searchFields: string[], typeOrTypes: string | string[]) => { + const types = Array.isArray(typeOrTypes) ? typeOrTypes : [typeOrTypes]; + return searchFields.map((x) => types.map((y) => `${y}.${x}`)).flat(); + }; - const test = ({ - searchFields, - rootSearchFields, - }: { - searchFields?: string[]; - rootSearchFields?: string[]; - }) => { - for (const typeOrTypes of ALL_TYPE_SUBSETS) { + const test = ({ + searchFields, + rootSearchFields, + }: { + searchFields?: string[]; + rootSearchFields?: string[]; + }) => { + for (const typeOrTypes of ALL_TYPE_SUBSETS) { + const result = getQueryParams({ + registry, + type: typeOrTypes, + search, + searchFields, + rootSearchFields, + }); + let fields = rootSearchFields || []; + if (searchFields) { + fields = fields.concat(getExpectedFields(searchFields, typeOrTypes)); + } + expectResult(result, expect.objectContaining({ fields })); + } + // also test with no specified type/s const result = getQueryParams({ - mappings, registry, - type: typeOrTypes, + type: undefined, search, searchFields, rootSearchFields, }); let fields = rootSearchFields || []; if (searchFields) { - fields = fields.concat(getExpectedFields(searchFields, typeOrTypes)); + fields = fields.concat(getExpectedFields(searchFields, ALL_TYPES)); } expectResult(result, expect.objectContaining({ fields })); - } - // also test with no specified type/s - const result = getQueryParams({ - mappings, - registry, - type: undefined, - search, - searchFields, - rootSearchFields, + }; + + it('throws an error if a raw search field contains a "." character', () => { + expect(() => + getQueryParams({ + registry, + type: undefined, + search, + searchFields: undefined, + rootSearchFields: ['foo', 'bar.baz'], + }) + ).toThrowErrorMatchingInlineSnapshot( + `"rootSearchFields entry \\"bar.baz\\" is invalid: cannot contain \\".\\" character"` + ); }); - let fields = rootSearchFields || []; - if (searchFields) { - fields = fields.concat(getExpectedFields(searchFields, ALL_TYPES)); - } - expectResult(result, expect.objectContaining({ fields })); - }; - it('throws an error if a raw search field contains a "." character', () => { - expect(() => - getQueryParams({ - mappings, + it('includes lenient flag and all fields when `searchFields` and `rootSearchFields` are not specified', () => { + const result = getQueryParams({ registry, - type: undefined, search, searchFields: undefined, - rootSearchFields: ['foo', 'bar.baz'], - }) - ).toThrowErrorMatchingInlineSnapshot( - `"rootSearchFields entry \\"bar.baz\\" is invalid: cannot contain \\".\\" character"` - ); + rootSearchFields: undefined, + }); + expectResult(result, expect.objectContaining({ lenient: true, fields: ['*'] })); + }); + + it('includes specified search fields for appropriate type/s', () => { + test({ searchFields: ['title'] }); + }); + + it('supports boosting', () => { + test({ searchFields: ['title^3'] }); + }); + + it('supports multiple search fields', () => { + test({ searchFields: ['title, title.raw'] }); + }); + + it('includes specified raw search fields', () => { + test({ rootSearchFields: ['_id'] }); + }); + + it('supports multiple raw search fields', () => { + test({ rootSearchFields: ['_id', 'originId'] }); + }); + + it('supports search fields and raw search fields', () => { + test({ searchFields: ['title'], rootSearchFields: ['_id'] }); + }); }); - it('includes lenient flag and all fields when `searchFields` and `rootSearchFields` are not specified', () => { - const result = getQueryParams({ - mappings, + describe('`defaultSearchOperator` parameter', () => { + it('does not include default_operator when `defaultSearchOperator` is not specified', () => { + const result = getQueryParams({ + registry, + search, + defaultSearchOperator: undefined, + }); + expectResult( + result, + expect.not.objectContaining({ default_operator: expect.anything() }) + ); + }); + + it('includes specified default operator', () => { + const defaultSearchOperator = 'AND'; + const result = getQueryParams({ + registry, + search, + defaultSearchOperator, + }); + expectResult( + result, + expect.objectContaining({ default_operator: defaultSearchOperator }) + ); + }); + }); + }); + + describe('when using prefix search (query.bool.should)', () => { + const searchQuery = 'foo*'; + + const getQueryParamForSearch = ({ + search, + searchFields, + type, + }: { + search?: string; + searchFields?: string[]; + type?: string[]; + }) => + getQueryParams({ registry, search, - searchFields: undefined, - rootSearchFields: undefined, + searchFields, + type, }); - expectResult(result, expect.objectContaining({ lenient: true, fields: ['*'] })); - }); - it('includes specified search fields for appropriate type/s', () => { - test({ searchFields: ['title'] }); - }); + it('uses a `should` clause instead of `must`', () => { + const result = getQueryParamForSearch({ search: searchQuery, searchFields: ['title'] }); - it('supports boosting', () => { - test({ searchFields: ['title^3'] }); + expect(result.query.bool.must).toBeUndefined(); + expect(result.query.bool.should).toEqual(expect.any(Array)); + expect(result.query.bool.should.length).toBeGreaterThanOrEqual(1); + expect(result.query.bool.minimum_should_match).toBe(1); }); - - it('supports multiple search fields', () => { - test({ searchFields: ['title, title.raw'] }); + it('includes the `simple_query_string` in the `should` clauses', () => { + const result = getQueryParamForSearch({ search: searchQuery, searchFields: ['title'] }); + expect(result.query.bool.should[0]).toEqual({ + simple_query_string: expect.objectContaining({ + query: searchQuery, + }), + }); }); - it('includes specified raw search fields', () => { - test({ rootSearchFields: ['_id'] }); + it('adds a should clause for each `searchFields` / `type` tuple', () => { + const result = getQueryParamForSearch({ + search: searchQuery, + searchFields: ['title', 'desc'], + type: ['saved', 'pending'], + }); + const shouldClauses = result.query.bool.should; + + expect(shouldClauses.length).toBe(5); + + const mppClauses = shouldClauses.slice(1); + + expect( + mppClauses.map((clause: any) => Object.keys(clause.match_phrase_prefix)[0]) + ).toEqual(['saved.title', 'pending.title', 'saved.desc', 'pending.desc']); }); - it('supports multiple raw search fields', () => { - test({ rootSearchFields: ['_id', 'originId'] }); + it('uses all registered types when `type` is not provided', () => { + const result = getQueryParamForSearch({ + search: searchQuery, + searchFields: ['title'], + type: undefined, + }); + const shouldClauses = result.query.bool.should; + + expect(shouldClauses.length).toBe(5); + + const mppClauses = shouldClauses.slice(1); + + expect( + mppClauses.map((clause: any) => Object.keys(clause.match_phrase_prefix)[0]) + ).toEqual(['pending.title', 'saved.title', 'shared.title', 'global.title']); }); - it('supports search fields and raw search fields', () => { - test({ searchFields: ['title'], rootSearchFields: ['_id'] }); + it('removes the prefix search wildcard from the query', () => { + const result = getQueryParamForSearch({ + search: searchQuery, + searchFields: ['title'], + type: ['saved'], + }); + const shouldClauses = result.query.bool.should; + const mppClauses = shouldClauses.slice(1); + + expect(mppClauses[0].match_phrase_prefix['saved.title'].query).toEqual('foo'); }); - }); - describe('`defaultSearchOperator` parameter', () => { - it('does not include default_operator when `defaultSearchOperator` is not specified', () => { - const result = getQueryParams({ - mappings, - registry, - search, - defaultSearchOperator: undefined, + it("defaults to the type's default search field when `searchFields` is not specified", () => { + const result = getQueryParamForSearch({ + search: searchQuery, + searchFields: undefined, + type: ['saved', 'global'], }); - expectResult(result, expect.not.objectContaining({ default_operator: expect.anything() })); + const shouldClauses = result.query.bool.should; + + expect(shouldClauses.length).toBe(3); + + const mppClauses = shouldClauses.slice(1); + + expect( + mppClauses.map((clause: any) => Object.keys(clause.match_phrase_prefix)[0]) + ).toEqual(['saved.title', 'global.name']); }); - it('includes specified default operator', () => { - const defaultSearchOperator = 'AND'; - const result = getQueryParams({ - mappings, - registry, - search, - defaultSearchOperator, + it('supports boosting', () => { + const result = getQueryParamForSearch({ + search: searchQuery, + searchFields: ['title^3', 'description'], + type: ['saved'], }); - expectResult(result, expect.objectContaining({ default_operator: defaultSearchOperator })); + const shouldClauses = result.query.bool.should; + + expect(shouldClauses.length).toBe(3); + + const mppClauses = shouldClauses.slice(1); + + expect(mppClauses.map((clause: any) => clause.match_phrase_prefix)).toEqual([ + { 'saved.title': { query: 'foo', boost: 3 } }, + { 'saved.description': { query: 'foo', boost: 1 } }, + ]); }); }); }); @@ -532,7 +682,6 @@ describe('#getQueryParams', () => { it(`throws for ${type} when namespaces is an empty array`, () => { expect(() => getQueryParams({ - mappings, registry, namespaces: [], }) diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts index 8d4fe13b9bede..f73777c4f454f 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts @@ -20,7 +20,6 @@ import { esKuery } from '../../../es_query'; type KueryNode = any; -import { getRootPropertiesObjects, IndexMapping } from '../../../mappings'; import { ISavedObjectTypeRegistry } from '../../../saved_objects_type_registry'; import { ALL_NAMESPACES_STRING, DEFAULT_NAMESPACE_STRING } from '../utils'; @@ -28,22 +27,17 @@ import { ALL_NAMESPACES_STRING, DEFAULT_NAMESPACE_STRING } from '../utils'; * Gets the types based on the type. Uses mappings to support * null type (all types), a single type string or an array */ -function getTypes(mappings: IndexMapping, type?: string | string[]) { +function getTypes(registry: ISavedObjectTypeRegistry, type?: string | string[]) { if (!type) { - return Object.keys(getRootPropertiesObjects(mappings)); + return registry.getAllTypes().map((registeredType) => registeredType.name); } - - if (Array.isArray(type)) { - return type; - } - - return [type]; + return Array.isArray(type) ? type : [type]; } /** * Get the field params based on the types, searchFields, and rootSearchFields */ -function getFieldsForTypes( +function getSimpleQueryStringTypeFields( types: string[], searchFields: string[] = [], rootSearchFields: string[] = [] @@ -130,7 +124,6 @@ export interface HasReferenceQueryParams { export type SearchOperator = 'AND' | 'OR'; interface QueryParams { - mappings: IndexMapping; registry: ISavedObjectTypeRegistry; namespaces?: string[]; type?: string | string[]; @@ -188,11 +181,26 @@ export function getClauseForReference(reference: HasReferenceQueryParams) { }; } +// A de-duplicated set of namespaces makes for a more efficient query. +// +// Additionally, we treat the `*` namespace as the `default` namespace. +// In the Default Distribution, the `*` is automatically expanded to include all available namespaces. +// However, the OSS distribution (and certain configurations of the Default Distribution) can allow the `*` +// to pass through to the SO Repository, and eventually to this module. When this happens, we translate to `default`, +// since that is consistent with how a single-namespace search behaves in the OSS distribution. Leaving the wildcard in place +// would result in no results being returned, as the wildcard is treated as a literal, and not _actually_ as a wildcard. +// We had a good discussion around the tradeoffs here: https://github.com/elastic/kibana/pull/67644#discussion_r441055716 +const normalizeNamespaces = (namespacesToNormalize?: string[]) => + namespacesToNormalize + ? Array.from( + new Set(namespacesToNormalize.map((x) => (x === '*' ? DEFAULT_NAMESPACE_STRING : x))) + ) + : undefined; + /** * Get the "query" related keys for the search body */ export function getQueryParams({ - mappings, registry, namespaces, type, @@ -206,7 +214,7 @@ export function getQueryParams({ kueryNode, }: QueryParams) { const types = getTypes( - mappings, + registry, typeToNamespacesMap ? Array.from(typeToNamespacesMap.keys()) : type ); @@ -214,28 +222,10 @@ export function getQueryParams({ hasReference = [hasReference]; } - // A de-duplicated set of namespaces makes for a more effecient query. - // - // Additonally, we treat the `*` namespace as the `default` namespace. - // In the Default Distribution, the `*` is automatically expanded to include all available namespaces. - // However, the OSS distribution (and certain configurations of the Default Distribution) can allow the `*` - // to pass through to the SO Repository, and eventually to this module. When this happens, we translate to `default`, - // since that is consistent with how a single-namespace search behaves in the OSS distribution. Leaving the wildcard in place - // would result in no results being returned, as the wildcard is treated as a literal, and not _actually_ as a wildcard. - // We had a good discussion around the tradeoffs here: https://github.com/elastic/kibana/pull/67644#discussion_r441055716 - const normalizeNamespaces = (namespacesToNormalize?: string[]) => - namespacesToNormalize - ? Array.from( - new Set(namespacesToNormalize.map((x) => (x === '*' ? DEFAULT_NAMESPACE_STRING : x))) - ) - : undefined; - const bool: any = { filter: [ ...(kueryNode != null ? [esKuery.toElasticsearchQuery(kueryNode)] : []), - ...(hasReference && hasReference.length - ? [getReferencesFilter(hasReference, hasReferenceOperator)] - : []), + ...(hasReference?.length ? [getReferencesFilter(hasReference, hasReferenceOperator)] : []), { bool: { should: types.map((shouldType) => { @@ -251,16 +241,133 @@ export function getQueryParams({ }; if (search) { - bool.must = [ - { - simple_query_string: { - query: search, - ...getFieldsForTypes(types, searchFields, rootSearchFields), - ...(defaultSearchOperator ? { default_operator: defaultSearchOperator } : {}), - }, - }, - ]; + const useMatchPhrasePrefix = shouldUseMatchPhrasePrefix(search); + const simpleQueryStringClause = getSimpleQueryStringClause({ + search, + types, + searchFields, + rootSearchFields, + defaultSearchOperator, + }); + + if (useMatchPhrasePrefix) { + bool.should = [ + simpleQueryStringClause, + ...getMatchPhrasePrefixClauses({ search, searchFields, types, registry }), + ]; + bool.minimum_should_match = 1; + } else { + bool.must = [simpleQueryStringClause]; + } } return { query: { bool } }; } + +// we only want to add match_phrase_prefix clauses +// if the search is a prefix search +const shouldUseMatchPhrasePrefix = (search: string): boolean => { + return search.trim().endsWith('*'); +}; + +const getMatchPhrasePrefixClauses = ({ + search, + searchFields, + registry, + types, +}: { + search: string; + searchFields?: string[]; + types: string[]; + registry: ISavedObjectTypeRegistry; +}) => { + // need to remove the prefix search operator + const query = search.replace(/[*]$/, ''); + const mppFields = getMatchPhrasePrefixFields({ searchFields, types, registry }); + return mppFields.map(({ field, boost }) => { + return { + match_phrase_prefix: { + [field]: { + query, + boost, + }, + }, + }; + }); +}; + +interface FieldWithBoost { + field: string; + boost?: number; +} + +const getMatchPhrasePrefixFields = ({ + searchFields = [], + types, + registry, +}: { + searchFields?: string[]; + types: string[]; + registry: ISavedObjectTypeRegistry; +}): FieldWithBoost[] => { + const output: FieldWithBoost[] = []; + + searchFields = searchFields.filter((field) => field !== '*'); + let fields: string[]; + if (searchFields.length === 0) { + fields = types.reduce((typeFields, type) => { + const defaultSearchField = registry.getType(type)?.management?.defaultSearchField; + if (defaultSearchField) { + return [...typeFields, `${type}.${defaultSearchField}`]; + } + return typeFields; + }, [] as string[]); + } else { + fields = []; + for (const field of searchFields) { + fields = fields.concat(types.map((type) => `${type}.${field}`)); + } + } + + fields.forEach((rawField) => { + const [field, rawBoost] = rawField.split('^'); + let boost: number = 1; + if (rawBoost) { + try { + boost = parseInt(rawBoost, 10); + } catch (e) { + boost = 1; + } + } + if (isNaN(boost)) { + boost = 1; + } + output.push({ + field, + boost, + }); + }); + return output; +}; + +const getSimpleQueryStringClause = ({ + search, + types, + searchFields, + rootSearchFields, + defaultSearchOperator, +}: { + search: string; + types: string[]; + searchFields?: string[]; + rootSearchFields?: string[]; + defaultSearchOperator?: SearchOperator; +}) => { + return { + simple_query_string: { + query: search, + ...getSimpleQueryStringTypeFields(types, searchFields, rootSearchFields), + ...(defaultSearchOperator ? { default_operator: defaultSearchOperator } : {}), + }, + }; +}; diff --git a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.test.ts b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.test.ts index a9f26f71a3f2b..3522ab9ef1736 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.test.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.test.ts @@ -76,7 +76,6 @@ describe('getSearchDsl', () => { getSearchDsl(mappings, registry, opts); expect(getQueryParams).toHaveBeenCalledTimes(1); expect(getQueryParams).toHaveBeenCalledWith({ - mappings, registry, namespaces: opts.namespaces, type: opts.type, diff --git a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts index d5da82e5617be..bddecc4d7f649 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts @@ -71,7 +71,6 @@ export function getSearchDsl( return { ...getQueryParams({ - mappings, registry, namespaces, type, diff --git a/src/plugins/vis_type_timeseries/common/types.ts b/src/plugins/vis_type_timeseries/common/types.ts index 4520069244527..8973060848b41 100644 --- a/src/plugins/vis_type_timeseries/common/types.ts +++ b/src/plugins/vis_type_timeseries/common/types.ts @@ -18,8 +18,9 @@ */ import { TypeOf } from '@kbn/config-schema'; -import { metricsItems, panel, seriesItems } from './vis_schema'; +import { metricsItems, panel, seriesItems, visPayloadSchema } from './vis_schema'; export type SeriesItemsSchema = TypeOf; export type MetricsItemsSchema = TypeOf; export type PanelSchema = TypeOf; +export type VisPayload = TypeOf; diff --git a/src/plugins/vis_type_timeseries/common/vis_schema.ts b/src/plugins/vis_type_timeseries/common/vis_schema.ts index 40f776050617e..27f09fb574b0f 100644 --- a/src/plugins/vis_type_timeseries/common/vis_schema.ts +++ b/src/plugins/vis_type_timeseries/common/vis_schema.ts @@ -273,4 +273,5 @@ export const visPayloadSchema = schema.object({ min: stringRequired, max: stringRequired, }), + sessionId: schema.maybe(schema.string()), }); diff --git a/src/plugins/vis_type_timeseries/public/request_handler.js b/src/plugins/vis_type_timeseries/public/request_handler.js index e33d0e254f609..12b7f3d417ef6 100644 --- a/src/plugins/vis_type_timeseries/public/request_handler.js +++ b/src/plugins/vis_type_timeseries/public/request_handler.js @@ -32,7 +32,8 @@ export const metricsRequestHandler = async ({ const config = getUISettings(); const timezone = getTimezone(config); const uiStateObj = uiState.get(visParams.type, {}); - const parsedTimeRange = getDataStart().query.timefilter.timefilter.calculateBounds(timeRange); + const dataSearch = getDataStart(); + const parsedTimeRange = dataSearch.query.timefilter.timefilter.calculateBounds(timeRange); const scaledDataFormat = config.get('dateFormat:scaled'); const dateFormat = config.get('dateFormat'); @@ -53,6 +54,7 @@ export const metricsRequestHandler = async ({ panels: [visParams], state: uiStateObj, savedObjectId: savedObjectId || 'unsaved', + sessionId: dataSearch.search.session.getSessionId(), }), }); diff --git a/src/plugins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.test.js b/src/plugins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.test.js index 9710f7daf69b6..2c38e883cd69f 100644 --- a/src/plugins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.test.js +++ b/src/plugins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.test.js @@ -28,6 +28,7 @@ describe('AbstractSearchStrategy', () => { beforeEach(() => { mockedFields = {}; req = { + payload: {}, pre: { indexPatternsService: { getFieldsForWildcard: jest.fn().mockReturnValue(mockedFields), @@ -60,6 +61,9 @@ describe('AbstractSearchStrategy', () => { const responses = await abstractSearchStrategy.search( { + payload: { + sessionId: 1, + }, requestContext: { search: { search: searchFn }, }, @@ -76,7 +80,9 @@ describe('AbstractSearchStrategy', () => { }, indexType: undefined, }, - {} + { + sessionId: 1, + } ); }); }); diff --git a/src/plugins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.ts b/src/plugins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.ts index eb22fcb1dd689..b1e21edf8b588 100644 --- a/src/plugins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.ts +++ b/src/plugins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.ts @@ -23,8 +23,10 @@ import { IUiSettingsClient, SavedObjectsClientContract, } from 'kibana/server'; + import { Framework } from '../../../plugin'; import { IndexPatternsFetcher } from '../../../../../data/server'; +import { VisPayload } from '../../../../common/types'; /** * ReqFacade is a regular KibanaRequest object extended with additional service @@ -32,17 +34,17 @@ import { IndexPatternsFetcher } from '../../../../../data/server'; * * This will be replaced by standard KibanaRequest and RequestContext objects in a later version. */ -export type ReqFacade = FakeRequest & { +export interface ReqFacade extends FakeRequest { requestContext: RequestHandlerContext; framework: Framework; - payload: unknown; + payload: T; pre: { indexPatternsService?: IndexPatternsFetcher; }; getUiSettingsService: () => IUiSettingsClient; getSavedObjectsClient: () => SavedObjectsClientContract; getEsShardTimeout: () => Promise; -}; +} export class AbstractSearchStrategy { public indexType?: string; @@ -53,8 +55,10 @@ export class AbstractSearchStrategy { this.additionalParams = additionalParams; } - async search(req: ReqFacade, bodies: any[], options = {}) { + async search(req: ReqFacade, bodies: any[], options = {}) { const requests: any[] = []; + const { sessionId } = req.payload; + bodies.forEach((body) => { requests.push( req.requestContext @@ -67,6 +71,7 @@ export class AbstractSearchStrategy { indexType: this.indexType, }, { + sessionId, ...options, } ) diff --git a/test/api_integration/apis/saved_objects/find.js b/test/api_integration/apis/saved_objects/find.js index c2e36b4a669ff..e5da46644672b 100644 --- a/test/api_integration/apis/saved_objects/find.js +++ b/test/api_integration/apis/saved_objects/find.js @@ -334,6 +334,70 @@ export default function ({ getService }) { }); }); + describe('searching for special characters', () => { + before(() => esArchiver.load('saved_objects/find_edgecases')); + after(() => esArchiver.unload('saved_objects/find_edgecases')); + + it('can search for objects with dashes', async () => + await supertest + .get('/api/saved_objects/_find') + .query({ + type: 'visualization', + search_fields: 'title', + search: 'my-vis*', + }) + .expect(200) + .then((resp) => { + const savedObjects = resp.body.saved_objects; + expect(savedObjects.map((so) => so.attributes.title)).to.eql(['my-visualization']); + })); + + it('can search with the prefix search character just after a special one', async () => + await supertest + .get('/api/saved_objects/_find') + .query({ + type: 'visualization', + search_fields: 'title', + search: 'my-*', + }) + .expect(200) + .then((resp) => { + const savedObjects = resp.body.saved_objects; + expect(savedObjects.map((so) => so.attributes.title)).to.eql(['my-visualization']); + })); + + it('can search for objects with asterisk', async () => + await supertest + .get('/api/saved_objects/_find') + .query({ + type: 'visualization', + search_fields: 'title', + search: 'some*vi*', + }) + .expect(200) + .then((resp) => { + const savedObjects = resp.body.saved_objects; + expect(savedObjects.map((so) => so.attributes.title)).to.eql(['some*visualization']); + })); + + it('can still search tokens by prefix', async () => + await supertest + .get('/api/saved_objects/_find') + .query({ + type: 'visualization', + search_fields: 'title', + search: 'visuali*', + }) + .expect(200) + .then((resp) => { + const savedObjects = resp.body.saved_objects; + expect(savedObjects.map((so) => so.attributes.title)).to.eql([ + 'my-visualization', + 'some*visualization', + ]); + })); + }); + describe('without kibana index', () => { before( async () => diff --git a/test/api_integration/fixtures/es_archiver/saved_objects/find_edgecases/data.json b/test/api_integration/fixtures/es_archiver/saved_objects/find_edgecases/data.json new file mode 100644 index 0000000000000..0c8b35fd3f499 --- /dev/null +++ b/test/api_integration/fixtures/es_archiver/saved_objects/find_edgecases/data.json @@ -0,0 +1,93 @@ +{ + "type": "doc", + "value": { + "index": ".kibana", + "id": "visualization:title-with-dash", + "source": { + "type": "visualization", + "updated_at": "2017-09-21T18:51:23.794Z", + "visualization": { + "title": "my-visualization", + "visState": "{}", + "uiStateJSON": "{\"spy\":{\"mode\":{\"name\":null,\"fill\":false}}}", + "description": "", + "version": 1, + "kibanaSavedObjectMeta": { + "searchSourceJSON": "{\"filter\":[],\"query\":{\"query\":\"\",\"language\":\"lucene\"}}" + } + }, + "references": [] + } + } +} + +{ + "type": "doc", + "value": { + "index": ".kibana", + "id": "visualization:title-with-asterisk", + "source": { + "type": "visualization", + "updated_at": "2017-09-21T18:51:23.794Z", + "visualization": { + "title": "some*visualization", + "visState": "{}", + "uiStateJSON": "{\"spy\":{\"mode\":{\"name\":null,\"fill\":false}}}", + "description": "", + "version": 1, + "kibanaSavedObjectMeta": { + "searchSourceJSON": "{\"filter\":[],\"query\":{\"query\":\"\",\"language\":\"lucene\"}}" + } + }, + "references": [] + } + } +} + + +{ + "type": "doc", + "value": { + "index": ".kibana", + "id": "visualization:noise-1", + "source": { + "type": "visualization", + "updated_at": "2017-09-21T18:51:23.794Z", + "visualization": { + "title": "Just some noise in the dataset", + "visState": "{}", + "uiStateJSON": "{\"spy\":{\"mode\":{\"name\":null,\"fill\":false}}}", + "description": "", + "version": 1, + "kibanaSavedObjectMeta": { + "searchSourceJSON": "{\"filter\":[],\"query\":{\"query\":\"\",\"language\":\"lucene\"}}" + } + }, + "references": [] + } + } +} + +{ + "type": "doc", + "value": { + "index": ".kibana", + "id": "visualization:noise-2", + "source": { + "type": "visualization", + "updated_at": "2017-09-21T18:51:23.794Z", + "visualization": { + "title": "Just some noise in the dataset", + "visState": "{}", + "uiStateJSON": "{\"spy\":{\"mode\":{\"name\":null,\"fill\":false}}}", + "description": "", + "version": 1, + "kibanaSavedObjectMeta": { + "searchSourceJSON": "{\"filter\":[],\"query\":{\"query\":\"\",\"language\":\"lucene\"}}" + } + }, + "references": [] + } + } +} + diff --git a/test/api_integration/fixtures/es_archiver/saved_objects/find_edgecases/mappings.json b/test/api_integration/fixtures/es_archiver/saved_objects/find_edgecases/mappings.json new file mode 100644 index 0000000000000..e601c43431437 --- /dev/null +++ b/test/api_integration/fixtures/es_archiver/saved_objects/find_edgecases/mappings.json @@ -0,0 +1,267 @@ +{ + "type": "index", + "value": { + "index": ".kibana", + "settings": { + "index": { + "number_of_shards": "1", + "number_of_replicas": "1" + } + }, + "mappings": { + "dynamic": "strict", + "properties": { + "config": { + "dynamic": "true", + "properties": { + "buildNum": { + "type": "keyword" + }, + "defaultIndex": { + "type": "text", + "fields": { + "keyword": { + "type": "keyword", + "ignore_above": 256 + } + } + } + } + }, + "dashboard": { + "properties": { + "description": { + "type": "text" + }, + "hits": { + "type": "integer" + }, + "kibanaSavedObjectMeta": { + "properties": { + "searchSourceJSON": { + "type": "text" + } + } + }, + "optionsJSON": { + "type": "text" + }, + "panelsJSON": { + "type": "text" + }, + "refreshInterval": { + "properties": { + "display": { + "type": "keyword" + }, + "pause": { + "type": "boolean" + }, + "section": { + "type": "integer" + }, + "value": { + "type": "integer" + } + } + }, + "timeFrom": { + "type": "keyword" + }, + "timeRestore": { + "type": "boolean" + }, + "timeTo": { + "type": "keyword" + }, + "title": { + "type": "text" + }, + "uiStateJSON": { + "type": "text" + }, + "version": { + "type": "integer" + } + } + }, + "index-pattern": { + "properties": { + "fieldFormatMap": { + "type": "text" + }, + "fields": { + "type": "text" + }, + "intervalName": { + "type": "keyword" + }, + "notExpandable": { + "type": "boolean" + }, + "sourceFilters": { + "type": "text" + }, + "timeFieldName": { + "type": "keyword" + }, + "title": { + "type": "text" + } + } + }, + "search": { + "properties": { + "columns": { + "type": "keyword" + }, + "description": { + "type": "text" + }, + "hits": { + "type": "integer" + }, + "kibanaSavedObjectMeta": { + "properties": { + "searchSourceJSON": { + "type": "text" + } + } + }, + "sort": { + "type": "keyword" + }, + "title": { + "type": "text" + }, + "version": { + "type": "integer" + } + } + }, + "server": { + "properties": { + "uuid": { + "type": "keyword" + } + } + }, + "timelion-sheet": { + "properties": { + "description": { + "type": "text" + }, + "hits": { + "type": "integer" + }, + "kibanaSavedObjectMeta": { + "properties": { + "searchSourceJSON": { + "type": "text" + } + } + }, + "timelion_chart_height": { + "type": "integer" + }, + "timelion_columns": { + "type": "integer" + }, + "timelion_interval": { + "type": "keyword" + }, + "timelion_other_interval": { + "type": "keyword" + }, + "timelion_rows": { + "type": "integer" + }, + "timelion_sheet": { + "type": "text" + }, + "title": { + "type": "text" + }, + "version": { + "type": "integer" + } + } + }, + "namespace": { + "type": "keyword" + }, + "references": { + "properties": { + "id": { + "type": "keyword" + }, + "name": { + "type": "keyword" + }, + "type": { + "type": "keyword" + } + }, + "type": "nested" + }, + "type": { + "type": "keyword" + }, + "updated_at": { + "type": "date" + }, + "url": { + "properties": { + "accessCount": { + "type": "long" + }, + "accessDate": { + "type": "date" + }, + "createDate": { + "type": "date" + }, + "url": { + "type": "text", + "fields": { + "keyword": { + "type": "keyword", + "ignore_above": 2048 + } + } + } + } + }, + "visualization": { + "properties": { + "description": { + "type": "text" + }, + "kibanaSavedObjectMeta": { + "properties": { + "searchSourceJSON": { + "type": "text" + } + } + }, + "savedSearchId": { + "type": "keyword" + }, + "title": { + "type": "text" + }, + "uiStateJSON": { + "type": "text" + }, + "version": { + "type": "integer" + }, + "visState": { + "type": "text" + } + } + } + } + } + } +} diff --git a/x-pack/examples/alerting_example/public/alert_types/astros.tsx b/x-pack/examples/alerting_example/public/alert_types/astros.tsx index 73c7dfea1263b..54f989b93e22f 100644 --- a/x-pack/examples/alerting_example/public/alert_types/astros.tsx +++ b/x-pack/examples/alerting_example/public/alert_types/astros.tsx @@ -127,9 +127,9 @@ export const PeopleinSpaceExpression: React.FunctionComponent - errs.map((e) => ( -

+ Object.entries(errors).map(([field, errs]: [string, string[]], fieldIndex) => + errs.map((e, index) => ( +

{field}:`: ${errs}`

)) diff --git a/x-pack/examples/alerting_example/server/alert_types/always_firing.ts b/x-pack/examples/alerting_example/server/alert_types/always_firing.ts index bb1cb0d97689b..d02406a23045e 100644 --- a/x-pack/examples/alerting_example/server/alert_types/always_firing.ts +++ b/x-pack/examples/alerting_example/server/alert_types/always_firing.ts @@ -5,25 +5,31 @@ */ import uuid from 'uuid'; -import { range } from 'lodash'; +import { range, random } from 'lodash'; import { AlertType } from '../../../../plugins/alerts/server'; import { DEFAULT_INSTANCES_TO_GENERATE, ALERTING_EXAMPLE_APP_ID } from '../../common/constants'; +const ACTION_GROUPS = [ + { id: 'small', name: 'small' }, + { id: 'medium', name: 'medium' }, + { id: 'large', name: 'large' }, +]; + export const alertType: AlertType = { id: 'example.always-firing', name: 'Always firing', - actionGroups: [{ id: 'default', name: 'default' }], - defaultActionGroupId: 'default', + actionGroups: ACTION_GROUPS, + defaultActionGroupId: 'small', async executor({ services, params: { instances = DEFAULT_INSTANCES_TO_GENERATE }, state }) { const count = (state.count ?? 0) + 1; range(instances) - .map(() => ({ id: uuid.v4() })) - .forEach((instance: { id: string }) => { + .map(() => ({ id: uuid.v4(), tshirtSize: ACTION_GROUPS[random(0, 2)].id! })) + .forEach((instance: { id: string; tshirtSize: string }) => { services .alertInstanceFactory(instance.id) .replaceState({ triggerdOnCycle: count }) - .scheduleActions('default'); + .scheduleActions(instance.tshirtSize); }); return { diff --git a/x-pack/plugins/apm/server/lib/helpers/create_es_client/create_apm_event_client/index.ts b/x-pack/plugins/apm/server/lib/helpers/create_es_client/create_apm_event_client/index.ts index 2bfd3c94ed34c..9020cb1b9953a 100644 --- a/x-pack/plugins/apm/server/lib/helpers/create_es_client/create_apm_event_client/index.ts +++ b/x-pack/plugins/apm/server/lib/helpers/create_es_client/create_apm_event_client/index.ts @@ -7,14 +7,16 @@ import { ValuesType } from 'utility-types'; import { APMBaseDoc } from '../../../../../typings/es_schemas/raw/apm_base_doc'; import { APMError } from '../../../../../typings/es_schemas/ui/apm_error'; -import { KibanaRequest } from '../../../../../../../../src/core/server'; +import { + KibanaRequest, + LegacyScopedClusterClient, +} from '../../../../../../../../src/core/server'; import { ProcessorEvent } from '../../../../../common/processor_event'; import { ESSearchRequest, ESSearchResponse, } from '../../../../../typings/elasticsearch'; import { ApmIndicesConfig } from '../../../settings/apm_indices/get_apm_indices'; -import { APMRequestHandlerContext } from '../../../../routes/typings'; import { addFilterToExcludeLegacyData } from './add_filter_to_exclude_legacy_data'; import { callClientWithDebug } from '../call_client_with_debug'; import { Transaction } from '../../../../../typings/es_schemas/ui/transaction'; @@ -51,20 +53,23 @@ type TypedSearchResponse< export type APMEventClient = ReturnType; export function createApmEventClient({ - context, + esClient, + debug, request, indices, options: { includeFrozen } = { includeFrozen: false }, }: { - context: APMRequestHandlerContext; + esClient: Pick< + LegacyScopedClusterClient, + 'callAsInternalUser' | 'callAsCurrentUser' + >; + debug: boolean; request: KibanaRequest; indices: ApmIndicesConfig; options: { includeFrozen: boolean; }; }) { - const client = context.core.elasticsearch.legacy.client; - return { search( params: TParams, @@ -77,14 +82,14 @@ export function createApmEventClient({ : withProcessorEventFilter; return callClientWithDebug({ - apiCaller: client.callAsCurrentUser, + apiCaller: esClient.callAsCurrentUser, operationName: 'search', params: { ...withPossibleLegacyDataFilter, ignore_throttled: !includeFrozen, }, request, - debug: context.params.query._debug, + debug, }); }, }; diff --git a/x-pack/plugins/apm/server/lib/helpers/setup_request.ts b/x-pack/plugins/apm/server/lib/helpers/setup_request.ts index 5e75535c678b3..363c4128137e0 100644 --- a/x-pack/plugins/apm/server/lib/helpers/setup_request.ts +++ b/x-pack/plugins/apm/server/lib/helpers/setup_request.ts @@ -88,7 +88,8 @@ export async function setupRequest( const coreSetupRequest = { indices, apmEventClient: createApmEventClient({ - context, + esClient: context.core.elasticsearch.legacy.client, + debug: context.params.query._debug, request, indices, options: { includeFrozen }, diff --git a/x-pack/plugins/apm/server/plugin.ts b/x-pack/plugins/apm/server/plugin.ts index d3341b6c1b163..44269b1775953 100644 --- a/x-pack/plugins/apm/server/plugin.ts +++ b/x-pack/plugins/apm/server/plugin.ts @@ -10,14 +10,17 @@ import { map, take } from 'rxjs/operators'; import { CoreSetup, CoreStart, + KibanaRequest, Logger, Plugin, PluginInitializerContext, + RequestHandlerContext, } from 'src/core/server'; import { APMConfig, APMXPackConfig, mergeConfigs } from '.'; import { APMOSSPluginSetup } from '../../../../src/plugins/apm_oss/server'; import { HomeServerPluginSetup } from '../../../../src/plugins/home/server'; import { UsageCollectionSetup } from '../../../../src/plugins/usage_collection/server'; +import { UI_SETTINGS } from '../../../../src/plugins/data/common'; import { ActionsPlugin } from '../../actions/server'; import { AlertingPlugin } from '../../alerts/server'; import { CloudSetup } from '../../cloud/server'; @@ -30,6 +33,7 @@ import { TaskManagerSetupContract } from '../../task_manager/server'; import { APM_FEATURE, registerFeaturesUsage } from './feature'; import { registerApmAlerts } from './lib/alerts/register_apm_alerts'; import { createApmTelemetry } from './lib/apm_telemetry'; +import { createApmEventClient } from './lib/helpers/create_es_client/create_apm_event_client'; import { getInternalSavedObjectsClient } from './lib/helpers/get_internal_saved_objects_client'; import { createApmAgentConfigurationIndex } from './lib/settings/agent_configuration/create_agent_config_index'; import { getApmIndices } from './lib/settings/apm_indices/get_apm_indices'; @@ -42,6 +46,11 @@ import { uiSettings } from './ui_settings'; export interface APMPluginSetup { config$: Observable; getApmIndices: () => ReturnType; + createApmEventClient: (params: { + debug?: boolean; + request: KibanaRequest; + context: RequestHandlerContext; + }) => Promise>; } export class APMPlugin implements Plugin { @@ -141,13 +150,41 @@ export class APMPlugin implements Plugin { }, }); + const boundGetApmIndices = async () => + getApmIndices({ + savedObjectsClient: await getInternalSavedObjectsClient(core), + config: await mergedConfig$.pipe(take(1)).toPromise(), + }); + return { config$: mergedConfig$, - getApmIndices: async () => - getApmIndices({ - savedObjectsClient: await getInternalSavedObjectsClient(core), - config: await mergedConfig$.pipe(take(1)).toPromise(), - }), + getApmIndices: boundGetApmIndices, + createApmEventClient: async ({ + request, + context, + debug, + }: { + debug?: boolean; + request: KibanaRequest; + context: RequestHandlerContext; + }) => { + const [indices, includeFrozen] = await Promise.all([ + boundGetApmIndices(), + context.core.uiSettings.client.get(UI_SETTINGS.SEARCH_INCLUDE_FROZEN), + ]); + + const esClient = context.core.elasticsearch.legacy.client; + + return createApmEventClient({ + debug: debug ?? false, + esClient, + request, + indices, + options: { + includeFrozen, + }, + }); + }, }; } diff --git a/x-pack/plugins/infra/public/containers/logs/log_entries/index.ts b/x-pack/plugins/infra/public/containers/logs/log_entries/index.ts index 4c8c610794b2e..214bb16b24283 100644 --- a/x-pack/plugins/infra/public/containers/logs/log_entries/index.ts +++ b/x-pack/plugins/infra/public/containers/logs/log_entries/index.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ import { useEffect, useState, useReducer, useCallback } from 'react'; +import { useMountedState } from 'react-use'; import createContainer from 'constate'; import { pick, throttle } from 'lodash'; import { TimeKey, timeKeyIsBetween } from '../../../../common/time'; @@ -146,15 +147,20 @@ const useFetchEntriesEffect = ( props: LogEntriesProps ) => { const { services } = useKibanaContextForPlugin(); + const isMounted = useMountedState(); const [prevParams, cachePrevParams] = useState(); const [startedStreaming, setStartedStreaming] = useState(false); + const dispatchIfMounted = useCallback((action) => (isMounted() ? dispatch(action) : undefined), [ + dispatch, + isMounted, + ]); const runFetchNewEntriesRequest = async (overrides: Partial = {}) => { if (!props.startTimestamp || !props.endTimestamp) { return; } - dispatch({ type: Action.FetchingNewEntries }); + dispatchIfMounted({ type: Action.FetchingNewEntries }); try { const commonFetchArgs: LogEntriesBaseRequest = { @@ -175,13 +181,15 @@ const useFetchEntriesEffect = ( }; const { data: payload } = await fetchLogEntries(fetchArgs, services.http.fetch); - dispatch({ type: Action.ReceiveNewEntries, payload }); + dispatchIfMounted({ type: Action.ReceiveNewEntries, payload }); // Move position to the bottom if it's the first load. // Do it in the next tick to allow the `dispatch` to fire if (!props.timeKey && payload.bottomCursor) { setTimeout(() => { - props.jumpToTargetPosition(payload.bottomCursor!); + if (isMounted()) { + props.jumpToTargetPosition(payload.bottomCursor!); + } }); } else if ( props.timeKey && @@ -192,7 +200,7 @@ const useFetchEntriesEffect = ( props.jumpToTargetPosition(payload.topCursor); } } catch (e) { - dispatch({ type: Action.ErrorOnNewEntries }); + dispatchIfMounted({ type: Action.ErrorOnNewEntries }); } }; @@ -210,7 +218,7 @@ const useFetchEntriesEffect = ( return; } - dispatch({ type: Action.FetchingMoreEntries }); + dispatchIfMounted({ type: Action.FetchingMoreEntries }); try { const commonFetchArgs: LogEntriesBaseRequest = { @@ -232,14 +240,14 @@ const useFetchEntriesEffect = ( const { data: payload } = await fetchLogEntries(fetchArgs, services.http.fetch); - dispatch({ + dispatchIfMounted({ type: getEntriesBefore ? Action.ReceiveEntriesBefore : Action.ReceiveEntriesAfter, payload, }); return payload.bottomCursor; } catch (e) { - dispatch({ type: Action.ErrorOnMoreEntries }); + dispatchIfMounted({ type: Action.ErrorOnMoreEntries }); } }; @@ -322,7 +330,7 @@ const useFetchEntriesEffect = ( after: props.endTimestamp > prevParams.endTimestamp, }; - dispatch({ type: Action.ExpandRange, payload: shouldExpand }); + dispatchIfMounted({ type: Action.ExpandRange, payload: shouldExpand }); }; const expandRangeEffectDependencies = [ diff --git a/x-pack/plugins/infra/public/utils/use_tracked_promise.ts b/x-pack/plugins/infra/public/utils/use_tracked_promise.ts index 9951b62fa64a3..42518127f68bf 100644 --- a/x-pack/plugins/infra/public/utils/use_tracked_promise.ts +++ b/x-pack/plugins/infra/public/utils/use_tracked_promise.ts @@ -6,13 +6,15 @@ /* eslint-disable max-classes-per-file */ -import { DependencyList, useEffect, useMemo, useRef, useState } from 'react'; +import { DependencyList, useEffect, useMemo, useRef, useState, useCallback } from 'react'; +import { useMountedState } from 'react-use'; interface UseTrackedPromiseArgs { createPromise: (...args: Arguments) => Promise; onResolve?: (result: Result) => void; onReject?: (value: unknown) => void; cancelPreviousOn?: 'creation' | 'settlement' | 'resolution' | 'rejection' | 'never'; + triggerOrThrow?: 'always' | 'whenMounted'; } /** @@ -64,6 +66,16 @@ interface UseTrackedPromiseArgs { * The last argument is a normal React hook dependency list that indicates * under which conditions a new reference to the configuration object should be * used. + * + * The `onResolve`, `onReject` and possible uncatched errors are only triggered + * if the underlying component is mounted. To ensure they always trigger (i.e. + * if the promise is called in a `useLayoutEffect`) use the `triggerOrThrow` + * attribute: + * + * 'whenMounted': (default) they are called only if the component is mounted. + * + * 'always': they always call. The consumer is then responsible of ensuring no + * side effects happen if the underlying component is not mounted. */ export const useTrackedPromise = ( { @@ -71,9 +83,20 @@ export const useTrackedPromise = ( onResolve = noOp, onReject = noOp, cancelPreviousOn = 'never', + triggerOrThrow = 'whenMounted', }: UseTrackedPromiseArgs, dependencies: DependencyList ) => { + const isComponentMounted = useMountedState(); + const shouldTriggerOrThrow = useCallback(() => { + switch (triggerOrThrow) { + case 'always': + return true; + case 'whenMounted': + return isComponentMounted(); + } + }, [isComponentMounted, triggerOrThrow]); + /** * If a promise is currently pending, this holds a reference to it and its * cancellation function. @@ -144,7 +167,7 @@ export const useTrackedPromise = ( (pendingPromise) => pendingPromise.promise !== newPendingPromise.promise ); - if (onResolve) { + if (onResolve && shouldTriggerOrThrow()) { onResolve(value); } @@ -173,11 +196,13 @@ export const useTrackedPromise = ( (pendingPromise) => pendingPromise.promise !== newPendingPromise.promise ); - if (onReject) { - onReject(value); - } + if (shouldTriggerOrThrow()) { + if (onReject) { + onReject(value); + } - throw value; + throw value; + } } ), }; diff --git a/x-pack/plugins/ingest_manager/server/routes/epm/handlers.ts b/x-pack/plugins/ingest_manager/server/routes/epm/handlers.ts index 198a54ca84125..1d221b8b1eead 100644 --- a/x-pack/plugins/ingest_manager/server/routes/epm/handlers.ts +++ b/x-pack/plugins/ingest_manager/server/routes/epm/handlers.ts @@ -35,8 +35,7 @@ import { getPackageInfo, handleInstallPackageFailure, isBulkInstallError, - installPackageFromRegistry, - installPackageByUpload, + installPackage, removeInstallation, getLimitedPackages, getInstallationObject, @@ -149,7 +148,8 @@ export const installPackageFromRegistryHandler: RequestHandler< const { pkgName, pkgVersion } = splitPkgKey(pkgkey); const installedPkg = await getInstallationObject({ savedObjectsClient, pkgName }); try { - const res = await installPackageFromRegistry({ + const res = await installPackage({ + installSource: 'registry', savedObjectsClient, pkgkey, callCluster, @@ -224,7 +224,8 @@ export const installPackageByUploadHandler: RequestHandler< const contentType = request.headers['content-type'] as string; // from types it could also be string[] or undefined but this is checked later const archiveBuffer = Buffer.from(request.body); try { - const res = await installPackageByUpload({ + const res = await installPackage({ + installSource: 'upload', savedObjectsClient, callCluster, archiveBuffer, diff --git a/x-pack/plugins/ingest_manager/server/services/epm/packages/_install_package.test.ts b/x-pack/plugins/ingest_manager/server/services/epm/packages/_install_package.test.ts index 5d3e8e9ce87d1..b7650d10b6b25 100644 --- a/x-pack/plugins/ingest_manager/server/services/epm/packages/_install_package.test.ts +++ b/x-pack/plugins/ingest_manager/server/services/epm/packages/_install_package.test.ts @@ -61,11 +61,7 @@ describe('_installPackage', () => { const installationPromise = _installPackage({ savedObjectsClient: soClient, callCluster, - pkgName: 'abc', - pkgVersion: '1.2.3', paths: [], - removable: false, - internal: false, packageInfo: { name: 'xyz', version: '4.5.6', diff --git a/x-pack/plugins/ingest_manager/server/services/epm/packages/_install_package.ts b/x-pack/plugins/ingest_manager/server/services/epm/packages/_install_package.ts index f570984cc61aa..a83d9428b7c93 100644 --- a/x-pack/plugins/ingest_manager/server/services/epm/packages/_install_package.ts +++ b/x-pack/plugins/ingest_manager/server/services/epm/packages/_install_package.ts @@ -21,6 +21,7 @@ import { installPipelines, deletePreviousPipelines } from '../elasticsearch/inge import { installILMPolicy } from '../elasticsearch/ilm/install'; import { installKibanaAssets, getKibanaAssets } from '../kibana/assets/install'; import { updateCurrentWriteIndices } from '../elasticsearch/template/template'; +import { isRequiredPackage } from './index'; import { deleteKibanaSavedObjectsAssets } from './remove'; import { installTransform } from '../elasticsearch/transform/install'; import { createInstallation, saveKibanaAssetsRefs, updateVersion } from './install'; @@ -32,28 +33,22 @@ import { createInstallation, saveKibanaAssetsRefs, updateVersion } from './insta export async function _installPackage({ savedObjectsClient, callCluster, - pkgName, - pkgVersion, installedPkg, paths, - removable, - internal, packageInfo, installType, installSource, }: { savedObjectsClient: SavedObjectsClientContract; callCluster: CallESAsCurrentUser; - pkgName: string; - pkgVersion: string; installedPkg?: SavedObject; paths: string[]; - removable: boolean; - internal: boolean; packageInfo: InstallablePackage; installType: InstallType; installSource: InstallSource; }): Promise { + const { internal = false, name: pkgName, version: pkgVersion } = packageInfo; + const removable = !isRequiredPackage(pkgName); const toSaveESIndexPatterns = generateESIndexPatterns(packageInfo.data_streams); // add the package installation to the saved object. // if some installation already exists, just update install info diff --git a/x-pack/plugins/ingest_manager/server/services/epm/packages/index.ts b/x-pack/plugins/ingest_manager/server/services/epm/packages/index.ts index 410a9c0b22537..a1128011d81e6 100644 --- a/x-pack/plugins/ingest_manager/server/services/epm/packages/index.ts +++ b/x-pack/plugins/ingest_manager/server/services/epm/packages/index.ts @@ -29,8 +29,7 @@ export { BulkInstallResponse, IBulkInstallPackageError, handleInstallPackageFailure, - installPackageFromRegistry, - installPackageByUpload, + installPackage, ensureInstalledPackage, } from './install'; export { removeInstallation } from './remove'; diff --git a/x-pack/plugins/ingest_manager/server/services/epm/packages/install.ts b/x-pack/plugins/ingest_manager/server/services/epm/packages/install.ts index e7d8c8d4695d4..00a5c689e906d 100644 --- a/x-pack/plugins/ingest_manager/server/services/epm/packages/install.ts +++ b/x-pack/plugins/ingest_manager/server/services/epm/packages/install.ts @@ -24,7 +24,6 @@ import * as Registry from '../registry'; import { getInstallation, getInstallationObject, - isRequiredPackage, bulkInstallPackages, isBulkInstallError, } from './index'; @@ -52,7 +51,7 @@ export async function installLatestPackage(options: { name: latestPackage.name, version: latestPackage.version, }); - return installPackageFromRegistry({ savedObjectsClient, pkgkey, callCluster }); + return installPackage({ installSource: 'registry', savedObjectsClient, pkgkey, callCluster }); } catch (err) { throw err; } @@ -148,7 +147,8 @@ export async function handleInstallPackageFailure({ } const prevVersion = `${pkgName}-${installedPkg.attributes.version}`; logger.error(`rolling back to ${prevVersion} after error installing ${pkgkey}`); - await installPackageFromRegistry({ + await installPackage({ + installSource: 'registry', savedObjectsClient, pkgkey: prevVersion, callCluster, @@ -186,7 +186,12 @@ export async function upgradePackage({ }); try { - const assets = await installPackageFromRegistry({ savedObjectsClient, pkgkey, callCluster }); + const assets = await installPackage({ + installSource: 'registry', + savedObjectsClient, + pkgkey, + callCluster, + }); return { name: pkgToUpgrade, newVersion: latestPkg.version, @@ -218,19 +223,19 @@ export async function upgradePackage({ } } -interface InstallPackageParams { +interface InstallRegistryPackageParams { savedObjectsClient: SavedObjectsClientContract; pkgkey: string; callCluster: CallESAsCurrentUser; force?: boolean; } -export async function installPackageFromRegistry({ +async function installPackageFromRegistry({ savedObjectsClient, pkgkey, callCluster, force = false, -}: InstallPackageParams): Promise { +}: InstallRegistryPackageParams): Promise { // TODO: change epm API to /packageName/version so we don't need to do this const { pkgName, pkgVersion } = Registry.splitPkgKey(pkgkey); // TODO: calls to getInstallationObject, Registry.fetchInfo, and Registry.fetchFindLatestPackge @@ -250,37 +255,36 @@ export async function installPackageFromRegistry({ const { paths, registryPackageInfo } = await Registry.getRegistryPackage(pkgName, pkgVersion); - const removable = !isRequiredPackage(pkgName); - const { internal = false } = registryPackageInfo; - const installSource = 'registry'; - return _installPackage({ savedObjectsClient, callCluster, - pkgName, - pkgVersion, installedPkg, paths, - removable, - internal, packageInfo: registryPackageInfo, installType, - installSource, + installSource: 'registry', }); } -export async function installPackageByUpload({ - savedObjectsClient, - callCluster, - archiveBuffer, - contentType, -}: { +interface InstallUploadedArchiveParams { savedObjectsClient: SavedObjectsClientContract; callCluster: CallESAsCurrentUser; archiveBuffer: Buffer; contentType: string; -}): Promise { +} + +export type InstallPackageParams = + | ({ installSource: Extract } & InstallRegistryPackageParams) + | ({ installSource: Extract } & InstallUploadedArchiveParams); + +async function installPackageByUpload({ + savedObjectsClient, + callCluster, + archiveBuffer, + contentType, +}: InstallUploadedArchiveParams): Promise { const { paths, archivePackageInfo } = await loadArchivePackage({ archiveBuffer, contentType }); + const installedPkg = await getInstallationObject({ savedObjectsClient, pkgName: archivePackageInfo.name, @@ -292,25 +296,45 @@ export async function installPackageByUpload({ ); } - const removable = !isRequiredPackage(archivePackageInfo.name); - const { internal = false } = archivePackageInfo; - const installSource = 'upload'; - return _installPackage({ savedObjectsClient, callCluster, - pkgName: archivePackageInfo.name, - pkgVersion: archivePackageInfo.version, installedPkg, paths, - removable, - internal, packageInfo: archivePackageInfo, installType, - installSource, + installSource: 'upload', }); } +export async function installPackage(args: InstallPackageParams) { + if (!('installSource' in args)) { + throw new Error('installSource is required'); + } + + if (args.installSource === 'registry') { + const { savedObjectsClient, pkgkey, callCluster, force } = args; + + return installPackageFromRegistry({ + savedObjectsClient, + pkgkey, + callCluster, + force, + }); + } else if (args.installSource === 'upload') { + const { savedObjectsClient, callCluster, archiveBuffer, contentType } = args; + + return installPackageByUpload({ + savedObjectsClient, + callCluster, + archiveBuffer, + contentType, + }); + } + // @ts-expect-error s/b impossibe b/c `never` by this point, but just in case + throw new Error(`Unknown installSource: ${args.installSource}`); +} + export const updateVersion = async ( savedObjectsClient: SavedObjectsClientContract, pkgName: string, @@ -421,7 +445,9 @@ export async function ensurePackagesCompletedInstall( const pkgkey = `${pkg.attributes.name}-${pkg.attributes.install_version}`; // reinstall package if (elapsedTime > MAX_TIME_COMPLETE_INSTALL) { - acc.push(installPackageFromRegistry({ savedObjectsClient, pkgkey, callCluster })); + acc.push( + installPackage({ installSource: 'registry', savedObjectsClient, pkgkey, callCluster }) + ); } return acc; }, []); diff --git a/x-pack/plugins/triggers_actions_ui/README.md b/x-pack/plugins/triggers_actions_ui/README.md index aabb9899cb343..32e157255c0cc 100644 --- a/x-pack/plugins/triggers_actions_ui/README.md +++ b/x-pack/plugins/triggers_actions_ui/README.md @@ -1319,19 +1319,19 @@ ActionForm Props definition: interface ActionAccordionFormProps { actions: AlertAction[]; defaultActionGroupId: string; + actionGroups?: ActionGroup[]; setActionIdByIndex: (id: string, index: number) => void; + setActionGroupIdByIndex?: (group: string, index: number) => void; setAlertProperty: (actions: AlertAction[]) => void; setActionParamsProperty: (key: string, value: any, index: number) => void; http: HttpSetup; - actionTypeRegistry: TypeRegistry; - toastNotifications: Pick< - ToastsApi, - 'get$' | 'add' | 'remove' | 'addSuccess' | 'addWarning' | 'addDanger' | 'addError' - >; + actionTypeRegistry: ActionTypeRegistryContract; + toastNotifications: ToastsSetup; + docLinks: DocLinksStart; actionTypes?: ActionType[]; messageVariables?: ActionVariable[]; defaultActionMessage?: string; - consumer: string; + capabilities: ApplicationStart['capabilities']; } ``` @@ -1339,17 +1339,20 @@ interface ActionAccordionFormProps { |Property|Description| |---|---| |actions|List of actions comes from alert.actions property.| -|defaultActionGroupId|Default action group id to which each new action will belong to.| +|defaultActionGroupId|Default action group id to which each new action will belong by default.| +|actionGroups|Optional. List of action groups to which new action can be assigned. The RunWhen field is only displayed when these action groups are specified| |setActionIdByIndex|Function for changing action 'id' by the proper index in alert.actions array.| +|setActionGroupIdByIndex|Function for changing action 'group' by the proper index in alert.actions array.| |setAlertProperty|Function for changing alert property 'actions'. Used when deleting action from the array to reset it.| |setActionParamsProperty|Function for changing action key/value property by index in alert.actions array.| |http|HttpSetup needed for executing API calls.| |actionTypeRegistry|Registry for action types.| -|toastNotifications|Toast messages.| +|toastNotifications|Toast messages Plugin Setup Contract.| +|docLinks|Documentation links Plugin Start Contract.| |actionTypes|Optional property, which allowes to define a list of available actions specific for a current plugin.| |actionTypes|Optional property, which allowes to define a list of variables for action 'message' property.| |defaultActionMessage|Optional property, which allowes to define a message value for action with 'message' property.| -|consumer|Name of the plugin that creates an action.| +|capabilities|Kibana core's Capabilities ApplicationStart['capabilities'].| AlertsContextProvider value options: diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/check_action_type_enabled.scss b/x-pack/plugins/triggers_actions_ui/public/application/lib/check_action_type_enabled.scss index 24dbb865742d8..bb622829e997a 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/check_action_type_enabled.scss +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/check_action_type_enabled.scss @@ -3,9 +3,15 @@ } .actAccordionActionForm { - .euiCard { - box-shadow: none; - } + background-color: $euiColorLightestShade; +} + +.actAccordionActionForm .euiCard { + box-shadow: none; +} + +.actAccordionActionForm__button { + padding: $euiSizeM; } .actConnectorsListGrid { diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.test.tsx index 7c718e8248e41..94452e70e6bfa 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.test.tsx @@ -6,7 +6,6 @@ import React, { Fragment, lazy } from 'react'; import { mountWithIntl, nextTick } from 'test_utils/enzyme_helpers'; import { coreMock } from '../../../../../../../src/core/public/mocks'; -import { ReactWrapper } from 'enzyme'; import { act } from 'react-dom/test-utils'; import { actionTypeRegistryMock } from '../../action_type_registry.mock'; import { ValidationResult, Alert, AlertAction } from '../../../types'; @@ -112,8 +111,6 @@ describe('action_form', () => { }; describe('action_form in alert', () => { - let wrapper: ReactWrapper; - async function setup(customActions?: AlertAction[]) { const { loadAllActions } = jest.requireMock('../../lib/action_connector_api'); loadAllActions.mockResolvedValueOnce([ @@ -217,7 +214,7 @@ describe('action_form', () => { mutedInstanceIds: [], } as unknown) as Alert; - wrapper = mountWithIntl( + const wrapper = mountWithIntl( { setActionIdByIndex={(id: string, index: number) => { initialAlert.actions[index].id = id; }} + actionGroups={[{ id: 'default', name: 'Default' }]} + setActionGroupIdByIndex={(group: string, index: number) => { + initialAlert.actions[index].group = group; + }} setAlertProperty={(_updatedActions: AlertAction[]) => {}} setActionParamsProperty={(key: string, value: any, index: number) => (initialAlert.actions[index] = { ...initialAlert.actions[index], [key]: value }) @@ -297,10 +298,12 @@ describe('action_form', () => { await nextTick(); wrapper.update(); }); + + return wrapper; } it('renders available action cards', async () => { - await setup(); + const wrapper = await setup(); const actionOption = wrapper.find( `[data-test-subj="${actionType.id}-ActionTypeSelectOption"]` ); @@ -314,7 +317,7 @@ describe('action_form', () => { }); it('does not render action types disabled by config', async () => { - await setup(); + const wrapper = await setup(); const actionOption = wrapper.find( '[data-test-subj="disabled-by-config-ActionTypeSelectOption"]' ); @@ -322,52 +325,72 @@ describe('action_form', () => { }); it('render action types which is preconfigured only (disabled by config and with preconfigured connectors)', async () => { - await setup(); + const wrapper = await setup(); const actionOption = wrapper.find('[data-test-subj="preconfigured-ActionTypeSelectOption"]'); expect(actionOption.exists()).toBeTruthy(); }); + it('renders available action groups for the selected action type', async () => { + const wrapper = await setup(); + const actionOption = wrapper.find( + `[data-test-subj="${actionType.id}-ActionTypeSelectOption"]` + ); + actionOption.first().simulate('click'); + const actionGroupsSelect = wrapper.find( + `[data-test-subj="addNewActionConnectorActionGroup-0"]` + ); + expect((actionGroupsSelect.first().props() as any).options).toMatchInlineSnapshot(` + Array [ + Object { + "data-test-subj": "addNewActionConnectorActionGroup-0-option-default", + "inputDisplay": "Default", + "value": "default", + }, + ] + `); + }); + it('renders available connectors for the selected action type', async () => { - await setup(); + const wrapper = await setup(); const actionOption = wrapper.find( `[data-test-subj="${actionType.id}-ActionTypeSelectOption"]` ); actionOption.first().simulate('click'); const combobox = wrapper.find(`[data-test-subj="selectActionConnector-${actionType.id}"]`); expect((combobox.first().props() as any).options).toMatchInlineSnapshot(` - Array [ - Object { - "id": "test", - "key": "test", - "label": "Test connector ", - }, - Object { - "id": "test2", - "key": "test2", - "label": "Test connector 2 (preconfigured)", - }, - ] - `); + Array [ + Object { + "id": "test", + "key": "test", + "label": "Test connector ", + }, + Object { + "id": "test2", + "key": "test2", + "label": "Test connector 2 (preconfigured)", + }, + ] + `); }); it('renders only preconfigured connectors for the selected preconfigured action type', async () => { - await setup(); + const wrapper = await setup(); const actionOption = wrapper.find('[data-test-subj="preconfigured-ActionTypeSelectOption"]'); actionOption.first().simulate('click'); const combobox = wrapper.find('[data-test-subj="selectActionConnector-preconfigured"]'); expect((combobox.first().props() as any).options).toMatchInlineSnapshot(` - Array [ - Object { - "id": "test3", - "key": "test3", - "label": "Preconfigured Only (preconfigured)", - }, - ] - `); + Array [ + Object { + "id": "test3", + "key": "test3", + "label": "Preconfigured Only (preconfigured)", + }, + ] + `); }); it('does not render "Add connector" button for preconfigured only action type', async () => { - await setup(); + const wrapper = await setup(); const actionOption = wrapper.find('[data-test-subj="preconfigured-ActionTypeSelectOption"]'); actionOption.first().simulate('click'); const preconfigPannel = wrapper.find('[data-test-subj="alertActionAccordion-default"]'); @@ -378,7 +401,7 @@ describe('action_form', () => { }); it('renders action types disabled by license', async () => { - await setup(); + const wrapper = await setup(); const actionOption = wrapper.find( '[data-test-subj="disabled-by-license-ActionTypeSelectOption"]' ); @@ -391,7 +414,7 @@ describe('action_form', () => { }); it(`shouldn't render action types without params component`, async () => { - await setup(); + const wrapper = await setup(); const actionOption = wrapper.find( `[data-test-subj="${actionTypeWithoutParams.id}-ActionTypeSelectOption"]` ); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx index 74432157f5659..3a7341afe3e07 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { Fragment, Suspense, useState, useEffect } from 'react'; +import React, { Fragment, useState, useEffect, useCallback } from 'react'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; import { @@ -14,25 +14,13 @@ import { EuiIcon, EuiTitle, EuiSpacer, - EuiFormRow, - EuiComboBox, EuiKeyPadMenuItem, - EuiAccordion, - EuiButtonIcon, - EuiEmptyPrompt, - EuiButtonEmpty, EuiToolTip, - EuiIconTip, EuiLink, - EuiCallOut, - EuiHorizontalRule, - EuiText, - EuiLoadingSpinner, } from '@elastic/eui'; import { HttpSetup, ToastsSetup, ApplicationStart, DocLinksStart } from 'kibana/public'; import { loadActionTypes, loadAllActions as loadConnectors } from '../../lib/action_connector_api'; import { - IErrorObject, ActionTypeModel, ActionTypeRegistryContract, AlertAction, @@ -43,15 +31,19 @@ import { } from '../../../types'; import { SectionLoading } from '../../components/section_loading'; import { ConnectorAddModal } from './connector_add_modal'; +import { ActionTypeForm, ActionTypeFormProps } from './action_type_form'; +import { AddConnectorInline } from './connector_add_inline'; import { actionTypeCompare } from '../../lib/action_type_compare'; import { checkActionFormActionTypeEnabled } from '../../lib/check_action_type_enabled'; import { VIEW_LICENSE_OPTIONS_LINK, DEFAULT_HIDDEN_ACTION_TYPES } from '../../../common/constants'; -import { hasSaveActionsCapability } from '../../lib/capabilities'; +import { ActionGroup } from '../../../../../alerts/common'; -interface ActionAccordionFormProps { +export interface ActionAccordionFormProps { actions: AlertAction[]; defaultActionGroupId: string; + actionGroups?: ActionGroup[]; setActionIdByIndex: (id: string, index: number) => void; + setActionGroupIdByIndex?: (group: string, index: number) => void; setAlertProperty: (actions: AlertAction[]) => void; setActionParamsProperty: (key: string, value: any, index: number) => void; http: HttpSetup; @@ -74,7 +66,9 @@ interface ActiveActionConnectorState { export const ActionForm = ({ actions, defaultActionGroupId, + actionGroups, setActionIdByIndex, + setActionGroupIdByIndex, setAlertProperty, setActionParamsProperty, http, @@ -88,8 +82,6 @@ export const ActionForm = ({ capabilities, docLinks, }: ActionAccordionFormProps) => { - const canSave = hasSaveActionsCapability(capabilities); - const [addModalVisible, setAddModalVisibility] = useState(false); const [activeActionItem, setActiveActionItem] = useState( undefined @@ -101,6 +93,10 @@ export const ActionForm = ({ const [actionTypesIndex, setActionTypesIndex] = useState(undefined); const [emptyActionsIds, setEmptyActionsIds] = useState([]); + const closeAddConnectorModal = useCallback(() => setAddModalVisibility(false), [ + setAddModalVisibility, + ]); + // load action types useEffect(() => { (async () => { @@ -183,359 +179,6 @@ export const ActionForm = ({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [actions, connectors]); - const preconfiguredMessage = i18n.translate( - 'xpack.triggersActionsUI.sections.actionForm.preconfiguredTitleMessage', - { - defaultMessage: '(preconfigured)', - } - ); - - const getSelectedOptions = (actionItemId: string) => { - const selectedConnector = connectors.find((connector) => connector.id === actionItemId); - if ( - !selectedConnector || - // if selected connector is not preconfigured and action type is for preconfiguration only, - // do not show regular connectors of this type - (actionTypesIndex && - !actionTypesIndex[selectedConnector.actionTypeId].enabledInConfig && - !selectedConnector.isPreconfigured) - ) { - return []; - } - const optionTitle = `${selectedConnector.name} ${ - selectedConnector.isPreconfigured ? preconfiguredMessage : '' - }`; - return [ - { - label: optionTitle, - value: optionTitle, - id: actionItemId, - 'data-test-subj': 'itemActionConnector', - }, - ]; - }; - - const getActionTypeForm = ( - actionItem: AlertAction, - actionConnector: ActionConnector, - actionParamsErrors: { - errors: IErrorObject; - }, - index: number - ) => { - if (!actionTypesIndex) { - return null; - } - - const actionType = actionTypesIndex[actionItem.actionTypeId]; - - const optionsList = connectors - .filter( - (connectorItem) => - connectorItem.actionTypeId === actionItem.actionTypeId && - // include only enabled by config connectors or preconfigured - (actionType.enabledInConfig || connectorItem.isPreconfigured) - ) - .map(({ name, id, isPreconfigured }) => ({ - label: `${name} ${isPreconfigured ? preconfiguredMessage : ''}`, - key: id, - id, - })); - const actionTypeRegistered = actionTypeRegistry.get(actionConnector.actionTypeId); - if (!actionTypeRegistered || actionItem.group !== defaultActionGroupId) return null; - const ParamsFieldsComponent = actionTypeRegistered.actionParamsFields; - const checkEnabledResult = checkActionFormActionTypeEnabled( - actionTypesIndex[actionConnector.actionTypeId], - connectors.filter((connector) => connector.isPreconfigured) - ); - - const accordionContent = checkEnabledResult.isEnabled ? ( - - - - - } - labelAppend={ - canSave && - actionTypesIndex && - actionTypesIndex[actionConnector.actionTypeId].enabledInConfig ? ( - { - setActiveActionItem({ actionTypeId: actionItem.actionTypeId, index }); - setAddModalVisibility(true); - }} - > - - - ) : null - } - > - { - setActionIdByIndex(selectedOptions[0].id ?? '', index); - }} - isClearable={false} - /> - - - - - {ParamsFieldsComponent ? ( - - - - - - } - > - - - ) : null} - - ) : ( - checkEnabledResult.messageCard - ); - - return ( - - - - - - - -
- - - - - - {checkEnabledResult.isEnabled === false && ( - - - - )} - - -
-
-
- - } - extraAction={ - { - const updatedActions = actions.filter( - (_item: AlertAction, i: number) => i !== index - ); - setAlertProperty(updatedActions); - setIsAddActionPanelOpen( - updatedActions.filter((item: AlertAction) => item.id !== actionItem.id).length === - 0 - ); - setActiveActionItem(undefined); - }} - /> - } - paddingSize="l" - > - {accordionContent} -
- -
- ); - }; - - const getAddConnectorsForm = (actionItem: AlertAction, index: number) => { - const actionTypeName = actionTypesIndex - ? actionTypesIndex[actionItem.actionTypeId].name - : actionItem.actionTypeId; - const actionTypeRegistered = actionTypeRegistry.get(actionItem.actionTypeId); - if (!actionTypeRegistered || actionItem.group !== defaultActionGroupId) return null; - - const noConnectorsLabel = ( - - ); - return ( - - - - - - - -
- -
-
-
- - } - extraAction={ - { - const updatedActions = actions.filter( - (_item: AlertAction, i: number) => i !== index - ); - setAlertProperty(updatedActions); - setIsAddActionPanelOpen( - updatedActions.filter((item: AlertAction) => item.id !== actionItem.id).length === - 0 - ); - setActiveActionItem(undefined); - }} - /> - } - paddingSize="l" - > - {canSave ? ( - actionItem.id === emptyId) ? ( - noConnectorsLabel - ) : ( - - ) - } - actions={[ - { - setActiveActionItem({ actionTypeId: actionItem.actionTypeId, index }); - setAddModalVisibility(true); - }} - > - - , - ]} - /> - ) : ( - -

- -

-
- )} -
- -
- ); - }; - function addActionType(actionTypeModel: ActionTypeModel) { if (!defaultActionGroupId) { toastNotifications!.addDanger({ @@ -628,116 +271,172 @@ export const ActionForm = ({ }); } - const alertActionsList = actions.map((actionItem: AlertAction, index: number) => { - const actionConnector = connectors.find((field) => field.id === actionItem.id); - // connectors doesn't exists - if (!actionConnector) { - return getAddConnectorsForm(actionItem, index); - } - - const actionErrors: { errors: IErrorObject } = actionTypeRegistry - .get(actionItem.actionTypeId) - ?.validateParams(actionItem.params); - - return getActionTypeForm(actionItem, actionConnector, actionErrors, index); - }); - - return ( + return isLoadingConnectors ? ( + + + + ) : ( - {isLoadingConnectors ? ( - + +

- - ) : ( - - -

- + + + {actionTypesIndex && + actions.map((actionItem: AlertAction, index: number) => { + const actionConnector = connectors.find((field) => field.id === actionItem.id); + // connectors doesn't exists + if (!actionConnector) { + return ( + { + const updatedActions = actions.filter( + (_item: AlertAction, i: number) => i !== index + ); + setAlertProperty(updatedActions); + setIsAddActionPanelOpen( + updatedActions.filter((item: AlertAction) => item.id !== actionItem.id) + .length === 0 + ); + setActiveActionItem(undefined); + }} + onAddConnector={() => { + setActiveActionItem({ actionTypeId: actionItem.actionTypeId, index }); + setAddModalVisibility(true); + }} /> -

-
- - {alertActionsList} - {isAddActionPanelOpen === false ? ( -
- - - - setIsAddActionPanelOpen(true)} - > - - - - -
- ) : null} - {isAddActionPanelOpen ? ( - - - - -
+ ); + } + + const actionParamsErrors: ActionTypeFormProps['actionParamsErrors'] = actionTypeRegistry + .get(actionItem.actionTypeId) + ?.validateParams(actionItem.params); + + return ( + { + setActiveActionItem({ actionTypeId: actionItem.actionTypeId, index }); + setAddModalVisibility(true); + }} + onConnectorSelected={(id: string) => { + setActionIdByIndex(id, index); + }} + onDeleteAction={() => { + const updatedActions = actions.filter( + (_item: AlertAction, i: number) => i !== index + ); + setAlertProperty(updatedActions); + setIsAddActionPanelOpen( + updatedActions.filter((item: AlertAction) => item.id !== actionItem.id).length === + 0 + ); + setActiveActionItem(undefined); + }} + /> + ); + })} + + {isAddActionPanelOpen ? ( + + + + +
+ +
+
+
+ {hasDisabledByLicenseActionTypes && ( + + +
+ -
-
-
- {hasDisabledByLicenseActionTypes && ( - - -
- - - -
-
-
- )} -
- - - {isLoadingActionTypes ? ( - - - - ) : ( - actionTypeNodes - )} - -
- ) : null} + +
+
+
+ )} +
+ + + {isLoadingActionTypes ? ( + + + + ) : ( + actionTypeNodes + )} +
+ ) : ( + + + setIsAddActionPanelOpen(true)} + > + + + + )} - {actionTypesIndex && activeActionItem ? ( + {actionTypesIndex && activeActionItem && addModalVisible ? ( { connectors.push(savedAction); setActionIdByIndex(savedAction.id, activeActionItem.index); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx new file mode 100644 index 0000000000000..38468283b9c19 --- /dev/null +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx @@ -0,0 +1,339 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React, { Fragment, Suspense, useState } from 'react'; +import { i18n } from '@kbn/i18n'; +import { FormattedMessage } from '@kbn/i18n/react'; +import { + EuiFlexGroup, + EuiFlexItem, + EuiIcon, + EuiSpacer, + EuiFormRow, + EuiComboBox, + EuiAccordion, + EuiButtonIcon, + EuiButtonEmpty, + EuiIconTip, + EuiText, + EuiFormLabel, + EuiFormControlLayout, + EuiSuperSelect, + EuiLoadingSpinner, + EuiBadge, +} from '@elastic/eui'; +import { IErrorObject, AlertAction, ActionTypeIndex, ActionConnector } from '../../../types'; +import { checkActionFormActionTypeEnabled } from '../../lib/check_action_type_enabled'; +import { hasSaveActionsCapability } from '../../lib/capabilities'; +import { ActionAccordionFormProps } from './action_form'; + +export type ActionTypeFormProps = { + actionItem: AlertAction; + actionConnector: ActionConnector; + actionParamsErrors: { + errors: IErrorObject; + }; + index: number; + onAddConnector: () => void; + onConnectorSelected: (id: string) => void; + onDeleteAction: () => void; + setActionParamsProperty: (key: string, value: any, index: number) => void; + actionTypesIndex: ActionTypeIndex; + connectors: ActionConnector[]; +} & Pick< + ActionAccordionFormProps, + | 'defaultActionGroupId' + | 'actionGroups' + | 'setActionGroupIdByIndex' + | 'setActionParamsProperty' + | 'http' + | 'actionTypeRegistry' + | 'toastNotifications' + | 'docLinks' + | 'messageVariables' + | 'defaultActionMessage' + | 'capabilities' +>; + +const preconfiguredMessage = i18n.translate( + 'xpack.triggersActionsUI.sections.actionForm.preconfiguredTitleMessage', + { + defaultMessage: '(preconfigured)', + } +); + +export const ActionTypeForm = ({ + actionItem, + actionConnector, + actionParamsErrors, + index, + onAddConnector, + onConnectorSelected, + onDeleteAction, + setActionParamsProperty, + actionTypesIndex, + connectors, + http, + toastNotifications, + docLinks, + capabilities, + actionTypeRegistry, + defaultActionGroupId, + defaultActionMessage, + messageVariables, + actionGroups, + setActionGroupIdByIndex, +}: ActionTypeFormProps) => { + const [isOpen, setIsOpen] = useState(true); + + const canSave = hasSaveActionsCapability(capabilities); + const getSelectedOptions = (actionItemId: string) => { + const selectedConnector = connectors.find((connector) => connector.id === actionItemId); + if ( + !selectedConnector || + // if selected connector is not preconfigured and action type is for preconfiguration only, + // do not show regular connectors of this type + (actionTypesIndex && + !actionTypesIndex[selectedConnector.actionTypeId].enabledInConfig && + !selectedConnector.isPreconfigured) + ) { + return []; + } + const optionTitle = `${selectedConnector.name} ${ + selectedConnector.isPreconfigured ? preconfiguredMessage : '' + }`; + return [ + { + label: optionTitle, + value: optionTitle, + id: actionItemId, + 'data-test-subj': 'itemActionConnector', + }, + ]; + }; + + const actionType = actionTypesIndex[actionItem.actionTypeId]; + + const optionsList = connectors + .filter( + (connectorItem) => + connectorItem.actionTypeId === actionItem.actionTypeId && + // include only enabled by config connectors or preconfigured + (actionType.enabledInConfig || connectorItem.isPreconfigured) + ) + .map(({ name, id, isPreconfigured }) => ({ + label: `${name} ${isPreconfigured ? preconfiguredMessage : ''}`, + key: id, + id, + })); + const actionTypeRegistered = actionTypeRegistry.get(actionConnector.actionTypeId); + if (!actionTypeRegistered) return null; + + const ParamsFieldsComponent = actionTypeRegistered.actionParamsFields; + const checkEnabledResult = checkActionFormActionTypeEnabled( + actionTypesIndex[actionConnector.actionTypeId], + connectors.filter((connector) => connector.isPreconfigured) + ); + + const defaultActionGroup = actionGroups?.find(({ id }) => id === defaultActionGroupId); + const selectedActionGroup = + actionGroups?.find(({ id }) => id === actionItem.group) ?? defaultActionGroup; + + const accordionContent = checkEnabledResult.isEnabled ? ( + + {actionGroups && selectedActionGroup && setActionGroupIdByIndex && ( + + + + + + + } + > + ({ + value, + inputDisplay: name, + 'data-test-subj': `addNewActionConnectorActionGroup-${index}-option-${value}`, + }))} + valueOfSelected={selectedActionGroup.id} + onChange={(group) => { + setActionGroupIdByIndex(group, index); + }} + /> + + + + + + )} + + + + } + labelAppend={ + canSave && + actionTypesIndex && + actionTypesIndex[actionConnector.actionTypeId].enabledInConfig ? ( + + + + ) : null + } + > + { + onConnectorSelected(selectedOptions[0].id ?? ''); + }} + isClearable={false} + /> + + + + + {ParamsFieldsComponent ? ( + + + + + + } + > + + + ) : null} + + ) : ( + checkEnabledResult.messageCard + ); + + return ( + + + + + + + +
+ + + + + {selectedActionGroup && !isOpen && ( + + {selectedActionGroup.name} + + )} + + {checkEnabledResult.isEnabled === false && ( + + + + )} + + +
+
+
+ + } + extraAction={ + + } + > + {accordionContent} +
+ +
+ ); +}; diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_inline.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_inline.tsx new file mode 100644 index 0000000000000..97baf4a36cb4c --- /dev/null +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_inline.tsx @@ -0,0 +1,153 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React, { Fragment } from 'react'; +import { i18n } from '@kbn/i18n'; +import { FormattedMessage } from '@kbn/i18n/react'; +import { + EuiButton, + EuiFlexGroup, + EuiFlexItem, + EuiIcon, + EuiSpacer, + EuiAccordion, + EuiButtonIcon, + EuiEmptyPrompt, + EuiCallOut, + EuiText, +} from '@elastic/eui'; +import { AlertAction, ActionTypeIndex } from '../../../types'; +import { hasSaveActionsCapability } from '../../lib/capabilities'; +import { ActionAccordionFormProps } from './action_form'; + +type AddConnectorInFormProps = { + actionTypesIndex: ActionTypeIndex; + actionItem: AlertAction; + index: number; + onAddConnector: () => void; + onDeleteConnector: () => void; + emptyActionsIds: string[]; +} & Pick; + +export const AddConnectorInline = ({ + actionTypesIndex, + actionItem, + index, + onAddConnector, + onDeleteConnector, + actionTypeRegistry, + emptyActionsIds, + defaultActionGroupId, + capabilities, +}: AddConnectorInFormProps) => { + const canSave = hasSaveActionsCapability(capabilities); + + const actionTypeName = actionTypesIndex + ? actionTypesIndex[actionItem.actionTypeId].name + : actionItem.actionTypeId; + const actionTypeRegistered = actionTypeRegistry.get(actionItem.actionTypeId); + if (!actionTypeRegistered || actionItem.group !== defaultActionGroupId) return null; + + const noConnectorsLabel = ( + + ); + return ( + + + + + + + +
+ +
+
+
+ + } + extraAction={ + + } + paddingSize="l" + > + {canSave ? ( + actionItem.id === emptyId) ? ( + noConnectorsLabel + ) : ( + + ) + } + actions={[ + + + , + ]} + /> + ) : ( + +

+ +

+
+ )} +
+ +
+ ); +}; diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_modal.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_modal.test.tsx index cba9eea3cf3f7..71a3936ed5055 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_modal.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_modal.test.tsx @@ -65,8 +65,7 @@ describe('connector_add_modal', () => { const wrapper = mountWithIntl( {}} + onClose={() => {}} actionType={actionType} http={deps!.http} actionTypeRegistry={deps!.actionTypeRegistry} diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_modal.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_modal.tsx index 13ec8395aa557..de27256bf566c 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_modal.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_modal.tsx @@ -32,8 +32,7 @@ import { interface ConnectorAddModalProps { actionType: ActionType; - addModalVisible: boolean; - setAddModalVisibility: React.Dispatch>; + onClose: () => void; postSaveEventHandler?: (savedAction: ActionConnector) => void; http: HttpSetup; actionTypeRegistry: ActionTypeRegistryContract; @@ -48,8 +47,7 @@ interface ConnectorAddModalProps { export const ConnectorAddModal = ({ actionType, - addModalVisible, - setAddModalVisibility, + onClose, postSaveEventHandler, http, toastNotifications, @@ -79,14 +77,11 @@ export const ConnectorAddModal = ({ >(undefined); const closeModal = useCallback(() => { - setAddModalVisibility(false); setConnector(initialConnector); setServerError(undefined); - }, [initialConnector, setAddModalVisibility]); + onClose(); + }, [initialConnector, onClose]); - if (!addModalVisible) { - return null; - } const actionTypeModel = actionTypeRegistry.get(actionType.id); const errors = { ...actionTypeModel?.validateConnector(connector).errors, diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.tsx index 9a637ea750f81..20ad9a8d7c701 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.tsx @@ -3,7 +3,7 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import React, { Fragment, useState, useEffect, Suspense } from 'react'; +import React, { Fragment, useState, useEffect, Suspense, useCallback } from 'react'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; import { @@ -153,9 +153,17 @@ export const AlertForm = ({ setAlertTypeModel(alert.alertTypeId ? alertTypeRegistry.get(alert.alertTypeId) : null); }, [alert, alertTypeRegistry]); - const setAlertProperty = (key: string, value: any) => { - dispatch({ command: { type: 'setProperty' }, payload: { key, value } }); - }; + const setAlertProperty = useCallback( + (key: string, value: any) => { + dispatch({ command: { type: 'setProperty' }, payload: { key, value } }); + }, + [dispatch] + ); + + const setActions = useCallback( + (updatedActions: AlertAction[]) => setAlertProperty('actions', updatedActions), + [setAlertProperty] + ); const setAlertParams = (key: string, value: any) => { dispatch({ command: { type: 'setAlertParams' }, payload: { key, value } }); @@ -169,9 +177,12 @@ export const AlertForm = ({ dispatch({ command: { type: 'setAlertActionProperty' }, payload: { key, value, index } }); }; - const setActionParamsProperty = (key: string, value: any, index: number) => { - dispatch({ command: { type: 'setAlertActionParams' }, payload: { key, value, index } }); - }; + const setActionParamsProperty = useCallback( + (key: string, value: any, index: number) => { + dispatch({ command: { type: 'setAlertActionParams' }, payload: { key, value, index } }); + }, + [dispatch] + ); const tagsOptions = alert.tags ? alert.tags.map((label: string) => ({ label })) : []; @@ -202,6 +213,7 @@ export const AlertForm = ({ label={item.name} onClick={() => { setAlertProperty('alertTypeId', item.id); + setActions([]); setAlertTypeModel(item); setAlertProperty('params', {}); if (alertTypesIndex && alertTypesIndex.has(item.id)) { @@ -289,26 +301,25 @@ export const AlertForm = ({ /> ) : null} - {canShowActions && defaultActionGroupId ? ( + {canShowActions && + defaultActionGroupId && + alertTypeModel && + alertTypesIndex?.has(alert.alertTypeId) ? ( - a.name.toUpperCase().localeCompare(b.name.toUpperCase()) - ) - : undefined - } + messageVariables={actionVariablesFromAlertType( + alertTypesIndex.get(alert.alertTypeId)! + ).sort((a, b) => a.name.toUpperCase().localeCompare(b.name.toUpperCase()))} defaultActionGroupId={defaultActionGroupId} + actionGroups={alertTypesIndex.get(alert.alertTypeId)!.actionGroups} setActionIdByIndex={(id: string, index: number) => setActionProperty('id', id, index)} - setAlertProperty={(updatedActions: AlertAction[]) => - setAlertProperty('actions', updatedActions) - } - setActionParamsProperty={(key: string, value: any, index: number) => - setActionParamsProperty(key, value, index) + setActionGroupIdByIndex={(group: string, index: number) => + setActionProperty('group', group, index) } + setAlertProperty={setActions} + setActionParamsProperty={setActionParamsProperty} http={http} actionTypeRegistry={actionTypeRegistry} defaultActionMessage={alertTypeModel?.defaultActionMessage} diff --git a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alert_create_flyout.ts b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alert_create_flyout.ts index 7d99d3635106d..ee0de582a9bff 100644 --- a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alert_create_flyout.ts +++ b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alert_create_flyout.ts @@ -55,6 +55,12 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { await nameInput.click(); } + async function defineAlwaysFiringAlert(alertName: string) { + await pageObjects.triggersActionsUI.clickCreateAlertButton(); + await testSubjects.setValue('alertNameInput', alertName); + await testSubjects.click('test.always-firing-SelectOption'); + } + describe('create alert', function () { before(async () => { await pageObjects.common.navigateToApp('triggersActions'); @@ -106,6 +112,57 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { await deleteAlerts(alertsToDelete.map((alertItem: { id: string }) => alertItem.id)); }); + it('should create an alert with actions in multiple groups', async () => { + const alertName = generateUniqueKey(); + await defineAlwaysFiringAlert(alertName); + + // create Slack connector and attach an action using it + await testSubjects.click('.slack-ActionTypeSelectOption'); + await testSubjects.click('addNewActionConnectorButton-.slack'); + const slackConnectorName = generateUniqueKey(); + await testSubjects.setValue('nameInput', slackConnectorName); + await testSubjects.setValue('slackWebhookUrlInput', 'https://test'); + await find.clickByCssSelector('[data-test-subj="saveActionButtonModal"]:not(disabled)'); + const createdConnectorToastTitle = await pageObjects.common.closeToast(); + expect(createdConnectorToastTitle).to.eql(`Created '${slackConnectorName}'`); + await testSubjects.setValue('messageTextArea', 'test message '); + await ( + await find.byCssSelector( + '[data-test-subj="alertActionAccordion-0"] [data-test-subj="messageTextArea"]' + ) + ).type('some text '); + + await testSubjects.click('addAlertActionButton'); + await testSubjects.click('.slack-ActionTypeSelectOption'); + await testSubjects.setValue('messageTextArea', 'test message '); + await ( + await find.byCssSelector( + '[data-test-subj="alertActionAccordion-1"] [data-test-subj="messageTextArea"]' + ) + ).type('some text '); + + await testSubjects.click('addNewActionConnectorActionGroup-1'); + await testSubjects.click('addNewActionConnectorActionGroup-1-option-other'); + + await testSubjects.click('saveAlertButton'); + const toastTitle = await pageObjects.common.closeToast(); + expect(toastTitle).to.eql(`Created alert "${alertName}"`); + await pageObjects.triggersActionsUI.searchAlerts(alertName); + const searchResultsAfterSave = await pageObjects.triggersActionsUI.getAlertsList(); + expect(searchResultsAfterSave).to.eql([ + { + name: alertName, + tagsText: '', + alertType: 'Always Firing', + interval: '1m', + }, + ]); + + // clean up created alert + const alertsToDelete = await getAlertsByName(alertName); + await deleteAlerts(alertsToDelete.map((alertItem: { id: string }) => alertItem.id)); + }); + it('should show save confirmation before creating alert with no actions', async () => { const alertName = generateUniqueKey(); await defineAlert(alertName); diff --git a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/details.ts b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/details.ts index 9e4006681dc8d..1d86d95b7a796 100644 --- a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/details.ts +++ b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/details.ts @@ -306,7 +306,8 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { }); }); - describe('Alert Instances', function () { + // FLAKY: https://github.com/elastic/kibana/issues/57426 + describe.skip('Alert Instances', function () { const testRunUuid = uuid.v4(); let alert: any; diff --git a/x-pack/test/functional_with_es_ssl/fixtures/plugins/alerts/server/plugin.ts b/x-pack/test/functional_with_es_ssl/fixtures/plugins/alerts/server/plugin.ts index e3927f6bfffb9..6f9d010378624 100644 --- a/x-pack/test/functional_with_es_ssl/fixtures/plugins/alerts/server/plugin.ts +++ b/x-pack/test/functional_with_es_ssl/fixtures/plugins/alerts/server/plugin.ts @@ -78,6 +78,7 @@ function createAlwaysFiringAlertType(alerts: AlertingSetup) { { id: 'default', name: 'Default' }, { id: 'other', name: 'Other' }, ], + defaultActionGroupId: 'default', producer: 'alerts', async executor(alertExecutorOptions: any) { const { services, state, params } = alertExecutorOptions;