FIX: modify notification after remove auto_watch_category (#10568)

When a category is removed from `auto_watch_category` we are removing
CategoryUser. However, there are still TopicUser with notification level
set to `watching` which was inherited from Category.

We should move them back to `regular` unless they were modified by a user.
This commit is contained in:
Krzysztof Kotlarek 2020-09-01 13:07:41 +10:00 committed by GitHub
parent e824385e64
commit 084e15b447
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 41 additions and 8 deletions

View File

@ -32,7 +32,7 @@ SiteSetting.reopenClass({
data[key] = value;
if (opts["updateExistingUsers"] === true) {
data["updateExistingUsers"] = true;
data["update_existing_user"] = true;
}
return ajax(`/admin/site_settings/${key}`, { type: "PUT", data });

View File

@ -20,7 +20,7 @@ class Admin::SiteSettingsController < Admin::AdminController
value = Upload.get_from_url(value) || ""
end
update_existing_users = params[:updateExistingUsers].present?
update_existing_users = params[:update_existing_user].present?
previous_value = SiteSetting.send(id) || "" if update_existing_users
SiteSetting.set_and_log(id, value, current_user)
@ -58,7 +58,14 @@ class Admin::SiteSettingsController < Admin::AdminController
notification_level = NotificationLevels.all[:regular]
end
CategoryUser.where(category_id: (previous_category_ids - new_category_ids), notification_level: notification_level).delete_all
categories_to_unwatch = previous_category_ids - new_category_ids
CategoryUser.where(category_id: categories_to_unwatch, notification_level: notification_level).delete_all
TopicUser
.joins(:topic)
.where(notification_level: TopicUser.notification_levels[:watching],
notifications_reason_id: TopicUser.notification_reasons[:auto_watch_category],
topics: { category_id: categories_to_unwatch })
.update_all(notification_level: TopicUser.notification_levels[:regular])
(new_category_ids - previous_category_ids).each do |category_id|
skip_user_ids = CategoryUser.where(category_id: category_id).pluck(:user_id)

View File

@ -65,7 +65,7 @@ describe Admin::SiteSettingsController do
put "/admin/site_settings/default_email_in_reply_to.json", params: {
default_email_in_reply_to: false,
updateExistingUsers: true
update_existing_user: true
}
user2.reload
@ -86,14 +86,14 @@ describe Admin::SiteSettingsController do
expect {
put "/admin/site_settings/default_email_digest_frequency.json", params: {
default_email_digest_frequency: 30,
updateExistingUsers: true
update_existing_user: true
}
}.to change { UserOption.where(email_digests: true).count }.by(1)
expect {
put "/admin/site_settings/default_email_digest_frequency.json", params: {
default_email_digest_frequency: 0,
updateExistingUsers: true
update_existing_user: true
}
}.to change { UserOption.where(email_digests: false).count }.by(User.count)
end
@ -120,11 +120,25 @@ describe Admin::SiteSettingsController do
it 'should update existing users user preference' do
put "/admin/site_settings/default_categories_watching.json", params: {
default_categories_watching: category_ids.last(2).join("|"),
updateExistingUsers: true
update_existing_user: true
}
expect(response.status).to eq(200)
expect(CategoryUser.where(category_id: category_ids.first, notification_level: watching).count).to eq(0)
expect(CategoryUser.where(category_id: category_ids.last, notification_level: watching).count).to eq(User.real.where(staged: false).count - 1)
topic = Fabricate(:topic, category_id: category_ids.last)
topic_user1 = Fabricate(:topic_user, topic: topic, notification_level: TopicUser.notification_levels[:watching], notifications_reason_id: TopicUser.notification_reasons[:auto_watch_category])
topic_user2 = Fabricate(:topic_user, topic: topic, notification_level: TopicUser.notification_levels[:watching], notifications_reason_id: TopicUser.notification_reasons[:user_changed])
put "/admin/site_settings/default_categories_watching.json", params: {
default_categories_watching: "",
update_existing_user: true
}
expect(response.status).to eq(200)
expect(CategoryUser.where(category_id: category_ids, notification_level: watching).count).to eq(0)
expect(topic_user1.reload.notification_level).to eq(TopicUser.notification_levels[:regular])
expect(topic_user2.reload.notification_level).to eq(TopicUser.notification_levels[:watching])
end
it 'should not update existing users user preference' do
@ -134,7 +148,19 @@ describe Admin::SiteSettingsController do
}
}.to change { CategoryUser.where(category_id: category_ids.first, notification_level: watching).count }.by(0)
expect(response.status).to eq(200)
expect(CategoryUser.where(category_id: category_ids.last, notification_level: watching).count).to eq(0)
topic = Fabricate(:topic, category_id: category_ids.last)
topic_user1 = Fabricate(:topic_user, topic: topic, notification_level: TopicUser.notification_levels[:watching], notifications_reason_id: TopicUser.notification_reasons[:auto_watch_category])
topic_user2 = Fabricate(:topic_user, topic: topic, notification_level: TopicUser.notification_levels[:watching], notifications_reason_id: TopicUser.notification_reasons[:user_changed])
put "/admin/site_settings/default_categories_watching.json", params: {
default_categories_watching: "",
}
expect(response.status).to eq(200)
expect(CategoryUser.where(category_id: category_ids.first, notification_level: watching).count).to eq(0)
expect(topic_user1.reload.notification_level).to eq(TopicUser.notification_levels[:watching])
expect(topic_user2.reload.notification_level).to eq(TopicUser.notification_levels[:watching])
end
end
@ -159,7 +185,7 @@ describe Admin::SiteSettingsController do
it 'should update existing users user preference' do
put "/admin/site_settings/default_tags_watching.json", params: {
default_tags_watching: tags.last(2).pluck(:name).join("|"),
updateExistingUsers: true
update_existing_user: true
}
expect(TagUser.where(tag_id: tags.first.id, notification_level: watching).count).to eq(0)