Default values aren't always set on mappings object

[cjcenizal]

During testing, @alisonelizabeth and I spotted a couple bugs:

1. If you skip the mappings step entirely in the wizard, the mappings object isn't set on the request payload.
2. A field's parameters' default values are only set on the mappings object if you hit Update. If you don't hit Update, the default values aren't set on the mappings object.

We agreed that we always want to set default values on the payload so that the user can depend upon the UI to behave consistently. This way, even if Elasticsearch's defaults diverge from those in the UI, the UI will still behave in a way that the user expects.

[elasticmachine]

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

[sebelga]

Let's discuss this over zoom, I am not sure I follow the reasoning as sending a payload with the default values from ES or not sending the parameter for a type is the same. With the later, the payload is smaller 😊 Unless I am missing something, the UI should behave the same way.

[cjcenizal]

Sure, let's zoom about it. My intention was to future-proof the UI against API changes. For example, if the UI indicates that the Depth limit is set to 20 (the default value) but then the API changes the default value to 10, then the UI has lied to the user and created a confusing and negative experience. We can solve this by always sending the UI's values to the API -- in essence, make the UI always be truthful.

I think tests to verify the UI's default values stay in sync with those in ES would be great, but I don't think these need to be mutually exclusive.

With the later, the payload is smaller

Do we already know how much larger the payload grows with the inclusion of default values? Is it exorbitant?

[sebelga]

Do we already know how much larger the payload grows with the inclusion of default values? Is it exorbitant?

I added a smiley to indicate it was kind of irrelevant.

if the UI indicates that the Depth limit is set to 20 (the default value) but then the API changes the default value to 10, then the UI has lied to the user and created a confusing and negative experience.

The only negative experience is if the user has opened and viewed the UI and does not see the value he has set later when he edit the template. But as soon as he opens a form and hit "update", we set the values in the payload so "the UI has lied to the user" should never happen.

If the user hasn't gone in the UI, it means he is OK with whatever default will be set by Elasticsearch. When he will then edit the template and then edit a field, the values in the form will be prepopulated with any default coming from ES.

But.. if the user edits a field he just created, look at the flyout all our hardcoded default value, then for some reason decides to cancel (does not click "update"). At that point, we won't send the values in the payload. He might then have that "bad experience" that you mention in case ES changed the default. He will then open an issue or talk about it in Discuss, and that's actually great. We now know that we need to update our hardcoded default value. 😊

We'll talk about it over zoom. 👍

[cjcenizal]

Outcome of Zoom discussion: we agreed that we want to strive for a UX in which the mappings portion of the request is consistently structured regardless of whether the user opens/uses the flyout to edit a field. We arrived at two possible solutions: either always include default values in the request or never include default values in the mappings portion of the request.

Both approaches have pros and cons. Always including default values in the request risks bloating the network request and never including default values risks the UI falling out of sync with ES.

We decided to defer choosing a solution until later because we don't think this problem is high priority.

[cjcenizal]

During conversation with Tal Levy, he mentioned that ES field settings sometimes distinguish between explicit and implicit setting values, though I haven't been able to verify this. If true, then resolving this issue will be more complicated than we originally thought.

For posterity, I tried to verify the difference between explicit and implicit default behavior using the Console requests below, but was unable to.

# Define an object that disallows searching its children. The child is implied to be searchable because its index param defaults to true.
PUT _template/implicit_default
{
  "index_patterns": [
    "implicit_default"
  ],
  "mappings": {
    "properties": {
      "unsearchable_object": {
        "type": "object",
        "enabled": false,
        "properties": {
          "implicitly_searchable_child": {
            "type": "text"
          }
        }
      }
    }
  }
}

# Create a document to search.
PUT implicit_default/_doc/1
{
  "unsearchable_object": {
    "implicitly_searchable_child": "foo"
  }
}

# When we search from it, the results will be empty.
GET implicit_default/_search
{
  "query": {
    "term": {
      "unsearchable_object.implicitly_searchable_child": {
        "value": "foo"
      }
    }
  }
}

# Now define an object with a child that should be explicitly searchable, by explicitly setting the index param to its default value of true.
PUT _template/explicit_default
{
  "index_patterns": [
    "explicit_default"
  ],
  "mappings": {
    "properties": {
      "unsearchable_object": {
        "type": "object",
        "enabled": false,
        "properties": {
          "explicitly_searchable_child": {
            "type": "text",
            "index": true
          }
        }
      }
    }
  }
}

# Create a document to search.
PUT explicit_default/_doc/1
{
  "unsearchable_object": {
    "explicitly_searchable_child": "foo"
  }
}

