Merge branch 'add-make-request-button-to-search-results' into rails-3-develop

13 files changed, 292 insertions, 75 deletions

diff --git a/app/assets/stylesheets/responsive/_global_style.scss b/app/assets/stylesheets/responsive/_global_style.scss
index 0ffa875ab..ef755c01e 100644
--- a/app/assets/stylesheets/responsive/_global_style.scss
+++ b/app/assets/stylesheets/responsive/_global_style.scss
@@ -116,7 +116,7 @@ dt + dd {
 }
 
 /* Notices to the user (usually on action completion) */
-#notice, #error {
+#notice, #error, .warning {
   font-size:1em;
   border-radius:3px;
   margin:1em 0;
@@ -136,7 +136,7 @@ dt + dd {
   background-color: lighten(#62b356, 23%);
 }
 
-#error {
+#error, .warning {
   background-color: lighten(#b05460, 23%);
 }
 
diff --git a/app/assets/stylesheets/responsive/_lists_layout.scss b/app/assets/stylesheets/responsive/_lists_layout.scss
index 69237ae91..6a874e8fe 100644
--- a/app/assets/stylesheets/responsive/_lists_layout.scss
+++ b/app/assets/stylesheets/responsive/_lists_layout.scss
@@ -81,4 +81,8 @@
   }
 }
 
-
+/* .make-request-quick-button displays in the typeahead search results in the 'make request' process */
+.make-request-quick-button {
+  margin-bottom: 1em;
+  margin-top: -0.5em;
+}
diff --git a/app/helpers/public_body_helper.rb b/app/helpers/public_body_helper.rb
new file mode 100644
index 000000000..d8a5d57b5
--- /dev/null
+++ b/app/helpers/public_body_helper.rb
@@ -0,0 +1,35 @@
+module PublicBodyHelper
+
+  # Public: The reasons a request can't be made to a PublicBody
+  # The returned reasons are ordered by priority. For example, if the body no
+  # longer exists there is no reason to ask for its contact details if we don't
+  # have an email for it.
+  #
+  # public_body - Instance of a PublicBody
+  #
+  # Returns an Array
+  def public_body_not_requestable_reasons(public_body)
+    reasons = []
+
+    if public_body.defunct?
+      reasons.push _('This authority no longer exists, so you cannot make a request to it.')
+    end
+
+    if public_body.not_apply?
+      reasons.push _('Freedom of Information law does not apply to this authority, so you cannot make a request to it.')
+    end
+
+    unless public_body.has_request_email?
+      # Make the authority appear requestable to encourage users to help find
+      # the authroty's email address
+      msg = link_to _("Make a request to this authority"),
+                      new_request_to_body_path(:url_name => public_body.url_name),
+                      :class => "link_button_green"
+
+      reasons.push(msg)
+    end
+
+    reasons.compact
+  end
+
+end
diff --git a/app/models/public_body.rb b/app/models/public_body.rb
index b8163b07d..5e5b35c5b 100644
--- a/app/models/public_body.rb
+++ b/app/models/public_body.rb
@@ -235,39 +235,38 @@ class PublicBody < ActiveRecord::Base
         return self.has_tag?('defunct')
     end
 
-    # Can an FOI (etc.) request be made to this body, and if not why not?
+    # Can an FOI (etc.) request be made to this body?
     def is_requestable?
-        if self.defunct?
-            return false
-        end
-        if self.not_apply?
-            return false
-        end
-        if self.request_email.nil?
-            return false
-        end
-        return !self.request_email.empty? && self.request_email != 'blank'
+        has_request_email? && !defunct? && !not_apply?
     end
+
     # Strict superset of is_requestable?
     def is_followupable?
-        if self.request_email.nil?
-            return false
-        end
-        return !self.request_email.empty? && self.request_email != 'blank'
+        has_request_email?
+    end
+
+    def has_request_email?
+       !request_email.blank? && request_email != 'blank'
     end
+
     # Also used as not_followable_reason
     def not_requestable_reason
         if self.defunct?
             return 'defunct'
         elsif self.not_apply?
             return 'not_apply'
-        elsif self.request_email.nil? or self.request_email.empty? or self.request_email == 'blank'
+        elsif !has_request_email?
             return 'bad_contact'
         else
