diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index c2d98ae5d0d..baeef8c3887 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -387,8 +387,7 @@ class TopicsController < ApplicationController success = true if changes.length > 0 - - bypass_bump = changes[:category_id].present? && SiteSetting.disable_category_edit_notifications + bypass_bump = should_bypass_bump?(changes) first_post = topic.ordered_posts.first success = PostRevisor.new(first_post, topic).revise!(current_user, changes, validate_post: false, bypass_bump: bypass_bump) @@ -1117,6 +1116,11 @@ class TopicsController < ApplicationController Promotion.new(current_user).review if current_user.present? end + def should_bypass_bump?(changes) + (changes[:category_id].present? && SiteSetting.disable_category_edit_notifications) || + (changes[:tags].present? && SiteSetting.disable_tags_edit_notifications) + end + def slugs_do_not_match if SiteSetting.slug_generation_method != "encoded" params[:slug] && @topic_view.topic.slug != params[:slug] diff --git a/app/services/post_action_notifier.rb b/app/services/post_action_notifier.rb index f58b784b503..71bf1e5d80f 100644 --- a/app/services/post_action_notifier.rb +++ b/app/services/post_action_notifier.rb @@ -102,10 +102,7 @@ class PostActionNotifier return if post_revision.user.blank? return if post.topic.blank? return if post.topic.private_message? - return if SiteSetting.disable_system_edit_notifications && post_revision.user_id == Discourse::SYSTEM_USER_ID - if SiteSetting.disable_category_edit_notifications && post_revision.modifications&.dig("category_id").present? - return - end + return if notification_is_disabled?(post_revision) user_ids = [] @@ -160,4 +157,13 @@ class PostActionNotifier def self.add_post_revision_notifier_recipients(&block) custom_post_revision_notifier_recipients << block end + + private + + def self.notification_is_disabled?(post_revision) + modifications = post_revision.modifications + (SiteSetting.disable_system_edit_notifications && post_revision.user_id == Discourse::SYSTEM_USER_ID) || + (SiteSetting.disable_category_edit_notifications && modifications&.dig("category_id").present?) || + (SiteSetting.disable_tags_edit_notifications && modifications&.dig("tags").present?) + end end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index ce1740ea64a..d7be57a3bf0 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2130,6 +2130,8 @@ en: disable_category_edit_notifications: "Disable category edit notifications on topics." + disable_tags_edit_notifications: "Disable tags edit notifications on topics." + notification_consolidation_threshold: "Number of liked or membership request notifications received before the notifications are consolidated into a single one. Set to 0 to disable." likes_notification_consolidation_window_mins: "Duration in minutes where liked notifications are consolidated into a single notification once the threshold has been reached. The threshold can be configured via `SiteSetting.notification_consolidation_threshold`." diff --git a/config/site_settings.yml b/config/site_settings.yml index fb70d5ed809..399815b4e8c 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -2210,6 +2210,9 @@ uncategorized: disable_category_edit_notifications: default: false + disable_tags_edit_notifications: + default: false + notification_consolidation_threshold: default: 3 min: 0 diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 0171366710e..7a76aa043ac 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -1358,34 +1358,50 @@ RSpec.describe TopicsController do expect(response.status).to eq(200) end - context 'when using SiteSetting.disable_category_edit_notifications' do - it "doesn't bump the topic if the setting is enabled" do - SiteSetting.disable_category_edit_notifications = true - last_bumped_at = topic.bumped_at - expect(last_bumped_at).not_to be_nil + context 'when using SiteSetting.disable_category_edit_notifications or SiteSetting.disable_tags_edit_notifications' do + shared_examples 'a topic bump suppressor' do + it "doesn't bump the topic if the setting is enabled" do + enable_setting + last_bumped_at = topic.bumped_at + expect(last_bumped_at).not_to be_nil - expect do - put "/t/#{topic.slug}/#{topic.id}.json", params: { - category_id: category.id - } - end.to change { topic.reload.category_id }.to(category.id) + expect do + put "/t/#{topic.slug}/#{topic.id}.json", params: params + end.to change { topic.reload.send(attribute_to_change) }.to(expected_new_value) - expect(response.status).to eq(200) - expect(topic.reload.bumped_at).to eq_time(last_bumped_at) + expect(response.status).to eq(200) + expect(topic.reload.bumped_at).to eq_time(last_bumped_at) + end + + it "bumps the topic if the setting is disabled" do + disable_setting + last_bumped_at = topic.bumped_at + expect(last_bumped_at).not_to be_nil + + expect do + put "/t/#{topic.slug}/#{topic.id}.json", params: params + end.to change { topic.reload.send(attribute_to_change) }.to(expected_new_value) + + expect(response.status).to eq(200) + expect(topic.reload.bumped_at).not_to eq_time(last_bumped_at) + end end - it "bumps the topic if the setting is disabled" do - last_bumped_at = topic.bumped_at - expect(last_bumped_at).not_to be_nil + it_behaves_like 'a topic bump suppressor' do + let(:attribute_to_change) { :category_id } + let(:expected_new_value) { category.id } + let(:params) { { category_id: category.id } } + let(:enable_setting) { SiteSetting.disable_category_edit_notifications = true } + let(:disable_setting) { SiteSetting.disable_category_edit_notifications = false } + end - expect do - put "/t/#{topic.slug}/#{topic.id}.json", params: { - category_id: category.id - } - end.to change { topic.reload.category_id }.to(category.id) - - expect(response.status).to eq(200) - expect(topic.reload.bumped_at).not_to eq_time(last_bumped_at) + it_behaves_like 'a topic bump suppressor' do + let(:tags) { [Fabricate(:tag), Fabricate(:tag)] } + let(:attribute_to_change) { :tags } + let(:expected_new_value) { tags } + let(:params) { { tags: tags.map(&:name) } } + let(:enable_setting) { SiteSetting.disable_tags_edit_notifications = true } + let(:disable_setting) { SiteSetting.disable_tags_edit_notifications = false } end end diff --git a/spec/services/post_action_notifier_spec.rb b/spec/services/post_action_notifier_spec.rb index fbdbd674e14..c78be23dc59 100644 --- a/spec/services/post_action_notifier_spec.rb +++ b/spec/services/post_action_notifier_spec.rb @@ -172,6 +172,25 @@ describe PostActionNotifier do end + context "tags edit notifications are disabled" do + it 'notifies a user of the revision made by another user' do + SiteSetting.disable_tags_edit_notifications = false + + expect { + post.revise(evil_trout, tags: [Fabricate(:tag).name]) + }.to change(post.user.notifications, :count).by(1) + end + + it 'does not notify a user of the revision made by the system user' do + SiteSetting.disable_tags_edit_notifications = true + + expect { + post.revise(evil_trout, tags: [Fabricate(:tag).name]) + }.not_to change(post.user.notifications, :count) + end + + end + context 'when using plugin API to add custom recipients' do let(:lurker) { Fabricate(:user) }