Add new code and specs for hiding attachments.

6 files changed, 139 insertions, 12 deletions

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 0c1d9880c..6a927d327 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -683,9 +683,13 @@ class RequestController < ApplicationController
             @info_request = incoming_message.info_request # used by view
             return render_hidden
         end
+        if !incoming_message.user_can_view?(authenticated_user)
+            @incoming_message = incoming_message # used by view
+            return render_hidden_message
+        end
         # Is this a completely public request that we can cache attachments for
         # to be served up without authentication?
-        if incoming_message.info_request.all_can_view?
+        if incoming_message.info_request.all_can_view? && incoming_message.all_can_view?
             @files_can_be_cached = true
         end
     end
@@ -945,5 +949,14 @@ class RequestController < ApplicationController
         false
     end
 
+    def render_hidden_message
+        respond_to do |format|
+            response_code = 410 # gone
+            format.html{ render :template => 'request/hidden_correspondence', :status => response_code }
+            format.any{ render :nothing => true, :status => response_code }
+        end
+        false
+    end
+
 end
 
diff --git a/app/models/incoming_message.rb b/app/models/incoming_message.rb
index 287120599..eea9c868e 100644
--- a/app/models/incoming_message.rb
+++ b/app/models/incoming_message.rb
@@ -73,6 +73,10 @@ class IncomingMessage < ActiveRecord::Base
         Ability.can_view_with_prominence?(self.prominence, self.info_request, user)
     end
 
+    def all_can_view?
+        self.prominence == 'normal'
+    end
+
     def indexed_by_search?
         self.prominence == 'normal'
     end
diff --git a/app/views/request/_hidden_correspondence.html.erb b/app/views/request/_hidden_correspondence.html.erb
new file mode 100644
index 000000000..1deed6735
--- /dev/null
+++ b/app/views/request/_hidden_correspondence.html.erb
@@ -0,0 +1,10 @@
+<p id="hidden_message">
+  <%- if !message.prominence_reason.blank? %>
+    <%= _('This message has been hidden. {{reason}} Please <a href="{{url}}">contact us</a> if you have any questions.', :url => help_contact_path.html_safe, :reason => message.prominence_reason) %>
+  <%- else %>
+    <%= _('This message has been hidden. There are various reasons why we might have done this, sorry we can\'t be more specific here. Please <a href="{{url}}">contact us</a> if you have any questions.', :url => help_contact_path.html_safe) %>
+  <%- end %>
+  <% if message.prominence == 'requester_only' %>
+    <%= _('If you are the requester, then you may <a href="{{url}}">sign in</a> to view the message.', :url => signin_url(:r => request.fullpath).html_safe) %>
+  <% end %>
+</p>
diff --git a/app/views/request/_incoming_correspondence.html.erb b/app/views/request/_incoming_correspondence.html.erb
index 2293db66d..277999174 100644
--- a/app/views/request/_incoming_correspondence.html.erb
+++ b/app/views/request/_incoming_correspondence.html.erb
@@ -1,15 +1,6 @@
 <div class="incoming correspondence <%= incoming_message.prominence %>" id="incoming-<%=incoming_message.id.to_s%>">
   <%- if not incoming_message.user_can_view?(@user) %>
-      <p id="hidden_message">
-        <%- if !incoming_message.prominence_reason.blank? %>
-          <%= _('This message has been hidden. {{reason}} Please <a href="{{url}}">contact us</a> if you have any questions.', :url => help_contact_path.html_safe, :reason => incoming_message.prominence_reason) %>
-        <%- else %>
-          <%= _('This message has been hidden. There are various reasons why we might have done this, sorry we can\'t be more specific here. Please <a href="{{url}}">contact us</a> if you have any questions.', :url => help_contact_path.html_safe) %>
-        <%- end %>
-        <% if incoming_message.prominence == 'requester_only' %>
-          <%= _('If you are the requester, then you may <a href="{{url}}">sign in</a> to view the message.', :url => signin_url(:r => request.fullpath).html_safe) %>
-        <% end %>
-      </p>
+    <%= render :partial => 'request/hidden_correspondence', :locals => { :message => incoming_message }%>
   <%- else %>
     <% if incoming_message.prominence == 'hidden' %>
         <p id="hidden_message">
diff --git a/app/views/request/hidden_correspondence.html.erb b/app/views/request/hidden_correspondence.html.erb
new file mode 100644
index 000000000..46bf3ee37
--- /dev/null
+++ b/app/views/request/hidden_correspondence.html.erb
@@ -0,0 +1,4 @@
+<% @title = _("Message has been removed") %>
+
+<h1><%=@title%></h1>
+<%= render :partial => 'request/hidden_correspondence', :locals => { :message => @incoming_message } %>
diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb
index 32d79ab5a..26a5de29c 100644
--- a/spec/controllers/request_controller_spec.rb
+++ b/spec/controllers/request_controller_spec.rb
@@ -918,6 +918,109 @@ describe RequestController, "when handling prominence" do
          end
     end
 
