Fix bug where follow up to defunct authority would cause errors.

6 files changed, 79 insertions, 15 deletions

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 97181ddf7..1d6189915 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -4,7 +4,7 @@
 # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: request_controller.rb,v 1.161 2009-06-15 14:42:11 francis Exp $
+# $Id: request_controller.rb,v 1.162 2009-06-16 17:28:16 francis Exp $
 
 class RequestController < ApplicationController
     
@@ -433,6 +433,13 @@ class RequestController < ApplicationController
             raise sprintf("Incoming message %d does not belong to request %d", @incoming_message.info_request_id, @info_request.id)
         end
 
+        if !RequestMailer.is_followupable?(@info_request, @incoming_message)
+            raise "unexpected followupable inconsistency" if @info_request.public_body.is_requestable?
+            @reason = @info_request.public_body.not_requestable_reason
+            render :action => 'followup_bad'
+            return
+        end
+
         # Force login early - this is really the "send followup" form. We want
         # to make sure they're the right user first, before they start writing a
         # message and wasting their time if they are not the requester.
diff --git a/app/models/info_request.rb b/app/models/info_request.rb
index 5b2d64753..59eade8fd 100644
--- a/app/models/info_request.rb
+++ b/app/models/info_request.rb
@@ -23,7 +23,7 @@
 # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: info_request.rb,v 1.192 2009-06-15 16:25:14 francis Exp $
+# $Id: info_request.rb,v 1.193 2009-06-16 17:28:17 francis Exp $
 
 require 'digest/sha1'
 require File.join(File.dirname(__FILE__),'../../vendor/plugins/acts_as_xapian/lib/acts_as_xapian')
@@ -525,6 +525,9 @@ public
     def recipient_email
         return self.public_body.request_email
     end
+    def recipient_email_valid?
+        return self.public_body.is_requestable?
+    end
     def recipient_name_and_email
         return TMail::Address.address_from_name_and_email(self.law_used_short + " requests at " + self.public_body.short_or_long_name, self.recipient_email).to_s
     end
diff --git a/app/models/request_mailer.rb b/app/models/request_mailer.rb
index ad1fd584e..6c5537b3f 100644
--- a/app/models/request_mailer.rb
+++ b/app/models/request_mailer.rb
@@ -4,7 +4,7 @@
 # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: request_mailer.rb,v 1.78 2009-06-15 23:52:44 francis Exp $
+# $Id: request_mailer.rb,v 1.79 2009-06-16 17:28:17 francis Exp $
 
 class RequestMailer < ApplicationMailer
     
@@ -58,6 +58,7 @@ class RequestMailer < ApplicationMailer
             return incoming_message_followup.mail.from_addrs[0].spec
         end
     end
+    # Subject to use for followup
     def RequestMailer.subject_for_followup(info_request, outgoing_message)
         if outgoing_message.what_doing == 'internal_review'
             return "Internal review of " + info_request.email_subject_request
@@ -65,6 +66,16 @@ class RequestMailer < ApplicationMailer
             return info_request.email_subject_followup 
         end
     end
+    # Whether we have a valid email address for a followup
+    def RequestMailer.is_followupable?(info_request, incoming_message_followup)
+        if incoming_message_followup.nil? || !incoming_message_followup.valid_to_reply_to?
+            return info_request.recipient_email_valid?
+        else
+            # email has been checked in incoming_message_followup.valid_to_reply_to? above
+            return true
+        end
+    end
+    
 
     # Used when an FOI officer uploads a response from their web browser - this is
     # the "fake" email used to store in the same format in the database as if they
diff --git a/app/views/request/_after_actions.rhtml b/app/views/request/_after_actions.rhtml
index b8d087a12..54eb20332 100644
--- a/app/views/request/_after_actions.rhtml
+++ b/app/views/request/_after_actions.rhtml
@@ -3,30 +3,48 @@
     <h2>Things to do with this request</h2>
 
     <div id="anyone_actions">