-            raise "requestable_failure_reason called with type that has no reason"
+            raise "not_requestable_reason called with type that has no reason"
         end
     end
 
+    def special_not_requestable_reason?
+        self.defunct? || self.not_apply?
+    end
+
+
     class Version
 
         def last_edit_comment_for_html_display
diff --git a/app/views/general/search.html.erb b/app/views/general/search.html.erb
index 4f9ef5b68..c5ff8e9fd 100644
--- a/app/views/general/search.html.erb
+++ b/app/views/general/search.html.erb
@@ -1,7 +1,5 @@
 <% @show_tips = @xapian_requests.nil? || (@total_hits == 0) %>
 
-<% @include_request_link_in_authority_listing = true %>
-
 <%= render :partial => 'localised_datepicker'  %>
 
 <% if @query.nil? %>
@@ -157,7 +155,9 @@
 
         <div class="results_block">
           <% for result in @xapian_bodies.results %>
-            <%= render :partial => 'public_body/body_listing_single', :locals => { :public_body => result[:model] }  %>
+            <%= render :partial => 'public_body/body_listing_single',
+                       :locals => { :public_body => result[:model],
+                                    :request_link => true }  %>
           <% end %>
         </div>
 
diff --git a/app/views/public_body/_body_listing_single.html.erb b/app/views/public_body/_body_listing_single.html.erb
index 91a07d09c..b343c20e1 100644
--- a/app/views/public_body/_body_listing_single.html.erb
+++ b/app/views/public_body/_body_listing_single.html.erb
@@ -1,6 +1,10 @@
-<% if @highlight_words.nil? 
-      @highlight_words = []
-   end %>
+<%
+  if @highlight_words.nil?
+    @highlight_words = []
+  end
+
+  request_link = false unless defined?(request_link)
+%>
 
 <div class="body_listing">
 	<span class="head">
@@ -16,23 +20,28 @@
         <% end %>
         <br>
     <% end %>
-	</span>
+  </span>
     <span class="bottomline">
         <%= n_('{{count}} request made.', '{{count}} requests made.', public_body.info_requests.size,
         :count => public_body.info_requests.size) %>
-        <% if !public_body.is_requestable? && public_body.not_requestable_reason != 'bad_contact' %>
-            <% if public_body.not_requestable_reason == 'defunct' %>
-                <%= _('Defunct.') %>
-            <% end %>
-        <% else %>
-            <% if !@include_request_link_in_authority_listing.nil? %>
-            <%= link_to _("Make your own request"), public_body_path(public_body) %>.
-            <% end %>
-        <% end %>
         <br>
         <span class="date_added">
             <%= _("Added on {{date}}", :date => simple_date(public_body.created_at)) %>.
         </span>
+        <br>
+        <% if public_body.special_not_requestable_reason? %>
+           <% if public_body.not_requestable_reason == 'not_apply' %>
+           <%= _('FOI law does not apply to this authority.')%>
+           <% elsif public_body.not_requestable_reason == 'defunct' %>
+           <%= _('Defunct.')%>
+           <% end %>
+        <% end %>
     </span>
+
+    <% if request_link && !public_body.special_not_requestable_reason? %>
+      <div class="make-request-quick-button">
+        <%= link_to _("Make a request"), new_request_to_body_path(:url_name => public_body.url_name), :class => "link_button_green" %>
+      </div>
+    <% end %>
 </div>
 
diff --git a/app/views/public_body/_search_ahead.html.erb b/app/views/public_body/_search_ahead.html.erb
index b5632bccd..b06ed5efa 100644
--- a/app/views/public_body/_search_ahead.html.erb
+++ b/app/views/public_body/_search_ahead.html.erb
@@ -7,7 +7,9 @@
   <% end %>
   <div id="authority_search_ahead_results">
     <% for result in @xapian_requests.results %>
-      <%= render :partial => 'public_body/body_listing_single', :locals => { :public_body => result[:model] } %>
+      <%= render :partial => 'public_body/body_listing_single',
+                 :locals => { :public_body => result[:model],
+                              :request_link => true } %>
     <% end %>
   </div>
   <%= will_paginate WillPaginate::Collection.new(@page, @per_page, @xapian_requests.matches_estimated), :params => {:controller=>"request", :action => "select_authority"} %>
