From 1dba7e4aa49cd169ee7f80468f50fff2cc25229e Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 14 Apr 2025 15:40:25 +0100 Subject: [PATCH] Ensure we do not set the account data twice. --- .../handlers/AccountSettingsHandler.ts | 6 ++-- .../handlers/RoomAccountSettingsHandler.ts | 6 ++-- .../MediaPreviewAccountSettingsTab-test.tsx | 29 ++++++++++++++++--- 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/src/settings/handlers/AccountSettingsHandler.ts b/src/settings/handlers/AccountSettingsHandler.ts index d1a307f089..3536169c59 100644 --- a/src/settings/handlers/AccountSettingsHandler.ts +++ b/src/settings/handlers/AccountSettingsHandler.ts @@ -176,7 +176,7 @@ export default class AccountSettingsHandler extends MatrixClientBackedSettingsHa await deferred.promise; } - public setValue(settingName: string, roomId: string, newValue: any): Promise { + public async setValue(settingName: string, roomId: string, newValue: any): Promise { switch (settingName) { // Special case URL previews case "urlPreviewsEnabled": @@ -202,7 +202,9 @@ export default class AccountSettingsHandler extends MatrixClientBackedSettingsHa // Special case analytics case "pseudonymousAnalyticsOptIn": return this.setAccountData(ANALYTICS_EVENT_TYPE, "pseudonymousAnalyticsOptIn", newValue); - + case "mediaPreviewConfig": + // Handled in MediaPreviewConfigController. + return; default: return this.setAccountData(DEFAULT_SETTINGS_EVENT_TYPE, settingName, newValue); } diff --git a/src/settings/handlers/RoomAccountSettingsHandler.ts b/src/settings/handlers/RoomAccountSettingsHandler.ts index be21f1d605..e98cb3725d 100644 --- a/src/settings/handlers/RoomAccountSettingsHandler.ts +++ b/src/settings/handlers/RoomAccountSettingsHandler.ts @@ -111,7 +111,7 @@ export default class RoomAccountSettingsHandler extends MatrixClientBackedSettin await deferred.promise; } - public setValue(settingName: string, roomId: string, newValue: any): Promise { + public async setValue(settingName: string, roomId: string, newValue: any): Promise { switch (settingName) { // Special case URL previews case "urlPreviewsEnabled": @@ -120,7 +120,9 @@ export default class RoomAccountSettingsHandler extends MatrixClientBackedSettin // Special case allowed widgets case "allowedWidgets": return this.setRoomAccountData(roomId, ALLOWED_WIDGETS_EVENT_TYPE, null, newValue); - + case "mediaPreviewConfig": + // Handled in MediaPreviewConfigController. + return; default: return this.setRoomAccountData(roomId, DEFAULT_SETTINGS_EVENT_TYPE, settingName, newValue); } diff --git a/test/unit-tests/components/views/settings/tabs/user/MediaPreviewAccountSettingsTab-test.tsx b/test/unit-tests/components/views/settings/tabs/user/MediaPreviewAccountSettingsTab-test.tsx index 2a9bc371e6..c79f963dc3 100644 --- a/test/unit-tests/components/views/settings/tabs/user/MediaPreviewAccountSettingsTab-test.tsx +++ b/test/unit-tests/components/views/settings/tabs/user/MediaPreviewAccountSettingsTab-test.tsx @@ -18,8 +18,9 @@ import { import MatrixClientBackedController from "../../../../../../../src/settings/controllers/MatrixClientBackedController"; import MatrixClientBackedSettingsHandler from "../../../../../../../src/settings/handlers/MatrixClientBackedSettingsHandler"; import type { MockedObject } from "jest-mock"; -import type { MatrixClient } from "matrix-js-sdk/src/client"; -import { MEDIA_PREVIEW_ACCOUNT_DATA_TYPE, MediaPreviewValue } from "../../../../../../../src/@types/media_preview"; +import { type MatrixClient, MatrixEvent } from "matrix-js-sdk/src/matrix"; +import { MEDIA_PREVIEW_ACCOUNT_DATA_TYPE, MediaPreviewConfig, MediaPreviewValue } from "../../../../../../../src/@types/media_preview"; +import MediaPreviewConfigController from "../../../../../../../src/settings/controllers/MediaPreviewConfigController"; describe("MediaPreviewAccountSettings", () => { let client: MockedObject; @@ -57,20 +58,40 @@ describe("MediaPreviewAccountSettings", () => { invite_avatars: MediaPreviewValue.Off, media_previews: MediaPreviewValue.On, }); + // Ensure we don't double set the account data. + expect(client.setAccountData).toHaveBeenCalledTimes(1); }); // Skip the default. it.each([ ["Always hide", MediaPreviewValue.Off], ["In private rooms", MediaPreviewValue.Private], - ])("should be able to toggle media preview %s", async (key, value) => { + ["Always show", MediaPreviewValue.On], + ])("should be able to toggle media preview option %s", async (key, value) => { + if (value === MediaPreviewConfigController.default.media_previews) { + // This is the default, so switch away first. + client.getAccountData.mockImplementation((type) => { + if (type === MEDIA_PREVIEW_ACCOUNT_DATA_TYPE) { + return new MatrixEvent({ + content: { + media_previews: MediaPreviewValue.Off + } satisfies Partial + }); + } + return undefined; + }) + } const { getByLabelText } = render(); - // Defaults + const element = getByLabelText(key); await userEvent.click(element); expect(client.setAccountData).toHaveBeenCalledWith(MEDIA_PREVIEW_ACCOUNT_DATA_TYPE, { invite_avatars: MediaPreviewValue.On, media_previews: value, }); + // Ensure we don't double set the account data. + expect(client.setAccountData).toHaveBeenCalledTimes(1); }); + + });