Merge branch '7879-requests-sent-to-multiple-addresses' into develop

app/mailers/request_mailer.rb

@@ -252,29 +252,31 @@ def receive(email, raw_email, source = :mailin)
     # Find exact matches for info requests
     exact_info_requests = requests_matching_email(email)
 
-    # Find any guesses for info requests
-    unless exact_info_requests.count == 1
-      guessed_info_requests = Guess.guessed_info_requests(email)
-    end
-
-    # If there is only one info request matching mail, it gets attached to the
-    # request to be archived with it
-    if exact_info_requests.count == 1 || guessed_info_requests.count == 1
-      info_request = exact_info_requests.first || guessed_info_requests.first
-
-      if exact_info_requests.empty? && guessed_info_requests.count == 1
-        info_request.log_event(
-          'redeliver_incoming',
-          editor: 'automatic',
-          destination_request: info_request
-        )
+    if exact_info_requests.count > 0
+      # Go through each exact info request and deliver the email
+      exact_info_requests.each do |info_request|
+        info_request.receive(email, raw_email, opts)
       end
 
+      return
+    end
+
+    # If there are no exact matches, find any guessed requests
+    guessed_info_requests = Guess.guessed_info_requests(email)
+
+    if guessed_info_requests.count == 1
+      # If there one guess automatically redeliver the email to that and log it
+      # as an event
+      info_request = guessed_info_requests.first
+      info_request.log_event(
+        'redeliver_incoming',
+        editor: 'automatic',
+        destination_request: info_request
+      )
       info_request.receive(email, raw_email, opts)
 
     else
-      # Otherwise, if there are no matching IRs, multiple IRs, or multiple IR
-      # guesses, we send the mail to the holding pen
+      # Otherwise we send the mail to the holding pen
       send_to_holding_pen(email, raw_email, opts)
     end
   end

app/models/info_request.rb

@@ -901,18 +901,14 @@ def find_existing_outgoing_message(body)
   end
 
   # Has this email already been received here? Based just on message id.
-  def already_received?(email, _raw_email_data)
-    message_id = email.message_id
-    raise "No message id for this message" if message_id.nil?
-
-    incoming_messages.each do |im|
-      return true if message_id == im.message_id
-    end
-
-    false
+  def already_received?(email)
+    return false unless email.message_id
+    incoming_messages.any? { email.message_id == _1.message_id }
   end
 
   def receive(email, raw_email_data, *args)
+    return if already_received?(email)
+
     defaults = { override_stop_new_responses: false,
                  rejected_reason: nil,
                  source: :internal }

spec/mailers/request_mailer_spec.rb

@@ -43,6 +43,85 @@
       deliveries.clear
     end
 
+    it "should append the email to each exact request address, unless that request has already received the email" do
+      ir = info_requests(:fancy_dog_request)
+      raw_email_data = <<~EML
+        From: EMAIL_FROM
+        To: EMAIL_TO
+        Message-ID: [email protected]
+        Subject: Basic Email
+
+        Hello, World
+      EML
+      expect(ir.incoming_messages.count).to eq(1) # in the fixture
+      receive_incoming_mail(
+        raw_email_data,
+        email_to: ir.incoming_email
+      )
+      expect(ir.incoming_messages.count).to eq(2) # one more arrives
+      # send the email again
+      receive_incoming_mail(
+        raw_email_data,
+        email_to: ir.incoming_email
+      )
+      # this shouldn't add to the number of incoming mails
+      expect(ir.incoming_messages.count).to eq(2)
+      # send an email with a new Message-ID
+      raw_email_data = <<~EML
+        From: EMAIL_FROM
+        To: EMAIL_TO
+        Message-ID: [email protected]
+        Subject: Basic Email
+
+        Hello, World
+      EML
+      receive_incoming_mail(
+        raw_email_data,
+        email_to: ir.incoming_email
+      )
+      # this should add to the number of incoming mails
+      expect(ir.incoming_messages.count).to eq(3)
+    end
+
+    it 'should append the email to every request matches, unless the requests has already received the email' do
+      info_request_1 = FactoryBot.create(:info_request)
+      info_request_2 = FactoryBot.create(:info_request)
+
+      expect(info_request_1.incoming_messages.count).to eq(0)
+      expect(info_request_2.incoming_messages.count).to eq(0)
+
+      raw_email_data = <<~EML
+        From: EMAIL_FROM
+        To: EMAIL_TO
+        Message-ID: [email protected]
+        Subject: Basic Email
+
+        Hello, World
+      EML
+
+      # send email to one request
+      receive_incoming_mail(
+        raw_email_data,
+        email_to: info_request_1.incoming_email
+      )
+
+      expect(info_request_1.incoming_messages.count).to eq(1)
+      expect(info_request_2.incoming_messages.count).to eq(0)
+
+      # send same email to both requests, should only be delivered to the
+      # request which hasn't already received the email
+      receive_incoming_mail(
+        raw_email_data,
+        email_to: [
+          info_request_1.incoming_email,
+          info_request_2.incoming_email
+        ].join(', ')
+      )
+
+      expect(info_request_1.incoming_messages.count).to eq(1)
+      expect(info_request_2.incoming_messages.count).to eq(1)
+    end
+
     it "should store mail in holding pen and send to admin when the email is not to any information request" do
       ir = info_requests(:fancy_dog_request)
       expect(ir.incoming_messages.count).to eq(1)
