[Time to Visualize] Clear Unsaved Changes When Dashboard Fails to Load (#90527)

* added error handling to dashboard_unsaved_listing. It should now remove all unsaved changes from dashboards which error on load

src/plugins/dashboard/public/application/listing/dashboard_listing.test.tsx

@@ -49,11 +49,16 @@ function makeDefaultServices(): DashboardAppServices {
       hits,
     });
   };
+  const dashboardPanelStorage = ({
+    getDashboardIdsWithUnsavedChanges: jest
+      .fn()
+      .mockResolvedValue(['dashboardUnsavedOne', 'dashboardUnsavedTwo']),
+  } as unknown) as DashboardPanelStorage;
+
   return {
     savedObjects: savedObjectsPluginMock.createStartContract(),
     embeddable: embeddablePluginMock.createInstance().doStart(),
     dashboardCapabilities: {} as DashboardCapabilities,
-    dashboardPanelStorage: {} as DashboardPanelStorage,
     initializerContext: {} as PluginInitializerContext,
     chrome: chromeServiceMock.createStartContract(),
     navigation: {} as NavigationPublicPluginStart,
@@ -68,6 +73,7 @@ function makeDefaultServices(): DashboardAppServices {
     restorePreviousUrl: () => {},
     onAppLeave: (handler) => {},
     allowByValueEmbeddables: true,
+    dashboardPanelStorage,
     savedDashboards,
     core,
   };

src/plugins/dashboard/public/application/listing/dashboard_listing.tsx

@@ -8,7 +8,7 @@
 
 import { FormattedMessage } from '@kbn/i18n/react';
 import { EuiLink, EuiButton, EuiEmptyPrompt } from '@elastic/eui';
-import React, { Fragment, useCallback, useEffect, useMemo } from 'react';
+import React, { Fragment, useCallback, useEffect, useMemo, useState } from 'react';
 import { attemptLoadDashboardByTitle } from '../lib';
 import { DashboardAppServices, DashboardRedirect } from '../types';
 import { getDashboardBreadcrumb, dashboardListingTable } from '../../dashboard_strings';
@@ -48,6 +48,10 @@ export const DashboardListing = ({
     },
   } = useKibana<DashboardAppServices>();
 
+  const [unsavedDashboardIds, setUnsavedDashboardIds] = useState<string[]>(
+    dashboardPanelStorage.getDashboardIdsWithUnsavedChanges()
+  );
+
   // Set breadcrumbs useEffect
   useEffect(() => {
     setBreadcrumbs([
@@ -135,8 +139,12 @@ export const DashboardListing = ({
   );
 
   const deleteItems = useCallback(
-    (dashboards: Array<{ id: string }>) => savedDashboards.delete(dashboards.map((d) => d.id)),
-    [savedDashboards]
+    (dashboards: Array<{ id: string }>) => {
+      dashboards.map((d) => dashboardPanelStorage.clearPanels(d.id));
+      setUnsavedDashboardIds(dashboardPanelStorage.getDashboardIdsWithUnsavedChanges());
+      return savedDashboards.delete(dashboards.map((d) => d.id));
+    },
+    [savedDashboards, dashboardPanelStorage]
   );
 
   const editItem = useCallback(
@@ -179,7 +187,13 @@ export const DashboardListing = ({
         tableColumns,
       }}
     >
-      <DashboardUnsavedListing redirectTo={redirectTo} />
+      <DashboardUnsavedListing
+        redirectTo={redirectTo}
+        unsavedDashboardIds={unsavedDashboardIds}
+        refreshUnsavedDashboards={() =>
+          setUnsavedDashboardIds(dashboardPanelStorage.getDashboardIdsWithUnsavedChanges())
+        }
+      />
     </TableListView>
   );
 };

src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.test.tsx

@@ -17,8 +17,8 @@ import { KibanaContextProvider } from '../../services/kibana_react';
 import { SavedObjectLoader } from '../../services/saved_objects';
 import { DashboardPanelStorage } from '../lib';
 import { DASHBOARD_PANELS_UNSAVED_ID } from '../lib/dashboard_panel_storage';
-import { DashboardAppServices, DashboardRedirect } from '../types';
-import { DashboardUnsavedListing } from './dashboard_unsaved_listing';
+import { DashboardAppServices } from '../types';
+import { DashboardUnsavedListing, DashboardUnsavedListingProps } from './dashboard_unsaved_listing';
 
 const mockedDashboards: { [key: string]: DashboardSavedObject } = {
   dashboardUnsavedOne: {
@@ -39,16 +39,11 @@ function makeDefaultServices(): DashboardAppServices {
   const core = coreMock.createStart();
   core.overlays.openConfirm = jest.fn().mockResolvedValue(true);
   const savedDashboards = {} as SavedObjectLoader;
-  savedDashboards.get = jest.fn().mockImplementation((id: string) => mockedDashboards[id]);
+  savedDashboards.get = jest
+    .fn()
+    .mockImplementation((id: string) => Promise.resolve(mockedDashboards[id]));
   const dashboardPanelStorage = {} as DashboardPanelStorage;
   dashboardPanelStorage.clearPanels = jest.fn();
-  dashboardPanelStorage.getDashboardIdsWithUnsavedChanges = jest
-    .fn()
-    .mockImplementation(() => [
-      'dashboardUnsavedOne',
-      'dashboardUnsavedTwo',
-      'dashboardUnsavedThree',
-    ]);
   return ({
     dashboardPanelStorage,
     savedDashboards,
@@ -56,14 +51,18 @@ function makeDefaultServices(): DashboardAppServices {
   } as unknown) as DashboardAppServices;
 }
 
-const makeDefaultProps = () => ({ redirectTo: jest.fn() });
+const makeDefaultProps = (): DashboardUnsavedListingProps => ({
+  redirectTo: jest.fn(),
+  unsavedDashboardIds: ['dashboardUnsavedOne', 'dashboardUnsavedTwo', 'dashboardUnsavedThree'],
+  refreshUnsavedDashboards: jest.fn(),
+});
 
 function mountWith({
   services: incomingServices,
   props: incomingProps,
 }: {
   services?: DashboardAppServices;
-  props?: { redirectTo: DashboardRedirect };
+  props?: DashboardUnsavedListingProps;
 }) {
   const services = incomingServices ?? makeDefaultServices();
   const props = incomingProps ?? makeDefaultProps();
@@ -89,11 +88,9 @@ describe('Unsaved listing', () => {
   });
 
   it('Does not attempt to get unsaved dashboard id', async () => {
-    const services = makeDefaultServices();
-    services.dashboardPanelStorage.getDashboardIdsWithUnsavedChanges = jest
-      .fn()
-      .mockImplementation(() => ['dashboardUnsavedOne', DASHBOARD_PANELS_UNSAVED_ID]);
-    mountWith({ services });
+    const props = makeDefaultProps();
+    props.unsavedDashboardIds = ['dashboardUnsavedOne', DASHBOARD_PANELS_UNSAVED_ID];
+    const { services } = mountWith({ props });
     await waitFor(() => {
       expect(services.savedDashboards.get).toHaveBeenCalledTimes(1);
     });
@@ -115,11 +112,9 @@ describe('Unsaved listing', () => {
   });
 
   it('Redirects to new dashboard when continue editing clicked', async () => {
-    const services = makeDefaultServices();
-    services.dashboardPanelStorage.getDashboardIdsWithUnsavedChanges = jest
-      .fn()
-      .mockImplementation(() => [DASHBOARD_PANELS_UNSAVED_ID]);
-    const { props, component } = mountWith({ services });
+    const props = makeDefaultProps();
+    props.unsavedDashboardIds = [DASHBOARD_PANELS_UNSAVED_ID];
+    const { component } = mountWith({ props });
     const getEditButton = () => findTestSubject(component, `edit-unsaved-New-Dashboard`);
     await waitFor(() => {
       component.update();
@@ -150,4 +145,34 @@ describe('Unsaved listing', () => {
       );
     });
   });
+
+  it('removes unsaved changes from any dashboard which errors on fetch', async () => {
+    const services = makeDefaultServices();
+    const props = makeDefaultProps();
+    services.savedDashboards.get = jest.fn().mockImplementation((id: string) => {
+      if (id === 'failCase1' || id === 'failCase2') {
+        return Promise.reject(new Error());
+      }
+      return Promise.resolve(mockedDashboards[id]);
+    });
+
+    props.unsavedDashboardIds = [
+      'dashboardUnsavedOne',
+      'dashboardUnsavedTwo',
+      'dashboardUnsavedThree',
+      'failCase1',
+      'failCase2',
+    ];
+    const { component } = mountWith({ services, props });
+    waitFor(() => {
+      component.update();
+      expect(services.dashboardPanelStorage.clearPanels).toHaveBeenCalledWith('failCase1');
+      expect(services.dashboardPanelStorage.clearPanels).toHaveBeenCalledWith('failCase2');
+
+      // clearing panels from dashboard with errors should cause getDashboardIdsWithUnsavedChanges to be called again.
+      expect(
+        services.dashboardPanelStorage.getDashboardIdsWithUnsavedChanges
+      ).toHaveBeenCalledTimes(2);
+    });
+  });
 });

src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx

@@ -106,7 +106,17 @@ interface UnsavedItemMap {
   [key: string]: DashboardSavedObject;
 }
 
-export const DashboardUnsavedListing = ({ redirectTo }: { redirectTo: DashboardRedirect }) => {
+export interface DashboardUnsavedListingProps {
+  refreshUnsavedDashboards: () => void;
+  redirectTo: DashboardRedirect;
+  unsavedDashboardIds: string[];
+}
+
+export const DashboardUnsavedListing = ({
+  redirectTo,
+  unsavedDashboardIds,
+  refreshUnsavedDashboards,
+}: DashboardUnsavedListingProps) => {
   const {
     services: {
       dashboardPanelStorage,
@@ -116,9 +126,6 @@ export const DashboardUnsavedListing = ({ redirectTo }: { redirectTo: DashboardR
   } = useKibana<DashboardAppServices>();
 
   const [items, setItems] = useState<UnsavedItemMap>({});
-  const [dashboardIds, setDashboardIds] = useState<string[]>(
-    dashboardPanelStorage.getDashboardIdsWithUnsavedChanges()
-  );
 
   const onOpen = useCallback(
     (id?: string) => {
@@ -133,48 +140,63 @@ export const DashboardUnsavedListing = ({ redirectTo }: { redirectTo: DashboardR
         overlays,
         () => {
           dashboardPanelStorage.clearPanels(id);
-          setDashboardIds(dashboardPanelStorage.getDashboardIdsWithUnsavedChanges());
+          refreshUnsavedDashboards();
         },
         createConfirmStrings.getCancelButtonText()
       );
     },
-    [overlays, dashboardPanelStorage]
+    [overlays, refreshUnsavedDashboards, dashboardPanelStorage]
   );
 
   useEffect(() => {
-    if (dashboardIds?.length === 0) {
+    if (unsavedDashboardIds?.length === 0) {
       return;
     }
     let canceled = false;
-    const dashPromises = dashboardIds
+    const dashPromises = unsavedDashboardIds
       .filter((id) => id !== DASHBOARD_PANELS_UNSAVED_ID)
-      .map((dashboardId) => savedDashboards.get(dashboardId));
-    Promise.all(dashPromises).then((dashboards: DashboardSavedObject[]) => {
+      .map((dashboardId) => {
+        return (savedDashboards.get(dashboardId) as Promise<DashboardSavedObject>).catch(
+          () => dashboardId
+        );
+      });
+    Promise.all(dashPromises).then((dashboards: Array<string | DashboardSavedObject>) => {
       const dashboardMap = {};
       if (canceled) {
         return;
       }
-      setItems(
-        dashboards.reduce((map, dashboard) => {
-          return {
-            ...map,
-            [dashboard.id || DASHBOARD_PANELS_UNSAVED_ID]: dashboard,
-          };
-        }, dashboardMap)
-      );
+      let hasError = false;
+      const newItems = dashboards.reduce((map, dashboard) => {
+        if (typeof dashboard === 'string') {
+          hasError = true;
+          dashboardPanelStorage.clearPanels(dashboard);
+          return map;
+        }
+        return {
+          ...map,
+          [dashboard.id || DASHBOARD_PANELS_UNSAVED_ID]: dashboard,
+        };
+      }, dashboardMap);
+      if (hasError) {
+        refreshUnsavedDashboards();
+        return;
+      }
+      setItems(newItems);
     });
     return () => {
       canceled = true;
     };
-  }, [dashboardIds, savedDashboards]);
+  }, [savedDashboards, dashboardPanelStorage, refreshUnsavedDashboards, unsavedDashboardIds]);
 
-  return dashboardIds.length === 0 ? null : (
+  return unsavedDashboardIds.length === 0 ? null : (
     <>
       <EuiCallOut
         heading="h3"
-        title={dashboardUnsavedListingStrings.getUnsavedChangesTitle(dashboardIds.length > 1)}
+        title={dashboardUnsavedListingStrings.getUnsavedChangesTitle(
+          unsavedDashboardIds.length > 1
+        )}
       >
-        {dashboardIds.map((dashboardId: string) => {
+        {unsavedDashboardIds.map((dashboardId: string) => {
           const title: string | undefined =
             dashboardId === DASHBOARD_PANELS_UNSAVED_ID
               ? getNewDashboardTitle()