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.

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

[Dashboard] Move all dashboard extract/inject into persistable state #96095

Merged
merged 13 commits into from
Apr 12, 2021

Conversation

crob611
Copy link
Contributor

@crob611 crob611 commented Apr 1, 2021

Closes #92900

Summary

This moves the dashboard references extract/inject logic into its embeddable factory as part of the Embeddable Persistable State interface and properly names the references so there is no chance of overlap (#92900)

Because there are existing dashboard migrations that run the extract/inject logic server side, I had to introduce the dashboard embeddable server side as well to ensure the same extract/inject logic happens there as well.

Because importing a saved object immediately runs it through the saved object save process, a pre 7.3 dashboard could cause problems, as was previously fixed by #94332. So, we kept the existing "pull out the saved object references" logic as well specifically for the fallback for those older dashboard versions that will fail if run through the persistable state extract/inject.

Testing

Important areas to test around this PR

  • Save a dashboard with a by ref panel. Ensure it loads properly. Ensure the saved object of the panel is referenced when viewing the dashboard's relationships
  • Save a dashboard with a by value panel. Ensure it loads properly. Ensure any references from that nested by value embeddable are referenced when viewing the dashboard's relationship.
  • Ensure an existing 7.12 dashboard loads properly. (not through import since that will go through the saved object save method. We want to make sure an already existing 7.12 dashboard will not need any changes to continue to function properly)
  • Ensure an old dashboard (<7.3) is able to be imported properly. That all the appropriate references are there once it's imported. That the dashboard itself loads up and works as expected.

@crob611 crob611 requested review from a team as code owners April 1, 2021 20:07
@botelastic botelastic bot added the Feature:Embedding Embedding content via iFrame label Apr 1, 2021
@crob611 crob611 added Feature:Dashboard Dashboard related features loe:large Large Level of Effort release_note:skip Skip the PR/issue when compiling release notes review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v7.13.0 v8.0.0 labels Apr 1, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@crob611 crob611 added the impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. label Apr 1, 2021
Copy link
Contributor

@lizozom lizozom left a comment

Choose a reason for hiding this comment

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

Only code owner review.
Didn't test.

@crob611
Copy link
Contributor Author

crob611 commented Apr 6, 2021

@elasticmachine merge upstream

@crob611
Copy link
Contributor Author

crob611 commented Apr 8, 2021

@elasticmachine merge upstream

Copy link
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

From a quick read-through the code all LGTM. Tested the following scenarios locally in Chrome:

  • Save a dashboard with a by ref panel. Ensure it loads properly. Ensure the saved object of the panel is referenced when viewing the dashboard's relationships

  • Save a dashboard with a by value panel. Ensure it loads properly. Ensure any references from that nested by value embeddable are referenced when viewing the dashboard's relationship.

  • Ensure an already existing 7.12 dashboard loads properly and doesn't need any changes to continue to function properly.

  • Imported a dashboard from 7.11, and saw that after saving all of the references were updated to the new format

Overall, everything works as expected!

@crob611
Copy link
Contributor Author

crob611 commented Apr 12, 2021

@elasticmachine merge upstream

@crob611 crob611 added the auto-backport Deprecated - use backport:version if exact versions are needed label Apr 12, 2021
@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 309.9KB 312.7KB +2.9KB

History

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

@crob611 crob611 merged commit b645fec into elastic:master Apr 12, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Apr 12, 2021
…lastic#96095)

* Move all dashboard inject/extract to be part of embeddable persistable state

* Fixes typescript errors

* Remove comments

* Fixes test

* API Doc changes

* Fix integration tests

* Fix functional testS

* Fix unit tests

* Update Dashboard plugin API to get dashboard embeddable renderer

* Fix Types

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Apr 12, 2021
…96095) (#96827)

* Move all dashboard inject/extract to be part of embeddable persistable state

* Fixes typescript errors

* Remove comments

* Fixes test

* API Doc changes

* Fix integration tests

* Fix functional testS

* Fix unit tests

* Update Dashboard plugin API to get dashboard embeddable renderer

* Fix Types

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Corey Robertson <corey.robertson@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Dashboard Dashboard related features Feature:Embedding Embedding content via iFrame impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:large Large Level of Effort release_note:skip Skip the PR/issue when compiling release notes review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v7.13.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Dashboard] Properly Extract and Inject references of by value panels
5 participants