Merge branch 'rails-3-develop' of ssh://git.mysociety.org/data/git/public/alaveteli into rails-3-develop

8 files changed, 84 insertions, 32 deletions

diff --git a/app/controllers/request_game_controller.rb b/app/controllers/request_game_controller.rb
index 298818bc7..7eadc1204 100644
--- a/app/controllers/request_game_controller.rb
+++ b/app/controllers/request_game_controller.rb
@@ -13,7 +13,8 @@ class RequestGameController < ApplicationController
         @total = InfoRequest.count
         @done = @total - @missing
         @percentage = (@done.to_f / @total.to_f * 10000).round / 100.0
-        @requests = InfoRequest.get_random_old_unclassified(3, :conditions => ["prominence = 'normal'"])
+        @requests = InfoRequest.includes(:public_body, :user).get_random_old_unclassified(3, :conditions => ["prominence = 'normal'"])
+
 
         if @missing == 0
             flash[:notice] = _('<p>All done! Thank you very much for your help.</p><p>There are <a href="{{helpus_url}}">more things you can do</a> to help {{site_name}}.</p>',
diff --git a/app/mailers/request_mailer.rb b/app/mailers/request_mailer.rb
index c9decc6db..bf04b1ab9 100644
--- a/app/mailers/request_mailer.rb
+++ b/app/mailers/request_mailer.rb
@@ -64,7 +64,7 @@ class RequestMailer < ApplicationMailer
 
         mail(:from => user.name_and_email,
              :to => contact_from_name_and_email,
-             :subject => _("FOI response requires admin ({{reason}}) - {{title}}", :reason => info_request.described_state, :title => info_request.title).html_safe)
+             :subject => _("FOI response requires admin ({{reason}}) - {{title}}", :reason => info_request.described_state, :title => info_request.title.html_safe))
     end
 
     # Tell the requester that a new response has arrived
@@ -80,7 +80,7 @@ class RequestMailer < ApplicationMailer
 
         mail(:from => contact_from_name_and_email,
              :to => info_request.user.name_and_email,
-             :subject => (_("New response to your FOI request - ") + info_request.title).html_safe,
+             :subject => _("New response to your FOI request - ") + info_request.title.html_safe,
              :charset => "UTF-8",
              # not much we can do if the user's email is broken
              :reply_to => contact_from_name_and_email)
@@ -105,7 +105,7 @@ class RequestMailer < ApplicationMailer
 
         mail(:from => contact_from_name_and_email,
              :to => user.name_and_email,
-             :subject => (_("Delayed response to your FOI request - ") + info_request.title).html_safe)
+             :subject => _("Delayed response to your FOI request - ") + info_request.title.html_safe)
     end
 
     # Tell the requester that the public body is very late in replying
@@ -125,7 +125,7 @@ class RequestMailer < ApplicationMailer
 
         mail(:from => contact_from_name_and_email,
              :to => user.name_and_email,
-             :subject => (_("You're long overdue a response to your FOI request - ") + info_request.title).html_safe)
+             :subject => _("You're long overdue a response to your FOI request - ") + info_request.title.html_safe)
     end
 
     # Tell the requester that they need to say if the new response
@@ -183,7 +183,7 @@ class RequestMailer < ApplicationMailer
 
         mail(:from => contact_from_name_and_email,
              :to => info_request.user.name_and_email,
-             :subject => (_("Clarify your FOI request - ") + info_request.title).html_safe)
+             :subject => _("Clarify your FOI request - ") + info_request.title.html_safe)
     end
 
     # Tell requester that somebody add an annotation to their request
@@ -197,7 +197,7 @@ class RequestMailer < ApplicationMailer
 
         mail(:from => contact_from_name_and_email,
              :to => info_request.user.name_and_email,
-             :subject => (_("Somebody added a note to your FOI request - ") + info_request.title).html_safe)
+             :subject => _("Somebody added a note to your FOI request - ") + info_request.title.html_safe)
     end
     def comment_on_alert_plural(info_request, count, earliest_unalerted_comment)
         @count, @info_request = count, info_request
@@ -209,7 +209,7 @@ class RequestMailer < ApplicationMailer
 
         mail(:from => contact_from_name_and_email,
              :to => info_request.user.name_and_email,
-             :subject => (_("Some notes have been added to your FOI request - ") + info_request.title).html_safe)
+             :subject => _("Some notes have been added to your FOI request - ") + info_request.title.html_safe)
     end
 
     # Class function, called by script/mailin with all incoming responses.
diff --git a/app/models/info_request.rb b/app/models/info_request.rb
index 7f6b358db..57a8fd7e0 100644
--- a/app/models/info_request.rb
+++ b/app/models/info_request.rb
@@ -803,11 +803,8 @@ public
 
     # Text from the the initial request, for use in summary display
     def initial_request_text
