[Dashboard Navigation] Make links panel available under technical preview #166896
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit dcc37c6.
Closes #154360 ## Summary This PR adds the first draft of the UI for creating the navigation embeddable and adding links to it. Note that this PR **only** addresses adding links - you cannot currently remove links from the panel or edit existing links. Also, because this PR contains critical code, some cleanup tasks were left as `TODO` in order to get it merged ASAP - these will be addressed in follow up PRs. https://github.com/elastic/kibana/assets/8698078/ac011207-1c4b-40cd-a151-78addb99d32d ### Checklist - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] ~[Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios~ Will be addressed in #161287 - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <[email protected]>
Closes #161552 ## Summary - Adds an illustration to the empty state in both light and dark mode. - Updates copy as suggested by Gail. ### Updated design <img width="450" alt="image" src="https://github.com/elastic/kibana/assets/4016496/bccb709f-a553-483d-942a-e6dd5ffa0b26"> <img width="450" alt="image" src="https://github.com/elastic/kibana/assets/4016496/b8f69218-911d-4a5a-9185-ed86a81ef652"> ### Checklist Delete any items that are not applicable to this PR. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### Risk Matrix Delete this section if it is not applicable to this PR. Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release. When forming the risk matrix, consider some of the following examples and how they may potentially impact the change: | Risk | Probability | Severity | Mitigation/Notes | |---------------------------|-------------|----------|-------------------------| | Multiple Spaces—unexpected behavior in non-default Kibana Space. | Low | High | Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces. | | Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. | High | Low | Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure. | | Code should gracefully handle cases when feature X or plugin Y are disabled. | Medium | High | Unit tests will verify that any feature flag or plugin combination still results in our service operational. | | [See more potential risk examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) | ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
Closes #154361 Closes #161274 Closes #161693 ## Summary This PR adds editing capabilities to the navigation embeddable, including deleting/editing existing links and reordering the list of links. It also fixes the delay in opening the editing flyout from the async import in `getExplicitInput` (from the navigation embeddable factory) by moving it to the constructor of the factory. https://github.com/elastic/kibana/assets/8698078/ace9dcd4-0607-40de-959e-94348a5fa4fa ### Checklist - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] ~[Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios~ Will be addressed in #161287 - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <[email protected]>
Fixes #154362 ## Summary Adds content management to navigation embeddable feature branch. Allows Links panels to be by-value or by-reference on a Dashboard. The UX for users to choose to save by-value or by-reference remains to be finalized and is out of scope for this PR. --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Hannah Mudge <[email protected]>
… error handling (#162285) Closes #154357 Closes #161563 ## Summary > **Warning** > I will be waiting to merge this PR until **after** #160896 is merged - I am simply opening it early so that we can start the design review process 👍 ### Layout This PR improves the rendering of the navigation embeddable to include both a horizontal and vertical layout option, as well as changing the style of how the links are rendered: https://github.com/elastic/kibana/assets/8698078/37d27683-a6c4-4e7a-9589-0eb0fb899e98 A known issue with the horizontal layout is that, as demonstrated in the above video, a "compact" horizontal navigation panel does not render as nicely in edit mode versus view mode - this is an **overall panel problem** and not specifically a problem with the navigation embeddable (although the navigation embeddable definitely makes it more obvious). This will be resolved for **all panels** by [removing the panel header altogether](#162182). ### Error handling This PR adds proper error handling to the navigation embeddable so that, if a dashboard link is "broken" (i.e. the destination dashboard has been deleted or cannot be fetched), an appropriate error message shows up in both the component and the editor flyout: https://github.com/elastic/kibana/assets/8698078/33a3e573-36a2-47ca-b367-3e04f9541ca3 > **Note** > When possible, we want to provide the user with as much context as possible for broken dashboard links - that is why, if a dashboard link was given a custom label, we still show this custom label even when the destination dashboard has been deleted/is unreachable. > > However, once a dashboard has been deleted, we no longer know what the title of that dashboard was because the saved object no longer exists - so, if a dashboard link is **not** given a custom label and the destination dashboard is deleted, we default to the "Error fetching dashboard" error message instead. In order to create a distinction between these two scenarios (a broken dashboard link with a custom label versus without), we italicize the generic "Error fetching dashboard" error text. ### Improved efficiency Previously, the navigation embeddable was handling its **own** dashboard cache, which meant that (a) every single embeddable had its own cache and (b) the navigation embeddable code had to be mindful when choosing to use the memoized/cached version of the dashboard versus fetching it fresh. After discussing with @ThomThomson about how to better handle this, we opted to move this logic to the dashboard content management service - not only does this clean up the navigation embeddable code, it also improves all the loading of dashboards in general. For example, consider the following video where I was testing re-loading a previously loaded dashboard on a throttled `Slow 3G` network: https://github.com/elastic/kibana/assets/8698078/41d68ac7-557c-4586-a59b-7268086991dd Notice in the above video how much faster the secondary load of the dashboard is in comparison to the first initial load - this is because, in the second load, we can hit the cache instead of re-fetching the dashboard from the content management client, which allows us to skip an entire loading state. ### Checklist - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] ~[Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios~ Will be addressed in #161287 - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: Andrea Del Rio <[email protected]> Co-authored-by: kibanamachine <[email protected]>
Fixes #154363 ## Summary Extracts and injects references to Dashboards in Link panels to allow sharing / importing to other Spaces. To test this: 1. Create an empty dashboard. 1. Create a Links panel on the dashboard and save to library. 1. Save the dashboard as one or more new dashboards. 1. Update the Links panel with links to the new dashboards. 1. Create two new Spaces, "space1" and "space2" with the default settings. 1. In Saved Objects, click the Actions button for your Links and choose "Copy to Spaces". 1. Copy the Links to "space1". 1. Switch to the "space1" space and verify all the dashboards using that Links panel were included. 1. In Saved Objects, select the Links saved object and export it including all related objects 1. Switch to "space2" and import the Links saved object from the file downloaded in the previous step. 1. Verify all the dashboards were also imported. 1. Inspect the saved object for the Links. The `references` array should contain objects for each dashboard. 1. The `attributes.links[].destination` property should be a string in the format of `links_<someuuid>_dashboard`. This string should match one of the `references[].id`. --------- Co-authored-by: kibanamachine <[email protected]>
…vigation-embeddable
Closes #154381 ## Summary This PR adds functionality to the link embeddable so that 1) dashboard to dashboard links will now navigate between dashboards, taking the context (filters, time range, queries, etc.) as the user navigates (assuming the appropriate settings are applied) and 2) URL links will now navigate to internal and/or external URLs as expected ### Dashboard Links https://github.com/elastic/kibana/assets/8698078/1034b454-3add-48c2-8505-44d89e2d87d0 > **Note** > In the above video, all links were configured to include queries, filter pills, and the selected time range in the passed context. It is possible to disable these settings, if necessary. #### Link settings We want dashboard drilldowns and dashboard links to work more-or-less the same way, which means that they should have identical settings - therefore, in order to share the same settings between dashboard drilldowns + dashboard links, I created the `DashboardDrilldownOptionsComponent`:  I put this new component into `presentation_util` so that it could easily be shared and used in the respective editing experiences. ### Testing - When testing, special attention should be paid to the different link settings - ensure that all settings are respected. - Ensure that `ctrl + click`, `shift + click`, and `meta + click` work as expected, regardless of whether the `Open in new tab` setting is true or false - specifically, we should use the **URL** to pass the filters/queries/time range whenever a dashboard is being opened in a new tab. ### URL Links https://github.com/elastic/kibana/assets/8698078/5abf9772-17b3-4ab4-a704-4a3ff432fa23 #### Link settings Similar to the dashboard links, we want the  ### Testing When testing, it is important to ensure that the new URL validation works as expected. For this, there are technically two types of invalid links: 1) links that do not fit the expected format; for example, we do not accept any links that start with something other than `http`, `https`, or `mailto`. 2) links that are disallowed due to the `externalUrl.policy` Testing number one should be simple; however, in order to test the second type of invalid links, you need to modify your `kibana.dev.yml` to include something like the following: ``` externalUrl.policy: - allow: false host: danger.example.com - allow: true host: example.com protocol: https ``` Refer to http://www.elastic.co/guide/en/kibana/current/url-drilldown-settings-kb.html for more information. Also, much like with the dashboard links, it is important to ensure that `ctrl + click`, `shift + click`, and `meta + click` work as expected, regardless of whether the `Open in new tab` setting is true or false. ### Checklist - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] ~[Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios~ Will be addressed in #161287 - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) > **Note** > How Chrome and Firefox handle modified `onClick` events varies significantly and, through this testing, I found a bug where `shift + click`ing was previously not opening the links in a new tab in Firefox specifically - this is why I had to use the default to `href` behaviour whenever possible. Something to keep in mind when testing. ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Nick Peihl <[email protected]> Co-authored-by: Nick Peihl <[email protected]>
Fixes #164114 ## Summary Disables the new Links panel in Canvas. Links panel is a dashboard only feature. Canvas does not have any use for the Links panels. Canvas supports adding multiple pages to a workpad which offers a similar functionality.
Adds the ability for Links Panels to choose their own size and placement strategy on creation.
Heenawter
added
Team:Presentation
|
Documentation preview: |
💛 Build succeeded, but was flakyFailed CI StepsTest Failures
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
async chunk count
ESLint disabled in files
ESLint disabled line counts
References to deprecated APIs
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
kibanamachine
added
v8.11.0
backport:skip
|
How to view the links menu without superuser role? In my case, editor users are viewing a ! mark in the links widget. Edit: I had to allow read Saved Objects Management in user roles. |
|
@andremacola Testing this, and looks like it is a genuine bug - I've created an issue to track. In the meantime, if you convert your links panels to by-value (by unlinking them from the library via the panel action), your users should no longer need the
We know this isn't the most convenient solution 🙇 Sorry about that - we'll continue to look into this! |
This was referenced Dec 13, 2023
3 tasks
Heenawter
added a commit
that referenced
this pull request
Jan 26, 2024
## Summary This PR fixes the alias redirect problem that was noted in the attached SDH. For context, as part of the [Links panel work](#166896), I added [dashboard caching](#162285) to make navigation more efficient - however, when implementing this, I did not consider the redirect behaviour. Specifically, we were **always** adding dashboards to the cache, even if the load outcome meant a redirect was necessary - so, because the meta info for the dashboard fetch result was cached, this meant that the `'aliasMatch'` outcome was **also** cached. Because of this, after a redirect happened, the second attempt to load the dashboard would return the result from the **cache** rather than fetching the dashboard from the CM client - but, as described previously, the cached dashboard would still appear as though it required a redirect because the cached outcome was `'aliasMatch'`. Therefore, because of hitting this early return on **both** fetch attempts (before the redirect, after the redirect)... https://github.com/elastic/kibana/blob/4565b1bfba91cd24f73f8ce37fa7be73283a4453/src/plugins/dashboard/public/dashboard_container/embeddable/create/create_dashboard.ts#L165-L171 ... the dashboard information would not get updated. _(Oof!)_ Despite the lengthy description of the problem, the solution is quite simple - just don't add dashboards to the cache if they require a redirect. And then everything works! 🎉 ### Videos **Before** https://github.com/elastic/kibana/assets/8698078/983b81a6-acf3-4e71-804a-6ed75c36896f **After** https://github.com/elastic/kibana/assets/8698078/028369ff-f73f-43e6-9abb-b35f0d2f33e1 ### Testing To test, I followed the directions from [this PR description](#163658) to get a dashboard that requires a redirect - then, I created a markdown panel with the **old** (from the Default space) dashboard ID in the new space to see the same redirect behaviour that the customer in the SDH was seeing. ### Checklist - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Jan 26, 2024
## Summary This PR fixes the alias redirect problem that was noted in the attached SDH. For context, as part of the [Links panel work](elastic#166896), I added [dashboard caching](elastic#162285) to make navigation more efficient - however, when implementing this, I did not consider the redirect behaviour. Specifically, we were **always** adding dashboards to the cache, even if the load outcome meant a redirect was necessary - so, because the meta info for the dashboard fetch result was cached, this meant that the `'aliasMatch'` outcome was **also** cached. Because of this, after a redirect happened, the second attempt to load the dashboard would return the result from the **cache** rather than fetching the dashboard from the CM client - but, as described previously, the cached dashboard would still appear as though it required a redirect because the cached outcome was `'aliasMatch'`. Therefore, because of hitting this early return on **both** fetch attempts (before the redirect, after the redirect)... https://github.com/elastic/kibana/blob/4565b1bfba91cd24f73f8ce37fa7be73283a4453/src/plugins/dashboard/public/dashboard_container/embeddable/create/create_dashboard.ts#L165-L171 ... the dashboard information would not get updated. _(Oof!)_ Despite the lengthy description of the problem, the solution is quite simple - just don't add dashboards to the cache if they require a redirect. And then everything works! 🎉 ### Videos **Before** https://github.com/elastic/kibana/assets/8698078/983b81a6-acf3-4e71-804a-6ed75c36896f **After** https://github.com/elastic/kibana/assets/8698078/028369ff-f73f-43e6-9abb-b35f0d2f33e1 ### Testing To test, I followed the directions from [this PR description](elastic#163658) to get a dashboard that requires a redirect - then, I created a markdown panel with the **old** (from the Default space) dashboard ID in the new space to see the same redirect behaviour that the customer in the SDH was seeing. ### Checklist - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) (cherry picked from commit 27b34d5)
kibanamachine
added a commit
that referenced
this pull request
Jan 26, 2024
# Backport This will backport the following commits from `main` to `8.12`: - [[Dashboard] Only cache non-alias dashboards (#175635)](#175635) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Hannah Mudge","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-01-26T16:09:28Z","message":"[Dashboard] Only cache non-alias dashboards (#175635)\n\n## Summary\r\n\r\nThis PR fixes the alias redirect problem that was noted in the attached\r\nSDH.\r\n\r\nFor context, as part of the [Links panel\r\nwork](#166896), I added [dashboard\r\ncaching](#162285) to make\r\nnavigation more efficient - however, when implementing this, I did not\r\nconsider the redirect behaviour.\r\n\r\nSpecifically, we were **always** adding dashboards to the cache, even if\r\nthe load outcome meant a redirect was necessary - so, because the meta\r\ninfo for the dashboard fetch result was cached, this meant that the\r\n`'aliasMatch'` outcome was **also** cached. Because of this, after a\r\nredirect happened, the second attempt to load the dashboard would return\r\nthe result from the **cache** rather than fetching the dashboard from\r\nthe CM client - but, as described previously, the cached dashboard would\r\nstill appear as though it required a redirect because the cached outcome\r\nwas `'aliasMatch'`.\r\n\r\nTherefore, because of hitting this early return on **both** fetch\r\nattempts (before the redirect, after the redirect)...\r\n\r\n\r\nhttps://github.com/elastic/kibana/blob/4565b1bfba91cd24f73f8ce37fa7be73283a4453/src/plugins/dashboard/public/dashboard_container/embeddable/create/create_dashboard.ts#L165-L171\r\n\r\n... the dashboard information would not get updated. _(Oof!)_ \r\n\r\nDespite the lengthy description of the problem, the solution is quite\r\nsimple - just don't add dashboards to the cache if they require a\r\nredirect. And then everything works! 🎉\r\n\r\n### Videos\r\n**Before**\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/8698078/983b81a6-acf3-4e71-804a-6ed75c36896f\r\n\r\n\r\n**After**\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/8698078/028369ff-f73f-43e6-9abb-b35f0d2f33e1\r\n\r\n\r\n### Testing\r\nTo test, I followed the directions from [this PR\r\ndescription](#163658) to get a\r\ndashboard that requires a redirect - then, I created a markdown panel\r\nwith the **old** (from the Default space) dashboard ID in the new space\r\nto see the same redirect behaviour that the customer in the SDH was\r\nseeing.\r\n\r\n### Checklist\r\n- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"27b34d591111a878001eaac7b0f3ef0173bc8f02","branchLabelMapping":{"^v8.13.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","Feature:Dashboard","release_note:fix","Team:Presentation","loe:small","impact:high","backport:prev-minor","v8.12.1","v8.13.0"],"title":"[Dashboard] Only cache non-alias dashboards","number":175635,"url":"https://github.com/elastic/kibana/pull/175635","mergeCommit":{"message":"[Dashboard] Only cache non-alias dashboards (#175635)\n\n## Summary\r\n\r\nThis PR fixes the alias redirect problem that was noted in the attached\r\nSDH.\r\n\r\nFor context, as part of the [Links panel\r\nwork](#166896), I added [dashboard\r\ncaching](#162285) to make\r\nnavigation more efficient - however, when implementing this, I did not\r\nconsider the redirect behaviour.\r\n\r\nSpecifically, we were **always** adding dashboards to the cache, even if\r\nthe load outcome meant a redirect was necessary - so, because the meta\r\ninfo for the dashboard fetch result was cached, this meant that the\r\n`'aliasMatch'` outcome was **also** cached. Because of this, after a\r\nredirect happened, the second attempt to load the dashboard would return\r\nthe result from the **cache** rather than fetching the dashboard from\r\nthe CM client - but, as described previously, the cached dashboard would\r\nstill appear as though it required a redirect because the cached outcome\r\nwas `'aliasMatch'`.\r\n\r\nTherefore, because of hitting this early return on **both** fetch\r\nattempts (before the redirect, after the redirect)...\r\n\r\n\r\nhttps://github.com/elastic/kibana/blob/4565b1bfba91cd24f73f8ce37fa7be73283a4453/src/plugins/dashboard/public/dashboard_container/embeddable/create/create_dashboard.ts#L165-L171\r\n\r\n... the dashboard information would not get updated. _(Oof!)_ \r\n\r\nDespite the lengthy description of the problem, the solution is quite\r\nsimple - just don't add dashboards to the cache if they require a\r\nredirect. And then everything works! 🎉\r\n\r\n### Videos\r\n**Before**\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/8698078/983b81a6-acf3-4e71-804a-6ed75c36896f\r\n\r\n\r\n**After**\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/8698078/028369ff-f73f-43e6-9abb-b35f0d2f33e1\r\n\r\n\r\n### Testing\r\nTo test, I followed the directions from [this PR\r\ndescription](#163658) to get a\r\ndashboard that requires a redirect - then, I created a markdown panel\r\nwith the **old** (from the Default space) dashboard ID in the new space\r\nto see the same redirect behaviour that the customer in the SDH was\r\nseeing.\r\n\r\n### Checklist\r\n- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"27b34d591111a878001eaac7b0f3ef0173bc8f02"}},"sourceBranch":"main","suggestedTargetBranches":["8.12"],"targetPullRequestStates":[{"branch":"8.12","label":"v8.12.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.13.0","branchLabelMappingKey":"^v8.13.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/175635","number":175635,"mergeCommit":{"message":"[Dashboard] Only cache non-alias dashboards (#175635)\n\n## Summary\r\n\r\nThis PR fixes the alias redirect problem that was noted in the attached\r\nSDH.\r\n\r\nFor context, as part of the [Links panel\r\nwork](#166896), I added [dashboard\r\ncaching](#162285) to make\r\nnavigation more efficient - however, when implementing this, I did not\r\nconsider the redirect behaviour.\r\n\r\nSpecifically, we were **always** adding dashboards to the cache, even if\r\nthe load outcome meant a redirect was necessary - so, because the meta\r\ninfo for the dashboard fetch result was cached, this meant that the\r\n`'aliasMatch'` outcome was **also** cached. Because of this, after a\r\nredirect happened, the second attempt to load the dashboard would return\r\nthe result from the **cache** rather than fetching the dashboard from\r\nthe CM client - but, as described previously, the cached dashboard would\r\nstill appear as though it required a redirect because the cached outcome\r\nwas `'aliasMatch'`.\r\n\r\nTherefore, because of hitting this early return on **both** fetch\r\nattempts (before the redirect, after the redirect)...\r\n\r\n\r\nhttps://github.com/elastic/kibana/blob/4565b1bfba91cd24f73f8ce37fa7be73283a4453/src/plugins/dashboard/public/dashboard_container/embeddable/create/create_dashboard.ts#L165-L171\r\n\r\n... the dashboard information would not get updated. _(Oof!)_ \r\n\r\nDespite the lengthy description of the problem, the solution is quite\r\nsimple - just don't add dashboards to the cache if they require a\r\nredirect. And then everything works! 🎉\r\n\r\n### Videos\r\n**Before**\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/8698078/983b81a6-acf3-4e71-804a-6ed75c36896f\r\n\r\n\r\n**After**\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/8698078/028369ff-f73f-43e6-9abb-b35f0d2f33e1\r\n\r\n\r\n### Testing\r\nTo test, I followed the directions from [this PR\r\ndescription](#163658) to get a\r\ndashboard that requires a redirect - then, I created a markdown panel\r\nwith the **old** (from the Default space) dashboard ID in the new space\r\nto see the same redirect behaviour that the customer in the SDH was\r\nseeing.\r\n\r\n### Checklist\r\n- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"27b34d591111a878001eaac7b0f3ef0173bc8f02"}}]}] BACKPORT--> Co-authored-by: Hannah Mudge <[email protected]>
CoenWarmer
pushed a commit
to CoenWarmer/kibana
that referenced
this pull request
Feb 15, 2024
## Summary This PR fixes the alias redirect problem that was noted in the attached SDH. For context, as part of the [Links panel work](elastic#166896), I added [dashboard caching](elastic#162285) to make navigation more efficient - however, when implementing this, I did not consider the redirect behaviour. Specifically, we were **always** adding dashboards to the cache, even if the load outcome meant a redirect was necessary - so, because the meta info for the dashboard fetch result was cached, this meant that the `'aliasMatch'` outcome was **also** cached. Because of this, after a redirect happened, the second attempt to load the dashboard would return the result from the **cache** rather than fetching the dashboard from the CM client - but, as described previously, the cached dashboard would still appear as though it required a redirect because the cached outcome was `'aliasMatch'`. Therefore, because of hitting this early return on **both** fetch attempts (before the redirect, after the redirect)... https://github.com/elastic/kibana/blob/4565b1bfba91cd24f73f8ce37fa7be73283a4453/src/plugins/dashboard/public/dashboard_container/embeddable/create/create_dashboard.ts#L165-L171 ... the dashboard information would not get updated. _(Oof!)_ Despite the lengthy description of the problem, the solution is quite simple - just don't add dashboards to the cache if they require a redirect. And then everything works! 🎉 ### Videos **Before** https://github.com/elastic/kibana/assets/8698078/983b81a6-acf3-4e71-804a-6ed75c36896f **After** https://github.com/elastic/kibana/assets/8698078/028369ff-f73f-43e6-9abb-b35f0d2f33e1 ### Testing To test, I followed the directions from [this PR description](elastic#163658) to get a dashboard that requires a redirect - then, I created a markdown panel with the **old** (from the Default space) dashboard ID in the new space to see the same redirect behaviour that the customer in the SDH was seeing. ### Checklist - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR wraps up the work the @elastic/kibana-presentation team has done to finish the MVP of Phase 1 of the
Linkembeddable, which enables users to add panels to their dashboard that contain links to other dashboards + external links - with respect to dashboard links, we give the author control over which pieces of context should be kept across dashboards so that things like filter pills, queries, and time ranges are not lost. This marks a huge improvement in dashboard navigation overall, which was previously only available via a variety of different workarounds including (but not limited to):noopdashboard-to-dashboard drilldownAs an added benefit, because these panels contain references / hard links to each dashboard rather than soft links, (1) unlike markdown links, they should not break after updates and (2) if a links panel is exported and imported into another space or instance, all of the dashboards it links to will also be imported.
LinksPanelDemo.mp4
Note about this PR
Flaky Test Runner
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/3251
Checklist
Documentation was added for features that require explanation or tutorialsThis will be addressed in a follow up: [Dashboard Navigation] Documentation #166750Units tests are added, functional tests are forthcomingEdit: All tests are in.For maintainers