Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Time to Visualize] Fix Unlink Action via Rollback of ReplacePanel #83873

Merged
merged 4 commits into from
Nov 25, 2020
Merged

[Time to Visualize] Fix Unlink Action via Rollback of ReplacePanel #83873

merged 4 commits into from
Nov 25, 2020

Conversation

ThomThomson
Copy link
Contributor

@ThomThomson ThomThomson commented Nov 19, 2020 •
edited
Loading

Summary

Fixes #83864 via a partial rollback of #74253.

The new version of the Replace Panel method introduced in that PR always retains Embeddable Ids through the replacement process. This avoids some flickering, which was especially prevalent in the clone function. Unfortunately, with Actions like Unlink From Library and Add to Library which add, remove, or update savedObjectIds, retaining the same EmbeddableId prevents the Embeddable from fully re-initializing with the new SavedObjectId.

In order to fix this, we can either make all by value / by reference embeddables respond properly to changes in their savedObjectId - POC PR for Lens #83874 -

OR we can force a total re-initialization for Embeddables in these special cases where SavedObjectIds change. The latter is what this PR does.

How to Test

  1. Add dashboard.allowByValueEmbeddables: true to the bottom of your config/kibana.dev.yml
  2. Load a dashboard, and add a Lens panel to it from the Visualize Library
  3. Use the 'unlink from library' action to unlink the panel from the library
  4. Edit the newly unlinked panel. You should not see a savedObjectId in the Url bar, and the address should contain edit_by_value.

This same procedure should work correctly with Visualize and Maps as well.

Checklist

Delete any items that are not applicable to this PR.

added to match the most common scenarios

For maintainers

@ThomThomson ThomThomson mentioned this pull request Nov 19, 2020
Closed
@ThomThomson ThomThomson marked this pull request as ready for review November 23, 2020 22:40
@ThomThomson ThomThomson requested a review from a team as a code owner November 23, 2020 22:40
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Page load bundle

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

id before after diff
dashboard 320.8KB 321.3KB +489.0B

History

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

@ThomThomson ThomThomson mentioned this pull request Nov 24, 2020
Closed
poffdeluxe
poffdeluxe approved these changes Nov 24, 2020
View reviewed changes
Copy link
Contributor

@poffdeluxe poffdeluxe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good to me. Was able to test that I could unlink visualizations, edit them, and save them without the original version in the library getting changes

