FIX: Allow Never selection for frequency of assigned topic reminders (#204)

FIX: Allow Never selection for frequency of assigned topic reminders (#204)

If the user selected Never for their frequency of assigned topic reminder PMs, the option automatically set back to the default value (Monthly) because of the user of the or computed helper, which considered the 0 value of Never to be falsey.

This is the same problem that occurred in core which was fixed by https://github.com/discourse/discourse/commit/d3779d4cf75bd9c21cf61e016047a92fe66d3fdd

We need to be careful around using the or helper if any of the options could be considered falsey.

diff --git a/assets/javascripts/discourse/components/remind-assigns-frequency.js.es6 b/assets/javascripts/discourse/components/remind-assigns-frequency.js.es6
index 47aa8e7..09b478f 100644
--- a/assets/javascripts/discourse/components/remind-assigns-frequency.js.es6
+++ b/assets/javascripts/discourse/components/remind-assigns-frequency.js.es6
@@ -1,13 +1,23 @@
 import Component from "@ember/component";
 import I18n from "I18n";
-import { or } from "@ember/object/computed";
 import discourseComputed from "discourse-common/utils/decorators";
 
 export default Component.extend({
-  selectedFrequency: or(
+  @discourseComputed(
     "user.custom_fields.remind_assigns_frequency",
     "siteSettings.remind_assigns_frequency"
-  ),
+  )
+  selectedFrequency(userAssignsFrequency, siteDefaultAssignsFrequency) {
+    if (
+      this.availableFrequencies
+        .map((freq) => freq.value)
+        .includes(userAssignsFrequency)
+    ) {
+      return userAssignsFrequency;
+    }
+
+    return siteDefaultAssignsFrequency;
+  },
 
   @discourseComputed("user.reminders_frequency")
   availableFrequencies(userRemindersFrequency) {
diff --git a/assets/javascripts/discourse/templates/components/remind-assigns-frequency.hbs b/assets/javascripts/discourse/templates/components/remind-assigns-frequency.hbs
index 90e6180..d11c828 100644
--- a/assets/javascripts/discourse/templates/components/remind-assigns-frequency.hbs
+++ b/assets/javascripts/discourse/templates/components/remind-assigns-frequency.hbs
@@ -2,6 +2,7 @@
   <div class="controls controls-dropdown">
     <label>{{i18n "discourse_assign.reminders_frequency.description"}}</label>
     {{combo-box
+      id="remind-assigns-frequency"
       valueProperty="value"
       content=availableFrequencies
       value=selectedFrequency
diff --git a/test/javascripts/acceptance/assign-enabled-test.js.es6 b/test/javascripts/acceptance/assign-enabled-test.js.es6
index 0d7aaa1..aaa8161 100644
--- a/test/javascripts/acceptance/assign-enabled-test.js.es6
+++ b/test/javascripts/acceptance/assign-enabled-test.js.es6
@@ -1,4 +1,6 @@
 import selectKit from "discourse/tests/helpers/select-kit-helper";
+import { cloneJSON } from "discourse-common/lib/object";
+import userFixtures from "discourse/tests/fixtures/user-fixtures";
 import {
   acceptance,
   exists,
@@ -89,3 +91,106 @@ acceptance("Discourse Assign | Assign desktop", function (needs) {
     await click(".assign-suggestions .avatar");
   });
 });
+
+// See RemindAssignsFrequencySiteSettings
+const remindersFrequency = [
+  {
+    name: "discourse_assign.reminders_frequency.never",
+    value: 0,
+  },
+  {
+    name: "discourse_assign.reminders_frequency.daily",
+    value: 1440,
+  },
+  {
+    name: "discourse_assign.reminders_frequency.weekly",
+    value: 10080,
+  },
+  {
+    name: "discourse_assign.reminders_frequency.monthly",
+    value: 43200,
+  },
+  {
+    name: "discourse_assign.reminders_frequency.quarterly",
+    value: 129600,
+  },
+];
+
+acceptance("Discourse Assign | User preferences", function (needs) {
+  needs.user({ can_assign: true, reminders_frequency: remindersFrequency });
+  needs.settings({
+    assign_enabled: true,
+    remind_assigns_frequency: 43200,
+  });
+
+  test("The frequency for assigned topic reminders defaults to the site setting", async (assert) => {
+    await visit("/u/eviltrout/preferences/notifications");
+
+    assert.equal(
+      selectKit("#remind-assigns-frequency").header().value(),
+      "43200",
+      "set frequency to default of Monthly"
+    );
+  });
+
+  test("The user can change the frequency to Never", async (assert) => {
+    await visit("/u/eviltrout/preferences/notifications");
+
+    await selectKit("#remind-assigns-frequency").expand();
+    await selectKit("#remind-assigns-frequency").selectRowByValue(0);
+
+    assert.equal(
+      selectKit("#remind-assigns-frequency").header().value(),
+      "0",
+      "set frequency to Never"
+    );
+  });
+
+  test("The user can change the frequency to some other non-default value", async (assert) => {
+    await visit("/u/eviltrout/preferences/notifications");
+
+    await selectKit("#remind-assigns-frequency").expand();
+    await selectKit("#remind-assigns-frequency").selectRowByValue(10080); // weekly
+
+    assert.equal(
+      selectKit("#remind-assigns-frequency").header().value(),
+      "10080",
+      "set frequency to Weekly"
+    );
+  });
+});
+
+acceptance(
+  "Discourse Assign | User preferences | Pre-selected reminder frequency",
+  function (needs) {
+    needs.user({ can_assign: true, reminders_frequency: remindersFrequency });
+    needs.settings({
+      assign_enabled: true,
+      remind_assigns_frequency: 43200,
+    });
+
+    needs.pretender((server, helper) => {
+      server.get("/u/eviltrout.json", () => {
+        let json = cloneJSON(userFixtures["/u/eviltrout.json"]);
+        json.user.custom_fields = { remind_assigns_frequency: 10080 };
+
+        // usually this is done automatically by this pretender but we
+        // have to do it manually here because we are overriding the
+        // pretender see app/assets/javascripts/discourse/tests/helpers/create-pretender.js
+        json.user.can_edit = true;
+
+        return helper.response(200, json);
+      });
+    });
+
+    test("The user's previously selected value is loaded", async (assert) => {
+      await visit("/u/eviltrout/preferences/notifications");
+
+      assert.equal(
+        selectKit("#remind-assigns-frequency").header().value(),
+        "10080",
+        "frequency is pre-selected to Weekly"
+      );
+    });
+  }
+);

GitHub sha: 5f7adcc78647f3e1a1f29f5cb39cd652ff37cb4d

This commit appears in #204 which was approved by romanrizzi. It was merged by martin.