diff --git a/app/mailers/request_mailer.rb b/app/mailers/request_mailer.rb index 30fc16d10be..b6bb18b9af4 100644 --- a/app/mailers/request_mailer.rb +++ b/app/mailers/request_mailer.rb @@ -209,6 +209,27 @@ def comment_on_alert_plural(info_request, count, earliest_unalerted_comment) ) end + # Find which info requests the email is for + def requests_matching_email(email) + addresses = MailHandler.get_all_addresses(email) + addresses.concat MailHandler.get_emails_within_received_headers(email) + InfoRequest.matching_incoming_email(addresses.uniq) + end + + def guess_email_request_address(email) + # Match the email address in the message without matching the hash + guess_addresses = MailHandler.get_all_addresses(email) + guessed_info_requests = + InfoRequest.guess_by_incoming_email(guess_addresses) + + # Match the email subject in the message + guess_by_subject = + InfoRequest.guess_by_incoming_subject(email.subject) + (guessed_info_requests + guess_by_subject). + select(&:info_request).uniq(&:info_request). + map { |ir_guess| ir_guess.info_request } + end + # Class function, called by script/mailin with all incoming responses. # [ This is a copy (Monkeypatch!) of function from action_mailer/base.rb, # but which additionally passes the raw_email to the member function, as we @@ -228,14 +249,6 @@ def self.receive(raw_email, source = :mailin) mail = MailHandler.mail_from_raw_email(raw_email) new.receive(mail, raw_email, source) end - - # Find which info requests the email is for - def requests_matching_email(email) - addresses = MailHandler.get_all_addresses(email) - addresses.concat MailHandler.get_emails_within_received_headers(email) - InfoRequest.matching_incoming_email(addresses.uniq) - end - # Member function, called on the new class made in self.receive above def receive(email, raw_email, source = :mailin) opts = { source: source } @@ -243,16 +256,29 @@ def receive(email, raw_email, source = :mailin) # Find which info requests the email is for reply_info_requests = requests_matching_email(email) - # Nothing found OR multiple different info requests, so save in holding pen + # Nothing found OR multiple different info requests, so a candidate for the + # holding pen if reply_info_requests.empty? or reply_info_requests.count > 1 - opts[:rejected_reason] = - _("Could not identify the request from the email address") - request = InfoRequest.holding_pen_request - + # Guess which requests this COULD be related to + guessed_info_requests = guess_email_request_address(email) + # If only one request is guessed, then this is should be automatically + # attached to that request + if guessed_info_requests.count != 1 + opts[:rejected_reason] = + _("Could not identify the request from the email address") + request = InfoRequest.holding_pen_request unless SpamAddress.spam?(MailHandler.get_all_addresses(email)) request.receive(email, raw_email, opts) end return + else + reply_info_requests = guessed_info_requests + reply_info_requests[0].log_event( + 'redeliver_incoming', + editor: 'automatic', + destination_request: reply_info_requests[0].id, + ) + end end # Send the message to each request, to be archived with it diff --git a/lib/mail_handler/backends/mail_backend.rb b/lib/mail_handler/backends/mail_backend.rb index a8f2e9a0ac8..964e1466b1f 100644 --- a/lib/mail_handler/backends/mail_backend.rb +++ b/lib/mail_handler/backends/mail_backend.rb @@ -405,7 +405,7 @@ def get_emails_within_received_headers(email) received_header_string = received_headers.to_s else received_header_string = received_headers. - map { | header | header.to_s } * ' ' + map(&:to_s) * ' ' end received_addresses = received_header_string. scan(MySociety::Validate.email_find_regexp).flatten diff --git a/spec/integration/admin_spec.rb b/spec/integration/admin_spec.rb index e298236a4a3..edfc4c0a962 100644 --- a/spec/integration/admin_spec.rb +++ b/spec/integration/admin_spec.rb @@ -127,26 +127,6 @@ expect(page).to have_content "Only the authority can reply to this request" end end - - it "guesses a misdirected request" do - info_request = FactoryBot.create(:info_request, - allow_new_responses_from: 'authority_only', - handle_rejected_responses: 'holding_pen') - mail_to = "request-#{info_request.id}-asdfg@example.com" - receive_incoming_mail('incoming-request-plain.email', email_to: mail_to) - interesting_email = last_holding_pen_mail - - # now we add another message to the queue, which we're not interested in - receive_incoming_mail('incoming-request-plain.email', - email_to: info_request.incoming_email, - email_from: "") - expect(holding_pen_messages.length).to eq(2) - using_session(@admin) do - visit admin_raw_email_path interesting_email - expect(page).to have_content "Could not identify the request" - expect(page).to have_content info_request.title - end - end end describe 'generating an upload url' do diff --git a/spec/mailers/request_mailer_spec.rb b/spec/mailers/request_mailer_spec.rb index 07a93433d3c..5354a0ce6cb 100644 --- a/spec/mailers/request_mailer_spec.rb +++ b/spec/mailers/request_mailer_spec.rb @@ -20,7 +20,26 @@ deliveries = ActionMailer::Base.deliveries expect(deliveries.size).to eq(1) mail = deliveries[0] - expect(mail.to).to eq([ 'bob@localhost' ]) # to the user who sent fancy_dog_request + # to the user who sent fancy_dog_request + expect(mail.to).to eq(['bob@localhost']) + deliveries.clear + end + + it "should append it to the appropriate request if there is only one guess of information request" do + ir = info_requests(:fancy_dog_request) + expect(ir.incoming_messages.count).to eq(1) # in the fixture + receive_incoming_mail( + 'incoming-request-plain.email', + email_to: "request-#{ir.id}-#{ir.idhash}a@localhost" + ) + expect(ir.incoming_messages.count).to eq(2) # one more arrives + expect(ir.info_request_events[-1].incoming_message_id).not_to be_nil + + deliveries = ActionMailer::Base.deliveries + expect(deliveries.size).to eq(1) + mail = deliveries[0] + # to the user who sent fancy_dog_request + expect(mail.to).to eq(['bob@localhost']) deliveries.clear end @@ -48,7 +67,6 @@ expect(InfoRequest.holding_pen_request.incoming_messages.count).to eq(1) end - it "attaches messages with an info request address in the Received headers to the appropriate request" do ir = info_requests(:fancy_dog_request) expect(ir.incoming_messages.count).to eq(1) # in the fixture @@ -69,7 +87,8 @@ deliveries = ActionMailer::Base.deliveries expect(deliveries.size).to eq(1) mail = deliveries[0] - expect(mail.to).to eq([ 'bob@localhost' ]) # to the user who sent fancy_dog_request + # to the user who sent fancy_dog_request + expect(mail.to).to eq(['bob@localhost']) deliveries.clear end