@ThomThomson ThomThomson merged commit 351cd6d into elastic:master Nov 25, 2020
ThomThomson added a commit to ThomThomson/kibana that referenced this pull request Nov 25, 2020
@ThomThomson
[Time to Visualize] Fix Unlink Action via Rollback of ReplacePanel (e…
f8ca827 
…lastic#83873)

* Fixed unlink action via rollback of replacePanel changes
@ThomThomson ThomThomson mentioned this pull request Nov 25, 2020
Merged
ThomThomson added a commit that referenced this pull request Nov 25, 2020
@ThomThomson
[Time to Visualize] Fix Unlink Action via Rollback of ReplacePanel (#…
dfc0123 
…83873) (#84346)

* Fixed unlink action via rollback of replacePanel changes
gmmorris added a commit to gmmorris/kibana that referenced this pull request Nov 26, 2020
@gmmorris
Merge branch 'master' into alerts/fix-buggy-default-message
2c5348f 
* master: (70 commits)
  [Uptime] Fix headers io-ts type (elastic#84089)
  [fleet] Add config options to accepted docker env vars (elastic#84338)
  [Fleet] Support URL query state in agent logs UI (elastic#84298)
  [basePathProxy] include query in redirect (elastic#84356)
  [Security Solution] Add Endpoint policy feature checks (elastic#83972)
  Fix issues with show_license_expiration (elastic#84361)
  [Security Solution][Resolver] Add support for predefined schemas for endpoint and winlogbeat (elastic#84103)
  [cli/dev] log a warning when --no-base-path is used with --dev (elastic#84354)
  [Fleet] Support input-level vars & templates (elastic#83878)
  [APM] Elastic chart issues (elastic#84238)
  [Time to Visualize] Fix Unlink Action via Rollback of ReplacePanel (elastic#83873)
  redirect to visualize listing page when by value visualization editor doesn't have a value input (elastic#84287)
  add live region for field search (elastic#84310)
  [ML] Persisted URL state for Anomalies table (elastic#84314)
  [dev/cli] detect worker type using env, not cluster module (elastic#83977)
  [Workplace Search] Migrate DisplaySettings tree (elastic#84283)
  Deprecate `xpack.task_manager.index` setting (elastic#84155)
  [Search] Search batching using bfetch (again) (elastic#84043)
  Use .kibana instead of .kibana_current to mark migration completion (elastic#83373)
  [Monitoring] Only look at ES for the missing data alert for now (elastic#83839)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Nov 26, 2020
@gmmorris
Merge branch 'master' into alerts/rename-resolved-to-recovered
f2bd0e0 
* master: (119 commits)
  [Uptime] Fix headers io-ts type (elastic#84089)
  [fleet] Add config options to accepted docker env vars (elastic#84338)
  [Fleet] Support URL query state in agent logs UI (elastic#84298)
  [basePathProxy] include query in redirect (elastic#84356)
  [Security Solution] Add Endpoint policy feature checks (elastic#83972)
  Fix issues with show_license_expiration (elastic#84361)
  [Security Solution][Resolver] Add support for predefined schemas for endpoint and winlogbeat (elastic#84103)
  [cli/dev] log a warning when --no-base-path is used with --dev (elastic#84354)
  [Fleet] Support input-level vars & templates (elastic#83878)
  [APM] Elastic chart issues (elastic#84238)
  [Time to Visualize] Fix Unlink Action via Rollback of ReplacePanel (elastic#83873)
  redirect to visualize listing page when by value visualization editor doesn't have a value input (elastic#84287)
  add live region for field search (elastic#84310)
  [ML] Persisted URL state for Anomalies table (elastic#84314)
  [dev/cli] detect worker type using env, not cluster module (elastic#83977)
  [Workplace Search] Migrate DisplaySettings tree (elastic#84283)
  Deprecate `xpack.task_manager.index` setting (elastic#84155)
  [Search] Search batching using bfetch (again) (elastic#84043)
  Use .kibana instead of .kibana_current to mark migration completion (elastic#83373)
  [Monitoring] Only look at ES for the missing data alert for now (elastic#83839)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Dec 9, 2020
@gmmorris
Merge branch 'master' into alerts/rename-resolved-to-recovered
205e526 
* master: (119 commits)
  [Uptime] Fix headers io-ts type (elastic#84089)
  [fleet] Add config options to accepted docker env vars (elastic#84338)
  [Fleet] Support URL query state in agent logs UI (elastic#84298)
  [basePathProxy] include query in redirect (elastic#84356)
  [Security Solution] Add Endpoint policy feature checks (elastic#83972)
  Fix issues with show_license_expiration (elastic#84361)
  [Security Solution][Resolver] Add support for predefined schemas for endpoint and winlogbeat (elastic#84103)
  [cli/dev] log a warning when --no-base-path is used with --dev (elastic#84354)
  [Fleet] Support input-level vars & templates (elastic#83878)
  [APM] Elastic chart issues (elastic#84238)
  [Time to Visualize] Fix Unlink Action via Rollback of ReplacePanel (elastic#83873)
  redirect to visualize listing page when by value visualization editor doesn't have a value input (elastic#84287)
  add live region for field search (elastic#84310)
  [ML] Persisted URL state for Anomalies table (elastic#84314)
  [dev/cli] detect worker type using env, not cluster module (elastic#83977)
  [Workplace Search] Migrate DisplaySettings tree (elastic#84283)
  Deprecate `xpack.task_manager.index` setting (elastic#84155)
  [Search] Search batching using bfetch (again) (elastic#84043)
  Use .kibana instead of .kibana_current to mark migration completion (elastic#83373)
  [Monitoring] Only look at ES for the missing data alert for now (elastic#83839)
  ...
@ThomThomson ThomThomson mentioned this pull request Dec 14, 2020
Closed
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@poffdeluxe poffdeluxe poffdeluxe approved these changes

Assignees
No one assigned
Labels
Feature:Dashboard Dashboard related features release_note:fix v7.11.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Time to Visualize] Unlink from Library does not properly delete savedObjectId
3 participants
@ThomThomson @kibanamachine @poffdeluxe