diff options
| -rw-r--r-- | app/controllers/admin_request_controller.rb | 5 | ||||
| -rw-r--r-- | app/models/info_request.rb | 15 | ||||
| -rw-r--r-- | app/models/request_mailer.rb | 5 | ||||
| -rw-r--r-- | app/views/admin_request/show_raw_email.rhtml | 5 | ||||
| -rw-r--r-- | spec/controllers/admin_request_controller_spec.rb | 19 | ||||
| -rw-r--r-- | spec/models/request_mailer_spec.rb | 44 |
6 files changed, 84 insertions, 9 deletions
diff --git a/app/controllers/admin_request_controller.rb b/app/controllers/admin_request_controller.rb index d5bd4c4d6..a617b06df 100644 --- a/app/controllers/admin_request_controller.rb +++ b/app/controllers/admin_request_controller.rb @@ -275,7 +275,6 @@ class AdminRequestController < AdminController def show_raw_email @raw_email = RawEmail.find(params[:id]) - # For the holding pen, try to guess where it should be ... @holding_pen = false if (@raw_email.incoming_message.info_request == InfoRequest.holding_pen_request && !@raw_email.incoming_message.mail.from_addrs.nil? && @raw_email.incoming_message.mail.from_addrs.size > 0) @@ -303,6 +302,10 @@ class AdminRequestController < AdminController for address in addresses @info_requests += InfoRequest.guess_by_incoming_email(address) end + + # 3. Give a reason why it's in the holding pen + last_event = @raw_email.incoming_message.info_request.get_last_event + @rejected_reason = last_event.params[:rejected_reason] end end diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 209954b16..158e6319e 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -379,21 +379,24 @@ public end # A new incoming email to this request - def receive(email, raw_email_data, override_stop_new_responses = false) + def receive(email, raw_email_data, override_stop_new_responses = false, rejected_reason = "") if !override_stop_new_responses allow = nil - + reason = nil # See if new responses are prevented for spam reasons if self.allow_new_responses_from == 'nobody' allow = false + reason = _('This request has been set by an administrator to "allow new responses from nobody"') elsif self.allow_new_responses_from == 'anybody' allow = true elsif self.allow_new_responses_from == 'authority_only' if email.from_addrs.nil? || email.from_addrs.size == 0 allow = false + reason = _('Only the authority can reply to this request, but there is no "From" address to check against') else sender_email = email.from_addrs[0].spec sender_domain = PublicBody.extract_domain_from_email(sender_email) + reason = _("Only the authority can reply to this request, and I don't recognise the address this reply was sent from") allow = false # Allow any domain that has already sent reply for row in self.who_can_followup_to @@ -411,7 +414,7 @@ public if self.handle_rejected_responses == 'bounce' RequestMailer.deliver_stopped_responses(self, email, raw_email_data) elsif self.handle_rejected_responses == 'holding_pen' - InfoRequest.holding_pen_request.receive(email, raw_email_data) + InfoRequest.holding_pen_request.receive(email, raw_email_data, false, reason) elsif self.handle_rejected_responses == 'blackhole' # do nothing - just lose the message (Note: a copy will be # in the backup mailbox if the server is configured to send @@ -435,7 +438,11 @@ public raw_email.save! self.awaiting_description = true - self.log_event("response", { :incoming_message_id => incoming_message.id }) + params = { :incoming_message_id => incoming_message.id } + if !rejected_reason.empty? + params[:rejected_reason] = rejected_reason + end + self.log_event("response", params) self.save! end diff --git a/app/models/request_mailer.rb b/app/models/request_mailer.rb index e73b153b9..fc317d20d 100644 --- a/app/models/request_mailer.rb +++ b/app/models/request_mailer.rb @@ -205,10 +205,11 @@ class RequestMailer < ApplicationMailer def receive(email, raw_email) # Find which info requests the email is for reply_info_requests = self.requests_matching_email(email) - # Nothing found, so save in holding pen if reply_info_requests.size == 0 - InfoRequest.holding_pen_request.receive(email, raw_email) + reason = _("Could not identify the request from the email address") + request = InfoRequest.holding_pen_request + request.receive(email, raw_email, false, reason) return end diff --git a/app/views/admin_request/show_raw_email.rhtml b/app/views/admin_request/show_raw_email.rhtml index 10b97f4fd..fa1470e77 100644 --- a/app/views/admin_request/show_raw_email.rhtml +++ b/app/views/admin_request/show_raw_email.rhtml @@ -3,8 +3,9 @@ <h1>Incoming message <%=@raw_email.incoming_message.id.to_s %></h1> <p> - FOI request: <%= link_to request_both_links(@raw_email.incoming_message.info_request) %> + FOI request: <%= request_both_links(@raw_email.incoming_message.info_request) %> <% if @holding_pen %> + <br>This is in the holding pen because: <strong><%= @rejected_reason %></strong> <% if @public_bodies.size > 0 %> <br>Guessed authority: <% for public_body in @public_bodies %> @@ -30,7 +31,7 @@ <h2>Raw email</h2> -<%= link_to "Download", "../download_raw_email/" + @raw_email.id.to_s %> +<p><%= link_to "Download", "../download_raw_email/" + @raw_email.id.to_s %></p> <pre><%=h(@raw_email.data).gsub(/\n/, '<br>') %></pre> diff --git a/spec/controllers/admin_request_controller_spec.rb b/spec/controllers/admin_request_controller_spec.rb index d82e4a49c..0b12fa97a 100644 --- a/spec/controllers/admin_request_controller_spec.rb +++ b/spec/controllers/admin_request_controller_spec.rb @@ -39,3 +39,22 @@ describe AdminRequestController, "when administering requests" do end +describe AdminRequestController, "when administering the holding pen" do + integrate_views + fixtures :info_requests, :incoming_messages, :raw_emails, :users, :public_bodies, :public_body_translations + before(:each) do + basic_auth_login @request + load_raw_emails_data(raw_emails) + end + + it "shows a rejection reason for an incoming message from an invalid address" do + ir = info_requests(:fancy_dog_request) + ir.allow_new_responses_from = 'authority_only' + ir.handle_rejected_responses = 'holding_pen' + ir.save! + receive_incoming_mail('incoming-request-plain.email', ir.incoming_email, "frob@nowhere.com") + get :show_raw_email, :id => InfoRequest.holding_pen_request.get_last_response.raw_email.id + response.should have_text(/Only the authority can reply to this request/) + end + +end diff --git a/spec/models/request_mailer_spec.rb b/spec/models/request_mailer_spec.rb index a5f59b9bf..fbe22c220 100644 --- a/spec/models/request_mailer_spec.rb +++ b/spec/models/request_mailer_spec.rb @@ -27,7 +27,49 @@ describe RequestMailer, " when receiving incoming mail" do receive_incoming_mail('incoming-request-plain.email', 'dummy@localhost') ir.incoming_messages.size.should == 1 InfoRequest.holding_pen_request.incoming_messages.size.should == 1 + last_event = InfoRequest.holding_pen_request.incoming_messages[0].info_request.get_last_event + last_event.params[:rejected_reason].should == "Could not identify the request from the email address" + + deliveries = ActionMailer::Base.deliveries + deliveries.size.should == 1 + mail = deliveries[0] + mail.to.should == [ MySociety::Config.get("CONTACT_EMAIL", 'contact@localhost') ] + deliveries.clear + end + it "should store mail in holding pen and send to admin when the from email is empty and only authorites can reply" do + ir = info_requests(:fancy_dog_request) + ir.allow_new_responses_from = 'authority_only' + ir.handle_rejected_responses = 'holding_pen' + ir.save! + ir.incoming_messages.size.should == 1 + InfoRequest.holding_pen_request.incoming_messages.size.should == 0 + receive_incoming_mail('incoming-request-plain.email', ir.incoming_email, "") + ir.incoming_messages.size.should == 1 + InfoRequest.holding_pen_request.incoming_messages.size.should == 1 + last_event = InfoRequest.holding_pen_request.incoming_messages[0].info_request.get_last_event + last_event.params[:rejected_reason].should =~ /there is no "From" address/ + + deliveries = ActionMailer::Base.deliveries + deliveries.size.should == 1 + mail = deliveries[0] + mail.to.should == [ MySociety::Config.get("CONTACT_EMAIL", 'contact@localhost') ] + deliveries.clear + end + + it "should store mail in holding pen and send to admin when the from email is unknown and only authorites can reply" do + ir = info_requests(:fancy_dog_request) + ir.allow_new_responses_from = 'authority_only' + ir.handle_rejected_responses = 'holding_pen' + ir.save! + ir.incoming_messages.size.should == 1 + InfoRequest.holding_pen_request.incoming_messages.size.should == 0 + receive_incoming_mail('incoming-request-plain.email', ir.incoming_email, "frob@nowhere.com") + ir.incoming_messages.size.should == 1 + InfoRequest.holding_pen_request.incoming_messages.size.should == 1 + last_event = InfoRequest.holding_pen_request.incoming_messages[0].info_request.get_last_event + last_event.params[:rejected_reason].should =~ /Only the authority can reply/ + deliveries = ActionMailer::Base.deliveries deliveries.size.should == 1 mail = deliveries[0] @@ -108,6 +150,8 @@ describe RequestMailer, " when receiving incoming mail" do receive_incoming_mail('incoming-request-plain.email', ir.incoming_email) ir.incoming_messages.size.should == 1 InfoRequest.holding_pen_request.incoming_messages.size.should == 1 # arrives in holding pen + last_event = InfoRequest.holding_pen_request.incoming_messages[0].info_request.get_last_event + last_event.params[:rejected_reason].should =~ /allow new responses from nobody/ # should be a message to admin regarding holding pen deliveries = ActionMailer::Base.deliveries |