FIX: User option fields definition was being mutated on save (#15837)

In the commit d8bf2810ffe49619077371d854fc12cba81f80fa we hoisted
the userOptionFields array to a module-level variable, but kept
the code inside save() the same. This causes an issue where if
save() is called twice on the same user with some array of user
option fields, the userOptionFields array is mutated, which means
the second save is likely not saving the fields intended.

This commit fixes the issue by not mutating the array. We cannot
change them into consts though, because we have an API to add more
items to the array.

app/assets/javascripts/discourse/app/models/user.js

@@ -334,13 +334,16 @@ const User = RestModel.extend({
       userFields.filter((uf) => !fields || fields.indexOf(uf) !== -1)
     );
 
+    let filteredUserOptionFields = [];
     if (fields) {
-      userOptionFields = userOptionFields.filter(
+      filteredUserOptionFields = userOptionFields.filter(
         (uo) => fields.indexOf(uo) !== -1
       );
+    } else {
+      filteredUserOptionFields = userOptionFields;
     }
 
-    userOptionFields.forEach((s) => {
+    filteredUserOptionFields.forEach((s) => {
       data[s] = this.get(`user_option.${s}`);
     });
 
@@ -379,6 +382,10 @@ const User = RestModel.extend({
       }
     });
 
+    return this._saveUserData(data, updatedState);
+  },
+
+  _saveUserData(data, updatedState) {
     // TODO: We can remove this when migrated fully to rest model.
     this.set("isSaving", true);
     return ajax(userPath(`${this.username_lower}.json`), {

app/assets/javascripts/discourse/tests/acceptance/user-test.js

@@ -1,3 +1,7 @@
+import EmberObject from "@ember/object";
+import User from "discourse/models/user";
+import selectKit from "discourse/tests/helpers/select-kit-helper";
+import sinon from "sinon";
 import {
   acceptance,
   exists,
@@ -131,3 +135,45 @@ acceptance("User Routes - Moderator viewing warnings", function (needs) {
     assert.ok($("div.alert-info").length, "has the permissions alert");
   });
 });
+
+acceptance("User - Saving user options", function (needs) {
+  needs.user({
+    admin: false,
+    moderator: false,
+    username: "eviltrout",
+    id: 1,
+    user_option: EmberObject.create({}),
+  });
+
+  needs.settings({
+    disable_mailing_list_mode: false,
+  });
+
+  needs.pretender((server, helper) => {
+    server.put("/u/eviltrout.json", () => {
+      return helper.response(200, { user: {} });
+    });
+  });
+
+  test("saving user options", async function (assert) {
+    const spy = sinon.spy(User.current(), "_saveUserData");
+
+    await visit("/u/eviltrout/preferences/emails");
+    await click(".pref-mailing-list-mode input[type='checkbox']");
+    await click(".save-changes");
+
+    assert.ok(
+      spy.calledWithMatch({ mailing_list_mode: true }),
+      "sends a PUT request to update the specified user option"
+    );
+
+    await selectKit("#user-email-messages-level").expand();
+    await selectKit("#user-email-messages-level").selectRowByValue(2); // never option
+    await click(".save-changes");
+
+    assert.ok(
+      spy.calledWithMatch({ email_messages_level: 2 }),
+      "is able to save a different user_option on a subsequent request"
+    );
+  });
+});

app/assets/javascripts/discourse/tests/unit/models/user-test.js

@@ -1,8 +1,8 @@
 import * as ajaxlib from "discourse/lib/ajax";
 import { module, test } from "qunit";
+import sinon from "sinon";
 import Group from "discourse/models/group";
 import User from "discourse/models/user";
-import sinon from "sinon";
 
 module("Unit | Model | user", function () {
   test("staff", function (assert) {