mirror of
https://github.com/elastic/kibana.git
synced 2025-04-24 09:48:58 -04:00
# Backport This will backport the following commits from `main` to `8.12`: - [[Controls] Fix validation query for nested fields (#173690)](https://github.com/elastic/kibana/pull/173690) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Hannah Mudge","email":"Heenawter@users.noreply.github.com"},"sourceCommit":{"committedDate":"2023-12-20T21:14:39Z","message":"[Controls] Fix validation query for nested fields (#173690)\n\nCloses https://github.com/elastic/kibana/issues/172694\r\n\r\n## Summary\r\n\r\nApparently, when we [originally added the options list\r\nqueries](d07c74d250/src/plugins/controls/server/control_types/options_list/options_list_queries.ts (L145)
),\r\nwe only considered nested fields for the **suggestion** aggregation; we\r\ncompletely forgot to consider nested fields in the **validation**\r\naggregation. So, from the introduction of the new controls, options list\r\nselections would be always be marked \"invalid\" for nested fields - oops!\r\n🙈\r\n\r\n\r\nThis PR fixes the validation aggregation so that nested field selections\r\nbehave as expected.\r\n\r\n**Before**\r\n\r\n\r\ndd22bd95
-f542-49bb-9dd1-7913c945c0f7\r\n\r\n> [!TIP]\r\n> Searching a nested field for specific values will always return\r\nslightly unexpected results, because if a subset of the nested field\r\nmatches, we don't return the subset - we return the **entire set**. In\r\nother words, if you have one document where the nested field equals\r\n`[\"one\", \"two\"]` and another document where the nested field equals\r\n`[\"one\", \"three\"]`, searching for `\"one\"` will return `[\"one\", \"two\",\r\n\"three\"]` as options, like so:\r\n> \r\n>\r\n\r\n\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"6bec71021ac92a84da05a3646fe0992579c52832","branchLabelMapping":{"^v8.13.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","Team:Presentation","loe:small","impact:medium","Project:Controls","backport:prev-minor","v8.13.0"],"number":173690,"url":"https://github.com/elastic/kibana/pull/173690","mergeCommit":{"message":"[Controls] Fix validation query for nested fields (#173690)\n\nCloses https://github.com/elastic/kibana/issues/172694\r\n\r\n## Summary\r\n\r\nApparently, when we [originally added the options list\r\nqueries](d07c74d250/src/plugins/controls/server/control_types/options_list/options_list_queries.ts (L145)
),\r\nwe only considered nested fields for the **suggestion** aggregation; we\r\ncompletely forgot to consider nested fields in the **validation**\r\naggregation. So, from the introduction of the new controls, options list\r\nselections would be always be marked \"invalid\" for nested fields - oops!\r\n🙈\r\n\r\n\r\nThis PR fixes the validation aggregation so that nested field selections\r\nbehave as expected.\r\n\r\n**Before**\r\n\r\n\r\ndd22bd95
-f542-49bb-9dd1-7913c945c0f7\r\n\r\n> [!TIP]\r\n> Searching a nested field for specific values will always return\r\nslightly unexpected results, because if a subset of the nested field\r\nmatches, we don't return the subset - we return the **entire set**. In\r\nother words, if you have one document where the nested field equals\r\n`[\"one\", \"two\"]` and another document where the nested field equals\r\n`[\"one\", \"three\"]`, searching for `\"one\"` will return `[\"one\", \"two\",\r\n\"three\"]` as options, like so:\r\n> \r\n>\r\n\r\n\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"6bec71021ac92a84da05a3646fe0992579c52832"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.13.0","labelRegex":"^v8.13.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/173690","number":173690,"mergeCommit":{"message":"[Controls] Fix validation query for nested fields (#173690)\n\nCloses https://github.com/elastic/kibana/issues/172694\r\n\r\n## Summary\r\n\r\nApparently, when we [originally added the options list\r\nqueries](d07c74d250/src/plugins/controls/server/control_types/options_list/options_list_queries.ts (L145)
),\r\nwe only considered nested fields for the **suggestion** aggregation; we\r\ncompletely forgot to consider nested fields in the **validation**\r\naggregation. So, from the introduction of the new controls, options list\r\nselections would be always be marked \"invalid\" for nested fields - oops!\r\n🙈\r\n\r\n\r\nThis PR fixes the validation aggregation so that nested field selections\r\nbehave as expected.\r\n\r\n**Before**\r\n\r\n\r\ndd22bd95
-f542-49bb-9dd1-7913c945c0f7\r\n\r\n> [!TIP]\r\n> Searching a nested field for specific values will always return\r\nslightly unexpected results, because if a subset of the nested field\r\nmatches, we don't return the subset - we return the **entire set**. In\r\nother words, if you have one document where the nested field equals\r\n`[\"one\", \"two\"]` and another document where the nested field equals\r\n`[\"one\", \"three\"]`, searching for `\"one\"` will return `[\"one\", \"two\",\r\n\"three\"]` as options, like so:\r\n> \r\n>\r\n\r\n\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"6bec71021ac92a84da05a3646fe0992579c52832"}}]}] BACKPORT--> Co-authored-by: Hannah Mudge <Heenawter@users.noreply.github.com>
This commit is contained in:
parent
487baa7cd4
commit
2a8afed857
4 changed files with 154 additions and 34 deletions
|
@ -112,12 +112,8 @@ export const setupOptionsListSuggestionsRoute = (
|
|||
const validationBuilder = getValidationAggregationBuilder();
|
||||
|
||||
const suggestionAggregation: any = suggestionBuilder.buildAggregation(request) ?? {};
|
||||
const builtValidationAggregation = validationBuilder.buildAggregation(request);
|
||||
const validationAggregations = builtValidationAggregation
|
||||
? {
|
||||
validation: builtValidationAggregation,
|
||||
}
|
||||
: {};
|
||||
const validationAggregation: any = validationBuilder.buildAggregation(request);
|
||||
|
||||
const body: SearchRequest['body'] = {
|
||||
size: 0,
|
||||
...timeoutSettings,
|
||||
|
@ -128,7 +124,7 @@ export const setupOptionsListSuggestionsRoute = (
|
|||
},
|
||||
aggs: {
|
||||
...suggestionAggregation,
|
||||
...validationAggregations,
|
||||
...validationAggregation,
|
||||
},
|
||||
runtime_mappings: {
|
||||
...runtimeFieldMap,
|
||||
|
@ -144,7 +140,7 @@ export const setupOptionsListSuggestionsRoute = (
|
|||
*/
|
||||
const results = suggestionBuilder.parse(rawEsResult, request);
|
||||
const totalCardinality = results.totalCardinality;
|
||||
const invalidSelections = validationBuilder.parse(rawEsResult);
|
||||
const invalidSelections = validationBuilder.parse(rawEsResult, request);
|
||||
return {
|
||||
suggestions: results.suggestions,
|
||||
totalCardinality,
|
||||
|
|
|
@ -7,6 +7,7 @@
|
|||
*/
|
||||
|
||||
import { SearchResponse } from '@elastic/elasticsearch/lib/api/types';
|
||||
import { FieldSpec } from '@kbn/data-views-plugin/common';
|
||||
|
||||
import { OptionsListRequestBody } from '../../common/options_list/types';
|
||||
import { getValidationAggregationBuilder } from './options_list_validation_queries';
|
||||
|
@ -33,7 +34,19 @@ describe('options list queries', () => {
|
|||
};
|
||||
});
|
||||
|
||||
describe('validation aggregation and parsing', () => {
|
||||
describe('validation aggregation', () => {
|
||||
test('returns empty aggregation when not given selections', () => {
|
||||
const validationAggBuilder = getValidationAggregationBuilder();
|
||||
const optionsListRequestBodyMock: OptionsListRequestBody = {
|
||||
size: 10,
|
||||
fieldName: 'coolTestField',
|
||||
allowExpensiveQueries: true,
|
||||
};
|
||||
expect(
|
||||
validationAggBuilder.buildAggregation(optionsListRequestBodyMock)
|
||||
).toMatchInlineSnapshot(`Object {}`);
|
||||
});
|
||||
|
||||
test('creates validation aggregation when given selections', () => {
|
||||
const validationAggBuilder = getValidationAggregationBuilder();
|
||||
const optionsListRequestBodyMock: OptionsListRequestBody = {
|
||||
|
@ -45,21 +58,23 @@ describe('options list queries', () => {
|
|||
expect(validationAggBuilder.buildAggregation(optionsListRequestBodyMock))
|
||||
.toMatchInlineSnapshot(`
|
||||
Object {
|
||||
"filters": Object {
|
||||
"validation": Object {
|
||||
"filters": Object {
|
||||
"coolOption1": Object {
|
||||
"match": Object {
|
||||
"coolTestField": "coolOption1",
|
||||
"filters": Object {
|
||||
"coolOption1": Object {
|
||||
"match": Object {
|
||||
"coolTestField": "coolOption1",
|
||||
},
|
||||
},
|
||||
},
|
||||
"coolOption2": Object {
|
||||
"match": Object {
|
||||
"coolTestField": "coolOption2",
|
||||
"coolOption2": Object {
|
||||
"match": Object {
|
||||
"coolTestField": "coolOption2",
|
||||
},
|
||||
},
|
||||
},
|
||||
"coolOption3": Object {
|
||||
"match": Object {
|
||||
"coolTestField": "coolOption3",
|
||||
"coolOption3": Object {
|
||||
"match": Object {
|
||||
"coolTestField": "coolOption3",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
|
@ -68,16 +83,55 @@ describe('options list queries', () => {
|
|||
`);
|
||||
});
|
||||
|
||||
test('returns undefined when not given selections', () => {
|
||||
test('creates validation aggregation for nested fields when given selections', () => {
|
||||
const validationAggBuilder = getValidationAggregationBuilder();
|
||||
const optionsListRequestBodyMock: OptionsListRequestBody = {
|
||||
size: 10,
|
||||
fieldName: 'coolTestField',
|
||||
fieldSpec: {
|
||||
type: 'string',
|
||||
subType: { nested: { path: 'path.to.nested' } },
|
||||
} as unknown as FieldSpec,
|
||||
allowExpensiveQueries: true,
|
||||
selectedOptions: ['coolOption1', 'coolOption2', 'coolOption3'],
|
||||
};
|
||||
expect(validationAggBuilder.buildAggregation(optionsListRequestBodyMock)).toBeUndefined();
|
||||
expect(validationAggBuilder.buildAggregation(optionsListRequestBodyMock))
|
||||
.toMatchInlineSnapshot(`
|
||||
Object {
|
||||
"nestedValidation": Object {
|
||||
"aggs": Object {
|
||||
"validation": Object {
|
||||
"filters": Object {
|
||||
"filters": Object {
|
||||
"coolOption1": Object {
|
||||
"match": Object {
|
||||
"coolTestField": "coolOption1",
|
||||
},
|
||||
},
|
||||
"coolOption2": Object {
|
||||
"match": Object {
|
||||
"coolTestField": "coolOption2",
|
||||
},
|
||||
},
|
||||
"coolOption3": Object {
|
||||
"match": Object {
|
||||
"coolTestField": "coolOption3",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
"nested": Object {
|
||||
"path": "path.to.nested",
|
||||
},
|
||||
},
|
||||
}
|
||||
`);
|
||||
});
|
||||
});
|
||||
|
||||
describe('validation parsing', () => {
|
||||
test('parses validation result', () => {
|
||||
const validationAggBuilder = getValidationAggregationBuilder();
|
||||
rawSearchResponseMock.aggregations = {
|
||||
|
@ -92,7 +146,13 @@ describe('options list queries', () => {
|
|||
},
|
||||
},
|
||||
};
|
||||
expect(validationAggBuilder.parse(rawSearchResponseMock)).toMatchInlineSnapshot(`
|
||||
expect(
|
||||
validationAggBuilder.parse(rawSearchResponseMock, {
|
||||
size: 10,
|
||||
fieldName: 'coolTestField',
|
||||
allowExpensiveQueries: true,
|
||||
})
|
||||
).toMatchInlineSnapshot(`
|
||||
Array [
|
||||
"cool1",
|
||||
"cool3",
|
||||
|
@ -100,5 +160,41 @@ describe('options list queries', () => {
|
|||
]
|
||||
`);
|
||||
});
|
||||
|
||||
test('parses validation result for nested field', () => {
|
||||
const validationAggBuilder = getValidationAggregationBuilder();
|
||||
rawSearchResponseMock.aggregations = {
|
||||
nestedValidation: {
|
||||
validation: {
|
||||
buckets: {
|
||||
cool1: { doc_count: 0 },
|
||||
cool2: { doc_count: 15 },
|
||||
cool3: { doc_count: 0 },
|
||||
cool4: { doc_count: 0 },
|
||||
cool5: { doc_count: 0 },
|
||||
cool6: { doc_count: 112 },
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
expect(
|
||||
validationAggBuilder.parse(rawSearchResponseMock, {
|
||||
size: 10,
|
||||
fieldSpec: {
|
||||
type: 'string',
|
||||
subType: { nested: { path: 'path.to.nested' } },
|
||||
} as unknown as FieldSpec,
|
||||
fieldName: 'coolTestField',
|
||||
allowExpensiveQueries: true,
|
||||
})
|
||||
).toMatchInlineSnapshot(`
|
||||
Array [
|
||||
"cool1",
|
||||
"cool3",
|
||||
"cool4",
|
||||
"cool5",
|
||||
]
|
||||
`);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
@ -6,6 +6,7 @@
|
|||
* Side Public License, v 1.
|
||||
*/
|
||||
|
||||
import { getFieldSubtypeNested } from '@kbn/data-views-plugin/common';
|
||||
import { get, isEmpty } from 'lodash';
|
||||
|
||||
import { OptionsListRequestBody } from '../../common/options_list/types';
|
||||
|
@ -16,7 +17,7 @@ import { OptionsListValidationAggregationBuilder } from './types';
|
|||
*/
|
||||
export const getValidationAggregationBuilder: () => OptionsListValidationAggregationBuilder =
|
||||
() => ({
|
||||
buildAggregation: ({ selectedOptions, fieldName }: OptionsListRequestBody) => {
|
||||
buildAggregation: ({ selectedOptions, fieldName, fieldSpec }: OptionsListRequestBody) => {
|
||||
let selectedOptionsFilters;
|
||||
if (selectedOptions) {
|
||||
selectedOptionsFilters = selectedOptions.reduce((acc, currentOption) => {
|
||||
|
@ -24,16 +25,43 @@ export const getValidationAggregationBuilder: () => OptionsListValidationAggrega
|
|||
return acc;
|
||||
}, {} as { [key: string]: { match: { [key: string]: string } } });
|
||||
}
|
||||
return selectedOptionsFilters && !isEmpty(selectedOptionsFilters)
|
||||
? {
|
||||
filters: {
|
||||
filters: selectedOptionsFilters,
|
||||
|
||||
if (isEmpty(selectedOptionsFilters ?? [])) {
|
||||
return {};
|
||||
}
|
||||
|
||||
let validationAggregation: any = {
|
||||
validation: {
|
||||
filters: {
|
||||
filters: selectedOptionsFilters,
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const isNested = fieldSpec && getFieldSubtypeNested(fieldSpec);
|
||||
if (isNested) {
|
||||
validationAggregation = {
|
||||
nestedValidation: {
|
||||
nested: {
|
||||
path: isNested.nested.path,
|
||||
},
|
||||
}
|
||||
: undefined;
|
||||
aggs: {
|
||||
...validationAggregation,
|
||||
},
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
return validationAggregation;
|
||||
},
|
||||
parse: (rawEsResult) => {
|
||||
const rawInvalidSuggestions = get(rawEsResult, 'aggregations.validation.buckets');
|
||||
parse: (rawEsResult, { fieldSpec }) => {
|
||||
const isNested = fieldSpec && getFieldSubtypeNested(fieldSpec);
|
||||
const rawInvalidSuggestions = get(
|
||||
rawEsResult,
|
||||
isNested
|
||||
? 'aggregations.nestedValidation.validation.buckets'
|
||||
: 'aggregations.validation.buckets'
|
||||
);
|
||||
return rawInvalidSuggestions && !isEmpty(rawInvalidSuggestions)
|
||||
? Object.keys(rawInvalidSuggestions).filter(
|
||||
(key) => rawInvalidSuggestions[key].doc_count === 0
|
||||
|
|
|
@ -19,7 +19,7 @@ export interface EsBucket {
|
|||
|
||||
export interface OptionsListValidationAggregationBuilder {
|
||||
buildAggregation: (req: OptionsListRequestBody) => unknown;
|
||||
parse: (response: SearchResponse) => string[];
|
||||
parse: (response: SearchResponse, req: OptionsListRequestBody) => string[];
|
||||
}
|
||||
|
||||
export interface OptionsListSuggestionAggregationBuilder {
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue