[Lens] Fix multi terms fields validation

[dej611]

Summary

Fix #126343

This PR provides an in depth rewrite of the field validation logic, taking into accounts new multi-fields column and per field specific validation error.

The rewrite managed to reduce the amount of check/validation logic into a single function, and fix some bugs around multi terms and drag and drop combine action, together with better error messages.

Unsupported types are discarded for the combine action now:

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

[dej611]

🕺 A little bit of dance as I thought to have applied the commit in the wrong branch :D

[dej611]

@elasticmachine merge upstream

[elasticmachine]

Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors)

[dej611]

@elasticmachine merge upstream

[dej611]

@elasticmachine merge upstream

[flash1293]

This looks mostly good to me, but there are two things we should put into consideration:

• Using date fields in a multi field terms was allowed previously, now we disallow it. This is a breaking change. Are we fine with this?
• It's still weird it's possible to select the field from the list - maybe we should just not select the field if we know it's going to be invalid?

cc @ghudgins as these are product questions too

[dej611]

• Using date fields in a multi field terms was allowed previously, now we disallow it. This is a breaking change. Are we fine with this?

I thought about this as well, but given dates not being supported for single term Top values, finding them enabled on multi terms (for disabled fields on the dropdown) is a good hint for no support.
I guess it is possible to provide a runtime field workaround in this case?

It's still weird it's possible to select the field from the list - maybe we should just not select the field if we know it's going to be invalid?

Fair point. Should this be extended also to single term mode?

[flash1293]

I guess it is possible to provide a runtime field workaround in this case?

Yeah, true, I don't expect it to become an issue

Fair point. Should this be extended also to single term mode?

No, in single term mode it's switching to date histogram which is a useful behavior.

[dej611]

@flash1293 I've added some logic to remove fields with unsupported type when in multi-terms mode.

[dej611]

@elasticmachine merge upstream

[dej611]

@elasticmachine merge upstream

[flash1293]

Sorry for the delay in review - it's close but in the single field case it should switch to date histogram automatically. At least that's what is happening for other operations, e.g. selecting a date field on "Intervals" switches to date histogram. If possible I would like to have this behaving consistent for the single field case.

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: cd9be9e
• Storybooks Preview
• Webpack Bundle Analyzer

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
lens	751	752	+1

Async chunks

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

id	before	after	diff
lens	1.1MB	1.1MB	+903.0B

History

• 💔 Build #28460 failed 5eb83f7
• 💚 Build #28007 succeeded f6f29c1
• 💛 Build #27907 was flaky 5365edb
• 💚 Build #27596 succeeded 9cd7b6e
• 💛 Build #27394 was flaky 89641d6

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

[dej611]

Sorry for the delay in review - it's close but in the single field case it should switch to date histogram automatically. At least that's what is happening for other operations, e.g. selecting a date field on "Intervals" switches to date histogram. If possible I would like to have this behaving consistent for the single field case.

I think this switch behaviour has changed some time ago.
With this PR and in master I can reproduce it only on initial state when defined the bucketed dimensions. Once the dimension is defined picking a date field will no longer switch automatically in both branches.

On the other hand testing it on a 7.17 instance the behaviour still has the automatic switch. Perhaps it is worth a distinct issue for it?

[flash1293]

I think it stopped working with the multi terms change (as the field change handling is now done on the terms operation level). Fine with splitting it out from this PR.

[flash1293]

LGTM for the rest of the behavior

[dej611]

I think it stopped working with the multi terms change (as the field change handling is now done on the terms operation level). Fine with splitting it out from this PR.

Created #127258