mirror of
https://github.com/discourse/discourse.git
synced 2024-11-23 02:19:27 +08:00
FIX: Stop incorrect emailing of group email from PostAlerter (#11723)
Fixes bug introduced by bd25627198
What happens is we send notifications to everyone involved in the group inbox topic about new posts, however we pass the param `skip_send_email_to: email_addresses`. In the above commit I removed the group email address from this `email_addresses` array. This breaks the IMAP inbox because we email the group with the reply, and the IMAP sync tool finds this email and opens a new unrelated topic with it.
This commit is contained in:
parent
eeb7aa735a
commit
6c155dba77
|
@ -575,12 +575,20 @@ class PostAlerter
|
|||
warn_if_not_sidekiq
|
||||
|
||||
# users who interacted with the post by _directly_ emailing the group
|
||||
emails_to_skip_send = nil
|
||||
if group = group_notifying_via_smtp(post)
|
||||
email_addresses = post.topic.incoming_email_addresses(group: group)
|
||||
|
||||
if email_addresses.any?
|
||||
Jobs.enqueue(:group_smtp_email, group_id: group.id, post_id: post.id, email: email_addresses)
|
||||
end
|
||||
|
||||
# add the group's email back into the array, because it is used for
|
||||
# skip_send_email_to in the case of user private message notifications
|
||||
# (we do not want the group to be sent any emails from here because it
|
||||
# will make another email for IMAP to pick up in the group's mailbox)
|
||||
emails_to_skip_send = email_addresses.dup
|
||||
emails_to_skip_send << group.email_username
|
||||
end
|
||||
|
||||
# users that aren't part of any mentioned groups
|
||||
|
@ -589,7 +597,7 @@ class PostAlerter
|
|||
users.each do |user|
|
||||
notification_level = TopicUser.get(post.topic, user)&.notification_level
|
||||
if reply_to_user == user || notification_level == TopicUser.notification_levels[:watching] || user.staged?
|
||||
create_notification(user, Notification.types[:private_message], post, skip_send_email_to: email_addresses)
|
||||
create_notification(user, Notification.types[:private_message], post, skip_send_email_to: emails_to_skip_send)
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -600,7 +608,7 @@ class PostAlerter
|
|||
case TopicUser.get(post.topic, user)&.notification_level
|
||||
when TopicUser.notification_levels[:watching]
|
||||
# only create a notification when watching the group
|
||||
create_notification(user, Notification.types[:private_message], post, skip_send_email_to: email_addresses)
|
||||
create_notification(user, Notification.types[:private_message], post, skip_send_email_to: emails_to_skip_send)
|
||||
when TopicUser.notification_levels[:tracking]
|
||||
notify_group_summary(user, post)
|
||||
end
|
||||
|
|
|
@ -1269,6 +1269,7 @@ describe PostAlerter do
|
|||
end
|
||||
|
||||
it "sends notifications for new posts in topic" do
|
||||
PostAlerter.any_instance.expects(:create_notification).at_least_once
|
||||
post = Fabricate(
|
||||
:post,
|
||||
topic: topic,
|
||||
|
@ -1281,6 +1282,11 @@ describe PostAlerter do
|
|||
cc_addresses: "bar@discourse.org"
|
||||
)
|
||||
)
|
||||
staged_group_user = Fabricate(:staged, email: "discourse@example.com")
|
||||
Fabricate(:topic_user, user: staged_group_user, topic: post.topic)
|
||||
topic.allowed_users << staged_group_user
|
||||
topic.save
|
||||
|
||||
expect { PostAlerter.new.after_save_post(post, true) }.to change { ActionMailer::Base.deliveries.size }.by(0)
|
||||
|
||||
post = Fabricate(:post, topic: topic)
|
||||
|
@ -1304,6 +1310,9 @@ describe PostAlerter do
|
|||
)
|
||||
expect { PostAlerter.new.after_save_post(post, true) }.to change { ActionMailer::Base.deliveries.size }.by(0)
|
||||
|
||||
# we never want the group to be notified by email about these posts
|
||||
PostAlerter.any_instance.expects(:create_notification).with(kind_of(User), Notification.types[:private_message], kind_of(Post), skip_send_email_to: ["foo@discourse.org", "bar@discourse.org", "baz@discourse.org", "discourse@example.com"]).at_least_once
|
||||
|
||||
post = Fabricate(:post, topic: topic.reload)
|
||||
expect { PostAlerter.new.after_save_post(post, true) }.to change { ActionMailer::Base.deliveries.size }.by(1)
|
||||
email = ActionMailer::Base.deliveries.last
|
||||
|
|
Loading…
Reference in New Issue
Block a user