diff options
| -rw-r--r-- | app/controllers/request_controller.rb | 39 | ||||
| -rw-r--r-- | app/views/request/describe_state_message.rhtml | 30 | ||||
| -rw-r--r-- | config/routes.rb | 1 | ||||
| -rw-r--r-- | spec/controllers/request_controller_spec.rb | 34 | ||||
| -rw-r--r-- | spec/integration/request_controller_spec.rb | 41 |
5 files changed, 121 insertions, 24 deletions
diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index c5788d131..dbfb12fe1 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -406,9 +406,18 @@ class RequestController < ApplicationController return end + described_state = params[:incoming_message][:described_state] + message = params[:incoming_message][:message] + # For requires_admin and error_message states we ask for an extra message to send to + # the administrators. + # If this message hasn't been included then ask for it + if ["error_message", "requires_admin"].include?(described_state) && message.nil? + redirect_to describe_state_message_url(:url_title => info_request.url_title, :described_state => described_state) + return + end + # Make the state change - info_request.set_described_state(params[:incoming_message][:described_state], authenticated_user, - params[:incoming_message][:message]) + info_request.set_described_state(described_state, authenticated_user, message) # If you're not the *actual* requester. e.g. you are playing the # classification game, or you're doing this just because you are an @@ -460,16 +469,15 @@ class RequestController < ApplicationController nil when 'internal_review' _("<p>Thank you! Hopefully your wait isn't too long.</p><p>You should get a response within {{late_number_of_days}} days, or be told if it will take longer (<a href=\"{{review_url}}\">details</a>).</p>",:late_number_of_days => Configuration.reply_late_after_days, :review_url => unhappy_url(info_request) + "#internal_review") - when 'error_message' - _("<p>Thank you! We'll look into what happened and try and fix it up.</p><p>If the error was a delivery failure, and you can find an up to date FOI email address for the authority, please tell us using the form below.</p>") - when 'requires_admin' - _("Please use the form below to tell us more.") + when 'error_message', 'requires_admin' + _("Thank you! We'll look into what happened and try and fix it up.") when 'user_withdrawn' _("If you have not done so already, please write a message below telling the authority that you have withdrawn your request. Otherwise they will not know it has been withdrawn.") end case info_request.calculate_status - when 'waiting_response', 'waiting_response_overdue', 'not_held', 'successful', 'internal_review' + when 'waiting_response', 'waiting_response_overdue', 'not_held', 'successful', + 'internal_review', 'error_message', 'requires_admin' redirect_to request_url(info_request) when 'waiting_response_very_overdue', 'rejected', 'partially_successful' redirect_to unhappy_url(info_request) @@ -477,8 +485,6 @@ class RequestController < ApplicationController redirect_to respond_to_last_url(info_request) when 'gone_postal' redirect_to respond_to_last_url(info_request) + "?gone_postal=1" - when 'error_message', 'requires_admin' - redirect_to help_general_url(:action => 'contact') else if @@custom_states_loaded return self.theme_describe_state(info_request) @@ -488,6 +494,21 @@ class RequestController < ApplicationController end end + # Collect a message to include with the change of state + def describe_state_message + @info_request = InfoRequest.find_by_url_title!(params[:url_title]) + @described_state = params[:described_state] + @last_info_request_event_id = @info_request.last_event_id_needing_description + @title = case @described_state + when "error_message" + _("I've received an error message") + when "requires_admin" + _("This request requires administrator attention") + else + raise "Unsupported state" + end + end + # Used for links from polymorphic URLs e.g. in Atom feeds - just redirect to # proper URL for the message the event refers to def show_request_event diff --git a/app/views/request/describe_state_message.rhtml b/app/views/request/describe_state_message.rhtml new file mode 100644 index 000000000..1427fea22 --- /dev/null +++ b/app/views/request/describe_state_message.rhtml @@ -0,0 +1,30 @@ +<h1><%= @title %></h1> + + +<p> + <% if @described_state == "error_message" %> + <%= _("If the error was a delivery failure, and you can find an up to date FOI email address for the authority, please tell us using the form below.") %> + <% else %> + <%= _("Just one more thing") %> + <% end %> +</p> + +<% form_for :incoming_message, :url => describe_state_url(:id => @info_request.id) do |f| %> + + <p> + <label class="form_label" for="incoming_message_message">Please tell us more:</label> + <%= f.text_area :message, :rows => 10, :cols => 60 %> + </p> + + <div> + <%= hidden_field_tag "incoming_message[described_state]", @described_state %> + <%= hidden_field_tag :last_info_request_event_id, @last_info_request_event_id %> + </div> + + <div class="form_button"> + <%= submit_tag _("Submit status and send message") %> + </div> + +<% end %> + + diff --git a/config/routes.rb b/config/routes.rb index 0a0736eaa..3476a6f87 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -55,6 +55,7 @@ ActionController::Routing::Routes.draw do |map| request.similar_request '/similar/request/:url_title', :action => 'similar' request.describe_state '/request/:id/describe', :action => 'describe_state', :conditions => {:method => :post} + request.describe_state_message '/request/:url_title/describe/:described_state', :action => 'describe_state_message' request.show_response_no_followup '/request/:id/response', :action => 'show_response' request.show_response '/request/:id/response/:incoming_message_id', :action => 'show_response' request.get_attachment_as_html '/request/:id/response/:incoming_message_id/attach/html/:part/*file_name', :action => 'get_attachment_as_html' diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index c42a3a3f2..f35523532 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -1359,7 +1359,7 @@ describe RequestController, "when classifying an information request" do end it "should send a mail from the user who changed the state to requires_admin" do - post :describe_state, :incoming_message => { :described_state => "requires_admin" }, :id => @dog_request.id, :incoming_message_id => incoming_messages(:useless_incoming_message), :last_info_request_event_id => @dog_request.last_event_id_needing_description + post :describe_state, :incoming_message => { :described_state => "requires_admin", :message => "a message" }, :id => @dog_request.id, :incoming_message_id => incoming_messages(:useless_incoming_message), :last_info_request_event_id => @dog_request.last_event_id_needing_description deliveries = ActionMailer::Base.deliveries deliveries.size.should == 1 @@ -1503,20 +1503,14 @@ describe RequestController, "when classifying an information request" do post_status('rejected') end - it "should send email when classified as requires_admin" do + it "should go to the page asking for more information when classified as requires_admin" do post :describe_state, :incoming_message => { :described_state => "requires_admin" }, :id => @dog_request.id, :incoming_message_id => incoming_messages(:useless_incoming_message), :last_info_request_event_id => @dog_request.last_event_id_needing_description - response.should redirect_to(:controller => 'help', :action => 'contact') + response.should redirect_to describe_state_message_url(:url_title => @dog_request.url_title, :described_state => "requires_admin") @dog_request.reload - @dog_request.awaiting_description.should == false - @dog_request.described_state.should == 'requires_admin' - @dog_request.get_last_response_event.calculated_state.should == 'requires_admin' + @dog_request.described_state.should_not == 'requires_admin' - deliveries = ActionMailer::Base.deliveries - deliveries.size.should == 1 - mail = deliveries[0] - mail.body.should =~ /as needing admin/ - mail.from_addrs.first.to_s.should == @request_owner.name_and_email + ActionMailer::Base.deliveries.should be_empty end context "message is included when classifying as requires_admin" do @@ -1647,12 +1641,22 @@ describe RequestController, "when classifying an information request" do expect_redirect('internal_review', request_url) end - it 'should redirect to the "help general url" when status is updated to "requires admin"' do - expect_redirect('requires_admin', "help/contact") + it 'should redirect to the "request url" when status is updated to "requires admin"' do + post :describe_state, :incoming_message => { + :described_state => 'requires_admin', + :message => "A message" }, + :id => @dog_request.id, + :last_info_request_event_id => @dog_request.last_event_id_needing_description + response.should redirect_to show_request_url(:url_title => @dog_request.url_title) end - it 'should redirect to the "help general url" when status is updated to "error message"' do - expect_redirect('error_message', "help/contact") + it 'should redirect to the "request url" when status is updated to "error message"' do + post :describe_state, :incoming_message => { + :described_state => 'error_message', + :message => "A message" }, + :id => @dog_request.id, + :last_info_request_event_id => @dog_request.last_event_id_needing_description + response.should redirect_to show_request_url(:url_title => @dog_request.url_title) end it 'should redirect to the "respond to last url url" when status is updated to "user_withdrawn"' do diff --git a/spec/integration/request_controller_spec.rb b/spec/integration/request_controller_spec.rb new file mode 100644 index 000000000..24667bdf1 --- /dev/null +++ b/spec/integration/request_controller_spec.rb @@ -0,0 +1,41 @@ +# -*- coding: utf-8 -*- +require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') + +describe RequestController, "when classifying an information request" do + + describe 'when the request is internal' do + + before(:each) do + @dog_request = info_requests(:fancy_dog_request) + # This should happen automatically before each test but doesn't with these integration + # tests for some reason. + ActionMailer::Base.deliveries = [] + end + + describe 'when logged in as the requestor' do + + before :each do + @request_owner = @dog_request.user + visit signin_path + fill_in "Your e-mail:", :with => @request_owner.email + fill_in "Password:", :with => "jonespassword" + click_button "Sign in" + end + + it "should send an email including the message" do + visit describe_state_message_path(:url_title => @dog_request.url_title, + :described_state => "requires_admin") + fill_in "Please tell us more:", :with => "Okay. I don't quite understand." + click_button "Submit status and send message" + + response.should contain "Thank you! We'll look into what happened and try and fix it up." + + deliveries = ActionMailer::Base.deliveries + deliveries.size.should == 1 + mail = deliveries[0] + mail.body.should =~ /as needing admin/ + mail.body.should =~ /Okay. I don't quite understand./ + end + end + end +end |