Skip to content

Commit fc22ff9

Browse files
committed
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>
1 parent 0c55e42 commit fc22ff9

2 files changed

Lines changed: 146 additions & 10 deletions

File tree

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
/**
2+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
3+
* SPDX-License-Identifier: AGPL-3.0-or-later
4+
*/
5+
6+
import { getCurrentUser } from '@nextcloud/auth'
7+
import { createPinia, setActivePinia } from 'pinia'
8+
import { beforeEach, describe, expect, it, vi } from 'vitest'
9+
import BrowserStorage from '../../services/BrowserStorage.js'
10+
import { getTalkConfig } from '../../services/CapabilitiesManager.ts'
11+
import { setPlaySounds } from '../../services/settingsService.ts'
12+
13+
vi.mock('@nextcloud/auth', () => ({
14+
getCurrentUser: vi.fn(),
15+
}))
16+
vi.mock('../../services/BrowserStorage.js', () => ({
17+
default: {
18+
getItem: vi.fn(),
19+
setItem: vi.fn(),
20+
},
21+
}))
22+
vi.mock('../../services/CapabilitiesManager.ts', () => ({
23+
getTalkConfig: vi.fn(),
24+
}))
25+
vi.mock('../../services/settingsService.ts', () => ({
26+
setPlaySounds: vi.fn(() => Promise.resolve()),
27+
}))
28+
29+
/**
30+
* The initial value is computed when the module is loaded, so load it fresh for every test
31+
*/
32+
async function loadSoundsStore() {
33+
vi.resetModules()
34+
const { useSoundsStore } = await import('../sounds.js')
35+
setActivePinia(createPinia())
36+
return useSoundsStore()
37+
}
38+
39+
describe('soundsStore', () => {
40+
beforeEach(() => {
41+
vi.clearAllMocks()
42+
getCurrentUser.mockReturnValue({ uid: 'alice' })
43+
getTalkConfig.mockReturnValue(undefined)
44+
BrowserStorage.getItem.mockReturnValue(null)
45+
})
46+
47+
describe('initial value for users', () => {
48+
it('takes the value from the capabilities on Talk 24+', async () => {
49+
getTalkConfig.mockReturnValue(false)
50+
const store = await loadSoundsStore()
51+
expect(store.shouldPlaySounds).toBe(false)
52+
expect(getTalkConfig).toHaveBeenCalledWith('local', 'call', 'play-sounds')
53+
})
54+
55+
it('prefers the capabilities over browser storage', async () => {
56+
getTalkConfig.mockReturnValue(true)
57+
BrowserStorage.getItem.mockReturnValue('no')
58+
const store = await loadSoundsStore()
59+
expect(store.shouldPlaySounds).toBe(true)
60+
})
61+
62+
it('falls back to browser storage when the server has no capability', async () => {
63+
BrowserStorage.getItem.mockReturnValue('no')
64+
const store = await loadSoundsStore()
65+
expect(store.shouldPlaySounds).toBe(false)
66+
})
67+
68+
it('defaults to enabled without capability or storage', async () => {
69+
const store = await loadSoundsStore()
70+
expect(store.shouldPlaySounds).toBe(true)
71+
})
72+
})
73+
74+
describe('initial value for guests', () => {
75+
beforeEach(() => {
76+
getCurrentUser.mockReturnValue(null)
77+
})
78+
79+
it('prefers browser storage over the capabilities', async () => {
80+
getTalkConfig.mockReturnValue(true)
81+
BrowserStorage.getItem.mockReturnValue('no')
82+
const store = await loadSoundsStore()
83+
expect(store.shouldPlaySounds).toBe(false)
84+
})
85+
86+
it('takes the value from the capabilities without storage', async () => {
87+
getTalkConfig.mockReturnValue(false)
88+
const store = await loadSoundsStore()
89+
expect(store.shouldPlaySounds).toBe(false)
90+
})
91+
92+
it('defaults to enabled on older servers', async () => {
93+
const store = await loadSoundsStore()
94+
expect(store.shouldPlaySounds).toBe(true)
95+
})
96+
})
97+
98+
describe('setShouldPlaySounds', () => {
99+
it('saves on the server only when the capability exists', async () => {
100+
getTalkConfig.mockReturnValue(true)
101+
const store = await loadSoundsStore()
102+
await store.setShouldPlaySounds(false)
103+
expect(setPlaySounds).toHaveBeenCalledWith(true, 'no')
104+
expect(BrowserStorage.setItem).not.toHaveBeenCalled()
105+
expect(store.shouldPlaySounds).toBe(false)
106+
})
107+
108+
it('leaves the value to browser storage when the server has no capability', async () => {
109+
const store = await loadSoundsStore()
110+
await store.setShouldPlaySounds(false)
111+
expect(setPlaySounds).toHaveBeenCalledWith(false, 'no')
112+
expect(BrowserStorage.setItem).not.toHaveBeenCalled()
113+
expect(store.shouldPlaySounds).toBe(false)
114+
})
115+
116+
it('saves guests to browser storage through the settings service only', async () => {
117+
getCurrentUser.mockReturnValue(null)
118+
const store = await loadSoundsStore()
119+
await store.setShouldPlaySounds(true)
120+
expect(setPlaySounds).toHaveBeenCalledWith(false, 'yes')
121+
expect(BrowserStorage.setItem).not.toHaveBeenCalled()
122+
})
123+
})
124+
})

