mirror of
https://github.com/discourse/discourse.git
synced 2025-03-20 12:35:34 +08:00
FIX: tag and category watchers regression (#8336)
I made a regression here 17366d3bcc (diff-ddeebb36d131f89ca91be9d04c2baefaR10)
When the tag is added, people watching specific tag are notified but also people watching specific category.
Therefore, `notify_post_users` should accept options who should be notified.
So when `category` is added to the topic, users watching topic and users watching category are notified.
When `tag` is added to the topic, users watching topic and users watching tag are notified
Finally, when a new post is created, everybody is notified, topic watchers, category watchers, tag watchers.
This commit is contained in:
parent
6ebffaaf6e
commit
69266f60ed
@ -7,7 +7,7 @@ module Jobs
|
|||||||
|
|
||||||
if post&.topic&.visible?
|
if post&.topic&.visible?
|
||||||
post_alerter = PostAlerter.new
|
post_alerter = PostAlerter.new
|
||||||
post_alerter.notify_post_users(post, User.where(id: args[:notified_user_ids]))
|
post_alerter.notify_post_users(post, User.where(id: args[:notified_user_ids]), include_tag_watchers: false)
|
||||||
post_alerter.notify_first_post_watchers(post, post_alerter.category_watchers(post.topic))
|
post_alerter.notify_first_post_watchers(post, post_alerter.category_watchers(post.topic))
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
@ -7,7 +7,7 @@ module Jobs
|
|||||||
|
|
||||||
if post&.topic&.visible?
|
if post&.topic&.visible?
|
||||||
post_alerter = PostAlerter.new
|
post_alerter = PostAlerter.new
|
||||||
post_alerter.notify_post_users(post, User.where(id: args[:notified_user_ids]))
|
post_alerter.notify_post_users(post, User.where(id: args[:notified_user_ids]), include_category_watchers: false)
|
||||||
post_alerter.notify_first_post_watchers(post, post_alerter.tag_watchers(post.topic))
|
post_alerter.notify_first_post_watchers(post, post_alerter.tag_watchers(post.topic))
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
@ -559,7 +559,7 @@ class PostAlerter
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def notify_post_users(post, notified)
|
def notify_post_users(post, notified, include_category_watchers: true, include_tag_watchers: true)
|
||||||
return unless post.topic
|
return unless post.topic
|
||||||
|
|
||||||
warn_if_not_sidekiq
|
warn_if_not_sidekiq
|
||||||
@ -570,8 +570,14 @@ class PostAlerter
|
|||||||
FROM topic_users
|
FROM topic_users
|
||||||
WHERE notification_level = :watching
|
WHERE notification_level = :watching
|
||||||
AND topic_id = :topic_id
|
AND topic_id = :topic_id
|
||||||
|
/*category*/
|
||||||
|
/*tags*/
|
||||||
|
)
|
||||||
|
SQL
|
||||||
|
|
||||||
UNION
|
if include_category_watchers
|
||||||
|
condition.sub! "/*category*/", <<~SQL
|
||||||
|
UNION
|
||||||
|
|
||||||
SELECT cu.user_id
|
SELECT cu.user_id
|
||||||
FROM category_users cu
|
FROM category_users cu
|
||||||
@ -580,14 +586,12 @@ class PostAlerter
|
|||||||
WHERE cu.notification_level = :watching
|
WHERE cu.notification_level = :watching
|
||||||
AND cu.category_id = :category_id
|
AND cu.category_id = :category_id
|
||||||
AND tu.user_id IS NULL
|
AND tu.user_id IS NULL
|
||||||
|
SQL
|
||||||
/*tags*/
|
end
|
||||||
)
|
|
||||||
SQL
|
|
||||||
|
|
||||||
tag_ids = post.topic.topic_tags.pluck('topic_tags.tag_id')
|
tag_ids = post.topic.topic_tags.pluck('topic_tags.tag_id')
|
||||||
|
|
||||||
if tag_ids.present?
|
if include_tag_watchers && tag_ids.present?
|
||||||
condition.sub! "/*tags*/", <<~SQL
|
condition.sub! "/*tags*/", <<~SQL
|
||||||
UNION
|
UNION
|
||||||
|
|
||||||
|
@ -25,4 +25,13 @@ describe ::Jobs::NotifyTagChange do
|
|||||||
expect(notification.user_id).to eq(user.id)
|
expect(notification.user_id).to eq(user.id)
|
||||||
expect(notification.topic_id).to eq(post.topic_id)
|
expect(notification.topic_id).to eq(post.topic_id)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it 'doesnt create notification for user watching category' do
|
||||||
|
CategoryUser.create!(
|
||||||
|
user_id: user.id,
|
||||||
|
category_id: post.topic.category_id,
|
||||||
|
notification_level: TopicUser.notification_levels[:watching]
|
||||||
|
)
|
||||||
|
expect { described_class.new.execute(post_id: post.id, notified_user_ids: [regular_user.id]) }.not_to change { Notification.count }
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
Loading…
x
Reference in New Issue
Block a user