From 5b1d99b8c4479eeb0d6307538ffad4392b7e1c98 Mon Sep 17 00:00:00 2001 From: Andrej Ocenas Date: Tue, 22 Oct 2024 11:46:29 +0200 Subject: [PATCH] Sidecar: Remove extension helpers and use the sidecar service directly (#94979) --- .../src/types/pluginExtensions.ts | 23 +-------- .../SidecarService_EXPERIMENTAL.test.ts | 47 +++++++++++++++++++ pkg/storage/unified/resource/index_mapping.go | 2 +- .../getExploreExtensionConfigs.test.tsx | 4 +- .../extensions/getPluginExtensions.test.tsx | 5 +- .../extensions/usePluginExtensions.tsx | 13 +---- .../app/features/plugins/extensions/utils.tsx | 19 +------- .../plugins/extensions/validators.test.tsx | 2 +- 8 files changed, 58 insertions(+), 57 deletions(-) create mode 100644 packages/grafana-runtime/src/services/SidecarService_EXPERIMENTAL.test.ts diff --git a/packages/grafana-data/src/types/pluginExtensions.ts b/packages/grafana-data/src/types/pluginExtensions.ts index d9e53bb1a7b..ae5cc9e1177 100644 --- a/packages/grafana-data/src/types/pluginExtensions.ts +++ b/packages/grafana-data/src/types/pluginExtensions.ts @@ -75,10 +75,7 @@ export type PluginExtensionAddedComponentConfig = PluginExtensionCon component: React.ComponentType; }; -export type PluginAddedLinksConfigureFunc = ( - context: Readonly | undefined, - helpers: PluginExtensionHelpers -) => +export type PluginAddedLinksConfigureFunc = (context: Readonly | undefined) => | Partial<{ title: string; description: string; @@ -145,27 +142,11 @@ export type PluginExtensionOpenModalOptions = { height?: string | number; }; -type PluginExtensionHelpers = { - /** Checks if the app plugin (that registers the extension) is currently visible (either in the main view or in the side view) - * @experimental - */ - isAppOpened: () => boolean; -}; - export type PluginExtensionEventHelpers = { context?: Readonly; // Opens a modal dialog and renders the provided React component inside it openModal: (options: PluginExtensionOpenModalOptions) => void; - - /** Opens the app plugin (that registers the extensions) in a side view - * @experimental - */ - openAppInSideview: (context?: unknown) => void; - /** Closes the side view for the app plugin (that registers the extensions) in case it was open - * @experimental - */ - closeAppInSideview: () => void; -} & PluginExtensionHelpers; +}; // Extension Points & Contexts // -------------------------------------------------------- diff --git a/packages/grafana-runtime/src/services/SidecarService_EXPERIMENTAL.test.ts b/packages/grafana-runtime/src/services/SidecarService_EXPERIMENTAL.test.ts new file mode 100644 index 00000000000..a58d5483fd3 --- /dev/null +++ b/packages/grafana-runtime/src/services/SidecarService_EXPERIMENTAL.test.ts @@ -0,0 +1,47 @@ +import { config } from '../config'; + +import { SidecarService_EXPERIMENTAL } from './SidecarService_EXPERIMENTAL'; + +describe('SidecarService_EXPERIMENTAL', () => { + beforeEach(() => { + config.featureToggles.appSidecar = true; + }); + afterEach(() => { + config.featureToggles.appSidecar = undefined; + }); + + it('has the correct state after opening and closing an app', () => { + const sidecarService = new SidecarService_EXPERIMENTAL({}); + sidecarService.openApp('pluginId', { filter: 'test' }); + + expect(sidecarService.activePluginId).toBe('pluginId'); + expect(sidecarService.initialContext).toMatchObject({ filter: 'test' }); + + sidecarService.closeApp('pluginId'); + expect(sidecarService.activePluginId).toBe(undefined); + expect(sidecarService.initialContext).toBe(undefined); + }); + + it('reports correct opened state', () => { + const sidecarService = new SidecarService_EXPERIMENTAL({}); + expect(sidecarService.isAppOpened('pluginId')).toBe(false); + + sidecarService.openApp('pluginId'); + + expect(sidecarService.isAppOpened('pluginId')).toBe(true); + + sidecarService.closeApp('pluginId'); + + expect(sidecarService.isAppOpened('pluginId')).toBe(false); + }); + + it('does not close app that is not opened', () => { + const sidecarService = new SidecarService_EXPERIMENTAL({}); + sidecarService.openApp('pluginId'); + + sidecarService.closeApp('foobar'); + + expect(sidecarService.isAppOpened('pluginId')).toBe(true); + expect(sidecarService.activePluginId).toBe('pluginId'); + }); +}); diff --git a/pkg/storage/unified/resource/index_mapping.go b/pkg/storage/unified/resource/index_mapping.go index 16fcc8c80df..fccd2833083 100644 --- a/pkg/storage/unified/resource/index_mapping.go +++ b/pkg/storage/unified/resource/index_mapping.go @@ -97,7 +97,7 @@ func createIndexMappings() *mapping.IndexMappingImpl { indexMapping.TypeField = "Kind" // for all kinds, create their index mappings - for k, _ := range getSpecObjectMappings() { + for k := range getSpecObjectMappings() { objMapping := createIndexMappingForKind(k) indexMapping.AddDocumentMapping(k, objMapping) } diff --git a/public/app/features/explore/extensions/getExploreExtensionConfigs.test.tsx b/public/app/features/explore/extensions/getExploreExtensionConfigs.test.tsx index 869a00d8006..c20ca7a9705 100644 --- a/public/app/features/explore/extensions/getExploreExtensionConfigs.test.tsx +++ b/public/app/features/explore/extensions/getExploreExtensionConfigs.test.tsx @@ -43,7 +43,7 @@ describe('getExploreExtensionConfigs', () => { const extensions = getExploreExtensionConfigs(); const [extension] = extensions; - expect(extension?.configure?.(undefined, { isAppOpened: () => false })).toBeUndefined(); + expect(extension?.configure?.(undefined)).toBeUndefined(); }); it('should return empty object if sufficient permissions', () => { @@ -52,7 +52,7 @@ describe('getExploreExtensionConfigs', () => { const extensions = getExploreExtensionConfigs(); const [extension] = extensions; - expect(extension?.configure?.(undefined, { isAppOpened: () => false })).toEqual({}); + expect(extension?.configure?.(undefined)).toEqual({}); }); }); }); diff --git a/public/app/features/plugins/extensions/getPluginExtensions.test.tsx b/public/app/features/plugins/extensions/getPluginExtensions.test.tsx index 925b0cfbd7b..14bf2cd9ba6 100644 --- a/public/app/features/plugins/extensions/getPluginExtensions.test.tsx +++ b/public/app/features/plugins/extensions/getPluginExtensions.test.tsx @@ -186,10 +186,7 @@ describe('getPluginExtensions()', () => { getPluginExtensions({ ...registries, context, extensionPointId: extensionPoint2 }); expect(link2.configure).toHaveBeenCalledTimes(1); - expect(link2.configure).toHaveBeenCalledWith( - context, - expect.objectContaining({ isAppOpened: expect.any(Function) }) - ); + expect(link2.configure).toHaveBeenCalledWith(context); }); test('should be possible to update the basic properties with the configure() function', async () => { diff --git a/public/app/features/plugins/extensions/usePluginExtensions.tsx b/public/app/features/plugins/extensions/usePluginExtensions.tsx index 2b89667aefb..bb041790b3e 100644 --- a/public/app/features/plugins/extensions/usePluginExtensions.tsx +++ b/public/app/features/plugins/extensions/usePluginExtensions.tsx @@ -2,7 +2,7 @@ import { useMemo } from 'react'; import { useObservable } from 'react-use'; import { PluginExtension, usePluginContext } from '@grafana/data'; -import { GetPluginExtensionsOptions, UsePluginExtensionsResult, useSidecar_EXPERIMENTAL } from '@grafana/runtime'; +import { GetPluginExtensionsOptions, UsePluginExtensionsResult } from '@grafana/runtime'; import { getPluginExtensions } from './getPluginExtensions'; import { log } from './logs/log'; @@ -18,7 +18,6 @@ export function createUsePluginExtensions(registries: PluginExtensionRegistries) const pluginContext = usePluginContext(); const addedComponentsRegistry = useObservable(observableAddedComponentsRegistry); const addedLinksRegistry = useObservable(observableAddedLinksRegistry); - const { activePluginId } = useSidecar_EXPERIMENTAL(); const { extensionPointId, context, limitPerPlugin } = options; const { extensions } = useMemo(() => { @@ -65,15 +64,7 @@ export function createUsePluginExtensions(registries: PluginExtensionRegistries) // options object so we are checking it's simple value attributes. // The context though still has to be memoized though and not mutated. // eslint-disable-next-line react-hooks/exhaustive-deps -- TODO: refactor `getPluginExtensions` to accept service dependencies as arguments instead of relying on the sidecar singleton under the hood - }, [ - addedLinksRegistry, - addedComponentsRegistry, - extensionPointId, - context, - limitPerPlugin, - activePluginId, - pluginContext, - ]); + }, [addedLinksRegistry, addedComponentsRegistry, extensionPointId, context, limitPerPlugin, pluginContext]); return { extensions, isLoading: false }; }; diff --git a/public/app/features/plugins/extensions/utils.tsx b/public/app/features/plugins/extensions/utils.tsx index 4489faca6c0..bf62b634b01 100644 --- a/public/app/features/plugins/extensions/utils.tsx +++ b/public/app/features/plugins/extensions/utils.tsx @@ -20,12 +20,7 @@ import { PluginExtensionExposedComponentConfig, PluginExtensionAddedComponentConfig, } from '@grafana/data'; -import { - reportInteraction, - config, - // TODO: instead of depending on the service as a singleton, inject it as an argument from the React context - sidecarServiceSingleton_EXPERIMENTAL, -} from '@grafana/runtime'; +import { reportInteraction, config } from '@grafana/runtime'; import { Modal } from '@grafana/ui'; import appEvents from 'app/core/app_events'; import { getPluginSettings } from 'app/features/plugins/pluginSettings'; @@ -312,7 +307,7 @@ export function getLinkExtensionOverrides( context?: object ) { try { - const overrides = config.configure?.(context, { isAppOpened: () => isAppOpened(pluginId) }); + const overrides = config.configure?.(context); // Hiding the extension if (overrides === undefined) { @@ -390,9 +385,6 @@ export function getLinkExtensionOnClick( const helpers: PluginExtensionEventHelpers = { context, openModal: createOpenModalFunction(pluginId), - isAppOpened: () => isAppOpened(pluginId), - openAppInSideview: (context?: unknown) => openAppInSideview(pluginId, context), - closeAppInSideview: () => closeAppInSideview(pluginId), }; log.debug(`onClick '${config.title}' at '${extensionPointId}'`); @@ -429,13 +421,6 @@ export function getLinkExtensionPathWithTracking(pluginId: string, path: string, ); } -export const openAppInSideview = (pluginId: string, context?: unknown) => - sidecarServiceSingleton_EXPERIMENTAL.openApp(pluginId, context); - -export const closeAppInSideview = (pluginId: string) => sidecarServiceSingleton_EXPERIMENTAL.closeApp(pluginId); - -export const isAppOpened = (pluginId: string) => sidecarServiceSingleton_EXPERIMENTAL.isAppOpened(pluginId); - // Comes from the `app_mode` setting in the Grafana config (defaults to "development") // Can be set with the `GF_DEFAULT_APP_MODE` environment variable export const isGrafanaDevMode = () => config.buildInfo.env === 'development'; diff --git a/public/app/features/plugins/extensions/validators.test.tsx b/public/app/features/plugins/extensions/validators.test.tsx index 2ce296c7210..3e968018e45 100644 --- a/public/app/features/plugins/extensions/validators.test.tsx +++ b/public/app/features/plugins/extensions/validators.test.tsx @@ -71,7 +71,7 @@ describe('Plugin Extension Validators', () => { title: 'Title', description: 'Description', targets: 'grafana/some-page/extension-point-a', - configure: (_, {}) => {}, + configure: () => {}, } as PluginExtensionAddedLinkConfig); }).not.toThrowError(); });