Index pattern field editor - Load and save fields by mattkime · Pull Request #89390 · elastic/kibana

mattkime · 2021-01-27T04:14:47Z

Summary

Allow creation and editing of runtime fields and editing of mapped fields. Also change index pattern list to work with es types instead of kibana types. This change is because runtime fields require es types. es types are a bit more informative, I have a hard time seeing how kibana types are actionable.

emit('hello world') is always a good painless script to start with.

To Do

Save index pattern when flyout is closed.
Update field list when flyout is closed.
Define type for index pattern field editor object. Should just contain the editable values of a field.
Extract field editor values from index pattern field.
Retrieve field editor values from field editor and apply to index pattern field.
Field formatter options should change based on field type.
Require painless script for runtime fields

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

For maintainers

This was checked for breaking API changes and was labeled appropriately

…ic/kibana into ipfe-load-and-save-index-pattern

…itor' into index-pattern-field-editor/editor

…ave-index-pattern

…ic/kibana into ipfe-load-and-save-index-pattern

src/plugins/index_pattern_field_editor/public/components/field_editor/field_editor.tsx

...ugins/index_pattern_field_editor/public/components/field_editor/form_fields/format_field.tsx

…save-index-pattern

sebelga

This starts to look great @mattkime ! I have added some comments in the code.

I did find a few issues:

If I edit a field and change its name, a new field is created instead of editing the current field

If I update the format, the new format does not get applied.

What I did is first set the format to be "To Upper case". I then saved the field and went to Discover. It worked fine. Then I went back to the field editor, changed the format to "truncate the field" as shown below:

I saved the field, went to Discover. The new format did not get applied

It might be good to look into those.

src/plugins/data/common/index_patterns/fields/index_pattern_field.test.ts

src/plugins/index_pattern_field_editor/public/components/field_editor_flyout_content.tsx

...ugins/index_pattern_field_editor/public/components/field_editor_flyout_content_container.tsx

src/plugins/index_pattern_field_editor/public/open_editor.tsx

…save-index-pattern

sebelga

Just tested locally and there are still a few things we'd need to address.

Updating the format does not work. I changed from Uppercase to truncate. The index pattern table indicates the new format (truncate) but it does not reflect inside Discover.

[Edit]: There is a very strange behavior. I can only reproduce this issue when creating the "AA field". When I create another field with a different name, the format updates correctly. Can it be some sort of corupted saved object?

After I delete the field, it still appears in Discover

[Edit]: It seems to be something to fix in Discover (remove the selected fields that are not part anymore of the field list) as the field is correctly removed from the list of fields. cc @elastic/kibana-app

The format does not reset the first time I change the type. I need to change the type a second type to have the format being reset.

The state of <FormatSelectEditor /> depends on the value prop.

this.state = {
      fieldTypeFormats: getFieldTypeFormatsList(
        kbnType,
        fieldFormats.getDefaultType(kbnType, esTypes) as FieldFormatInstanceType,
        fieldFormats
      ),
      format,
      fieldFormatId: value?.id, // here
      fieldFormatParams: value?.params, // here
      kbnType,
    };
  }

But I don't see in the code a static getDerivedStateFromProps that would update the state when the value prop changes. I think that might be the problem.

sebelga · 2021-02-08T09:36:37Z

src/plugins/index_pattern_management/public/components/edit_index_pattern/tabs/tabs.tsx

