[8.14] [Dashboard] [Controls] Fix controls getting overwritten on nav…

…igation (#187509) (#187694)

# Backport

This will backport the following commits from `main` to `8.14`:
- [[Dashboard] [Controls] Fix controls getting overwritten on navigation
(#187509)](#187509)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Hannah
Mudge","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-07-05T15:35:24Z","message":"[Dashboard]
[Controls] Fix controls getting overwritten on navigation
(#187509)\n\n## Summary\r\n\r\n\r\n> [!WARNING]\r\n> Beware - the
longest description ever for a one line change
is\r\nincoming.\r\n>\r\n>\r\n![The-Hangover-Math-GIF-source](https://github.com/elastic/kibana/assets/8698078/3ab94f20-0401-4c42-9bb2-c35b6025301b)\r\n>\r\n>
**TLDR:** We were previously `await`ing the initialization of
the\r\ncontrol group before navigating to the destination dashboard,
which\r\ncaused a race condition where, if the control group had time to
report\r\nunsaved changes before the initialization promise was
resolved, the\r\ncontrol group's input would get backed up under the
wrong ID. If we no\r\nlonger `await` control group initialization, we
remove this race\r\ncondition.\r\n\r\nPreviously, on dashboard
navigation, we were `await`ing the\r\ninitialization of the control
group before navigating to the destination\r\ndashboard - this was
because, before\r\nhttps://github.com//pull/174201, the
control group could\r\nchange its selections and the dashboard needed to
know the most\r\nup-to-date control group output before it could start
loading its\r\npanels. However, once
#175146 was\r\nmerged and the
control group started reporting its own unsaved changes,\r\nthis caused
a race condition on navigation depending on whether or not\r\nthe
dashboard had time to backup its unsaved changes to the
session\r\nstorage before the control group was
initialized.\r\n\r\n\r\n### Description of the race
condition\r\n\r\nConsider the following repro steps:\r\n\r\n1. You start
at your source dashboard (which has no controls), clear\r\nyour cache,
and slow down your network speed.\r\n2. You click on a markdown link to
navigate to your destination\r\ndashboard (which has controls).\r\n3.
You think everything worked as expected - hoorah!\r\n4. You click on a
markdown link to navigate back to your source\r\ndashboard.... but your
source dashboard now has the controls of your\r\ndestination dashboard!
What just happened?\r\n\r\n> [!NOTE]\r\n> If the initialization of the
control group happens **before the\r\ndashboard has a chance to backup
the control group input to session\r\nstorage under the wrong ID**, then
this bug does not happen - that is\r\nwhy it is important to slow down
the network speed when trying to\r\nreproduce this, and it is also why
this bug was more prevalent on Cloud\r\nthan local instances of
Kibana.\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/8698078/91f9b9e1-87f0-44aa-b596-577dd4a541f9\r\n\r\n\r\nOn
step 2 when the markdown link is clicked, this is what happens in
the\r\ncode:\r\n \r\n1. The `navigateToDashboard` method is
called.\r\n2. The control group is told to update its input and
reinitialize via\r\nthe call to
`controlGroup.updateInputAndReinitialize` in
the\r\n`initializeDashboard` method.\r\n3. The dashboard is `await`ing
the initialization of the control group\r\nbefore proceeding with
navigation.\r\n4. The control group is updated, which triggers its
`unsavedChanges`\r\nsubscription - this is comparing its own state to
that of the **source**\r\ndashboard, which is the **wrong** input to be
comparing against.\r\n5. The control group reports to the dashboard that
it **has** unsaved\r\nchanges.\r\n6. The dashboard backs up its unsaved
changes to session storage under\r\nthe wrong ID since navigation hasn't
happened yet - i.e. the\r\n**destination dashboard's** control group
input gets backed up under the\r\n**source dashboard's ID**\r\n7.
Finally, the control group reports that it is initialized and
the\r\ndashboard can proceed with navigation - so, the dashboard ID
changes and\r\nits input gets updated.\r\n8. This triggers the control
group to **once again** trigger the\r\n`unsavedChanges` subscription -
this time, the comparison occurs with\r\nthe **proper** dashboard input
(i.e. the input from the **destination**\r\ndashboard). Assuming no
previous unsaved changes, this would return\r\n**false** (i.e. the
control group reports to the dashboard that it has\r\n**no** unsaved
changes).\r\n\r\nOn step 3, that is why the destination dashboard
appears as expected -\r\nit has the correct controls, and no unsaved
changes. But then, on step\r\n4, this is what happens:\r\n\r\n1. The
`navigateToDashboard` method is called.\r\n2. We fetch the session
storage so that the \"unsaved changes\" can be\r\napplied to the
dashboard saved object\r\n3. Uh oh! As described in step 6 above, the
session storage for the\r\nsource dashboard includes the control group
input from the\r\n**destination** dashboard!\r\n4. So, when you go back
to the source, the destination dashboard's\r\ncontrols come with you 🔥 🔥
🔥\r\n\r\n\r\n### Description of the fix\r\n\r\n\r\nNow, let's instead
consider what happens when we **don't** `await` the\r\ncontrol group
initialization - if we go back to step 2 of the repro\r\nsteps, then
this is what happens in the code:\r\n\r\n\r\n1. The
`navigateToDashboard` method is called.\r\n2. The control group is told
to update its input and reinitialize via\r\nthe call to
`controlGroup.updateInputAndReinitialize` in
the\r\n`initializeDashboard` method.\r\n3. The dashboard is **not**
waiting for initialization, so it goes ahead\r\nwith navigation
**before** the control group has time to report its\r\nunsaved changes
(the control group's unsaved changes subscription
is\r\ndebounced).\r\n4. Navigation occurs and **nothing** gets backed up
to session storage! \r\n\r\nThat is why, by no longer waiting for the
control group to be\r\ninitialized on navigation, we are no longer
seeing the bug where\r\ncontrols were getting \"replaced\" on
navigation:\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/8698078/0e45a207-ff2a-46a6-9609-11a8dc5bcf67\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":"e5cc4d58fb7e0b12ef99ec69ca35fda97925de5f","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","Feature:Dashboard","release_note:fix","Team:Presentation","loe:small","impact:high","Project:Controls","backport:prev-minor","v8.16.0"],"number":187509,"url":"https://github.com/elastic/kibana/pull/187509","mergeCommit":{"message":"[Dashboard]
[Controls] Fix controls getting overwritten on navigation
(#187509)\n\n## Summary\r\n\r\n\r\n> [!WARNING]\r\n> Beware - the
longest description ever for a one line change
is\r\nincoming.\r\n>\r\n>\r\n![The-Hangover-Math-GIF-source](https://github.com/elastic/kibana/assets/8698078/3ab94f20-0401-4c42-9bb2-c35b6025301b)\r\n>\r\n>
**TLDR:** We were previously `await`ing the initialization of
the\r\ncontrol group before navigating to the destination dashboard,
which\r\ncaused a race condition where, if the control group had time to
report\r\nunsaved changes before the initialization promise was
resolved, the\r\ncontrol group's input would get backed up under the
wrong ID. If we no\r\nlonger `await` control group initialization, we
remove this race\r\ncondition.\r\n\r\nPreviously, on dashboard
navigation, we were `await`ing the\r\ninitialization of the control
group before navigating to the destination\r\ndashboard - this was
because, before\r\nhttps://github.com//pull/174201, the
control group could\r\nchange its selections and the dashboard needed to
know the most\r\nup-to-date control group output before it could start
loading its\r\npanels. However, once
#175146 was\r\nmerged and the
control group started reporting its own unsaved changes,\r\nthis caused
a race condition on navigation depending on whether or not\r\nthe
dashboard had time to backup its unsaved changes to the
session\r\nstorage before the control group was
initialized.\r\n\r\n\r\n### Description of the race
condition\r\n\r\nConsider the following repro steps:\r\n\r\n1. You start
at your source dashboard (which has no controls), clear\r\nyour cache,
and slow down your network speed.\r\n2. You click on a markdown link to
navigate to your destination\r\ndashboard (which has controls).\r\n3.
You think everything worked as expected - hoorah!\r\n4. You click on a
markdown link to navigate back to your source\r\ndashboard.... but your
source dashboard now has the controls of your\r\ndestination dashboard!
What just happened?\r\n\r\n> [!NOTE]\r\n> If the initialization of the
control group happens **before the\r\ndashboard has a chance to backup
the control group input to session\r\nstorage under the wrong ID**, then
this bug does not happen - that is\r\nwhy it is important to slow down
the network speed when trying to\r\nreproduce this, and it is also why
this bug was more prevalent on Cloud\r\nthan local instances of
Kibana.\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/8698078/91f9b9e1-87f0-44aa-b596-577dd4a541f9\r\n\r\n\r\nOn
step 2 when the markdown link is clicked, this is what happens in
the\r\ncode:\r\n \r\n1. The `navigateToDashboard` method is
called.\r\n2. The control group is told to update its input and
reinitialize via\r\nthe call to
`controlGroup.updateInputAndReinitialize` in
the\r\n`initializeDashboard` method.\r\n3. The dashboard is `await`ing
the initialization of the control group\r\nbefore proceeding with
navigation.\r\n4. The control group is updated, which triggers its
`unsavedChanges`\r\nsubscription - this is comparing its own state to
that of the **source**\r\ndashboard, which is the **wrong** input to be
comparing against.\r\n5. The control group reports to the dashboard that
it **has** unsaved\r\nchanges.\r\n6. The dashboard backs up its unsaved
changes to session storage under\r\nthe wrong ID since navigation hasn't
happened yet - i.e. the\r\n**destination dashboard's** control group
input gets backed up under the\r\n**source dashboard's ID**\r\n7.
Finally, the control group reports that it is initialized and
the\r\ndashboard can proceed with navigation - so, the dashboard ID
changes and\r\nits input gets updated.\r\n8. This triggers the control
group to **once again** trigger the\r\n`unsavedChanges` subscription -
this time, the comparison occurs with\r\nthe **proper** dashboard input
(i.e. the input from the **destination**\r\ndashboard). Assuming no
previous unsaved changes, this would return\r\n**false** (i.e. the
control group reports to the dashboard that it has\r\n**no** unsaved
changes).\r\n\r\nOn step 3, that is why the destination dashboard
appears as expected -\r\nit has the correct controls, and no unsaved
changes. But then, on step\r\n4, this is what happens:\r\n\r\n1. The
`navigateToDashboard` method is called.\r\n2. We fetch the session
storage so that the \"unsaved changes\" can be\r\napplied to the
dashboard saved object\r\n3. Uh oh! As described in step 6 above, the
session storage for the\r\nsource dashboard includes the control group
input from the\r\n**destination** dashboard!\r\n4. So, when you go back
to the source, the destination dashboard's\r\ncontrols come with you 🔥 🔥
🔥\r\n\r\n\r\n### Description of the fix\r\n\r\n\r\nNow, let's instead
consider what happens when we **don't** `await` the\r\ncontrol group
initialization - if we go back to step 2 of the repro\r\nsteps, then
this is what happens in the code:\r\n\r\n\r\n1. The
`navigateToDashboard` method is called.\r\n2. The control group is told
to update its input and reinitialize via\r\nthe call to
`controlGroup.updateInputAndReinitialize` in
the\r\n`initializeDashboard` method.\r\n3. The dashboard is **not**
waiting for initialization, so it goes ahead\r\nwith navigation
**before** the control group has time to report its\r\nunsaved changes
(the control group's unsaved changes subscription
is\r\ndebounced).\r\n4. Navigation occurs and **nothing** gets backed up
to session storage! \r\n\r\nThat is why, by no longer waiting for the
control group to be\r\ninitialized on navigation, we are no longer
seeing the bug where\r\ncontrols were getting \"replaced\" on
navigation:\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/8698078/0e45a207-ff2a-46a6-9609-11a8dc5bcf67\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":"e5cc4d58fb7e0b12ef99ec69ca35fda97925de5f"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.16.0","labelRegex":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/187509","number":187509,"mergeCommit":{"message":"[Dashboard]
[Controls] Fix controls getting overwritten on navigation
(#187509)\n\n## Summary\r\n\r\n\r\n> [!WARNING]\r\n> Beware - the
longest description ever for a one line change
is\r\nincoming.\r\n>\r\n>\r\n![The-Hangover-Math-GIF-source](https://github.com/elastic/kibana/assets/8698078/3ab94f20-0401-4c42-9bb2-c35b6025301b)\r\n>\r\n>
**TLDR:** We were previously `await`ing the initialization of
the\r\ncontrol group before navigating to the destination dashboard,
which\r\ncaused a race condition where, if the control group had time to
report\r\nunsaved changes before the initialization promise was
resolved, the\r\ncontrol group's input would get backed up under the
wrong ID. If we no\r\nlonger `await` control group initialization, we
remove this race\r\ncondition.\r\n\r\nPreviously, on dashboard
navigation, we were `await`ing the\r\ninitialization of the control
group before navigating to the destination\r\ndashboard - this was
because, before\r\nhttps://github.com//pull/174201, the
control group could\r\nchange its selections and the dashboard needed to
know the most\r\nup-to-date control group output before it could start
loading its\r\npanels. However, once
#175146 was\r\nmerged and the
control group started reporting its own unsaved changes,\r\nthis caused
a race condition on navigation depending on whether or not\r\nthe
dashboard had time to backup its unsaved changes to the
session\r\nstorage before the control group was
initialized.\r\n\r\n\r\n### Description of the race
condition\r\n\r\nConsider the following repro steps:\r\n\r\n1. You start
at your source dashboard (which has no controls), clear\r\nyour cache,
and slow down your network speed.\r\n2. You click on a markdown link to
navigate to your destination\r\ndashboard (which has controls).\r\n3.
You think everything worked as expected - hoorah!\r\n4. You click on a
markdown link to navigate back to your source\r\ndashboard.... but your
source dashboard now has the controls of your\r\ndestination dashboard!
What just happened?\r\n\r\n> [!NOTE]\r\n> If the initialization of the
control group happens **before the\r\ndashboard has a chance to backup
the control group input to session\r\nstorage under the wrong ID**, then
this bug does not happen - that is\r\nwhy it is important to slow down
the network speed when trying to\r\nreproduce this, and it is also why
this bug was more prevalent on Cloud\r\nthan local instances of
Kibana.\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/8698078/91f9b9e1-87f0-44aa-b596-577dd4a541f9\r\n\r\n\r\nOn
step 2 when the markdown link is clicked, this is what happens in
the\r\ncode:\r\n \r\n1. The `navigateToDashboard` method is
called.\r\n2. The control group is told to update its input and
reinitialize via\r\nthe call to
`controlGroup.updateInputAndReinitialize` in
the\r\n`initializeDashboard` method.\r\n3. The dashboard is `await`ing
the initialization of the control group\r\nbefore proceeding with
navigation.\r\n4. The control group is updated, which triggers its
`unsavedChanges`\r\nsubscription - this is comparing its own state to
that of the **source**\r\ndashboard, which is the **wrong** input to be
comparing against.\r\n5. The control group reports to the dashboard that
it **has** unsaved\r\nchanges.\r\n6. The dashboard backs up its unsaved
changes to session storage under\r\nthe wrong ID since navigation hasn't
happened yet - i.e. the\r\n**destination dashboard's** control group
input gets backed up under the\r\n**source dashboard's ID**\r\n7.
Finally, the control group reports that it is initialized and
the\r\ndashboard can proceed with navigation - so, the dashboard ID
changes and\r\nits input gets updated.\r\n8. This triggers the control
group to **once again** trigger the\r\n`unsavedChanges` subscription -
this time, the comparison occurs with\r\nthe **proper** dashboard input
(i.e. the input from the **destination**\r\ndashboard). Assuming no
previous unsaved changes, this would return\r\n**false** (i.e. the
control group reports to the dashboard that it has\r\n**no** unsaved
changes).\r\n\r\nOn step 3, that is why the destination dashboard
appears as expected -\r\nit has the correct controls, and no unsaved
changes. But then, on step\r\n4, this is what happens:\r\n\r\n1. The
`navigateToDashboard` method is called.\r\n2. We fetch the session
storage so that the \"unsaved changes\" can be\r\napplied to the
dashboard saved object\r\n3. Uh oh! As described in step 6 above, the
session storage for the\r\nsource dashboard includes the control group
input from the\r\n**destination** dashboard!\r\n4. So, when you go back
to the source, the destination dashboard's\r\ncontrols come with you 🔥 🔥
🔥\r\n\r\n\r\n### Description of the fix\r\n\r\n\r\nNow, let's instead
consider what happens when we **don't** `await` the\r\ncontrol group
initialization - if we go back to step 2 of the repro\r\nsteps, then
this is what happens in the code:\r\n\r\n\r\n1. The
`navigateToDashboard` method is called.\r\n2. The control group is told
to update its input and reinitialize via\r\nthe call to
`controlGroup.updateInputAndReinitialize` in
the\r\n`initializeDashboard` method.\r\n3. The dashboard is **not**
waiting for initialization, so it goes ahead\r\nwith navigation
**before** the control group has time to report its\r\nunsaved changes
(the control group's unsaved changes subscription
is\r\ndebounced).\r\n4. Navigation occurs and **nothing** gets backed up
to session storage! \r\n\r\nThat is why, by no longer waiting for the
control group to be\r\ninitialized on navigation, we are no longer
seeing the bug where\r\ncontrols were getting \"replaced\" on
navigation:\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/8698078/0e45a207-ff2a-46a6-9609-11a8dc5bcf67\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":"e5cc4d58fb7e0b12ef99ec69ca35fda97925de5f"}}]}]
BACKPORT-->

Co-authored-by: Elastic Machine <[email protected]>

src/plugins/dashboard/public/dashboard_container/embeddable/create/create_dashboard.test.ts

@@ -420,12 +420,11 @@ test('creates new embeddable with specified size if size is provided', async ()
   expect(dashboard!.getState().explicitInput.panels.new_panel.gridData.h).toBe(1);
 });
 
-test('creates a control group from the control group factory and waits for it to be initialized', async () => {
+test('creates a control group from the control group factory', async () => {
   const mockControlGroupContainer = {
     destroy: jest.fn(),
     render: jest.fn(),
     updateInput: jest.fn(),
-    untilInitialized: jest.fn(),
     getInput: jest.fn().mockReturnValue({}),
     getInput$: jest.fn().mockReturnValue(new Observable()),
     getOutput: jest.fn().mockReturnValue({}),
@@ -455,7 +454,6 @@ test('creates a control group from the control group factory and waits for it to
     undefined,
     { lastSavedInput: expect.objectContaining({ controlStyle: 'oneLine' }) }
   );
-  expect(mockControlGroupContainer.untilInitialized).toHaveBeenCalled();
 });
 
 /*

src/plugins/dashboard/public/dashboard_container/embeddable/create/create_dashboard.ts

@@ -427,7 +427,6 @@ export const initializeDashboard = async ({
       dashboardContainer.controlGroup = controlGroup;
       startSyncingDashboardControlGroup.bind(dashboardContainer)();
     });
-    await controlGroup.untilInitialized();
   }
 
   // --------------------------------------------------------------------------------------