diff options
| -rw-r--r-- | app/controllers/admin_general_controller.rb | 33 | ||||
| -rw-r--r-- | app/mailers/request_mailer.rb | 3 | ||||
| -rw-r--r-- | app/models/info_request.rb | 29 | ||||
| -rw-r--r-- | app/models/user.rb | 1 | ||||
| -rw-r--r-- | app/views/admin_general/index.html.erb | 99 | ||||
| -rw-r--r-- | config/initializers/alaveteli.rb | 2 | ||||
| -rw-r--r-- | spec/mailers/request_mailer_spec.rb | 21 | ||||
| -rw-r--r-- | spec/models/info_request_spec.rb | 29 |
8 files changed, 151 insertions, 66 deletions
diff --git a/app/controllers/admin_general_controller.rb b/app/controllers/admin_general_controller.rb index 13edec37d..4927f631b 100644 --- a/app/controllers/admin_general_controller.rb +++ b/app/controllers/admin_general_controller.rb @@ -7,28 +7,29 @@ class AdminGeneralController < AdminController def index - # Overview counts of things - @public_body_count = PublicBody.count - - @info_request_count = InfoRequest.count - @outgoing_message_count = OutgoingMessage.count - @incoming_message_count = IncomingMessage.count - - @user_count = User.count - @track_thing_count = TrackThing.count - - @comment_count = Comment.count - # Tasks to do @requires_admin_requests = InfoRequest.find_in_state('requires_admin') @error_message_requests = InfoRequest.find_in_state('error_message') @attention_requests = InfoRequest.find_in_state('attention_requested') - @blank_contacts = PublicBody.where(:request_email => "").order(:updated_at).select { |pb| !pb.defunct? } + @blank_contacts = PublicBody. + includes(:tags, :translations). + where(:request_email => ""). + order(:updated_at). + select { |pb| !pb.defunct? } @old_unclassified = InfoRequest.find_old_unclassified(:limit => 20, :conditions => ["prominence = 'normal'"]) - @holding_pen_messages = InfoRequest.holding_pen_request.incoming_messages - @new_body_requests = PublicBodyChangeRequest.new_body_requests.open - @body_update_requests = PublicBodyChangeRequest.body_update_requests.open + @holding_pen_messages = InfoRequest. + includes(:incoming_messages => :raw_email). + holding_pen_request. + incoming_messages + @new_body_requests = PublicBodyChangeRequest. + includes(:public_body, :user). + new_body_requests. + open + @body_update_requests = PublicBodyChangeRequest. + includes(:public_body, :user). + body_update_requests. + open end def timeline diff --git a/app/mailers/request_mailer.rb b/app/mailers/request_mailer.rb index bf04b1ab9..8f9f91a1c 100644 --- a/app/mailers/request_mailer.rb +++ b/app/mailers/request_mailer.rb @@ -234,8 +234,9 @@ class RequestMailer < ApplicationMailer def requests_matching_email(email) # We deliberately don't use Envelope-to here, so ones that are BCC # drop into the holding pen for checking. + addresses = ((email.to || []) + (email.cc || [])).compact reply_info_requests = [] # TODO: should be set? - for address in (email.to || []) + (email.cc || []) + addresses.each do |address| reply_info_request = InfoRequest.find_by_incoming_email(address) reply_info_requests.push(reply_info_request) if reply_info_request end diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 01d5f5c52..02faffcfa 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -428,6 +428,7 @@ public # A new incoming email to this request def receive(email, raw_email_data, override_stop_new_responses = false, rejected_reason = "") + # Is this request allowing responses? if !override_stop_new_responses allow = nil reason = nil @@ -458,9 +459,15 @@ public raise "Unknown allow_new_responses_from '" + self.allow_new_responses_from + "'" end + # If its not allowing responses, handle the message if !allow if self.handle_rejected_responses == 'bounce' - RequestMailer.stopped_responses(self, email, raw_email_data).deliver if !is_external? + if MailHandler.get_from_address(email).nil? + # do nothing – can't bounce the mail as there's no + # address to send it to + else + RequestMailer.stopped_responses(self, email, raw_email_data).deliver if !is_external? + end elsif self.handle_rejected_responses == 'holding_pen' InfoRequest.holding_pen_request.receive(email, raw_email_data, false, reason) elsif self.handle_rejected_responses == 'blackhole' @@ -777,7 +784,14 @@ public end def public_response_events - self.info_request_events.select{|e| e.response? && e.incoming_message.all_can_view? } + condition = <<-SQL + info_request_events.event_type = ? + AND incoming_messages.prominence = ? + SQL + + info_request_events. + joins(:incoming_message). + where(condition, 'response', 'normal') end # The last public response is the default one people might want to reply to @@ -926,7 +940,7 @@ public # Called by incoming_email - and used to be called to generate separate # envelope from address until we abandoned it. def magic_email(prefix_part) - raise "id required to make magic" if not self.id + raise "id required to create a magic email" if not self.id return InfoRequest.magic_email_for_id(prefix_part, self.id) end @@ -1384,6 +1398,15 @@ public end end + # The DateTime of the last InfoRequestEvent belonging to the InfoRequest + # Only available if the last_event_time attribute has been set. This is + # currentlt only set through .find_in_state + # + # Returns a DateTime + def last_event_time + attributes['last_event_time'].try(:to_datetime) + end + private def set_defaults diff --git a/app/models/user.rb b/app/models/user.rb index 1fb5d9139..bb40dbc26 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,3 +1,4 @@ +# -*- encoding : utf-8 -*- # == Schema Information # # Table name: users diff --git a/app/views/admin_general/index.html.erb b/app/views/admin_general/index.html.erb index ba0563bb6..8840bce74 100644 --- a/app/views/admin_general/index.html.erb +++ b/app/views/admin_general/index.html.erb @@ -2,17 +2,10 @@ <div class="row"> <div class="span12"> - <h1><%=@title%></h1> - - <ul> - <li><%=@public_body_count%> public authorities</li> - <li> - <%=@info_request_count%> requests, <%=@outgoing_message_count%> outgoing messages, - <%=@incoming_message_count%> incoming messages - </li> - <li><%=@user_count%> users, <%=@track_thing_count%> tracked things</li> - <li><%=@comment_count%> annotations</li> - </ul> + <h1><%= @title %></h1> + <p> + <%= link_to 'Summary stats have moved →', admin_stats_path %> + </p> </div> </div> @@ -28,8 +21,14 @@ <% if @holding_pen_messages.size > 0 %> <div class="accordion-group"> <div class="accordion-heading"> - <a class="accordion-toggle" href="#holding-pen" data-toggle="collapse" data-parent="things-to-do"><span class="label label-important"><%=@holding_pen_messages.size%></span><%= chevron_right %> Put misdelivered responses with the right request</a> + <a class="accordion-toggle" href="#holding-pen" data-toggle="collapse" data-parent="things-to-do"> + <span class="label label-important"> + <%= @holding_pen_messages.size %> + </span> + <%= chevron_right %> Put misdelivered responses with the right request + </a> </div> + <div id="holding-pen" class="accordion-body collapse"> <table class="table table-striped table-condensed"> <tbody> @@ -39,11 +38,13 @@ <% if message.get_body_for_quoting.strip.size == 0 %> <%= link_to "(no body)", admin_raw_email_path(message.raw_email_id) %> <% else %> - <%= link_to excerpt(message.get_body_for_quoting, "", :radius => 60), admin_raw_email_path(message.raw_email_id) %> + <%= link_to admin_raw_email_path(message.raw_email_id) do %> + <%= excerpt(message.get_body_for_quoting, "", :radius => 60) %> + <% end %> <% end %> </td> <td class="span2"> - <%=simple_date(message.sent_at)%> + <%= simple_date(message.sent_at) %> </td> </tr> <% end %> @@ -67,7 +68,7 @@ <%= request_both_links(@request) %> </td> <td class="span2"> - <%=simple_date(@request.info_request_events.last.created_at)%> + <%= simple_date(@request.last_event_time) %> </td> </tr> <% end %> @@ -92,7 +93,7 @@ <%= request_both_links(@request) %> </td> <td class="span2"> - <%=simple_date(@request.info_request_events.last.created_at)%> + <%= simple_date(@request.last_event_time) %> </td> </tr> <% end %> @@ -116,7 +117,7 @@ <%= request_both_links(@request) %> </td> <td class="span2"> - <%=simple_date(@request.info_request_events.last.created_at)%> + <%= simple_date(@request.last_event_time) %> </td> </tr> <% end %> @@ -178,18 +179,33 @@ <% if @new_body_requests.size > 0 %> <div class="accordion-group"> <div class="accordion-heading"> - <a class="accordion-toggle" href="#new-authorities" data-toggle="collapse" data-parent="things-to-do"><span class="label label-important"><%= @new_body_requests.size %></span><%= chevron_right %> Add new authorities</a> + <a class="accordion-toggle" href="#new-authorities" data-toggle="collapse" data-parent="things-to-do"> + <span class="label label-important"> + <%= @new_body_requests.size %> + </span> + <%= chevron_right %> Add new authorities + </a> </div> + <div id="new-authorities" class="accordion-body collapse"> - <% for @change_request in @new_body_requests %> - <%= render :partial => 'change_request_summary'%> - <%= form_tag admin_change_request_path(@change_request), :method => 'put', :class => "form form-horizontal" do %> - <%= submit_tag 'Close', :class => "btn btn-danger" %> - <%= link_to("Close and respond", edit_admin_change_request_path(@change_request), :class => 'btn') %> - <%= link_to("Add authority", new_admin_body_path(:change_request_id => @change_request.id), :class => 'btn btn-primary') %> - <% end %> + <% for @change_request in @new_body_requests %> + <%= render :partial => 'change_request_summary'%> - <% end %> + <%= form_tag admin_change_request_path(@change_request), + :method => 'put', + :class => "form form-horizontal" do %> + + <%= submit_tag 'Close', :class => "btn btn-danger" %> + + <%= link_to "Close and respond", + edit_admin_change_request_path(@change_request), + :class => 'btn' %> + + <%= link_to "Add authority", + new_admin_body_path(:change_request_id => @change_request.id), + :class => 'btn btn-primary' %> + <% end %> + <% end %> </div> </div> <% end %> @@ -197,17 +213,34 @@ <% if @body_update_requests.size > 0 %> <div class="accordion-group"> <div class="accordion-heading"> - <a class="accordion-toggle" href="#update-authorities" data-toggle="collapse" data-parent="things-to-do"><span class="label label-important"><%= @body_update_requests.size %></span><%= chevron_right %> Update authorities</a> + <a class="accordion-toggle" href="#update-authorities" data-toggle="collapse" data-parent="things-to-do"> + <span class="label label-important"> + <%= @body_update_requests.size %> + </span> + <%= chevron_right %> Update authorities + </a> </div> + <div id="update-authorities" class="accordion-body collapse"> <% for @change_request in @body_update_requests %> - <%= render :partial => 'change_request_summary' %> - <%= form_tag admin_change_request_path(@change_request), :class => "form form-horizontal", :method => 'put' do %> - <%= submit_tag 'Close', :class => "btn btn-danger" %> - <%= link_to("Close and respond", edit_admin_change_request_path(@change_request), :class => 'btn') %> - <%= link_to("Make update", edit_admin_body_path(@change_request.public_body, :change_request_id => @change_request.id), :class => 'btn btn-primary') %> - <% end %> + <%= render :partial => 'change_request_summary' %> + + <%= form_tag admin_change_request_path(@change_request), + :class => "form form-horizontal", + :method => 'put' do %> + + <%= submit_tag 'Close', :class => "btn btn-danger" %> + + <%= link_to "Close and respond", + edit_admin_change_request_path(@change_request), + :class => 'btn' %> + + <%= link_to "Make update", + edit_admin_body_path(@change_request.public_body, + :change_request_id => @change_request.id), + :class => 'btn btn-primary' %> <% end %> + <% end %> </div> </div> <% end %> diff --git a/config/initializers/alaveteli.rb b/config/initializers/alaveteli.rb index 53a9456d2..71169b684 100644 --- a/config/initializers/alaveteli.rb +++ b/config/initializers/alaveteli.rb @@ -10,7 +10,7 @@ load "debug_helpers.rb" load "util.rb" # Application version -ALAVETELI_VERSION = '0.21.0.27' +ALAVETELI_VERSION = '0.21.0.30' # Add new inflection rules using the following format # (all these examples are active by default): diff --git a/spec/mailers/request_mailer_spec.rb b/spec/mailers/request_mailer_spec.rb index 6b54c25d2..0c7622fd1 100644 --- a/spec/mailers/request_mailer_spec.rb +++ b/spec/mailers/request_mailer_spec.rb @@ -40,6 +40,12 @@ describe RequestMailer, " when receiving incoming mail" do deliveries.clear end + it "puts messages with a malformed To: in the holding pen" do + request = FactoryGirl.create(:info_request) + receive_incoming_mail('incoming-request-plain.email', 'asdfg') + expect(InfoRequest.holding_pen_request.incoming_messages).to have(1).item + end + it "should parse attachments from mails sent with apple mail" do ir = info_requests(:fancy_dog_request) ir.incoming_messages.size.should == 1 @@ -180,6 +186,21 @@ describe RequestMailer, " when receiving incoming mail" do deliveries.clear end + it "discards rejected responses with a malformed From: when set to bounce" do + ir = info_requests(:fancy_dog_request) + ir.allow_new_responses_from = 'nobody' + ir.handle_rejected_responses = 'bounce' + ir.save! + ir.incoming_messages.size.should == 1 + + receive_incoming_mail('incoming-request-plain.email', ir.incoming_email, "") + ir.incoming_messages.size.should == 1 + + deliveries = ActionMailer::Base.deliveries + deliveries.size.should == 0 + deliveries.clear + end + it "should send all new responses to holding pen if a request is marked to do so" do # mark request as anti-spam ir = info_requests(:fancy_dog_request) diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index 1ead1e0bf..941561c81 100644 --- a/spec/models/info_request_spec.rb +++ b/spec/models/info_request_spec.rb @@ -658,17 +658,22 @@ describe InfoRequest do before do Time.stub!(:now).and_return(Time.utc(2007, 11, 9, 23, 59)) - @mock_comment_event = mock_model(InfoRequestEvent, :created_at => Time.now - 23.days, - :event_type => 'comment', - :response? => false) - mock_incoming_message = mock_model(IncomingMessage, :all_can_view? => true) - @mock_response_event = mock_model(InfoRequestEvent, :created_at => Time.now - 22.days, - :event_type => 'response', - :response? => true, - :incoming_message => mock_incoming_message) - @info_request = InfoRequest.new(:prominence => 'normal', - :awaiting_description => true, - :info_request_events => [@mock_response_event, @mock_comment_event]) + @info_request = FactoryGirl.create(:info_request, + :prominence => 'normal', + :awaiting_description => true) + @comment_event = FactoryGirl.create(:info_request_event, + :created_at => Time.now - 23.days, + :event_type => 'comment', + :info_request => @info_request) + @incoming_message = FactoryGirl.create(:incoming_message, + :prominence => 'normal', + :info_request => @info_request) + @response_event = FactoryGirl.create(:info_request_event, + :info_request => @info_request, + :created_at => Time.now - 22.days, + :event_type => 'response', + :incoming_message => @incoming_message) + @info_request.update_attribute(:awaiting_description, true) end it 'should return false if it is the holding pen' do @@ -682,7 +687,7 @@ describe InfoRequest do end it 'should return false if its last response event occurred less than 21 days ago' do - @mock_response_event.stub!(:created_at).and_return(Time.now - 20.days) + @response_event.update_attribute(:created_at, Time.now - 20.days) @info_request.is_old_unclassified?.should be_false end |