[Query] Remove es query dependency on format.convert #103174

lizozom · 2021-06-23T20:11:00Z

Summary

Removes the dependency of ES query on field.format.convert
Filter display values are not handled by the filter mappers.

Testing

Make sure that the following work:

Formatting of a phrases filter with a custom formatter (one of list) - currently a regression
Formatting of a phrases filter without a custom formatter
Formatting of a range filter on a date field
Formatting of a range filter on a numeric scripted field

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2021-06-23T20:11:02Z

Pinging @elastic/kibana-app-services (Team:AppServices)

lizozom · 2021-06-23T20:12:15Z

src/plugins/data/common/es_query/filters/phrases_filter.ts

@@ -60,7 +55,7 @@ export const buildPhrasesFilter = (
  }

  return {
-    meta: { index, type, key, value, params },


We actually weren't using the field formatter correctly here, showing the non-formatted values on the filter.
I assume this is a regression from the new index patterns changes.

lizozom · 2021-06-23T20:13:19Z

src/plugins/data/common/es_query/filters/range_filter.ts

-
-const format = (field: IFieldType, value: any) =>
-  field && field.format && field.format.convert ? field.format.convert(value) : value;
+  map(params, (val: any, key: string) => get(operators, key) + val).join(' ');


This value wan't used, as map_range already builds it's own label.

…ve-convert

lukasolson

LGTM. Tested and things seem to be formatted properly.

…ve-convert

lizozom · 2021-06-27T14:05:41Z

@elasticmachine merge upstream

kibanamachine · 2021-06-27T16:12:59Z

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`data`	833.3KB	833.2KB	-155.0B

History

💛 Build #134409 was flaky 5b40ebc
💔 Build #133777 failed adc6234
💔 Build #133792 failed 1a2284f

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @lizozom

* remove es query dependency on format.convert * fix types * types Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

kibanamachine · 2021-06-27T16:37:35Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

…-png-pdf-report-type * 'master' of github.com:elastic/kibana: (253 commits) [Enterprise Search] Test coverage pass (elastic#103406) [App Search] Success toast polish pass (elastic#103410) Role mappings: remove deprecated reset copy (elastic#103411) [Fleet] Add a message when log collection is disabled on the log details page (elastic#103166) [Query] Remove es query dependency on format.convert (elastic#103174) Home & Kibana Overview Page Template Update (elastic#103003) [ML] Converts management app jobs list pages to new layout (elastic#103117) Allow additive csp configuration (elastic#102059) [Lens] Document common formulas in product and add formula tutorial (elastic#103154) [Lens] Enable actions on Lens Embeddable (elastic#102038) [Osquery] Return proper indices permissions for osquery_manager package (elastic#103363) Dashboard locator (elastic#102854) Maps locators (elastic#102810) [Fleet] Add support for constant_keyword "value" in package field definitions (elastic#103000) [Maps] Add capability to delete features from layer & index (elastic#103145) [Security Solution] Correct linux OS lookup for Endpoint Exceptions (elastic#103038) [Enterprise Search] Add notices for deactivated users and SMTP callout (elastic#103285) [canvas] Reduce bundle size by combining SCSS imports (elastic#102822) [Enterprise Search] Final KibanaPageTemplate cleanup (elastic#103355) [docs][migrations v2] Update SO migration docs to include removal of index write block when handling corrupt SOs. (elastic#103014) ...

…-policy-2 * 'master' of github.com:elastic/kibana: (190 commits) [Enterprise Search] Test coverage pass (elastic#103406) [App Search] Success toast polish pass (elastic#103410) Role mappings: remove deprecated reset copy (elastic#103411) [Fleet] Add a message when log collection is disabled on the log details page (elastic#103166) [Query] Remove es query dependency on format.convert (elastic#103174) Home & Kibana Overview Page Template Update (elastic#103003) [ML] Converts management app jobs list pages to new layout (elastic#103117) Allow additive csp configuration (elastic#102059) [Lens] Document common formulas in product and add formula tutorial (elastic#103154) [Lens] Enable actions on Lens Embeddable (elastic#102038) [Osquery] Return proper indices permissions for osquery_manager package (elastic#103363) Dashboard locator (elastic#102854) Maps locators (elastic#102810) [Fleet] Add support for constant_keyword "value" in package field definitions (elastic#103000) [Maps] Add capability to delete features from layer & index (elastic#103145) [Security Solution] Correct linux OS lookup for Endpoint Exceptions (elastic#103038) [Enterprise Search] Add notices for deactivated users and SMTP callout (elastic#103285) [canvas] Reduce bundle size by combining SCSS imports (elastic#102822) [Enterprise Search] Final KibanaPageTemplate cleanup (elastic#103355) [docs][migrations v2] Update SO migration docs to include removal of index write block when handling corrupt SOs. (elastic#103014) ... # Conflicts: # x-pack/plugins/reporting/server/lib/store/store.test.ts # x-pack/plugins/reporting/server/lib/store/store.ts

* remove es query dependency on format.convert * fix types * types Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* remove es query dependency on format.convert * fix types * types Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Liza Katz <lizka.k@gmail.com> Co-authored-by: Liza K <liza.katz@elastic.co>

kibanamachine · 2021-06-29T16:38:07Z

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

kibanamachine · 2021-06-30T16:48:07Z

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

kibanamachine · 2021-07-01T16:58:06Z

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

remove es query dependency on format.convert

adc6234

lizozom added v8.0.0 Team:AppServices release_note:skip Skip the PR/issue when compiling release notes v7.14.0 labels Jun 23, 2021

lizozom self-assigned this Jun 23, 2021

lizozom requested a review from a team as a code owner June 23, 2021 20:11

lizozom commented Jun 23, 2021

View reviewed changes

lizozom mentioned this pull request Jun 23, 2021

[Data - Query Service] Move es-query back into a dedicated package #51659

Closed

3 tasks

Liza K added 2 commits June 23, 2021 23:33

Merge branch 'master' of github.com:elastic/kibana into es-kuery/remo…

759a016

…ve-convert

fix types

1a2284f

lukasolson approved these changes Jun 24, 2021

View reviewed changes

Liza K added 2 commits June 27, 2021 14:58

Merge branch 'master' of github.com:elastic/kibana into es-kuery/remo…

9111374

…ve-convert

types

5b40ebc

lizozom added the auto-backport Deprecated - use backport:version if exact versions are needed label Jun 27, 2021

Merge branch 'master' into es-kuery/remove-convert

4609a8c

lizozom merged commit 07af5d3 into elastic:master Jun 27, 2021

kibanamachine mentioned this pull request Jun 27, 2021

[7.x] [Query] Remove es query dependency on format.convert (#103174) #103423

Merged

lizozom mentioned this pull request Jun 28, 2021

[7.x] [Query] Remove es query dependency on format.convert (#103174) #103449

Closed

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jun 29, 2021

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jul 2, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Query] Remove es query dependency on format.convert #103174

[Query] Remove es query dependency on format.convert #103174

lizozom commented Jun 23, 2021 •

edited

Loading

elasticmachine commented Jun 23, 2021

lizozom Jun 23, 2021

lizozom Jun 23, 2021

lukasolson left a comment

lizozom commented Jun 27, 2021

kibanamachine commented Jun 27, 2021

kibanamachine commented Jun 27, 2021

kibanamachine commented Jun 29, 2021

kibanamachine commented Jun 30, 2021

kibanamachine commented Jul 1, 2021

[Query] Remove es query dependency on format.convert #103174

[Query] Remove es query dependency on format.convert #103174

Conversation

lizozom commented Jun 23, 2021 • edited Loading

Summary

Testing

Checklist

Risk Matrix

For maintainers

elasticmachine commented Jun 23, 2021

lizozom Jun 23, 2021

Choose a reason for hiding this comment

lizozom Jun 23, 2021

Choose a reason for hiding this comment

lukasolson left a comment

Choose a reason for hiding this comment

lizozom commented Jun 27, 2021

kibanamachine commented Jun 27, 2021

💚 Build Succeeded

Metrics [docs]

Page load bundle

History

kibanamachine commented Jun 27, 2021

💚 Backport successful

kibanamachine commented Jun 29, 2021

kibanamachine commented Jun 30, 2021

kibanamachine commented Jul 1, 2021

lizozom commented Jun 23, 2021 •

edited

Loading