diff --git a/src/features/application-menu/application-menu-in-legacy-extension-api.test.ts b/src/features/application-menu/application-menu-in-legacy-extension-api.test.ts index 06dbc384c7..96bdaa8660 100644 --- a/src/features/application-menu/application-menu-in-legacy-extension-api.test.ts +++ b/src/features/application-menu/application-menu-in-legacy-extension-api.test.ts @@ -4,25 +4,33 @@ */ import { getInjectable } from "@ogre-tools/injectable"; import { noop } from "lodash/fp"; -import { action } from "mobx"; +import { runInAction } from "mobx"; import type { ApplicationBuilder } from "../../renderer/components/test-utils/get-application-builder"; import { getApplicationBuilder } from "../../renderer/components/test-utils/get-application-builder"; import type { FakeExtensionOptions } from "../../renderer/components/test-utils/get-extension-fake"; import applicationMenuItemInjectionToken from "./main/menu-items/application-menu-item-injection-token"; +import logErrorInjectable from "../../common/log-error.injectable"; describe("application-menu-in-legacy-extension-api", () => { let builder: ApplicationBuilder; + let logErrorMock: jest.Mock; beforeEach(async () => { builder = getApplicationBuilder(); builder.beforeApplicationStart( - action((mainDi) => { - mainDi.register( - someTopMenuItemInjectable, - someNonExtensionBasedMenuItemInjectable, - ); - }), + (mainDi) => { + runInAction(() => { + mainDi.register( + someTopMenuItemInjectable, + someNonExtensionBasedMenuItemInjectable, + ); + }); + + logErrorMock = jest.fn(); + + mainDi.override(logErrorInjectable, () => logErrorMock); + }, ); await builder.startHidden(); @@ -137,6 +145,55 @@ describe("application-menu-in-legacy-extension-api", () => { }); }); }); + + describe("when extension with unrecognizable application menu items is enabled", () => { + + beforeEach(() => { + const testExtensionOptions: FakeExtensionOptions = { + id: "some-test-extension", + name: "some-extension-name", + + mainOptions: { + appMenus: [ + { + id: "some-recognizable-item", + parentId: "some-top-menu-item", + click: noop, + }, + + { + id: "some-unrecognizable-item", + parentId: "some-top-menu-item", + // Note: there is no way to recognize this + // click: noop, + // role: "help" + // submenu: [] + // type: "separator" + }, + ], + }, + }; + + builder.extensions.enable(testExtensionOptions); + }); + + it("only recognizable menu items from extension exist", () => { + const menuItemPathsForExtension = builder.applicationMenu.items.filter( + (x) => + x.startsWith("root.some-top-menu-item.some-extension-name"), + ); + + expect(menuItemPathsForExtension).toEqual([ + "root.some-top-menu-item.some-extension-name/some-recognizable-item", + ]); + }); + + it("logs about the unrecognizable item", () => { + expect(logErrorMock).toHaveBeenCalledWith( + '[MENU]: Tried to register menu item "some-extension-name/some-unrecognizable-item" but it is not recognizable as any of ApplicationMenuItemTypes', + ); + }); + }); }); const someTopMenuItemInjectable = getInjectable({ diff --git a/src/features/application-menu/main/application-menu-item-registrator.injectable.ts b/src/features/application-menu/main/application-menu-item-registrator.injectable.ts index ba8d34953f..8c3d635db0 100644 --- a/src/features/application-menu/main/application-menu-item-registrator.injectable.ts +++ b/src/features/application-menu/main/application-menu-item-registrator.injectable.ts @@ -15,16 +15,22 @@ import type { } from "./menu-items/application-menu-item-injection-token"; import applicationMenuItemInjectionToken from "./menu-items/application-menu-item-injection-token"; import type { MenuRegistration } from "./menu-registration"; +import logErrorInjectable from "../../../common/log-error.injectable"; const applicationMenuItemRegistratorInjectable = getInjectable({ id: "application-menu-item-registrator", - instantiate: () => (ext: LensExtension) => { - const extension = ext as LensMainExtension; + instantiate: (di) => { + const logError = di.inject(logErrorInjectable); + const toRecursedInjectables = toRecursedInjectablesFor(logError); - return extension.appMenus.flatMap( - toRecursedInjectables([extension.sanitizedExtensionId]), - ); + return (ext: LensExtension) => { + const extension = ext as LensMainExtension; + + return extension.appMenus.flatMap( + toRecursedInjectables([extension.sanitizedExtensionId]), + ); + }; }, injectionToken: extensionRegistratorInjectionToken, @@ -32,13 +38,17 @@ const applicationMenuItemRegistratorInjectable = getInjectable({ export default applicationMenuItemRegistratorInjectable; -const toRecursedInjectables = - (previousIdPath: string[]) => +const toRecursedInjectablesFor = (logError: (errorMessage: string) => void) => { + const toRecursedInjectables = (previousIdPath: string[]) => ( registration: MenuRegistration, index: number, - // Todo: new version of injectable would require less type parameters with defaults. - ): Injectable[] => { + // Todo: new version of injectable would require less type parameters with defaults. + ): Injectable< + ApplicationMenuItemTypes, + ApplicationMenuItemTypes, + void + >[] => { const previousIdPathString = previousIdPath.join("/"); const registrationId = registration.id || index.toString(); const currentIdPath = [...previousIdPath, registrationId]; @@ -52,7 +62,9 @@ const toRecursedInjectables = index, }); - if(!menuItem) { + if (!menuItem) { + logError(`[MENU]: Tried to register menu item "${currentIdPathString}" but it is not recognizable as any of ApplicationMenuItemTypes`); + return []; } @@ -73,6 +85,9 @@ const toRecursedInjectables = ]; }; + return toRecursedInjectables; +}; + const getApplicationMenuItem = ({ registration, index,