From 6a103d8a5db59f525675cdd7456c6784961cc4cd 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] 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. Keep the capability as the first source and fall back to the value remembered in browser storage, then to the initial state older servers still provide, and finally to enabled. When the capability is missing, also mirror the setting to browser storage on change so the next start can pick it up. Ref nextcloud/talk-desktop#1087 Signed-off-by: Baki Burak Ogun <63836730+bakiburakogun@users.noreply.github.com> --- src/stores/__tests__/sounds.spec.js | 137 ++++++++++++++++++++++++++++ src/stores/sounds.js | 45 +++++++-- 2 files changed, 173 insertions(+), 9 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..0743b8e7f90 --- /dev/null +++ b/src/stores/__tests__/sounds.spec.js @@ -0,0 +1,137 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { getCurrentUser } from '@nextcloud/auth' +import { loadState } from '@nextcloud/initial-state' +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/initial-state', () => ({ + loadState: vi.fn((app, key, fallback) => fallback), +})) +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) + loadState.mockImplementation((app, key, fallback) => fallback) + }) + + 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('falls back to the initial state when there is neither capability nor storage', async () => { + loadState.mockReturnValue(false) + const store = await loadSoundsStore() + expect(store.shouldPlaySounds).toBe(false) + expect(loadState).toHaveBeenCalledWith('spreed', 'play_sounds', true) + }) + + it('defaults to enabled', 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('falls back to the initial state on older servers', async () => { + loadState.mockReturnValue(false) + const store = await loadSoundsStore() + expect(store.shouldPlaySounds).toBe(false) + }) + }) + + 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('also remembers the value in browser storage on older servers', async () => { + const store = await loadSoundsStore() + await store.setShouldPlaySounds(false) + expect(setPlaySounds).toHaveBeenCalledWith(true, 'no') + expect(BrowserStorage.setItem).toHaveBeenCalledWith('play_sounds', 'no') + 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..9a1f1f2a55c 100644 --- a/src/stores/sounds.js +++ b/src/stores/sounds.js @@ -4,6 +4,7 @@ */ import { getCurrentUser } from '@nextcloud/auth' +import { loadState } from '@nextcloud/initial-state' import { generateFilePath } from '@nextcloud/router' import { defineStore } from 'pinia' import BrowserStorage from '../services/BrowserStorage.js' @@ -11,20 +12,42 @@ import { getTalkConfig } from '../services/CapabilitiesManager.ts' import { setPlaySounds } from '../services/settingsService.ts' const hasUserAccount = Boolean(getCurrentUser()?.uid) + +/** + * Whether the server hands out the "play sounds" setting in the capabilities (Talk 24+) + * + * @return {boolean} + */ +function hasPlaySoundsCapability() { + return getTalkConfig('local', 'call', 'play-sounds') !== undefined +} + /** - * Get play sounds option (from server for user or from browser storage for guest) + * Get play sounds option: from the capabilities for users, from browser storage for guests. + * Servers before Talk 24 don't expose the value in the capabilities, so fall back to what was + * remembered in this browser, then to the initial state the web page still provides, and finally 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 getTalkConfig('local', 'call', 'play-sounds') + } + + if (fromStorage) { + return fromStorage !== 'no' } + + return loadState('spreed', 'play_sounds', true) } +const shouldPlaySounds = getInitialShouldPlaySounds() + /** * Preferred version is the .ogg, with .flac fallback if .ogg is not supported (Safari) */ @@ -56,6 +79,10 @@ export const useSoundsStore = defineStore('sounds', { */ async setShouldPlaySounds(value) { await setPlaySounds(hasUserAccount, value ? 'yes' : 'no') + if (hasUserAccount && !hasPlaySoundsCapability()) { + // Server can't hand the value back via capabilities, so remember it here as well + BrowserStorage.setItem('play_sounds', value ? 'yes' : 'no') + } this.shouldPlaySounds = value },