github-mirror/discourse
github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-03-06 09:35:36 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

FIX: don't allow category and tag tracking settings on staged users (#13688)

Browse Source 
Configuring staged users to watch categories and tags is a way to sign
them up to get many emails. These emails may be unwanted and get marked
as spam, hurting the site's email deliverability.
Users can opt-in to email notifications by logging on to their
account and configuring their own preferences.

If staff need to be able to configure these preferences on behalf of
staged users, the "allow changing staged user tracking" site setting
can be enabled. Default is to not allow it.

Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
This commit is contained in:
Neil Lalonde 2021-07-16 14:50:40 -04:00 committed by GitHub
parent e12b00eab7
commit b0f06b8ed0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 262 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
app/assets/javascripts/discourse/app/controllers/preferences.js
View File

@ -1,2 +1,3 @@
import Controller from "@ember/controller";
export default Controller.extend({});

14
app/assets/javascripts/discourse/app/templates/preferences.hbs
View File

@ -25,17 +25,19 @@
{{i18n "user.preferences_nav.notifications"}}
{{/link-to}}
</li>
<li class="indent nav-categories">
{{#link-to "preferences.categories"}}
{{i18n "user.preferences_nav.categories"}}
{{/link-to}}
</li>
{{#if model.can_change_tracking_preferences}}
<li class="indent nav-categories">
{{#link-to "preferences.categories"}}
{{i18n "user.preferences_nav.categories"}}
{{/link-to}}
</li>
{{/if}}
<li class="indent nav-users">
{{#link-to "preferences.users"}}
{{i18n "user.preferences_nav.users"}}
{{/link-to}}
</li>
{{#if siteSettings.tagging_enabled}}
{{#if (and model.can_change_tracking_preferences siteSettings.tagging_enabled)}}
<li class="indent nav-tags">
{{#link-to "preferences.tags"}}
{{i18n "user.preferences_nav.tags"}}

33
app/assets/javascripts/discourse/tests/acceptance/preferences-test.js
View File

@ -524,3 +524,36 @@ acceptance("Security", function (needs) {
);
});
});
acceptance(
"User Preferences for staged user and don't allow tracking prefs",
function (needs) {
needs.settings({
allow_changing_staged_user_tracking: false,
tagging_enabled: true,
});
needs.pretender(preferencesPretender);
test("staged user doesn't show category and tag preferences", async function (assert) {
await visit("/u/staged/preferences");
assert.ok($("body.user-preferences-page").length, "has the body class");
assert.equal(
currentURL(),
"/u/staged/preferences/account",
"defaults to account tab"
);
assert.ok(exists(".user-preferences"), "it shows the preferences");
assert.ok(
!exists(".preferences-nav .nav-categories a"),
"categories tab isn't there for staged users"
);
assert.ok(
!exists(".preferences-nav .nav-tags a"),
"tags tab isn't there for staged users"
);
});
}
);

125
app/assets/javascripts/discourse/tests/fixtures/user-fixtures.js vendored
View File

@ -190,6 +190,7 @@ export default {
card_image_badge_id: 120,
muted_usernames: [],
can_change_location: true,
can_change_tracking_preferences: true,
ignored_usernames: [],
invited_by: {
id: 1,
@ -2534,6 +2535,7 @@ export default {
ignored_usernames: [],
mailing_list_posts_per_day: 0,
can_change_bio: true,
can_change_tracking_preferences: true,
user_api_keys: null,
user_auth_tokens: [],
invited_by: null,
@ -3322,4 +3324,127 @@ export default {
timezone: "Australia/Brisbane",
},
},
"/u/staged.json": {
user_badges: [],
badges: [],
badge_types: [
{ id: 1, name: "Gold", sort_order: 9 },
{ id: 2, name: "Silver", sort_order: 8 },
{ id: 3, name: "Bronze", sort_order: 7 },
],
users: [
{
id: 20,
username: "staged",
uploaded_avatar_id: null,
avatar_template:
"/letter_avatar/staged/{size}/3_f9720745f5ce6dfc2b5641fca999d934.png",
},
{
id: -1,
username: "system",
uploaded_avatar_id: null,
avatar_template:
"/letter_avatar/system/{size}/3_f9720745f5ce6dfc2b5641fca999d934.png",
},
],
topics: [],
user: {
user_option: {
text_size_seq: 1,
},
id: 20,
username: "staged",
staged: true,
uploaded_avatar_id: null,
avatar_template:
"/letter_avatar/staged/{size}/3_f9720745f5ce6dfc2b5641fca999d934.png",
name: "Staged",
email: "staged.user@example.com",
associated_accounts: [],
last_posted_at: "2015-05-07T15:23:35.074Z",
last_seen_at: "2015-05-13T14:34:23.188Z",
bio_raw: "",
bio_cooked: "",
created_at: "2013-02-03T15:19:22.704Z",
website: "",
location: "",
can_edit: true,
can_edit_username: true,
can_edit_email: true,
can_edit_name: true,
stats: [],
can_send_private_messages: true,
can_send_private_message_to_user: true,
bio_excerpt: "",
trust_level: 0,
moderator: false,
admin: false,
title: null,
badge_count: 0,
notification_count: 0,
has_title_badges: false,
custom_fields: {},
user_fields: {},
pending_count: 0,
post_count: 0,
can_be_deleted: true,
can_delete_all_posts: true,
locale: "",
email_digests: false,
email_messages_level: 0,
email_level: 1,
digest_after_minutes: 10080,
mailing_list_mode: false,
auto_track_topics_after_msecs: 60000,
new_topic_duration_minutes: 1440,
external_links_in_new_tab: false,
dynamic_favicon: true,
skip_new_user_tips: false,
enable_quoting: true,
muted_category_ids: [],
regular_category_ids: [],
tracked_category_ids: [],
watched_category_ids: [],
watched_first_post_category_ids: [],
private_messages_stats: {},
gravatar_avatar_upload_id: 5275,
custom_avatar_upload_id: 1573,
card_image_badge: "/images/avatar.png",
card_image_badge_id: 120,
muted_usernames: [],
can_change_location: true,
can_change_tracking_preferences: false,
ignored_usernames: [],
invited_by: {
id: 1,
username: "sam",
uploaded_avatar_id: null,
avatar_template:
"/letter_avatar/sam/{size}/3_f9720745f5ce6dfc2b5641fca999d934.png",
},
custom_groups: [],
featured_user_badge_ids: [],
card_badge: null,
user_auth_tokens: [],
user_notification_schedule: {
enabled: false,
day_0_start_time: 480,
day_0_end_time: 1020,
day_1_start_time: 480,
day_1_end_time: 1020,
day_2_start_time: 480,
day_2_end_time: 1020,
day_3_start_time: 480,
day_3_end_time: 1020,
day_4_start_time: 480,
day_4_end_time: 1020,
day_5_start_time: 480,
day_5_end_time: 1020,
day_6_start_time: 480,
day_6_end_time: 1020,
},
timezone: "Australia/Brisbane",
},
},
};

5
app/serializers/user_serializer.rb
View File

@ -57,6 +57,7 @@ class UserSerializer < UserCardSerializer
:can_change_bio,
:can_change_location,
:can_change_website,
:can_change_tracking_preferences,
:user_api_keys,
:user_auth_tokens,
:user_notification_schedule,
@ -132,6 +133,10 @@ class UserSerializer < UserCardSerializer
!(SiteSetting.enable_discourse_connect && SiteSetting.discourse_connect_overrides_website)
end
def can_change_tracking_preferences
scope.can_change_tracking_preferences?(object)
end
def user_api_keys
keys = object.user_api_keys.where(revoked_at: nil).map do |k|
{

1
app/serializers/web_hook_user_serializer.rb
View File

@ -30,6 +30,7 @@ class WebHookUserSerializer < UserSerializer
can_change_bio
can_change_location
can_change_website
can_change_tracking_preferences
user_api_keys
group_users
user_auth_tokens

16
app/services/user_updater.rb
View File

@ -129,15 +129,17 @@ class UserUpdater
user.flair_group_id = attributes[:flair_group_id]
end
CATEGORY_IDS.each do |attribute, level|
if ids = attributes[attribute]
CategoryUser.batch_set(user, level, ids)
if @guardian.can_change_tracking_preferences?(user)
CATEGORY_IDS.each do |attribute, level|
if ids = attributes[attribute]
CategoryUser.batch_set(user, level, ids)
end
end
end
TAG_NAMES.each do |attribute, level|
if attributes.has_key?(attribute)
TagUser.batch_set(user, level, attributes[attribute]&.split(',') || [])
TAG_NAMES.each do |attribute, level|
if attributes.has_key?(attribute)
TagUser.batch_set(user, level, attributes[attribute]&.split(',') || [])
end
end
end

2
config/locales/server.en.yml
View File

@ -2326,6 +2326,8 @@ en:
create_revision_on_bulk_topic_moves: "Create revision for first posts when topics are moved into a new category in bulk."
allow_changing_staged_user_tracking: "Allow a staged user's category and tag notification preferences to be changed by an admin user."
errors:
invalid_email: "Invalid email address."
invalid_username: "There's no user with that username."

2
config/site_settings.yml
View File

@ -2309,6 +2309,8 @@ uncategorized:
create_revision_on_bulk_topic_moves:
default: true
allow_changing_staged_user_tracking: false
user_preferences:
default_email_digest_frequency:
enum: "DigestEmailSiteSetting"

4
lib/guardian/user_guardian.rb
View File

@ -179,4 +179,8 @@ module UserGuardian
def can_delete_sso_record?(user)
SiteSetting.enable_discourse_connect && user && is_admin?
end
def can_change_tracking_preferences?(user)
(SiteSetting.allow_changing_staged_user_tracking || !user.staged) && can_edit_user?(user)
end
end

37
spec/components/guardian/user_guardian_spec.rb
View File

@ -455,4 +455,41 @@ describe UserGuardian do
expect(guardian.can_upload_user_card_background?(trust_level_1)).to eq(false)
end
end
describe "#can_change_tracking_preferences?" do
let(:staged_user) { Fabricate(:staged) }
let(:admin_user) { Fabricate(:admin) }
it "is true for normal TL0 user" do
expect(Guardian.new(user).can_change_tracking_preferences?(user)).to eq(true)
end
it "is true for admin user" do
expect(Guardian.new(admin_user).can_change_tracking_preferences?(admin_user)).to eq(true)
end
context "allow_changing_staged_user_tracking is false" do
before { SiteSetting.allow_changing_staged_user_tracking = false }
it "is false to staged user" do
expect(Guardian.new(staged_user).can_change_tracking_preferences?(staged_user)).to eq(false)
end
it "is false for staged user as admin user" do
expect(Guardian.new(admin_user).can_change_tracking_preferences?(staged_user)).to eq(false)
end
end
context "allow_changing_staged_user_tracking is true" do
before { SiteSetting.allow_changing_staged_user_tracking = true }
it "is true to staged user" do
expect(Guardian.new(staged_user).can_change_tracking_preferences?(staged_user)).to eq(true)
end
it "is true for staged user as admin user" do
expect(Guardian.new(admin_user).can_change_tracking_preferences?(staged_user)).to eq(true)
end
end
end
end

36
spec/services/user_updater_spec.rb
View File

@ -52,7 +52,7 @@ describe UserUpdater do
it 'can update categories and tags' do
user = Fabricate(:user)
updater = UserUpdater.new(acting_user, user)
updater = UserUpdater.new(user, user)
updater.update(watched_tags: "#{tag.name},#{tag2.name}", muted_category_ids: [category.id])
expect(TagUser.where(
@ -72,7 +72,41 @@ describe UserUpdater do
category_id: category.id,
notification_level: CategoryUser.notification_levels[:muted]
).count).to eq(1)
end
context "staged user" do
let(:staged_user) { Fabricate(:staged) }
context "allow_changing_staged_user_tracking is false" do
before { SiteSetting.allow_changing_staged_user_tracking = false }
it "doesn't update muted categories and watched tags" do
updater = UserUpdater.new(Fabricate(:admin), staged_user)
updater.update(watched_tags: "#{tag.name}", muted_category_ids: [category.id])
expect(TagUser.exists?(user_id: staged_user.id)).to eq(false)
expect(CategoryUser.exists?(user_id: staged_user.id)).to eq(false)
end
end
context "allow_changing_staged_user_tracking is true" do
before { SiteSetting.allow_changing_staged_user_tracking = true }
it "updates muted categories and watched tags" do
updater = UserUpdater.new(Fabricate(:admin), staged_user)
updater.update(watched_tags: "#{tag.name}", muted_category_ids: [category.id])
expect(TagUser.exists?(
user_id: staged_user.id,
tag_id: tag.id,
notification_level: TagUser.notification_levels[:watching]
)).to eq(true)
expect(CategoryUser.exists?(
user_id: staged_user.id,
category_id: category.id,
notification_level: CategoryUser.notification_levels[:muted]
)).to eq(true)
end
end
end
it "doesn't remove notification prefs when updating something else" do