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/request_controller_spec.rb | 16 ---------------- 1 file changed, 16 deletions(-) (limited to 'spec/controllers/request_controller_spec.rb') 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/request_controller_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'spec/controllers/request_controller_spec.rb') 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 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/request_controller_spec.rb') 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/request_controller_spec.rb') 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/request_controller_spec.rb') 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/request_controller_spec.rb') 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/request_controller_spec.rb') 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/request_controller_spec.rb | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) (limited to 'spec/controllers/request_controller_spec.rb') 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 + -- 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/request_controller_spec.rb') 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/request_controller_spec.rb') 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