Skip to content

Commit

Permalink
Plugins: Renamed parts of the UI extension APIs (#63070)
Browse files Browse the repository at this point in the history
* Renamed target -> id and href -> path after feedback.

* fixed type issues in test page.

* chore(pluginschemajson): update extensions props target -> id

* this is the final.

* fixed typings...again...

---------

Co-authored-by: Jack Westbrook <jack.westbrook@gmail.com>
  • Loading branch information
mckn and jackw committed Feb 8, 2023
1 parent b88206d commit f46f8bd
Show file tree
Hide file tree
Showing 13 changed files with 46 additions and 46 deletions.
4 changes: 2 additions & 2 deletions docs/sources/developers/plugins/plugin.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@
"type": "object",
"description": "Expose a page link that can be used by Grafana core or other plugins to navigate users to the plugin",
"additionalProperties": false,
"required": ["type", "title", "target", "path"],
"required": ["type", "title", "placement", "path"],
"properties": {
"type": {
"type": "string",
Expand All @@ -491,7 +491,7 @@
"minLength": 3,
"maxLength": 22
},
"target": {
"placement": {
"type": "string",
"pattern": "^(plugins|grafana)/[a-z-/0-9]*$"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/grafana-runtime/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export enum PluginExtensionTypes {
}

export type PluginsExtensionLinkConfig = {
target: string;
placement: string;
type: PluginExtensionTypes.link;
title: string;
description: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ describe('getPluginExtensions', () => {
type: 'link',
title: 'Declare incident',
description: 'Declaring an incident in the app',
href: `/a/${pluginId}/declare-incident`,
path: `/a/${pluginId}/declare-incident`,
key: 1,
},
],
Expand All @@ -22,26 +22,26 @@ describe('getPluginExtensions', () => {

it('should return a collection of extensions to the plugin', () => {
const { extensions, error } = getPluginExtensions({
target: `plugins/${pluginId}/${linkId}`,
placement: `plugins/${pluginId}/${linkId}`,
});

expect(extensions[0].href).toBe(`/a/${pluginId}/declare-incident`);
expect(extensions[0].path).toBe(`/a/${pluginId}/declare-incident`);
expect(error).toBeUndefined();
});

it('should return a description for the requested link', () => {
const { extensions, error } = getPluginExtensions({
target: `plugins/${pluginId}/${linkId}`,
placement: `plugins/${pluginId}/${linkId}`,
});

expect(extensions[0].href).toBe(`/a/${pluginId}/declare-incident`);
expect(extensions[0].path).toBe(`/a/${pluginId}/declare-incident`);
expect(extensions[0].description).toBe('Declaring an incident in the app');
expect(error).toBeUndefined();
});

it('should return an empty array when no links can be found', () => {
const { extensions, error } = getPluginExtensions({
target: `an-unknown-app/${linkId}`,
placement: `an-unknown-app/${linkId}`,
});

expect(extensions.length).toBe(0);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { getPluginsExtensionRegistry, PluginsExtension } from './registry';

export type GetPluginExtensionsOptions = {
target: string;
placement: string;
};

export type PluginExtensionsResult = {
Expand All @@ -10,23 +10,23 @@ export type PluginExtensionsResult = {
};

export class PluginExtensionsMissingError extends Error {
readonly target: string;
readonly placement: string;

constructor(target: string) {
super(`Could not find extensions for '${target}'`);
this.target = target;
constructor(placement: string) {
super(`Could not find extensions for '${placement}'`);
this.placement = placement;
this.name = PluginExtensionsMissingError.name;
}
}

export function getPluginExtensions({ target }: GetPluginExtensionsOptions): PluginExtensionsResult {
export function getPluginExtensions({ placement }: GetPluginExtensionsOptions): PluginExtensionsResult {
const registry = getPluginsExtensionRegistry();
const extensions = registry[target];
const extensions = registry[placement];

if (!Array.isArray(extensions)) {
return {
extensions: [],
error: new PluginExtensionsMissingError(target),
error: new PluginExtensionsMissingError(placement),
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ export type PluginsExtensionLink = {
type: 'link';
title: string;
description: string;
href: string;
path: string;
key: number;
};

Expand Down
6 changes: 3 additions & 3 deletions pkg/api/frontendsettings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ func TestHTTPServer_GetFrontendSettings_apps(t *testing.T) {
return &plugins.FakePluginStore{
PluginList: newPlugins("test-app", []*plugindef.ExtensionsLink{
{
Target: "core/home/menu",
Placement: "core/home/menu",
Type: plugindef.ExtensionsLinkTypeLink,
Title: "Title",
Description: "Home route of app",
Expand All @@ -267,7 +267,7 @@ func TestHTTPServer_GetFrontendSettings_apps(t *testing.T) {
Version: "0.5.0",
Extensions: []*plugindef.ExtensionsLink{
{
Target: "core/home/menu",
Placement: "core/home/menu",
Type: plugindef.ExtensionsLinkTypeLink,
Title: "Title",
Description: "Home route of app",
Expand All @@ -284,7 +284,7 @@ func TestHTTPServer_GetFrontendSettings_apps(t *testing.T) {
return &plugins.FakePluginStore{
PluginList: newPlugins("test-app", []*plugindef.ExtensionsLink{
{
Target: "core/home/menu",
Placement: "core/home/menu",
Type: plugindef.ExtensionsLinkTypeLink,
Title: "Title",
Description: "Home route of app",
Expand Down
4 changes: 2 additions & 2 deletions pkg/plugins/manager/loader/loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -502,14 +502,14 @@ func TestLoader_Load(t *testing.T) {
},
Extensions: []*plugindef.ExtensionsLink{
{
Target: "plugins/grafana-slo-app/slo-breach",
Placement: "plugins/grafana-slo-app/slo-breach",
Title: "Declare incident",
Type: plugindef.ExtensionsLinkTypeLink,
Description: "Declares a new incident",
Path: "/incidents/declare",
},
{
Target: "plugins/grafana-slo-app/slo-breach",
Placement: "plugins/grafana-slo-app/slo-breach",
Title: "Declare incident",
Type: plugindef.ExtensionsLinkTypeLink,
Description: "Declares a new incident (path without backslash)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@
],
"extensions": [
{
"target": "plugins/grafana-slo-app/slo-breach",
"placement": "plugins/grafana-slo-app/slo-breach",
"type": "link",
"title": "Declare incident",
"description": "Declares a new incident",
"path": "/incidents/declare"
},
{
"target": "plugins/grafana-slo-app/slo-breach",
"placement": "plugins/grafana-slo-app/slo-breach",
"type": "link",
"title": "Declare incident",
"description": "Declares a new incident (path without backslash)",
Expand Down
4 changes: 2 additions & 2 deletions pkg/plugins/plugindef/plugindef.cue
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ seqs: [
component?: string

// The minimum role a user must have to see this page in the navigation menu.
role?: "Admin" | "Editor" | "Viewer"
role?: "Admin" | "Editor" | "Viewer"

// RBAC action the user must have to access the route
action?: string
Expand All @@ -124,7 +124,7 @@ seqs: [

#ExtensionsLink: {
// Target where the link will be rendered
target: =~"^(plugins|grafana)\/[a-z-/0-9]*$"
placement: =~"^(plugins|grafana)\/[a-z-/0-9]*$"
// Type of extension
type: "link"
// Title that will be displayed for the rendered link
Expand Down
2 changes: 1 addition & 1 deletion pkg/plugins/plugindef/plugindef_types_gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 7 additions & 7 deletions public/app/features/plugins/extensions/registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ describe('Plugin registry', () => {
const registry = createPluginExtensionsRegistry({
'belugacdn-app': createConfig([
{
target: 'plugins/belugacdn-app/menu',
placement: 'plugins/belugacdn-app/menu',
title: 'The title',
type: PluginExtensionTypes.link,
description: 'Incidents are occurring!',
Expand All @@ -16,7 +16,7 @@ describe('Plugin registry', () => {
]),
'strava-app': createConfig([
{
target: 'plugins/strava-app/menu',
placement: 'plugins/strava-app/menu',
title: 'The title',
type: PluginExtensionTypes.link,
description: 'Incidents are occurring!',
Expand All @@ -25,14 +25,14 @@ describe('Plugin registry', () => {
]),
'duplicate-links-app': createConfig([
{
target: 'plugins/duplicate-links-app/menu',
placement: 'plugins/duplicate-links-app/menu',
title: 'The title',
type: PluginExtensionTypes.link,
description: 'Incidents are occurring!',
path: '/incidents/declare',
},
{
target: 'plugins/duplicate-links-app/menu',
placement: 'plugins/duplicate-links-app/menu',
title: 'The title',
type: PluginExtensionTypes.link,
description: 'Incidents are occurring!',
Expand All @@ -49,7 +49,7 @@ describe('Plugin registry', () => {
title: 'The title',
type: 'link',
description: 'Incidents are occurring!',
href: '/a/belugacdn-app/incidents/declare',
path: '/a/belugacdn-app/incidents/declare',
key: 539074708,
});
});
Expand All @@ -68,15 +68,15 @@ describe('Plugin registry', () => {
title: 'The title',
type: 'link',
description: 'Incidents are occurring!',
href: '/a/belugacdn-app/incidents/declare',
path: '/a/belugacdn-app/incidents/declare',
key: 539074708,
});

expect(pluginBLink).toEqual({
title: 'The title',
type: 'link',
description: 'Incidents are occurring!',
href: '/a/strava-app/incidents/declare',
path: '/a/strava-app/incidents/declare',
key: -1637066384,
});
});
Expand Down
14 changes: 7 additions & 7 deletions public/app/features/plugins/extensions/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ export function createPluginExtensionsRegistry(apps: Record<string, AppPluginCon
}

for (const extension of extensions) {
const target = extension.target;
const placement = extension.placement;
const item = createRegistryItem(pluginId, extension);

if (!Array.isArray(registry[target])) {
registry[target] = [item];
if (!Array.isArray(registry[placement])) {
registry[placement] = [item];
continue;
}

registry[target].push(item);
registry[placement].push(item);
continue;
}
}
Expand All @@ -38,14 +38,14 @@ export function createPluginExtensionsRegistry(apps: Record<string, AppPluginCon
}

function createRegistryItem(pluginId: string, extension: PluginsExtensionLinkConfig): PluginsExtensionLink {
const href = `/a/${pluginId}${extension.path}`;
const path = `/a/${pluginId}${extension.path}`;

return Object.freeze({
type: PluginExtensionTypes.link,
title: extension.title,
description: extension.description,
href: href,
key: hashKey(`${extension.title}${href}`),
path: path,
key: hashKey(`${extension.title}${path}`),
});
}

Expand Down
8 changes: 4 additions & 4 deletions public/app/features/sandbox/TestStuffPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export const TestStuffPage = () => {
<Page navModel={{ node: node, main: node }}>
<Page.Contents>
<HorizontalGroup>
<LinkToBasicApp target="grafana/sandbox/testing" />
<LinkToBasicApp placement="grafana/sandbox/testing" />
</HorizontalGroup>
{data && (
<AutoSizer style={{ width: '100%', height: '600px' }}>
Expand Down Expand Up @@ -148,8 +148,8 @@ export function getDefaultState(): State {
};
}

function LinkToBasicApp({ target }: { target: string }) {
const { extensions, error } = getPluginExtensions({ target });
function LinkToBasicApp({ placement }: { placement: string }) {
const { extensions, error } = getPluginExtensions({ placement });

if (error) {
return null;
Expand All @@ -159,7 +159,7 @@ function LinkToBasicApp({ target }: { target: string }) {
<div>
{extensions.map((extension) => {
return (
<LinkButton href={extension.href} title={extension.description} key={extension.key}>
<LinkButton href={extension.path} title={extension.description} key={extension.key}>
{extension.title}
</LinkButton>
);
Expand Down

0 comments on commit f46f8bd

Please sign in to comment.