diff --git a/.changeset/eighty-wombats-smile.md b/.changeset/eighty-wombats-smile.md new file mode 100644 index 0000000000000..23ceecd25050b --- /dev/null +++ b/.changeset/eighty-wombats-smile.md @@ -0,0 +1,6 @@ +--- +'@rocket.chat/apps-engine': minor +'@rocket.chat/meteor': minor +--- + +Adds the ability to dynamically add and remove options from select/multi-select settings in the Apps Engine to support more flexible configuration scenarios by exposing two new methods on the settings API. diff --git a/packages/apps-engine/src/definition/accessors/ISettingUpdater.ts b/packages/apps-engine/src/definition/accessors/ISettingUpdater.ts index 3826286df6c93..9cdc15c7b1464 100644 --- a/packages/apps-engine/src/definition/accessors/ISettingUpdater.ts +++ b/packages/apps-engine/src/definition/accessors/ISettingUpdater.ts @@ -2,4 +2,5 @@ import type { ISetting } from '../settings/ISetting'; export interface ISettingUpdater { updateValue(id: ISetting['id'], value: ISetting['value']): Promise; + updateSelectOptions(id: ISetting['id'], values: ISetting['values']): Promise; } diff --git a/packages/apps-engine/src/server/accessors/SettingUpdater.ts b/packages/apps-engine/src/server/accessors/SettingUpdater.ts index 879c1282ee58a..8af37b4bccf75 100644 --- a/packages/apps-engine/src/server/accessors/SettingUpdater.ts +++ b/packages/apps-engine/src/server/accessors/SettingUpdater.ts @@ -3,23 +3,64 @@ import type { ISetting } from '../../definition/settings'; import type { ProxiedApp } from '../ProxiedApp'; import type { AppSettingsManager } from '../managers'; +/** + * Implementation of ISettingUpdater that provides methods to update app settings. + */ export class SettingUpdater implements ISettingUpdater { constructor( private readonly app: ProxiedApp, private readonly manager: AppSettingsManager, ) {} - public async updateValue(id: ISetting['id'], value: ISetting['value']) { - if (!this.app.getStorageItem().settings[id]) { - return; + /** + * Updates a single setting value + * @param id The setting ID to update + * @param value The new value to set + * @returns Promise that resolves when the update is complete + * @throws Error if the setting doesn't exist + */ + public async updateValue(id: ISetting['id'], value: ISetting['value']): Promise { + const appId = this.app.getID(); + const storageItem = this.app.getStorageItem(); + + if (!storageItem.settings?.[id]) { + throw new Error(`Setting "${id}" not found for app ${appId}`); } - const setting = this.manager.getAppSetting(this.app.getID(), id); + const setting = this.manager.getAppSetting(appId, id); - this.manager.updateAppSetting(this.app.getID(), { + this.manager.updateAppSetting(appId, { ...setting, updatedAt: new Date(), value, }); } + + /** + * Updates the values for a multi-value setting by overwriting them + * @param id The setting ID to update + * @param values The new values to set + * @returns Promise that resolves when the update is complete + * @throws Error if the setting doesn't exist + */ + public async updateSelectOptions(id: ISetting['id'], values: ISetting['values']): Promise { + const appId = this.app.getID(); + const storageItem = this.app.getStorageItem(); + + if (!storageItem.settings?.[id]) { + throw new Error(`Setting "${id}" not found for app ${appId}`); + } + + const setting = this.manager.getAppSetting(appId, id); + + // TODO: This operation completely overwrites existing values + // which could lead to loss of selected values. Consider: + // Adding warning logs when selected value will be removed + + this.manager.updateAppSetting(appId, { + ...setting, + updatedAt: new Date(), + values, // Overwrite the values instead of merging + }); + } } diff --git a/packages/apps-engine/tests/server/accessors/SettingUpdater.spec.ts b/packages/apps-engine/tests/server/accessors/SettingUpdater.spec.ts new file mode 100644 index 0000000000000..ebd982640bb81 --- /dev/null +++ b/packages/apps-engine/tests/server/accessors/SettingUpdater.spec.ts @@ -0,0 +1,104 @@ +import { AsyncTest, Expect, SetupFixture, SpyOn } from 'alsatian'; + +import type { ProxiedApp } from '../../../src/server/ProxiedApp'; +import { SettingUpdater } from '../../../src/server/accessors'; +import type { AppSettingsManager } from '../../../src/server/managers'; +import type { IAppStorageItem } from '../../../src/server/storage'; +import { TestData } from '../../test-data/utilities'; + +export class SettingUpdaterAccessorTestFixture { + private mockStorageItem: IAppStorageItem; + + private mockProxiedApp: ProxiedApp; + + private mockSettingsManager: AppSettingsManager; + + @SetupFixture + public setupFixture() { + // Set up mock storage with test settings + this.mockStorageItem = { + settings: {}, + } as IAppStorageItem; + + this.mockStorageItem.settings.singleValue = TestData.getSetting('singleValue'); + this.mockStorageItem.settings.multiValue = { + ...TestData.getSetting('multiValue'), + values: [ + { key: 'key1', i18nLabel: 'value1' }, + { key: 'key2', i18nLabel: 'value2' }, + ], + }; + + // Mock ProxiedApp + const si = this.mockStorageItem; + this.mockProxiedApp = { + getStorageItem(): IAppStorageItem { + return si; + }, + getID(): string { + return 'test-app-id'; + }, + } as ProxiedApp; + + // Mock AppSettingsManager + this.mockSettingsManager = {} as AppSettingsManager; + this.mockSettingsManager.getAppSetting = (appId: string, settingId: string) => { + return this.mockStorageItem.settings[settingId]; + }; + this.mockSettingsManager.updateAppSetting = (appId: string, setting: any) => { + this.mockStorageItem.settings[setting.id] = setting; + return Promise.resolve(); + }; + + SpyOn(this.mockSettingsManager, 'getAppSetting'); + SpyOn(this.mockSettingsManager, 'updateAppSetting'); + } + + @AsyncTest() + public async updateValueSuccessfully() { + const settingUpdater = new SettingUpdater(this.mockProxiedApp, this.mockSettingsManager); + + await settingUpdater.updateValue('singleValue', 'updated value'); + + Expect(this.mockSettingsManager.updateAppSetting).toHaveBeenCalled(); + Expect(this.mockStorageItem.settings.singleValue.value).toBe('updated value'); + // Verify updatedAt was set + Expect(this.mockStorageItem.settings.singleValue.updatedAt).toBeDefined(); + } + + @AsyncTest() + public async updateValueThrowsErrorForNonExistentSetting() { + const settingUpdater = new SettingUpdater(this.mockProxiedApp, this.mockSettingsManager); + + await Expect(() => settingUpdater.updateValue('nonExistent', 'value')).toThrowErrorAsync(Error, 'Setting "nonExistent" not found for app test-app-id'); + } + + @AsyncTest() + public async updateSelectOptionsSuccessfully() { + const settingUpdater = new SettingUpdater(this.mockProxiedApp, this.mockSettingsManager); + const newValues = [ + { key: 'key3', i18nLabel: 'value3' }, + { key: 'key4', i18nLabel: 'value4' }, + ]; + + await settingUpdater.updateSelectOptions('multiValue', newValues); + + Expect(this.mockSettingsManager.updateAppSetting).toHaveBeenCalled(); + const updatedValues = this.mockStorageItem.settings.multiValue.values; + // Should completely replace old values + Expect((updatedValues ?? []).length).toBe(2); + Expect(updatedValues).toEqual(newValues); + // Verify updatedAt was set + Expect(this.mockStorageItem.settings.multiValue.updatedAt).toBeDefined(); + } + + @AsyncTest() + public async updateSelectOptionsThrowsErrorForNonExistentSetting() { + const settingUpdater = new SettingUpdater(this.mockProxiedApp, this.mockSettingsManager); + + await Expect(() => settingUpdater.updateSelectOptions('nonExistent', [{ key: 'test', i18nLabel: 'value' }])).toThrowErrorAsync( + Error, + 'Setting "nonExistent" not found for app test-app-id', + ); + } +}