@@ -92,17 +171,6 @@
       deliveries.clear
     end
 
-    it "puts messages with multiple request addresses in Bcc: in the holding pen" do
-      request1 = FactoryBot.create(:info_request)
-      request2 = FactoryBot.create(:info_request)
-      request3 = FactoryBot.create(:info_request)
-      bcc_addrs = [request1, request2, request3].map(&:incoming_email)
-      receive_incoming_mail('bcc-contact-reply.email',
-                            email_to: 'dummy@localhost',
-                            email_bcc: bcc_addrs.join(', '))
-      expect(InfoRequest.holding_pen_request.incoming_messages.count).to eq(1)
-    end
-
     it "should parse attachments from mails sent with apple mail" do
       ir = info_requests(:fancy_dog_request)
       expect(ir.incoming_messages.count).to eq(1)

spec/models/info_request_spec.rb

@@ -1054,6 +1054,23 @@
       ActionMailer::Base.deliveries.clear
     end
 
+    context 'when email has already been received' do
+
+      let(:info_request) { FactoryBot.create(:info_request) }
+
+      before do
+        allow(info_request).to receive(:already_received?).and_return(true)
+      end
+
+      it 'does not create a new incoming message' do
+        email, raw_email = email_and_raw_email
+        expect { info_request.receive(email, raw_email) }.to_not change {
+          info_request.incoming_messages.count
+        }
+      end
+
+    end
+
   end
 
   describe "#url_title" do
@@ -1567,6 +1584,53 @@
 
   end
 
+  describe '#already_received?' do
+    it 'returns false if email has no Message-ID header' do
+      info_request = FactoryBot.build(:info_request)
+
+      email = Mail.new(
+        <<~EML
+          Subject: Basic Email
+
+          Hello, World
+        EML
+      )
+
+      expect(info_request.already_received?(email)).to eq(false)
+    end
+
+    it 'returns false if a message with the same Message-ID has not been received' do
+      info_request = FactoryBot.build(:info_request)
+
+      email = Mail.new(
+        <<~EML
+          Message-ID: [email protected]
+          Subject: Basic Email
+
+          Hello, World
+        EML
+      )
+
+      expect(info_request.already_received?(email)).to eq(false)
+    end
+
+    it 'returns true if a message with the same Message-ID has already been received' do
+      info_request = FactoryBot.create(:info_request)
+
+      raw_email_data = <<~EML
+        Message-ID: [email protected]
+        Subject: Basic Email
+
+        Hello, World
+      EML
+      email, raw_email = email_and_raw_email(raw_email: raw_email_data)
+
+      info_request.receive(email, raw_email)
+      expect(info_request.incoming_messages.count).to eq(1)
+      expect(info_request.already_received?(email)).to eq(true)
+    end
+  end
+
   describe '#is_external?' do
 
     it 'returns true if there is an external url' do

spec/support/email_helpers.rb

@@ -9,7 +9,7 @@ def load_raw_emails_data
 def receive_incoming_mail(email_name_or_string, **kargs)
   kargs[:email_from] ||= 'geraldinequango@localhost'
   content = load_file_fixture(email_name_or_string) || email_name_or_string
-  content = gsub_addresses(content, **kargs)
+  content = gsub_addresses(content.dup, **kargs)
   content = ::Mail::Utilities.binary_unsafe_to_crlf(content)
   RequestMailer.receive(content)
 end