mirror of
https://github.com/elastic/kibana.git
synced 2025-04-24 09:48:58 -04:00
[8.13] [Dashboard] [Controls] Fix bug with drilldowns when source dashboard has no controls (#179485) (#179842)
# Backport This will backport the following commits from `main` to `8.13`: - [[Dashboard] [Controls] Fix bug with drilldowns when source dashboard has no controls (#179485)](https://github.com/elastic/kibana/pull/179485) **Note:** This only fixes the "no controls" bug - the race condition also fixed in that PR does not need to be backported, since the code that caused it was not merged in 8.13. <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Hannah Mudge","email":"Heenawter@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-04-02T14:10:53Z","message":"[Dashboard] [Controls] Fix bug with drilldowns when source dashboard has no controls (#179485)\n\nCloses https://github.com/elastic/kibana/issues/179391\r\n\r\n## Summary\r\n\r\nWe were previously only calling `setSavedState` for the control group on\r\ndashboard navigation **when the control group was defined in the loaded\r\ndashboard** - however, if either the source or destination dashboard had\r\nzero controls, this caused problems on navigation:\r\n- If the source dashboard had at least one control and the destination\r\ndashboard had zero, the destination dashboard's control group\r\n`lastSavedInput` would **not** get set in `navigateToDashboard` since it\r\nis undefined - i.e. the `lastSavedInput` on the destination dashboard's\r\ncontrol group would **still be equal to** the `lastSavedInput` of the\r\nsource dashboard's control group after navigation. Therefore, hitting\r\n\"reset\" would replace the destination dashboard's empty control group\r\nwith the source's control group.\r\n- If the source dashboard had zero controls and the destination\r\ndashboard had at least one, the first step in navigation would work as\r\nexpected - the `lastSavedInput` of the destination dashboard would be\r\nset appropriately. However, upon hitting the browser back button and\r\ntriggering `navigateToDashboard` a second time, the source dashboard's\r\ncontrol group's `lastSavedInput` would **not** get set properly (since\r\nit is undefined) and would therefore still be equal to the\r\n`lastSavedInput` of the destination dashboard. Therefore, hitting\r\n\"reset\" would replace the source's empty control group with the\r\ndestination dashboard's controls.\r\n\r\nThis fixes the above scenarios by calling `setSavedState` on the control\r\ngroup **even if** the last saved control group state is undefined.\r\n\r\n### Race Condition Fix\r\n\r\nIn my testing for this, I discovered **another** bug caused by a race\r\ncondition where, on dashboard navigation, the subscription to the\r\ncontrol group's `initialize# Backport This will backport the following commits from `main` to `8.13`: - [[Dashboard] [Controls] Fix bug with drilldowns when source dashboard has no controls (#179485)](https://github.com/elastic/kibana/pull/179485) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT subject was firing with the **wrong**\r\ninput, so the `lastSavedFilters` would be calculated incorrectly. This\r\ncaused the dashboard to get stuck in an unsaved changes state, like so:\r\n\r\n\r\n\r\n14c553a9
-21b3-40d0-96ac-0282e9e66911\r\n\r\n\r\n\r\nI fixed this by removing this subscription (it was messy anyway 🙈) and\r\nreplaced this logic with individual calls to\r\n`calculateFiltersFromSelections` - this should be easier to follow, and\r\nwe ensure that we are **always** doing this calculation with the\r\nexpected input. Since we aren't `awaiting` these calculations, it also\r\nshouldn't slow down the control group's initialization (which was the\r\noriginal reason for using a subscription).\r\n\r\n\r\n### Checklist\r\n\r\n\r\n- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n- [x] Flaky test runner -\r\ncd8ce150
-6104-4395-b70c-7309185cc04d)\r\n\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":"507f09d0b69a99f03f0c0cb19f10f1ea8584cd47","branchLabelMapping":{"^v8.14.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Presentation","loe:small","impact:high","backport:prev-minor","v8.14.0"],"number":179485,"url":"https://github.com/elastic/kibana/pull/179485","mergeCommit":{"message":"[Dashboard] [Controls] Fix bug with drilldowns when source dashboard has no controls (#179485)\n\nCloses https://github.com/elastic/kibana/issues/179391\r\n\r\n## Summary\r\n\r\nWe were previously only calling `setSavedState` for the control group on\r\ndashboard navigation **when the control group was defined in the loaded\r\ndashboard** - however, if either the source or destination dashboard had\r\nzero controls, this caused problems on navigation:\r\n- If the source dashboard had at least one control and the destination\r\ndashboard had zero, the destination dashboard's control group\r\n`lastSavedInput` would **not** get set in `navigateToDashboard` since it\r\nis undefined - i.e. the `lastSavedInput` on the destination dashboard's\r\ncontrol group would **still be equal to** the `lastSavedInput` of the\r\nsource dashboard's control group after navigation. Therefore, hitting\r\n\"reset\" would replace the destination dashboard's empty control group\r\nwith the source's control group.\r\n- If the source dashboard had zero controls and the destination\r\ndashboard had at least one, the first step in navigation would work as\r\nexpected - the `lastSavedInput` of the destination dashboard would be\r\nset appropriately. However, upon hitting the browser back button and\r\ntriggering `navigateToDashboard` a second time, the source dashboard's\r\ncontrol group's `lastSavedInput` would **not** get set properly (since\r\nit is undefined) and would therefore still be equal to the\r\n`lastSavedInput` of the destination dashboard. Therefore, hitting\r\n\"reset\" would replace the source's empty control group with the\r\ndestination dashboard's controls.\r\n\r\nThis fixes the above scenarios by calling `setSavedState` on the control\r\ngroup **even if** the last saved control group state is undefined.\r\n\r\n### Race Condition Fix\r\n\r\nIn my testing for this, I discovered **another** bug caused by a race\r\ncondition where, on dashboard navigation, the subscription to the\r\ncontrol group's `initialize# Backport This will backport the following commits from `main` to `8.13`: - [[Dashboard] [Controls] Fix bug with drilldowns when source dashboard has no controls (#179485)](https://github.com/elastic/kibana/pull/179485) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT subject was firing with the **wrong**\r\ninput, so the `lastSavedFilters` would be calculated incorrectly. This\r\ncaused the dashboard to get stuck in an unsaved changes state, like so:\r\n\r\n\r\n\r\n14c553a9
-21b3-40d0-96ac-0282e9e66911\r\n\r\n\r\n\r\nI fixed this by removing this subscription (it was messy anyway 🙈) and\r\nreplaced this logic with individual calls to\r\n`calculateFiltersFromSelections` - this should be easier to follow, and\r\nwe ensure that we are **always** doing this calculation with the\r\nexpected input. Since we aren't `awaiting` these calculations, it also\r\nshouldn't slow down the control group's initialization (which was the\r\noriginal reason for using a subscription).\r\n\r\n\r\n### Checklist\r\n\r\n\r\n- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n- [x] Flaky test runner -\r\ncd8ce150
-6104-4395-b70c-7309185cc04d)\r\n\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":"507f09d0b69a99f03f0c0cb19f10f1ea8584cd47"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.14.0","labelRegex":"^v8.14.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/179485","number":179485,"mergeCommit":{"message":"[Dashboard] [Controls] Fix bug with drilldowns when source dashboard has no controls (#179485)\n\nCloses https://github.com/elastic/kibana/issues/179391\r\n\r\n## Summary\r\n\r\nWe were previously only calling `setSavedState` for the control group on\r\ndashboard navigation **when the control group was defined in the loaded\r\ndashboard** - however, if either the source or destination dashboard had\r\nzero controls, this caused problems on navigation:\r\n- If the source dashboard had at least one control and the destination\r\ndashboard had zero, the destination dashboard's control group\r\n`lastSavedInput` would **not** get set in `navigateToDashboard` since it\r\nis undefined - i.e. the `lastSavedInput` on the destination dashboard's\r\ncontrol group would **still be equal to** the `lastSavedInput` of the\r\nsource dashboard's control group after navigation. Therefore, hitting\r\n\"reset\" would replace the destination dashboard's empty control group\r\nwith the source's control group.\r\n- If the source dashboard had zero controls and the destination\r\ndashboard had at least one, the first step in navigation would work as\r\nexpected - the `lastSavedInput` of the destination dashboard would be\r\nset appropriately. However, upon hitting the browser back button and\r\ntriggering `navigateToDashboard` a second time, the source dashboard's\r\ncontrol group's `lastSavedInput` would **not** get set properly (since\r\nit is undefined) and would therefore still be equal to the\r\n`lastSavedInput` of the destination dashboard. Therefore, hitting\r\n\"reset\" would replace the source's empty control group with the\r\ndestination dashboard's controls.\r\n\r\nThis fixes the above scenarios by calling `setSavedState` on the control\r\ngroup **even if** the last saved control group state is undefined.\r\n\r\n### Race Condition Fix\r\n\r\nIn my testing for this, I discovered **another** bug caused by a race\r\ncondition where, on dashboard navigation, the subscription to the\r\ncontrol group's `initialize# Backport This will backport the following commits from `main` to `8.13`: - [[Dashboard] [Controls] Fix bug with drilldowns when source dashboard has no controls (#179485)](https://github.com/elastic/kibana/pull/179485) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT subject was firing with the **wrong**\r\ninput, so the `lastSavedFilters` would be calculated incorrectly. This\r\ncaused the dashboard to get stuck in an unsaved changes state, like so:\r\n\r\n\r\n\r\n14c553a9
-21b3-40d0-96ac-0282e9e66911\r\n\r\n\r\n\r\nI fixed this by removing this subscription (it was messy anyway 🙈) and\r\nreplaced this logic with individual calls to\r\n`calculateFiltersFromSelections` - this should be easier to follow, and\r\nwe ensure that we are **always** doing this calculation with the\r\nexpected input. Since we aren't `awaiting` these calculations, it also\r\nshouldn't slow down the control group's initialization (which was the\r\noriginal reason for using a subscription).\r\n\r\n\r\n### Checklist\r\n\r\n\r\n- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n- [x] Flaky test runner -\r\ncd8ce150
-6104-4395-b70c-7309185cc04d)\r\n\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":"507f09d0b69a99f03f0c0cb19f10f1ea8584cd47"}}]}] BACKPORT-->
This commit is contained in:
parent
536a7dd3cd
commit
e5027f51e5
3 changed files with 4 additions and 4 deletions
|
@ -210,7 +210,7 @@ export class ControlGroupContainer extends Container<
|
|||
componentState: { lastSavedInput },
|
||||
} = this.getState();
|
||||
if (!persistableControlGroupInputIsEqual(this.getPersistableInput(), lastSavedInput)) {
|
||||
this.updateInput(lastSavedInput);
|
||||
this.updateInput(lastSavedInput ?? {});
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -41,7 +41,7 @@ export interface ControlGroupSettings {
|
|||
}
|
||||
|
||||
export type ControlGroupComponentState = ControlGroupSettings & {
|
||||
lastSavedInput: PersistableControlGroupInput;
|
||||
lastSavedInput?: PersistableControlGroupInput;
|
||||
};
|
||||
|
||||
export {
|
||||
|
|
|
@ -603,9 +603,9 @@ export class DashboardContainer
|
|||
omit(loadDashboardReturn?.dashboardInput, 'controlGroupInput')
|
||||
);
|
||||
this.dispatch.setManaged(loadDashboardReturn?.managed);
|
||||
if (this.controlGroup && loadDashboardReturn?.dashboardInput.controlGroupInput) {
|
||||
if (this.controlGroup) {
|
||||
this.controlGroup.dispatch.setLastSavedInput(
|
||||
loadDashboardReturn?.dashboardInput.controlGroupInput
|
||||
loadDashboardReturn.dashboardInput?.controlGroupInput
|
||||
);
|
||||
}
|
||||
this.dispatch.setAnimatePanelTransforms(false); // prevents panels from animating on navigate.
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue