[Mappings editor] Remove boost parameter from field types by sebelga · Pull Request #113142 · elastic/kibana

sebelga · 2021-09-27T16:57:29Z

This PR removes support for the boost parameter in the mappings editor from 8.x.

Updated types

boolean
date and date_nanos
flattened
ip
keyword
numeric (all of them)
range (all of them)
text
token_count

I am going to check if index templates or component templates with mappings that contains the boost parameters are surfaced in Upgrade assistant. As this should be done ES side it does not block merging this work.

How to test

Navigate to index management > Index template tab
Create a new template
Fill the logistic step and navigate to the mappings editor
Create a field
Edit the field and make sure that the "boost" parameter is not present anymore
Change the type to one of the list above and make sure that the boost parameter not present

Fixes #78293

elasticmachine · 2021-09-27T16:58:04Z

Pinging @elastic/kibana-stack-management (Team:Stack Management)

sebelga · 2021-09-27T17:00:50Z

x-pack/plugins/index_management/public/plugin.ts

    plugins: SetupDependencies
  ): IndexManagementPluginSetup {
    const { fleet, usageCollection, management } = plugins;
+    const kibanaVersion = new SemVer(this.ctx.env.packageInfo.version);


@cjcenizal as you can see, with a system like this in place it is easy to test 7.x and 8.x UIs without switching branch (and we know how long that can take).

Replace this line with: const kibanaVersion = new SemVer('7.16.0') and that's it.

Could we document this process in the readme of the app?

I am not sure (yet) that this deserves an entry in the README. It is practical to know this trick to test different UI behaviour on 7.x and 8.x while developing but not sure it is of interest of user testing the app.

It might be good to document it maybe in the dev docs. But we probably want to wait and see if the branching system changes in any way with the "make it minor" effort.

sebelga · 2021-09-27T17:01:56Z

src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_field.ts

-        setValue(newValue);
-        return newValue;
+
+      if (isMounted.current) {


I needed to add this if statement as there was a strange race condition in one of the tests.

Should we add any tests for this?

There is test coverage for form.reset() in the use_form.test.tsx which test this part of the code. Aside this is a very low risk change as we are simply protecting ourselves from updating the state on an unmounted component.

sebelga · 2021-09-27T17:02:48Z

x-pack/plugins/index_management/common/constants/plugin.ts

+// "PluginInitializerContext.env.packageInfo.version". In some cases it is not possible
+// to dynamically inject that version without a huge refactor on the code base.
+// We will then keep this single constant to declare on which major branch we are.
+export const MAJOR_VERSION = '8.0.0';


This will be changed to 7.16.0 when backported

Should we create a ticket so we don't forget to do so?

I will immediately do it after merging 😊

Done on the PR itself: d45518a

sebelga · 2021-09-27T17:04:24Z

...ation/components/mappings_editor/components/document_fields/fields/edit_field/edit_field.tsx

-                      })
-                    : i18n.translate('xpack.idxMgmt.mappingsEditor.editFieldTitle', {
-                        defaultMessage: "Edit field '{fieldName}'",
+export const EditField = React.memo(


To review this file make sure to toggle "Hide whitespace changes" in Github.

…remove-boost-index-template

sebelga · 2021-09-29T09:07:25Z

@elasticmachine merge upstream

kibanamachine · 2021-09-29T11:14:35Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`indexManagement`	508	546	+38

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`indexManagement`	163	164	+1

Async chunks

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

id	before	after	diff
`indexManagement`	534.3KB	527.9KB	-6.4KB

Page load bundle

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

id	before	after	diff
`esUiShared`	125.7KB	125.7KB	+13.0B
`indexManagement`	29.3KB	50.3KB	+21.0KB
total			+21.0KB

Unknown metric groups

API count

id	before	after	diff
`indexManagement`	168	169	+1

History

💚 Build #156154 succeeded 957ea8c
💚 Build #155911 succeeded 832f37e
💔 Build #155843 failed cbb918c

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

sabarasaba

Thanks for working on this @sebelga! Code LGTM, tested locally. Left a few minor comments only 🥳

sabarasaba · 2021-09-30T07:41:13Z

src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_field.ts

-        setValue(newValue);
-        return newValue;
+
+      if (isMounted.current) {


Should we add any tests for this?

sabarasaba · 2021-09-30T07:42:23Z

src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_field.ts

+        if (resetValue) {
+          hasBeenReset.current = true;
+          const newValue = deserializeValue(updatedDefaultValue ?? defaultValue);
+          // updateStateIfMounted('value', newValue);


can we remove this comment?

sabarasaba · 2021-09-30T08:05:32Z

x-pack/plugins/index_management/public/plugin.ts

    plugins: SetupDependencies
  ): IndexManagementPluginSetup {
    const { fleet, usageCollection, management } = plugins;
+    const kibanaVersion = new SemVer(this.ctx.env.packageInfo.version);


Could we document this process in the readme of the app?

sabarasaba · 2021-09-30T08:06:18Z

x-pack/plugins/index_management/common/constants/plugin.ts

+// "PluginInitializerContext.env.packageInfo.version". In some cases it is not possible
+// to dynamically inject that version without a huge refactor on the code base.
+// We will then keep this single constant to declare on which major branch we are.
+export const MAJOR_VERSION = '8.0.0';


Should we create a ticket so we don't forget to do so?

sebelga · 2021-09-30T09:17:43Z

Thanks for the review @sabarasaba ! I will merge this and remove the unnecessary comment on my current branch. 👍

…3142) # Conflicts: # x-pack/plugins/index_management/__jest__/client_integration/helpers/constants.ts # x-pack/plugins/index_management/public/application/lib/indices.ts # x-pack/plugins/index_management/public/application/store/selectors/indices_filter.test.ts

) (#113512) * [Mappings editor] Remove boost parameter from field types (#113142) # Conflicts: # x-pack/plugins/index_management/__jest__/client_integration/helpers/constants.ts # x-pack/plugins/index_management/public/application/lib/indices.ts # x-pack/plugins/index_management/public/application/store/selectors/indices_filter.test.ts * Change MAJOR_VERSION to 7.16.0

sebelga added 5 commits September 27, 2021 13:06

Inject the kibana version in the Kibana React context services

faadfb5

Provide the kibana version to the parameter forms

9949994

Update parameters form to remove boost in 8.x

a8d4b3c

Fix tests

37757e7

Add test to verify that boost parameter is still present in 7.x

cbb918c

sebelga requested a review from a team as a code owner September 27, 2021 16:57

sebelga added release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// v7.16.0 v8.0.0 labels Sep 27, 2021

sebelga commented Sep 27, 2021

View reviewed changes

sebelga requested a review from sabarasaba September 27, 2021 17:05

sebelga added 3 commits September 27, 2021 21:04

Fix test

832f37e

Merge remote-tracking branch 'upstream/master' into index-management/…

ee756c4

…remove-boost-index-template

Use single MAJOR_VERSION constant

957ea8c

cjcenizal added Feature:Index Management Index and index templates UI Feature:Mappings Editor Index mappings editor UI labels Sep 28, 2021

Merge branch 'master' into index-management/remove-boost-index-template

50f6077

sabarasaba approved these changes Sep 30, 2021

View reviewed changes

sebelga merged commit 50134cb into elastic:master Sep 30, 2021

sebelga deleted the index-management/remove-boost-index-template branch September 30, 2021 09:18

sebelga mentioned this pull request Sep 30, 2021

[7.x] [Mappings editor] Remove boost parameter from field types (#113142) #113512

Merged

Conversation

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

Uh oh!

Updated types

How to test

Uh oh!

elasticmachine commented Sep 27, 2021

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!

sebelga Sep 27, 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!

sebelga commented Sep 29, 2021

Uh oh!

kibanamachine commented Sep 29, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

History

Uh oh!

sabarasaba left a comment

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!

sebelga commented Sep 30, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sebelga commented Sep 27, 2021 •

edited

Loading

sebelga Sep 27, 2021 •

edited

Loading