diff options
| -rw-r--r-- | app/controllers/reports_controller.rb | 31 | ||||
| -rw-r--r-- | app/controllers/request_controller.rb | 19 | ||||
| -rw-r--r-- | app/models/info_request.rb | 15 | ||||
| -rw-r--r-- | app/views/reports/new.html.erb | 26 | ||||
| -rw-r--r-- | app/views/request/_sidebar.html.erb | 2 | ||||
| -rw-r--r-- | config/routes.rb | 11 | ||||
| -rw-r--r-- | spec/controllers/reports_controller_spec.rb | 104 | ||||
| -rw-r--r-- | spec/controllers/request_controller_spec.rb | 116 | ||||
| -rw-r--r-- | spec/views/reports/new.erb_spec.rb | 29 |
9 files changed, 240 insertions, 113 deletions
diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb new file mode 100644 index 000000000..a1dd53125 --- /dev/null +++ b/app/controllers/reports_controller.rb @@ -0,0 +1,31 @@ +class ReportsController < ApplicationController + def create + @info_request = InfoRequest.find_by_url_title!(params[:request_id]) + @reason = params[:reason] + @message = params[:message] + if @reason.empty? + flash[:error] = _("Please choose a reason") + render "new" + return + end + + if !authenticated_user + flash[:notice] = _("You need to be logged in to report a request for administrator attention") + elsif @info_request.attention_requested + flash[:notice] = _("This request has already been reported for administrator attention") + else + @info_request.report!(@reason, @message, @user) + flash[:notice] = _("This request has been reported for administrator attention") + end + redirect_to request_url(@info_request) + end + + def new + @info_request = InfoRequest.find_by_url_title!(params[:request_id]) + if authenticated?( + :web => _("To report this request"), + :email => _("Then you can report the request '{{title}}'", :title => @info_request.title), + :email_subject => _("Report an offensive or unsuitable request")) + end + end +end diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index e8547f72f..6ca4e9f82 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -680,25 +680,6 @@ class RequestController < ApplicationController end end - def report_request - info_request = InfoRequest.find_by_url_title!(params[:url_title]) - return if !authenticated?( - :web => _("To report this FOI request"), - :email => _("Then you can report the request '{{title}}'", :title => info_request.title), - :email_subject => _("Report an offensive or unsuitable request") - ) - - if !info_request.attention_requested - info_request.set_described_state('attention_requested', @user) - info_request.attention_requested = true # tells us if attention has ever been requested - info_request.save! - flash[:notice] = _("This request has been reported for administrator attention") - else - flash[:notice] = _("This request has already been reported for administrator attention") - end - redirect_to request_url(info_request) - end - # special caching code so mime types are handled right around_filter :cache_attachments, :only => [ :get_attachment, :get_attachment_as_html ] def cache_attachments diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 553bb2436..aaf171c4c 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -108,6 +108,12 @@ class InfoRequest < ActiveRecord::Base states end + # Possible reasons that a request could be reported for administrator attention + def report_reasons + ["Contains defamatory material", "Not a valid request", "Request for personal information", + "Contains personal information", "Vexatious", "Other"] + end + def must_be_valid_state errors.add(:described_state, "is not a valid state") if !InfoRequest.enumerate_states.include? described_state @@ -543,6 +549,15 @@ public ['requires_admin', 'error_message', 'attention_requested'].include?(described_state) end + # Report this request for administrator attention + def report!(reason, message, user) + ActiveRecord::Base.transaction do + set_described_state('attention_requested', user, "Reason: #{reason}\n\n#{message}") + self.attention_requested = true # tells us if attention has ever been requested + save! + end + end + # change status, including for last event for later historical purposes def set_described_state(new_state, set_by = nil, message = "") old_described_state = described_state diff --git a/app/views/reports/new.html.erb b/app/views/reports/new.html.erb new file mode 100644 index 000000000..7d558ab4e --- /dev/null +++ b/app/views/reports/new.html.erb @@ -0,0 +1,26 @@ +<h1>Report request: <%= @info_request.title %></h1> + +<% if @info_request.attention_requested %> + <p><%= _("This request has already been reported for administrator attention") %></p> +<% else %> + <p> + Reporting a request notifies the site administrators. They will respond as soon as possible. + </p> + <p>Why specifically do you consider this request unsuitable?</p> + + <%= form_tag request_report_path(:request_id => @info_request.url_title) do %> + <p> + <label class="form_label" for="reason">Reason:</label> + <%= select_tag :reason, options_for_select(@info_request.report_reasons, @reason), :prompt => "Choose a reason" %> + </p> + <p> + <label class="form_label" for="message">Please tell us more:</label> + <%= text_area_tag :message, @message, :rows => 10, :cols => 60 %> + </p> + + <div class="form_button"> + <%= submit_tag _("Report request") %> + </div> + + <% end %> +<% end %> diff --git a/app/views/request/_sidebar.html.erb b/app/views/request/_sidebar.html.erb index 4bc8826fd..aba5c2fb3 100644 --- a/app/views/request/_sidebar.html.erb +++ b/app/views/request/_sidebar.html.erb @@ -30,7 +30,7 @@ <% else %> <p><%= _('Requests for personal information and vexatious requests are not considered valid for FOI purposes (<a href="/help/about">read more</a>).') %></p> <p><%= _('If you believe this request is not suitable, you can report it for attention by the site administrators') %></p> - <%= button_to _("Report this request"), report_path(:url_title => @info_request.url_title), :class => "link_button_green" %> + <%= link_to _("Report this request"), new_request_report_path(:request_id => @info_request.url_title) %> <% end %> <% end %> <h2><%= _("Act on what you've learnt") %></h2> diff --git a/config/routes.rb b/config/routes.rb index bc03d91e1..56be975c3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -57,15 +57,12 @@ Alaveteli::Application.routes.draw do match '/upload/request/:url_title' => 'request#upload_response', :as => :upload_response match '/request/:url_title/download' => 'request#download_entire_request', :as => :download_entire_request - - # It would be nice to add :conditions => { :method => :post } to this next one, - # because it ought not really to be available as a GET request since it changes - # the server state. Unfortunately this doesn’t play well with the PostRedirect - # mechanism, which assumes all post-login actions are available via GET, so we - # refrain. - match '/request/:url_title/report' => 'request#report_request', :as => :report #### + resources :request, :only => [] do + resource :report, :only => [:new, :create] + end + #### User controller # Use /profile for things to do with the currently signed in user. # Use /user/XXXX for things that anyone can see about that user. diff --git a/spec/controllers/reports_controller_spec.rb b/spec/controllers/reports_controller_spec.rb new file mode 100644 index 000000000..fa8c72eaa --- /dev/null +++ b/spec/controllers/reports_controller_spec.rb @@ -0,0 +1,104 @@ +require 'spec_helper' + +describe ReportsController, "when reporting a request when not logged in" do + it "should only allow logged-in users to report requests" do + post :create, :request_id => info_requests(:badger_request).url_title, :reason => "my reason" + + flash[:notice].should =~ /You need to be logged in/ + response.should redirect_to show_request_path(:url_title => info_requests(:badger_request).url_title) + end +end + +describe ReportsController, "when reporting a request (logged in)" do + render_views + + before do + @user = users(:robin_user) + session[:user_id] = @user.id + end + + it "should 404 for non-existent requests" do + lambda { + post :create, :request_id => "hjksfdhjk_louytu_qqxxx" + }.should raise_error(ActiveRecord::RecordNotFound) + end + + it "should mark a request as having been reported" do + ir = info_requests(:badger_request) + title = ir.url_title + ir.attention_requested.should == false + + post :create, :request_id => title, :reason => "my reason" + response.should redirect_to show_request_path(:url_title => title) + + ir.reload + ir.attention_requested.should == true + ir.described_state.should == "attention_requested" + end + + it "should pass on the reason and message" do + info_request = mock_model(InfoRequest, :url_title => "foo", :attention_requested= => nil, :save! => nil) + InfoRequest.should_receive(:find_by_url_title!).with("foo").and_return(info_request) + info_request.should_receive(:report!).with("Not valid request", "It's just not", @user) + post :create, :request_id => "foo", :reason => "Not valid request", :message => "It's just not" + end + + it "should not allow a request to be reported twice" do + title = info_requests(:badger_request).url_title + + post :create, :request_id => title, :reason => "my reason" + response.should redirect_to show_request_url(:url_title => title) + + post :create, :request_id => title, :reason => "my reason" + response.should redirect_to show_request_url(:url_title => title) + flash[:notice].should =~ /has already been reported/ + end + + it "should send an email from the reporter to admins" do + ir = info_requests(:badger_request) + title = ir.url_title + post :create, :request_id => title, :reason => "my reason" + deliveries = ActionMailer::Base.deliveries + deliveries.size.should == 1 + mail = deliveries[0] + mail.subject.should =~ /attention_requested/ + mail.from.should include(@user.email) + mail.body.should include(@user.name) + end + + it "should force the user to pick a reason" do + info_request = mock_model(InfoRequest, :report! => nil, :url_title => "foo", + :report_reasons => ["Not FOIish enough"]) + InfoRequest.should_receive(:find_by_url_title!).with("foo").and_return(info_request) + + post :create, :request_id => "foo", :reason => "" + response.should render_template("new") + flash[:error].should == "Please choose a reason" + end +end + +describe ReportsController, "#new_report_request" do + let(:info_request) { mock_model(InfoRequest, :url_title => "foo") } + before :each do + InfoRequest.should_receive(:find_by_url_title!).with("foo").and_return(info_request) + end + + context "not logged in" do + it "should require the user to be logged in" do + get :new, :request_id => "foo" + response.should_not render_template("new") + end + end + + context "logged in" do + before :each do + session[:user_id] = users(:bob_smith_user).id + end + it "should show the form" do + get :new, :request_id => "foo" + response.should render_template("new") + end + end +end + + diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index 122584c7d..5d10c88ad 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -241,6 +241,36 @@ describe RequestController, "when showing one request" do end end + context "when the request has not yet been reported" do + it "should allow the user to report" do + title = info_requests(:badger_request).url_title + get :show, :url_title => title + response.should_not contain("This request has been reported") + response.should contain("Offensive?") + end + end + + context "when the request has been reported for admin attention" do + before :each do + info_requests(:fancy_dog_request).report!("", "", nil) + end + it "should inform the user" do + get :show, :url_title => 'why_do_you_have_such_a_fancy_dog' + response.should contain("This request has been reported") + response.should_not contain("Offensive?") + end + + context "and then deemed okay and left to complete" do + before :each do + info_requests(:fancy_dog_request).set_described_state("successful") + end + it "should let the user know that the administrators have not hidden this request" do + get :show, :url_title => 'why_do_you_have_such_a_fancy_dog' + response.body.should =~ (/the site administrators.*have not hidden it/) + end + end + end + describe 'when the request is being viewed by an admin' do describe 'if the request is awaiting description' do @@ -2319,91 +2349,6 @@ describe RequestController, "when showing similar requests" do end - -describe RequestController, "when reporting a request when not logged in" do - it "should only allow logged-in users to report requests" do - get :report_request, :url_title => info_requests(:badger_request).url_title - post_redirect = PostRedirect.get_last_post_redirect - response.should redirect_to(:controller => 'user', :action => 'signin', :token => post_redirect.token) - end -end - -describe RequestController, "when reporting a request (logged in)" do - render_views - - before do - @user = users(:robin_user) - session[:user_id] = @user.id - end - - it "should 404 for non-existent requests" do - lambda { - post :report_request, :url_title => "hjksfdhjk_louytu_qqxxx" - }.should raise_error(ActiveRecord::RecordNotFound) - end - - it "should mark a request as having been reported" do - ir = info_requests(:badger_request) - title = ir.url_title - get :show, :url_title => title - assigns[:info_request].attention_requested.should == false - - post :report_request, :url_title => title - response.should redirect_to(:action => :show, :url_title => title) - - get :show, :url_title => title - response.should be_success - assigns[:info_request].attention_requested.should == true - assigns[:info_request].described_state.should == "attention_requested" - end - - it "should not allow a request to be reported twice" do - title = info_requests(:badger_request).url_title - - post :report_request, :url_title => title - response.should redirect_to(:action => :show, :url_title => title) - get :show, :url_title => title - response.should be_success - response.body.should include("has been reported") - - post :report_request, :url_title => title - response.should redirect_to(:action => :show, :url_title => title) - get :show, :url_title => title - response.should be_success - response.body.should include("has already been reported") - end - - it "should let users know a request has been reported" do - title = info_requests(:badger_request).url_title - get :show, :url_title => title - response.body.should include("Offensive?") - - post :report_request, :url_title => title - response.should redirect_to(:action => :show, :url_title => title) - - get :show, :url_title => title - response.body.should_not include("Offensive?") - response.body.should include("This request has been reported") - - info_requests(:badger_request).set_described_state("successful") - get :show, :url_title => title - response.body.should_not include("This request has been reported") - response.body.should =~ (/the site administrators.*have not hidden it/) - end - - it "should send an email from the reporter to admins" do - ir = info_requests(:badger_request) - title = ir.url_title - post :report_request, :url_title => title - deliveries = ActionMailer::Base.deliveries - deliveries.size.should == 1 - mail = deliveries[0] - mail.subject.should =~ /attention_requested/ - mail.from.should include(@user.email) - mail.body.should include(@user.name) - end -end - describe RequestController, "when caching fragments" do it "should not fail with long filenames" do long_name = "blahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblah.txt" @@ -2431,4 +2376,3 @@ describe RequestController, "when caching fragments" do end - diff --git a/spec/views/reports/new.erb_spec.rb b/spec/views/reports/new.erb_spec.rb new file mode 100644 index 000000000..66b738261 --- /dev/null +++ b/spec/views/reports/new.erb_spec.rb @@ -0,0 +1,29 @@ +require File.expand_path(File.join('..', '..', '..', 'spec_helper'), __FILE__) + +describe 'reports/new.html.erb' do + let(:info_request) { mock_model(InfoRequest, :url_title => "foo", :report_reasons => ["Weird"]) } + before :each do + assign(:info_request, info_request) + end + + it "should show a form" do + render + rendered.should have_selector("form") + end + + context "request has already been reported" do + before :each do + info_request.stub!(:attention_requested).and_return(true) + end + + it "should not show a form" do + render + rendered.should_not have_selector("form") + end + + it "should say it's already been reported" do + render + rendered.should contain("This request has already been reported") + end + end +end |