diff options
| -rw-r--r-- | app/controllers/request_controller.rb | 28 | ||||
| -rw-r--r-- | app/models/info_request.rb | 5 | ||||
| -rw-r--r-- | spec/controllers/request_controller_spec.rb | 66 |
3 files changed, 76 insertions, 23 deletions
diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index 9cad01edf..c4fb9e030 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/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: request_controller.rb,v 1.186 2009-10-01 01:43:38 francis Exp $ +# $Id: request_controller.rb,v 1.187 2009-10-01 12:01:26 francis Exp $ class RequestController < ApplicationController @@ -266,7 +266,7 @@ class RequestController < ApplicationController @old_unclassified = @info_request.is_old_unclassified? && !authenticated_user.nil? # Check authenticated, and parameters set. We check is_owning_user - # to get admin overrides (see owns_every_request? above) + # to get admin overrides (see is_owning_user? above) if !@old_unclassified && !@is_owning_user && !authenticated_as_user?(@info_request.user, :web => "To classify the response to this FOI request", :email => "Then you can classify the FOI response you have got from " + @info_request.public_body.name + ".", @@ -292,30 +292,28 @@ class RequestController < ApplicationController old_described_state = @info_request.described_state @info_request.set_described_state(params[:incoming_message][:described_state]) - # Log it if not made by user - if authenticated_user != @info_request.user + # If you're not the *actual* requester owner. e.g. you are playing the + # classification game, or you're doing this just because you are an + # admin user (not because you also own the request). + if !@info_request.is_actual_owning_user?(authenticated_user) + # Log what you did, for classification game score purposes. We + # don't log if you were the requester XXX This is presumably so you + # don't score for classifying your own requests. Could instead + # always log and filter at display time. @info_request.log_event("status_update", { :user_id => authenticated_user.id, :old_described_state => old_described_state, :described_state => @info_request.described_state, }) - end - - # TODO harmonise the next two methods? - if User.owns_every_request?(authenticated_user) && !@is_owning_user - flash[:notice] = '<p>The request status has been updated</p>' - redirect_to session[:request_game] ? play_url : request_url(@info_request) - return - end - - if @old_unclassified && !@is_owning_user + + # Don't give advice on what to do next, as it isn't their request flash[:notice] = '<p>Thank you for updating this request!</p>' RequestMailer.deliver_old_unclassified_updated(@info_request) redirect_to session[:request_game] ? play_url : request_url(@info_request) return end - # Display appropriate next page (e.g. help for complaint etc.) + # Display advice for requester on what to do next, as appropriate if @info_request.calculate_status == 'waiting_response' flash[:notice] = "<p>Thank you! Hopefully your wait isn't too long.</p> <p>By law, you should get a response before the end of <strong>" + simple_date(@info_request.date_response_required_by) + "</strong>.</p>" redirect_to request_url(@info_request) diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 02517a1fb..17a19d0e4 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -24,7 +24,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.207 2009-09-17 21:10:05 francis Exp $ +# $Id: info_request.rb,v 1.208 2009-10-01 12:01:26 francis Exp $ require 'digest/sha1' require File.join(File.dirname(__FILE__),'../../vendor/plugins/acts_as_xapian/lib/acts_as_xapian') @@ -844,6 +844,9 @@ public def is_owning_user?(user) !user.nil? && (user.id == user_id || user.owns_every_request?) end + def is_actual_owning_user?(user) + !user.nil? && user.id == user_id + end def user_can_view?(user) if self.prominence == 'hidden' diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index 917826d45..3d9a2940a 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -526,7 +526,7 @@ describe RequestController, "when classifying an information request" do end end - describe 'when logged in as an admin user' do + describe 'when logged in as an admin user who is not the actual requester' do before do @admin_user = users(:admin_user) @@ -548,19 +548,62 @@ describe RequestController, "when classifying an information request" do @dog_request.should_receive(:log_event).with("status_update", expected_params) post_status('rejected') end - - it 'should show the message "The request status has been updated"' do + + it 'should send an email to the requester letting them know someone has updated the status of their request' do + RequestMailer.should_receive(:deliver_old_unclassified_updated) post_status('rejected') - flash[:notice].should == '<p>The request status has been updated</p>' end - - it 'should redirect to the page that shows the request' do + + it 'should redirect to the request page' do post_status('rejected') response.should redirect_to(:action => 'show', :controller => 'request', :url_title => @dog_request.url_title) end - end + it 'should show a message thanking the user for a good deed' do + post_status('rejected') + flash[:notice].should == '<p>Thank you for updating this request!</p>' + end + end + + describe 'when logged in as an admin user who is also the actual requester' do + before do + @admin_user = users(:admin_user) + session[:user_id] = @admin_user.id + @dog_request = info_requests(:fancy_dog_request) + @dog_request.user = @admin_user + @dog_request.save! + InfoRequest.stub!(:find).and_return(@dog_request) + end + + it 'should update the status of the request' do + @dog_request.stub!(:calculate_status).and_return('rejected') + @dog_request.should_receive(:set_described_state).with('rejected') + post_status('rejected') + end + + it 'should not log a status update event' do + @dog_request.should_not_receive(:log_event) + post_status('rejected') + end + + it 'should not send an email to the requester letting them know someone has updated the status of their request' do + RequestMailer.should_not_receive(:deliver_old_unclassified_updated) + post_status('rejected') + end + + it 'should say it is showing advice as to what to do next' do + post_status('rejected') + flash[:notice].should match(/Here is what to do now/) + end + + it 'should redirect to the unhappy page' do + post_status('rejected') + response.should redirect_to(:controller => 'help', :action => 'unhappy', :url_title => @dog_request.url_title) + end + + end + describe 'when logged in as the requestor' do before do @@ -604,7 +647,16 @@ describe RequestController, "when classifying an information request" do mail.body.should =~ /as needing admin/ mail.from_addrs.to_s.should == @request_owner.name_and_email end + + it 'should say it is showing advice as to what to do next' do + post_status('rejected') + flash[:notice].should match(/Here is what to do now/) + end + it 'should redirect to the unhappy page' do + post_status('rejected') + response.should redirect_to(:controller => 'help', :action => 'unhappy', :url_title => @dog_request.url_title) + end end describe 'when redirecting after a successful status update by the request owner' do |