FEATURE: Add setting to disable notifications for topic tags edits (#14794)

app/controllers/topics_controller.rb

@@ -387,8 +387,7 @@ class TopicsController < ApplicationController
     success = true
 
     if changes.length > 0
-
+      bypass_bump = should_bypass_bump?(changes)
-      bypass_bump = changes[:category_id].present? && SiteSetting.disable_category_edit_notifications
 
       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]

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
+    return if notification_is_disabled?(post_revision)
-    if SiteSetting.disable_category_edit_notifications && post_revision.modifications&.dig("category_id").present?
-      return
-    end
 
     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

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`."

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

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
+        context 'when using SiteSetting.disable_category_edit_notifications or SiteSetting.disable_tags_edit_notifications' do
-          it "doesn't bump the topic if the setting is enabled" do
+          shared_examples 'a topic bump suppressor' do
-            SiteSetting.disable_category_edit_notifications = true
+            it "doesn't bump the topic if the setting is enabled" do
-            last_bumped_at = topic.bumped_at
+              enable_setting
-            expect(last_bumped_at).not_to be_nil
+              last_bumped_at = topic.bumped_at
+              expect(last_bumped_at).not_to be_nil
 
-            expect do
+              expect do
-              put "/t/#{topic.slug}/#{topic.id}.json", params: {
+                put "/t/#{topic.slug}/#{topic.id}.json", params: params
-                category_id: category.id
+              end.to change { topic.reload.send(attribute_to_change) }.to(expected_new_value)
-              }
-            end.to change { topic.reload.category_id }.to(category.id)
 
-            expect(response.status).to eq(200)
+              expect(response.status).to eq(200)
-            expect(topic.reload.bumped_at).to eq_time(last_bumped_at)
+              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
+          it_behaves_like 'a topic bump suppressor' do
-            last_bumped_at = topic.bumped_at
+            let(:attribute_to_change) { :category_id }
-            expect(last_bumped_at).not_to be_nil
+            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
+          it_behaves_like 'a topic bump suppressor' do
-              put "/t/#{topic.slug}/#{topic.id}.json", params: {
+            let(:tags) { [Fabricate(:tag), Fabricate(:tag)] }
-                category_id: category.id
+            let(:attribute_to_change) { :tags }
-              }
+            let(:expected_new_value) { tags }
-            end.to change { topic.reload.category_id }.to(category.id)
+            let(:params) { { tags: tags.map(&:name) } }
-
+            let(:enable_setting) { SiteSetting.disable_tags_edit_notifications = true }
-            expect(response.status).to eq(200)
+            let(:disable_setting) { SiteSetting.disable_tags_edit_notifications = false }
-            expect(topic.reload.bumped_at).not_to eq_time(last_bumped_at)
           end
         end
 

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) }