From e5fba788d6ac3de087f030a43a2bee39366981c6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Torkel=20=C3=96degaard?= <torkel@grafana.com>
Date: Wed, 31 Aug 2022 15:36:08 +0200
Subject: [PATCH] AppRootPage: Fixes issue navigating between two app plugin
 pages (#54519)

* AppRootPage: Fixes issue where it was not possible to navigate to another plugin

* Externalize react-router

* fixing test
---
 .../src/config/webpack.plugin.config.ts       |  1 +
 .../plugins/components/AppRootPage.test.tsx   | 29 +++++++++----------
 .../plugins/components/AppRootPage.tsx        | 26 ++++++++++-------
 3 files changed, 30 insertions(+), 26 deletions(-)

diff --git a/packages/grafana-toolkit/src/config/webpack.plugin.config.ts b/packages/grafana-toolkit/src/config/webpack.plugin.config.ts
index 5a67061b598..7bc3897c6a2 100644
--- a/packages/grafana-toolkit/src/config/webpack.plugin.config.ts
+++ b/packages/grafana-toolkit/src/config/webpack.plugin.config.ts
@@ -180,6 +180,7 @@ const getBaseWebpackConfig: WebpackConfigurationGetter = async (options) => {
       'react-redux',
       'redux',
       'rxjs',
+      'react-router',
       'react-router-dom',
       'd3',
       'angular',
diff --git a/public/app/features/plugins/components/AppRootPage.test.tsx b/public/app/features/plugins/components/AppRootPage.test.tsx
index b0b38ff30a4..b8526de96c0 100644
--- a/public/app/features/plugins/components/AppRootPage.test.tsx
+++ b/public/app/features/plugins/components/AppRootPage.test.tsx
@@ -81,17 +81,18 @@ describe('AppRootPage', () => {
     setEchoSrv(new Echo());
   });
 
+  const pluginMeta = getMockPlugin({
+    id: 'my-awesome-plugin',
+    type: PluginType.app,
+    enabled: true,
+  });
+
   it('should not mount plugin twice if nav is changed', async () => {
     // reproduces https://github.com/grafana/grafana/pull/28105
-
-    getPluginSettingsMock.mockResolvedValue(
-      getMockPlugin({
-        type: PluginType.app,
-        enabled: true,
-      })
-    );
+    getPluginSettingsMock.mockResolvedValue(pluginMeta);
 
     const plugin = new AppPlugin();
+    plugin.meta = pluginMeta;
     plugin.root = RootComponent;
 
     importAppPluginMock.mockResolvedValue(plugin);
@@ -106,12 +107,7 @@ describe('AppRootPage', () => {
   });
 
   it('should not render component if not at plugin path', async () => {
-    getPluginSettingsMock.mockResolvedValue(
-      getMockPlugin({
-        type: PluginType.app,
-        enabled: true,
-      })
-    );
+    getPluginSettingsMock.mockResolvedValue(pluginMeta);
 
     class RootComponent extends Component<AppRootProps> {
       static timesRendered = 0;
@@ -122,6 +118,7 @@ describe('AppRootPage', () => {
     }
 
     const plugin = new AppPlugin();
+    plugin.meta = pluginMeta;
     plugin.root = RootComponent;
 
     importAppPluginMock.mockResolvedValue(plugin);
@@ -131,18 +128,18 @@ describe('AppRootPage', () => {
     expect(await screen.findByText('my great component')).toBeVisible();
 
     // renders the first time
-    expect(RootComponent.timesRendered).toEqual(1);
+    expect(RootComponent.timesRendered).toEqual(2);
 
     await act(async () => {
       locationService.push('/foo');
     });
 
-    expect(RootComponent.timesRendered).toEqual(1);
+    expect(RootComponent.timesRendered).toEqual(2);
 
     await act(async () => {
       locationService.push('/a/my-awesome-plugin');
     });
 
-    expect(RootComponent.timesRendered).toEqual(2);
+    expect(RootComponent.timesRendered).toEqual(4);
   });
 });
diff --git a/public/app/features/plugins/components/AppRootPage.tsx b/public/app/features/plugins/components/AppRootPage.tsx
index d28f89ba652..8e98fdb59c8 100644
--- a/public/app/features/plugins/components/AppRootPage.tsx
+++ b/public/app/features/plugins/components/AppRootPage.tsx
@@ -92,7 +92,15 @@ class AppRootPage extends Component<Props, State> {
   render() {
     const { loading, plugin, nav, portalNode } = this.state;
 
-    if (plugin && !plugin.root) {
+    if (!plugin || this.props.match.params.pluginId !== plugin.meta.id) {
+      return (
+        <Page>
+          <PageLoader />
+        </Page>
+      );
+    }
+
+    if (!plugin.root) {
       // TODO? redirect to plugin page?
       return <div>No Root App</div>;
     }
@@ -100,15 +108,13 @@ class AppRootPage extends Component<Props, State> {
     return (
       <>
         <InPortal node={portalNode}>
-          {plugin && plugin.root && (
-            <plugin.root
-              meta={plugin.meta}
-              basename={this.props.match.url}
-              onNavChanged={this.onNavChanged}
-              query={this.props.queryParams as KeyValue}
-              path={this.props.location.pathname}
-            />
-          )}
+          <plugin.root
+            meta={plugin.meta}
+            basename={this.props.match.url}
+            onNavChanged={this.onNavChanged}
+            query={this.props.queryParams as KeyValue}
+            path={this.props.location.pathname}
+          />
         </InPortal>
         {nav ? (
           <Page navModel={nav}>