From 8c32ad47eea6b698b9f40febb5d87d950104696a Mon Sep 17 00:00:00 2001 From: axel7083 <42176370+axel7083@users.noreply.github.com> Date: Mon, 16 Sep 2024 10:25:20 +0200 Subject: [PATCH] feat(extension-api): allow extension to register custom navigation route (#8837) * feat: adding navigation route registry Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> * test: ensuring navigation manager work as expected Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> * fix: simplified navigation route ids Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> * fix: adding extension id has routeId prefix Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> * docs: enhancing documentation for custom navigation namespace Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> * fix: prevent registered route from being overwritten Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> * Apply suggestions from @feloy Co-authored-by: Philippe Martin Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> --------- Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> Co-authored-by: Philippe Martin --- packages/extension-api/src/extension-api.d.ts | 36 ++++++++ .../main/src/plugin/extension-loader.spec.ts | 12 +++ packages/main/src/plugin/extension-loader.ts | 13 +++ packages/main/src/plugin/index.ts | 1 + .../navigation/navigation-manager.spec.ts | 89 ++++++++++++++++++- .../plugin/navigation/navigation-manager.ts | 42 ++++++++- 6 files changed, 191 insertions(+), 2 deletions(-) diff --git a/packages/extension-api/src/extension-api.d.ts b/packages/extension-api/src/extension-api.d.ts index 5a06a747f33..71dff416799 100644 --- a/packages/extension-api/src/extension-api.d.ts +++ b/packages/extension-api/src/extension-api.d.ts @@ -4685,6 +4685,42 @@ declare module '@podman-desktop/api' { * Navigate to the Edit Provider Container Connection page */ export function navigateToEditProviderContainerConnection(connection: ProviderContainerConnection): Promise; + + /** + * Allow to define custom route for an extension. + * + * @remarks + * The commandId used must have been registered through {@link commands.registerCommand} + * + * @example + * ```ts + * import { navigation, commands } from '@podman-desktop/api'; + * + * commands.registerCommand('redirect-download-command', (trackingId: string) => { + * // todo: do something with the trackingId + * }); + * + * // register the route + * navigation.register('download-page', 'redirect-download-command'); + * + * // when needed call the navigate with the route id registered to + * // trigger the command + * navigation.navigate('download-page', 'dummy-tracking-id'); + * ``` + * + * @param routeId a unique string value that could be used in {@link navigation.navigate} + * @param commandId the command that will be executed on navigate + */ + export function register(routeId: string, commandId: string): Disposable; + + /** + * Allow extension to navigate to a custom route. + * The route needs to have been registered using {@link navigation.register} + * + * @param routeId the identifier of the route to use + * @param args the arguments to provide to the command linked to the routeId + */ + export function navigate(routeId: string, ...args: unknown[]): Promise; } /** diff --git a/packages/main/src/plugin/extension-loader.spec.ts b/packages/main/src/plugin/extension-loader.spec.ts index b61991fc300..25d8b785063 100644 --- a/packages/main/src/plugin/extension-loader.spec.ts +++ b/packages/main/src/plugin/extension-loader.spec.ts @@ -229,6 +229,7 @@ const navigationManager: NavigationManager = new NavigationManager( contributionManager, providerRegistry, webviewRegistry, + commandRegistry, ); const colorRegistry = { @@ -2274,6 +2275,17 @@ test('when loading registry registerRegistry, do not push to disposables', async expect(disposables.length).toBe(0); }); +test('when registering a navigation route, should be pushed to disposables', () => { + const disposables: IDisposable[] = []; + + const api = extensionLoader.createApi('/path', {}, disposables); + expect(api).toBeDefined(); + + expect(disposables.length).toBe(0); + api.navigation.register('dummy-route-id', 'dummy-command-id'); + expect(disposables.length).toBe(1); +}); + describe('loading extension folders', () => { const fileEntry = { isDirectory: () => false, diff --git a/packages/main/src/plugin/extension-loader.ts b/packages/main/src/plugin/extension-loader.ts index 87fe4b6e861..28d85672d77 100644 --- a/packages/main/src/plugin/extension-loader.ts +++ b/packages/main/src/plugin/extension-loader.ts @@ -1420,6 +1420,19 @@ export class ExtensionLoader { ): Promise => { await this.navigationManager.navigateToEditProviderContainerConnection(connection); }, + navigate: async (routeId: string, ...args: unknown[]): Promise => { + return this.navigationManager.navigateToRoute(`${extensionInfo.id}.${routeId}`, args); + }, + register: (routeId: string, commandId: string): Disposable => { + const disposable = this.navigationManager.registerRoute({ + routeId: `${extensionInfo.id}.${routeId}`, + commandId: commandId, + }); + + disposables.push(disposable); + + return disposable; + }, }; const version = app.getVersion(); diff --git a/packages/main/src/plugin/index.ts b/packages/main/src/plugin/index.ts index 49455f48b76..3626b0ea748 100644 --- a/packages/main/src/plugin/index.ts +++ b/packages/main/src/plugin/index.ts @@ -633,6 +633,7 @@ export class PluginSystem { contributionManager, providerRegistry, webviewRegistry, + commandRegistry, ); this.extensionLoader = new ExtensionLoader( diff --git a/packages/main/src/plugin/navigation/navigation-manager.spec.ts b/packages/main/src/plugin/navigation/navigation-manager.spec.ts index c73c420b441..70d818ec79b 100644 --- a/packages/main/src/plugin/navigation/navigation-manager.spec.ts +++ b/packages/main/src/plugin/navigation/navigation-manager.spec.ts @@ -17,8 +17,9 @@ ***********************************************************************/ import type { ProviderContainerConnection } from '@podman-desktop/api'; -import { beforeEach, expect, test, vi } from 'vitest'; +import { beforeEach, describe, expect, test, vi } from 'vitest'; +import type { CommandRegistry } from '/@/plugin/command-registry.js'; import { NavigationPage } from '/@api/navigation-page.js'; import type { WebviewInfo } from '/@api/webview-info.js'; @@ -59,6 +60,11 @@ const webviewRegistry = { listWebviews: vi.fn(), } as unknown as WebviewRegistry; +const commandRegistry: CommandRegistry = { + hasCommand: vi.fn(), + executeCommand: vi.fn(), +} as unknown as CommandRegistry; + beforeEach(() => { vi.resetAllMocks(); navigationManager = new TestNavigationManager( @@ -67,6 +73,7 @@ beforeEach(() => { contributionManager, providerRegistry, webviewRegistry, + commandRegistry, ); }); @@ -162,3 +169,83 @@ test('check navigateToEditProviderContainerConnection', async () => { }, }); }); + +describe('register route', () => { + test('registering route should provide a disposable', () => { + const routeId = 'dummy-route-id'; + const disposable = navigationManager.registerRoute({ + routeId: routeId, + commandId: 'fake-command-id', + }); + + expect(navigationManager.hasRoute(routeId)).toBeTruthy(); + + disposable.dispose(); + + expect(navigationManager.hasRoute(routeId)).toBeFalsy(); + }); + + test('registering existing route should throw an error', async () => { + const routeId = 'dummy-route-id'; + navigationManager.registerRoute({ + routeId: routeId, + commandId: 'fake-command-id', + }); + + expect(() => { + return navigationManager.registerRoute({ + routeId: routeId, + commandId: 'fake-command-id', + }); + }).toThrowError('routeId dummy-route-id is already registered.'); + }); + + test('calling navigateToRoute with invalid routeId should raise an error', async () => { + await expect(() => { + return navigationManager.navigateToRoute('invalidId'); + }).rejects.toThrowError('navigation route invalidId does not exists.'); + }); + + test('calling navigateToRoute on route with invalid command should raise an error', async () => { + vi.mocked(commandRegistry.hasCommand).mockReturnValue(false); + const routeId = 'dummy-route-id'; + navigationManager.registerRoute({ + routeId: routeId, + commandId: 'fake-command-id', + }); + + await expect(() => { + return navigationManager.navigateToRoute(routeId); + }).rejects.toThrowError('navigation route dummy-route-id registered an unknown command: fake-command-id'); + + expect(commandRegistry.hasCommand).toHaveBeenCalledOnce(); + }); + + test('calling navigateToRoute should propagate the argument to the command', async () => { + vi.mocked(commandRegistry.hasCommand).mockReturnValue(true); + vi.mocked(commandRegistry.executeCommand).mockResolvedValue(undefined); + const routeId = 'dummy-route-id'; + navigationManager.registerRoute({ + routeId: routeId, + commandId: 'dummy-command-id', + }); + + await navigationManager.navigateToRoute(routeId, 'potatoes', 'candies'); + + expect(commandRegistry.executeCommand).toHaveBeenCalledWith('dummy-command-id', 'potatoes', 'candies'); + }); + + test('error in the command should be propagate to the caller', async () => { + vi.mocked(commandRegistry.hasCommand).mockReturnValue(true); + vi.mocked(commandRegistry.executeCommand).mockRejectedValue('Dummy error'); + const routeId = 'dummy-route-id'; + navigationManager.registerRoute({ + routeId: routeId, + commandId: 'dummy-command-id', + }); + + await expect(() => { + return navigationManager.navigateToRoute(routeId); + }).rejects.toThrowError('Dummy error'); + }); +}); diff --git a/packages/main/src/plugin/navigation/navigation-manager.ts b/packages/main/src/plugin/navigation/navigation-manager.ts index 623772f8bda..00940b51afb 100644 --- a/packages/main/src/plugin/navigation/navigation-manager.ts +++ b/packages/main/src/plugin/navigation/navigation-manager.ts @@ -19,27 +19,67 @@ import type { ProviderContainerConnection } from '@podman-desktop/api'; import type { ApiSenderType } from '/@/plugin/api.js'; +import type { CommandRegistry } from '/@/plugin/command-registry.js'; import type { ContainerProviderRegistry } from '/@/plugin/container-registry.js'; import type { ContributionManager } from '/@/plugin/contribution-manager.js'; import { NavigationPage } from '/@api/navigation-page.js'; import type { NavigationRequest } from '/@api/navigation-request.js'; import type { ProviderRegistry } from '../provider-registry.js'; +import { Disposable } from '../types/disposable.js'; import type { WebviewRegistry } from '../webview/webview-registry.js'; +export interface NavigationRoute { + routeId: string; + commandId: string; +} + export class NavigationManager { + #registry: Map; + constructor( private apiSender: ApiSenderType, private containerRegistry: ContainerProviderRegistry, private contributionManager: ContributionManager, private providerRegistry: ProviderRegistry, private webviewRegistry: WebviewRegistry, - ) {} + private commandRegistry: CommandRegistry, + ) { + this.#registry = new Map(); + } navigateTo(navigateRequest: NavigationRequest): void { this.apiSender.send('navigate', navigateRequest); } + registerRoute(route: NavigationRoute): Disposable { + if (this.hasRoute(route.routeId)) { + throw new Error(`routeId ${route.routeId} is already registered.`); + } + this.#registry.set(route.routeId, route); + + return Disposable.create(() => { + this.#registry.delete(route.routeId); + }); + } + + hasRoute(routeId: string): boolean { + return this.#registry.has(routeId); + } + + async navigateToRoute(routeId: string, ...args: unknown[]): Promise { + const route = this.#registry.get(routeId); + if (!route) { + throw new Error(`navigation route ${routeId} does not exists.`); + } + + if (!this.commandRegistry.hasCommand(route.commandId)) { + throw new Error(`navigation route ${routeId} registered an unknown command: ${route.commandId}`); + } + + return this.commandRegistry.executeCommand(route.commandId, ...args); + } + async navigateToProviderTask(internalProviderId: string, taskId?: number): Promise { this.navigateTo({ page: NavigationPage.PROVIDER_TASK,