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] Transfer state when drilldown is opened in a new tab #124770

Merged
merged 10 commits into from
Feb 11, 2022

Conversation

Heenawter
Copy link
Contributor

@Heenawter Heenawter commented Feb 5, 2022

Closes #122942

Summary

The current way of following through with a dashboard-to-dashboard drilldown is to exclusively use the Dashboard router - however, since this relies on in-memory storage, this fails when the user opens the drilldown in a new tab. To get around this, we have gone back to using URL state transfer only when the drilldown is opened in a new tab (i.e. when the getHref action is called; the onClick action remains unchanged) - this allows state to be fully transferred even when the session storage and/or in-memory storage is lost.

Video of Fix

2022-02-07_Dash2Dash-Drilldown-New-Tab.mp4

How I Tested

I tested multiple configurations of source/destination dashboards to ensure that all elements of state where transferred:

  • Use date range from origin dashboard
    • Source has no filters
      ✅ Destination has no filters
      ✅ Destination has unique local filter
    • Source has local filter
      ✅ Destination has no filters
      ✅ Destination has unique local filter
      ✅ Destination shares local filter with source
  • Don't use date range from origin dashboard
    • Source has no filters
      ✅ Destination has no filters
      ✅ Destination has unique local filter
    • Source has local filter
      ✅ Destination has no filters
      ✅ Destination has unique local filter
      ✅ Destination shares local filter with source

Note, however, that there is a bug where the unsaved changes badge won't go away for dashboard to dashboard drilldowns (fixed in #124278 which is currently not merged) - so, with all of the above tests, this bug was still present; I was simply verifying that the state was transferred as expected.

Checklist

@Heenawter Heenawter added Feature:Dashboard Dashboard related features Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas loe:small Small Level of Effort v8.0.0 impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Feature:Drilldowns Embeddable panel Drilldowns auto-backport Deprecated - use backport:version if exact versions are needed v7.16.0 v8.1.0 v7.16.1 labels Feb 5, 2022
@Heenawter Heenawter self-assigned this Feb 5, 2022
@Heenawter Heenawter force-pushed the fix_new_tab_drilldown_2022-02-04 branch 3 times, most recently from 9f8bc2b to 3d2f192 Compare February 7, 2022 18:46
@Heenawter Heenawter force-pushed the fix_new_tab_drilldown_2022-02-04 branch from 3d2f192 to 550ce12 Compare February 7, 2022 18:53
@Heenawter Heenawter added bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes v8.2.0 v7.17.0 labels Feb 7, 2022
@Heenawter Heenawter removed release_note:skip Skip the PR/issue when compiling release notes v7.16.0 v7.17.0 v7.16.1 labels Feb 9, 2022
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
dashboardEnhanced 32 88 +56

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
dashboard 136 138 +2
dashboardEnhanced 50 51 +1
total +3

Page load bundle

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

id before after diff
dashboard 65.8KB 65.8KB +83.0B
dashboardEnhanced 14.2KB 41.3KB +27.1KB
total +27.2KB
Unknown metric groups

API count

id before after diff
dashboard 138 140 +2
dashboardEnhanced 51 52 +1
total +3

References to deprecated APIs

id before after diff
dashboardEnhanced 13 0 -13

Unreferenced deprecated APIs

id before after diff
data 68 70 +2

History

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

cc @Heenawter

@Heenawter Heenawter merged commit 17a997c into elastic:main Feb 11, 2022
@Heenawter Heenawter deleted the fix_new_tab_drilldown_2022-02-04 branch February 11, 2022 21:27
@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.1 Backport failed because of merge conflicts
7.16 Backport failed because of merge conflicts
8.0 Backport failed because of merge conflicts
7.17 Backport failed because of merge conflicts

How to fix

Re-run the backport manually:

node scripts/backport --pr 124770

Questions ?

Please refer to the Backport tool documentation

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 15, 2022
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 124770 or prevent reminders by adding the backport:skip label.

Heenawter added a commit to Heenawter/kibana that referenced this pull request Feb 15, 2022
…stic#124770)

* Translate dashboard state to URL conditionally

* Add functional tests

* Fix typo in functional test descriptions

* Remove deprecated references

* Rename useUrl to be more specific

(cherry picked from commit 17a997c)

# Conflicts:
#	src/plugins/dashboard/public/index.ts
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 15, 2022
Heenawter added a commit that referenced this pull request Feb 15, 2022
…4770) (#125726)

* Translate dashboard state to URL conditionally

* Add functional tests

* Fix typo in functional test descriptions

* Remove deprecated references

* Rename useUrl to be more specific

(cherry picked from commit 17a997c)

# Conflicts:
#	src/plugins/dashboard/public/index.ts
Heenawter added a commit to Heenawter/kibana that referenced this pull request Feb 15, 2022
…stic#124770)

* Translate dashboard state to URL conditionally

* Add functional tests

* Fix typo in functional test descriptions

* Remove deprecated references

* Rename useUrl to be more specific

(cherry picked from commit 17a997c)

# Conflicts:
#	src/plugins/dashboard/public/index.ts
Heenawter added a commit that referenced this pull request Feb 16, 2022
#124770) (#125740)

* [Dashboard] Transfer state when drilldown is opened in a new tab (#124770)

* Translate dashboard state to URL conditionally

* Add functional tests

* Fix typo in functional test descriptions

* Remove deprecated references

* Rename useUrl to be more specific

(cherry picked from commit 17a997c)

# Conflicts:
#	src/plugins/dashboard/public/index.ts

* Fix lint
@reuvenik
Copy link

reuvenik commented Aug 3, 2022

Hi.
We have version 7.16 of Kibana and have this issue.
I see it was fixed for version 8. when it will be fixed for 7.16?

@ThomThomson
Copy link
Contributor

Hi @reuvenik! We aren't planning on releasing any more patches for 7.16. Additionally, this fix was only backported as far as 8.0.

@reuvenik
Copy link

reuvenik commented Aug 7, 2022

7.17 is must as per your official documentation.
can this be done there so we be able to upgrade minor version and not major version to get this fix?

@Heenawter
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
7.17

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

Heenawter added a commit to Heenawter/kibana that referenced this pull request Aug 8, 2022
…stic#124770)

* Translate dashboard state to URL conditionally

* Add functional tests

* Fix typo in functional test descriptions

* Remove deprecated references

* Rename useUrl to be more specific

(cherry picked from commit 17a997c)

# Conflicts:
#	src/plugins/dashboard/public/index.ts
@Heenawter
Copy link
Contributor Author

@reuvenik This has now been backported to 7.17 - hopefully that helps :)

@reuvenik
Copy link

reuvenik commented Aug 8, 2022

Very helpful.
Thanks!

darnautov pushed a commit to darnautov/kibana that referenced this pull request Aug 9, 2022
…ab (elastic#124770) (elastic#138278)

* [Dashboard] Transfer state when drilldown is opened in a new tab (elastic#124770)

* Translate dashboard state to URL conditionally

* Add functional tests

* Fix typo in functional test descriptions

* Remove deprecated references

* Rename useUrl to be more specific

(cherry picked from commit 17a997c)

# Conflicts:
#	src/plugins/dashboard/public/index.ts

* Fix conflicts

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
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 bug Fixes for quality problems that affect the customer experience Feature:Dashboard Dashboard related features Feature:Drilldowns Embeddable panel Drilldowns impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:small Small Level of Effort release_note:fix Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v7.17.6 v8.0.1 v8.1.0 v8.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dashboard drilldown "open in a new tab" not working
7 participants