Catalog: Provide default tab fallback in Plugin Details page (#41979) (#42139)

* feat(catalog): introduce defaultTab to usePluginDetailsTabs hook

* feat(catalog): use defaultTab as fallback tab for PluginDetails

* chore(catalog): remove hardcoded page query param in list items

* refactor(catalog): prefer let over react ref when setting default tab in PluginDetails

* refactor(catalog): pass pageId to plugin details body rather than duplicate logic

* test(catalog): remove query param from List item test hrefs

* test(catalog): introduce a test for default app config page for installed app plugins

(cherry picked from commit 694600ed04)

Co-authored-by: Jack Westbrook <jack.westbrook@gmail.com>

public/app/features/plugins/admin/components/PluginDetailsBody.tsx

@@ -13,12 +13,12 @@ import { PluginDashboards } from '../../PluginDashboards';
 type Props = {
   plugin: CatalogPlugin;
   queryParams: UrlQueryMap;
+  pageId: string;
 };
 
-export function PluginDetailsBody({ plugin, queryParams }: Props): JSX.Element {
+export function PluginDetailsBody({ plugin, queryParams, pageId }: Props): JSX.Element {
   const styles = useStyles2(getStyles);
   const { value: pluginConfig } = usePluginConfig(plugin);
-  const pageId = queryParams.page;
 
   if (pageId === PluginTabIds.OVERVIEW) {
     return (

public/app/features/plugins/admin/components/PluginListItem.test.tsx

@@ -59,7 +59,7 @@ describe('PluginListItem', () => {
   it('renders a card with link, image, name, orgName and badges', () => {
     render(<PluginListItem plugin={plugin} pathName="/plugins" />);
 
-    expect(screen.getByRole('link')).toHaveAttribute('href', '/plugins/test-plugin?page=overview');
+    expect(screen.getByRole('link')).toHaveAttribute('href', '/plugins/test-plugin');
 
     const logo = screen.getByRole('img');
     expect(logo).toHaveAttribute('src', plugin.info.logos.small);
@@ -102,7 +102,7 @@ describe('PluginListItem', () => {
   it('renders a row with link, image, name, orgName and badges', () => {
     render(<PluginListItem plugin={plugin} pathName="/plugins" displayMode={PluginListDisplayMode.List} />);
 
-    expect(screen.getByRole('link')).toHaveAttribute('href', '/plugins/test-plugin?page=overview');
+    expect(screen.getByRole('link')).toHaveAttribute('href', '/plugins/test-plugin');
 
     const logo = screen.getByRole('img');
     expect(logo).toHaveAttribute('src', plugin.info.logos.small);

public/app/features/plugins/admin/components/PluginListItem.tsx

@@ -1,7 +1,7 @@
 import React from 'react';
 import { css, cx } from '@emotion/css';
 import { GrafanaTheme2 } from '@grafana/data';
-import { CatalogPlugin, PluginIconName, PluginListDisplayMode, PluginTabIds } from '../types';
+import { CatalogPlugin, PluginIconName, PluginListDisplayMode } from '../types';
 import { PluginListItemBadges } from './PluginListItemBadges';
 import { PluginLogo } from './PluginLogo';
 import { Icon, useStyles2 } from '@grafana/ui';
@@ -19,10 +19,7 @@ export function PluginListItem({ plugin, pathName, displayMode = PluginListDispl
   const isList = displayMode === PluginListDisplayMode.List;
 
   return (
-    <a
+    <a href={`${pathName}/${plugin.id}`} className={cx(styles.container, { [styles.list]: isList })}>
-      href={`${pathName}/${plugin.id}?page=${PluginTabIds.OVERVIEW}`}
-      className={cx(styles.container, { [styles.list]: isList })}
-    >
       <PluginLogo src={plugin.info.logos.small} className={styles.pluginLogo} height={LOGO_SIZE} alt="" />
       <h2 className={cx(styles.name, 'plugin-name')}>{plugin.name}</h2>
       <div className={cx(styles.content, 'plugin-content')}>

public/app/features/plugins/admin/hooks/usePluginDetailsTabs.tsx

@@ -9,15 +9,18 @@ type ReturnType = {
   error: Error | undefined;
   loading: boolean;
   tabs: PluginDetailsTab[];
+  defaultTab: string;
 };
 
 export const usePluginDetailsTabs = (plugin?: CatalogPlugin, defaultTabs: PluginDetailsTab[] = []): ReturnType => {
   const { loading, error, value: pluginConfig } = usePluginConfig(plugin);
   const isPublished = Boolean(plugin?.isPublished);
   const { pathname } = useLocation();
-  const tabs = useMemo(() => {
+
+  const [tabs, defaultTab] = useMemo(() => {
     const canConfigurePlugins = isOrgAdmin();
     const tabs: PluginDetailsTab[] = [...defaultTabs];
+    let defaultTab;
 
     if (isPublished) {
       tabs.push({
@@ -30,7 +33,8 @@ export const usePluginDetailsTabs = (plugin?: CatalogPlugin, defaultTabs: Plugin
 
     // Not extending the tabs with the config pages if the plugin is not installed
     if (!pluginConfig) {
-      return tabs;
+      defaultTab = PluginTabIds.OVERVIEW;
+      return [tabs, defaultTab];
     }
 
     if (canConfigurePlugins) {
@@ -42,6 +46,7 @@ export const usePluginDetailsTabs = (plugin?: CatalogPlugin, defaultTabs: Plugin
             id: PluginTabIds.CONFIG,
             href: `${pathname}?page=${PluginTabIds.CONFIG}`,
           });
+          defaultTab = PluginTabIds.CONFIG;
         }
 
         if (pluginConfig.configPages) {
@@ -52,6 +57,9 @@ export const usePluginDetailsTabs = (plugin?: CatalogPlugin, defaultTabs: Plugin
               id: page.id,
               href: `${pathname}?page=${page.id}`,
             });
+            if (!defaultTab) {
+              defaultTab = page.id;
+            }
           }
         }
 
@@ -66,12 +74,17 @@ export const usePluginDetailsTabs = (plugin?: CatalogPlugin, defaultTabs: Plugin
       }
     }
 
-    return tabs;
+    if (!defaultTab) {
+      defaultTab = PluginTabIds.OVERVIEW;
+    }
+
+    return [tabs, defaultTab];
   }, [pluginConfig, defaultTabs, pathname, isPublished]);
 
   return {
     error,
     loading,
     tabs,
+    defaultTab,
   };
 };

public/app/features/plugins/admin/pages/PluginDetails.test.tsx

@@ -40,7 +40,7 @@ jest.mock('../hooks/usePluginConfig.tsx', () => ({
 const renderPluginDetails = (
   pluginOverride: Partial<CatalogPlugin>,
   {
-    pageId = PluginTabIds.OVERVIEW,
+    pageId,
     pluginsStateOverride,
   }: {
     pageId?: PluginTabIds;
@@ -55,7 +55,7 @@ const renderPluginDetails = (
     location: {
       hash: '',
       pathname: `/plugins/${id}`,
-      search: `?page=${pageId}`,
+      search: pageId ? `?page=${pageId}` : '',
       state: undefined,
     },
   });
@@ -118,11 +118,11 @@ describe('Plugin details page', () => {
 
       const props = getRouteComponentProps({
         match: { params: { pluginId: id }, isExact: true, url: '', path: '' },
-        queryParams: { page: PluginTabIds.OVERVIEW },
+        queryParams: {},
         location: {
           hash: '',
           pathname: `/plugins/${id}`,
-          search: `?page=${PluginTabIds.OVERVIEW}`,
+          search: '',
           state: undefined,
         },
       });
@@ -145,6 +145,40 @@ describe('Plugin details page', () => {
       await waitFor(() => expect(queryByText(/licensed under the apache 2.0 license/i)).toBeInTheDocument());
     });
 
+    it('should display an app config page by default for installed app plugins', async () => {
+      const name = 'Akumuli';
+
+      // @ts-ignore
+      usePluginConfig.mockReturnValue({
+        value: {
+          meta: {
+            type: PluginType.app,
+            enabled: false,
+            pinned: false,
+            jsonData: {},
+          },
+          configPages: [
+            {
+              title: 'Config',
+              icon: 'cog',
+              id: 'configPage',
+              body: function ConfigPage() {
+                return <div>Custom Config Page!</div>;
+              },
+            },
+          ],
+        },
+      });
+
+      const { queryByText } = renderPluginDetails({
+        name,
+        isInstalled: true,
+        type: PluginType.app,
+      });
+
+      await waitFor(() => expect(queryByText(/custom config page/i)).toBeInTheDocument());
+    });
+
     it('should display the number of downloads in the header', async () => {
       // depending on what locale you have the Intl.NumberFormat will return a format that contains
       // whitespaces. In that case we don't want testing library to remove whitespaces.

public/app/features/plugins/admin/pages/PluginDetails.tsx

@@ -25,7 +25,6 @@ export default function PluginDetails({ match, queryParams }: Props): JSX.Elemen
     params: { pluginId = '' },
     url,
   } = match;
-  const pageId = (queryParams.page as PluginTabIds) || PluginTabIds.OVERVIEW;
   const parentUrl = url.substring(0, url.lastIndexOf('/'));
   const defaultTabs: PluginDetailsTab[] = [
     {
@@ -36,11 +35,12 @@ export default function PluginDetails({ match, queryParams }: Props): JSX.Elemen
     },
   ];
   const plugin = useGetSingle(pluginId); // fetches the localplugin settings
-  const { tabs } = usePluginDetailsTabs(plugin, defaultTabs);
+  const { tabs, defaultTab } = usePluginDetailsTabs(plugin, defaultTabs);
   const { isLoading: isFetchLoading } = useFetchStatus();
   const { isLoading: isFetchDetailsLoading } = useFetchDetailsStatus();
   const styles = useStyles2(getStyles);
   const prevTabs = usePrevious(tabs);
+  const pageId = (queryParams.page as PluginTabIds) || defaultTab;
 
   // If an app plugin is uninstalled we need to reset the active tab when the config / dashboards tabs are removed.
   useEffect(() => {
@@ -95,7 +95,7 @@ export default function PluginDetails({ match, queryParams }: Props): JSX.Elemen
         <TabContent className={styles.tabContent}>
           <PluginDetailsSignature plugin={plugin} className={styles.alert} />
           <PluginDetailsDisabledError plugin={plugin} className={styles.alert} />
-          <PluginDetailsBody queryParams={queryParams} plugin={plugin} />
+          <PluginDetailsBody queryParams={queryParams} plugin={plugin} pageId={pageId} />
         </TabContent>
       </PluginPage>
     </Page>