From 5f7adcc78647f3e1a1f29f5cb39cd652ff37cb4d Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Wed, 15 Sep 2021 08:31:59 +1000 Subject: [PATCH] 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. --- .../remind-assigns-frequency.js.es6 | 16 ++- .../components/remind-assigns-frequency.hbs | 1 + .../acceptance/assign-enabled-test.js.es6 | 105 ++++++++++++++++++ 3 files changed, 119 insertions(+), 3 deletions(-) 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 @@
{{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" + ); + }); + } +);