diff options
| -rw-r--r-- | app/controllers/request_controller.rb | 2 | ||||
| -rwxr-xr-x | app/helpers/link_to_helper.rb | 2 | ||||
| -rw-r--r-- | app/mailers/request_mailer.rb | 8 | ||||
| -rw-r--r-- | app/models/info_request.rb | 34 | ||||
| -rw-r--r-- | app/views/admin_general/index.html.erb | 2 | ||||
| -rw-r--r-- | spec/controllers/admin_request_controller_spec.rb | 5 | ||||
| -rw-r--r-- | spec/controllers/request_controller_spec.rb | 14 | ||||
| -rw-r--r-- | spec/mailers/request_mailer_spec.rb | 10 | ||||
| -rw-r--r-- | spec/models/info_request_spec.rb | 63 | ||||
| -rw-r--r-- | spec/views/request/show.html.erb_spec.rb | 88 |
10 files changed, 145 insertions, 83 deletions
diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index a09939509..ac92801b9 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -919,7 +919,7 @@ class RequestController < ApplicationController @last_info_request_event_id = info_request.last_event_id_needing_description @new_responses_count = info_request.events_needing_description.select {|i| i.event_type == 'response'}.size # For send followup link at bottom - @last_response = info_request.get_last_response + @last_response = info_request.get_last_public_response end def make_request_zip(info_request, file_path) diff --git a/app/helpers/link_to_helper.rb b/app/helpers/link_to_helper.rb index 5533402c5..8df28f350 100755 --- a/app/helpers/link_to_helper.rb +++ b/app/helpers/link_to_helper.rb @@ -53,7 +53,7 @@ module LinkToHelper # Respond to request def respond_to_last_url(info_request, options = {}) - last_response = info_request.get_last_response + last_response = info_request.get_last_public_response if last_response.nil? show_response_no_followup_url(options.merge(:id => info_request.id)) else diff --git a/app/mailers/request_mailer.rb b/app/mailers/request_mailer.rb index 4dbce6738..13b3bc4a1 100644 --- a/app/mailers/request_mailer.rb +++ b/app/mailers/request_mailer.rb @@ -347,8 +347,8 @@ class RequestMailer < ApplicationMailer :age_in_days => days_since) for info_request in info_requests - alert_event_id = info_request.get_last_response_event_id - last_response_message = info_request.get_last_response + alert_event_id = info_request.get_last_public_response_event_id + last_response_message = info_request.get_last_public_response if alert_event_id.nil? raise "internal error, no last response while making alert new response reminder, request id " + info_request.id.to_s end @@ -373,8 +373,8 @@ class RequestMailer < ApplicationMailer def self.alert_not_clarified_request() info_requests = InfoRequest.find(:all, :conditions => [ "awaiting_description = ? and described_state = 'waiting_clarification' and info_requests.updated_at < ?", false, Time.now() - 3.days ], :include => [ :user ], :order => "info_requests.id" ) for info_request in info_requests - alert_event_id = info_request.get_last_response_event_id - last_response_message = info_request.get_last_response + alert_event_id = info_request.get_last_public_response_event_id + last_response_message = info_request.get_last_public_response if alert_event_id.nil? raise "internal error, no last response while making alert not clarified reminder, request id " + info_request.id.to_s end diff --git a/app/models/info_request.rb b/app/models/info_request.rb index fe0c94056..847a57ef4 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -733,28 +733,30 @@ public self.info_request_events.create!(:event_type => type, :params => params) end - def response_events - self.info_request_events.select{|e| e.response?} + def public_response_events + self.info_request_events.select{|e| e.response? && e.incoming_message.all_can_view? } end - # The last response is the default one people might want to reply to - def get_last_response_event_id - get_last_response_event.id if get_last_response_event + # The last public response is the default one people might want to reply to + def get_last_public_response_event_id + get_last_public_response_event.id if get_last_public_response_event end - def get_last_response_event - response_events.last + + def get_last_public_response_event + public_response_events.last end - def get_last_response - get_last_response_event.incoming_message if get_last_response_event + + def get_last_public_response + get_last_public_response_event.incoming_message if get_last_public_response_event end - def outgoing_events - info_request_events.select{|e| e.outgoing? } + def public_outgoing_events + info_request_events.select{|e| e.outgoing? && e.outgoing_message.all_can_view? } end - # The last outgoing message - def get_last_outgoing_event - outgoing_events.last + # The last public outgoing message + def get_last_public_outgoing_event + public_outgoing_events.last end # Text from the the initial request, for use in summary display @@ -989,8 +991,8 @@ public end def is_old_unclassified? - !is_external? && awaiting_description && url_title != 'holding_pen' && get_last_response_event && - Time.now > get_last_response_event.created_at + OLD_AGE_IN_DAYS + !is_external? && awaiting_description && url_title != 'holding_pen' && get_last_public_response_event && + Time.now > get_last_public_response_event.created_at + OLD_AGE_IN_DAYS end # List of incoming messages to followup, by unique email diff --git a/app/views/admin_general/index.html.erb b/app/views/admin_general/index.html.erb index b239a2b3f..976860fa7 100644 --- a/app/views/admin_general/index.html.erb +++ b/app/views/admin_general/index.html.erb @@ -164,7 +164,7 @@ <%= request_both_links(@request) %> </td> <td class="span2"> - <%=simple_date(@request.get_last_response_event.created_at)%> + <%=simple_date(@request.get_last_public_response_event.created_at)%> </td> </tr> <% end %> diff --git a/spec/controllers/admin_request_controller_spec.rb b/spec/controllers/admin_request_controller_spec.rb index 42b4bcbc1..c374ff90d 100644 --- a/spec/controllers/admin_request_controller_spec.rb +++ b/spec/controllers/admin_request_controller_spec.rb @@ -77,7 +77,7 @@ describe AdminRequestController, "when administering the holding pen" do 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 + get :show_raw_email, :id => InfoRequest.holding_pen_request.get_last_public_response.raw_email.id response.should contain "Only the authority can reply to this request" end @@ -88,7 +88,8 @@ describe AdminRequestController, "when administering the holding pen" do ir.save! mail_to = "request-#{ir.id}-asdfg@example.com" receive_incoming_mail('incoming-request-plain.email', mail_to) - interesting_email = InfoRequest.holding_pen_request.get_last_response.raw_email.id + interesting_email = InfoRequest.holding_pen_request.get_last_public_response +.raw_email.id # now we add another message to the queue, which we're not interested in receive_incoming_mail('incoming-request-plain.email', ir.incoming_email, "") InfoRequest.holding_pen_request.incoming_messages.length.should == 2 diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index c5ee8cbf7..ec10d99d8 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -1596,7 +1596,7 @@ describe RequestController, "when classifying an information request" do @dog_request.reload @dog_request.awaiting_description.should == false @dog_request.described_state.should == 'rejected' - @dog_request.get_last_response_event.should == info_request_events(:useless_incoming_message_event) + @dog_request.get_last_public_response_event.should == info_request_events(:useless_incoming_message_event) @dog_request.info_request_events.last.event_type.should == "status_update" @dog_request.info_request_events.last.calculated_state.should == 'rejected' end @@ -1749,13 +1749,13 @@ describe RequestController, "when classifying an information request" do it 'should redirect to the "response url" when there is a last response' do incoming_message = mock_model(IncomingMessage) - @dog_request.stub!(:get_last_response).and_return(incoming_message) + @dog_request.stub!(:get_last_public_response).and_return(incoming_message) expect_redirect('waiting_clarification', "request/#{@dog_request.id}/response/#{incoming_message.id}") end it 'should redirect to the "response no followup url" when there are no events needing description' do - @dog_request.stub!(:get_last_response).and_return(nil) + @dog_request.stub!(:get_last_public_response).and_return(nil) expect_redirect('waiting_clarification', "request/#{@dog_request.id}/response") end @@ -1794,7 +1794,7 @@ describe RequestController, "when classifying an information request" do context 'when status is updated to "gone postal"' do it 'should redirect to the "respond to last url"' do - expect_redirect('gone_postal', "request/#{@dog_request.id}/response/#{@dog_request.get_last_response.id}?gone_postal=1") + expect_redirect('gone_postal', "request/#{@dog_request.id}/response/#{@dog_request.get_last_public_response.id}?gone_postal=1") end end @@ -1836,7 +1836,7 @@ describe RequestController, "when classifying an information request" do context 'when status is updated to "user_withdrawn"' do it 'should redirect to the "respond to last url url" ' do - expect_redirect('user_withdrawn', "request/#{@dog_request.id}/response/#{@dog_request.get_last_response.id}") + expect_redirect('user_withdrawn', "request/#{@dog_request.id}/response/#{@dog_request.get_last_public_response.id}") end end @@ -1889,7 +1889,7 @@ describe RequestController, "when sending a followup message" do # fake that this is a clarification info_requests(:fancy_dog_request).set_described_state('waiting_clarification') info_requests(:fancy_dog_request).described_state.should == 'waiting_clarification' - info_requests(:fancy_dog_request).get_last_response_event.calculated_state.should == 'waiting_clarification' + info_requests(:fancy_dog_request).get_last_public_response_event.calculated_state.should == 'waiting_clarification' # make the followup session[:user_id] = users(:bob_smith_user).id @@ -1907,7 +1907,7 @@ describe RequestController, "when sending a followup message" do # and that the status changed info_requests(:fancy_dog_request).reload info_requests(:fancy_dog_request).described_state.should == 'waiting_response' - info_requests(:fancy_dog_request).get_last_response_event.calculated_state.should == 'waiting_clarification' + info_requests(:fancy_dog_request).get_last_public_response_event.calculated_state.should == 'waiting_clarification' end it "should give an error if the same followup is submitted twice" do diff --git a/spec/mailers/request_mailer_spec.rb b/spec/mailers/request_mailer_spec.rb index 23806b35b..f30beae82 100644 --- a/spec/mailers/request_mailer_spec.rb +++ b/spec/mailers/request_mailer_spec.rb @@ -204,10 +204,10 @@ describe RequestMailer, "when sending reminders to requesters to classify a resp before do Time.stub!(:now).and_return(Time.utc(2007, 11, 12, 23, 59)) @mock_event = mock_model(InfoRequestEvent) - @mock_response = mock_model(IncomingMessage) + @mock_response = mock_model(IncomingMessage, :user_can_view? => true) @mock_user = mock_model(User) - @mock_request = mock_model(InfoRequest, :get_last_response_event_id => @mock_event.id, - :get_last_response => @mock_response, + @mock_request = mock_model(InfoRequest, :get_last_public_response_event_id => @mock_event.id, + :get_last_public_response => @mock_response, :user_id => 2, :url_title => 'test_title', :user => @mock_user) @@ -252,7 +252,7 @@ describe RequestMailer, "when sending reminders to requesters to classify a resp end it 'should raise an error if a request does not have a last response event id' do - @mock_request.stub!(:get_last_response_event_id).and_return(nil) + @mock_request.stub!(:get_last_public_response_event_id).and_return(nil) expected_message = "internal error, no last response while making alert new response reminder, request id #{@mock_request.id}" lambda{ send_alerts }.should raise_error(expected_message) end @@ -289,7 +289,7 @@ describe RequestMailer, "when sending reminders to requesters to classify a resp mock_sent_alert.should_receive(:info_request=).with(@mock_request) mock_sent_alert.should_receive(:user=).with(@mock_user) mock_sent_alert.should_receive(:alert_type=).with('new_response_reminder_1') - mock_sent_alert.should_receive(:info_request_event_id=).with(@mock_request.get_last_response_event_id) + mock_sent_alert.should_receive(:info_request_event_id=).with(@mock_request.get_last_public_response_event_id) mock_sent_alert.should_receive(:save!) send_alerts end diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index 2846fa6dc..a6f877b20 100644 --- a/spec/models/info_request_spec.rb +++ b/spec/models/info_request_spec.rb @@ -451,8 +451,14 @@ 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_response_event = mock_model(InfoRequestEvent, :created_at => Time.now - 22.days, :event_type => 'response', :response? => true) + @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]) @@ -589,6 +595,59 @@ describe InfoRequest do end end + describe 'when asked for the last public response event' do + + before do + @info_request = FactoryGirl.create(:info_request_with_incoming) + @incoming_message = @info_request.incoming_messages.first + end + + it 'should not return an event with a hidden prominence message' do + @incoming_message.prominence = 'hidden' + @incoming_message.save! + @info_request.get_last_public_response_event.should == nil + end + + it 'should not return an event with a requester_only prominence message' do + @incoming_message.prominence = 'requester_only' + @incoming_message.save! + @info_request.get_last_public_response_event.should == nil + end + + it 'should return an event with a normal prominence message' do + @incoming_message.prominence = 'normal' + @incoming_message.save! + @info_request.get_last_public_response_event.should == @incoming_message.response_event + end + end + + describe 'when asked for the last public outgoing event' do + + before do + @info_request = FactoryGirl.create(:info_request) + @outgoing_message = @info_request.outgoing_messages.first + end + + it 'should not return an event with a hidden prominence message' do + @outgoing_message.prominence = 'hidden' + @outgoing_message.save! + @info_request.get_last_public_outgoing_event.should == nil + end + + it 'should not return an event with a requester_only prominence message' do + @outgoing_message.prominence = 'requester_only' + @outgoing_message.save! + @info_request.get_last_public_outgoing_event.should == nil + end + + it 'should return an event with a normal prominence message' do + @outgoing_message.prominence = 'normal' + @outgoing_message.save! + @info_request.get_last_public_outgoing_event.should == @outgoing_message.info_request_events.first + end + + end + describe 'when generating json for the api' do before do diff --git a/spec/views/request/show.html.erb_spec.rb b/spec/views/request/show.html.erb_spec.rb index 4578268b2..6e63b9b43 100644 --- a/spec/views/request/show.html.erb_spec.rb +++ b/spec/views/request/show.html.erb_spec.rb @@ -1,8 +1,8 @@ require File.expand_path(File.join('..', '..', '..', 'spec_helper'), __FILE__) describe 'request/show' do - - before do + + before do @mock_body = mock_model(PublicBody, :name => 'test body', :url_name => 'test_body', :is_school? => false) @@ -10,101 +10,101 @@ describe 'request/show' do :url_name => 'test_user', :profile_photo => nil) @mock_request = mock_model(InfoRequest, :title => 'test request', - :awaiting_description => false, + :awaiting_description => false, :law_used_with_a => 'A Freedom of Information request', :law_used_full => 'Freedom of Information', :public_body => @mock_body, - :user => @mock_user, - :user_name => @mock_user.name, + :user => @mock_user, + :user_name => @mock_user.name, :is_external? => false, - :calculate_status => 'waiting_response', + :calculate_status => 'waiting_response', :date_response_required_by => Date.today, :prominence => 'normal', :comments_allowed? => true, :all_can_view? => true, :url_title => 'test_request') end - + def request_page assign :info_request, @mock_request assign :info_request_events, [] assign :status, @mock_request.calculate_status render end - - describe 'when a status update has been requested' do - - before do + + describe 'when a status update has been requested' do + + before do assign :update_status, true end - + it 'should show the first form for describing the state of the request' do request_page response.should have_selector("div.describe_state_form#describe_state_form_1") - end - + end + end - - describe 'when it is awaiting a description' do - - before do + + describe 'when it is awaiting a description' do + + before do @mock_request.stub!(:awaiting_description).and_return(true) end - + it 'should show the first form for describing the state of the request' do request_page response.should have_selector("div.describe_state_form#describe_state_form_1") end - - it 'should show the second form for describing the state of the request' do + + it 'should show the second form for describing the state of the request' do request_page response.should have_selector("div.describe_state_form#describe_state_form_2") end - + end - - describe 'when the user is the request owner' do - - before do + + describe 'when the user is the request owner' do + + before do assign :is_owning_user, true end - - describe 'when the request status is "waiting clarification"' do - - before do + + describe 'when the request status is "waiting clarification"' do + + before do @mock_request.stub!(:calculate_status).and_return('waiting_clarification') end - - describe 'when there is a last response' do - + + describe 'when there is a last response' do + before do @mock_response = mock_model(IncomingMessage) - @mock_request.stub!(:get_last_response).and_return(@mock_response) + @mock_request.stub!(:get_last_public_response).and_return(@mock_response) end - - it 'should show a link to follow up the last response with clarification' do + + it 'should show a link to follow up the last response with clarification' do request_page expected_url = "/en/request/#{@mock_request.id}/response/#{@mock_response.id}#followup" response.should have_selector("a", :href => expected_url, :content => 'send a follow up message') end - + end - + describe 'when there is no last response' do - - before do - @mock_request.stub!(:get_last_response).and_return(nil) + + before do + @mock_request.stub!(:get_last_public_response).and_return(nil) end - - it 'should show a link to follow up the request without reference to a specific response' do + + it 'should show a link to follow up the request without reference to a specific response' do request_page expected_url = "/en/request/#{@mock_request.id}/response#followup" response.should have_selector("a", :href => expected_url, :content => 'send a follow up message') end end end - + end end |