feat: Apps "select" setting updater (#35644)

Co-authored-by: Douglas Gubert <1810309+d-gubert@users.noreply.github.com>

.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.

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<void>;
+    updateSelectOptions(id: ISetting['id'], values: ISetting['values']): Promise<void>;
 }

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<void> {
+        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<void> {
+        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
+        });
+    }
 }

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',
+        );
+    }
+}