[Mappings editor] Add support for histogram field type #76671

alisonelizabeth · 2020-09-03T16:52:24Z

I also created a common component for the meta parameter. While working on this PR, I found out from the ES team that this parameter is available to most field types so I added support where appropriate via df56ddc.

Release note

The mappings editor in the Index Templates UI now supports configuring the histogram field type. Support for the meta parameter was also added to the boolean, binary, completion, date, flattened, geo_point, numeric, range, search_as_you_type, token_count and text field types.

elasticmachine · 2020-09-03T19:51:10Z

Pinging @elastic/es-ui (Team:Elasticsearch UI)

sebelga · 2020-09-07T07:11:52Z

@elasticmachine merge upstream

sebelga

Great work @alisonelizabeth ! I left a few comments to tidy things up but no blockers. Cheers.

sebelga · 2020-09-07T08:16:08Z

...tion/components/mappings_editor/components/document_fields/fields/field_types/range_type.tsx

 } from '../../field_parameters';
 import { BasicParametersSection, AdvancedParametersSection } from '../edit_field';
 import { FormDataProvider } from '../../../../shared_imports';

-const getDefaultToggleValue = (param: 'locale' | 'format' | 'boost', field: FieldType) => {
+const getDefaultToggleValue = (param: ParameterName, field: FieldType) => {
+  if (param === 'meta') {


The default value for meta is '' so L27 should be enough

sebelga · 2020-09-07T08:17:33Z

...ion/components/mappings_editor/components/document_fields/fields/field_types/binary_type.tsx

 import { AdvancedParametersSection } from '../edit_field';

-export const BinaryType = () => {
+const getDefaultToggleValue = (param: ParameterName, field: FieldType) => {
+  return field[param] !== undefined && field[param] !== '';


Could we default to the more generic

return field[param] !== undefined && field[param] !== getFieldConfig(param).defaultValue;

like we do in the "range_type"?

sebelga · 2020-09-07T08:18:21Z

...on/components/mappings_editor/components/document_fields/fields/field_types/boolean_type.tsx

@@ -24,8 +25,9 @@ const getDefaultToggleValue = (param: string, field: FieldType) => {
    case 'boost': {
      return field[param] !== undefined && field[param] !== getFieldConfig(param).defaultValue;
    }
+    case 'meta':


I think "null_value" could have been left, and simply add "meta" on top of "boost"

sebelga · 2020-09-07T08:19:07Z

...components/mappings_editor/components/document_fields/fields/field_types/completion_type.tsx

 import { EditFieldFormRow, AdvancedParametersSection } from '../edit_field';

 const getDefaultToggleValue = (param: string, field: FieldType) => {
  switch (param) {
    case 'max_input_length': {
      return field[param] !== undefined && field[param] !== getFieldConfig(param).defaultValue;
    }
+    case 'meta': {


I think we can simply add "meta" on top of "max_input_length"

sebelga · 2020-09-07T08:19:30Z

...ation/components/mappings_editor/components/document_fields/fields/field_types/date_type.tsx

    case 'null_value': {
-      return field.null_value !== undefined && field.null_value !== '';
+      return field[param] !== undefined && field[param] !== '';


Same here: I think "null_value" could have been left, and simply add "meta" on top of "boost"

sebelga · 2020-09-07T08:24:07Z

...on/components/mappings_editor/components/document_fields/fields/field_types/numeric_type.tsx

    case 'null_value': {
-      return field.null_value !== undefined && field.null_value !== '';
+      return field[param] !== undefined && field[param] !== '';


Same here :) on top of "ignore_malformed"

sebelga · 2020-09-07T08:25:00Z

...ponents/mappings_editor/components/document_fields/fields/field_types/search_as_you_type.tsx

@@ -29,6 +30,9 @@ const getDefaultToggleValue = (param: string, field: FieldType) => {
    case 'max_shingle_size': {
      return field[param] !== undefined && field[param] !== getFieldConfig(param).defaultValue;
    }
+    case 'meta': {


same here: on top of "max_shingle_size"

sebelga · 2020-09-07T08:25:42Z

...ation/components/mappings_editor/components/document_fields/fields/field_types/text_type.tsx

@@ -46,8 +47,9 @@ const getDefaultToggleValue = (param: string, field: FieldType) => {
    case 'analyzers': {
      return field.search_analyzer !== undefined && field.search_analyzer !== field.analyzer;
    }
+    case 'meta':


Same here: on top of "boost"

sebelga · 2020-09-07T08:26:10Z

...omponents/mappings_editor/components/document_fields/fields/field_types/token_count_type.tsx

    case 'null_value': {
-      return field.null_value !== undefined && field.null_value !== '';
+      return field[param] !== undefined && field[param] !== '';


Same here :)

sebelga · 2020-09-07T08:28:52Z

...management/public/application/components/mappings_editor/constants/parameters_definition.tsx

+        return JSON.stringify(value, null, 2);
+      },
+      serializer: (value: string) => {
+        try {


There is no more the need of try/catch in serializer as now it is guaranteed to be a valid value (it has passed the validations defined, so here isJsonField).

So it could simply by

serializer: (value: string) => { const parsed = JSON.parse(value); // If an empty object was passed, strip out this value entirely. if (!Object.keys(parsed).length) { return undefined; } return parsed; },

sebelga · 2020-09-07T08:37:10Z

Ups! I went a bit fast on clicking the "Approve button" 😊
There is a strange bug I discovered.

Create a template with an histogram field and a meta like this

{ "meta": true }

Save ---> everything ok
Edit the template and the field and set the meta as

["meta"] // valid JSON but not valid for ES

Save ---> error

[illegal_argument_exception] composable template [my-template] template after composition is invalid

Edit the template and put back the initial meta (object) value
Save ---> still error?
Edit the field... and disable the meta (toggle OFF)
Save ---> ok
Edit the template + edit the field and add valid meta (object)
Save ---> still error??? (this is very strange, we can't save a meta anymore on that field)

sebelga · 2020-09-07T10:05:16Z

[EDIT]

So I played a bit more with it and the steps to reproduce are slightly different:

Create an index with JUST a name + and the logs* index pattern
Save the template (this is the request about to be saved)

Edit the template and add 1 histogram field. Edit the field to add a meta.

Even without looking at the advanced configuration form (navigate to that tab), we get all of them in the request

And saving it not allowed.

[illegal_argument_exception] composable template [my-template] template after composition is invalid

I then remove the meta (toggle OFF). The request sill has all the advanced settings but I am allowed to save

In your other PR with constant_keyword, no advanced config is added to the mappings but the same error is returned when adding some meta (with logs* as index pattern)

I'd like to investigate a bit further before approving as there is a very odd behavior. Maybe it is 100% on ES side, but I'd like to confirm this.

…ngs_editor/histogram_field

alisonelizabeth · 2020-09-14T17:03:18Z

Thanks for the review @sebelga! I believe I addressed your comments. Would you mind taking another look?

Also, as mentioned in #76564, I believe the error you were hitting when saving meta was due to setting a boolean value instead of a string.

I was not able to reproduce the issue you saw related to the advanced settings config. Can you retest and let me know if you're still able to reproduce?

alisonelizabeth · 2020-09-16T18:17:40Z

@elasticmachine merge upstream

sebelga

Thanks for making the changes! Tested locally and works as expected. You didn't add the validation for the meta values (string) but I guess it will get merged with the other PR.

[Edit] I could not reproduce the issue I mentioned with the advanced settings, which is good 👍

sebelga · 2020-09-17T10:46:57Z

@elasticmachine merge upstream

…ngs_editor/histogram_field

alisonelizabeth · 2020-09-17T15:04:38Z

Thanks for the second review @sebelga!

Thanks for making the changes! Tested locally and works as expected. You didn't add the validation for the meta values (string) but I guess it will get merged with the other PR.

The validation is there now that #76564 has been merged. I also updated the constant_keyword field type to use the common meta component.

kibanamachine · 2020-09-17T16:38:21Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 7974df4

Build metrics

@kbn/optimizer bundle module count

id	value	diff	baseline
indexManagement	506	+2	504

async chunks size

id	value	diff	baseline
indexManagement	1.6MB	+3.7KB	1.6MB

History

💚 Build #75133 succeeded ed38891
💔 Build #74919 failed a24d8a8
💚 Build #74220 succeeded 45363e5
💚 Build #72844 succeeded 7886b0d
💚 Build #72414 succeeded 3e85940

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

add support for histogram field type

80bdc59

alisonelizabeth added release_note:enhancement v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more Feature:Mappings Editor Index mappings editor UI v7.10.0 labels Sep 3, 2020

alisonelizabeth added 3 commits September 3, 2020 13:17

fix i18n

fe6cbd5

add support for meta parameter to field types

df56ddc

fix TS

3e85940

alisonelizabeth marked this pull request as ready for review September 3, 2020 19:51

alisonelizabeth requested a review from a team as a code owner September 3, 2020 19:51

alisonelizabeth requested a review from sebelga September 3, 2020 19:51

Merge branch 'master' into feature/mappings_editor/histogram_field

7886b0d

sebelga previously approved these changes Sep 7, 2020

View reviewed changes

sebelga mentioned this pull request Sep 7, 2020

[Mappings editor] Add support for constant_keyword field type #76564

Merged

alisonelizabeth added 3 commits September 14, 2020 12:21

Merge branch 'master' of github.com:elastic/kibana into feature/mappi…

6240c3d

…ngs_editor/histogram_field

address review feedback

56ef8bd

address review feedback

45363e5

alisonelizabeth mentioned this pull request Sep 15, 2020

[Mappings editor] Add support for point field type #77543

Merged

Merge branch 'master' into feature/mappings_editor/histogram_field

a24d8a8

sebelga self-requested a review September 17, 2020 10:41

sebelga approved these changes Sep 17, 2020

View reviewed changes

elasticmachine and others added 3 commits September 17, 2020 04:47

Merge branch 'master' into feature/mappings_editor/histogram_field

ed38891

Merge branch 'master' of github.com:elastic/kibana into feature/mappi…

7e363d9

…ngs_editor/histogram_field

update constant_keyword to use common meta component

7974df4

alisonelizabeth merged commit 11f100b into elastic:master Sep 17, 2020

alisonelizabeth deleted the feature/mappings_editor/histogram_field branch September 17, 2020 16:41

alisonelizabeth mentioned this pull request Sep 17, 2020

[7.x] [Mappings editor] Add support for histogram field type (#76671) #77787

Merged

alisonelizabeth added a commit to alisonelizabeth/kibana that referenced this pull request Sep 17, 2020

[Mappings editor] Add support for histogram field type (elastic#76671)

cecfbf0

alisonelizabeth added a commit that referenced this pull request Sep 17, 2020

[Mappings editor] Add support for histogram field type (#76671) (#77787)

19fd225

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Mappings editor] Add support for histogram field type #76671

[Mappings editor] Add support for histogram field type #76671

alisonelizabeth commented Sep 3, 2020 •

edited

Loading

elasticmachine commented Sep 3, 2020

sebelga commented Sep 7, 2020

sebelga left a comment

sebelga Sep 7, 2020

sebelga Sep 7, 2020

sebelga Sep 7, 2020

sebelga Sep 7, 2020

sebelga Sep 7, 2020

sebelga Sep 7, 2020

sebelga Sep 7, 2020

sebelga Sep 7, 2020

sebelga Sep 7, 2020

sebelga Sep 7, 2020

sebelga commented Sep 7, 2020

sebelga commented Sep 7, 2020

alisonelizabeth commented Sep 14, 2020 •

edited

Loading

alisonelizabeth commented Sep 16, 2020

sebelga left a comment •

edited

Loading

sebelga commented Sep 17, 2020

alisonelizabeth commented Sep 17, 2020

kibanamachine commented Sep 17, 2020

[Mappings editor] Add support for histogram field type #76671

[Mappings editor] Add support for histogram field type #76671

Conversation

alisonelizabeth commented Sep 3, 2020 • edited Loading

Release note

elasticmachine commented Sep 3, 2020

sebelga commented Sep 7, 2020

sebelga left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebelga commented Sep 7, 2020

sebelga commented Sep 7, 2020

alisonelizabeth commented Sep 14, 2020 • edited Loading

alisonelizabeth commented Sep 16, 2020

sebelga left a comment • edited Loading

Choose a reason for hiding this comment

sebelga commented Sep 17, 2020

alisonelizabeth commented Sep 17, 2020

kibanamachine commented Sep 17, 2020

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

async chunks size

History

alisonelizabeth commented Sep 3, 2020 •

edited

Loading

alisonelizabeth commented Sep 14, 2020 •

edited

Loading

sebelga left a comment •

edited

Loading