PluginExtensions: Added onClick to link as a replacement for the command type (#65837)

* Added onClick to the link and made path optional.

* Added type to the import.

* revert(plugin-extensions): put back isPromise utility function

* Minor update to the isPromise function.

* added onClick in the panel menu item.

---------

Co-authored-by: Jack Westbrook <jack.westbrook@gmail.com>
Co-authored-by: Levente Balogh <balogh.levente.hu@gmail.com>
This commit is contained in:
Marcus Andersson
2023-04-03 17:27:55 +02:00
committed by GitHub
parent bd29071a0d
commit 8b738d770c
13 changed files with 191 additions and 24 deletions

View File

@@ -1,4 +1,4 @@
import { PluginExtensionLinkConfig } from '@grafana/data';
import { PluginExtensionLinkConfig, PluginExtensionTypes } from '@grafana/data';
import { createPluginExtensionRegistry } from './createPluginExtensionRegistry';
@@ -10,6 +10,7 @@ describe('createRegistry()', () => {
beforeEach(() => {
link1 = {
type: PluginExtensionTypes.link,
title: 'Link 1',
description: 'Link 1 description',
path: `/a/${pluginId}/declare-incident`,
@@ -17,6 +18,7 @@ describe('createRegistry()', () => {
configure: jest.fn().mockReturnValue({}),
};
link2 = {
type: PluginExtensionTypes.link,
title: 'Link 2',
description: 'Link 2 description',
path: `/a/${pluginId}/declare-incident`,

View File

@@ -12,6 +12,7 @@ describe('getPluginExtensions()', () => {
beforeEach(() => {
link1 = {
type: PluginExtensionTypes.link,
title: 'Link 1',
description: 'Link 1 description',
path: `/a/${pluginId}/declare-incident`,
@@ -19,6 +20,7 @@ describe('getPluginExtensions()', () => {
configure: jest.fn().mockReturnValue({}),
};
link2 = {
type: PluginExtensionTypes.link,
title: 'Link 2',
description: 'Link 2 description',
path: `/a/${pluginId}/declare-incident`,
@@ -183,4 +185,65 @@ describe('getPluginExtensions()', () => {
expect(extensions).toHaveLength(0);
expect(global.console.warn).toHaveBeenCalledTimes(0); // As this is intentional, no warning should be logged
});
test('should pass event, context and helper to extension onClick()', () => {
link2.path = undefined;
link2.onClick = jest.fn().mockImplementation(() => {
throw new Error('Something went wrong!');
});
const context = {};
const registry = createPluginExtensionRegistry([{ pluginId, extensionConfigs: [link2] }]);
const { extensions } = getPluginExtensions({ registry, placement: placement2 });
const [extension] = extensions;
assertPluginExtensionLink(extension);
const event = {} as React.MouseEvent;
extension.onClick?.(event);
expect(link2.onClick).toHaveBeenCalledTimes(1);
expect(link2.onClick).toHaveBeenCalledWith(
event,
expect.objectContaining({
context,
openModal: expect.any(Function),
})
);
});
test('should catch errors in async/promise-based onClick function and log them as warnings', async () => {
link2.path = undefined;
link2.onClick = jest.fn().mockRejectedValue(new Error('testing'));
const registry = createPluginExtensionRegistry([{ pluginId, extensionConfigs: [link2] }]);
const { extensions } = getPluginExtensions({ registry, placement: placement2 });
const [extension] = extensions;
assertPluginExtensionLink(extension);
await extension.onClick?.({} as React.MouseEvent);
expect(extensions).toHaveLength(1);
expect(link2.onClick).toHaveBeenCalledTimes(1);
expect(global.console.warn).toHaveBeenCalledTimes(1);
});
test('should catch errors in the onClick() function and log them as warnings', () => {
link2.path = undefined;
link2.onClick = jest.fn().mockImplementation(() => {
throw new Error('Something went wrong!');
});
const registry = createPluginExtensionRegistry([{ pluginId, extensionConfigs: [link2] }]);
const { extensions } = getPluginExtensions({ registry, placement: placement2 });
const [extension] = extensions;
assertPluginExtensionLink(extension);
extension.onClick?.({} as React.MouseEvent);
expect(link2.onClick).toHaveBeenCalledTimes(1);
expect(global.console.warn).toHaveBeenCalledTimes(1);
expect(global.console.warn).toHaveBeenCalledWith('[Plugin Extensions] Something went wrong!');
});
});

View File

@@ -6,8 +6,8 @@ import {
} from '@grafana/data';
import type { PluginExtensionRegistry } from './types';
import { isPluginExtensionLinkConfig, deepFreeze, logWarning, generateExtensionId } from './utils';
import { assertIsNotPromise, assertLinkPathIsValid, assertStringProps } from './validators';
import { isPluginExtensionLinkConfig, deepFreeze, logWarning, generateExtensionId, getEventHelpers } from './utils';
import { assertIsNotPromise, assertLinkPathIsValid, assertStringProps, isPromise } from './validators';
type GetExtensions = ({
context,
@@ -30,7 +30,6 @@ export const getPluginExtensions: GetExtensions = ({ context, placement, registr
try {
const extensionConfig = registryItem.config;
// LINK extension
if (isPluginExtensionLinkConfig(extensionConfig)) {
const overrides = getLinkExtensionOverrides(registryItem.pluginId, extensionConfig, frozenContext);
@@ -43,6 +42,7 @@ export const getPluginExtensions: GetExtensions = ({ context, placement, registr
id: generateExtensionId(registryItem.pluginId, extensionConfig),
type: PluginExtensionTypes.link,
pluginId: registryItem.pluginId,
onClick: getLinkExtensionOnClick(extensionConfig, frozenContext),
// Configurable properties
title: overrides?.title || extensionConfig.title,
@@ -78,7 +78,7 @@ function getLinkExtensionOverrides(pluginId: string, config: PluginExtensionLink
`The configure() function for "${config.title}" returned a promise, skipping updates.`
);
assertLinkPathIsValid(pluginId, path);
path && assertLinkPathIsValid(pluginId, path);
assertStringProps({ title, description }, ['title', 'description']);
if (Object.keys(rest).length > 0) {
@@ -104,3 +104,32 @@ function getLinkExtensionOverrides(pluginId: string, config: PluginExtensionLink
return undefined;
}
}
function getLinkExtensionOnClick(
config: PluginExtensionLinkConfig,
context?: object
): ((event: React.MouseEvent) => void) | undefined {
const { onClick } = config;
if (!onClick) {
return;
}
return function onClickExtensionLink(event: React.MouseEvent) {
try {
const result = onClick(event, getEventHelpers(context));
if (isPromise(result)) {
result.catch((e) => {
if (e instanceof Error) {
logWarning(e.message);
}
});
}
} catch (error) {
if (error instanceof Error) {
logWarning(error.message);
}
}
};
}

View File

@@ -1,4 +1,4 @@
import { PluginExtensionLinkConfig } from '@grafana/data';
import { PluginExtensionLinkConfig, PluginExtensionTypes } from '@grafana/data';
import { deepFreeze, isPluginExtensionLinkConfig, handleErrorsInFn } from './utils';
@@ -185,6 +185,7 @@ describe('Plugin Extensions / Utils', () => {
test('should return TRUE if the object is a command extension config', () => {
expect(
isPluginExtensionLinkConfig({
type: PluginExtensionTypes.link,
title: 'Title',
description: 'Description',
path: '...',
@@ -196,6 +197,7 @@ describe('Plugin Extensions / Utils', () => {
isPluginExtensionLinkConfig({
title: 'Title',
description: 'Description',
path: '...',
} as PluginExtensionLinkConfig)
).toBe(false);
});

View File

@@ -4,6 +4,7 @@ import {
type PluginExtensionLinkConfig,
type PluginExtensionConfig,
type PluginExtensionEventHelpers,
PluginExtensionTypes,
} from '@grafana/data';
import { Modal } from '@grafana/ui';
import appEvents from 'app/core/app_events';
@@ -16,7 +17,7 @@ export function logWarning(message: string) {
export function isPluginExtensionLinkConfig(
extension: PluginExtensionConfig | undefined
): extension is PluginExtensionLinkConfig {
return typeof extension === 'object' && 'path' in extension;
return typeof extension === 'object' && 'type' in extension && extension['type'] === PluginExtensionTypes.link;
}
export function handleErrorsInFn(fn: Function, errorMessagePrefix = '') {
@@ -32,12 +33,12 @@ export function handleErrorsInFn(fn: Function, errorMessagePrefix = '') {
}
// Event helpers are designed to make it easier to trigger "core actions" from an extension event handler, e.g. opening a modal or showing a notification.
export function getEventHelpers(): PluginExtensionEventHelpers {
export function getEventHelpers(context?: Readonly<object>): PluginExtensionEventHelpers {
const openModal: PluginExtensionEventHelpers['openModal'] = ({ title, body }) => {
appEvents.publish(new ShowModalReactEvent({ component: getModalWrapper({ title, body }) }));
};
return { openModal };
return { openModal, context };
}
export type ModalWrapperProps = {

View File

@@ -81,6 +81,7 @@ describe('Plugin Extension Validators', () => {
it('should throw an error if the placement does not have the right prefix', () => {
expect(() => {
assertPlacementIsValid({
type: PluginExtensionTypes.link,
title: 'Title',
description: 'Description',
path: '...',
@@ -92,6 +93,7 @@ describe('Plugin Extension Validators', () => {
it('should NOT throw an error if the placement is correct', () => {
expect(() => {
assertPlacementIsValid({
type: PluginExtensionTypes.link,
title: 'Title',
description: 'Description',
path: '...',
@@ -99,6 +101,7 @@ describe('Plugin Extension Validators', () => {
});
assertPlacementIsValid({
type: PluginExtensionTypes.link,
title: 'Title',
description: 'Description',
path: '...',
@@ -203,18 +206,20 @@ describe('Plugin Extension Validators', () => {
describe('isPluginExtensionConfigValid()', () => {
it('should return TRUE if the plugin extension configuration is valid', () => {
const pluginId = 'my-super-plugin';
// Command
expect(
isPluginExtensionConfigValid(pluginId, {
type: PluginExtensionTypes.link,
title: 'Title',
description: 'Description',
placement: 'grafana/some-page/some-placement',
onClick: jest.fn(),
} as PluginExtensionLinkConfig)
).toBe(true);
// Link
expect(
isPluginExtensionConfigValid(pluginId, {
type: PluginExtensionTypes.link,
title: 'Title',
description: 'Description',
placement: 'grafana/some-page/some-placement',
@@ -231,6 +236,7 @@ describe('Plugin Extension Validators', () => {
// Link (wrong path)
expect(
isPluginExtensionConfigValid(pluginId, {
type: PluginExtensionTypes.link,
title: 'Title',
description: 'Description',
placement: 'grafana/some-page/some-placement',
@@ -238,9 +244,20 @@ describe('Plugin Extension Validators', () => {
} as PluginExtensionLinkConfig)
).toBe(false);
// Link (no path and no onClick)
expect(
isPluginExtensionConfigValid(pluginId, {
type: PluginExtensionTypes.link,
title: 'Title',
description: 'Description',
placement: 'grafana/some-page/some-placement',
} as PluginExtensionLinkConfig)
).toBe(false);
// Link (missing title)
expect(
isPluginExtensionConfigValid(pluginId, {
type: PluginExtensionTypes.link,
title: '',
description: 'Description',
placement: 'grafana/some-page/some-placement',

View File

@@ -77,10 +77,6 @@ export function isStringPropValid(prop: unknown) {
return typeof prop === 'string' && prop.length > 0;
}
export function isPromise(value: unknown) {
return value instanceof Promise || (typeof value === 'object' && value !== null && 'then' in value);
}
export function isPluginExtensionConfigValid(pluginId: string, extension: PluginExtensionLinkConfig): boolean {
try {
assertStringProps(extension, ['title', 'description', 'placement']);
@@ -88,7 +84,14 @@ export function isPluginExtensionConfigValid(pluginId: string, extension: Plugin
assertConfigureIsValid(extension);
if (isPluginExtensionLinkConfig(extension)) {
assertLinkPathIsValid(pluginId, extension.path);
if (!extension.path && !extension.onClick) {
logWarning(`Invalid extension "${extension.title}". Either "path" or "onClick" is required.`);
return false;
}
if (extension.path) {
assertLinkPathIsValid(pluginId, extension.path);
}
}
return true;
@@ -100,3 +103,9 @@ export function isPluginExtensionConfigValid(pluginId: string, extension: Plugin
return false;
}
}
export function isPromise(value: unknown): value is Promise<unknown> {
return (
value instanceof Promise || (typeof value === 'object' && value !== null && 'then' in value && 'catch' in value)
);
}