-        <%= link_to "Add an annotation", new_comment_url(:url_title => @info_request.url_title) %> (to help
+        <strong>Anyone:</strong>
+        <ul>
+        <li>
+            <%= link_to "Add an annotation", new_comment_url(:url_title => @info_request.url_title) %> (to help
         the requester or others)
+        </li>
         <% if @old_unclassified %> 
-        |      <%= link_to "Update the status of this request", '#describe_state_form_1' %>
+            <li>
+                <%= link_to "Update the status of this request", '#describe_state_form_1' %>
+            </li>
         <% end %>
+    </ul>
     </div>
 
     <div id="owner_actions">
       <strong><%=h @info_request.user.name %> only:</strong>
-      <% if @last_response.nil? %>
-          <%= link_to "Send follow up to " + RequestMailer.name_for_followup(@info_request, @last_response), show_response_no_followup_url(:id => @info_request.id, :incoming_message_id => nil) + "#followup" %> 
-      <% else %>
-          <%= link_to "Reply to " + RequestMailer.name_for_followup(@info_request, @last_response), show_response_url(:id => @info_request.id, :incoming_message_id => @last_response.id) + "#followup" %> 
-      <% end %>
+      <ul>
+      <li>
+          <% if @last_response.nil? %>
+              <%= link_to "Send follow up to " + RequestMailer.name_for_followup(@info_request, @last_response), show_response_no_followup_url(:id => @info_request.id, :incoming_message_id => nil) + "#followup" %> 
+          <% else %>
+              <%= link_to "Reply to " + RequestMailer.name_for_followup(@info_request, @last_response), show_response_url(:id => @info_request.id, :incoming_message_id => @last_response.id) + "#followup" %> 
+          <% end %>
+      </li>
       <% if !@old_unclassified %>
-      | <%= link_to "Update the status of this request", request_url(@info_request, :update_status => 1) %>
+          <li>
+          <%= link_to "Update the status of this request", request_url(@info_request, :update_status => 1) %>
+          </li>
       <% end %>
-      |
-      <%= link_to "Request an internal review", show_response_no_followup_url(:id => @info_request.id, :incoming_message_id => nil) + "?internal_review=1#followup" %> 
+      <li>
+          <%= link_to "Request an internal review", show_response_no_followup_url(:id => @info_request.id, :incoming_message_id => nil) + "?internal_review=1#followup" %> 
+      </li>
+  </ul>
     </div>
   
     <div id="public_body_actions">
       <strong><%=h @info_request.public_body.name %> only:</strong>
-      <%= link_to "Respond to request", upload_response_url(:url_title => @info_request.url_title) %> 
+      <ul>
+      <li>
+          <%= link_to "Respond to request", upload_response_url(:url_title => @info_request.url_title) %> 
+      </li>
+      </ul>
     </div>
   
-</div>
\ No newline at end of file
+</div>
diff --git a/app/views/request/followup_bad.rhtml b/app/views/request/followup_bad.rhtml
new file mode 100644
index 000000000..c6d9c88dc
--- /dev/null
+++ b/app/views/request/followup_bad.rhtml
@@ -0,0 +1,24 @@
+<% if @incoming_message.nil? || !@incoming_message.valid_to_reply_to? %>
+    <% @title = "Unable to send follow up message to " + RequestMailer.name_for_followup(@info_request, nil) %>
+<% else %>
+    <% @title = "Unable to send a reply to " + RequestMailer.name_for_followup(@info_request, @incoming_message) %>
+<% end %>
+
+<h1><%=@title%></h1>
+
+<% if @reason == 'not_apply' %>
+    <!-- we should never get here, but just in case give a sensible message -->
+    <p>Freedom of Information law no longer applies to <%=h @info_request.public_body.name %>.</p>
+<% elsif @reason == 'defunct' %>
+    <!-- we should never get here, but just in case give a sensible message -->
+    <p><%=h @info_request.public_body.name %> no longer exists.</p>
+<% elsif @reason == 'bad_contact' %>
+    <p>We do not have a working <%=h @info_request.law_used_full %>
+    address for <%=h @info_request.public_body.name %>. You may be able to find
+    one on their website, or by phoning them up and asking. If you manage
+    to find one, then please <a href="/help/contact">send it to us</a>.
+    </p>
+<% else %>
+    <% raise "unknown reason " + @reason %>
+<% end %>
+
diff --git a/config/packages b/config/packages
index 2ece4601b..ea09267a7 100644
--- a/config/packages
+++ b/config/packages
@@ -17,5 +17,6 @@ ttf-bitstream-vera
 sharutils
 unzip
 libzip-ruby1.8
+libzlib-ruby
 mahoro-ruby1.8
 wdg-html-validator