diff --git a/.changeset/pretty-jeans-warn.md b/.changeset/pretty-jeans-warn.md new file mode 100644 index 0000000000000..9a99fac55d111 --- /dev/null +++ b/.changeset/pretty-jeans-warn.md @@ -0,0 +1,6 @@ +--- +'@rocket.chat/apps-engine': minor +'@rocket.chat/meteor': minor +--- + +Fix an issue where action buttons registered by apps would be displayed even if their apps were disabled diff --git a/apps/meteor/ee/server/apps/communication/endpoints/actionButtonsHandler.ts b/apps/meteor/ee/server/apps/communication/endpoints/actionButtonsHandler.ts index 3f3af296ae3e8..fef8902dfcb84 100644 --- a/apps/meteor/ee/server/apps/communication/endpoints/actionButtonsHandler.ts +++ b/apps/meteor/ee/server/apps/communication/endpoints/actionButtonsHandler.ts @@ -6,8 +6,8 @@ export const registerActionButtonsHandler = ({ api, _manager }: AppsRestApi) => 'actionButtons', { authRequired: false }, { - get() { - const buttons = _manager.getUIActionButtonManager().getAllActionButtons(); + async get() { + const buttons = await _manager.getUIActionButtonManager().getAllActionButtons(); return API.v1.success(buttons); }, diff --git a/packages/apps-engine/package.json b/packages/apps-engine/package.json index 07a47f5b0cc40..53bab8f53b536 100644 --- a/packages/apps-engine/package.json +++ b/packages/apps-engine/package.json @@ -7,7 +7,7 @@ "scripts": { "start": "run-s .:build:clean .:build:watch", "testunit": "run-p .:test:node .:test:deno", - ".:test:node": "NODE_ENV=test ts-node ./tests/runner.ts", + ".:test:node": "NODE_ENV=test ts-node --transpileOnly ./tests/runner.ts", ".:test:deno": "cd deno-runtime && deno task test", ".:lint:eslint": "eslint .", ".:lint:deno": "deno lint --ignore=deno-runtime/.deno deno-runtime/", diff --git a/packages/apps-engine/src/server/managers/UIActionButtonManager.ts b/packages/apps-engine/src/server/managers/UIActionButtonManager.ts index 37221774d6006..b062017db9b24 100644 --- a/packages/apps-engine/src/server/managers/UIActionButtonManager.ts +++ b/packages/apps-engine/src/server/managers/UIActionButtonManager.ts @@ -1,3 +1,4 @@ +import { AppStatusUtils } from '../../definition/AppStatus'; import type { IUIActionButton, IUIActionButtonDescriptor } from '../../definition/ui'; import type { AppManager } from '../AppManager'; import type { AppActivationBridge } from '../bridges'; @@ -8,9 +9,12 @@ import { AppPermissions } from '../permissions/AppPermissions'; export class UIActionButtonManager { private readonly activationBridge: AppActivationBridge; + private readonly manager: AppManager; + private registeredActionButtons = new Map>(); constructor(manager: AppManager) { + this.manager = manager; this.activationBridge = manager.getBridges().getAppActivationBridge(); } @@ -39,18 +43,37 @@ export class UIActionButtonManager { return this.registeredActionButtons.get(appId); } - public getAllActionButtons() { + public async getAllActionButtons(): Promise> { const buttonList: Array = []; - // Flatten map to a simple list of all buttons - this.registeredActionButtons.forEach((appButtons, appId) => + // Flatten map to a simple list of buttons from enabled apps only + for (const [appId, appButtons] of this.registeredActionButtons) { + const app = this.manager.getOneById(appId); + + // Skip if app doesn't exist + if (!app) { + continue; + } + + // or if it is not enabled + try { + const appStatus = await app.getStatus(); + if (!AppStatusUtils.isEnabled(appStatus)) { + continue; + } + } catch (error) { + // If we can't get the app status, skip this app's buttons + continue; + } + + // Add buttons from this enabled app appButtons.forEach((button) => buttonList.push({ ...button, appId, }), - ), - ); + ); + } return buttonList; } diff --git a/packages/apps-engine/tests/server/managers/UIActionButtonManager.spec.ts b/packages/apps-engine/tests/server/managers/UIActionButtonManager.spec.ts new file mode 100644 index 0000000000000..9db20785c280c --- /dev/null +++ b/packages/apps-engine/tests/server/managers/UIActionButtonManager.spec.ts @@ -0,0 +1,342 @@ +import type { RestorableFunctionSpy } from 'alsatian'; +import { AsyncTest, Expect, Setup, SetupFixture, SpyOn, Teardown, Test } from 'alsatian'; + +import { AppStatus } from '../../../src/definition/AppStatus'; +import type { IUIActionButtonDescriptor } from '../../../src/definition/ui'; +import { UIActionButtonContext } from '../../../src/definition/ui'; +import type { AppManager } from '../../../src/server/AppManager'; +import type { ProxiedApp } from '../../../src/server/ProxiedApp'; +import type { AppActivationBridge, AppBridges } from '../../../src/server/bridges'; +import { AppPermissionManager } from '../../../src/server/managers/AppPermissionManager'; +import { UIActionButtonManager } from '../../../src/server/managers/UIActionButtonManager'; +import { AppPermissions } from '../../../src/server/permissions/AppPermissions'; +import { TestsAppBridges } from '../../test-data/bridges/appBridges'; + +export class UIActionButtonManagerTestFixture { + private mockBridges: TestsAppBridges; + + private mockApp: ProxiedApp; + + private mockApp2: ProxiedApp; + + private mockManager: AppManager; + + private mockActivationBridge: AppActivationBridge; + + private hasPermissionSpy: RestorableFunctionSpy; + + private notifyAboutErrorSpy: RestorableFunctionSpy; + + private doActionsChangedSpy: RestorableFunctionSpy; + + @SetupFixture + public setupFixture() { + this.mockBridges = new TestsAppBridges(); + this.mockActivationBridge = this.mockBridges.getAppActivationBridge(); + + this.mockApp = { + getID() { + return 'testing-app'; + }, + getName() { + return 'Test App'; + }, + getStatus() { + return Promise.resolve(AppStatus.AUTO_ENABLED); + }, + } as ProxiedApp; + + this.mockApp2 = { + getID() { + return 'testing-app-2'; + }, + getName() { + return 'Test App 2'; + }, + getStatus() { + return Promise.resolve(AppStatus.AUTO_ENABLED); + }, + } as ProxiedApp; + + const bri = this.mockBridges; + this.mockManager = { + getBridges(): AppBridges { + return bri; + }, + getOneById: (appId: string): ProxiedApp | undefined => { + if (appId === 'testing-app') { + return this.mockApp; + } + if (appId === 'testing-app-2') { + return this.mockApp2; + } + }, + } as AppManager; + } + + @Setup + public setup() { + this.notifyAboutErrorSpy = SpyOn(AppPermissionManager, 'notifyAboutError'); + this.hasPermissionSpy = SpyOn(AppPermissionManager, 'hasPermission'); + this.doActionsChangedSpy = SpyOn(this.mockActivationBridge, 'doActionsChanged'); + } + + @Teardown + public teardown() { + this.notifyAboutErrorSpy.restore(); + this.hasPermissionSpy.restore(); + this.doActionsChangedSpy.restore(); + } + + @Test() + public basicUIActionButtonManager() { + Expect(() => new UIActionButtonManager(this.mockManager)).not.toThrow(); + + const manager = new UIActionButtonManager(this.mockManager); + Expect((manager as any).manager).toBe(this.mockManager); + Expect((manager as any).activationBridge).toBe(this.mockActivationBridge); + Expect((manager as any).registeredActionButtons).toBeDefined(); + Expect((manager as any).registeredActionButtons.size).toBe(0); + } + + @Test() + public registerActionButtonWithPermission() { + this.hasPermissionSpy.andReturn(true); + + const manager = new UIActionButtonManager(this.mockManager); + const button: IUIActionButtonDescriptor = { + actionId: 'test-action', + context: UIActionButtonContext.MESSAGE_ACTION, + labelI18n: 'test.label', + }; + + const result = manager.registerActionButton('testing-app', button); + + Expect(result).toBe(true); + Expect(this.hasPermissionSpy).toHaveBeenCalledWith('testing-app', AppPermissions.ui.registerButtons); + Expect(this.mockActivationBridge.doActionsChanged).toHaveBeenCalled(); + Expect((manager as any).registeredActionButtons.size).toBe(1); + Expect((manager as any).registeredActionButtons.get('testing-app').size).toBe(1); + Expect((manager as any).registeredActionButtons.get('testing-app').get('test-action')).toBe(button); + } + + @Test() + public registerActionButtonWithoutPermission() { + this.hasPermissionSpy.andReturn(false); + this.notifyAboutErrorSpy.andCall(() => {}); + + const manager = new UIActionButtonManager(this.mockManager); + const button: IUIActionButtonDescriptor = { + actionId: 'test-action', + context: UIActionButtonContext.MESSAGE_ACTION, + labelI18n: 'test.label', + }; + + const result = manager.registerActionButton('testing-app', button); + + Expect(result).toBe(false); + Expect(this.hasPermissionSpy).toHaveBeenCalledWith('testing-app', AppPermissions.ui.registerButtons); + Expect(this.notifyAboutErrorSpy).toHaveBeenCalled(); + Expect(this.mockActivationBridge.doActionsChanged).not.toHaveBeenCalled(); + Expect((manager as any).registeredActionButtons.size).toBe(0); + } + + @Test() + public registerMultipleButtonsForSameApp() { + this.hasPermissionSpy.andReturn(true); + + const manager = new UIActionButtonManager(this.mockManager); + const button1: IUIActionButtonDescriptor = { + actionId: 'action-1', + context: UIActionButtonContext.MESSAGE_ACTION, + labelI18n: 'test.label1', + }; + const button2: IUIActionButtonDescriptor = { + actionId: 'action-2', + context: UIActionButtonContext.ROOM_ACTION, + labelI18n: 'test.label2', + }; + + manager.registerActionButton('testing-app', button1); + manager.registerActionButton('testing-app', button2); + + Expect((manager as any).registeredActionButtons.size).toBe(1); + Expect((manager as any).registeredActionButtons.get('testing-app').size).toBe(2); + Expect((manager as any).registeredActionButtons.get('testing-app').get('action-1')).toBe(button1); + Expect((manager as any).registeredActionButtons.get('testing-app').get('action-2')).toBe(button2); + } + + @Test() + public clearAppActionButtons() { + this.hasPermissionSpy.andReturn(true); + + const manager = new UIActionButtonManager(this.mockManager); + const button: IUIActionButtonDescriptor = { + actionId: 'test-action', + context: UIActionButtonContext.MESSAGE_ACTION, + labelI18n: 'test.label', + }; + + manager.registerActionButton('testing-app', button); + Expect((manager as any).registeredActionButtons.get('testing-app').size).toBe(1); + + manager.clearAppActionButtons('testing-app'); + + Expect((manager as any).registeredActionButtons.get('testing-app').size).toBe(0); + Expect(this.mockActivationBridge.doActionsChanged).toHaveBeenCalled().exactly(2); + } + + @Test() + public getAppActionButtons() { + this.hasPermissionSpy.andReturn(true); + + const manager = new UIActionButtonManager(this.mockManager); + const button: IUIActionButtonDescriptor = { + actionId: 'test-action', + context: UIActionButtonContext.MESSAGE_ACTION, + labelI18n: 'test.label', + }; + + manager.registerActionButton('testing-app', button); + + const buttons = manager.getAppActionButtons('testing-app'); + Expect(buttons).toBeDefined(); + Expect(buttons?.size).toBe(1); + Expect(buttons?.get('test-action')).toBe(button); + + const nonExistentButtons = manager.getAppActionButtons('non-existent'); + Expect(nonExistentButtons).toBe(undefined); + } + + @AsyncTest() + public async getAllActionButtonsFromEnabledApp() { + this.hasPermissionSpy.andReturn(true); + + const spy = SpyOn(this.mockApp, 'getStatus'); + + spy.andReturn(Promise.resolve(AppStatus.AUTO_ENABLED)); + + const manager = new UIActionButtonManager(this.mockManager); + const button: IUIActionButtonDescriptor = { + actionId: 'test-action', + context: UIActionButtonContext.MESSAGE_ACTION, + labelI18n: 'test.label', + }; + + manager.registerActionButton('testing-app', button); + + const allButtons = await manager.getAllActionButtons(); + + Expect(allButtons).toBeDefined(); + Expect(allButtons.length).toBe(1); + Expect(allButtons[0].actionId).toBe('test-action'); + Expect(allButtons[0].appId).toBe('testing-app'); + Expect(allButtons[0].context).toBe(UIActionButtonContext.MESSAGE_ACTION); + Expect(allButtons[0].labelI18n).toBe('test.label'); + + spy.restore(); + } + + @AsyncTest() + public async getAllActionButtonsFromDisabledApp() { + this.hasPermissionSpy.andReturn(true); + + const spy = SpyOn(this.mockApp, 'getStatus'); + + spy.andReturn(Promise.resolve(AppStatus.DISABLED)); + + const manager = new UIActionButtonManager(this.mockManager); + const button: IUIActionButtonDescriptor = { + actionId: 'test-action', + context: UIActionButtonContext.MESSAGE_ACTION, + labelI18n: 'test.label', + }; + + manager.registerActionButton('testing-app', button); + + const allButtons = await manager.getAllActionButtons(); + + Expect(allButtons).toBeDefined(); + Expect(allButtons.length).toBe(0); + + spy.restore(); + } + + @AsyncTest() + public async getAllActionButtonsFromNonExistentApp() { + this.hasPermissionSpy.andReturn(true); + + const manager = new UIActionButtonManager(this.mockManager); + const button: IUIActionButtonDescriptor = { + actionId: 'test-action', + context: UIActionButtonContext.MESSAGE_ACTION, + labelI18n: 'test.label', + }; + + manager.registerActionButton('non-existent-app', button); + + const allButtons = await manager.getAllActionButtons(); + + Expect(allButtons).toBeDefined(); + Expect(allButtons.length).toBe(0); + } + + @AsyncTest() + public async getAllActionButtonsWithStatusError() { + this.hasPermissionSpy.andReturn(true); + + const spy = SpyOn(this.mockApp, 'getStatus'); + + spy.andReturn(Promise.reject(new Error('Status error'))); + + const manager = new UIActionButtonManager(this.mockManager); + const button: IUIActionButtonDescriptor = { + actionId: 'test-action', + context: UIActionButtonContext.MESSAGE_ACTION, + labelI18n: 'test.label', + }; + + manager.registerActionButton('testing-app', button); + + const allButtons = await manager.getAllActionButtons(); + + Expect(allButtons).toBeDefined(); + Expect(allButtons.length).toBe(0); + + spy.restore(); + } + + @AsyncTest() + public async getAllActionButtonsFromMultipleApps() { + this.hasPermissionSpy.andReturn(true); + + const button1: IUIActionButtonDescriptor = { + actionId: 'action-1', + context: UIActionButtonContext.MESSAGE_ACTION, + labelI18n: 'test.label1', + }; + const button2: IUIActionButtonDescriptor = { + actionId: 'action-2', + context: UIActionButtonContext.ROOM_ACTION, + labelI18n: 'test.label2', + }; + + const manager = new UIActionButtonManager(this.mockManager); + + manager.registerActionButton('testing-app', button1); + manager.registerActionButton('testing-app-2', button2); + + const allButtons = await manager.getAllActionButtons(); + + Expect(allButtons).toBeDefined(); + Expect(allButtons.length).toBe(2); + + const app1Button = allButtons.find((b) => b.appId === 'testing-app'); + const app2Button = allButtons.find((b) => b.appId === 'testing-app-2'); + + Expect(app1Button).toBeDefined(); + Expect(app1Button!.actionId).toBe('action-1'); + Expect(app2Button).toBeDefined(); + Expect(app2Button!.actionId).toBe('action-2'); + } +}