diff --git a/app/jobs/regular/notify_tag_change.rb b/app/jobs/regular/notify_tag_change.rb index 16cab8de7b5..4b513dd53b7 100644 --- a/app/jobs/regular/notify_tag_change.rb +++ b/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 diff --git a/lib/post_revisor.rb b/lib/post_revisor.rb index 76e2c4b28e6..c94d03e108f 100644 --- a/lib/post_revisor.rb +++ b/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 diff --git a/spec/jobs/notify_tag_change_spec.rb b/spec/jobs/notify_tag_change_spec.rb index 5215f0fa342..b9b3ac1c51d 100644 --- a/spec/jobs/notify_tag_change_spec.rb +++ b/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