src/stores/sounds.js

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,32 @@ import { getTalkConfig } from '../services/CapabilitiesManager.ts'
1111
import { setPlaySounds } from '../services/settingsService.ts'
1212

1313
const hasUserAccount = Boolean(getCurrentUser()?.uid)
14+
const playSoundsCapability = getTalkConfig('local', 'call', 'play-sounds')
15+
const hasPlaySoundsCapability = playSoundsCapability !== undefined
16+
1417
/**
15-
* Get play sounds option (from server for user or from browser storage for guest)
18+
* Get play sounds option. A guest keeps whatever this browser remembered, otherwise the
19+
* capability decides. Servers before Talk 24 don't hand the value out at all, so fall back
20+
* to the browser and then to enabled.
21+
*
22+
* @return {boolean}
1623
*/
17-
let shouldPlaySounds = false
18-
if (hasUserAccount) {
19-
shouldPlaySounds = getTalkConfig('local', 'call', 'play-sounds')
20-
} else {
21-
if (BrowserStorage.getItem('play_sounds')) {
22-
shouldPlaySounds = BrowserStorage.getItem('play_sounds') !== 'no'
23-
} else {
24-
shouldPlaySounds = getTalkConfig('local', 'call', 'play-sounds')
24+
function getInitialShouldPlaySounds() {
25+
const fromStorage = BrowserStorage.getItem('play_sounds')
26+
27+
if (!hasUserAccount && fromStorage) {
28+
return fromStorage !== 'no'
29+
}
30+
31+
if (hasPlaySoundsCapability) {
32+
return playSoundsCapability
2533
}
34+
35+
return fromStorage ? fromStorage !== 'no' : true
2636
}
2737

38+
const shouldPlaySounds = getInitialShouldPlaySounds()
39+
2840
/**
2941
* Preferred version is the .ogg, with .flac fallback if .ogg is not supported (Safari)
3042
*/
@@ -55,7 +67,7 @@ export const useSoundsStore = defineStore('sounds', {
5567
* @param {boolean} value whether sounds should be played
5668
*/
5769
async setShouldPlaySounds(value) {
58-
await setPlaySounds(hasUserAccount, value ? 'yes' : 'no')
70+
await setPlaySounds(hasUserAccount && hasPlaySoundsCapability, value ? 'yes' : 'no')
5971
this.shouldPlaySounds = value
6072
},
6173

0 commit comments

Comments
 (0)