diff --git a/src/features/application-menu/handling-of-orphan-application-menu-items.test.ts b/src/features/application-menu/handling-of-orphan-application-menu-items.test.ts new file mode 100644 index 0000000000..e2678138a2 --- /dev/null +++ b/src/features/application-menu/handling-of-orphan-application-menu-items.test.ts @@ -0,0 +1,82 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import type { ApplicationBuilder } from "../../renderer/components/test-utils/get-application-builder"; +import { getApplicationBuilder } from "../../renderer/components/test-utils/get-application-builder"; +import populateApplicationMenuInjectable from "./main/populate-application-menu.injectable"; +import { advanceFakeTime, useFakeTime } from "../../common/test-utils/use-fake-time"; +import { getCompositePaths } from "./main/menu-items/get-composite/get-composite-paths/get-composite-paths"; +import { getInjectable } from "@ogre-tools/injectable"; +import applicationMenuItemInjectionToken from "./main/menu-items/application-menu-item-injection-token"; +import { runInAction } from "mobx"; +import loggerInjectable from "../../common/logger.injectable"; +import type { Logger } from "../../common/logger"; + +describe("handling-of-orphan-application-menu-items, given orphan menu item", () => { + let builder: ApplicationBuilder; + let populateApplicationMenuMock: jest.Mock; + let logErrorMock: jest.Mock; + + beforeEach(async () => { + useFakeTime(); + + populateApplicationMenuMock = jest.fn(); + logErrorMock = jest.fn(); + + builder = getApplicationBuilder(); + + builder.beforeApplicationStart((mainDi) => { + const someOrphanMenuItemInjectable = getInjectable({ + id: "some-orphan-menu-item", + instantiate: di => ({ + kind: "sub-menu" as const, + id: "some-item-id", + // Note: unknown id makes this item an orphan. + parentId: "some-unknown-parent-id", + orderNumber: 0, + label: "irrelevant", + }), + + injectionToken: applicationMenuItemInjectionToken, + }); + + runInAction(() => { + mainDi.register(someOrphanMenuItemInjectable); + }); + + mainDi.override(loggerInjectable, () => ({ error: logErrorMock }) as unknown as Logger); + + mainDi.override( + populateApplicationMenuInjectable, + () => populateApplicationMenuMock, + ); + }); + + await builder.startHidden(); + }); + + describe("given some time passes", () => { + let applicationMenuPaths: string[]; + + beforeEach(() => { + advanceFakeTime(100); + + applicationMenuPaths = getCompositePaths( + populateApplicationMenuMock.mock.calls[0][0], + ); + }); + + it("keeps showing the other application menu items without throwing", () => { + expect(applicationMenuPaths.length).toBeGreaterThan(0); + }); + + it("does not show orphan application menu item", () => { + expect(applicationMenuPaths.find(x => x.endsWith("some-item-id"))); + }); + + it("logs about bad menu item", () => { + expect(logErrorMock).toHaveBeenCalledWith('[MENU]: cannot render menu item for missing parentIds: "some-unknown-parent-id"'); + }); + }); +}); diff --git a/src/features/application-menu/main/application-menu-item-composite.injectable.ts b/src/features/application-menu/main/application-menu-item-composite.injectable.ts index e39538face..3125603f04 100644 --- a/src/features/application-menu/main/application-menu-item-composite.injectable.ts +++ b/src/features/application-menu/main/application-menu-item-composite.injectable.ts @@ -9,6 +9,7 @@ import getComposite from "./menu-items/get-composite/get-composite"; import { computed } from "mobx"; import { pipeline } from "@ogre-tools/fp"; import type { ApplicationMenuItemTypes } from "./menu-items/application-menu-item-injection-token"; +import loggerInjectable from "../../../common/logger.injectable"; export interface MenuItemRoot { id: "root"; parentId: undefined; kind: "root"; orderNumber: 0 } @@ -17,6 +18,7 @@ const applicationMenuItemCompositeInjectable = getInjectable({ instantiate: (di) => { const menuItems = di.inject(applicationMenuItemsInjectable); + const logger = di.inject(loggerInjectable); return computed((): Composite => { const items = menuItems.get(); @@ -33,7 +35,15 @@ const applicationMenuItemCompositeInjectable = getInjectable({ ...items, ], - (x) => getComposite({ source: x }), + (x) => getComposite({ + source: x, + + handleMissingParentIds: ({ missingParentIds }) => { + logger.error( + `[MENU]: cannot render menu item for missing parentIds: "${missingParentIds.join('", "')}"`, + ); + }, + }), ); }); }, diff --git a/src/features/application-menu/main/application-menu-items.injectable.ts b/src/features/application-menu/main/application-menu-items.injectable.ts index 8c3ccd2c0f..3bd6ea9379 100644 --- a/src/features/application-menu/main/application-menu-items.injectable.ts +++ b/src/features/application-menu/main/application-menu-items.injectable.ts @@ -23,24 +23,6 @@ const applicationMenuItemsInjectable = getInjectable({ .get() .filter(isShown), ); - - // Prepare menu items order - - // // Modify menu from extensions-api - // for (const menuItem of electronMenuItems.get()) { - // const parentMenu = appMenu.get(menuItem.parentId); - // - // if (!parentMenu) { - // logger.error( - // `[MENU]: cannot register menu item for parentId=${menuItem.parentId}, parent item doesn't exist`, - // { menuItem }, - // ); - // - // continue; - // } - // - // // (parentMenu.submenu ??= []).push(menuItem); - // } }, });