[Feature] Adds the Drag and Drop editor to Visualize

[ashwin-pc]

Description

Draft PR to merge the long running work in Drag and Drop to mainline

Issues Resolved

Check List

• New functionality includes testing.
  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
    • yarn test:ftr
• New functionality has been documented.
• Commits are signed per the DCO using --signoff

[joshuarrrr]

Initial feedback on everything except for src/plugins/wizard/common/* and src/plugins/wizard/public.

[joshuarrrr]

Do we need to define wizard-specific capabilities, or can we just use the existing visualization capabilities?

[ashwin-pc]

I'll defer this to you, since you have more context here. Not entirely sure what the capabilities provider does w.r.t saved object management.

[joshuarrrr]

Yeah, I’m not at all an expert in how capabilities work generally… but given the way the user flow works for D&D (we start by creating a visualization, and the result is essentially a visualization), using the existing visualization plugin capabilities actually makes more sense. So instead of defining and checking wizard.show or wizard.save, it seems like it makes more sense to just check for visualization.show or visualization.save capabilities (which is what I do in the embeddable to see whether to enable the Edit wizard action. Because we don’t really have a use case where a user has wizard capabilities that are different from their visualization capabilities.

But we'll definitely want to come back and figure out what exactly we should do here.

[joshuarrrr]

remove?

[ashwin-pc]

I'll defer this to you, since you have more context here. Not entirely sure what the capabilities provider does w.r.t saved object management.

[joshuarrrr]

Yeah, we can leave it for now - see my other comment about capabilities.

[codecov-commenter]

Codecov Report

Merging #1966 (d79f3e0) into main (5fb4143) will decrease coverage by 0.28%.
The diff coverage is 21.39%.

@@            Coverage Diff             @@
##             main    #1966      +/-   ##
==========================================
- Coverage   67.51%   67.22%   -0.29%     
==========================================
  Files        3077     3100      +23     
  Lines       59188    59564     +376     
  Branches     9003     9062      +59     
==========================================
+ Hits        39958    40043      +85     
- Misses      17045    17334     +289     
- Partials     2185     2187       +2     

Impacted Files	Coverage Δ
.../plugins/saved_objects_management/public/plugin.ts	62.50% <ø> (ø)
...ault_editor/public/components/agg_params_helper.ts	74.41% <ø> (ø)
src/plugins/vis_default_editor/public/index.ts	0.00% <ø> (ø)
...rc/plugins/vis_type_metric/public/metric_vis_fn.ts	44.44% <ø> (ø)
...ins/visualizations/public/legacy/build_pipeline.ts	49.18% <0.00%> (-0.27%)	⬇️
...ations/public/vis_types/vis_type_alias_registry.ts	33.33% <ø> (ø)
...s/wizard/public/embeddable/disabled_embeddable.tsx	0.00% <0.00%> (ø)
...izard/public/embeddable/disabled_visualization.tsx	0.00% <0.00%> (ø)
...s/wizard/public/saved_visualizations/_saved_vis.ts	0.00% <0.00%> (ø)
...ublic/saved_visualizations/saved_visualizations.ts	0.00% <0.00%> (ø)
... and 32 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[ananzh]

NIT:
should we remove this import? since the only place uses it is a comment (toDO)

[ashwin-pc]

yep. will do that :)

[AMoo-Miki]

Isn't this just visType.type.image?

[ashwin-pc]

Yes, but Typescript cries because Property 'image' does not exist on type 'VisTypeAlias'.

[AMoo-Miki]

1/ Isn't this just A?.id === B?.id?
2/ Shouldn't it be A?.id && A.id === B?.id?

[ashwin-pc]

@CPTNB i'll let you take this since you did mention during the initial PR why we needed this. The only reason i can think of for this is when both A and B are undefined, case 1 will return true but your function returns false, and you explicitly needed that.

[AMoo-Miki]

Missing header

[ashwin-pc]

Same as comment above

[AMoo-Miki]

i18n?

[ashwin-pc]

Didnt get your question here miki

[joshuarrrr]

more minor comments/responses

[joshuarrrr]

Because we don't actually use/reference their plugin lifecycle methods, we only need to import from them. That is, there's no runtime dependency on them, just a dependency on their bundle. I'm not 100% sure of this distinction, by the way, but it's the intuition I've pieced together from looking at other plugins.

[joshuarrrr]

Yeah, I’m not at all an expert in how capabilities work generally… but given the way the user flow works for D&D (we start by creating a visualization, and the result is essentially a visualization), using the existing visualization plugin capabilities actually makes more sense. So instead of defining and checking wizard.show or wizard.save, it seems like it makes more sense to just check for visualization.show or visualization.save capabilities (which is what I do in the embeddable to see whether to enable the Edit wizard action. Because we don’t really have a use case where a user has wizard capabilities that are different from their visualization capabilities.

But we'll definitely want to come back and figure out what exactly we should do here.

[joshuarrrr]

Yeah, we can leave it for now - see my other comment about capabilities.

[ashwin-pc]

I've fixed all the comments in #2071. Once that is merged in, i will resolve the comments here.

[kavilla]

NIT: Would be cool to add the enabled config here and in the build repo, but not blocking.

[kavilla]

it's important thing to note that i believe once this gets merged in. if stuff was removed (i believe) or fields type was changed, a migration will need to be created.

[kavilla]

i think this was potentially exported because there might have been x-pack plugins that referenced it but i dont see none in the opensearch-project using.

[ashwin-pc]

Hmm interesting. I was unable to add a new ciGroup until i removed these exports.

[kavilla]

LGTM, played around with it. Secluded enough! Thank you!!!

[joshuarrrr]

Assuming latest workflows all pass, shipit!

[ananzh]

looking forward to using new D&D

[opensearch-trigger-bot]

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-1966-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 1e34c060c23f8fb2ec5f54cfebc9b8b67813d694
# Push it to GitHub
git push --set-upstream origin backport/backport-1966-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-1966-to-2.x.