Fix Canvas double render by preventing workpad and assets from being …

…reloaded if it's already available (#114712)

* Fix Canvas double render by preventing workpad and assets from being reloaded if it's already available

* Better fix

* Another fix

* updating tests

* updating tests again

* remove unused async

x-pack/plugins/canvas/public/routes/workpad/hooks/use_workpad.test.tsx

@@ -5,7 +5,6 @@
  * 2.0.
  */
 
-import { waitFor } from '@testing-library/react';
 import { renderHook } from '@testing-library/react-hooks';
 import { useWorkpad } from './use_workpad';
 
@@ -61,14 +60,20 @@ describe('useWorkpad', () => {
       workpad: workpadResponse,
     });
 
-    renderHook(() => useWorkpad(workpadId, true, getRedirectPath));
+    const { waitFor, unmount } = renderHook(() => useWorkpad(workpadId, true, getRedirectPath));
 
-    await waitFor(() => expect(mockResolveWorkpad).toHaveBeenCalledWith(workpadId));
+    try {
+      await waitFor(() => expect(mockDispatch).toHaveBeenCalledTimes(3));
 
-    expect(mockResolveWorkpad).toHaveBeenCalledWith(workpadId);
-    expect(mockDispatch).toHaveBeenCalledWith({ type: 'setAssets', payload: assets });
-    expect(mockDispatch).toHaveBeenCalledWith({ type: 'setWorkpad', payload: workpad });
-    expect(mockDispatch).toHaveBeenCalledWith({ type: 'setZoomScale', payload: 1 });
+      expect(mockResolveWorkpad).toHaveBeenCalledWith(workpadId);
+      expect(mockDispatch).toHaveBeenCalledWith({ type: 'setAssets', payload: assets });
+      expect(mockDispatch).toHaveBeenCalledWith({ type: 'setWorkpad', payload: workpad });
+      expect(mockDispatch).toHaveBeenCalledWith({ type: 'setZoomScale', payload: 1 });
+    } catch (e) {
+      throw e;
+    } finally {
+      unmount();
+    }
   });
 
   test('sets alias id of workpad on a conflict', async () => {
@@ -81,17 +86,23 @@ describe('useWorkpad', () => {
       aliasId,
     });
 
-    renderHook(() => useWorkpad(workpadId, true, getRedirectPath));
-
-    await waitFor(() => expect(mockResolveWorkpad).toHaveBeenCalledWith(workpadId));
-
-    expect(mockResolveWorkpad).toHaveBeenCalledWith(workpadId);
-    expect(mockDispatch).toHaveBeenCalledWith({ type: 'setAssets', payload: assets });
-    expect(mockDispatch).toHaveBeenCalledWith({
-      type: 'setWorkpad',
-      payload: { ...workpad, aliasId },
-    });
-    expect(mockDispatch).toHaveBeenCalledWith({ type: 'setZoomScale', payload: 1 });
+    const { waitFor, unmount } = renderHook(() => useWorkpad(workpadId, true, getRedirectPath));
+
+    try {
+      await waitFor(() => expect(mockDispatch).toHaveBeenCalledTimes(3));
+
+      expect(mockResolveWorkpad).toHaveBeenCalledWith(workpadId);
+      expect(mockDispatch).toHaveBeenCalledWith({ type: 'setAssets', payload: assets });
+      expect(mockDispatch).toHaveBeenCalledWith({
+        type: 'setWorkpad',
+        payload: { ...workpad, aliasId },
+      });
+      expect(mockDispatch).toHaveBeenCalledWith({ type: 'setZoomScale', payload: 1 });
+    } catch (e) {
+      throw e;
+    } finally {
+      unmount();
+    }
   });
 
   test('redirects on alias match', async () => {
@@ -104,10 +115,14 @@ describe('useWorkpad', () => {
       aliasId,
     });
 
-    renderHook(() => useWorkpad(workpadId, true, getRedirectPath));
-
-    await waitFor(() => expect(mockResolveWorkpad).toHaveBeenCalledWith(workpadId));
-
-    expect(mockRedirectLegacyUrl).toBeCalledWith(`#${aliasId}`, 'Workpad');
+    const { waitFor, unmount } = renderHook(() => useWorkpad(workpadId, true, getRedirectPath));
+    try {
+      await waitFor(() => expect(mockRedirectLegacyUrl).toHaveBeenCalled());
+      expect(mockRedirectLegacyUrl).toBeCalledWith(`#${aliasId}`, 'Workpad');
+    } catch (e) {
+      throw e;
+    } finally {
+      unmount();
+    }
   });
 });

x-pack/plugins/canvas/public/routes/workpad/hooks/use_workpad.ts

@@ -28,19 +28,26 @@ export const useWorkpad = (
   getRedirectPath: (workpadId: string) => string
 ): [CanvasWorkpad | undefined, string | Error | undefined] => {
   const workpadService = useWorkpadService();
+  const workpadResolve = workpadService.resolve;
   const platformService = usePlatformService();
   const dispatch = useDispatch();
   const storedWorkpad = useSelector(getWorkpad);
   const [error, setError] = useState<string | Error | undefined>(undefined);
 
+  const [resolveInfo, setResolveInfo] = useState<
+    { aliasId: string | undefined; outcome: string } | undefined
+  >(undefined);
+
   useEffect(() => {
     (async () => {
       try {
         const {
           outcome,
           aliasId,
           workpad: { assets, ...workpad },
-        } = await workpadService.resolve(workpadId);
+        } = await workpadResolve(workpadId);
+
+        setResolveInfo({ aliasId, outcome });
 
         if (outcome === 'conflict') {
           workpad.aliasId = aliasId;
@@ -49,15 +56,22 @@ export const useWorkpad = (
         dispatch(setAssets(assets));
         dispatch(setWorkpad(workpad, { loadPages }));
         dispatch(setZoomScale(1));
-
-        if (outcome === 'aliasMatch' && platformService.redirectLegacyUrl && aliasId) {
-          platformService.redirectLegacyUrl(`#${getRedirectPath(aliasId)}`, getWorkpadLabel());
-        }
       } catch (e) {
         setError(e as Error | string);
       }
     })();
-  }, [workpadId, dispatch, setError, loadPages, workpadService, getRedirectPath, platformService]);
+  }, [workpadId, dispatch, setError, loadPages, workpadResolve]);
+
+  useEffect(() => {
+    (() => {
+      if (!resolveInfo) return;
+
+      const { aliasId, outcome } = resolveInfo;
+      if (outcome === 'aliasMatch' && platformService.redirectLegacyUrl && aliasId) {
+        platformService.redirectLegacyUrl(`#${getRedirectPath(aliasId)}`, getWorkpadLabel());
+      }
+    })();
+  }, [resolveInfo, getRedirectPath, platformService]);
 
   return [storedWorkpad.id === workpadId ? storedWorkpad : undefined, error];
 };