-        if outgoing_messages.empty? # mainly for use with incomplete fixtures
-            return ""
-        end
-        excerpt = self.outgoing_messages[0].get_text_for_indexing
-        return excerpt
+        return '' if outgoing_messages.empty? # mainly for use with incomplete fixtures
+        outgoing_messages.first.get_text_for_indexing
     end
 
     # Returns index of last event which is described or nil if none described.
diff --git a/app/models/outgoing_message.rb b/app/models/outgoing_message.rb
index fa83c7381..c2c8ef4f2 100644
--- a/app/models/outgoing_message.rb
+++ b/app/models/outgoing_message.rb
@@ -302,7 +302,7 @@ class OutgoingMessage < ActiveRecord::Base
     end
 
     def set_default_letter
-        self.body = get_default_message if body.nil?
+        self.body = get_default_message if raw_body.nil?
     end
 
     def format_of_body
diff --git a/app/views/request/_request_listing_single.html.erb b/app/views/request/_request_listing_single.html.erb
index 50f889d75..0f1d7f4ef 100644
--- a/app/views/request/_request_listing_single.html.erb
+++ b/app/views/request/_request_listing_single.html.erb
@@ -2,14 +2,20 @@
   <span class="head">
       <%= link_to h(info_request.title), (@play_urls ? categorise_request_path(:url_title => info_request.url_title) : request_path(info_request)) %>
   </span>
+
   <span class="desc">
       <%= excerpt(info_request.initial_request_text, "", :radius => 150) %>
-    </span>
-    <span class="bottomline icon_<%= info_request.calculate_status %>">
-        <strong>
+  </span>
+
+  <span class="bottomline icon_<%= info_request.calculate_status %>">
+      <strong>
         <%= info_request.display_status %>
-        </strong><br>
-        <%= _('Requested from {{public_body_name}} by {{info_request_user}} on {{date}}',:public_body_name=>public_body_link(info_request.public_body),:info_request_user=>user_link(info_request.user),:date=>simple_date(info_request.created_at)) %>
-    </span>
+      </strong>
+      <br>
+      <%= _('Requested from {{public_body_name}} by {{info_request_user}} on {{date}}',
+            :public_body_name => public_body_link(info_request.public_body),
+            :info_request_user => user_link(info_request.user),
+            :date => simple_date(info_request.created_at)) %>
+  </span>
 </div>
 
diff --git a/app/views/request_game/play.html.erb b/app/views/request_game/play.html.erb
index 783bb7f00..44fe641f9 100644
--- a/app/views/request_game/play.html.erb
+++ b/app/views/request_game/play.html.erb
@@ -10,43 +10,47 @@
         %>
         <br><%=pluralize(@missing, 'request')%> left to categorise / <%=@total %> total
     </p>
+
     <h2>Top recent players</h2>
     <table>
-    <% c = 0; for classifications in @league_table_28_days %>
+    <% @league_table_28_days.each_with_index do |classifications, index| %>
         <tr>
-            <td> <%= c += 1 %>. <td>
+            <td> <%= index += 1 %>. <td>
             <td> <%= user_link(classifications.user) %> </td>
-            <td> <%=pluralize(classifications.cnt, 'request').gsub(" ", "&nbsp;").html_safe %> </td>
+            <td> <%= pluralize(classifications.cnt, 'request').gsub(" ", "&nbsp;").html_safe %> </td>
         </tr>
     <% end %>
     </table>
 
     <h2>All time best players</h2>
     <table>
-    <% c = 0; for classifications in @league_table_all_time %>
+    <% @league_table_all_time.each_with_index do |classifications, index| %>
         <tr>
-            <td> <%= c += 1 %>. <td>
+            <td> <%= index += 1 %>. <td>
             <td> <%= user_link(classifications.user) %> </td>
             <td> <%= pluralize(classifications.cnt, 'request').gsub(" ", "&nbsp;").html_safe %> </td>
         </tr>
     <% end %>
     </table>
 </div>
+
 <div id="game">
   <h2><%= _("Play the request categorisation game!")%></h2>
