diff options
| -rw-r--r-- | app/controllers/admin_general_controller.rb | 6 | ||||
| -rw-r--r-- | app/controllers/admin_request_controller.rb | 5 | ||||
| -rw-r--r-- | app/models/info_request.rb | 46 | ||||
| -rw-r--r-- | app/views/admin_general/index.rhtml | 3 | ||||
| -rw-r--r-- | app/views/admin_request/list_old_unclassified.rhtml | 4 | ||||
| -rw-r--r-- | spec/models/info_request_spec.rb | 42 |
6 files changed, 67 insertions, 39 deletions
diff --git a/app/controllers/admin_general_controller.rb b/app/controllers/admin_general_controller.rb index cbb381e21..bcfd11e03 100644 --- a/app/controllers/admin_general_controller.rb +++ b/app/controllers/admin_general_controller.rb @@ -4,7 +4,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: admin_general_controller.rb,v 1.4 2009-04-08 10:45:33 louise Exp $ +# $Id: admin_general_controller.rb,v 1.5 2009-04-08 16:13:11 louise Exp $ class AdminGeneralController < AdminController def index @@ -19,7 +19,9 @@ class AdminGeneralController < AdminController @requires_admin_requests = InfoRequest.find(:all, :select => '*, ' + InfoRequest.last_event_time_clause + ' as last_event_time', :conditions => ["described_state = 'requires_admin'"], :order => "last_event_time") @error_message_requests = InfoRequest.find(:all, :select => '*, ' + InfoRequest.last_event_time_clause + ' as last_event_time', :conditions => ["described_state = 'error_message'"], :order => "last_event_time") @blank_contacts = PublicBody.find(:all, :conditions => ["request_email = ''"], :order => "updated_at") - @ten_days_old_unclassified = InfoRequest.find_old_unclassified(limit=50) + @ten_days_old_unclassified = InfoRequest.find_old_unclassified(:limit => 50, + :conditions => ["prominence != 'backpage'"], + :age_in_days => 10) @holding_pen_messages = InfoRequest.holding_pen_request.incoming_messages end diff --git a/app/controllers/admin_request_controller.rb b/app/controllers/admin_request_controller.rb index 15faacac2..dfd44f32b 100644 --- a/app/controllers/admin_request_controller.rb +++ b/app/controllers/admin_request_controller.rb @@ -4,7 +4,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: admin_request_controller.rb,v 1.33 2009-04-08 10:45:33 louise Exp $ +# $Id: admin_request_controller.rb,v 1.34 2009-04-08 16:13:11 louise Exp $ class AdminRequestController < AdminController def index @@ -19,7 +19,8 @@ class AdminRequestController < AdminController end def list_old_unclassified - @info_requests = InfoRequest.find_old_unclassified + @info_requests = InfoRequest.find_old_unclassified(:conditions => ["prominence != 'backpage'"], + :age_in_days => 10) end def show diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 7ea355a18..fed3d8801 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -23,7 +23,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: info_request.rb,v 1.184 2009-04-08 10:45:34 louise Exp $ +# $Id: info_request.rb,v 1.185 2009-04-08 16:13:11 louise Exp $ require 'digest/sha1' require File.join(File.dirname(__FILE__),'../../vendor/plugins/acts_as_xapian/lib/acts_as_xapian') @@ -74,7 +74,7 @@ class InfoRequest < ActiveRecord::Base 'eir', # Environmental Information Regulations ] - OLD_AGE_IN_DAYS = 10.days + OLD_AGE_IN_DAYS = 14.days def after_initialize if self.described_state.nil? @@ -672,25 +672,39 @@ public end # Used to find when event last changed - def InfoRequest.last_event_time_clause - '(select created_at from info_request_events where info_request_events.info_request_id = info_requests.id order by created_at desc limit 1)' - end - - def InfoRequest.find_old_unclassified(limit=nil) - params = {:select => "*, #{last_event_time_clause} as last_event_time", - :conditions => ["awaiting_description = ? and #{last_event_time_clause} < ? and prominence != 'backpage'", - true, Time.now() - OLD_AGE_IN_DAYS], - :order => "last_event_time"} - params[:limit] = limit if limit + def InfoRequest.last_event_time_clause(event_type=nil) + event_type_clause = '' + event_type_clause = " and info_request_events.event_type = '#{event_type}'" if event_type + "(select created_at from info_request_events where info_request_events.info_request_id = info_requests.id#{event_type_clause} order by created_at desc limit 1)" + end + + def InfoRequest.find_old_unclassified(extra_params={}) + last_response_created_at = last_event_time_clause('response') + age = extra_params[:age_in_days] ? extra_params[:age_in_days].days : OLD_AGE_IN_DAYS + params = {:select => "*, #{last_response_created_at} as last_response_time", + :conditions => ["awaiting_description = ? and #{last_response_created_at} < ? and url_title != 'holding_pen'", + true, Time.now() - age], + :order => "last_response_time"} + params[:limit] = extra_params[:limit] if extra_params[:limit] + params[:include] = extra_params[:include] if extra_params[:include] + if extra_params[:order] + params[:order] = extra_params[:order] + params.delete(:select) + end + if extra_params[:conditions] + condition_string = extra_params[:conditions].shift + params[:conditions][0] += " and #{condition_string}" + params[:conditions] += extra_params[:conditions] + end find(:all, params) end def is_old_unclassified? return false if !awaiting_description - return false if prominence == 'backpage' - last_event = get_last_event - return false unless last_event - return false if last_event.created_at >= Time.now - OLD_AGE_IN_DAYS + return false if url_title == 'holding_pen' + last_response_event = get_last_response_event + return false unless last_response_event + return false if last_response_event.created_at >= Time.now - OLD_AGE_IN_DAYS return true end diff --git a/app/views/admin_general/index.rhtml b/app/views/admin_general/index.rhtml index 87de83752..636a36c48 100644 --- a/app/views/admin_general/index.rhtml +++ b/app/views/admin_general/index.rhtml @@ -39,10 +39,11 @@ <% for @request in @ten_days_old_unclassified %> <li> <%= request_both_links(@request) %> - – <%=simple_date(@request.get_last_event.created_at)%> + – <%=simple_date(@request.get_last_response_event.created_at)%> </li> <% end %> </ul> + <p>(<%= link_to "Full list", admin_url("unclassified") %>) <% end %> diff --git a/app/views/admin_request/list_old_unclassified.rhtml b/app/views/admin_request/list_old_unclassified.rhtml index 3db40528e..38f2822e0 100644 --- a/app/views/admin_request/list_old_unclassified.rhtml +++ b/app/views/admin_request/list_old_unclassified.rhtml @@ -1,4 +1,4 @@ -<% @title = "Unclassfied responses" %> +<% @title = "Unclassified responses" %> <h1><%=@title%></h1> @@ -7,7 +7,7 @@ <% for @request in @info_requests %> <li> <%= request_both_links(@request) %> - – <%=simple_date(@request.get_last_event.created_at)%> + – <%=simple_date(@request.get_last_response_event.created_at)%> </li> <% end %> </ul> diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index cbcf7eecc..09b258ee4 100644 --- a/spec/models/info_request_spec.rb +++ b/spec/models/info_request_spec.rb @@ -195,7 +195,7 @@ describe InfoRequest do :order => anything, :conditions=> anything, :limit => 5}) - InfoRequest.find_old_unclassified(limit=5) + InfoRequest.find_old_unclassified(:limit => 5) end it 'should not limit the number of requests returned by default' do @@ -206,12 +206,21 @@ describe InfoRequest do InfoRequest.find_old_unclassified end - it 'should ask the database for requests that are awaiting description, older than 10 days old and not backpaged' do + it 'should add extra conditions if supplied' do InfoRequest.should_receive(:find).with(:all, - {:select=>"*, (select created_at from info_request_events where info_request_events.info_request_id = info_requests.id order by created_at desc limit 1) as last_event_time", - :order=>"last_event_time", - :conditions=>["awaiting_description = ? and (select created_at from info_request_events where info_request_events.info_request_id = info_requests.id order by created_at desc limit 1) < ? and prominence != 'backpage'", - true, Time.now - 10.days]}) + {:select=> anything, + :order=> anything, + :conditions=>["awaiting_description = ? and (select created_at from info_request_events where info_request_events.info_request_id = info_requests.id and info_request_events.event_type = 'response' order by created_at desc limit 1) < ? and url_title != 'holding_pen' and prominence != 'backpage'", + true, Time.now - 14.days]}) + InfoRequest.find_old_unclassified({:conditions => ["prominence != 'backpage'"]}) + end + + it 'should ask the database for requests that are awaiting description, have a last response older than 14 days old, are not the holding pen and are not backpaged' do + InfoRequest.should_receive(:find).with(:all, + {:select=>"*, (select created_at from info_request_events where info_request_events.info_request_id = info_requests.id and info_request_events.event_type = 'response' order by created_at desc limit 1) as last_response_time", + :order=>"last_response_time", + :conditions=>["awaiting_description = ? and (select created_at from info_request_events where info_request_events.info_request_id = info_requests.id and info_request_events.event_type = 'response' order by created_at desc limit 1) < ? and url_title != 'holding_pen'", + true, Time.now - 14.days]}) InfoRequest.find_old_unclassified end @@ -221,28 +230,29 @@ describe InfoRequest do before do Time.stub!(:now).and_return(Time.utc(2007, 11, 12, 23, 59)) - @mock_event = mock_model(InfoRequestEvent, :created_at => Time.now - 11.days) + @mock_comment_event = mock_model(InfoRequestEvent, :created_at => Time.now - 16.days, :event_type => 'comment') + @mock_response_event = mock_model(InfoRequestEvent, :created_at => Time.now - 15.days, :event_type => 'response') @info_request = InfoRequest.new(:prominence => 'normal', :awaiting_description => true, - :info_request_events => [@mock_event]) + :info_request_events => [@mock_response_event, @mock_comment_event]) end - it 'should return false if it is not awaiting description' do - @info_request.stub!(:awaiting_description).and_return(false) + it 'should return false if it is the holding pen' do + @info_request.stub!(:url_title).and_return('holding_pen') @info_request.is_old_unclassified?.should be_false end - it 'should return false if it\'s last event occurred less than 10 days ago' do - @mock_event.stub!(:created_at).and_return(Time.now - 9.days) + it 'should return false if it is not awaiting description' do + @info_request.stub!(:awaiting_description).and_return(false) @info_request.is_old_unclassified?.should be_false end - it 'should return false if it\'s backpaged' do - @info_request.stub!(:prominence).and_return('backpage') - @info_request.is_old_unclassified?.should be_false + it 'should return false if it\'s last response event occurred less than 14 days ago' do + @mock_response_event.stub!(:created_at).and_return(Time.now - 13.days) + @info_request.is_old_unclassified?.should be_false end - it 'should return true if it is awaiting description, hasn\'t had an event in 10 days and is not backpaged' do + it 'should return true if it is awaiting description, isn\'t the holding pen and hasn\'t had an event in 14 days' do @info_request.is_old_unclassified?.should be_true end |