FIX: Do not email group user with user_private_message notification (#11754)

There was an issue that occurred with this order of operations:

* An IMAP topic was created by emailing a group
* A second user was invited to the topic (not the OP and not the group)
* A user with access to the group replies to the topic
* The second user receives a user_private_message notification email because of their involvement in the topic
* The second user replies to the email via email

This new reply would then go and notify the other group PM users, except for those who emailed the group topic directly, which is handled via the group SMTP mailer. However because the new post already has an incoming email because it is parsed via the Email::Receiver via POP3 the group SMTP section of the post alerter is skipped, and the group's email address is not ignored for the user_private_message notification.

This PR fixes it so the group is not ever sent an email via the PM notification. This is important because any new emails in the group's IMAP inbox will be picked up by the Imap::Sync code and created as a new topic which is not at all desirable.

Also in this PR I split up the specs a bit more for group SMTP in the post alerter to make them easier to read and they each only test one thing.
This commit is contained in:
Martin Brennan
2021-01-20 10:53:08 +10:00
committed by GitHub
parent 8d3f803b3f
commit 44f15d4281
2 changed files with 84 additions and 63 deletions

View File

@@ -1261,67 +1261,77 @@ describe PostAlerter do
end
fab!(:topic) do
Fabricate(:private_message_topic,
Fabricate(
:private_message_topic,
topic_allowed_groups: [
Fabricate.build(:topic_allowed_group, group: group)
]
)
end
it "sends notifications for new posts in topic" do
PostAlerter.any_instance.expects(:create_notification).at_least_once
post = Fabricate(
def create_post_with_incoming
Fabricate(
:post,
topic: topic,
incoming_email:
Fabricate(
:incoming_email,
topic: topic,
from_address: "foo@discourse.org",
to_addresses: group.email_username,
cc_addresses: "bar@discourse.org"
)
Fabricate(
:incoming_email,
topic: topic,
from_address: "foo@discourse.org",
to_addresses: group.email_username,
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
end
it "does not send a group smtp email when the post already has an incoming email" do
post = create_post_with_incoming
expect { PostAlerter.new.after_save_post(post, true) }.to change { ActionMailer::Base.deliveries.size }.by(0)
end
it "sends a group smtp email when the post does not have an incoming email" do
create_post_with_incoming
post = Fabricate(:post, topic: topic)
expect { PostAlerter.new.after_save_post(post, true) }.to change { ActionMailer::Base.deliveries.size }.by(1)
email = ActionMailer::Base.deliveries.last
expect(email.from).to include(group.email_username)
expect(email.to).to contain_exactly("foo@discourse.org", "bar@discourse.org")
expect(email.subject).to eq("Re: #{topic.title}")
end
post = Fabricate(
:post,
topic: topic,
incoming_email:
Fabricate(
:incoming_email,
topic: topic,
from_address: "bar@discourse.org",
to_addresses: group.email_username,
cc_addresses: "baz@discourse.org"
)
)
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
expect(email.from).to eq([group.email_username])
expect(email.to).to contain_exactly("foo@discourse.org", "bar@discourse.org", "baz@discourse.org")
expect(email.subject).to eq("Re: #{topic.title}")
it "does not send group smtp emails for a whisper" do
create_post_with_incoming
post = Fabricate(:post, topic: topic, post_type: Post.types[:whisper])
expect { PostAlerter.new.after_save_post(post, true) }.to change { ActionMailer::Base.deliveries.size }.by(0)
end
it "does not send a notification email to the group when the post does not have an incoming email" do
PostAlerter.any_instance.expects(:create_notification).with(kind_of(User), Notification.types[:private_message], kind_of(Post), skip_send_email_to: ["discourse@example.com"]).at_least_once
post = create_post_with_incoming
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)
end
it "skips sending a notification email to the group and all other incoming email addresses" do
create_post_with_incoming
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", "discourse@example.com"]).at_least_once
post = Fabricate(:post, topic: topic.reload)
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(1)
email = ActionMailer::Base.deliveries.last
expect(email.from).to eq([group.email_username])
expect(email.to).to contain_exactly("foo@discourse.org", "bar@discourse.org")
expect(email.subject).to eq("Re: #{topic.title}")
end
end
end