diff --git a/app/views/public_body/show.html.erb b/app/views/public_body/show.html.erb
index 403216c6c..e7c5fa2b6 100644
--- a/app/views/public_body/show.html.erb
+++ b/app/views/public_body/show.html.erb
@@ -39,36 +39,22 @@
         <% end %>
         </p>
 
-        <% if @public_body.is_requestable? || @public_body.not_requestable_reason == 'bad_contact' %>
-            <% if @public_body.has_notes? %>
-                <p><%= @public_body.notes_as_html.html_safe %></p>
-            <% end %>
-            <% if @public_body.eir_only? %>
-                <p><%= _('You can only request information about the environment from this authority.')%></p>
-            <% end %>
-        <% else %>
-            <% if @public_body.not_requestable_reason == 'not_apply' %>
-                <p><%= _('Freedom of Information law does not apply to this authority, so you cannot make
-                a request to it.')%></p>
-            <% elsif @public_body.not_requestable_reason == 'defunct' %>
-                <p><%= _('This authority no longer exists, so you cannot make a request to it.')%></p>
-            <% else %>
-                <p><%= _('For an unknown reason, it is not possible to make a request to this authority.')%></p>
-            <% end %>
-        <% end %>
-
         <div id="stepwise_make_request">
-            <% if @public_body.is_requestable? || @public_body.not_requestable_reason == 'bad_contact' %>
-                <%= link_to _("Make a request to this authority"), new_request_to_body_path(:url_name => @public_body.url_name), :class => "link_button_green" %>
-            <% elsif @public_body.has_notes? %>
-                <%= @public_body.notes_as_html.html_safe %>
-            <% end %>
+          <% if @public_body.has_notes? %>
+            <%= @public_body.notes_as_html.html_safe %>
+          <% end %>
 
-            <% if @public_body.override_request_email %>
-                <p>
-                    <%= _("<strong>Note:</strong> Because we're testing, requests are being sent to {{email}} rather than to the actual authority.", :email => @public_body.override_request_email) %>
-                </p>
+          <% if @public_body.is_requestable? %>
+            <% if @public_body.eir_only? %>
+              <p><%= _('You can only request information about the environment from this authority.')%></p>
             <% end %>
+
+            <%= link_to _("Make a request to this authority"),
+                        new_request_to_body_path(:url_name => @public_body.url_name),
+                        :class => "link_button_green" %>
+          <% else %>
+            <p><%= public_body_not_requestable_reasons(@public_body).first %></p>
+          <% end %>
         </div>
     </div>
 
diff --git a/app/views/public_body/view_email.html.erb b/app/views/public_body/view_email.html.erb
index 5f4bc95f4..399caaa61 100644
--- a/app/views/public_body/view_email.html.erb
+++ b/app/views/public_body/view_email.html.erb
@@ -24,7 +24,7 @@
 </p>
 
 <p>
-    <% if @public_body.is_requestable? || @public_body.not_requestable_reason != 'bad_contact' %>
+    <% if @public_body.has_request_email? %>
         <%=  raw(_('If the address is wrong, or you know a better address, please <a href="{{url}}">contact us</a>.', :url => help_contact_path.html_safe)) %>
     <% else %>
        <%= raw(_(' If you know the address to use, then please <a href="{{url}}">send it to us</a>.
diff --git a/app/views/request/new.html.erb b/app/views/request/new.html.erb
index 51224129e..688d9e87b 100644
--- a/app/views/request/new.html.erb
+++ b/app/views/request/new.html.erb
@@ -99,6 +99,12 @@
                 </div>
               <% end %>
 
+              <% if @info_request.public_body.override_request_email %>
+                <div class="warning">
+                  <%= _("<strong>Note:</strong> Because we're testing, requests are being sent to {{email}} rather than to the actual authority.", :email => @info_request.public_body.override_request_email) %>
+                </div>
+              <% end %>
+
               <% if @info_request.public_body.eir_only? %>
                 <h3><%= _('Please ask for environmental information only') %></h3>
 
diff --git a/spec/helpers/public_body_helper_spec.rb b/spec/helpers/public_body_helper_spec.rb
new file mode 100644
index 000000000..89a4d0641
--- /dev/null
+++ b/spec/helpers/public_body_helper_spec.rb
@@ -0,0 +1,50 @@
+require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
+
+describe PublicBodyHelper do
+  include PublicBodyHelper
+
+  describe :public_body_not_requestable_reasons do
+
+    before do
+      @body = FactoryGirl.build(:public_body)
+    end
+
+    it 'returns an empty array if there are no reasons' do
+      expect(public_body_not_requestable_reasons(@body)).to eq([])
+    end
+
+    it 'includes a reason if the law does not apply to the authority' do
+      @body.tag_string = 'not_apply'
+      msg = 'Freedom of Information law does not apply to this authority, so you cannot make a request to it.'
+      expect(public_body_not_requestable_reasons(@body)).to include(msg)
+    end
+
+    it 'includes a reason if the body no longer exists' do
+      @body.tag_string = 'defunct'
+      msg = 'This authority no longer exists, so you cannot make a request to it.'
+      expect(public_body_not_requestable_reasons(@body)).to include(msg)
+    end 
+
+    it 'links to the request page if the body has no contact email' do
+      @body.request_email = ''
+      msg = %Q(<a href="/new/#{ @body.url_name }"
+               class="link_button_green">Make
+               a request to this authority</a>).squish
+
+      expect(public_body_not_requestable_reasons(@body)).to include(msg)
+    end
+
+    it 'returns the reasons in order of importance' do
+      @body.tag_string = 'defunct not_apply'
+      @body.request_email = ''
+
+      reasons = public_body_not_requestable_reasons(@body)
+
+      expect(reasons[0]).to match(/no longer exists/)
+      expect(reasons[1]).to match(/does not apply/)
+      expect(reasons[2]).to match(/Make a request/)
+    end
+
+  end
+
+end
diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb
index a9e801bfd..7d48fa169 100644
--- a/spec/models/public_body_spec.rb
+++ b/spec/models/public_body_spec.rb
@@ -868,6 +868,53 @@ describe PublicBody do
 
     end
 
+    describe :has_request_email? do
+
+        before do
+            @body = PublicBody.new(:request_email => 'test@example.com')
+        end
+
+        it 'should return false if request_email is nil' do
+            @body.request_email = nil
+            @body.has_request_email?.should == false
+        end
+
+        it 'should return false if the request email is "blank"' do
+            @body.request_email = 'blank'
+            @body.has_request_email?.should == false
+        end
+
+        it 'should return false if the request email is an empty string' do
+            @body.request_email = ''
+            @body.has_request_email?.should == false
+        end
+
+        it 'should return true if the request email is an email address' do
+            @body.has_request_email?.should == true
+        end
+    end
+
+    describe :special_not_requestable_reason do
+
+        before do
+            @body = PublicBody.new
+        end
+
+        it 'should return true if the body is defunct' do
+            @body.stub!(:defunct?).and_return(true)
+            @body.special_not_requestable_reason?.should == true
+        end
+
+        it 'should return true if FOI does not apply' do
+            @body.stub!(:not_apply?).and_return(true)
+            @body.special_not_requestable_reason?.should == true
+        end
+
+        it 'should return false if the body is not defunct and FOI applies' do
+            @body.special_not_requestable_reason?.should == false
+        end
+    end
+
 end
 
 describe PublicBody, " when override all public body request emails set" do
@@ -960,3 +1007,81 @@ describe PublicBody, 'when asked for popular bodies' do
     end
 
 end
+
+describe PublicBody do
+
+    describe :is_requestable? do
+
+        before do
+            @body = PublicBody.new(:request_email => 'test@example.com')
+        end
+
+        it 'should return false if the body is defunct' do
+            @body.stub!(:defunct?).and_return true
+            @body.is_requestable?.should == false
+        end
+
+        it 'should return false if FOI does not apply' do
+            @body.stub!(:not_apply?).and_return true
+            @body.is_requestable?.should == false
+        end
+
+        it 'should return false there is no request_email' do
+            @body.stub!(:has_request_email?).and_return false
+            @body.is_requestable?.should == false
+        end
+
+        it 'should return true if the request email is an email address' do
+            @body.is_requestable?.should == true
+        end
+
+    end
+
+    describe :is_followupable? do
+
+        before do
+            @body = PublicBody.new(:request_email => 'test@example.com')
+        end
+
+        it 'should return false there is no request_email' do
+            @body.stub!(:has_request_email?).and_return false
+            @body.is_followupable?.should == false
+        end
+
+        it 'should return true if the request email is an email address' do
+            @body.is_followupable?.should == true
+        end
+
+    end
+
+    describe :not_requestable_reason do
+
+        before do
+            @body = PublicBody.new(:request_email => 'test@example.com')
+        end
+
+        it 'should return "defunct" if the body is defunct' do
+            @body.stub!(:defunct?).and_return true
+            @body.not_requestable_reason.should == 'defunct'
+        end
+
+        it 'should return "not_apply" if FOI does not apply' do
+            @body.stub!(:not_apply?).and_return true
+            @body.not_requestable_reason.should == 'not_apply'
+        end
+
+
+        it 'should return "bad_contact" there is no request_email' do
+            @body.stub!(:has_request_email?).and_return false
+            @body.not_requestable_reason.should == 'bad_contact'
+        end
+
+        it 'should raise an error if the body is not defunct, FOI applies and has an email address' do
+            expected_error = "not_requestable_reason called with type that has no reason"
+            lambda{ @body.not_requestable_reason }.should raise_error(expected_error)
+        end
+
+    end
+
+end
+
diff --git a/spec/views/public_body/show.html.erb_spec.rb b/spec/views/public_body/show.html.erb_spec.rb
index 0559fc8ef..917c0c793 100644
--- a/spec/views/public_body/show.html.erb_spec.rb
+++ b/spec/views/public_body/show.html.erb_spec.rb
@@ -2,10 +2,10 @@ require File.expand_path(File.join('..', '..', '..', 'spec_helper'), __FILE__)
 
 describe "public_body/show" do
     before do
-        @pb = mock_model(PublicBody, 
-                         :name => 'Test Quango', 
+        @pb = mock_model(PublicBody,
+                         :name => 'Test Quango',
                          :short_name => 'tq',
-                         :url_name => 'testquango', 
+                         :url_name => 'testquango',
                          :notes => '',
                          :type_of_authority => 'A public body',
                          :eir_only? => nil,
@@ -15,6 +15,7 @@ describe "public_body/show" do
                          :calculated_home_page => '')
         @pb.stub!(:override_request_email).and_return(nil)
         @pb.stub!(:is_requestable?).and_return(true)
+        @pb.stub!(:special_not_requestable_reason?).and_return(false)
         @pb.stub!(:has_notes?).and_return(false)
         @pb.stub!(:has_tag?).and_return(false)
         @xap = mock(ActsAsXapian::Search, :matches_estimated => 2)
@@ -69,7 +70,7 @@ describe "public_body/show" do
         response.should have_selector("div#header_right") do
             have_selector "a", :href => /www.charity-commission.gov.uk.*RegisteredCharityNumber=12345$/
         end
-    end 
+    end
 
     it "should link to Scottish Charity Regulator site if we have an SC number" do
         @pb.stub!(:has_tag?).and_return(true)
@@ -79,7 +80,7 @@ describe "public_body/show" do
         response.should have_selector("div#header_right") do
             have_selector "a", :href => /www.oscr.org.uk.*id=SC1234$/
         end
-    end 
+    end
 
 
     it "should not link to Charity Commission site if we don't have number" do
@@ -87,15 +88,15 @@ describe "public_body/show" do
         response.should have_selector("div#header_right") do
             have_selector "a", :href => /charity-commission.gov.uk/
         end
-    end 
+    end
 
 
 end
 
-def mock_event 
-    return mock_model(InfoRequestEvent, 
-        :info_request => mock_model(InfoRequest, 
-            :title => 'Title', 
+def mock_event
+    return mock_model(InfoRequestEvent,
+        :info_request => mock_model(InfoRequest,
+            :title => 'Title',
             :url_title => 'title',
             :display_status => 'waiting_response',
             :calculate_status => 'waiting_response',