[ML] Add index pattern management to index Data Visualizer#101316
[ML] Add index pattern management to index Data Visualizer#101316qn895 merged 18 commits intoelastic:masterfrom
Conversation
5e4c520 to
fbbf1e9
Compare
fbbf1e9 to
e5385bd
Compare
|
@elasticmachine merge upstream |
| }); | ||
| } | ||
|
|
||
| // Allow to edit index pattern field |
There was a problem hiding this comment.
I see in Discover you get the option to delete runtime fields that have been added to the index pattern? Did you look into providing that functionality? Not a requirement for this PR, but just wondered how much work that would involve?
There was a problem hiding this comment.
Note as far I'm aware, the delete action should only be added for runtime fields defined in the index pattern.
There was a problem hiding this comment.
Added the delete index pattern field option here if it's a runtime field and if user has edit index pattern permission. Note that the current EUI behavior automatically makes it a dropdown menu if there's > 2 actions. This seems to be a strict EUI guideline so I think we should revisit where would be the best place for the delete button

There was a problem hiding this comment.
I think this looks ok, and follows the EUI guidelines with the ellipses icon used when there are more than 2 actions.
I have updated the PR to fix this issue here. Also added the index pattern deletion flyout although I don't think the current UI is very ideal. |
|
Pinging @elastic/ml-ui (:ml) |
peteharverson
left a comment
There was a problem hiding this comment.
Tested latest changes and LGTM
jgowdyelastic
left a comment
There was a problem hiding this comment.
Added a couple of nit pick comments but generally LGTM
| actionFlyoutRef.current = indexPatternFieldEditor?.openEditor({ | ||
| ctx: { indexPattern }, | ||
| fieldName: item.fieldName, | ||
| onSave: () => { |
There was a problem hiding this comment.
nit, onSave and onDelete are the same so could be moved to a common function
| services: { lens: lensPlugin, docLinks, notifications, uiSettings }, | ||
| } = useDataVisualizerKibana(); | ||
| const { services } = useDataVisualizerKibana(); | ||
| const { docLinks, notifications, uiSettings } = services; |
There was a problem hiding this comment.
this could all be done in one statement. but i'm not sure which is better
const {
services,
services: {
docLinks,
notifications: { toasts },
uiSettings,
},
} = useDataVisualizerKibana();`
There was a problem hiding this comment.
I think I unpacked the services here because it's being used later in getActions
pheyos
left a comment
There was a problem hiding this comment.
Great to see functional tests coming as part of this PR 👏
Just left a few comments / suggestions.
|
Checking test stability in a flaky test runner job ... no failure in 50 runs ✔️ |
mattkime
left a comment
There was a problem hiding this comment.
lgtm, great to see this being implemented!
streamich
left a comment
There was a problem hiding this comment.
AppServices changes LGTM.
|
@elasticmachine merge upstream |
|
Checking test stability in a flaky test runner job ... no failure in 50 runs ✅ |
…01316) * [ML] Add index pattern editor flyout * [ML] Add indexPatternField editor plugin as opt dependency * [ML] Remove lens from ML's dependency * [ML] Fix custom display name cause field to be missing * [ML] Add delete option * [ML] Fix aggregatableFields logic * [ML] Add functional tests * [ML] Fix labels & consolidate addRuntimeFields * [ML] Add tooltip to show or hide distributions * Consolidate refreshPage * [ML] Fix tests Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
…103156) * [ML] Add index pattern editor flyout * [ML] Add indexPatternField editor plugin as opt dependency * [ML] Remove lens from ML's dependency * [ML] Fix custom display name cause field to be missing * [ML] Add delete option * [ML] Fix aggregatableFields logic * [ML] Add functional tests * [ML] Fix labels & consolidate addRuntimeFields * [ML] Add tooltip to show or hide distributions * Consolidate refreshPage * [ML] Fix tests Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Quynh Nguyen <43350163+qn895@users.noreply.github.com>
💔 Build Failed
Failed CI StepsTest FailuresKibana Pipeline / general / X-Pack API Integration Tests.x-pack/test/api_integration/apis/management/index_lifecycle_management/policies·js.apis management index lifecycle management policies list should have a default policy to manage the Watcher history indicesStandard OutStack TraceKibana Pipeline / general / X-Pack API Integration Tests.x-pack/test/api_integration/apis/management/index_lifecycle_management/policies·js.apis management index lifecycle management policies list should have a default policy to manage the Watcher history indicesStandard OutStack TraceMetrics [docs]Module Count
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: cc @qn895 |



Summary
Part of #101435 and follow up of #100922. This PR adds the index pattern field editor flyout to the Data Visualizer. New functionalities include:
Add new index pattern field

Edit index pattern field

Manage index pattern fields

Delete index pattern field
Added tooltip for the Show/Hide distribution button
Review Hints:
data-test-subjfor index pattern editor form to help with functional testsDelete any items that are not applicable to this PR.