+                    indexedFieldTypeFilter={indexedFieldTypeFilter}
+                    helpers={{
+                      editField: openFieldEditor,
+                      deleteField: (fieldName) => deleteFields(fieldName),


nit: no need to redeclare a function. You can name it deleteField on L220 and forward it here.

sebelga · 2021-02-08T11:24:57Z

...ugins/index_pattern_field_editor/public/components/field_editor/form_fields/script_field.tsx

              <FormattedMessage
                id="indexPatternFieldEditor.editor.form.source.scriptFieldHelpText"
-                defaultMessage="Runtime fields without a script retrieve values from a field with the same name in {source}. If a field with the same name doesn’t exist, no values return when a search request includes the runtime field. {learnMoreLink}"
+                defaultMessage="Runtime fields without a script retrieve values from _source. If the field doesn't exist in _source, a search request returns no value. {learnMoreLink}"


This copy text was reviewed by the doc team for the editor inside index template. If we want to continue with this change we would need to change it as well in the other editor.
For now I would revert it and let them raise any concern during the copy text review.

These are updates I got from @gchaps, I'll reach out to her.

sebelga · 2021-02-08T11:27:07Z

...ugins/index_pattern_field_editor/public/components/field_editor/form_fields/script_field.tsx

                        'indexPatternFieldEditor.editor.form.script.learnMoreLinkText',
                        {
-                          defaultMessage: 'Learn about script syntax.',
+                          defaultMessage: 'Learn script syntax.',


Here too we have the "Learn about script syntax." in index template. I think it sounds less like an order ("Learn script syntax!" 😊 )

My preference is "Learn about script syntax", but if that breaks from consistency in other areas of Kibana, I'm fine with the suggested change.

"about" is ok

sebelga · 2021-02-08T11:28:33Z

src/plugins/index_pattern_field_editor/public/components/field_editor/form_schema.ts

            'indexPatternFieldEditor.editor.form.validations.nameIsRequiredErrorMessage',
            {
-              defaultMessage: 'Give a name to the field.',
+              defaultMessage: 'A name is required.',


Here again, for consistency, we should keep the same error message. If you have a strong opinion on those changes we would need to make them in multiple places.

If this change is to align with consistency in other areas of Kibana, then I'm 👍

lockewritesdocs · 2021-02-08T17:10:48Z

...ugins/index_pattern_field_editor/public/components/field_editor/form_fields/script_field.tsx

              <FormattedMessage
                id="indexPatternFieldEditor.editor.form.source.scriptFieldHelpText"
-                defaultMessage="Runtime fields without a script retrieve values from a field with the same name in {source}. If a field with the same name doesn’t exist, no values return when a search request includes the runtime field. {learnMoreLink}"
+                defaultMessage="Runtime fields without a script retrieve values from _source. If the field doesn't exist in _source, a search request returns no value. {learnMoreLink}"


Suggested change

defaultMessage="Runtime fields without a script retrieve values from _source. If the field doesn't exist in _source, a search request returns no value. {learnMoreLink}"

defaultMessage="Runtime fields without a script retrieve values from a field with the same name in _source. If the field doesn't exist in _source, a search request returns no value. {learnMoreLink}"

It's important to indicate that runtime fields without a script retrieve values from a field with the same name, and that the field resides in _source. I'm 👍 to the change for the second sentence and included it in the suggested change.

++ to what Adam said. We should also change the text above the Define script box as well:

Set a value for the field instead of retrieving it from the field with the same name in _source.

...plugins/index_pattern_management/public/components/edit_index_pattern/edit_index_pattern.tsx

sebelga

As we discussed over Zoom, happy to merge this PR after adding the fix to not reset the format field on mount

useEffect(() => {
  if (isMounted.current) {
    getFields().format.reset();
  }
  isMounted.current = true;
}, [type, getFields]);

We can fix the "switch format twice" issue in a separate work.

Awesome work! 👍

cjcenizal · 2021-02-08T18:57:55Z

Forgive my comment from the peanut gallery! I just want to suggest that we either add tests to cover the bugs Seb identified or create issues to track the need for these tests. They seem like core UX flows that we'll want to ensure remain intact as we continue to iterate on this feature in future releases.

…dex_pattern/edit_index_pattern.tsx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>

…save-index-pattern

…e/kibana into ipfe-load-and-save-index-pattern

mattkime · 2021-02-09T03:26:30Z

src/plugins/index_pattern_field_editor/public/components/field_editor_flyout_content.tsx

+    .filter((fld) => fld.isMapped)
+    .map((fld) => ({
+      name: fld.name,
+      type: fld.esTypes![0]!,


@alisonelizabeth I wanted to point this out to you. The painless autocomplete needs a single type value although index pattern fields can have more than one type. We should address how this is handled to avoid running into problems.

kibanamachine · 2021-02-09T06:48:33Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 9074ef9

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`indexPatternFieldEditor`	19.3KB	21.0KB	+1.8KB
`indexPatternManagement`	564.1KB	565.4KB	+1.4KB
total			+3.1KB

Page load bundle

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

id	before	after	diff
`data`	798.0KB	798.6KB	+586.0B
`esUiShared`	217.0KB	217.1KB	+75.0B
`indexPatternFieldEditor`	78.4KB	82.9KB	+4.5KB
total			+5.2KB

History

💔 Build #104994 failed 9185ba4
💔 Build #104956 failed b0e7164
💚 Build #104851 succeeded 0cb508a
💚 Build #104519 succeeded b4c97a4
💔 Build #104517 failed 4c73a13

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

sebelga · 2021-02-09T08:56:22Z

Great point @cjcenizal. Those seem like good candidates for functional tests. 👍

partial progress, saving index pattern, loading field

1a6758d

mattkime changed the title ~~partial progress, saving index pattern, loading field~~ Index pattern field editor - Load and save fields Jan 27, 2021

mattkime added the WIP Work in progress label Jan 27, 2021

mattkime and others added 25 commits January 27, 2021 07:11

Merge branch 'feature/index-pattern-field-editor' of github.com:elast…

7b912ca

…ic/kibana into ipfe-load-and-save-index-pattern

Export Runtime type from "data" plugin

337805d

Add Field interface form the editor form

b678147

Add name and type field to the form

c7abd80

fix imports

5ebc1bc

Add FormRow component to structure sections

f518078

Add advanced settings section

e178336

Close the field editor when the component unmounts

92ea15b

Add CustomLabelField & PopularityField

23be6e9

Add ScriptField component

31572a8

Add FormatField

7379efe

Forward field to the editor

a931995

Merge remote-tracking branch 'upstream/feature/index-pattern-field-ed…

ff69f90

…itor' into index-pattern-field-editor/editor

Connect to field editor state in flyout and prevent saving with errors

2e6c171

Fix TS issue

29ed93c

Merge branch 'index-pattern-field-editor/editor' into ipfe-load-and-s…

90afa11

…ave-index-pattern

Merge branch 'feature/index-pattern-field-editor' of github.com:elast…

ec6252e

…ic/kibana into ipfe-load-and-save-index-pattern

Merge branch 'feature/index-pattern-field-editor' of github.com:elast…

160c1e0

…ic/kibana into ipfe-load-and-save-index-pattern

saving result of field editor

edc9983

minor code cleanups and fix tests

3a5bd06

fix functional test, add error on save failure

9fde5ff

partial progress

253d404

more partial progress but nearly working

a048183

fix types

11fac3b

clean up loading field formatter info

579d958

sebelga reviewed Feb 1, 2021

View reviewed changes

src/plugins/index_pattern_field_editor/public/components/field_editor/field_editor.tsx Outdated Show resolved Hide resolved

sebelga reviewed Feb 1, 2021

View reviewed changes

...ugins/index_pattern_field_editor/public/components/field_editor/form_fields/format_field.tsx Show resolved Hide resolved

mattkime added 3 commits February 4, 2021 08:15

Merge branch 'feature/index-pattern-field-editor' into ipfe-load-and-…

1895121

…save-index-pattern

clear formatting on type change

72e5ef6

refactor field format editor types

cec5fdf

qn895 mentioned this pull request Feb 4, 2021

[ML] Add better UI support for runtime fields Transforms #90363

Merged

3 tasks

sebelga suggested changes Feb 5, 2021

View reviewed changes

mattkime added 8 commits February 5, 2021 11:56

Merge branch 'feature/index-pattern-field-editor' into ipfe-load-and-…

ef34995

…save-index-pattern

fix renaming field

ddb2f6d

improve text

2350945

add delete functionality

8f1b025

fix i18n

080a417

Merge branch 'feature/index-pattern-field-editor' into ipfe-load-and-…

1ac6091

…save-index-pattern

fix functional tests

4c73a13

fix i18n

b4c97a4

sebelga suggested changes Feb 8, 2021

View reviewed changes

lockewritesdocs reviewed Feb 8, 2021

View reviewed changes

gchaps reviewed Feb 8, 2021

View reviewed changes

...plugins/index_pattern_management/public/components/edit_index_pattern/edit_index_pattern.tsx Outdated Show resolved Hide resolved

sebelga self-requested a review February 8, 2021 18:41

sebelga approved these changes Feb 8, 2021

View reviewed changes

fix saving format when its not changed

0cb508a

mattkime and others added 5 commits February 8, 2021 17:45

Update src/plugins/index_pattern_management/public/components/edit_in…

b0e7164

…dex_pattern/edit_index_pattern.tsx Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>

Merge branch 'feature/index-pattern-field-editor' into ipfe-load-and-…

a07c8b3

…save-index-pattern

Merge branch 'ipfe-load-and-save-index-pattern' of github.com:mattkim…

f7a61eb

…e/kibana into ipfe-load-and-save-index-pattern

text content changes

9fa8bca

pass along field list and type for painless autocomplete

9185ba4

mattkime commented Feb 9, 2021

View reviewed changes

fix case where field doesn't have type

9074ef9

sebelga merged commit bb04664 into elastic:feature/index-pattern-field-editor Feb 9, 2021

	defaultMessage="Runtime fields without a script retrieve values from _source. If the field doesn't exist in _source, a search request returns no value. {learnMoreLink}"
	defaultMessage="Runtime fields without a script retrieve values from a field with the same name in _source. If the field doesn't exist in _source, a search request returns no value. {learnMoreLink}"

Conversation

mattkime commented Jan 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Uh oh!

Uh oh!

Uh oh!

sebelga left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sebelga left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattkime Feb 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gchaps Feb 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sebelga left a comment

Choose a reason for hiding this comment

Uh oh!

cjcenizal commented Feb 8, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Feb 9, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

Uh oh!

sebelga commented Feb 9, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mattkime commented Jan 27, 2021 •

edited

Loading

sebelga left a comment •

edited

Loading

mattkime Feb 8, 2021 •

edited

Loading

gchaps Feb 8, 2021 •

edited

Loading