+    context 'when the incoming message has prominence hidden' do
+
+        before(:each) do
+            @incoming_message = FactoryGirl.create(:incoming_message_with_attachments, prominence: 'hidden')
+            @info_request = @incoming_message.info_request
+        end
+
+        it "should not download attachments for a non-logged in user" do
+            get :get_attachment, :incoming_message_id => @incoming_message.id,
+                                 :id => @info_request.id,
+                                 :part => 2,
+                                 :file_name => 'interesting.pdf',
+                                 :skip_cache => 1
+            expect_hidden_attachment('request/hidden_correspondence')
+        end
+
+        it 'should not download attachments for the request owner' do
+            session[:user_id] = @info_request.user.id
+            get :get_attachment, :incoming_message_id => @incoming_message.id,
+                                 :id => @info_request.id,
+                                 :part => 2,
+                                 :file_name => 'interesting.pdf',
+                                 :skip_cache => 1
+            expect_hidden_attachment('request/hidden_correspondence')
+        end
+
+        it 'should download attachments for an admin user', :focus => true do
+            session[:user_id] = FactoryGirl.create(:admin_user).id
+            get :get_attachment, :incoming_message_id => @incoming_message.id,
+                                 :id => @info_request.id,
+                                 :part => 2,
+                                 :file_name => 'interesting.pdf',
+                                 :skip_cache => 1
+            response.content_type.should == 'application/pdf'
+            response.should be_success
+        end
+
+        it 'should not generate an HTML version of an attachment for a request whose prominence
+            is hidden even for an admin but should return a 404' do
+            session[:user_id] = FactoryGirl.create(:admin_user).id
+            lambda do
+                get :get_attachment_as_html, :incoming_message_id => @incoming_message.id,
+                                          :id => @info_request.id,
+                                          :part => 2,
+                                          :file_name => 'interesting.pdf',
+                                          :skip_cache => 1
+            end.should raise_error(ActiveRecord::RecordNotFound)
+        end
+    end
+
+    context 'when the incoming message has prominence requester_only' do
+
+        before(:each) do
+            @incoming_message = FactoryGirl.create(:incoming_message_with_attachments,
+                                                    prominence: 'requester_only')
+            @info_request = @incoming_message.info_request
+        end
+
+        it "should not download attachments for a non-logged in user" do
+            get :get_attachment, :incoming_message_id => @incoming_message.id,
+                                 :id => @info_request.id,
+                                 :part => 2,
+                                 :file_name => 'interesting.pdf',
+                                 :skip_cache => 1
+            expect_hidden_attachment('request/hidden_correspondence')
+        end
+
+        it 'should download attachments for the request owner' do
+            session[:user_id] = @info_request.user.id
+            get :get_attachment, :incoming_message_id => @incoming_message.id,
+                                 :id => @info_request.id,
+                                 :part => 2,
+                                 :file_name => 'interesting.pdf',
+                                 :skip_cache => 1
+            response.content_type.should == 'application/pdf'
+            response.should be_success
+        end
+
+        it 'should download attachments for an admin user', :focus => true do
+            session[:user_id] = FactoryGirl.create(:admin_user).id
+            get :get_attachment, :incoming_message_id => @incoming_message.id,
+                                 :id => @info_request.id,
+                                 :part => 2,
+                                 :file_name => 'interesting.pdf',
+                                 :skip_cache => 1
+            response.content_type.should == 'application/pdf'
+            response.should be_success
+        end
+
+        it 'should not generate an HTML version of an attachment for a request whose prominence
+            is hidden even for an admin but should return a 404' do
+            session[:user_id] = FactoryGirl.create(:admin_user)
+            lambda do
+                get :get_attachment_as_html, :incoming_message_id => @incoming_message.id,
+                                          :id => @info_request.id,
+                                          :part => 2,
+                                          :file_name => 'interesting.pdf',
+                                          :skip_cache => 1
+            end.should raise_error(ActiveRecord::RecordNotFound)
+        end
+
+    end
+
 end
 
 # XXX do this for invalid ids
@@ -2416,7 +2519,9 @@ describe RequestController, "when caching fragments" do
                                                  :info_request_id => 132,
                                                  :id => 44,
                                                  :get_attachments_for_display => nil,
-                                                 :html_mask_stuff! => nil)
+                                                 :html_mask_stuff! => nil,
+                                                 :user_can_view? => true,
+                                                 :all_can_view? => true)
         attachment = mock(FoiAttachment, :display_filename => long_name,
                                          :body_as_html => ['some text', 'wrapper'])
         IncomingMessage.stub!(:find).with("44").and_return(incoming_message)