diff --git a/lib/email/message_id_service.rb b/lib/email/message_id_service.rb index e1a9fea0e88..2bfa81067a1 100644 --- a/lib/email/message_id_service.rb +++ b/lib/email/message_id_service.rb @@ -71,29 +71,13 @@ module Email def find_post_from_message_ids(message_ids) message_ids = message_ids.map { |message_id| message_id_clean(message_id) } - # TODO (martin) 2023-04-01 We should remove these backwards-compatible - # formats for the Message-ID and solely use the discourse/post/999@host - # format. - topic_ids = - message_ids - .map { |message_id| message_id[message_id_topic_id_regexp, 1] } - .compact - .map(&:to_i) post_ids = message_ids - .map { |message_id| message_id[message_id_post_id_regexp, 1] } + .map { |message_id| message_id[message_id_discourse_regexp, 1] } .compact .map(&:to_i) - post_ids << message_ids - .map { |message_id| message_id[message_id_discourse_regexp, 1] } - .compact - .map(&:to_i) - - post_ids << Post - .where(outbound_message_id: message_ids) - .or(Post.where(topic_id: topic_ids, post_number: 1)) - .pluck(:id) + post_ids << Post.where(outbound_message_id: message_ids).pluck(:id) post_ids << EmailLog.where(message_id: message_ids).pluck(:post_id) post_ids << IncomingEmail.where(message_id: message_ids).pluck(:post_id) @@ -106,24 +90,8 @@ module Email Post.where(id: post_ids).order(:created_at).last end - # TODO (martin) 2023-04-01 We should remove these backwards-compatible - # formats for the Message-ID and solely use the discourse/post/999@host - # format. def discourse_generated_message_id?(message_id) - !!(message_id =~ message_id_post_id_regexp) || - !!(message_id =~ message_id_topic_id_regexp) || - !!(message_id =~ message_id_discourse_regexp) - end - - # TODO (martin) 2023-04-01 We should remove these backwards-compatible - # formats for the Message-ID and solely use the discourse/post/999@host - # format. - def message_id_post_id_regexp - Regexp.new "topic/\\d+/(\\d+|\\d+\.\\w+)@#{Regexp.escape(host)}" - end - - def message_id_topic_id_regexp - Regexp.new "topic/(\\d+|\\d+\.\\w+)@#{Regexp.escape(host)}" + message_id_discourse_regexp.match?(message_id) end def message_id_discourse_regexp diff --git a/spec/fixtures/emails/old_destination.eml b/spec/fixtures/emails/old_destination.eml index ee51811d05e..479b3161f33 100644 --- a/spec/fixtures/emails/old_destination.eml +++ b/spec/fixtures/emails/old_destination.eml @@ -1,13 +1,13 @@ -Return-Path: -From: Foo Bar -To: reply+4f97315cc828096c9cb34c6f1a0d6fe8@bar.com -Cc: foofoo@bar.com -Date: Fri, 15 Jan 2018 00:12:43 +0100 -Message-ID: <999@foo.bar.mail> -In-Reply-To: -Mime-Version: 1.0 -Content-Type: text/plain -Content-Transfer-Encoding: 7bit -Subject: Some Old Post - -Lorem ipsum dolor sit amet, consectetur adipiscing elit. +Return-Path: +From: Foo Bar +To: reply+4f97315cc828096c9cb34c6f1a0d6fe8@bar.com +Cc: foofoo@bar.com +Date: Fri, 15 Jan 2018 00:12:43 +0100 +Message-ID: <999@foo.bar.mail> +In-Reply-To: +Mime-Version: 1.0 +Content-Type: text/plain +Content-Transfer-Encoding: 7bit +Subject: Some Old Post + +Lorem ipsum dolor sit amet, consectetur adipiscing elit. diff --git a/spec/lib/email/processor_spec.rb b/spec/lib/email/processor_spec.rb index cfd5d659767..c9da1604ae8 100644 --- a/spec/lib/email/processor_spec.rb +++ b/spec/lib/email/processor_spec.rb @@ -180,10 +180,7 @@ RSpec.describe Email::Processor do fab!(:topic) fab!(:post) { Fabricate(:post, topic: topic, created_at: 3.days.ago) } let(:mail) do - file_from_fixtures("old_destination.eml", "emails") - .read - .gsub("424242", topic.id.to_s) - .gsub("123456", post.id.to_s) + file_from_fixtures("old_destination.eml", "emails").read.gsub(":post_id", post.id.to_s) end it "rejects the email with the right response" do diff --git a/spec/lib/email/receiver_spec.rb b/spec/lib/email/receiver_spec.rb index 32e30f2191b..297fb1a6a1d 100644 --- a/spec/lib/email/receiver_spec.rb +++ b/spec/lib/email/receiver_spec.rb @@ -93,7 +93,7 @@ RSpec.describe Email::Receiver do post = Fabricate(:post, topic: topic) user = Fabricate(:user, email: "discourse@bar.com") - mail = email(:old_destination).gsub("424242", topic.id.to_s) + mail = email(:old_destination).gsub(":post_id", post.id.to_s) expect { Email::Receiver.new(mail).process! }.to raise_error( Email::Receiver::BadDestinationAddress, ) @@ -1120,44 +1120,6 @@ RSpec.describe Email::Receiver do Email::Receiver.new(mail_string).process! end - it "posts a reply using a message-id in the format topic/TOPIC_ID/POST_ID@HOST" do - expect { - process_mail_with_message_id("topic/#{topic.id}/#{post.id}@test.localhost") - }.to change { Post.count }.by(1) - expect(topic.reload.posts.last.raw).to include( - "This is email reply testing with Message-ID formats", - ) - end - - it "posts a reply using a message-id in the format topic/TOPIC_ID@HOST" do - expect { process_mail_with_message_id("topic/#{topic.id}@test.localhost") }.to change { - Post.count - }.by(1) - expect(topic.reload.posts.last.raw).to include( - "This is email reply testing with Message-ID formats", - ) - end - - it "posts a reply using a message-id in the format topic/TOPIC_ID/POST_ID.RANDOM_SUFFIX@HOST" do - expect { - process_mail_with_message_id("topic/#{topic.id}/#{post.id}.rjc3yr79834y@test.localhost") - }.to change { Post.count }.by(1) - expect(topic.reload.posts.last.raw).to include( - "This is email reply testing with Message-ID formats", - ) - end - - it "posts a reply using a message-id in the format topic/TOPIC_ID.RANDOM_SUFFIX@HOST" do - expect { - process_mail_with_message_id( - "topic/#{topic.id}/#{post.id}.x3487nxy877843x@test.localhost", - ) - }.to change { Post.count }.by(1) - expect(topic.reload.posts.last.raw).to include( - "This is email reply testing with Message-ID formats", - ) - end - it "posts a reply using a message-id in the format discourse/post/POST_ID@HOST" do expect { process_mail_with_message_id("discourse/post/#{post.id}@test.localhost") diff --git a/spec/lib/imap/sync_spec.rb b/spec/lib/imap/sync_spec.rb index b993e157f3a..b4975abb9ef 100644 --- a/spec/lib/imap/sync_spec.rb +++ b/spec/lib/imap/sync_spec.rb @@ -142,7 +142,7 @@ RSpec.describe Imap::Sync do end context "when the message id matches the receiver post id regex" do - let(:message_id) { "topic/999/324@test.localhost" } + let(:message_id) { "discourse/post/324@test.localhost" } it "does not duplicate incoming email" do incoming_email = Fabricate(:incoming_email, message_id: message_id) diff --git a/spec/lib/message_id_service_spec.rb b/spec/lib/message_id_service_spec.rb index 9ee1cab0b5b..5b2573bdfec 100644 --- a/spec/lib/message_id_service_spec.rb +++ b/spec/lib/message_id_service_spec.rb @@ -52,22 +52,11 @@ RSpec.describe Email::MessageIdService do end describe "find_post_from_message_ids" do - let(:post_format_message_id) { "" } - let(:topic_format_message_id) { "" } let(:discourse_format_message_id) { "" } let(:default_format_message_id) { "<36ac1ddd-5083-461d-b72c-6372fb0e7f33@test.localhost>" } let(:gmail_format_message_id) do "" end - - it "finds a post based only on a post-format message id" do - expect(described_class.find_post_from_message_ids([post_format_message_id])).to eq(post) - end - - it "finds a post based only on a topic-format message id" do - expect(described_class.find_post_from_message_ids([topic_format_message_id])).to eq(post) - end - it "finds a post based only on a discourse-format message id" do expect(described_class.find_post_from_message_ids([discourse_format_message_id])).to eq(post) end @@ -101,16 +90,6 @@ RSpec.describe Email::MessageIdService do incoming_email.post, ) end - - it "gets the last created post if multiple are returned" do - incoming_email = - Fabricate( - :incoming_email, - message_id: described_class.message_id_clean(post_format_message_id), - post: Fabricate(:post, created_at: 10.days.ago), - ) - expect(described_class.find_post_from_message_ids([post_format_message_id])).to eq(post) - end end describe "#discourse_generated_message_id?" do @@ -119,14 +98,6 @@ RSpec.describe Email::MessageIdService do end it "works correctly for the different possible formats" do - expect(check_format("topic/1223/4525.3c4f8n9@test.localhost")).to eq(true) - expect(check_format("")).to eq(true) - expect(check_format("topic/1223.fc3j4843@test.localhost")).to eq(true) - expect(check_format("")).to eq(true) - expect(check_format("topic/1223/4525@test.localhost")).to eq(true) - expect(check_format("")).to eq(true) - expect(check_format("topic/1223@test.localhost")).to eq(true) - expect(check_format("")).to eq(true) expect(check_format("discourse/post/1223@test.localhost")).to eq(true) expect(check_format("")).to eq(true)