diff options
| -rw-r--r-- | app/controllers/request_controller.rb | 8 | ||||
| -rw-r--r-- | app/views/request/_after_actions.html.erb | 2 | ||||
| -rw-r--r-- | spec/controllers/request_controller_spec.rb | 6 | ||||
| -rw-r--r-- | spec/integration/download_request_spec.rb | 55 | ||||
| -rw-r--r-- | spec/views/request/_after_actions.html.erb_spec.rb | 21 |
5 files changed, 62 insertions, 30 deletions
diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index 0180ad840..8b978cc01 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -868,10 +868,6 @@ class RequestController < ApplicationController @locale = self.locale_from_params() I18n.with_locale(@locale) do @info_request = InfoRequest.find_by_url_title!(params[:url_title]) - # Test for whole request being hidden or requester-only - if !@info_request.all_can_view? - return render_hidden - end if authenticated?( :web => _("To download the zip file"), :email => _("Then you can download a zip file of {{info_request_title}}.", @@ -879,6 +875,10 @@ class RequestController < ApplicationController :email_subject => _("Log in to download a zip file of {{info_request_title}}", :info_request_title=>@info_request.title) ) + # Test for whole request being hidden or requester-only + if !@info_request.user_can_view?(@user) + return render_hidden + end @url_path = File.join("/download", request_dirs(@info_request), @info_request.last_update_hash, diff --git a/app/views/request/_after_actions.html.erb b/app/views/request/_after_actions.html.erb index b54a8f5fb..f780e3a37 100644 --- a/app/views/request/_after_actions.html.erb +++ b/app/views/request/_after_actions.html.erb @@ -15,11 +15,9 @@ <%= link_to _('Update the status of this request'), '#describe_state_form_1' %> </li> <% end %> - <% if @info_request.all_can_view? %> <li> <%= link_to _("Download a zip file of all correspondence"), download_entire_request_path(:url_title => @info_request.url_title) %> </li> - <% end %> </ul> </div> <% if ! @info_request.is_external? %> diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index 8f8e3afa0..c5ee8cbf7 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -858,12 +858,6 @@ describe RequestController, "when handling prominence" do response.should render_template('show') end - it 'should not allow download of the entire request by admin user (or anyone)' do - session[:user_id] = FactoryGirl.create(:admin_user).id - get :download_entire_request, :url_title => @info_request.url_title - expect_hidden('hidden') - end - it 'should not cache an attachment when showing an attachment to the requester or admin' do session[:user_id] = @info_request.user.id incoming_message = @info_request.incoming_messages.first diff --git a/spec/integration/download_request_spec.rb b/spec/integration/download_request_spec.rb index 8af1c9ff1..dd492e42a 100644 --- a/spec/integration/download_request_spec.rb +++ b/spec/integration/download_request_spec.rb @@ -56,7 +56,60 @@ describe 'when making a zipfile available' do end end + context 'when a request is "requester_only"' do + + before do + @non_owner = login(FactoryGirl.create(:user)) + @info_request = FactoryGirl.create(:info_request_with_incoming, + :prominence => 'requester_only') + @request_owner = login(@info_request.user) + @admin = login(FactoryGirl.create(:admin_user)) + end + + + it 'should allow a download of the request by the request owner and admin only' do + # Requester can access the zip + inspect_zip_download(@request_owner, @info_request) do |zip| + zip.count.should == 1 + zip.read('correspondence.txt').should match('hereisthetext') + end + # Non-owner can't + @non_owner.get_via_redirect "request/#{@info_request.url_title}/download" + @non_owner.response.code.should == '410' + # Admin can + inspect_zip_download(@admin, @info_request) do |zip| + zip.count.should == 1 + zip.read('correspondence.txt').should match('hereisthetext') + end + end + end + + context 'when a request is made "hidden"' do + + it 'should not allow a download of the request by an admin only' do + @non_owner = login(FactoryGirl.create(:user)) + @info_request = FactoryGirl.create(:info_request_with_incoming, + :prominence => 'hidden') + @request_owner = login(@info_request.user) + @admin = login(FactoryGirl.create(:admin_user)) + + # Requester can't access the zip + @request_owner.get_via_redirect "request/#{@info_request.url_title}/download" + @request_owner.response.code.should == '410' + # Non-owner can't + @non_owner.get_via_redirect "request/#{@info_request.url_title}/download" + @non_owner.response.code.should == '410' + # Admin can + inspect_zip_download(@admin, @info_request) do |zip| + zip.count.should == 1 + zip.read('correspondence.txt').should match('hereisthetext') + end + end + + end + context 'when an incoming message is made "requester_only"' do + it 'should not include the incoming message or attachments in a download of the entire request by a non-request owner' do @@ -90,7 +143,7 @@ describe 'when making a zipfile available' do end - it 'should successfully make a zipfile for an external request', :focus => true do + it 'should successfully make a zipfile for an external request' do external_request = FactoryGirl.create(:external_request) user = login(FactoryGirl.create(:user)) inspect_zip_download(user, external_request){ |zip| zip.count.should == 1 } diff --git a/spec/views/request/_after_actions.html.erb_spec.rb b/spec/views/request/_after_actions.html.erb_spec.rb index ae398f4ce..833323d68 100644 --- a/spec/views/request/_after_actions.html.erb_spec.rb +++ b/spec/views/request/_after_actions.html.erb_spec.rb @@ -69,24 +69,11 @@ describe 'when displaying actions that can be taken with regard to a request' do end end - describe 'if the request is viewable by all' do - it 'should display the link to download the entire request' do - render :partial => 'request/after_actions' - response.should have_selector('div#anyone_actions') do |div| - div.should have_selector('a', :content => 'Download a zip file of all correspondence') - end - end - end - - describe 'if the request is not viewable by all' do - - it 'should not display the link to download the entire request' do - @mock_request.stub!(:all_can_view?).and_return(false) - render :partial => 'request/after_actions' - response.should have_selector('div#anyone_actions') do |div| - div.should_not have_selector('a', :content => 'Download a zip file of all correspondence') - end + it 'should display the link to download the entire request' do + render :partial => 'request/after_actions' + response.should have_selector('div#anyone_actions') do |div| + div.should have_selector('a', :content => 'Download a zip file of all correspondence') end end |