diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index 645722c8324..fce8c76b82e 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -927,23 +927,27 @@ module Email def create_group_post(group, user, body, elided) message_ids = Email::Receiver.extract_reply_message_ids(@mail, max_message_id_count: 5) - # incoming emails with matching message ids, and then cross references + # Incoming emails with matching message ids, and then cross references # these with any email addresses for the user vs to/from/cc of the # incoming emails. in effect, any incoming email record for these - # message ids where the user is involved in any way will be returned + # message ids where the user is involved in any way will be returned. incoming_emails = IncomingEmail.where(message_id: message_ids) if !group.allow_unknown_sender_topic_replies incoming_emails = incoming_emails.addressed_to_user(user) end post_ids = incoming_emails.pluck(:post_id) || [] - # if the user is directly replying to an email send to them from discourse, + # If the user is directly replying to an email send to them from discourse, # there will be a corresponding EmailLog record, so we can use that as the - # reply post if it exists - if Email::MessageIdService.discourse_generated_message_id?(mail.in_reply_to) + # reply post if it exists. + # + # Since In-Reply-To can technically have multiple message ids, we only + # consider the first one here to simplify things. + first_in_reply_to = Array.wrap(mail.in_reply_to).first + if Email::MessageIdService.discourse_generated_message_id?(first_in_reply_to) post_id_from_email_log = EmailLog - .where(message_id: mail.in_reply_to) + .where(message_id: first_in_reply_to) .addressed_to_user(user) .order(created_at: :desc) .limit(1) diff --git a/spec/fixtures/emails/email_to_group_email_username_3.eml b/spec/fixtures/emails/email_to_group_email_username_3.eml new file mode 100644 index 00000000000..d445aa5b17c --- /dev/null +++ b/spec/fixtures/emails/email_to_group_email_username_3.eml @@ -0,0 +1,14 @@ +Return-Path: <two@foo.com> +From: Two <two@foo.com> +To: team@somesmtpaddress.com +Subject: Full email group username flow +Date: Saturday, 16 Jan 2016 00:12:43 +0100 +Message-ID: <348ct38794nyt9338dsfsd@foo.bar.mail> +In-Reply-To: MESSAGE_ID_REPLY_TO +References: <u4w8c9r4y984yh98r3h69873@foo.bar.mail> + <MESSAGE_ID_REPLY_TO> +Mime-Version: 1.0 +Content-Type: text/plain +Content-Transfer-Encoding: 7bit + +Hey thanks for the suggestion, it didn't work. diff --git a/spec/lib/email/receiver_spec.rb b/spec/lib/email/receiver_spec.rb index db28035f47a..bb5d553d02d 100644 --- a/spec/lib/email/receiver_spec.rb +++ b/spec/lib/email/receiver_spec.rb @@ -218,9 +218,7 @@ RSpec.describe Email::Receiver do end it "strips null bytes from the subject" do - expect do process(:null_byte_in_subject) end.to raise_error( - Email::Receiver::BadDestinationAddress, - ) + expect { process(:null_byte_in_subject) }.to raise_error(Email::Receiver::BadDestinationAddress) end describe "bounces to VERP" do @@ -1366,13 +1364,29 @@ RSpec.describe Email::Receiver do expect(IncomingEmail.exists?(post_id: group_post.id)).to eq(false) end - it "processes a reply from the OP user to the group SMTP username, linking the reply_to_post_number correctly by - matching in_reply_to to the email log" do + it "processes a reply from the OP user to the group SMTP username, linking the reply_to_post_number correctly by matching in_reply_to to the email log" do email_log, group_post = reply_as_group_user reply_email = email(:email_to_group_email_username_2) reply_email.gsub!("MESSAGE_ID_REPLY_TO", email_log.message_id) - expect do Email::Receiver.new(reply_email).process! end.to not_change { + expect { Email::Receiver.new(reply_email).process! }.to not_change { + Topic.count + }.and change { Post.count }.by(1) + + reply_post = Post.last + expect(reply_post.reply_to_user).to eq(user_in_group) + expect(reply_post.reply_to_post_number).to eq(group_post.post_number) + end + + it "handles multiple message IDs in the in_reply_to header by only using the first one" do + email_log, group_post = reply_as_group_user + + reply_email = email(:email_to_group_email_username_3) + reply_email.gsub!( + "MESSAGE_ID_REPLY_TO", + "<#{email_log.message_id}> <test/message/id@discourse.com>", + ) + expect { Email::Receiver.new(reply_email).process! }.to not_change { Topic.count }.and change { Post.count }.by(1) @@ -1386,7 +1400,7 @@ RSpec.describe Email::Receiver do reply_email = email(:email_to_group_email_username_2_as_unknown_sender) reply_email.gsub!("MESSAGE_ID_REPLY_TO", email_log.message_id) - expect do Email::Receiver.new(reply_email).process! end.to change { Topic.count }.by( + expect { Email::Receiver.new(reply_email).process! }.to change { Topic.count }.by( 1, ).and change { Post.count }.by(1) @@ -1400,7 +1414,7 @@ RSpec.describe Email::Receiver do reply_email = email(:email_to_group_email_username_2_as_unknown_sender) reply_email.gsub!("MESSAGE_ID_REPLY_TO", email_log.message_id) - expect do Email::Receiver.new(reply_email).process! end.to not_change { + expect { Email::Receiver.new(reply_email).process! }.to not_change { Topic.count }.and change { Post.count }.by(1) @@ -1417,7 +1431,7 @@ RSpec.describe Email::Receiver do reply_email = email(:email_to_group_email_username_2) reply_email.gsub!("MESSAGE_ID_REPLY_TO", email_log.message_id) - expect do Email::Receiver.new(reply_email).process! end.to change { Topic.count }.by( + expect { Email::Receiver.new(reply_email).process! }.to change { Topic.count }.by( 1, ).and change { Post.count }.by(1)