From ca33d091b39cd01a00b24f2e0b13f3e57b194354 Mon Sep 17 00:00:00 2001
From: Maja Komel <maja.komel@gmail.com>
Date: Fri, 5 Apr 2019 15:06:38 +0200
Subject: [PATCH] FIX: don't trigger notifications when changing category/tags
 of unlisted topics (#7323)

---
 app/jobs/regular/notify_category_change.rb |  2 +-
 app/jobs/regular/notify_tag_change.rb      |  2 +-
 spec/models/topic_spec.rb                  | 16 +++++++++++++---
 spec/services/post_alerter_spec.rb         | 11 ++++++++++-
 4 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/app/jobs/regular/notify_category_change.rb b/app/jobs/regular/notify_category_change.rb
index ed8a55c7f7a..f3af9bce43b 100644
--- a/app/jobs/regular/notify_category_change.rb
+++ b/app/jobs/regular/notify_category_change.rb
@@ -5,7 +5,7 @@ module Jobs
     def execute(args)
       post = Post.find_by(id: args[:post_id])
 
-      if post&.topic
+      if post&.topic&.visible?
         post_alerter = PostAlerter.new
         post_alerter.notify_post_users(post, User.where(id: args[:notified_user_ids]))
         post_alerter.notify_first_post_watchers(post, post_alerter.category_watchers(post.topic))
diff --git a/app/jobs/regular/notify_tag_change.rb b/app/jobs/regular/notify_tag_change.rb
index b0fcb390c87..ad3d36d0273 100644
--- a/app/jobs/regular/notify_tag_change.rb
+++ b/app/jobs/regular/notify_tag_change.rb
@@ -5,7 +5,7 @@ module Jobs
     def execute(args)
       post = Post.find_by(id: args[:post_id])
 
-      if post&.topic
+      if post&.topic&.visible?
         post_alerter = PostAlerter.new
         post_alerter.notify_post_users(post, User.where(id: args[:notified_user_ids]))
         post_alerter.notify_first_post_watchers(post, post_alerter.tag_watchers(post.topic))
diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb
index 94b1b3e7dd0..4e9ca706f25 100644
--- a/spec/models/topic_spec.rb
+++ b/spec/models/topic_spec.rb
@@ -1275,6 +1275,7 @@ describe Topic do
 
       describe 'to a different category' do
         let(:new_category) { Fabricate(:category, user: user, name: '2nd category') }
+        let(:another_user) { Fabricate(:user) }
 
         it 'should work' do
           topic.change_category_to_id(new_category.id)
@@ -1285,7 +1286,8 @@ describe Topic do
         end
 
         describe 'user that is watching the new category' do
-          it 'should generate the notification for the topic' do
+
+          before do
             Jobs.run_immediately!
 
             topic.posts << Fabricate(:post)
@@ -1296,14 +1298,14 @@ describe Topic do
               new_category.id
             )
 
-            another_user = Fabricate(:user)
-
             CategoryUser.set_notification_level_for_category(
               another_user,
               CategoryUser::notification_levels[:watching_first_post],
               new_category.id
             )
+          end
 
+          it 'should generate the notification for the topic' do
             expect do
               topic.change_category_to_id(new_category.id)
             end.to change { Notification.count }.by(2)
@@ -1322,6 +1324,14 @@ describe Topic do
               notification_type: Notification.types[:watching_first_post]
             ).exists?).to eq(true)
           end
+
+          it "should not generate a notification for unlisted topic" do
+            topic.update_column(:visible, false)
+
+            expect do
+              topic.change_category_to_id(new_category.id)
+            end.to change { Notification.count }.by(0)
+          end
         end
 
         describe 'when new category is set to auto close by default' do
diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb
index cc6453d8a36..c81a25b81a6 100644
--- a/spec/services/post_alerter_spec.rb
+++ b/spec/services/post_alerter_spec.rb
@@ -945,10 +945,10 @@ describe PostAlerter do
       before do
         SiteSetting.tagging_enabled = true
         Jobs.run_immediately!
+        TagUser.change(user.id, watched_tag.id, TagUser.notification_levels[:watching_first_post])
       end
 
       it "triggers a notification" do
-        TagUser.change(user.id, watched_tag.id, TagUser.notification_levels[:watching_first_post])
         expect(user.notifications.where(notification_type: Notification.types[:watching_first_post]).count).to eq(0)
 
         PostRevisor.new(post).revise!(Fabricate(:user), tags: [other_tag.name, watched_tag.name])
@@ -957,6 +957,15 @@ describe PostAlerter do
         PostRevisor.new(post).revise!(Fabricate(:user), tags: [watched_tag.name, other_tag.name])
         expect(user.notifications.where(notification_type: Notification.types[:watching_first_post]).count).to eq(1)
       end
+
+      it "doesn't trigger a notification if topic is unlisted" do
+        post.topic.update_column(:visible, false)
+
+        expect(user.notifications.where(notification_type: Notification.types[:watching_first_post]).count).to eq(0)
+
+        PostRevisor.new(post).revise!(Fabricate(:user), tags: [other_tag.name, watched_tag.name])
+        expect(user.notifications.where(notification_type: Notification.types[:watching_first_post]).count).to eq(0)
+      end
     end
   end