From 22ab22d4703cd454b50c75c71a81184d3f580980 Mon Sep 17 00:00:00 2001 From: Levente Balogh Date: Thu, 20 Nov 2025 11:27:58 +0100 Subject: [PATCH] test: add test for the `configure()` function updates --- packages/grafana-data/src/index.ts | 1 + .../src/types/pluginExtensions.ts | 24 +- .../ExtensionSidebarProvider.tsx | 13 +- .../scene/PanelMenuBehavior.test.tsx | 48 +- .../scene/PanelMenuBehavior.tsx | 92 ++-- .../extensions/getPluginExtensions.test.tsx | 412 ++++++++++++++---- .../plugins/extensions/getPluginExtensions.ts | 357 +++++++++------ .../app/features/plugins/extensions/types.ts | 6 +- .../extensions/usePluginComponents.tsx | 12 +- .../plugins/extensions/usePluginFunctions.tsx | 12 +- .../extensions/usePluginLinks.test.tsx | 211 ++++++++- .../plugins/extensions/usePluginLinks.tsx | 141 +++--- .../app/features/plugins/extensions/utils.tsx | 43 +- .../extensions/validateExtensionPoint.test.ts | 19 +- .../extensions/validateExtensionPoint.ts | 24 +- 15 files changed, 1023 insertions(+), 392 deletions(-) diff --git a/packages/grafana-data/src/index.ts b/packages/grafana-data/src/index.ts index 6027b566764..c07b0b55f66 100644 --- a/packages/grafana-data/src/index.ts +++ b/packages/grafana-data/src/index.ts @@ -583,6 +583,7 @@ export { type PluginExtensionComponentMeta, type ComponentTypeWithExtensionMeta, type PluginExtensionFunction, + type PluginExtensionLinkUpdate, type PluginExtensionEventHelpers, type DataSourceConfigErrorStatusContext, type PluginExtensionPanelContext, diff --git a/packages/grafana-data/src/types/pluginExtensions.ts b/packages/grafana-data/src/types/pluginExtensions.ts index cc2a97b4a50..a0a0fbf2c05 100644 --- a/packages/grafana-data/src/types/pluginExtensions.ts +++ b/packages/grafana-data/src/types/pluginExtensions.ts @@ -99,17 +99,21 @@ export type PluginExtensionAddedFunctionConfig = PluginExte */ fn: Signature; }; +export interface PluginExtensionLinkUpdate { + title: string; + description: string; + path: string; + onClick: (event: React.MouseEvent | undefined, helpers: PluginExtensionEventHelpers) => void; + icon: IconName; + category: string; + openInNewTab: boolean; +} -export type PluginAddedLinksConfigureFunc = (context: Readonly | undefined) => - | Partial<{ - title: string; - description: string; - path: string; - onClick: (event: React.MouseEvent | undefined, helpers: PluginExtensionEventHelpers) => void; - icon: IconName; - category: string; - openInNewTab: boolean; - }> +export type PluginAddedLinksConfigureFunc = ( + context: Readonly | undefined +) => + | Partial> + | Promise> | undefined> | undefined; export type PluginExtensionAddedLinkConfig = PluginExtensionConfigBase & { diff --git a/public/app/core/components/AppChrome/ExtensionSidebar/ExtensionSidebarProvider.tsx b/public/app/core/components/AppChrome/ExtensionSidebar/ExtensionSidebarProvider.tsx index 51d73d2e8e5..b75ab0683ca 100644 --- a/public/app/core/components/AppChrome/ExtensionSidebar/ExtensionSidebarProvider.tsx +++ b/public/app/core/components/AppChrome/ExtensionSidebar/ExtensionSidebarProvider.tsx @@ -75,6 +75,12 @@ export const ExtensionSidebarContextProvider = ({ children }: ExtensionSidebarCo ); const [currentPath, setCurrentPath] = useState(locationService.getLocation().pathname); + const context = useMemo( + () => ({ + path: currentPath, + }), + [currentPath] + ); useEffect(() => { const subscription = locationService.getLocationObservable().subscribe((location) => { @@ -92,9 +98,10 @@ export const ExtensionSidebarContextProvider = ({ children }: ExtensionSidebarCo // whether the component is rendered or not const { links, isLoading } = usePluginLinks({ extensionPointId: PluginExtensionPoints.ExtensionSidebar, - context: { - path: currentPath, - }, + context, + // context: { + // path: currentPath, + // }, }); // get all components for this extension point, but only for the permitted plugins diff --git a/public/app/features/dashboard-scene/scene/PanelMenuBehavior.test.tsx b/public/app/features/dashboard-scene/scene/PanelMenuBehavior.test.tsx index 029cc0c7ccd..ac611c24c31 100644 --- a/public/app/features/dashboard-scene/scene/PanelMenuBehavior.test.tsx +++ b/public/app/features/dashboard-scene/scene/PanelMenuBehavior.test.tsx @@ -1,3 +1,4 @@ +import { of } from 'rxjs'; import { FieldType, LoadingState, @@ -22,6 +23,7 @@ import { contextSrv } from 'app/core/services/context_srv'; import { GetExploreUrlArguments } from 'app/core/utils/explore'; import { grantUserPermissions } from 'app/features/alerting/unified/mocks'; import { scenesPanelToRuleFormValues } from 'app/features/alerting/unified/utils/rule-form'; +import * as getPluginExtensions from 'app/features/plugins/extensions/getPluginExtensions'; import * as storeModule from 'app/store/store'; import { AccessControlAction } from 'app/types/accessControl'; @@ -36,6 +38,8 @@ const mocks = { contextSrv: jest.mocked(contextSrv), getExploreUrl: jest.fn(), notifyApp: jest.fn(), + getPluginExtensions: jest.fn().mockReturnValue({ extensions: [] }), + getObservablePluginLinks: jest.fn().mockReturnValue(of([])), }; jest.mock('app/core/utils/explore', () => ({ @@ -51,10 +55,10 @@ jest.mock('app/store/store', () => ({ dispatch: jest.fn(), })); -const getPluginExtensionsMock = jest.fn().mockReturnValue({ extensions: [] }); jest.mock('app/features/plugins/extensions/getPluginExtensions', () => ({ ...jest.requireActual('app/features/plugins/extensions/getPluginExtensions'), - createPluginExtensionsGetter: () => getPluginExtensionsMock, + createPluginExtensionsGetter: () => mocks.getPluginExtensions, + getObservablePluginLinks: () => mocks.getObservablePluginLinks(), })); describe('panelMenuBehavior', () => { @@ -125,8 +129,8 @@ describe('panelMenuBehavior', () => { describe('when extending panel menu from plugins', () => { it('should contain menu item from link extension', async () => { - getPluginExtensionsMock.mockReturnValue({ - extensions: [ + mocks.getObservablePluginLinks.mockReturnValue( + of([ { id: '1', pluginId: '...', @@ -135,8 +139,8 @@ describe('panelMenuBehavior', () => { description: 'Declaring an incident in the app', path: '/a/grafana-basic-app/declare-incident', }, - ], - }); + ]) + ); const { menu, panel } = await buildTestScene({}); @@ -164,7 +168,7 @@ describe('panelMenuBehavior', () => { }); it('should truncate menu item title to 25 chars', async () => { - getPluginExtensionsMock.mockReturnValue({ + mocks.getPluginExtensions.mockReturnValue({ extensions: [ { id: '1', @@ -203,7 +207,7 @@ describe('panelMenuBehavior', () => { }); it('should show icons for link extensions (if they provide it)', async () => { - getPluginExtensionsMock.mockReturnValue({ + mocks.getPluginExtensions.mockReturnValue({ extensions: [ { id: '1', @@ -246,7 +250,7 @@ describe('panelMenuBehavior', () => { it('should pass onClick from plugin extension link to menu item', async () => { const expectedOnClick = jest.fn(); - getPluginExtensionsMock.mockReturnValue({ + mocks.getPluginExtensions.mockReturnValue({ extensions: [ { id: '1', @@ -332,7 +336,7 @@ describe('panelMenuBehavior', () => { data, }; - expect(getPluginExtensionsMock).toBeCalledWith(expect.objectContaining({ context })); + expect(mocks.getPluginExtensions).toBeCalledWith(expect.objectContaining({ context })); }); it('should pass context with default time zone values when configuring extension', async () => { @@ -389,12 +393,12 @@ describe('panelMenuBehavior', () => { data, }; - expect(getPluginExtensionsMock).toBeCalledWith(expect.objectContaining({ context })); + expect(mocks.getPluginExtensions).toBeCalledWith(expect.objectContaining({ context })); }); - it('should contain menu item with category', async () => { - getPluginExtensionsMock.mockReturnValue({ - extensions: [ + it.only('should contain menu item with category', async () => { + mocks.getObservablePluginLinks.mockReturnValue( + of([ { id: '1', pluginId: '...', @@ -404,8 +408,8 @@ describe('panelMenuBehavior', () => { path: '/a/grafana-basic-app/declare-incident', category: 'Incident', }, - ], - }); + ]) + ); const { menu, panel } = await buildTestScene({}); @@ -416,7 +420,7 @@ describe('panelMenuBehavior', () => { menu.activate(); - await new Promise((r) => setTimeout(r, 1)); + await new Promise((r) => setTimeout(r, 100)); expect(menu.state.items?.length).toBe(7); @@ -438,7 +442,7 @@ describe('panelMenuBehavior', () => { }); it('should truncate category to 25 chars', async () => { - getPluginExtensionsMock.mockReturnValue({ + mocks.getPluginExtensions.mockReturnValue({ extensions: [ { id: '1', @@ -483,7 +487,7 @@ describe('panelMenuBehavior', () => { }); it('should contain menu item with category and append items without category after divider', async () => { - getPluginExtensionsMock.mockReturnValue({ + mocks.getPluginExtensions.mockReturnValue({ extensions: [ { id: '1', @@ -596,7 +600,7 @@ describe('panelMenuBehavior', () => { describe('plugin links', () => { it('should not show Metrics Drilldown menu when no Metrics Drilldown links exist', async () => { - getPluginExtensionsMock.mockReturnValue({ + mocks.getPluginExtensions.mockReturnValue({ extensions: [ { id: '1', @@ -634,7 +638,7 @@ describe('panelMenuBehavior', () => { }); it('should separate Metrics Drilldown links into their own menu', async () => { - getPluginExtensionsMock.mockReturnValue({ + mocks.getPluginExtensions.mockReturnValue({ extensions: [ { id: '1', @@ -698,7 +702,7 @@ describe('panelMenuBehavior', () => { }); it('should not show extensions menu when no non-Metrics Drilldown links exist', async () => { - getPluginExtensionsMock.mockReturnValue({ + mocks.getPluginExtensions.mockReturnValue({ extensions: [ { id: '1', diff --git a/public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx b/public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx index cb671517eaa..f6dc8f76a7a 100644 --- a/public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx +++ b/public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx @@ -1,3 +1,5 @@ +import { Subscription } from 'rxjs'; + import { getTimeZone, InterpolateFunction, @@ -8,7 +10,6 @@ import { PluginExtensionLink, PluginExtensionPanelContext, PluginExtensionPoints, - PluginExtensionTypes, urlUtil, } from '@grafana/data'; import { t } from '@grafana/i18n'; @@ -25,9 +26,7 @@ import { scenesPanelToRuleFormValues } from 'app/features/alerting/unified/utils import { getTrackingSource, shareDashboardType } from 'app/features/dashboard/components/ShareModal/utils'; import { InspectTab } from 'app/features/inspector/types'; import { getScenePanelLinksSupplier } from 'app/features/panel/panellinks/linkSuppliers'; -import { createPluginExtensionsGetter } from 'app/features/plugins/extensions/getPluginExtensions'; -import { pluginExtensionRegistries } from 'app/features/plugins/extensions/registry/setup'; -import { GetPluginExtensions } from 'app/features/plugins/extensions/types'; +import { getObservablePluginLinks } from 'app/features/plugins/extensions/getPluginExtensions'; import { createExtensionSubMenu } from 'app/features/plugins/extensions/utils'; import { dispatch } from 'app/store/store'; import { AccessControlAction } from 'app/types/accessControl'; @@ -45,18 +44,6 @@ import { VizPanelLinks, VizPanelLinksMenu } from './PanelLinks'; import { UnlinkLibraryPanelModal } from './UnlinkLibraryPanelModal'; import { PanelTimeRangeDrawer } from './panel-timerange/PanelTimeRangeDrawer'; -let getPluginExtensions: GetPluginExtensions; - -function setupGetPluginExtensions() { - if (getPluginExtensions) { - return getPluginExtensions; - } - - getPluginExtensions = createPluginExtensionsGetter(pluginExtensionRegistries); - - return getPluginExtensions; -} - // Define the category for metrics drilldown links const METRICS_DRILLDOWN_CATEGORY = 'metrics-drilldown'; @@ -64,6 +51,8 @@ const METRICS_DRILLDOWN_CATEGORY = 'metrics-drilldown'; * Behavior is called when VizPanelMenu is activated (ie when it's opened). */ export function panelMenuBehavior(menu: VizPanelMenu) { + let extensionsSubscription: Subscription; + const asyncFunc = async () => { // hm.. add another generic param to SceneObject to specify parent type? // eslint-disable-next-line @typescript-eslint/consistent-type-assertions @@ -300,20 +289,32 @@ export function panelMenuBehavior(menu: VizPanelMenu) { }); } - setupGetPluginExtensions(); - - const { extensions } = getPluginExtensions({ + // Extensions + // -------------- + extensionsSubscription = getObservablePluginLinks({ extensionPointId: PluginExtensionPoints.DashboardPanelMenu, context: createExtensionContext(panel, dashboard), limitPerPlugin: 3, - }); + }).subscribe((extensions) => { + console.log('EXTENSIONS 1', extensions); + if (dashboard.state.isEditing) { + return; + } - if (extensions.length > 0 && !dashboard.state.isEditing) { - const linkExtensions = extensions.filter((extension) => extension.type === PluginExtensionTypes.link); + const updatedItems = [...(menu.state.items ?? [])]; + const metricsDrilldownText = t( + 'dashboard-scene.panel-menu-behavior.async-func.text.metrics-drilldown', + 'Metrics drilldown' + ); + const extensionsText = t('dashboard-scene.panel-menu-behavior.async-func.text.extensions', 'Extensions'); + const metricsDrilldownItem = updatedItems.find((item) => item.text === metricsDrilldownText); + const extensionsItem = updatedItems.find((item) => item.text === extensionsText); + const extensionsItemIndex = extensionsItem ? updatedItems.indexOf(extensionsItem) : -1; + const metricsDrilldownItemIndex = metricsDrilldownItem ? updatedItems.indexOf(metricsDrilldownItem) : -1; // Separate metrics drilldown links from other links - const [metricsDrilldownLinks, otherLinks] = linkExtensions.reduce<[PluginExtensionLink[], PluginExtensionLink[]]>( - ([metricsDrilldownLinks, otherLinks], link) => { + const [metricsDrilldownLinks, otherLinks] = extensions.reduce<[PluginExtensionLink[], PluginExtensionLink[]]>( + ([metricsDrilldownLinks, otherLinks], link: PluginExtensionLink) => { if (link.category === METRICS_DRILLDOWN_CATEGORY) { metricsDrilldownLinks.push(link); } else { @@ -324,26 +325,46 @@ export function panelMenuBehavior(menu: VizPanelMenu) { [[], []] ); - // Add specific "Metrics drilldown" menu if (metricsDrilldownLinks.length > 0) { - items.push({ - text: t('dashboard-scene.panel-menu-behavior.async-func.text.metrics-drilldown', 'Metrics drilldown'), + const newMetricsDrilldownItem: PanelMenuItem = { + text: metricsDrilldownText, iconClassName: 'code-branch', type: 'submenu', subMenu: createExtensionSubMenu(metricsDrilldownLinks), - }); + }; + + // Replace existing menu item + if (metricsDrilldownItemIndex >= 0) { + updatedItems[metricsDrilldownItemIndex] = newMetricsDrilldownItem; + // Add new item + } else { + updatedItems.push(newMetricsDrilldownItem); + } } - // Add generic "Extensions" menu for other links + // Nit: this code does not take care of reactively removing the extensions submenu, + // in case all the related extensions would be removed from the registry during runtime. if (otherLinks.length > 0) { - items.push({ - text: t('dashboard-scene.panel-menu-behavior.async-func.text.extensions', 'Extensions'), + const newExtensionsItem: PanelMenuItem = { + text: extensionsText, iconClassName: 'plug', type: 'submenu', subMenu: createExtensionSubMenu(otherLinks), - }); + }; + + // Replace existing menu item + if (extensionsItemIndex >= 0) { + updatedItems[extensionsItemIndex] = newExtensionsItem; + // Add new item + } else { + updatedItems.push(newExtensionsItem); + } } - } + + console.log('EXTENSIONS 2', updatedItems); + + menu.setState({ items: updatedItems }); + }); if (moreSubMenu.length) { items.push({ @@ -378,6 +399,11 @@ export function panelMenuBehavior(menu: VizPanelMenu) { }; asyncFunc(); + + // Deactivation + return () => { + extensionsSubscription.unsubscribe(); + }; } async function getExploreMenuItem(panel: VizPanel): Promise { diff --git a/public/app/features/plugins/extensions/getPluginExtensions.test.tsx b/public/app/features/plugins/extensions/getPluginExtensions.test.tsx index c9d6ca7cafa..f20d8a5ae1e 100644 --- a/public/app/features/plugins/extensions/getPluginExtensions.test.tsx +++ b/public/app/features/plugins/extensions/getPluginExtensions.test.tsx @@ -1,10 +1,12 @@ import * as React from 'react'; -import { first, firstValueFrom, take } from 'rxjs'; +import { first, lastValueFrom, Observable, take, takeUntil, timer } from 'rxjs'; import { type PluginExtensionAddedLinkConfig, type PluginExtensionAddedComponentConfig, + PluginExtensionLink, PluginExtensionTypes, + PluginExtension, } from '@grafana/data'; import { reportInteraction } from '@grafana/runtime'; @@ -61,11 +63,15 @@ async function createRegistries( } return { - addedLinksRegistry: await addedLinksRegistry.getState(), - addedComponentsRegistry: await addedComponentsRegistry.getState(), + addedLinksRegistry, + addedComponentsRegistry, }; } +function getLastEmittedValue(observable: Observable) { + return lastValueFrom(observable.pipe(takeUntil(timer(100)))); +} + describe('getPluginExtensions()', () => { const extensionPoint1 = 'grafana/dashboard/panel/menu/v1'; const extensionPoint2 = 'plugins/myorg-basic-app/start/v1'; @@ -108,10 +114,12 @@ describe('getPluginExtensions()', () => { const registries = await createRegistries([ { pluginId, addedLinkConfigs: [link1, link2], addedComponentConfigs: [] }, ]); - const { extensions } = getPluginExtensions({ - ...registries, - extensionPointId: extensionPoint1, - }); + const { extensions } = await getLastEmittedValue( + getPluginExtensions({ + ...registries, + extensionPointId: extensionPoint1, + }) + ); expect(extensions).toHaveLength(1); expect(extensions[0]).toEqual( @@ -127,12 +135,19 @@ describe('getPluginExtensions()', () => { test('should not limit the number of extensions per plugin by default', async () => { // Registering 3 extensions for the same plugin for the same placement const registries = await createRegistries([ - { pluginId, addedLinkConfigs: [link1, link1, link1, link2], addedComponentConfigs: [] }, + { + pluginId, + addedLinkConfigs: [link1, link1, link1, link2], + addedComponentConfigs: [], + }, ]); - const { extensions } = getPluginExtensions({ - ...registries, - extensionPointId: extensionPoint1, - }); + + const { extensions } = await getLastEmittedValue( + getPluginExtensions({ + ...registries, + extensionPointId: extensionPoint1, + }) + ); expect(extensions).toHaveLength(3); expect(extensions[0]).toEqual( @@ -143,6 +158,7 @@ describe('getPluginExtensions()', () => { path: expect.stringContaining(link1.path!), }) ); + // }); }); test('should be possible to limit the number of extensions per plugin for a given placement', async () => { @@ -161,11 +177,13 @@ describe('getPluginExtensions()', () => { ]); // Limit to 1 extension per plugin - const { extensions } = getPluginExtensions({ - ...registries, - extensionPointId: extensionPoint1, - limitPerPlugin: 1, - }); + const { extensions } = await getLastEmittedValue( + getPluginExtensions({ + ...registries, + extensionPointId: extensionPoint1, + limitPerPlugin: 1, + }) + ); expect(extensions).toHaveLength(2); expect(extensions[0]).toEqual( @@ -182,10 +200,12 @@ describe('getPluginExtensions()', () => { const registries = await createRegistries([ { pluginId, addedLinkConfigs: [link1, link2], addedComponentConfigs: [] }, ]); - const { extensions } = getPluginExtensions({ - ...registries, - extensionPointId: 'placement-with-no-extensions', - }); + const { extensions } = await getLastEmittedValue( + getPluginExtensions({ + ...registries, + extensionPointId: 'placement-with-no-extensions', + }) + ); expect(extensions).toEqual([]); }); @@ -194,7 +214,7 @@ describe('getPluginExtensions()', () => { const context = { title: 'New title from the context!' }; const registries = await createRegistries([{ pluginId, addedLinkConfigs: [link2], addedComponentConfigs: [] }]); - getPluginExtensions({ ...registries, context, extensionPointId: extensionPoint2 }); + await getLastEmittedValue(getPluginExtensions({ ...registries, context, extensionPointId: extensionPoint2 })); expect(link2.configure).toHaveBeenCalledTimes(1); expect(link2.configure).toHaveBeenCalledWith(context); @@ -210,10 +230,12 @@ describe('getPluginExtensions()', () => { })); const registries = await createRegistries([{ pluginId, addedLinkConfigs: [link2], addedComponentConfigs: [] }]); - const { extensions } = getPluginExtensions({ - ...registries, - extensionPointId: extensionPoint2, - }); + const { extensions } = await getLastEmittedValue( + getPluginExtensions({ + ...registries, + extensionPointId: extensionPoint2, + }) + ); const [extension] = extensions; assertPluginExtensionLink(extension); @@ -236,10 +258,12 @@ describe('getPluginExtensions()', () => { })); const registries = await createRegistries([{ pluginId, addedLinkConfigs: [link2], addedComponentConfigs: [] }]); - const { extensions } = getPluginExtensions({ - ...registries, - extensionPointId: extensionPoint2, - }); + const { extensions } = await getLastEmittedValue( + getPluginExtensions({ + ...registries, + extensionPointId: extensionPoint2, + }) + ); const [extension] = extensions; assertPluginExtensionLink(extension); @@ -264,10 +288,12 @@ describe('getPluginExtensions()', () => { })); const registries = await createRegistries([{ pluginId, addedLinkConfigs: [link2], addedComponentConfigs: [] }]); - const { extensions } = getPluginExtensions({ - ...registries, - extensionPointId: extensionPoint2, - }); + const { extensions } = await getLastEmittedValue( + getPluginExtensions({ + ...registries, + extensionPointId: extensionPoint2, + }) + ); const [extension] = extensions; expect(link2.configure).toHaveBeenCalledTimes(1); @@ -281,11 +307,13 @@ describe('getPluginExtensions()', () => { test('should pass a read only context to the configure() function', async () => { const context = { title: 'New title from the context!' }; const registries = await createRegistries([{ pluginId, addedLinkConfigs: [link2], addedComponentConfigs: [] }]); - const { extensions } = getPluginExtensions({ - ...registries, - context, - extensionPointId: extensionPoint2, - }); + const { extensions } = await getLastEmittedValue( + getPluginExtensions({ + ...registries, + context, + extensionPointId: extensionPoint2, + }) + ); const [extension] = extensions; const readOnlyContext = (link2.configure as jest.Mock).mock.calls[0][0]; @@ -306,8 +334,8 @@ describe('getPluginExtensions()', () => { const registries = await createRegistries([{ pluginId, addedLinkConfigs: [link2], addedComponentConfigs: [] }]); - expect(() => { - getPluginExtensions({ ...registries, extensionPointId: extensionPoint2 }); + expect(async () => { + await getLastEmittedValue(getPluginExtensions({ ...registries, extensionPointId: extensionPoint2 })); }).not.toThrow(); expect(link2.configure).toHaveBeenCalledTimes(1); @@ -327,29 +355,35 @@ describe('getPluginExtensions()', () => { link2.configure = jest.fn().mockImplementation(() => overrides); const registries = await createRegistries([{ pluginId, addedLinkConfigs: [link2], addedComponentConfigs: [] }]); - const { extensions } = getPluginExtensions({ ...registries, extensionPointId: extensionPoint2 }); + const { extensions } = await getLastEmittedValue( + getPluginExtensions({ ...registries, extensionPointId: extensionPoint2 }) + ); expect(extensions).toHaveLength(0); expect(link2.configure).toHaveBeenCalledTimes(1); expect(log.error).toHaveBeenCalledTimes(1); }); - test('should skip the extension if the configure() function returns a promise', async () => { + test('should not skip the extension if the configure() function returns a promise', async () => { link2.configure = jest.fn().mockImplementation(() => Promise.resolve({})); const registries = await createRegistries([{ pluginId, addedLinkConfigs: [link2], addedComponentConfigs: [] }]); - const { extensions } = getPluginExtensions({ ...registries, extensionPointId: extensionPoint2 }); + const { extensions } = await getLastEmittedValue( + getPluginExtensions({ ...registries, extensionPointId: extensionPoint2 }) + ); - expect(extensions).toHaveLength(0); + expect(extensions).toHaveLength(1); expect(link2.configure).toHaveBeenCalledTimes(1); - expect(log.error).toHaveBeenCalledTimes(1); + expect(log.error).toHaveBeenCalledTimes(0); }); test('should skip (hide) the extension if the configure() function returns undefined', async () => { link2.configure = jest.fn().mockImplementation(() => undefined); const registries = await createRegistries([{ pluginId, addedLinkConfigs: [link2], addedComponentConfigs: [] }]); - const { extensions } = getPluginExtensions({ ...registries, extensionPointId: extensionPoint2 }); + const { extensions } = await getLastEmittedValue( + getPluginExtensions({ ...registries, extensionPointId: extensionPoint2 }) + ); expect(extensions).toHaveLength(0); expect(log.warning).toHaveBeenCalledTimes(0); // As this is intentional, no warning should be logged @@ -363,7 +397,9 @@ describe('getPluginExtensions()', () => { const context = {}; const registries = await createRegistries([{ pluginId, addedLinkConfigs: [link2], addedComponentConfigs: [] }]); - const { extensions } = getPluginExtensions({ ...registries, extensionPointId: extensionPoint2 }); + const { extensions } = await getLastEmittedValue( + getPluginExtensions({ ...registries, extensionPointId: extensionPoint2 }) + ); const [extension] = extensions; assertPluginExtensionLink(extension); @@ -387,7 +423,9 @@ describe('getPluginExtensions()', () => { link2.onClick = jest.fn().mockRejectedValue(new Error('testing')); const registries = await createRegistries([{ pluginId, addedLinkConfigs: [link2], addedComponentConfigs: [] }]); - const { extensions } = getPluginExtensions({ ...registries, extensionPointId: extensionPoint2 }); + const { extensions } = await getLastEmittedValue( + getPluginExtensions({ ...registries, extensionPointId: extensionPoint2 }) + ); const [extension] = extensions; assertPluginExtensionLink(extension); @@ -406,7 +444,9 @@ describe('getPluginExtensions()', () => { }); const registries = await createRegistries([{ pluginId, addedLinkConfigs: [link2], addedComponentConfigs: [] }]); - const { extensions } = getPluginExtensions({ ...registries, extensionPointId: extensionPoint2 }); + const { extensions } = await getLastEmittedValue( + getPluginExtensions({ ...registries, extensionPointId: extensionPoint2 }) + ); const [extension] = extensions; assertPluginExtensionLink(extension); @@ -427,7 +467,9 @@ describe('getPluginExtensions()', () => { link2.onClick = jest.fn(); const registries = await createRegistries([{ pluginId, addedLinkConfigs: [link2], addedComponentConfigs: [] }]); - const { extensions } = getPluginExtensions({ ...registries, context, extensionPointId: extensionPoint2 }); + const { extensions } = await getLastEmittedValue( + getPluginExtensions({ ...registries, context, extensionPointId: extensionPoint2 }) + ); const [extension] = extensions; assertPluginExtensionLink(extension); @@ -475,7 +517,9 @@ describe('getPluginExtensions()', () => { addedComponentConfigs: [], }, ]); - const { extensions } = getPluginExtensions({ ...registries, extensionPointId: extensionPoint1 }); + const { extensions } = await getLastEmittedValue( + getPluginExtensions({ ...registries, extensionPointId: extensionPoint1 }) + ); const [extension] = extensions; assertPluginExtensionLink(extension); @@ -499,10 +543,12 @@ describe('getPluginExtensions()', () => { addedComponentConfigs: [component1], }, ]); - const { extensions } = getPluginExtensions({ - ...registries, - extensionPointId: Array.isArray(component1.targets) ? component1.targets[0] : component1.targets, - }); + const { extensions } = await getLastEmittedValue( + getPluginExtensions({ + ...registries, + extensionPointId: Array.isArray(component1.targets) ? component1.targets[0] : component1.targets, + }) + ); expect(extensions).toHaveLength(1); expect(extensions[0]).toEqual( @@ -532,11 +578,13 @@ describe('getPluginExtensions()', () => { ], }, ]); - const { extensions } = getPluginExtensions({ - ...registries, - limitPerPlugin: 1, - extensionPointId: Array.isArray(component1.targets) ? component1.targets[0] : component1.targets, - }); + const { extensions } = await getLastEmittedValue( + getPluginExtensions({ + ...registries, + limitPerPlugin: 1, + extensionPointId: Array.isArray(component1.targets) ? component1.targets[0] : component1.targets, + }) + ); expect(extensions).toHaveLength(1); expect(extensions[0]).toEqual( @@ -585,10 +633,10 @@ describe('getObservablePluginExtensions()', () => { }); it('should emit the initial state when no changes are made to the registries', async () => { - const observable = getObservablePluginExtensions({ extensionPointId }).pipe(first()); + const observable = getObservablePluginExtensions({ extensionPointId }).pipe(takeUntil(timer(10))); await expect(observable).toEmitValuesWith((received) => { - const { extensions } = received[0]; + const { extensions } = received[received.length - 1]; expect(extensions).toHaveLength(2); expect(extensions[0].pluginId).toBe(pluginId); expect(extensions[1].pluginId).toBe(pluginId); @@ -596,7 +644,7 @@ describe('getObservablePluginExtensions()', () => { }); it('should emit the new state when the registries change', async () => { - const observable = getObservablePluginExtensions({ extensionPointId }).pipe(take(2)); + const observable = getObservablePluginExtensions({ extensionPointId }).pipe(takeUntil(timer(10))); setTimeout(() => { pluginExtensionRegistries.addedLinksRegistry.register({ @@ -614,12 +662,12 @@ describe('getObservablePluginExtensions()', () => { }, 0); await expect(observable).toEmitValuesWith((received) => { - const { extensions } = received[0]; + const { extensions } = received[1]; expect(extensions).toHaveLength(2); expect(extensions[0].pluginId).toBe(pluginId); expect(extensions[1].pluginId).toBe(pluginId); - const { extensions: extensions2 } = received[1]; + const { extensions: extensions2 } = received[received.length - 1]; expect(extensions2).toHaveLength(3); expect(extensions2[0].pluginId).toBe(pluginId); expect(extensions2[1].pluginId).toBe(pluginId); @@ -676,7 +724,7 @@ describe('getObservablePluginLinks()', () => { it('should be possible to get the last value from the observable', async () => { const observable = getObservablePluginLinks({ extensionPointId }); - const links = await firstValueFrom(observable); + const links = await getLastEmittedValue(observable); expect(links).toHaveLength(1); expect(links[0].pluginId).toBe(pluginId); @@ -699,7 +747,7 @@ describe('getObservablePluginLinks()', () => { }); const observable = getObservablePluginLinks({ extensionPointId }); - const links = await firstValueFrom(observable); + const links = await getLastEmittedValue(observable); expect(links).toHaveLength(2); expect(links[0].pluginId).toBe(pluginId); @@ -708,14 +756,220 @@ describe('getObservablePluginLinks()', () => { expect(links[1].type).toBe(PluginExtensionTypes.link); }); - it('should receive an empty array if there are no links', async () => { + it('should emit static links immediately and then emit again as async configure() functions resolve', async () => { pluginExtensionRegistries.addedLinksRegistry = new AddedLinksRegistry(); pluginExtensionRegistries.addedComponentsRegistry = new AddedComponentsRegistry(); - const observable = getObservablePluginLinks({ extensionPointId }).pipe(first()); - const links = await firstValueFrom(observable); + const staticLinkPromise = new Promise<{ title: string }>((resolve) => + setTimeout(() => resolve({ title: 'Updated Async' }), 50) + ); - expect(links).toHaveLength(0); + pluginExtensionRegistries.addedLinksRegistry.register({ + pluginId, + configs: [ + { + title: 'Static Link', + description: 'Static Description', + path: `/a/${pluginId}/static`, + targets: extensionPointId, + // No configure function - should appear immediately + }, + { + title: 'Async Link', + description: 'Async Description', + path: `/a/${pluginId}/async`, + targets: extensionPointId, + configure: jest.fn().mockReturnValue(staticLinkPromise), + }, + ], + }); + + const observable = getObservablePluginLinks({ extensionPointId }); + const emittedValues: PluginExtensionLink[][] = []; + + const subscription = observable.subscribe((links) => { + emittedValues.push([...links]); + }); + + // Wait for initial emission (should include static link) + await new Promise((resolve) => setTimeout(resolve, 10)); + + expect(emittedValues.length).toBeGreaterThanOrEqual(1); + const firstEmission = emittedValues[0]; + expect(firstEmission.length).toBeGreaterThanOrEqual(1); + const staticLink = firstEmission.find((link) => link.title === 'Static Link'); + expect(staticLink).toBeDefined(); + + // Wait for async configure to resolve + await new Promise((resolve) => setTimeout(resolve, 100)); + + // Should have emitted again with the async link + expect(emittedValues.length).toBeGreaterThanOrEqual(2); + const lastEmission = emittedValues[emittedValues.length - 1]; + expect(lastEmission.length).toBe(2); + const asyncLink = lastEmission.find((link) => link.title === 'Updated Async'); + expect(asyncLink).toBeDefined(); + + subscription.unsubscribe(); + }); + + it('should emit the links with a sync configure() function first', async () => { + pluginExtensionRegistries.addedLinksRegistry = new AddedLinksRegistry(); + pluginExtensionRegistries.addedComponentsRegistry = new AddedComponentsRegistry(); + + pluginExtensionRegistries.addedLinksRegistry.register({ + pluginId, + configs: [ + { + title: 'Sync Link', + description: 'Sync Description', + path: `/a/${pluginId}/sync`, + targets: extensionPointId, + configure: jest.fn().mockReturnValue({ title: 'Sync Link (updated)' }), + }, + { + title: 'Async Link', + description: 'Async Description', + path: `/a/${pluginId}/async`, + targets: extensionPointId, + configure: jest.fn().mockReturnValue(Promise.resolve({ title: 'Async Link (updated)' })), + }, + ], + }); + + await expect(getObservablePluginLinks({ extensionPointId }).pipe(take(2))).toEmitValuesWith((received) => { + expect(received.length).toBe(2); + + const links1 = received[0]; + expect(links1).toHaveLength(1); + expect(links1[0].title).toBe('Sync Link (updated)'); + + const links2 = received[1]; + expect(links2).toHaveLength(2); + expect(links2[0].title).toBe('Sync Link (updated)'); + expect(links2[1].title).toBe('Async Link (updated)'); + }); + }); + + it('should emit incrementally as multiple async configure() functions resolve at different times', async () => { + pluginExtensionRegistries.addedLinksRegistry = new AddedLinksRegistry(); + pluginExtensionRegistries.addedComponentsRegistry = new AddedComponentsRegistry(); + + const fastConfigure = new Promise<{ title: string }>((resolve) => + setTimeout(() => resolve({ title: 'Fast Link (updated)' }), 20) + ); + const slowConfigure = new Promise<{ title: string }>((resolve) => + setTimeout(() => resolve({ title: 'Slow Link (updated)' }), 80) + ); + + pluginExtensionRegistries.addedLinksRegistry.register({ + pluginId, + configs: [ + { + title: 'Static Link', // no dynamic behaviour = no configure() function + description: 'Static Description', + path: `/a/${pluginId}/static`, + targets: extensionPointId, + }, + { + title: 'Sync Link', + description: 'Sync Description', + path: `/a/${pluginId}/sync`, + targets: extensionPointId, + configure: jest.fn().mockReturnValue({ title: 'Sync Link (updated)' }), + }, + { + title: 'Fast Link', + description: 'Fast Description', + path: `/a/${pluginId}/fast`, + targets: extensionPointId, + configure: jest.fn().mockReturnValue(fastConfigure), + }, + { + title: 'Slow Link', + description: 'Slow Description', + path: `/a/${pluginId}/slow`, + targets: extensionPointId, + configure: jest.fn().mockReturnValue(slowConfigure), + }, + ], + }); + + await expect(getObservablePluginLinks({ extensionPointId }).pipe(take(4))).toEmitValuesWith((received) => { + expect(received.length).toBe(4); + + const links0 = received[0]; + expect(links0).toHaveLength(1); + expect(links0[0].title).toBe('Static Link'); + + const links1 = received[1]; + expect(links1).toHaveLength(2); + expect(links1[0].title).toBe('Static Link'); + expect(links1[1].title).toBe('Sync Link (updated)'); + + const links2 = received[2]; + expect(links2).toHaveLength(3); + expect(links2[0].title).toBe('Static Link'); + expect(links2[1].title).toBe('Sync Link (updated)'); + expect(links2[2].title).toBe('Fast Link (updated)'); + + const links3 = received[3]; + expect(links3).toHaveLength(4); + expect(links3[0].title).toBe('Static Link'); + expect(links3[1].title).toBe('Sync Link (updated)'); + expect(links3[2].title).toBe('Fast Link (updated)'); + expect(links3[3].title).toBe('Slow Link (updated)'); + }); + }); + + it('should handle buggy async configure() functions that never resolve without blocking', async () => { + pluginExtensionRegistries.addedLinksRegistry = new AddedLinksRegistry(); + pluginExtensionRegistries.addedComponentsRegistry = new AddedComponentsRegistry(); + + const buggyPromise = new Promise(() => { + // Never resolves - simulating a buggy implementation + }); + + pluginExtensionRegistries.addedLinksRegistry.register({ + pluginId, + configs: [ + { + title: 'Static Link', + description: 'Static Description', + path: `/a/${pluginId}/static`, + targets: extensionPointId, + }, + { + title: 'Buggy Async Link', + description: 'Buggy Description', + path: `/a/${pluginId}/buggy`, + targets: extensionPointId, + configure: jest.fn().mockReturnValue(buggyPromise), + }, + ], + }); + + const observable = getObservablePluginLinks({ extensionPointId }); + const emittedValues: PluginExtensionLink[][] = []; + + const subscription = observable.subscribe((links) => { + emittedValues.push([...links]); + }); + + // Wait a bit + await new Promise((resolve) => setTimeout(resolve, 50)); + + // Should have emitted initial value with static link + expect(emittedValues.length).toBeGreaterThanOrEqual(1); + const firstEmission = emittedValues[0]; + expect(firstEmission.length).toBe(1); + expect(firstEmission[0].title).toBe('Static Link'); + + // Buggy link should never appear + const hasBuggyLink = emittedValues.some((emission) => emission.some((link) => link.title === 'Buggy Async Link')); + expect(hasBuggyLink).toBe(false); + + subscription.unsubscribe(); }); }); @@ -767,7 +1021,7 @@ describe('getObservablePluginComponents()', () => { it('should be possible to get the last value from the observable', async () => { const observable = getObservablePluginComponents({ extensionPointId }); - const components = await firstValueFrom(observable); + const components = await getLastEmittedValue(observable); expect(components).toHaveLength(1); expect(components[0].pluginId).toBe(pluginId); @@ -791,7 +1045,7 @@ describe('getObservablePluginComponents()', () => { }); const observable = getObservablePluginComponents({ extensionPointId }); - const components = await firstValueFrom(observable); + const components = await getLastEmittedValue(observable); expect(components).toHaveLength(2); expect(components[0].pluginId).toBe(pluginId); @@ -799,14 +1053,4 @@ describe('getObservablePluginComponents()', () => { expect(components[1].pluginId).toBe(pluginId); expect(components[1].type).toBe(PluginExtensionTypes.component); }); - - it('should receive an empty array if there are no components', async () => { - pluginExtensionRegistries.addedLinksRegistry = new AddedLinksRegistry(); - pluginExtensionRegistries.addedComponentsRegistry = new AddedComponentsRegistry(); - - const observable = getObservablePluginComponents({ extensionPointId }).pipe(first()); - const components = await firstValueFrom(observable); - - expect(components).toHaveLength(0); - }); }); diff --git a/public/app/features/plugins/extensions/getPluginExtensions.ts b/public/app/features/plugins/extensions/getPluginExtensions.ts index c81fd7c26f4..d478e48a1b1 100644 --- a/public/app/features/plugins/extensions/getPluginExtensions.ts +++ b/public/app/features/plugins/extensions/getPluginExtensions.ts @@ -1,5 +1,18 @@ import { isString } from 'lodash'; -import { combineLatest, map, Observable } from 'rxjs'; +import { + combineLatest, + lastValueFrom, + from, + map, + merge, + mergeMap, + Observable, + of, + scan, + startWith, + identity, + filter, +} from 'rxjs'; import { PluginExtensionTypes, @@ -9,13 +22,12 @@ import { } from '@grafana/data'; import { type GetObservablePluginLinks, type GetObservablePluginComponents } from '@grafana/runtime/internal'; -import { log } from './logs/log'; -import { AddedComponentRegistryItem } from './registry/AddedComponentsRegistry'; -import { AddedLinkRegistryItem } from './registry/AddedLinksRegistry'; -import { RegistryType } from './registry/Registry'; +import { ExtensionsLog, log } from './logs/log'; +import { AddedComponentRegistryItem, AddedComponentsRegistry } from './registry/AddedComponentsRegistry'; +import { AddedLinksRegistry } from './registry/AddedLinksRegistry'; import { pluginExtensionRegistries } from './registry/setup'; import type { PluginExtensionRegistries } from './registry/types'; -import { GetExtensions, GetExtensionsOptions, GetPluginExtensions } from './types'; +import { GetExtensionsOptions, GetPluginExtensions } from './types'; import { getReadOnlyProxy, generateExtensionId, @@ -36,159 +48,258 @@ import { export const getObservablePluginExtensions = ( options: Omit -): Observable> => { - const { extensionPointId } = options; +): Observable<{ extensions: PluginExtension[] }> => { const { addedComponentsRegistry, addedLinksRegistry } = pluginExtensionRegistries; - return combineLatest([ - addedComponentsRegistry.asObservableSlice((state) => state[extensionPointId]), - addedLinksRegistry.asObservableSlice((state) => state[extensionPointId]), - ]).pipe( - map(([components, links]) => - getPluginExtensions({ - ...options, - addedComponentsRegistry: { - [extensionPointId]: components, - }, - addedLinksRegistry: { - [extensionPointId]: links, - }, - }) - ) - ); + return getPluginExtensions({ + ...options, + addedComponentsRegistry, + addedLinksRegistry, + }); }; export const getObservablePluginLinks: GetObservablePluginLinks = (options) => { return getObservablePluginExtensions(options).pipe( - map((value) => value.extensions.filter((extension) => extension.type === PluginExtensionTypes.link)) + map((value) => value.extensions.filter((extension) => extension.type === PluginExtensionTypes.link)), + filter((extensions) => extensions.length > 0) ); }; export const getObservablePluginComponents: GetObservablePluginComponents = (options) => { return getObservablePluginExtensions(options).pipe( - map((value) => value.extensions.filter((extension) => extension.type === PluginExtensionTypes.component)) + map((value) => value.extensions.filter((extension) => extension.type === PluginExtensionTypes.component)), + filter((extensions) => extensions.length > 0) ); }; export function createPluginExtensionsGetter(registries: PluginExtensionRegistries): GetPluginExtensions { - let addedComponentsRegistry: RegistryType; - let addedLinksRegistry: RegistryType>>; + return async (options) => { + const observable = getPluginExtensions({ + ...options, + addedComponentsRegistry: registries.addedComponentsRegistry, + addedLinksRegistry: registries.addedLinksRegistry, + }); - // Create registry subscriptions to keep an copy of the registry state for use in the non-async - // plugin extensions getter. - registries.addedComponentsRegistry.asObservable().subscribe((componentsRegistry) => { - addedComponentsRegistry = componentsRegistry; - }); - - registries.addedLinksRegistry.asObservable().subscribe((linksRegistry) => { - addedLinksRegistry = linksRegistry; - }); - - return (options) => getPluginExtensions({ ...options, addedComponentsRegistry, addedLinksRegistry }); + // Convert Observable to Promise by taking the last emitted value + // This will wait for all configure() functions to resolve and return the final state + return lastValueFrom(observable, { defaultValue: { extensions: [] } }); + }; } -// Returns with a list of plugin extensions for the given extension point -export const getPluginExtensions: GetExtensions = ({ +function getAddedComponentLog(registryItem: AddedComponentRegistryItem) { + return log.child({ + title: registryItem.title, + description: registryItem.description ?? '', + pluginId: registryItem.pluginId, + }); +} + +function createPluginExtensionComponent({ + extensionPointId, + registryItem, + log, +}: { + extensionPointId: string; + registryItem: AddedComponentRegistryItem; + log?: ExtensionsLog; +}): PluginExtensionComponent { + return { + id: generateExtensionId(registryItem.pluginId, extensionPointId, registryItem.title), + type: PluginExtensionTypes.component, + pluginId: registryItem.pluginId, + title: registryItem.title, + description: registryItem.description ?? '', + component: wrapWithPluginContext({ + pluginId: registryItem.pluginId, + extensionTitle: registryItem.title, + Component: registryItem.component, + log: log ?? getAddedComponentLog(registryItem), + }), + }; +} + +// Returns an observable that emits plugin extensions for the given extension point +// Emits incrementally as configure() functions resolve for link extensions +export function getPluginExtensions({ context, extensionPointId, limitPerPlugin, addedLinksRegistry, addedComponentsRegistry, -}) => { +}: { + context?: object | Record; + extensionPointId: string; + limitPerPlugin?: number; + addedLinksRegistry: AddedLinksRegistry; + addedComponentsRegistry: AddedComponentsRegistry; +}): Observable<{ extensions: PluginExtension[] }> { const frozenContext = context ? getReadOnlyProxy(context) : {}; - // We don't return the extensions separated by type, because in that case it would be much harder to define a sort-order for them. - const extensions: PluginExtension[] = []; - const extensionsByPlugin: Record = {}; - for (const addedLink of addedLinksRegistry?.[extensionPointId] ?? []) { - try { - const { pluginId } = addedLink; - // Only limit if the `limitPerPlugin` is set - if (limitPerPlugin && extensionsByPlugin[pluginId] >= limitPerPlugin) { - continue; + return combineLatest([ + addedComponentsRegistry.asObservableSlice((state) => state[extensionPointId]), + addedLinksRegistry.asObservableSlice((state) => state[extensionPointId]), + ]).pipe( + mergeMap(([addedComponents, addedLinks]) => { + const staticLinkExtensionsByPlugin: Record = {}; + const componentExtensionsByPlugin: Record = {}; + + // ADDED COMPONENTS --------------------------------------------------- + // Process components immediately (they don't have async configure) + const componentExtensions: PluginExtensionComponent[] = []; + for (const registryItem of addedComponents ?? []) { + // Only limit if the `limitPerPlugin` is set + if (limitPerPlugin && componentExtensionsByPlugin[registryItem.pluginId] >= limitPerPlugin) { + continue; + } + + if (componentExtensionsByPlugin[registryItem.pluginId] === undefined) { + componentExtensionsByPlugin[registryItem.pluginId] = 0; + } + + componentExtensions.push( + createPluginExtensionComponent({ + registryItem, + extensionPointId, + }) + ); + + componentExtensionsByPlugin[registryItem.pluginId] += 1; } - if (extensionsByPlugin[pluginId] === undefined) { - extensionsByPlugin[pluginId] = 0; - } + // LINKS ------------------------------------------------------------- + const links = addedLinks ?? []; + const linksWithConfigure = links.filter((addedLink) => addedLink.configure); + const linksWithoutConfigure = links.filter((addedLink) => !addedLink.configure); - const linkLog = log.child({ - pluginId, - extensionPointId, - path: addedLink.path ?? '', - title: addedLink.title, - description: addedLink.description ?? '', - onClick: typeof addedLink.onClick, - }); - // Run the configure() function with the current context, and apply the ovverides - const overrides = getLinkExtensionOverrides(pluginId, addedLink, linkLog, frozenContext); + // Process static links (without configure function) immediately + const staticLinkExtensions: PluginExtensionLink[] = []; + for (const addedLink of linksWithoutConfigure) { + const { pluginId } = addedLink; - // configure() returned an `undefined` -> hide the extension - if (addedLink.configure && overrides === undefined) { - continue; - } + // Only limit if the `limitPerPlugin` is set + if (limitPerPlugin && staticLinkExtensionsByPlugin[pluginId] >= limitPerPlugin) { + continue; + } - const path = overrides?.path || addedLink.path; - const extension: PluginExtensionLink = { - id: generateExtensionId(pluginId, extensionPointId, addedLink.title), - type: PluginExtensionTypes.link, - pluginId: pluginId, - onClick: getLinkExtensionOnClick(pluginId, extensionPointId, addedLink, linkLog, frozenContext), + if (staticLinkExtensionsByPlugin[pluginId] === undefined) { + staticLinkExtensionsByPlugin[pluginId] = 0; + } - // Configurable properties - icon: overrides?.icon || addedLink.icon, - title: overrides?.title || addedLink.title, - description: overrides?.description || addedLink.description || '', - path: isString(path) ? getLinkExtensionPathWithTracking(pluginId, path, extensionPointId) : undefined, - category: overrides?.category || addedLink.category, - }; - - extensions.push(extension); - extensionsByPlugin[pluginId] += 1; - } catch (error) { - if (error instanceof Error) { - log.error(error.message, { - stack: error.stack ?? '', - message: error.message, + const linkLog = log.child({ + pluginId, + extensionPointId, + path: addedLink.path ?? '', + title: addedLink.title, + description: addedLink.description ?? '', + onClick: typeof addedLink.onClick, }); + + const extension: PluginExtensionLink = { + id: generateExtensionId(pluginId, extensionPointId, addedLink.title), + type: PluginExtensionTypes.link, + pluginId: pluginId, + onClick: getLinkExtensionOnClick(pluginId, extensionPointId, addedLink, linkLog, frozenContext), + icon: addedLink.icon, + title: addedLink.title, + description: addedLink.description || '', + path: isString(addedLink.path) + ? getLinkExtensionPathWithTracking(pluginId, addedLink.path, extensionPointId) + : undefined, + category: addedLink.category, + }; + + staticLinkExtensions.push(extension); + staticLinkExtensionsByPlugin[pluginId] += 1; } - } - } - const addedComponents = addedComponentsRegistry?.[extensionPointId] ?? []; - for (const addedComponent of addedComponents) { - // Only limit if the `limitPerPlugin` is set - if (limitPerPlugin && extensionsByPlugin[addedComponent.pluginId] >= limitPerPlugin) { - continue; - } + // No links with configure, return components + static links immediately + if (linksWithConfigure.length === 0) { + return of({ extensions: [...componentExtensions, ...staticLinkExtensions] }); + } - if (extensionsByPlugin[addedComponent.pluginId] === undefined) { - extensionsByPlugin[addedComponent.pluginId] = 0; - } + // Process links incrementally - emit as each configure() resolves + const linkObservables = linksWithConfigure.map((addedLink) => { + const { pluginId } = addedLink; + const linkId = generateExtensionId(pluginId, extensionPointId, addedLink.title); + const linkLog = log.child({ + pluginId, + extensionPointId, + path: addedLink.path ?? '', + title: addedLink.title, + description: addedLink.description ?? '', + onClick: typeof addedLink.onClick, + }); - const componentLog = log.child({ - title: addedComponent.title, - description: addedComponent.description ?? '', - pluginId: addedComponent.pluginId, - }); + return from(getLinkExtensionOverrides(addedLink, linkLog, frozenContext)).pipe( + map((overrides): PluginExtensionLink | null => { + // configure() returned an `undefined` -> hide the extension + if (overrides === undefined) { + return null; + } - const extension: PluginExtensionComponent = { - id: generateExtensionId(addedComponent.pluginId, extensionPointId, addedComponent.title), - type: PluginExtensionTypes.component, - pluginId: addedComponent.pluginId, - title: addedComponent.title, - description: addedComponent.description ?? '', - component: wrapWithPluginContext({ - pluginId: addedComponent.pluginId, - extensionTitle: addedComponent.title, - Component: addedComponent.component, - log: componentLog, - }), - }; + const path = overrides?.path || addedLink.path; + const extension: PluginExtensionLink = { + id: linkId, + type: PluginExtensionTypes.link, + pluginId: pluginId, + onClick: getLinkExtensionOnClick(pluginId, extensionPointId, addedLink, linkLog, frozenContext), - extensions.push(extension); - extensionsByPlugin[addedComponent.pluginId] += 1; - } + // Configurable properties + icon: overrides?.icon || addedLink.icon, + title: overrides?.title || addedLink.title, + description: overrides?.description || addedLink.description || '', + path: isString(path) ? getLinkExtensionPathWithTracking(pluginId, path, extensionPointId) : undefined, + category: overrides?.category || addedLink.category, + }; - return { extensions }; -}; + return extension; + }) + ); + }); + + // Merge all link observables and accumulate results as they resolve + // We use startWith to emit immediately with static links + components, + // then emit again as each configure() function resolves + return merge(...linkObservables).pipe( + scan((acc: Set, result: PluginExtensionLink | null) => { + if (!result) { + return acc; + } + + return new Set([...acc.values(), result]); + }, new Set()), + map((linkResults) => { + // Build extensions array: components + static links + resolved links (excluding null/hidden ones) + const linkExtensions: PluginExtensionLink[] = []; + const linkExtensionsByPlugin: Record = {}; + + for (const result of [...staticLinkExtensions, ...linkResults.values()]) { + if (result) { + const { pluginId } = result; + + // Only limit if the `limitPerPlugin` is set + if (limitPerPlugin && linkExtensionsByPlugin[pluginId] >= limitPerPlugin) { + continue; + } + + if (linkExtensionsByPlugin[pluginId] === undefined) { + linkExtensionsByPlugin[pluginId] = 0; + } + + linkExtensions.push(result); + linkExtensionsByPlugin[pluginId] += 1; + } + } + + // Combine components and links + return { + extensions: [...componentExtensions, ...linkExtensions], + }; + }), + [...componentExtensions, ...staticLinkExtensions].length > 0 + ? startWith({ extensions: [...componentExtensions, ...staticLinkExtensions] }) + : identity + ); + }) + ); +} diff --git a/public/app/features/plugins/extensions/types.ts b/public/app/features/plugins/extensions/types.ts index b83c2837fe7..d122a7b9307 100644 --- a/public/app/features/plugins/extensions/types.ts +++ b/public/app/features/plugins/extensions/types.ts @@ -12,11 +12,13 @@ export type GetExtensionsOptions = { addedLinksRegistry: RegistryType | undefined; }; -export type GetExtensions = (options: GetExtensionsOptions) => { extensions: PluginExtension[] }; +export type GetExtensions = (options: GetExtensionsOptions) => Promise<{ extensions: PluginExtension[] }>; + +export type GetExtensionsSync = (options: GetExtensionsOptions) => { extensions: PluginExtension[] }; export type GetPluginExtensions = (options: { extensionPointId: string; // Make sure this object is properly memoized and not mutated. context?: object | Record; limitPerPlugin?: number; -}) => { extensions: T[] }; +}) => Promise<{ extensions: T[] }>; diff --git a/public/app/features/plugins/extensions/usePluginComponents.tsx b/public/app/features/plugins/extensions/usePluginComponents.tsx index c7f5b40815f..92df9e46814 100644 --- a/public/app/features/plugins/extensions/usePluginComponents.tsx +++ b/public/app/features/plugins/extensions/usePluginComponents.tsx @@ -11,7 +11,7 @@ import { UsePluginComponentsOptions, UsePluginComponentsResult } from '@grafana/ import { AddedComponentRegistryItem } from './registry/AddedComponentsRegistry'; import { useAddedComponentsRegistrySlice } from './registry/useRegistrySlice'; import { useLoadAppPlugins } from './useLoadAppPlugins'; -import { generateExtensionId, getExtensionPointPluginDependencies } from './utils'; +import { generateExtensionId, getExtensionPointPluginDependencies, useExtensionPointLog } from './utils'; import { validateExtensionPoint } from './validateExtensionPoint'; // Returns an array of component extensions for the given extension point @@ -21,10 +21,16 @@ export function usePluginComponents({ }: UsePluginComponentsOptions): UsePluginComponentsResult { const registryItems = useAddedComponentsRegistrySlice(extensionPointId); const pluginContext = usePluginContext(); + const extensionPointLog = useExtensionPointLog(extensionPointId); const { isLoading: isLoadingAppPlugins } = useLoadAppPlugins(getExtensionPointPluginDependencies(extensionPointId)); return useMemo(() => { - const { result } = validateExtensionPoint({ extensionPointId, pluginContext, isLoadingAppPlugins }); + const { result } = validateExtensionPoint({ + extensionPointId, + pluginContext, + isLoadingAppPlugins, + extensionPointLog, + }); if (result) { return { @@ -58,7 +64,7 @@ export function usePluginComponents({ isLoading: false, components, }; - }, [extensionPointId, limitPerPlugin, pluginContext, registryItems, isLoadingAppPlugins]); + }, [extensionPointId, limitPerPlugin, pluginContext, registryItems, extensionPointLog, isLoadingAppPlugins]); } export function createComponentWithMeta( diff --git a/public/app/features/plugins/extensions/usePluginFunctions.tsx b/public/app/features/plugins/extensions/usePluginFunctions.tsx index b0c065d7aaa..5c5f6782406 100644 --- a/public/app/features/plugins/extensions/usePluginFunctions.tsx +++ b/public/app/features/plugins/extensions/usePluginFunctions.tsx @@ -5,7 +5,7 @@ import { UsePluginFunctionsOptions, UsePluginFunctionsResult } from '@grafana/ru import { useAddedFunctionsRegistrySlice } from './registry/useRegistrySlice'; import { useLoadAppPlugins } from './useLoadAppPlugins'; -import { generateExtensionId, getExtensionPointPluginDependencies } from './utils'; +import { generateExtensionId, getExtensionPointPluginDependencies, useExtensionPointLog } from './utils'; import { validateExtensionPoint } from './validateExtensionPoint'; // Returns an array of component extensions for the given extension point @@ -15,11 +15,17 @@ export function usePluginFunctions({ }: UsePluginFunctionsOptions): UsePluginFunctionsResult { const registryItems = useAddedFunctionsRegistrySlice(extensionPointId); const pluginContext = usePluginContext(); + const extensionPointLog = useExtensionPointLog(extensionPointId); const deps = getExtensionPointPluginDependencies(extensionPointId); const { isLoading: isLoadingAppPlugins } = useLoadAppPlugins(deps); return useMemo(() => { - const { result } = validateExtensionPoint({ extensionPointId, pluginContext, isLoadingAppPlugins }); + const { result } = validateExtensionPoint({ + extensionPointId, + pluginContext, + isLoadingAppPlugins, + extensionPointLog, + }); if (result) { return { @@ -58,5 +64,5 @@ export function usePluginFunctions({ isLoading: false, functions: results, }; - }, [extensionPointId, limitPerPlugin, pluginContext, registryItems, isLoadingAppPlugins]); + }, [extensionPointId, limitPerPlugin, pluginContext, registryItems, isLoadingAppPlugins, extensionPointLog]); } diff --git a/public/app/features/plugins/extensions/usePluginLinks.test.tsx b/public/app/features/plugins/extensions/usePluginLinks.test.tsx index ba6414749db..533ee52a7c6 100644 --- a/public/app/features/plugins/extensions/usePluginLinks.test.tsx +++ b/public/app/features/plugins/extensions/usePluginLinks.test.tsx @@ -1,4 +1,4 @@ -import { act, renderHook } from '@testing-library/react'; +import { act, renderHook, waitFor } from '@testing-library/react'; import type { JSX } from 'react'; import { @@ -222,6 +222,215 @@ describe('usePluginLinks()', () => { expect(result.current.links[1].title).toBe('2'); }); + it('should update link properties using the provided SYNC configure() function', async () => { + const context = { foo: 'bar' }; + let { result, rerender } = renderHook(() => usePluginLinks({ extensionPointId, context }), { wrapper }); + + // Add extensions to the registry + act(() => { + registries.addedLinksRegistry.register({ + pluginId, + configs: [ + { + targets: extensionPointId, + title: 'Original Title', + description: 'Original Description', + path: `/a/${pluginId}/original`, + icon: 'heart', + category: 'original-category', + // @ts-ignore + configure: (context: { foo: 'bar' }) => { + return { + title: `Title: ${context.foo}`, + description: 'Updated Description', + path: 'updated/path', + icon: 'star', + category: 'updated-category', + }; + }, + }, + ], + }); + }); + + // Check if the hook returns the new extensions + rerender(); + + await waitFor(() => { + expect(result.current.isLoading).toBe(false); + expect(result.current.links.length).toBe(1); + }); + + rerender(); + + expect(result.current.links.length).toBe(1); + expect(result.current.links[0].title).toBe('Title: bar'); + expect(result.current.links[0].description).toBe('Updated Description'); + // We are adding link tracking parameters to the link when updating via the `configure()` function + expect(result.current.links[0].path).toBe( + `updated/path?uel_pid=${pluginId}&uel_epid=${encodeURIComponent(extensionPointId)}` + ); + expect(result.current.links[0].icon).toBe('star'); + expect(result.current.links[0].category).toBe('updated-category'); + }); + + it('should update link properties using the provided ASYNC configure() function', async () => { + const context = { foo: 'bar' }; + let { result, rerender } = renderHook(() => usePluginLinks({ extensionPointId, context }), { wrapper }); + + // Add extensions to the registry with async configure function + act(() => { + registries.addedLinksRegistry.register({ + pluginId, + configs: [ + { + targets: extensionPointId, + title: 'Original Title', + description: 'Original Description', + path: `/a/${pluginId}/original`, + icon: 'heart', + category: 'original-category', + // @ts-ignore + configure: async (context: { foo: 'bar' }) => { + // Simulate async operation + await new Promise((resolve) => setTimeout(resolve, 10)); + return { + title: `Async Title: ${context.foo}`, + description: 'Async Updated Description', + path: 'async/updated/path', + icon: 'cloud', + category: 'async-updated-category', + }; + }, + }, + ], + }); + }); + + // With the new implementation, links should appear incrementally as configure() resolves + // Initially, the link may not be visible yet (depending on timing) + rerender(); + + // Wait for async configure to complete - link should appear + await waitFor(() => { + expect(result.current.links.length).toBe(1); + }); + + rerender(); + + expect(result.current.links.length).toBe(1); + expect(result.current.links[0].title).toBe('Async Title: bar'); + expect(result.current.links[0].description).toBe('Async Updated Description'); + // We are adding link tracking parameters to the link when updating via the async `configure()` function + expect(result.current.links[0].path).toBe( + `async/updated/path?uel_pid=${pluginId}&uel_epid=${encodeURIComponent(extensionPointId)}` + ); + expect(result.current.links[0].icon).toBe('cloud'); + expect(result.current.links[0].category).toBe('async-updated-category'); + }); + + it('should return static links immediately and update incrementally as async configure() functions resolve', async () => { + const context = { foo: 'bar' }; + let { result, rerender } = renderHook(() => usePluginLinks({ extensionPointId, context }), { wrapper }); + + // Add a mix of static link and async configure link + act(() => { + registries.addedLinksRegistry.register({ + pluginId, + configs: [ + { + targets: extensionPointId, + title: 'Static Link', + description: 'Static Description', + path: `/a/${pluginId}/static`, + // No configure function - should appear immediately + }, + { + targets: extensionPointId, + title: 'Async Link', + description: 'Async Description', + path: `/a/${pluginId}/async`, + // @ts-ignore + configure: async (context: { foo: 'bar' }) => { + await new Promise((resolve) => setTimeout(resolve, 50)); + return { + title: 'Updated Async Title', + }; + }, + }, + ], + }); + }); + + rerender(); + + // Static link should appear immediately + await waitFor(() => { + expect(result.current.links.length).toBeGreaterThanOrEqual(1); + const staticLink = result.current.links.find((link) => link.title === 'Static Link'); + expect(staticLink).toBeDefined(); + }); + + // Wait for async configure to complete + await waitFor( + () => { + expect(result.current.links.length).toBe(2); + const asyncLink = result.current.links.find((link) => link.title === 'Updated Async Title'); + expect(asyncLink).toBeDefined(); + }, + { timeout: 2000 } + ); + }); + + it('should handle buggy async configure() functions that never resolve without blocking', async () => { + const context = { foo: 'bar' }; + let { result, rerender } = renderHook(() => usePluginLinks({ extensionPointId, context }), { wrapper }); + + // Add a static link and a buggy async configure link that never resolves + act(() => { + registries.addedLinksRegistry.register({ + pluginId, + configs: [ + { + targets: extensionPointId, + title: 'Static Link', + description: 'Static Description', + path: `/a/${pluginId}/static`, + }, + { + targets: extensionPointId, + title: 'Buggy Async Link', + description: 'Buggy Description', + path: `/a/${pluginId}/buggy`, + // @ts-ignore + configure: async () => { + // This promise never resolves - simulating a buggy implementation + return new Promise(() => { + // Never resolves + }); + }, + }, + ], + }); + }); + + rerender(); + + // Static link should appear immediately, even though buggy async configure never resolves + await waitFor(() => { + expect(result.current.links.length).toBe(1); + expect(result.current.links[0].title).toBe('Static Link'); + }); + + // Wait a bit to ensure buggy async configure doesn't cause issues + await new Promise((resolve) => setTimeout(resolve, 100)); + + // Static link should still be there, buggy link should not appear + rerender(); + expect(result.current.links.length).toBe(1); + expect(result.current.links[0].title).toBe('Static Link'); + }); + it('should not validate the extension point meta-info in production mode', () => { // Empty list of extension points in the plugin meta (from plugin.json) wrapper = ({ children }: { children: React.ReactNode }) => ( diff --git a/public/app/features/plugins/extensions/usePluginLinks.tsx b/public/app/features/plugins/extensions/usePluginLinks.tsx index 9ec5d215f94..45bff4f66f3 100644 --- a/public/app/features/plugins/extensions/usePluginLinks.tsx +++ b/public/app/features/plugins/extensions/usePluginLinks.tsx @@ -1,100 +1,79 @@ -import { isString } from 'lodash'; -import { useMemo } from 'react'; +import { isEqual } from 'lodash'; +import { useEffect, useMemo, useRef, useState } from 'react'; +import { useObservable } from 'react-use'; +import { of } from 'rxjs'; -import { PluginExtensionLink, PluginExtensionTypes, usePluginContext } from '@grafana/data'; +import { usePluginContext } from '@grafana/data'; import { UsePluginLinksOptions, UsePluginLinksResult } from '@grafana/runtime'; -import { useAddedLinksRegistrySlice } from './registry/useRegistrySlice'; +import { getObservablePluginLinks } from './getPluginExtensions'; import { useLoadAppPlugins } from './useLoadAppPlugins'; -import { - generateExtensionId, - getExtensionPointPluginDependencies, - getLinkExtensionOnClick, - getLinkExtensionOverrides, - getLinkExtensionPathWithTracking, - getReadOnlyProxy, -} from './utils'; +import { getExtensionPointPluginDependencies, getReadOnlyProxy, useExtensionPointLog } from './utils'; import { validateExtensionPoint } from './validateExtensionPoint'; -// Returns an array of component extensions for the given extension point +// Returns an array of link extensions for the given extension point export function usePluginLinks({ limitPerPlugin, extensionPointId, - context, + context: contextProp, }: UsePluginLinksOptions): UsePluginLinksResult { - const registryItems = useAddedLinksRegistrySlice(extensionPointId); + // Context: + // - protecting against inline object definitions at the call-site + // - protecting against mutating the common context object by freezing it const pluginContext = usePluginContext(); + const prevContext = useRef(); + const [context, setContext] = useState(); + // Preloading app plugins that register links to this extension point const { isLoading: isLoadingAppPlugins } = useLoadAppPlugins(getExtensionPointPluginDependencies(extensionPointId)); + const extensionPointLog = useExtensionPointLog(extensionPointId); - return useMemo(() => { - const { result, pointLog } = validateExtensionPoint({ + // Context object equality check + // (Ideally the callsite passes in a memoized object, or an object that doesn't change between rerenders.) + useEffect(() => { + if (prevContext.current === undefined || !isEqual(prevContext.current, contextProp)) { + prevContext.current = contextProp; + setContext(getReadOnlyProxy(contextProp ?? {})); + } + }, [contextProp]); + + // Extension point validation + const { result: validationResult } = useMemo( + () => + validateExtensionPoint({ + extensionPointId, + pluginContext, + isLoadingAppPlugins, + extensionPointLog, + }), + [extensionPointId, extensionPointLog, pluginContext, isLoadingAppPlugins] + ); + + // Create observable for plugin links that emits incrementally as configure() functions resolve + const observableLinks = useMemo(() => { + if (validationResult) { + // Return empty observable if validation failed + return of([]); + } + return getObservablePluginLinks({ extensionPointId, - pluginContext, - isLoadingAppPlugins, + context, + limitPerPlugin, }); + }, [extensionPointId, context, limitPerPlugin, validationResult]); - if (result) { - return { - isLoading: result.isLoading, - links: [], - }; - } + // Subscribe to the observable - this will rerender as each configure() function resolves + const links = useObservable(observableLinks, []); - const frozenContext = context ? getReadOnlyProxy(context) : {}; - const extensions: PluginExtensionLink[] = []; - const extensionsByPlugin: Record = {}; + // Determine loading state + // We're loading if: + // 1. App plugins are still loading + // 2. Validation is in progress + // Note: Links with async configure() functions will appear incrementally as they resolve, + // so we don't need to track individual loading states - the observable handles this. + const isLoading = (validationResult?.isLoading ?? false) || isLoadingAppPlugins; - for (const addedLink of registryItems ?? []) { - const { pluginId } = addedLink; - const linkLog = pointLog.child({ - path: addedLink.path ?? '', - title: addedLink.title, - description: addedLink.description ?? '', - onClick: typeof addedLink.onClick, - openInNewTab: addedLink.openInNewTab ? 'true' : 'false', - }); - - // Only limit if the `limitPerPlugin` is set - if (limitPerPlugin && extensionsByPlugin[pluginId] >= limitPerPlugin) { - linkLog.debug(`Skipping link extension from plugin "${pluginId}". Reason: Limit reached.`); - continue; - } - - if (extensionsByPlugin[pluginId] === undefined) { - extensionsByPlugin[pluginId] = 0; - } - - // Run the configure() function with the current context, and apply the ovverides - const overrides = getLinkExtensionOverrides(pluginId, addedLink, linkLog, frozenContext); - - // configure() returned an `undefined` -> hide the extension - if (addedLink.configure && overrides === undefined) { - continue; - } - - const path = overrides?.path || addedLink.path; - const extension: PluginExtensionLink = { - id: generateExtensionId(pluginId, extensionPointId, addedLink.title), - type: PluginExtensionTypes.link, - pluginId: pluginId, - onClick: getLinkExtensionOnClick(pluginId, extensionPointId, addedLink, linkLog, frozenContext), - - // Configurable properties - icon: overrides?.icon || addedLink.icon, - title: overrides?.title || addedLink.title, - description: overrides?.description || addedLink.description || '', - path: isString(path) ? getLinkExtensionPathWithTracking(pluginId, path, extensionPointId) : undefined, - category: overrides?.category || addedLink.category, - openInNewTab: overrides?.openInNewTab ?? addedLink.openInNewTab, - }; - - extensions.push(extension); - extensionsByPlugin[pluginId] += 1; - } - - return { - isLoading: false, - links: extensions, - }; - }, [context, extensionPointId, limitPerPlugin, registryItems, pluginContext, isLoadingAppPlugins]); + return { + isLoading, + links: validationResult ? [] : links, + }; } diff --git a/public/app/features/plugins/extensions/utils.tsx b/public/app/features/plugins/extensions/utils.tsx index adb1ed8616d..d5e94fcf60f 100644 --- a/public/app/features/plugins/extensions/utils.tsx +++ b/public/app/features/plugins/extensions/utils.tsx @@ -6,6 +6,7 @@ import { useAsync } from 'react-use'; import { type PluginExtensionEventHelpers, type PluginExtensionOpenModalOptions, + type IconName, isDateTime, dateTime, PluginContextProvider, @@ -15,6 +16,7 @@ import { urlUtil, PluginExtensionPoints, ExtensionInfo, + usePluginContext, } from '@grafana/data'; import { reportInteraction, config, AppPluginConfig } from '@grafana/runtime'; import { Modal } from '@grafana/ui'; @@ -32,7 +34,7 @@ import { RestrictedGrafanaApisProvider } from '../components/restrictedGrafanaAp import { ExtensionErrorBoundary } from './ExtensionErrorBoundary'; import { ExtensionsLog, log as baseLog } from './logs/log'; import { AddedLinkRegistryItem } from './registry/AddedLinksRegistry'; -import { assertIsNotPromise, assertStringProps, isPromise } from './validators'; +import { assertStringProps, isPromise } from './validators'; export function handleErrorsInFn(fn: Function, errorMessagePrefix = '') { return (...args: unknown[]) => { @@ -460,14 +462,24 @@ export function createExtensionSubMenu(extensions: PluginExtensionLink[]): Panel return subMenu; } -export function getLinkExtensionOverrides( - pluginId: string, +export async function getLinkExtensionOverrides( config: AddedLinkRegistryItem, log: ExtensionsLog, context?: object -) { +): Promise< + | { + title: string; + description?: string; + path?: string; + icon?: IconName; + category?: string; + openInNewTab?: boolean; + } + | undefined +> { try { - const overrides = config.configure?.(context); + const configureResult = config.configure?.(context); + const overrides = isPromise(configureResult) ? await configureResult : configureResult; // Hiding the extension if (overrides === undefined) { @@ -485,11 +497,6 @@ export function getLinkExtensionOverrides( ...rest } = overrides; - assertIsNotPromise( - overrides, - `The configure() function for "${config.title}" returned a promise, skipping updates.` - ); - assertStringProps({ title, description }, ['title', 'description']); if (Object.keys(rest).length > 0) { @@ -730,3 +737,19 @@ export const getAppPluginsToPreload = () => { return isNotAwaited(app) && (app.preload || dashboardPanelMenuPluginIds.includes(app.id)); }); }; + +export const createAddedLinkLog = (addedLink: AddedLinkRegistryItem, parentLog: ExtensionsLog) => + parentLog.child({ + path: addedLink.path ?? '', + title: addedLink.title, + description: addedLink.description ?? '', + onClick: typeof addedLink.onClick, + openInNewTab: addedLink.openInNewTab ? 'true' : 'false', + }); + +export const useExtensionPointLog = (extensionPointId: string) => { + const pluginContext = usePluginContext(); + const pluginId = pluginContext?.meta.id ?? ''; + + return React.useMemo(() => baseLog.child({ pluginId, extensionPointId }), [pluginId, extensionPointId]); +}; diff --git a/public/app/features/plugins/extensions/validateExtensionPoint.test.ts b/public/app/features/plugins/extensions/validateExtensionPoint.test.ts index 611db3b8aa5..d45626add9a 100644 --- a/public/app/features/plugins/extensions/validateExtensionPoint.test.ts +++ b/public/app/features/plugins/extensions/validateExtensionPoint.test.ts @@ -1,7 +1,7 @@ import { PluginContextType } from '@grafana/data'; import * as errors from './errors'; -import { ExtensionsLog } from './logs/log'; +import { ExtensionsLog, log } from './logs/log'; import { isGrafanaDevMode } from './utils'; import { validateExtensionPoint } from './validateExtensionPoint'; import * as validators from './validators'; @@ -37,8 +37,11 @@ const setup = ({ }; describe('getExtensionValidationResults', () => { + let extensionPointLog: ExtensionsLog; + beforeEach(() => { jest.clearAllMocks(); + extensionPointLog = log.child({}); }); describe('when calling in production mode', () => { @@ -53,10 +56,10 @@ describe('getExtensionValidationResults', () => { extensionPointId, isLoadingAppPlugins: true, pluginContext, + extensionPointLog, }); expect(actual.result).toEqual({ isLoading: true }); - expect(actual.pointLog).toBeDefined(); }); it('should return null when all validations pass', () => { @@ -66,10 +69,10 @@ describe('getExtensionValidationResults', () => { extensionPointId, isLoadingAppPlugins: false, pluginContext, + extensionPointLog, }); expect(actual.result).toBe(null); - expect(actual.pointLog).toBeDefined(); }); }); @@ -93,10 +96,10 @@ describe('getExtensionValidationResults', () => { extensionPointId, isLoadingAppPlugins: true, pluginContext, + extensionPointLog, }); expect(actual.result).toEqual({ isLoading: false }); - expect(actual.pointLog).toBeDefined(); expect(spyisExtensionPointMetaInfoMissing).not.toHaveBeenCalled(); expect(spyIsExtensionPointIdValid).toHaveBeenCalledTimes(1); expect(spyIsExtensionPointIdValid).toHaveBeenCalledWith({ @@ -117,10 +120,10 @@ describe('getExtensionValidationResults', () => { extensionPointId, isLoadingAppPlugins: true, pluginContext, + extensionPointLog, }); expect(actual.result).toEqual({ isLoading: false }); - expect(actual.pointLog).toBeDefined(); expect(spyisExtensionPointMetaInfoMissing).toHaveBeenCalled(); expect(spyisExtensionPointMetaInfoMissing).toHaveBeenCalledWith(extensionPointId, pluginContext); expect(errorSpy).toHaveBeenCalled(); @@ -137,10 +140,10 @@ describe('getExtensionValidationResults', () => { extensionPointId, isLoadingAppPlugins: false, pluginContext, + extensionPointLog, }); expect(actual.result).toEqual(null); - expect(actual.pointLog).toBeDefined(); expect(spyisExtensionPointMetaInfoMissing).not.toHaveBeenCalled(); expect(errorSpy).not.toHaveBeenCalled(); }); @@ -152,10 +155,10 @@ describe('getExtensionValidationResults', () => { extensionPointId, isLoadingAppPlugins: true, pluginContext, + extensionPointLog, }); expect(actual.result).toEqual({ isLoading: true }); - expect(actual.pointLog).toBeDefined(); }); it('should return null when all validations pass', () => { @@ -165,10 +168,10 @@ describe('getExtensionValidationResults', () => { extensionPointId, isLoadingAppPlugins: false, pluginContext, + extensionPointLog, }); expect(actual.result).toBe(null); - expect(actual.pointLog).toBeDefined(); }); }); }); diff --git a/public/app/features/plugins/extensions/validateExtensionPoint.ts b/public/app/features/plugins/extensions/validateExtensionPoint.ts index aef4ab1fb49..d785dcd21d2 100644 --- a/public/app/features/plugins/extensions/validateExtensionPoint.ts +++ b/public/app/features/plugins/extensions/validateExtensionPoint.ts @@ -1,7 +1,7 @@ import { PluginContextType } from '@grafana/data'; import * as errors from './errors'; -import { ExtensionsLog, log } from './logs/log'; +import { ExtensionsLog } from './logs/log'; import { isGrafanaDevMode } from './utils'; import { isExtensionPointIdValid, isExtensionPointMetaInfoMissing } from './validators'; @@ -9,6 +9,7 @@ interface ValidateExtensionPointOptions { extensionPointId: string; isLoadingAppPlugins: boolean; pluginContext: PluginContextType | null; + extensionPointLog: ExtensionsLog; } interface ValidateExtensionPoint { @@ -17,25 +18,30 @@ interface ValidateExtensionPoint { type ValidateExtensionPointResult = { result: ValidateExtensionPoint | null; - pointLog: ExtensionsLog; }; export function validateExtensionPoint({ extensionPointId, isLoadingAppPlugins, pluginContext, + extensionPointLog, }: ValidateExtensionPointOptions): ValidateExtensionPointResult { const isInsidePlugin = Boolean(pluginContext); const isCoreGrafanaPlugin = pluginContext?.meta.module.startsWith('core:') ?? false; const pluginId = pluginContext?.meta.id ?? ''; - const pointLog = log.child({ pluginId, extensionPointId }); // Don't show extensions if the extension-point id is invalid in DEV mode if ( isGrafanaDevMode() && - !isExtensionPointIdValid({ extensionPointId, pluginId, isInsidePlugin, isCoreGrafanaPlugin, log: pointLog }) + !isExtensionPointIdValid({ + extensionPointId, + pluginId, + isInsidePlugin, + isCoreGrafanaPlugin, + log: extensionPointLog, + }) ) { - return { result: { isLoading: false }, pointLog }; + return { result: { isLoading: false } }; } // Don't show extensions if the extension-point misses meta info (plugin.json) in DEV mode @@ -45,13 +51,13 @@ export function validateExtensionPoint({ pluginContext && isExtensionPointMetaInfoMissing(extensionPointId, pluginContext) ) { - pointLog.error(errors.EXTENSION_POINT_META_INFO_MISSING); - return { result: { isLoading: false }, pointLog }; + extensionPointLog.error(errors.EXTENSION_POINT_META_INFO_MISSING); + return { result: { isLoading: false } }; } if (isLoadingAppPlugins) { - return { result: { isLoading: true }, pointLog }; + return { result: { isLoading: true } }; } - return { result: null, pointLog }; + return { result: null }; }