+
   <p><%= _("Some people who've made requests haven't let us know whether they were
 successful or not.  We need <strong>your</strong> help &ndash;
 choose one of these requests, read it, and let everyone know whether or not the
 information has been provided. Everyone'll be exceedingly grateful.")%></p>
-  <% for info_request in @requests %>
+
+  <% @requests.each do |info_request| %>
       <%= render :partial => 'request/request_listing_single', :locals => { :info_request => info_request } %>
   <% end %>
+
   <p id="game_buttons">
-  <%= button_to _('I don\'t like these ones &mdash; give me some more!'), categorise_play_url %>
-  <%= button_to _('I don\'t want to do any more tidying now!'), categorise_stop_url %>
+    <%= button_to _('I don\'t like these ones &mdash; give me some more!'), categorise_play_url %>
+    <%= button_to _('I don\'t want to do any more tidying now!'), categorise_stop_url %>
   </p>
+
   <p><%= _('Thanks for helping - your work will make it easier for everyone to find successful
 responses, and maybe even let us make league tables...')%></p>
 </div>
-
-
diff --git a/config/initializers/alaveteli.rb b/config/initializers/alaveteli.rb
index 27d40b223..d09bfec28 100644
--- a/config/initializers/alaveteli.rb
+++ b/config/initializers/alaveteli.rb
@@ -10,7 +10,7 @@ load "debug_helpers.rb"
 load "util.rb"
 
 # Application version
-ALAVETELI_VERSION = '0.21.0.22'
+ALAVETELI_VERSION = '0.21.0.23'
 
 # Add new inflection rules using the following format
 # (all these examples are active by default):
diff --git a/spec/mailers/request_mailer_spec.rb b/spec/mailers/request_mailer_spec.rb
index 9e98dbc00..6b54c25d2 100644
--- a/spec/mailers/request_mailer_spec.rb
+++ b/spec/mailers/request_mailer_spec.rb
@@ -1,6 +1,8 @@
 # encoding: utf-8
 require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
 
+# TODO: Combine all these separate "describe" blocks to tidy things up
+
 describe RequestMailer, " when receiving incoming mail" do
     before(:each) do
         load_raw_emails_data
@@ -411,6 +413,10 @@ describe RequestMailer, 'when sending a new response email' do
     @mail = RequestMailer.new_response(@info_request, @incoming_message)
   end
 
+  it 'should not create HTML entities in the subject line' do
+    mail = RequestMailer.new_response(FactoryGirl.create(:info_request, :title => "Here's a request"), FactoryGirl.create(:incoming_message))
+    expect(mail.subject).to eq "New response to your FOI request - Here's a request"
+  end
 end
 
 describe RequestMailer, 'requires_admin' do
@@ -419,7 +425,7 @@ describe RequestMailer, 'requires_admin' do
                                 :name => 'Bruce Jones')
         @info_request = mock_model(InfoRequest, :user => user,
                                                 :described_state => 'error_message',
-                                                :title => 'Test request',
+                                                :title => "It's a Test request",
                                                 :url_title => 'test_request',
                                                 :law_used_short => 'FOI',
                                                 :id => 123)
@@ -435,4 +441,42 @@ describe RequestMailer, 'requires_admin' do
         mail.body.should include 'Something has gone wrong'
     end
 
+    it 'should not create HTML entities in the subject line' do
+        expect(RequestMailer.requires_admin(@info_request).subject).to eq "FOI response requires admin (error_message) - It's a Test request"
+    end
+end
+
+describe RequestMailer, "overdue_alert" do
+    it 'should not create HTML entities in the subject line' do
+        mail = RequestMailer.overdue_alert(FactoryGirl.create(:info_request, :title => "Here's a request"), FactoryGirl.create(:user))
+        expect(mail.subject).to eq "Delayed response to your FOI request - Here's a request"
+    end
+end
+
+describe RequestMailer, "very_overdue_alert" do
+    it 'should not create HTML entities in the subject line' do
+        mail = RequestMailer.very_overdue_alert(FactoryGirl.create(:info_request, :title => "Here's a request"), FactoryGirl.create(:user))
+        expect(mail.subject).to eq "You're long overdue a response to your FOI request - Here's a request"
+    end
+end
+
+describe RequestMailer, "not_clarified_alert" do
+    it 'should not create HTML entities in the subject line' do
+        mail = RequestMailer.not_clarified_alert(FactoryGirl.create(:info_request, :title => "Here's a request"), FactoryGirl.create(:incoming_message))
+        expect(mail.subject).to eq "Clarify your FOI request - Here's a request"
+    end
+end
+
+describe RequestMailer, "comment_on_alert" do
+    it 'should not create HTML entities in the subject line' do
+        mail = RequestMailer.comment_on_alert(FactoryGirl.create(:info_request, :title => "Here's a request"), FactoryGirl.create(:comment))
+        expect(mail.subject).to eq "Somebody added a note to your FOI request - Here's a request"
+    end
+end
+
+describe RequestMailer, "comment_on_alert_plural" do
+    it 'should not create HTML entities in the subject line' do
+        mail = RequestMailer.comment_on_alert_plural(FactoryGirl.create(:info_request, :title => "Here's a request"), 2, FactoryGirl.create(:comment))
+        expect(mail.subject).to eq "Some notes have been added to your FOI request - Here's a request"
+    end
 end