FIX: allows to change sound when no sound was ever set (#19136)

It fixes a bug which was only allowing users with a sound to change it. Users with `none` could not change it again after a full page reset.

plugins/chat/assets/javascripts/discourse/controllers/preferences-chat.js

@@ -34,7 +34,7 @@ export default class PreferencesChatController extends Controller {
 
   @action
   onChangeChatSound(sound) {
-    if (sound && !isTesting()) {
+    if (sound) {
       this.chatAudioManager.playImmediately(sound);
     }
     this.model.set("user_option.chat_sound", sound);

plugins/chat/assets/javascripts/discourse/initializers/chat-audio.js

@@ -8,10 +8,9 @@ export default {
   name: "chat-audio",
 
   initialize(container) {
-    const currentUser = container.lookup("service:current-user");
     const chatService = container.lookup("service:chat");
 
-    if (!chatService.userCanChat || !currentUser?.chat_sound) {
+    if (!chatService.userCanChat) {
       return;
     }
 
@@ -20,6 +19,10 @@ export default {
 
     withPluginApi("0.12.1", (api) => {
       api.registerDesktopNotificationHandler((data, siteSettings, user) => {
+        if (!user.chat_sound) {
+          return;
+        }
+
         if (CHAT_NOTIFICATION_TYPES.includes(data.notification_type)) {
           chatAudioManager.play(user.chat_sound);
         }

plugins/chat/assets/javascripts/discourse/services/chat-audio-manager.js

@@ -1,5 +1,6 @@
 import Service from "@ember/service";
 import { debounce } from "discourse-common/utils/decorators";
+import { isTesting } from "discourse-common/config/environment";
 
 const AUDIO_DEBOUNCE_DELAY = 3000;
 
@@ -49,6 +50,8 @@ export default class ChatAudioManager extends Service {
     const audio =
       this._audioCache[soundName] || this._audioCache[DEFAULT_SOUND_NAME];
 
+    audio.muted = isTesting();
+
     if (!audio.paused) {
       audio.pause();
       if (typeof audio.fastSeek === "function") {
@@ -59,10 +62,12 @@ export default class ChatAudioManager extends Service {
     }
 
     return audio.play().catch(() => {
+      if (!isTesting()) {
         // eslint-disable-next-line no-console
         console.info(
           "[chat] User needs to interact with DOM before we can play notification sounds."
         );
+      }
     });
   }
 }

plugins/chat/test/javascripts/acceptance/chat-preferences-test.js

@@ -0,0 +1,34 @@
+import { acceptance } from "discourse/tests/helpers/qunit-helpers";
+import { visit } from "@ember/test-helpers";
+import { test } from "qunit";
+import { chatChannels } from "discourse/plugins/chat/chat-fixtures";
+import selectKit from "discourse/tests/helpers/select-kit-helper";
+import { CHAT_SOUNDS } from "discourse/plugins/chat/discourse/services/chat-audio-manager";
+
+function preferencesPretender(server, helper) {
+  server.get("/u/eviltrout/activity.json", () => helper.response({}));
+  server.get("/chat/chat_channels.json", () => helper.response(chatChannels));
+}
+
+acceptance("Discourse Chat | User Preferences", function (needs) {
+  needs.user({ can_chat: true, has_chat_enabled: true });
+  needs.settings({ chat_enabled: true });
+  needs.pretender(preferencesPretender);
+
+  test("when user has not chat sound set", async function (assert) {
+    const sounds = Object.keys(CHAT_SOUNDS);
+    await visit("/u/eviltrout/preferences/chat");
+    const dropdown = selectKit("#user_chat_sounds");
+
+    assert.strictEqual(dropdown.header().value(), null, "it displays no sound");
+
+    await dropdown.expand();
+    await dropdown.selectRowByValue(sounds[1]);
+
+    assert.strictEqual(
+      dropdown.header().value(),
+      sounds[1],
+      "it selects the sound"
+    );
+  });
+});