From 8148f0c3bbdacca6996cb460bc3b1bbab9d57c61 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Mon, 11 Nov 2024 12:22:57 +0100 Subject: [PATCH] Plugin extensions: Make description optional (#95555) make description optional --- .../src/types/pluginExtensions.ts | 2 +- .../plugins/extensions/getPluginExtensions.ts | 8 +-- .../registry/AddedComponentsRegistry.test.ts | 23 -------- .../registry/AddedComponentsRegistry.ts | 9 +--- .../extensions/registry/AddedLinksRegistry.ts | 9 +--- .../ExportedComponentsRegistry.test.ts | 23 -------- .../registry/ExposedComponentsRegistry.ts | 9 +--- .../plugins/extensions/usePluginComponent.tsx | 2 +- .../plugins/extensions/usePluginLinks.tsx | 4 +- .../plugins/extensions/utils.test.tsx | 54 ------------------- .../app/features/plugins/extensions/utils.tsx | 24 --------- 11 files changed, 13 insertions(+), 154 deletions(-) diff --git a/packages/grafana-data/src/types/pluginExtensions.ts b/packages/grafana-data/src/types/pluginExtensions.ts index ae5cc9e1177..fcbbecf5f41 100644 --- a/packages/grafana-data/src/types/pluginExtensions.ts +++ b/packages/grafana-data/src/types/pluginExtensions.ts @@ -50,7 +50,7 @@ type PluginExtensionConfigBase = { /** * A short description */ - description: string; + description?: string; }; export type PluginExtensionAddedComponentConfig = PluginExtensionConfigBase & { diff --git a/public/app/features/plugins/extensions/getPluginExtensions.ts b/public/app/features/plugins/extensions/getPluginExtensions.ts index acaaf8d97ac..a47292c8497 100644 --- a/public/app/features/plugins/extensions/getPluginExtensions.ts +++ b/public/app/features/plugins/extensions/getPluginExtensions.ts @@ -83,7 +83,7 @@ export const getPluginExtensions: GetExtensions = ({ extensionPointId, path: addedLink.path ?? '', title: addedLink.title, - description: addedLink.description, + description: addedLink.description ?? '', onClick: typeof addedLink.onClick, }); // Run the configure() function with the current context, and apply the ovverides @@ -104,7 +104,7 @@ export const getPluginExtensions: GetExtensions = ({ // Configurable properties icon: overrides?.icon || addedLink.icon, title: overrides?.title || addedLink.title, - description: overrides?.description || addedLink.description, + description: overrides?.description || addedLink.description || '', path: isString(path) ? getLinkExtensionPathWithTracking(pluginId, path, extensionPointId) : undefined, category: overrides?.category || addedLink.category, }; @@ -134,7 +134,7 @@ export const getPluginExtensions: GetExtensions = ({ const componentLog = log.child({ title: addedComponent.title, - description: addedComponent.description, + description: addedComponent.description ?? '', pluginId: addedComponent.pluginId, }); @@ -143,7 +143,7 @@ export const getPluginExtensions: GetExtensions = ({ type: PluginExtensionTypes.component, pluginId: addedComponent.pluginId, title: addedComponent.title, - description: addedComponent.description, + description: addedComponent.description ?? '', component: wrapWithPluginContext(addedComponent.pluginId, addedComponent.component, componentLog), }; diff --git a/public/app/features/plugins/extensions/registry/AddedComponentsRegistry.test.ts b/public/app/features/plugins/extensions/registry/AddedComponentsRegistry.test.ts index fcf3d4616bb..d04a137e52d 100644 --- a/public/app/features/plugins/extensions/registry/AddedComponentsRegistry.test.ts +++ b/public/app/features/plugins/extensions/registry/AddedComponentsRegistry.test.ts @@ -380,29 +380,6 @@ describe('AddedComponentsRegistry', () => { expect(Object.keys(currentState)).toHaveLength(1); }); - it('should not register component when description is missing', async () => { - const registry = new AddedComponentsRegistry(); - const extensionPointId = 'grafana/alerting/home'; - - registry.register({ - pluginId, - configs: [ - { - title: 'Component 1 title', - description: '', - targets: [extensionPointId], - component: () => React.createElement('div', null, 'Hello World1'), - }, - ], - }); - - expect(log.error).toHaveBeenCalledWith( - "Could not register added component with title 'Component 1 title'. Reason: Description is missing." - ); - const currentState = await registry.getState(); - expect(Object.keys(currentState)).toHaveLength(0); - }); - it('should not register component when title is missing', async () => { const registry = new AddedComponentsRegistry(); const extensionPointId = 'grafana/alerting/home'; diff --git a/public/app/features/plugins/extensions/registry/AddedComponentsRegistry.ts b/public/app/features/plugins/extensions/registry/AddedComponentsRegistry.ts index 7cedc79d4bf..f46d233f987 100644 --- a/public/app/features/plugins/extensions/registry/AddedComponentsRegistry.ts +++ b/public/app/features/plugins/extensions/registry/AddedComponentsRegistry.ts @@ -10,7 +10,7 @@ import { PluginExtensionConfigs, Registry, RegistryType } from './Registry'; export type AddedComponentRegistryItem = { pluginId: string; title: string; - description: string; + description?: string; component: React.ComponentType; }; @@ -45,13 +45,6 @@ export class AddedComponentsRegistry extends Registry< continue; } - if (!config.description) { - configLog.error( - `Could not register added component with title '${config.title}'. Reason: Description is missing.` - ); - continue; - } - if ( pluginId !== 'grafana' && isGrafanaDevMode() && diff --git a/public/app/features/plugins/extensions/registry/AddedLinksRegistry.ts b/public/app/features/plugins/extensions/registry/AddedLinksRegistry.ts index 0e978b993c6..d7e66acf798 100644 --- a/public/app/features/plugins/extensions/registry/AddedLinksRegistry.ts +++ b/public/app/features/plugins/extensions/registry/AddedLinksRegistry.ts @@ -17,7 +17,7 @@ export type AddedLinkRegistryItem = { pluginId: string; extensionPointId: string; title: string; - description: string; + description?: string; path?: string; onClick?: (event: React.MouseEvent | undefined, helpers: PluginExtensionEventHelpers) => void; configure?: PluginAddedLinksConfigureFunc; @@ -45,7 +45,7 @@ export class AddedLinksRegistry extends Registry { expect(Object.keys(currentState)).toHaveLength(1); }); - it('should not register component when description is missing', async () => { - const registry = new ExposedComponentsRegistry(); - - registry.register({ - pluginId: 'grafana-basic-app', - configs: [ - { - id: 'grafana-basic-app/hello-world/v1', - title: 'not important', - description: '', - component: () => React.createElement('div', null, 'Hello World1'), - }, - ], - }); - - expect(log.error).toHaveBeenCalledWith( - "Could not register exposed component with id 'grafana-basic-app/hello-world/v1'. Reason: Description is missing." - ); - - const currentState = await registry.getState(); - expect(Object.keys(currentState)).toHaveLength(0); - }); - it('should not register component when title is missing', async () => { const registry = new ExposedComponentsRegistry(); diff --git a/public/app/features/plugins/extensions/registry/ExposedComponentsRegistry.ts b/public/app/features/plugins/extensions/registry/ExposedComponentsRegistry.ts index 85f3776f5b9..6f4cee8e6c1 100644 --- a/public/app/features/plugins/extensions/registry/ExposedComponentsRegistry.ts +++ b/public/app/features/plugins/extensions/registry/ExposedComponentsRegistry.ts @@ -10,7 +10,7 @@ import { Registry, RegistryType, PluginExtensionConfigs } from './Registry'; export type ExposedComponentRegistryItem = { pluginId: string; title: string; - description: string; + description?: string; component: React.ComponentType; }; @@ -39,7 +39,7 @@ export class ExposedComponentsRegistry extends Registry< const { id, description, title } = config; const pointIdLog = this.logger.child({ extensionPointId: id, - description, + description: description ?? '', title, pluginId, }); @@ -69,11 +69,6 @@ export class ExposedComponentsRegistry extends Registry< continue; } - if (!description) { - pointIdLog.error(`Could not register exposed component with id '${id}'. Reason: Description is missing.`); - continue; - } - if ( pluginId !== 'grafana' && isGrafanaDevMode() && diff --git a/public/app/features/plugins/extensions/usePluginComponent.tsx b/public/app/features/plugins/extensions/usePluginComponent.tsx index a32b6aa974c..c93ff04b10a 100644 --- a/public/app/features/plugins/extensions/usePluginComponent.tsx +++ b/public/app/features/plugins/extensions/usePluginComponent.tsx @@ -29,7 +29,7 @@ export function usePluginComponent(id: string): UsePl const registryItem = registryState[id]; const componentLog = log.child({ title: registryItem.title, - description: registryItem.description, + description: registryItem.description ?? '', pluginId: registryItem.pluginId, }); diff --git a/public/app/features/plugins/extensions/usePluginLinks.tsx b/public/app/features/plugins/extensions/usePluginLinks.tsx index bd9bba9bcc9..8a50b558e0e 100644 --- a/public/app/features/plugins/extensions/usePluginLinks.tsx +++ b/public/app/features/plugins/extensions/usePluginLinks.tsx @@ -85,7 +85,7 @@ export function usePluginLinks({ const linkLog = pointLog.child({ path: addedLink.path ?? '', title: addedLink.title, - description: addedLink.description, + description: addedLink.description ?? '', onClick: typeof addedLink.onClick, }); // Run the configure() function with the current context, and apply the ovverides @@ -106,7 +106,7 @@ export function usePluginLinks({ // Configurable properties icon: overrides?.icon || addedLink.icon, title: overrides?.title || addedLink.title, - description: overrides?.description || addedLink.description, + description: overrides?.description || addedLink.description || '', path: isString(path) ? getLinkExtensionPathWithTracking(pluginId, path, extensionPointId) : undefined, category: overrides?.category || addedLink.category, }; diff --git a/public/app/features/plugins/extensions/utils.test.tsx b/public/app/features/plugins/extensions/utils.test.tsx index c090e2ac232..6da25110752 100644 --- a/public/app/features/plugins/extensions/utils.test.tsx +++ b/public/app/features/plugins/extensions/utils.test.tsx @@ -554,24 +554,6 @@ describe('Plugin Extensions / Utils', () => { expect(log.warning).toHaveBeenCalledTimes(1); expect(jest.mocked(log.warning).mock.calls[0][0]).toMatch('"targets" don\'t match'); }); - - it('should return TRUE and log a warning if the "description" does not match', () => { - const log = createLogMock(); - config.apps[pluginId].extensions.addedLinks.push(extensionConfig); - - const returnValue = isAddedLinkMetaInfoMissing( - pluginId, - { - ...extensionConfig, - description: 'Link description UPDATED', - }, - log - ); - - expect(returnValue).toBe(true); - expect(log.warning).toHaveBeenCalledTimes(1); - expect(jest.mocked(log.warning).mock.calls[0][0]).toMatch('"description" doesn\'t match'); - }); }); describe('isAddedComponentMetaInfoMissing()', () => { @@ -667,24 +649,6 @@ describe('Plugin Extensions / Utils', () => { expect(log.warning).toHaveBeenCalledTimes(1); expect(jest.mocked(log.warning).mock.calls[0][0]).toMatch('"targets" don\'t match'); }); - - it('should return TRUE and log a warning if the "description" does not match', () => { - const log = createLogMock(); - config.apps[pluginId].extensions.addedComponents.push(extensionConfig); - - const returnValue = isAddedComponentMetaInfoMissing( - pluginId, - { - ...extensionConfig, - description: 'UPDATED', - }, - log - ); - - expect(returnValue).toBe(true); - expect(log.warning).toHaveBeenCalledTimes(1); - expect(jest.mocked(log.warning).mock.calls[0][0]).toMatch('"description" doesn\'t match'); - }); }); describe('isExposedComponentMetaInfoMissing()', () => { @@ -780,24 +744,6 @@ describe('Plugin Extensions / Utils', () => { expect(log.warning).toHaveBeenCalledTimes(1); expect(jest.mocked(log.warning).mock.calls[0][0]).toMatch('"title" doesn\'t match'); }); - - it('should return TRUE and log a warning if the "description" does not match', () => { - const log = createLogMock(); - config.apps[pluginId].extensions.exposedComponents.push(exposedComponentConfig); - - const returnValue = isExposedComponentMetaInfoMissing( - pluginId, - { - ...exposedComponentConfig, - description: 'UPDATED', - }, - log - ); - - expect(returnValue).toBe(true); - expect(log.warning).toHaveBeenCalledTimes(1); - expect(jest.mocked(log.warning).mock.calls[0][0]).toMatch('"description" doesn\'t match'); - }); }); describe('isExposedComponentDependencyMissing()', () => { diff --git a/public/app/features/plugins/extensions/utils.tsx b/public/app/features/plugins/extensions/utils.tsx index bf62b634b01..6ce6b18bbfe 100644 --- a/public/app/features/plugins/extensions/utils.tsx +++ b/public/app/features/plugins/extensions/utils.tsx @@ -490,14 +490,6 @@ export const isAddedLinkMetaInfoMissing = ( return true; } - if (pluginJsonMetaInfo.description !== metaInfo.description) { - log.warning( - `${logPrefix} the "description" doesn't match with one in the plugin.json under "extensions.addedLinks[]".` - ); - - return true; - } - return false; }; @@ -530,14 +522,6 @@ export const isAddedComponentMetaInfoMissing = ( return true; } - if (pluginJsonMetaInfo.description !== metaInfo.description) { - log.warning( - `${logPrefix} the "description" doesn't match with one in the plugin.json under "extensions.addedComponents[]".` - ); - - return true; - } - return false; }; @@ -569,13 +553,5 @@ export const isExposedComponentMetaInfoMissing = ( return true; } - if (pluginJsonMetaInfo.description !== metaInfo.description) { - log.warning( - `${logPrefix} the "description" doesn't match with one in the plugin.json under "extensions.exposedComponents[]".` - ); - - return true; - } - return false; };