From 211fe84dc40d97df8aa8724906d9170ed4f78477 Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Tue, 9 Oct 2012 18:15:06 +0100 Subject: Revert "Merge remote-tracking branch 'henare_github/patch-1'" Mistakenly merged into master. Please note that we'll want to merge this work into master on the next release, at which point we'll want to revert this reversion in order that the changes are properly re-applied. This reverts commit 54281fd50c3271835a54ab4bc08d40da09d643ee, reversing changes made to 793ca358c37458e6cc4385d2366621aaee93a25e. --- spec/controllers/comment_controller_spec.rb | 11 ----------- spec/controllers/request_controller_spec.rb | 16 ---------------- 2 files changed, 27 deletions(-) (limited to 'spec/controllers') diff --git a/spec/controllers/comment_controller_spec.rb b/spec/controllers/comment_controller_spec.rb index 4a7acee23..b71bc0aea 100644 --- a/spec/controllers/comment_controller_spec.rb +++ b/spec/controllers/comment_controller_spec.rb @@ -53,17 +53,6 @@ describe CommentController, "when commenting on a request" do response.should render_template('new') end - - it "should not allow comments if comments are not allowed" do - session[:user_id] = users(:silly_name_user).id - - expect { - post :new, :url_title => info_requests(:spam_1_request).url_title, - :comment => { :body => "I demand to be heard!" }, - :type => 'request', :submitted_comment => 1, :preview => 0 - }.to raise_error("Comments are not allowed on this request") - - end describe 'when commenting on an external request' do diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index 77f43b618..95737a250 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -238,22 +238,6 @@ describe RequestController, "when showing one request" do response.should have_tag('div#owner_actions') end - describe 'when the request does allow comments' do - it 'should have a comment link' do - get :show, { :url_title => 'why_do_you_have_such_a_fancy_dog' }, - { :user_id => users(:admin_user).id } - response.should have_tag('#anyone_actions', /Add an annotation/) - end - end - - describe 'when the request does not allow comments' do - it 'should not have a comment link' do - get :show, { :url_title => 'spam_1' }, - { :user_id => users(:admin_user).id } - response.should_not have_tag('#anyone_actions', /Add an annotation/) - end - end - describe 'when the request is being viewed by an admin' do describe 'if the request is awaiting description' do -- cgit v1.2.3 From e6dc0f6606b26e13cb0cd16124fdb2aad3c1b5a6 Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Wed, 17 Oct 2012 14:55:55 +0100 Subject: Revert "Revert "Merge remote-tracking branch 'henare_github/patch-1'"" This reverts commit 211fe84dc40d97df8aa8724906d9170ed4f78477. --- spec/controllers/comment_controller_spec.rb | 11 +++++++++++ spec/controllers/request_controller_spec.rb | 16 ++++++++++++++++ 2 files changed, 27 insertions(+) (limited to 'spec/controllers') diff --git a/spec/controllers/comment_controller_spec.rb b/spec/controllers/comment_controller_spec.rb index b71bc0aea..4a7acee23 100644 --- a/spec/controllers/comment_controller_spec.rb +++ b/spec/controllers/comment_controller_spec.rb @@ -53,6 +53,17 @@ describe CommentController, "when commenting on a request" do response.should render_template('new') end + + it "should not allow comments if comments are not allowed" do + session[:user_id] = users(:silly_name_user).id + + expect { + post :new, :url_title => info_requests(:spam_1_request).url_title, + :comment => { :body => "I demand to be heard!" }, + :type => 'request', :submitted_comment => 1, :preview => 0 + }.to raise_error("Comments are not allowed on this request") + + end describe 'when commenting on an external request' do diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index 95737a250..77f43b618 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -238,6 +238,22 @@ describe RequestController, "when showing one request" do response.should have_tag('div#owner_actions') end + describe 'when the request does allow comments' do + it 'should have a comment link' do + get :show, { :url_title => 'why_do_you_have_such_a_fancy_dog' }, + { :user_id => users(:admin_user).id } + response.should have_tag('#anyone_actions', /Add an annotation/) + end + end + + describe 'when the request does not allow comments' do + it 'should not have a comment link' do + get :show, { :url_title => 'spam_1' }, + { :user_id => users(:admin_user).id } + response.should_not have_tag('#anyone_actions', /Add an annotation/) + end + end + describe 'when the request is being viewed by an admin' do describe 'if the request is awaiting description' do -- cgit v1.2.3 From 3421afdb9eb79effa562ded447923b529ef8b714 Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Wed, 28 Nov 2012 15:57:58 +0000 Subject: Give the implicit default locale in a URL without locale precedence over the session in the case where the default locale is not being included in URLs. This allows the user to return to the default locale. --- spec/controllers/general_controller_spec.rb | 50 +++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) (limited to 'spec/controllers') diff --git a/spec/controllers/general_controller_spec.rb b/spec/controllers/general_controller_spec.rb index 830486493..642ed0e05 100644 --- a/spec/controllers/general_controller_spec.rb +++ b/spec/controllers/general_controller_spec.rb @@ -97,8 +97,57 @@ describe GeneralController, "when showing the frontpage" do response.should be_success end + describe 'when there is more than one locale' do + + describe 'when using the default locale' do + + before do + @default_lang_home_link = /href=".*\/en\// + @other_lang_home_link = /href=".*\/es\// + @old_include_default_locale_in_urls = Configuration::include_default_locale_in_urls + end + + def set_default_locale_in_urls(value) + Configuration.stub!(:include_default_locale_in_urls).and_return(value) + load Rails.root.join("config/initializers/fast_gettext.rb") + end + + describe 'when the config value INCLUDE_DEFAULT_LOCALE_IN_URLS is false' do + + before do + set_default_locale_in_urls(false) + end + + it 'should generate URLs without a locale prepended' do + get :frontpage + response.should_not have_text(@default_lang_home_link) + end + + it 'should render the front page in the default language when no locale param + is present and the session locale is not the default' do + get(:frontpage, {}, {:locale => 'es'}) + response.should_not have_text(@other_lang_home_link) + end + end + + it 'should generate URLs with a locale prepended when the config value + INCLUDE_DEFAULT_LOCALE_IN_URLS is true' do + set_default_locale_in_urls(true) + get :frontpage + response.should have_text(@default_lang_home_link) + end + + after do + set_default_locale_in_urls(@old_include_default_locale_in_urls) + end + + end + end + + describe "when using different locale settings" do home_link_regex = /href=".*\/en\// + it "should generate URLs with a locale prepended when there's more than one locale set" do get :frontpage response.should have_text(home_link_regex) @@ -137,6 +186,7 @@ describe GeneralController, "when showing the frontpage" do FastGettext.default_available_locales = old_fgt_available_locales I18n.available_locales = old_i18n_available_locales end + end end describe GeneralController, "when showing the front page with fixture data" do -- cgit v1.2.3 From 8ef3be53197c02f51df623a4c31c4ca1a996e1ce Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Mon, 8 Oct 2012 15:02:13 +0100 Subject: Make regex for space normalization more unambiguous - it's the spaces at the beginning and end of the whole string that we want to strip entirely, not at the beginning and end of each line (interpretation of ^ and $ is subject to default multiline behaviour of the regexp interpreter) --- spec/controllers/api_controller_spec.rb | 110 ++++++++++++++++---------------- 1 file changed, 55 insertions(+), 55 deletions(-) (limited to 'spec/controllers') diff --git a/spec/controllers/api_controller_spec.rb b/spec/controllers/api_controller_spec.rb index 8d8a39950..85cb8bb29 100644 --- a/spec/controllers/api_controller_spec.rb +++ b/spec/controllers/api_controller_spec.rb @@ -2,7 +2,7 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') def normalise_whitespace(s) - s = s.gsub(/^\s+|\s+$/, "") + s = s.gsub(/\A\s+|\s+\Z/, "") s = s.gsub(/\s+/, " ") return s end @@ -18,19 +18,19 @@ describe ApiController, "when using the API" do request_data = { "title" => "Tell me about your chickens", "body" => "Dear Sir,\n\nI should like to know about your chickens.\n\nYours in faith,\nBob\n", - + "external_url" => "http://www.example.gov.uk/foi/chickens_23", "external_user_name" => "Bob Smith", } - + number_of_requests = InfoRequest.count expect { post :create_request, :k => "This is not really an API key", :request_json => request_data.to_json }.to raise_error ApplicationController::PermissionDenied - + InfoRequest.count.should == number_of_requests end - + it "should create a new request from a POST" do number_of_requests = InfoRequest.count( :conditions => [ @@ -38,61 +38,61 @@ describe ApiController, "when using the API" do public_bodies(:geraldine_public_body).id ] ) - + request_data = { "title" => "Tell me about your chickens", "body" => "Dear Sir,\n\nI should like to know about your chickens.\n\nYours in faith,\nBob\n", - + "external_url" => "http://www.example.gov.uk/foi/chickens_23", "external_user_name" => "Bob Smith", } - + post :create_request, :k => public_bodies(:geraldine_public_body).api_key, :request_json => request_data.to_json response.should be_success response.content_type.should == "application/json" - + response_body = ActiveSupport::JSON.decode(response.body) response_body["errors"].should be_nil response_body["url"].should =~ /^http/ - + InfoRequest.count(:conditions => [ "public_body_id = ?", public_bodies(:geraldine_public_body).id] ).should == number_of_requests + 1 - + new_request = InfoRequest.find(response_body["id"]) new_request.user_id.should be_nil new_request.external_user_name.should == request_data["external_user_name"] new_request.external_url.should == request_data["external_url"] - + new_request.title.should == request_data["title"] new_request.last_event_forming_initial_request.outgoing_message.body.should == request_data["body"].strip - + new_request.public_body_id.should == public_bodies(:geraldine_public_body).id end - + def _create_request post :create_request, :k => public_bodies(:geraldine_public_body).api_key, :request_json => { "title" => "Tell me about your chickens", "body" => "Dear Sir,\n\nI should like to know about your chickens.\n\nYours in faith,\nBob\n", - + "external_url" => "http://www.example.gov.uk/foi/chickens_23", "external_user_name" => "Bob Smith", }.to_json response.content_type.should == "application/json" return ActiveSupport::JSON.decode(response.body)["id"] end - + it "should add a response to a request" do # First we need an external request request_id = info_requests(:external_request).id - + # Initially it has no incoming messages IncomingMessage.count(:conditions => ["info_request_id = ?", request_id]).should == 0 - + # Now add one sent_at = "2012-05-28T12:35:39+01:00" response_body = "Thank you for your request for information, which we are handling in accordance with the Freedom of Information Act 2000. You will receive a response within 20 working days or before the next full moon, whichever is sooner.\n\nYours sincerely,\nJohn Gandermulch,\nExample Council FOI Officer\n" @@ -104,13 +104,13 @@ describe ApiController, "when using the API" do "sent_at" => sent_at, "body" => response_body }.to_json - + # And make sure it worked response.should be_success incoming_messages = IncomingMessage.all(:conditions => ["info_request_id = ?", request_id]) incoming_messages.count.should == 1 incoming_message = incoming_messages[0] - + incoming_message.sent_at.should == Time.iso8601(sent_at) incoming_message.get_main_body_text_folded.should be_equal_modulo_whitespace_to(response_body) end @@ -118,10 +118,10 @@ describe ApiController, "when using the API" do it "should add a followup to a request" do # First we need an external request request_id = info_requests(:external_request).id - + # Initially it has one outgoing message OutgoingMessage.count(:conditions => ["info_request_id = ?", request_id]).should == 1 - + # Add another, as a followup sent_at = "2012-05-29T12:35:39+01:00" followup_body = "Pls answer ASAP.\nkthxbye\n" @@ -133,7 +133,7 @@ describe ApiController, "when using the API" do "sent_at" => sent_at, "body" => followup_body }.to_json - + # Make sure it worked response.should be_success followup_messages = OutgoingMessage.all( @@ -141,15 +141,15 @@ describe ApiController, "when using the API" do ) followup_messages.size.should == 1 followup_message = followup_messages[0] - + followup_message.last_sent_at.should == Time.iso8601(sent_at) followup_message.body.should == followup_body.strip end - + it "should not allow internal requests to be updated" do n_incoming_messages = IncomingMessage.count n_outgoing_messages = OutgoingMessage.count - + request_id = info_requests(:naughty_chicken_request).id post :add_correspondence, :k => public_bodies(:geraldine_public_body).api_key, @@ -159,20 +159,20 @@ describe ApiController, "when using the API" do "sent_at" => Time.now.iso8601, "body" => "xxx" }.to_json - + response.status.should == "500 Internal Server Error" ActiveSupport::JSON.decode(response.body)["errors"].should == [ "Request #{request_id} cannot be updated using the API"] - + IncomingMessage.count.should == n_incoming_messages OutgoingMessage.count.should == n_outgoing_messages end - + it "should not allow other people's requests to be updated" do request_id = _create_request n_incoming_messages = IncomingMessage.count n_outgoing_messages = OutgoingMessage.count - + post :add_correspondence, :k => public_bodies(:humpadink_public_body).api_key, :id => request_id, @@ -181,15 +181,15 @@ describe ApiController, "when using the API" do "sent_at" => Time.now.iso8601, "body" => "xxx" }.to_json - + response.status.should == "500 Internal Server Error" ActiveSupport::JSON.decode(response.body)["errors"].should == [ "You do not own request #{request_id}"] - + IncomingMessage.count.should == n_incoming_messages OutgoingMessage.count.should == n_outgoing_messages end - + it "should not allow files to be attached to a followup" do post :add_correspondence, :k => public_bodies(:geraldine_public_body).api_key, @@ -202,21 +202,21 @@ describe ApiController, "when using the API" do :attachments => [ fixture_file_upload("files/tfl.pdf") ] - - + + # Make sure it worked response.status.to_i.should == 500 errors = ActiveSupport::JSON.decode(response.body)["errors"] errors.should == ["You cannot attach files to messages in the 'request' direction"] end - + it "should allow files to be attached to a response" do # First we need an external request request_id = info_requests(:external_request).id - + # Initially it has no incoming messages IncomingMessage.count(:conditions => ["info_request_id = ?", request_id]).should == 0 - + # Now add one sent_at = "2012-05-28T12:35:39+01:00" response_body = "Thank you for your request for information, which we are handling in accordance with the Freedom of Information Act 2000. You will receive a response within 20 working days or before the next full moon, whichever is sooner.\n\nYours sincerely,\nJohn Gandermulch,\nExample Council FOI Officer\n" @@ -231,34 +231,34 @@ describe ApiController, "when using the API" do :attachments => [ fixture_file_upload("files/tfl.pdf") ] - + # And make sure it worked response.should be_success incoming_messages = IncomingMessage.all(:conditions => ["info_request_id = ?", request_id]) incoming_messages.count.should == 1 incoming_message = incoming_messages[0] - + incoming_message.sent_at.should == Time.iso8601(sent_at) incoming_message.get_main_body_text_folded.should be_equal_modulo_whitespace_to(response_body) - + # Get the attachment attachments = incoming_message.get_attachments_for_display attachments.size.should == 1 attachment = attachments[0] - + attachment.filename.should == "tfl.pdf" attachment.body.should == load_file_fixture("tfl.pdf") end - + it "should show information about a request" do info_request = info_requests(:naughty_chicken_request) get :show_request, :k => public_bodies(:geraldine_public_body).api_key, :id => info_request.id - + response.should be_success assigns[:request].id.should == info_request.id - + r = ActiveSupport::JSON.decode(response.body) r["title"].should == info_request.title # Let’s not test all the fields here, because it would @@ -266,13 +266,13 @@ describe ApiController, "when using the API" do # assigns them and changing assignment to an equality # check, which does not really test anything at all. end - + it "should show an Atom feed of new request events" do get :body_request_events, :id => public_bodies(:geraldine_public_body).id, :k => public_bodies(:geraldine_public_body).api_key, :feed_type => "atom" - + response.should be_success response.should render_template("api/request_events.atom") assigns[:events].size.should > 0 @@ -288,7 +288,7 @@ describe ApiController, "when using the API" do :id => public_bodies(:geraldine_public_body).id, :k => public_bodies(:geraldine_public_body).api_key, :feed_type => "json" - + response.should be_success assigns[:events].size.should > 0 assigns[:events].each do |event| @@ -296,13 +296,13 @@ describe ApiController, "when using the API" do event.outgoing_message.should_not be_nil event.event_type.should satisfy {|x| ['sent', 'followup_sent', 'resent', 'followup_resent'].include?(x)} end - + assigns[:event_data].size.should == assigns[:events].size assigns[:event_data].each do |event_record| event_record[:event_type].should satisfy {|x| ['sent', 'followup_sent', 'resent', 'followup_resent'].include?(x)} end end - + it "should honour the since_event_id parameter" do get :body_request_events, :id => public_bodies(:geraldine_public_body).id, @@ -311,7 +311,7 @@ describe ApiController, "when using the API" do response.should be_success first_event = assigns[:event_data][0] second_event_id = assigns[:event_data][1][:event_id] - + get :body_request_events, :id => public_bodies(:geraldine_public_body).id, :k => public_bodies(:geraldine_public_body).api_key, @@ -320,14 +320,14 @@ describe ApiController, "when using the API" do response.should be_success assigns[:event_data].should == [first_event] end - + it "should honour the since_date parameter for the Atom feed" do get :body_request_events, :id => public_bodies(:humpadink_public_body).id, :k => public_bodies(:humpadink_public_body).api_key, :since_date => "2010-01-01", :feed_type => "atom" - + response.should be_success response.should render_template("api/request_events.atom") assigns[:events].size.should > 0 @@ -335,7 +335,7 @@ describe ApiController, "when using the API" do event.created_at.should >= Date.new(2010, 1, 1) end end - + it "should return a JSON 404 error for non-existent requests" do request_id = 123459876 # Let's hope this doesn't exist! sent_at = "2012-05-28T12:35:39+01:00" @@ -351,7 +351,7 @@ describe ApiController, "when using the API" do response.status.should == "404 Not Found" ActiveSupport::JSON.decode(response.body)["errors"].should == ["Could not find request 123459876"] end - + it "should return a JSON 500 error if we try to add correspondence to a request we don't own" do request_id = info_requests(:naughty_chicken_request).id sent_at = "2012-05-28T12:35:39+01:00" -- cgit v1.2.3 From b54c1023f15611518a08f8deaec296f70c2d093e Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Mon, 3 Dec 2012 13:30:20 +0000 Subject: The absence of an API key in an API request should be a permission denied error, so that notification emails don't get sent. --- spec/controllers/api_controller_spec.rb | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) (limited to 'spec/controllers') diff --git a/spec/controllers/api_controller_spec.rb b/spec/controllers/api_controller_spec.rb index 85cb8bb29..5e148a9f5 100644 --- a/spec/controllers/api_controller_spec.rb +++ b/spec/controllers/api_controller_spec.rb @@ -14,21 +14,34 @@ Spec::Matchers.define :be_equal_modulo_whitespace_to do |expected| end describe ApiController, "when using the API" do - it "should check the API key" do - request_data = { + + describe 'checking API keys' do + before do + @number_of_requests = InfoRequest.count + @request_data = { "title" => "Tell me about your chickens", "body" => "Dear Sir,\n\nI should like to know about your chickens.\n\nYours in faith,\nBob\n", "external_url" => "http://www.example.gov.uk/foi/chickens_23", "external_user_name" => "Bob Smith", } + end - number_of_requests = InfoRequest.count + it 'should check that an API key is given as a param' do expect { - post :create_request, :k => "This is not really an API key", :request_json => request_data.to_json + post :create_request, :request_json => @request_data.to_json }.to raise_error ApplicationController::PermissionDenied - - InfoRequest.count.should == number_of_requests + InfoRequest.count.should == @number_of_requests + end + + it "should check the API key" do + expect { + post :create_request, + :k => "This is not really an API key", + :request_json => @request_data.to_json + }.to raise_error ApplicationController::PermissionDenied + InfoRequest.count.should == @number_of_requests + end end it "should create a new request from a POST" do -- cgit v1.2.3 From 9af56ac5901790fb265a6492531eecbe5dd32f73 Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Wed, 14 Nov 2012 13:10:51 +0000 Subject: Add option to load_file_fixture to specify that the file contents should just be loaded as binary. --- spec/controllers/api_controller_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'spec/controllers') diff --git a/spec/controllers/api_controller_spec.rb b/spec/controllers/api_controller_spec.rb index 5e148a9f5..1c320f85c 100644 --- a/spec/controllers/api_controller_spec.rb +++ b/spec/controllers/api_controller_spec.rb @@ -258,9 +258,8 @@ describe ApiController, "when using the API" do attachments = incoming_message.get_attachments_for_display attachments.size.should == 1 attachment = attachments[0] - attachment.filename.should == "tfl.pdf" - attachment.body.should == load_file_fixture("tfl.pdf") + attachment.body.should == load_file_fixture("tfl.pdf", as_binary=true) end it "should show information about a request" do -- cgit v1.2.3 From 4ac3732880d4084832825dccabef3d6361bb169b Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Wed, 14 Nov 2012 13:14:01 +0000 Subject: Fix syntax of example email. The number of expected attachments was based on the specific handling of a bad end boundary in tmail. --- spec/controllers/request_controller_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/controllers') diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index e898fb91b..f40eecfff 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -757,7 +757,7 @@ describe RequestController, "when showing one request" do assigns[:url_path].should_not == old_path response.location.should have_text(/#{assigns[:url_path]}/) zipfile = Zip::ZipFile.open(File.join(File.dirname(__FILE__), "../../cache/zips", assigns[:url_path])) { |zipfile| - zipfile.count.should == 5 # the message, two hello.txt, the unknown attachment, and its empty message + zipfile.count.should == 4 # the message, two hello.txt plus the unknown attachment } end -- cgit v1.2.3 From ef8e7b1afac46bf016cb1485c546bf7aee734c0e Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Thu, 13 Dec 2012 12:16:46 +0000 Subject: Don't offer or allow viewing of an HTML version of a request if it is hidden, or requester_only. Google docs viewer won't be able to access it, and our own conversion process currently produces image files that will then be publicly viewable. If necessary we can revisit this code to enable admins and requesters to view the HTML version created by our own conversion without adding these files to a path that is served directly by the web server. --- spec/controllers/request_controller_spec.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'spec/controllers') diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index f40eecfff..e8f93b8fa 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -859,6 +859,21 @@ describe RequestController, "when changing prominence of a request" do response.should render_template('request/hidden') end + it 'should not generate an HTML version of an attachment whose prominence is hidden/requester + only even for the requester or an admin but should return a 404' do + ir = info_requests(:fancy_dog_request) + ir.prominence = 'hidden' + ir.save! + receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email) + session[:user_id] = users(:admin_user).id + lambda do + get :get_attachment_as_html, :incoming_message_id => ir.incoming_messages[1].id, + :id => ir.id, + :part => 2, + :file_name => ['hello.txt'] + end.should raise_error(ActiveRecord::RecordNotFound) + end + end # XXX do this for invalid ids -- cgit v1.2.3 From bc1de61f7af280dfced0cd17f6cf55e2b0dcef7c Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Thu, 13 Dec 2012 12:19:17 +0000 Subject: Reformat for line length, add the expected response code. --- spec/controllers/request_controller_spec.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'spec/controllers') diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index e8f93b8fa..d5c929d3b 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -849,14 +849,21 @@ describe RequestController, "when changing prominence of a request" do ir.save! receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email) - get :get_attachment, :incoming_message_id => ir.incoming_messages[1].id, :id => ir.id, :part => 2, :skip_cache => 1 + get :get_attachment, :incoming_message_id => ir.incoming_messages[1].id, + :id => ir.id, + :part => 2, + :skip_cache => 1 response.content_type.should == "text/html" response.should_not have_text(/Second hello/) response.should render_template('request/hidden') - get :get_attachment, :incoming_message_id => ir.incoming_messages[1].id, :id => ir.id, :part => 3, :skip_cache => 1 + get :get_attachment, :incoming_message_id => ir.incoming_messages[1].id, + :id => ir.id, + :part => 3, + :skip_cache => 1 response.content_type.should == "text/html" response.should_not have_text(/First hello/) response.should render_template('request/hidden') + response.code.should == '410' end it 'should not generate an HTML version of an attachment whose prominence is hidden/requester -- cgit v1.2.3 From 3910f7f545177cdb69a5ee0196ffa54a9dba0541 Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Thu, 13 Dec 2012 12:16:46 +0000 Subject: Don't offer or allow viewing of an HTML version of a response attachment if the request is hidden, or requester_only. Google docs viewer won't be able to access it, and our own conversion process currently can produce image files that will then be publicly viewable directly from the webserver (see config/httpd.conf). If necessary we can revisit this code to enable admins and requesters to view the HTML version created by our own conversion without adding these files to a path that is served directly by the web server. --- spec/controllers/request_controller_spec.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'spec/controllers') diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index b0223588e..43eca46cd 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -859,6 +859,21 @@ describe RequestController, "when changing prominence of a request" do response.should render_template('request/hidden') end + it 'should not generate an HTML version of an attachment whose prominence is hidden/requester + only even for the requester or an admin but should return a 404' do + ir = info_requests(:fancy_dog_request) + ir.prominence = 'hidden' + ir.save! + receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email) + session[:user_id] = users(:admin_user).id + lambda do + get :get_attachment_as_html, :incoming_message_id => ir.incoming_messages[1].id, + :id => ir.id, + :part => 2, + :file_name => ['hello.txt'] + end.should raise_error(ActiveRecord::RecordNotFound) + end + end # XXX do this for invalid ids -- cgit v1.2.3 From 482d0d351b48877ac1bf1e13678c5591997755b4 Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Thu, 13 Dec 2012 19:05:17 +0000 Subject: Check that a request is publicly visible before generating a download link. --- spec/controllers/request_controller_spec.rb | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'spec/controllers') diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index 43eca46cd..625bf17e7 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -727,6 +727,16 @@ describe RequestController, "when showing one request" do describe 'when making a zipfile available' do + it 'should return a 410 for a request that is hidden' do + title = 'why_do_you_have_such_a_fancy_dog' + ir = info_requests(:fancy_dog_request) + ir.prominence = 'hidden' + ir.save! + get :download_entire_request, {:url_title => title}, { :user_id => ir.user.id } + response.should render_template('request/hidden') + response.code.should == '410' + end + it "should have a different zipfile URL when the request changes" do title = 'why_do_you_have_such_a_fancy_dog' ir = info_requests(:fancy_dog_request) @@ -765,7 +775,7 @@ describe RequestController, "when showing one request" do info_request = info_requests(:external_request) get :download_entire_request, { :url_title => info_request.url_title }, { :user_id => users(:bob_smith_user) } - response.location.should have_text(/#{assigns[:url_path]}/) + response.location.should have_text(/#{assigns[:url_path]}$/) end end end -- cgit v1.2.3 From 610eac0af6f8faf426b46f14ecbe9a312b428bb0 Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Mon, 17 Dec 2012 10:32:36 +0000 Subject: Rewrite specs that were in spec/controller/application_controller as full-stack controller specs in the relevant controllers. It turns out that having spec blocks that reference the ApplicationController class directly i.e. "describe ApplicationController" can have unpredictable effects. actionpack's action_controller/test_case.rb rewrites rescue_action_without_handler on whatever it is included in, and if this is done on a controller class, and then directly on action controller, it can result in an infinite loop of recursive calls. This turns out to be the problem that was causing some tests in error_spec.rb to fail in Travis under Ruby 1.9. --- spec/controllers/application_controller_spec.rb | 54 ------------------------- spec/controllers/request_controller_spec.rb | 27 +++++++++++++ spec/controllers/services_controller_spec.rb | 46 ++++++++++++++++++++- 3 files changed, 71 insertions(+), 56 deletions(-) delete mode 100644 spec/controllers/application_controller_spec.rb (limited to 'spec/controllers') diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb deleted file mode 100644 index 18341ae6f..000000000 --- a/spec/controllers/application_controller_spec.rb +++ /dev/null @@ -1,54 +0,0 @@ -require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') -require 'fakeweb' - -describe ApplicationController, "when accessing third party services" do - - before (:each) do - FakeWeb.clean_registry - end - - after (:each) do - FakeWeb.clean_registry - end - - it "should succeed if the service responds OK" do - Configuration.stub!(:gaze_url).and_return('http://denmark.com') - FakeWeb.register_uri(:get, %r|denmark.com|, :body => "DK") - country = self.controller.send :country_from_ip - country.should == "DK" - end - it "should fail silently if the country_from_ip domain doesn't exist" do - Configuration.stub!(:gaze_url).and_return('http://12123sdf14qsd.com') - country = self.controller.send :country_from_ip - country.should == Configuration.iso_country_code - end - it "should fail silently if the country_from_ip service doesn't exist" do - Configuration.stub!(:gaze_url).and_return('http://www.google.com') - country = self.controller.send :country_from_ip - country.should == Configuration.iso_country_code - end - it "should fail silently if the country_from_ip service returns an error" do - FakeWeb.register_uri(:get, %r|500.com|, :body => "Error", :status => ["500", "Error"]) - Configuration.stub!(:gaze_url).and_return('http://500.com') - country = self.controller.send :country_from_ip - country.should == Configuration.iso_country_code - end -end - -describe ApplicationController, "when caching fragments" do - - it "should not fail with long filenames" do - long_name = "blahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblah.txt" - params = { :only_path => true, - :file_name => [long_name], - :controller => "request", - :action => "get_attachment_as_html", - :id => "132", - :incoming_message_id => "44", - :part => "2" } - path = self.controller.send(:foi_fragment_cache_path, params) - self.controller.send(:foi_fragment_cache_write, path, "whassap") - end - -end - diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index 521ad7b5a..7cfa5cb4a 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -2313,4 +2313,31 @@ describe RequestController, "when reporting a request (logged in)" do end end +describe RequestController, "when caching fragments" do + + it "should not fail with long filenames" do + long_name = "blahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblah.txt" + info_request = mock(InfoRequest, :user_can_view? => true, + :all_can_view? => true) + incoming_message = mock(IncomingMessage, :info_request => info_request, + :parse_raw_email! => true, + :info_request_id => 132, + :get_attachments_for_display => nil, + :html_mask_stuff! => nil) + attachment = mock(FoiAttachment, :display_filename => long_name, + :body_as_html => ['some text', 'wrapper']) + IncomingMessage.stub!(:find).with("44").and_return(incoming_message) + IncomingMessage.stub!(:get_attachment_by_url_part_number).and_return(attachment) + InfoRequest.stub!(:find).with("132").and_return(info_request) + params = { :file_name => [long_name], + :controller => "request", + :action => "get_attachment_as_html", + :id => "132", + :incoming_message_id => "44", + :part => "2" } + get :get_attachment_as_html, params + end + +end + diff --git a/spec/controllers/services_controller_spec.rb b/spec/controllers/services_controller_spec.rb index a701ae247..a9950d520 100644 --- a/spec/controllers/services_controller_spec.rb +++ b/spec/controllers/services_controller_spec.rb @@ -1,7 +1,8 @@ # -*- coding: utf-8 -*- require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') +require 'fakeweb' -describe ServicesController, "when using web services" do +describe ServicesController, "when returning a message for people in other countries" do integrate_views @@ -40,4 +41,45 @@ describe ServicesController, "when using web services" do FastGettext.set_locale(@old_locale) end -end \ No newline at end of file + describe 'when the external country from IP service is in different states' do + + before (:each) do + FakeWeb.clean_registry + end + + after (:each) do + FakeWeb.clean_registry + end + + it "should return the 'another country' message if the service responds OK" do + config = MySociety::Config.load_default() + config['ISO_COUNTRY_CODE'] = "DE" + Configuration.stub!(:gaze_url).and_return('http://denmark.com') + FakeWeb.register_uri(:get, %r|denmark.com|, :body => "DK") + get :other_country_message + response.should be_success + response.body.should == 'Hello! We have an important message for visitors outside Deutschland X' + end + it "should default to no message if the country_from_ip domain doesn't exist" do + Configuration.stub!(:gaze_url).and_return('http://12123sdf14qsd.com') + get :other_country_message + response.should be_success + response.body.should == '' + end + it "should default to no message if the country_from_ip service doesn't exist" do + Configuration.stub!(:gaze_url).and_return('http://www.google.com') + get :other_country_message + response.should be_success + response.body.should == '' + end + it "should default to no message if the country_from_ip service returns an error" do + FakeWeb.register_uri(:get, %r|500.com|, :body => "Error", :status => ["500", "Error"]) + Configuration.stub!(:gaze_url).and_return('http://500.com') + get :other_country_message + response.should be_success + response.body.should == '' + end + + end + +end -- cgit v1.2.3 From e3dc628a38bb91b9d5e83971e247328bd421c47e Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Mon, 17 Dec 2012 12:04:22 +0000 Subject: Adding mocking of incoming message id. --- spec/controllers/request_controller_spec.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'spec/controllers') diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index 7cfa5cb4a..45803d74f 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -2322,6 +2322,7 @@ describe RequestController, "when caching fragments" do incoming_message = mock(IncomingMessage, :info_request => info_request, :parse_raw_email! => true, :info_request_id => 132, + :id => 44, :get_attachments_for_display => nil, :html_mask_stuff! => nil) attachment = mock(FoiAttachment, :display_filename => long_name, -- cgit v1.2.3 From 153984fd2e6841f3b0bc62e25e1718800a7c63ed Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Mon, 17 Dec 2012 15:32:39 +0000 Subject: Only serve up 'similar' pages up to the offset we use for list. --- spec/controllers/request_controller_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'spec/controllers') diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index 45803d74f..148e4327d 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -2226,6 +2226,14 @@ describe RequestController, "when showing similar requests" do }.should raise_error(ActiveRecord::RecordNotFound) end + + it "should return 404 for pages we don't want to serve up" do + badger_request = info_requests(:badger_request) + lambda { + get :similar, :url_title => badger_request.url_title, :page => 100 + }.should raise_error(ActiveRecord::RecordNotFound) + end + end -- cgit v1.2.3