From 4f140f174920b810b9342a0fbba4a7cbc8d99485 Mon Sep 17 00:00:00 2001 From: Baki Burak Ogun <63836730+bakiburakogun@users.noreply.github.com> Date: Thu, 3 Sep 2026 07:34:47 +0300 Subject: [PATCH 1/2] fix(sounds): fall back when the server has no play-sounds capability Since Talk 24 the sounds store takes the user's "play sounds" setting from the capabilities (config.call.play-sounds). Servers before Talk 24 don't expose it there, so against such a server getTalkConfig() returns undefined, sounds are off after every start and the toggle in the settings dialog doesn't stick. Read the capability once. Guests keep what this browser remembered, otherwise the capability decides; without it, fall back to the value in browser storage and finally to enabled. On change, pass the capability along to setPlaySounds: a server that cannot hand the value back is not written to either, and the setting stays in the browser. Ref nextcloud/talk-desktop#1087 Signed-off-by: Baki Burak Ogun <63836730+bakiburakogun@users.noreply.github.com> --- src/stores/__tests__/sounds.spec.js | 124 ++++++++++++++++++++++++++++ src/stores/sounds.js | 32 ++++--- 2 files changed, 146 insertions(+), 10 deletions(-) create mode 100644 src/stores/__tests__/sounds.spec.js diff --git a/src/stores/__tests__/sounds.spec.js b/src/stores/__tests__/sounds.spec.js new file mode 100644 index 00000000000..a75c2e6784a --- /dev/null +++ b/src/stores/__tests__/sounds.spec.js @@ -0,0 +1,124 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { getCurrentUser } from '@nextcloud/auth' +import { createPinia, setActivePinia } from 'pinia' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import BrowserStorage from '../../services/BrowserStorage.js' +import { getTalkConfig } from '../../services/CapabilitiesManager.ts' +import { setPlaySounds } from '../../services/settingsService.ts' + +vi.mock('@nextcloud/auth', () => ({ + getCurrentUser: vi.fn(), +})) +vi.mock('../../services/BrowserStorage.js', () => ({ + default: { + getItem: vi.fn(), + setItem: vi.fn(), + }, +})) +vi.mock('../../services/CapabilitiesManager.ts', () => ({ + getTalkConfig: vi.fn(), +})) +vi.mock('../../services/settingsService.ts', () => ({ + setPlaySounds: vi.fn(() => Promise.resolve()), +})) + +/** + * The initial value is computed when the module is loaded, so load it fresh for every test + */ +async function loadSoundsStore() { + vi.resetModules() + const { useSoundsStore } = await import('../sounds.js') + setActivePinia(createPinia()) + return useSoundsStore() +} + +describe('soundsStore', () => { + beforeEach(() => { + vi.clearAllMocks() + getCurrentUser.mockReturnValue({ uid: 'alice' }) + getTalkConfig.mockReturnValue(undefined) + BrowserStorage.getItem.mockReturnValue(null) + }) + + describe('initial value for users', () => { + it('takes the value from the capabilities on Talk 24+', async () => { + getTalkConfig.mockReturnValue(false) + const store = await loadSoundsStore() + expect(store.shouldPlaySounds).toBe(false) + expect(getTalkConfig).toHaveBeenCalledWith('local', 'call', 'play-sounds') + }) + + it('prefers the capabilities over browser storage', async () => { + getTalkConfig.mockReturnValue(true) + BrowserStorage.getItem.mockReturnValue('no') + const store = await loadSoundsStore() + expect(store.shouldPlaySounds).toBe(true) + }) + + it('falls back to browser storage when the server has no capability', async () => { + BrowserStorage.getItem.mockReturnValue('no') + const store = await loadSoundsStore() + expect(store.shouldPlaySounds).toBe(false) + }) + + it('defaults to enabled without capability or storage', async () => { + const store = await loadSoundsStore() + expect(store.shouldPlaySounds).toBe(true) + }) + }) + + describe('initial value for guests', () => { + beforeEach(() => { + getCurrentUser.mockReturnValue(null) + }) + + it('prefers browser storage over the capabilities', async () => { + getTalkConfig.mockReturnValue(true) + BrowserStorage.getItem.mockReturnValue('no') + const store = await loadSoundsStore() + expect(store.shouldPlaySounds).toBe(false) + }) + + it('takes the value from the capabilities without storage', async () => { + getTalkConfig.mockReturnValue(false) + const store = await loadSoundsStore() + expect(store.shouldPlaySounds).toBe(false) + }) + + it('defaults to enabled on older servers', async () => { + const store = await loadSoundsStore() + expect(store.shouldPlaySounds).toBe(true) + }) + }) + + describe('setShouldPlaySounds', () => { + it('saves on the server only when the capability exists', async () => { + getTalkConfig.mockReturnValue(true) + const store = await loadSoundsStore() + await store.setShouldPlaySounds(false) + expect(setPlaySounds).toHaveBeenCalledWith(true, 'no') + expect(BrowserStorage.setItem).not.toHaveBeenCalled() + expect(store.shouldPlaySounds).toBe(false) + }) + + it('leaves the value to browser storage when the server has no capability', async () => { + const store = await loadSoundsStore() + await store.setShouldPlaySounds(false) + expect(setPlaySounds).toHaveBeenCalledWith(false, 'no') + expect(BrowserStorage.setItem).not.toHaveBeenCalled() + expect(store.shouldPlaySounds).toBe(false) + }) + + it('saves guests to browser storage through the settings service only', async () => { + getCurrentUser.mockReturnValue(null) + const store = await loadSoundsStore() + await store.setShouldPlaySounds(true) + expect(setPlaySounds).toHaveBeenCalledWith(false, 'yes') + expect(BrowserStorage.setItem).not.toHaveBeenCalled() + }) + }) +}) diff --git a/src/stores/sounds.js b/src/stores/sounds.js index 24dd0e2a350..8bb4db86719 100644 --- a/src/stores/sounds.js +++ b/src/stores/sounds.js @@ -11,20 +11,32 @@ import { getTalkConfig } from '../services/CapabilitiesManager.ts' import { setPlaySounds } from '../services/settingsService.ts' const hasUserAccount = Boolean(getCurrentUser()?.uid) +const playSoundsCapability = getTalkConfig('local', 'call', 'play-sounds') +const hasPlaySoundsCapability = playSoundsCapability !== undefined + /** - * Get play sounds option (from server for user or from browser storage for guest) + * Get play sounds option. A guest keeps whatever this browser remembered, otherwise the + * capability decides. Servers before Talk 24 don't hand the value out at all, so fall back + * to the browser and then to enabled. + * + * @return {boolean} */ -let shouldPlaySounds = false -if (hasUserAccount) { - shouldPlaySounds = getTalkConfig('local', 'call', 'play-sounds') -} else { - if (BrowserStorage.getItem('play_sounds')) { - shouldPlaySounds = BrowserStorage.getItem('play_sounds') !== 'no' - } else { - shouldPlaySounds = getTalkConfig('local', 'call', 'play-sounds') +function getInitialShouldPlaySounds() { + const fromStorage = BrowserStorage.getItem('play_sounds') + + if (!hasUserAccount && fromStorage) { + return fromStorage !== 'no' + } + + if (hasPlaySoundsCapability) { + return playSoundsCapability } + + return fromStorage ? fromStorage !== 'no' : true } +const shouldPlaySounds = getInitialShouldPlaySounds() + /** * Preferred version is the .ogg, with .flac fallback if .ogg is not supported (Safari) */ @@ -55,7 +67,7 @@ export const useSoundsStore = defineStore('sounds', { * @param {boolean} value whether sounds should be played */ async setShouldPlaySounds(value) { - await setPlaySounds(hasUserAccount, value ? 'yes' : 'no') + await setPlaySounds(hasUserAccount && hasPlaySoundsCapability, value ? 'yes' : 'no') this.shouldPlaySounds = value }, From cac92a9ac76a32aae532beaf91e8c35041e4206e Mon Sep 17 00:00:00 2001 From: Maksim Sukharev Date: Sat, 5 Sep 2026 10:16:36 +0200 Subject: [PATCH 2/2] fix(sounds): adjust test scenarios - setPlaySounds mock was unnecessary, since it skipped BrowserStorage call - mock axios.post instead Assisted-by: ClaudeCode:claude-sonnet-5 Signed-off-by: Maksim Sukharev --- src/stores/__tests__/sounds.spec.js | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/stores/__tests__/sounds.spec.js b/src/stores/__tests__/sounds.spec.js index a75c2e6784a..48ed851198e 100644 --- a/src/stores/__tests__/sounds.spec.js +++ b/src/stores/__tests__/sounds.spec.js @@ -4,15 +4,21 @@ */ import { getCurrentUser } from '@nextcloud/auth' +import axios from '@nextcloud/axios' +import { generateOcsUrl } from '@nextcloud/router' import { createPinia, setActivePinia } from 'pinia' import { beforeEach, describe, expect, it, vi } from 'vitest' import BrowserStorage from '../../services/BrowserStorage.js' import { getTalkConfig } from '../../services/CapabilitiesManager.ts' -import { setPlaySounds } from '../../services/settingsService.ts' vi.mock('@nextcloud/auth', () => ({ getCurrentUser: vi.fn(), })) +vi.mock('@nextcloud/axios', () => ({ + default: { + post: vi.fn(() => Promise.resolve()), + }, +})) vi.mock('../../services/BrowserStorage.js', () => ({ default: { getItem: vi.fn(), @@ -22,9 +28,6 @@ vi.mock('../../services/BrowserStorage.js', () => ({ vi.mock('../../services/CapabilitiesManager.ts', () => ({ getTalkConfig: vi.fn(), })) -vi.mock('../../services/settingsService.ts', () => ({ - setPlaySounds: vi.fn(() => Promise.resolve()), -})) /** * The initial value is computed when the module is loaded, so load it fresh for every test @@ -100,25 +103,28 @@ describe('soundsStore', () => { getTalkConfig.mockReturnValue(true) const store = await loadSoundsStore() await store.setShouldPlaySounds(false) - expect(setPlaySounds).toHaveBeenCalledWith(true, 'no') + expect(axios.post).toHaveBeenCalledWith( + generateOcsUrl('apps/spreed/api/v1/settings/user'), + { key: 'play_sounds', value: 'no' }, + ) expect(BrowserStorage.setItem).not.toHaveBeenCalled() expect(store.shouldPlaySounds).toBe(false) }) - it('leaves the value to browser storage when the server has no capability', async () => { + it('saves to browser storage instead on older servers, since they cannot hand it back', async () => { const store = await loadSoundsStore() await store.setShouldPlaySounds(false) - expect(setPlaySounds).toHaveBeenCalledWith(false, 'no') - expect(BrowserStorage.setItem).not.toHaveBeenCalled() + expect(axios.post).not.toHaveBeenCalled() + expect(BrowserStorage.setItem).toHaveBeenCalledWith('play_sounds', 'no') expect(store.shouldPlaySounds).toBe(false) }) - it('saves guests to browser storage through the settings service only', async () => { + it('saves guests to browser storage', async () => { getCurrentUser.mockReturnValue(null) const store = await loadSoundsStore() await store.setShouldPlaySounds(true) - expect(setPlaySounds).toHaveBeenCalledWith(false, 'yes') - expect(BrowserStorage.setItem).not.toHaveBeenCalled() + expect(axios.post).not.toHaveBeenCalled() + expect(BrowserStorage.setItem).toHaveBeenCalledWith('play_sounds', 'yes') }) }) })