FIX: do not notify when the hidden tag is added or removed (#12025)

The bug was mentioned on meta https://meta.discourse.org/t/users-are-seeing-handling-of-unhandled-tag-again/155367

It was related to users who are watching a specific topic. In that case, when the hidden tag was added or removed to the topic they were notified by `NotifyTagChangeJob`.

That job should take hidden tags into consideration. If all changed tags are in a hidden group, it should exclude user not belong to that group.

At the same time, if visible to anyone tag is added or removed users watching topic should be notified.

app/jobs/regular/notify_tag_change.rb

@@ -7,9 +7,33 @@ module Jobs
 
       if post&.topic&.visible?
         post_alerter = PostAlerter.new
-        post_alerter.notify_post_users(post, User.where(id: args[:notified_user_ids]), include_topic_watchers: !post.topic.private_message?, include_category_watchers: false)
+        post_alerter.notify_post_users(post, excluded_users(args), include_topic_watchers: !post.topic.private_message?, include_category_watchers: false)
         post_alerter.notify_first_post_watchers(post, post_alerter.tag_watchers(post.topic))
       end
     end
+
+    private
+
+    def excluded_users(args)
+      if !args[:diff_tags] || !all_tags_in_hidden_groups?(args)
+        return User.where(id: args[:notified_user_ids])
+      end
+      group_users_join = DB.sql_fragment("LEFT JOIN group_users ON group_users.user_id = users.id AND group_users.group_id IN (:group_ids)", group_ids: tag_group_ids(args))
+      condition = DB.sql_fragment("group_users.id IS NULL OR users.id IN (:notified_user_ids)", notified_user_ids: args[:notified_user_ids])
+      User.joins(group_users_join).where(condition)
+    end
+
+    def all_tags_in_hidden_groups?(args)
+      Tag
+        .where(name: args[:diff_tags])
+        .joins(tag_groups: :tag_group_permissions)
+        .where.not(tag_group_permissions: { group_id: 0 })
+        .distinct
+        .count == args[:diff_tags].count
+    end
+
+    def tag_group_ids(args)
+      Tag.where(name: args[:diff_tags]).joins(tag_groups: :tag_group_permissions).pluck("tag_group_permissions.group_id")
+    end
   end
 end

lib/post_revisor.rb

@@ -98,7 +98,7 @@ class PostRevisor
         DB.after_commit do
           post = tc.topic.ordered_posts.first
           notified_user_ids = [post.user_id, post.last_editor_id].uniq
-          Jobs.enqueue(:notify_tag_change, post_id: post.id, notified_user_ids: notified_user_ids)
+          Jobs.enqueue(:notify_tag_change, post_id: post.id, notified_user_ids: notified_user_ids, diff_tags: ((tags - prev_tags) | (prev_tags - tags)))
         end
       end
     end

spec/jobs/notify_tag_change_spec.rb

@@ -35,4 +35,30 @@ describe ::Jobs::NotifyTagChange do
     )
     expect { described_class.new.execute(post_id: post.id, notified_user_ids: [regular_user.id]) }.not_to change { Notification.count }
   end
+
+  context 'hidden tag' do
+    let!(:hidden_group) { Fabricate(:group, name: 'hidden_group') }
+    let!(:hidden_tag_group) { Fabricate(:tag_group, name: 'hidden', permissions: [[hidden_group.id, :full]]) }
+    let!(:topic_user) { Fabricate(:topic_user, user: user, topic: post.topic, notification_level: TopicUser.notification_levels[:watching]) }
+
+    it 'does not create notification for watching user who does not belong to group' do
+      TagGroupMembership.create!(tag_group_id: hidden_tag_group.id, tag_id: tag.id)
+
+      expect { described_class.new.execute(post_id: post.id, notified_user_ids: [regular_user.id], diff_tags: [tag.name]) }.not_to change { Notification.count }
+
+      Fabricate(:group_user, group: hidden_group, user: user)
+
+      expect { described_class.new.execute(post_id: post.id, notified_user_ids: [regular_user.id], diff_tags: [tag.name]) }.to change { Notification.count }
+    end
+
+    it 'creates notification when at least added or removed tag is visible to everyone' do
+      visible_tag = Fabricate(:tag, name: 'visible tag')
+      visible_group = Fabricate(:tag_group, name: 'visible group')
+      TagGroupMembership.create!(tag_group_id: visible_group.id, tag_id: visible_tag.id)
+      TagGroupMembership.create!(tag_group_id: hidden_tag_group.id, tag_id: tag.id)
+
+      expect { described_class.new.execute(post_id: post.id, notified_user_ids: [regular_user.id], diff_tags: [tag.name]) }.not_to change { Notification.count }
+      expect { described_class.new.execute(post_id: post.id, notified_user_ids: [regular_user.id], diff_tags: [tag.name, visible_tag.name]) }.to change { Notification.count }
+    end
+  end
 end