# When we search from it, the results are still empty. I expected that explicitly making the child searchable would allow it to turn up in search results.
GET explicit_default/_search
{
  "query": {
    "term": {
      "unsearchable_object.explicitly_searchable_child": {
        "value": "foo"
      }
    }
  }
}

[cjcenizal]

As mentioned in https://github.com/elastic/kibana/issues/106151, this is surprising behavior and potentially problematic. As we near the 8.0 upgrade, the chance of users encountering this problem will increase:

This will be hazardous and unexpected, moving forward across major versions. Settings will be deprecated, and/or default behaviours may be modified.

Can we provide a short-term solution in the UI? I can think of a couple things that would help prevent the user from being surprised.

Disable Update button if user hasn't made any changes

Can we diff between the initial state and the current state of the form, to determine whether or not the user has made any changes? And if there aren't any changes, can we disable the Update button? This will prevent the user from clicking "Update" as a means of dismissing the flyout and expecting the field to be unchanged, and then being surprised to see that it actually has been changed.

Warn user about behavior

When there are saveable changes, we can show the user some text guiding them to review all of the field's settings. We could also add a tooltip that provides more context, for example: "Updating this field will apply default values to the settings you haven't changed".

[yuliacech]

I would recommend first adding a warning that clicking the "Update" button will add default values to the settings. After that I would look into disabling the button if a user didn't change anything as @cjcenizal suggested. This should decrease users' confusion in the short term. We still need to consider a long term solution and I think the UI should not set any default values. We have a similar UI in Ingest pipelines so we could replicate it here as well.

[ElenaStoeva]

Hi @sebelga, I was advised to reach out to you regarding a question about the ES UI Form lib. I'm working on disabling the Update button if the user hasn't made any changes on the Edit mappings fields form which uses the ES UI form lib. I tried using the getFields() method and then checking if any of the returned field hooks has isChangingValue set to True (also tried with the isModified property), but this didn't work out because getFields() sometimes returns an empty object. Do you have any suggestions on how to check if any field in the form has been changed by the user?

[sebelga]

There is a useFormIsModified hook (https://docs.elastic.dev/form-lib/core/use-form-is-modified) for that purpose. It should work but I know it has some limitation for complex form with dynamic fields (which the mapping editor do heavily). Try it and see if it works.

Another approach would be to listen to form changes at the root level with the useFormData hook (https://docs.elastic.dev/form-lib/core/use-form-data). When it triggers set a isFormChangedByUser flag to true. The caveat is that if the user changes a field value then put back to the previous value it will still be considered a change by the user.

[sebelga]

The useFormIsModified was added when doing the runtime field editor. You can test it by creating a runtime field and trying to close the flyout. A warning message will appear if you have modified the form.

[ElenaStoeva]

Thanks for the suggestions @sebelga! The useFormIsModified hook worked well for some components (like toggle switches) but didn't work quite well for others like the for drop-down fields - selecting a different option from the drop-down list is not detected as a change. Another issue that I found was that in text fields, when you only add or delete exactly one character, this is not detected as a changed - you need to add/remove at least two characters.

I will now try the other approach that you suggested (using the useFormData hook) and see how it goes.

The useFormIsModified was added when doing the runtime field editor. You can test it by creating a runtime field and trying to close the flyout. A warning message will appear if you have modified the form.

Did you mean the runtime field editor under the Mappings step of the index template creating/editing flow? I tried creating and editing a runtime field, closing the flyout without saving the changes, but I didn't see any warning message.

[sebelga]

I found was that in text fields, when you only add or delete exactly one character, this is not detected as a changed - you need to add/remove at least two characters.

That's strange, I don't recall putting this limitation. It is probably a bug.

selecting a different option from the drop-down list is not detected as a change

If you want to investigate and debug why that is, go ahead 😊 . Maybe there is something to add to the hook logic?

Did you mean the runtime field editor under the Mappings step of the index template creating/editing flow

No it is the runtime field on the Data views. Go to stack management > Kibana > Data views, and create a runtime field on a data view. Try with 1 char change in the name field. It should trigger the warning when closing the unsaved flyout.

[ElenaStoeva]

selecting a different option from the drop-down list is not detected as a change

If you want to investigate and debug why that is, go ahead 😊 . Maybe there is something to add to the hook logic?

Sure, I'll play around with this hook and will try to figure out why it behaves in this way.

[ElenaStoeva]

The short term-solution proposed in https://github.com/elastic/kibana/issues/52708#issuecomment-892843365 was implemented in https://github.com/elastic/kibana/pull/152730. We decided to defer the implementation of a long-term solution until later as this is not of highest priority at the moment.

[ElenaStoeva]

In https://github.com/elastic/kibana/pull/194148, we implemented a solution for the Advanced options form, which prevents default values from being automatically added to the request. We can use the same approach to avoid setting the default values in the mapping fields edit form.