Merge branch 'develop', remote-tracking branch 'origin' into feature/preview_emails

8 files changed, 292 insertions, 26 deletions

diff --git a/app/controllers/general_controller.rb b/app/controllers/general_controller.rb
index 839064fcd..5a7b006a0 100644
--- a/app/controllers/general_controller.rb
+++ b/app/controllers/general_controller.rb
@@ -32,8 +32,7 @@ class GeneralController < ApplicationController
                 if body_short_names.empty?
                     # This is too slow
                     @popular_bodies = PublicBody.find(:all,
-                        :select => "public_bodies.*, (select count(*) from info_requests where info_requests.public_body_id = public_bodies.id) as c",
-                        :order => "c desc",
+                        :order => "info_requests_count desc",
                         :limit => 32,
                         :conditions => conditions,
                         :joins => :translations
diff --git a/app/models/application_mailer.rb b/app/models/application_mailer.rb
index 044006f7c..b6cd5a4bf 100644
--- a/app/models/application_mailer.rb
+++ b/app/models/application_mailer.rb
@@ -5,7 +5,7 @@
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
 # $Id: application_mailer.rb,v 1.8 2009-02-09 10:37:12 francis Exp $
-
+require 'action_mailer/version'
 class ApplicationMailer < ActionMailer::Base
     # Include all the functions views get, as emails call similar things.
     helper :application
@@ -26,5 +26,126 @@ class ApplicationMailer < ActionMailer::Base
 
     # Site-wide access to configuration settings
     include ConfigHelper
+
+    # Instantiate a new mailer object. If +method_name+ is not +nil+, the mailer
+    # will be initialized according to the named method. If not, the mailer will
+    # remain uninitialized (useful when you only need to invoke the "receive"
+    # method, for instance).
+    def initialize(method_name=nil, *parameters) #:nodoc:
+      create!(method_name, *parameters) if method_name
+    end
+
+    # For each multipart template (e.g. "the_template_file.text.html.erb") available,
+        # add the one from the view path with the highest priority as a part to the mail
+        def render_multipart_templates
+            added_content_types = {}
+            self.view_paths.each do |view_path|
+                Dir.glob("#{view_path}/#{mailer_name}/#{@template}.*").each do |path|
+                  template = view_path["#{mailer_name}/#{File.basename(path)}"]
+
+                  # Skip unless template has a multipart format
+                  next unless template && template.multipart?
+                  next if added_content_types[template.content_type] == true
+                  @parts << Part.new(
+                    :content_type => template.content_type,
+                    :disposition => "inline",
+                    :charset => charset,
+                    :body => render_message(template, @body)
+                  )
+                  added_content_types[template.content_type] = true
+                end
+            end
+        end
+
+        # Look for the current template in each element of view_paths in order,
+        # return the first
+        def find_template
+            self.view_paths.each do |view_path|
+                if template = view_path["#{mailer_name}/#{@template}"]
+                    return template
+                end
+            end
+            return nil
+        end
+
+    if ActionMailer::VERSION::MAJOR == 2
+
+        # This method is a customised version of ActionMailer::Base.create!
+        # modified to allow templates to be selected correctly for multipart
+        # mails when themes have added to the view_paths. The problem from our
+        # point of view with ActionMailer::Base is that it sets template_root to
+        # the first element of the view_paths array and then uses only that (directly
+        # and via template_path, which is created from it) in the create! method when
+        # looking for templates. Our modified version looks for templates in the view_paths
+        # in order.
+        # Changed lines marked with ***
+
+        # Initialize the mailer via the given +method_name+. The body will be
+        # rendered and a new TMail::Mail object created.
+        def create!(method_name, *parameters) #:nodoc:
+          initialize_defaults(method_name)
+          __send__(method_name, *parameters)
+
+          # If an explicit, textual body has not been set, we check assumptions.
+          unless String === @body
+            # First, we look to see if there are any likely templates that match,
+            # which include the content-type in their file name (i.e.,
+            # "the_template_file.text.html.erb", etc.). Only do this if parts
+            # have not already been specified manually.
+            if @parts.empty?
+              # *** render_multipart_templates replaces the following code
+              # Dir.glob("#{template_path}/#{@template}.*").each do |path|
+              #   template = template_root["#{mailer_name}/#{File.basename(path)}"]
+              #
+              #   # Skip unless template has a multipart format
+              #   next unless template && template.multipart?
+              #
+              #   @parts << Part.new(
+              #     :content_type => template.content_type,
+              #     :disposition => "inline",
+              #     :charset => charset,
+              #     :body => render_message(template, @body)
+              #   )
+              # end
+              render_multipart_templates
+
+              unless @parts.empty?
+                @content_type = "multipart/alternative" if @content_type !~ /^multipart/
+                @parts = sort_parts(@parts, @implicit_parts_order)
+              end
+            end
+
+            # Then, if there were such templates, we check to see if we ought to
+            # also render a "normal" template (without the content type). If a
+            # normal template exists (or if there were no implicit parts) we render
+            # it.
+            template_exists = @parts.empty?
+
+            # *** find_template replaces template_root call
+            # template_exists ||= template_root["#{mailer_name}/#{@template}"]
+            template_exists ||= find_template
+
+            @body = render_message(@template, @body) if template_exists
+
+            # Finally, if there are other message parts and a textual body exists,
+            # we shift it onto the front of the parts and set the body to nil (so
+            # that create_mail doesn't try to render it in addition to the parts).
+            if !@parts.empty? && String === @body
+              @parts.unshift ActionMailer::Part.new(:charset => charset, :body => @body)
+              @body = nil
+            end
+          end
+
+          # If this is a multipart e-mail add the mime_version if it is not
+          # already set.
+          @mime_version ||= "1.0" if !@parts.empty?
+
+          # build the mail object itself
+          @mail = create_mail
+        end
+    else
+        raise "ApplicationMailer.create! is obsolete - find another way to ensure that themes can override mail templates for multipart mails"
+    end
+
 end
 
diff --git a/app/views/general/frontpage.rhtml b/app/views/general/frontpage.rhtml
index 38133e7ab..8b078836e 100644
--- a/app/views/general/frontpage.rhtml
+++ b/app/views/general/frontpage.rhtml
@@ -40,7 +40,7 @@
         <ul>
             <% for popular_body in @popular_bodies %>
                 <li><%=public_body_link(popular_body)%>
-                 <%= n_('%d request', '%d requests', popular_body.info_requests.count) % popular_body.info_requests.count %>
+                 <%= n_('%d request', '%d requests', popular_body.info_requests_count) % popular_body.info_requests_count %>
                 </li>
             <% end%>
         </ul>
@@ -52,14 +52,14 @@
 
     <div id="examples_1">
       <h3><%= _("What information has been released?") %></h3>
-       <%= _("{{site_name}} users have made {{number_of_requests}} requests, including:", 
+       <%= _("{{site_name}} users have made {{number_of_requests}} requests, including:",
         :site_name => site_name, :number_of_requests => InfoRequest.count) %>
         <ul>
           <% for event in @request_events %>
             <li>
                <%= public_body_link(event.info_request.public_body) %> <%= _('answered a request about') %>
-               <%=link_to h(event.info_request.title), request_url(event.info_request)%> 
-               <%= _('{{length_of_time}} ago', :length_of_time => time_ago_in_words(event.described_at)) %> 
+               <%=link_to h(event.info_request.title), request_url(event.info_request)%>
+               <%= _('{{length_of_time}} ago', :length_of_time => time_ago_in_words(event.described_at)) %>
                <p class="excerpt" onclick="document.location.href='<%=request_url(event.info_request)%>'"><%= excerpt(event.search_text_main(true), "", 200) %></p>
               </li>
           <% end %>
diff --git a/app/views/help/contact.rhtml b/app/views/help/contact.rhtml
index dd49f7951..37df68f49 100644
--- a/app/views/help/contact.rhtml
+++ b/app/views/help/contact.rhtml
@@ -10,27 +10,18 @@
         <h2>Contact an authority to get official information</h2>
         <ul>
             <li><a href="/new">Go here</a> to make a request, in public, for information
-            from UK public authorities.</li>
+            from public authorities.</li>
 
             <li>
             Asking for private information about yourself?
-            Please read our help page about
-            <a href="/help/requesting#data_protection">data protection</a>.
+            Please read our
+            <a href="/help/requesting#data_protection">help page</a>.
             </li>
         </ul>
 
-        <h2>Take up an issue with Government</h2>
-
-        <ul>
-            <li><a href="http://www.writetothem.com">Write to your MP,
-            local councillor or other representative</a>.
-            <li><a href="http://www.number10.gov.uk/">Number 10</a> is a good place to start if you would like to take an issue up with central government. </li>
-        </ul>
-
-
     <% end %>
 
-    <h2>Contact the WhatDoTheyKnow team</h2>
+    <h2>Contact the <%= site_name %> team</h2>
     <% if !flash[:notice] %>
             <ul>
             <li>
@@ -91,8 +82,7 @@
 
     <p class="form_note">
     We can only help you with <strong>technical problems</strong>, or questions
-    about Freedom of Information. See the top of this page if you would like to
-    contact the Government.
+    about Freedom of Information.
     </P>
 
 
diff --git a/config/general.yml-example b/config/general.yml-example
index fd27b151a..7aa01e899 100644
--- a/config/general.yml-example
+++ b/config/general.yml-example
@@ -35,6 +35,7 @@ SPECIAL_REPLY_VERY_LATE_AFTER_DAYS: 60
 WORKING_OR_CALENDAR_DAYS: working
 
 # example public bodies for the home page, semicolon delimited - short_names
+# Comment out if you want this to be auto-generated. WARNING: this is slow & don't use production!
 FRONTPAGE_PUBLICBODY_EXAMPLES: 'tgq'
 
 # URLs of themes to download and use (when running rails-post-deploy
diff --git a/locale/es/app.po b/locale/es/app.po
index 5e571502c..aa1e5c650 100644
--- a/locale/es/app.po
+++ b/locale/es/app.po
@@ -1,7 +1,7 @@
 # SOME DESCRIPTIVE TITLE.
 # Copyright (C) YEAR THE PACKAGE'S COPYRIGHT HOLDER
 # This file is distributed under the same license as the PACKAGE package.
-# 
+#
 # Translators:
 # David Cabo <david.cabo@gmail.com>, 2011-2012.
 #   <fabrizio.scrollini@gmail.com>, 2012.
diff --git a/spec/models/application_mailer_spec.rb b/spec/models/application_mailer_spec.rb
index 12527c6e8..a90f79c01 100644
--- a/spec/models/application_mailer_spec.rb
+++ b/spec/models/application_mailer_spec.rb
@@ -1,8 +1,160 @@
 require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
 
-describe ApplicationMailer, " when blah" do
-    before do
+
+describe ApplicationMailer do
+
+    context 'when using plugins' do
+
+        def set_base_views
+            ApplicationMailer.class_eval do
+                @previous_view_paths = self.view_paths.dup
+                self.view_paths.clear
+                self.view_paths << File.join(Rails.root, 'spec', 'fixtures', 'theme_views', 'core')
+            end
+        end
+
+        def add_mail_methods(method_names)
+            method_names.each{ |method_name| ApplicationMailer.send(:define_method, method_name){} }
+        end
+
+        def remove_mail_methods(method_names)
+            method_names.each do |method_name|
+                if ApplicationMailer.respond_to?(method_name)
+                    ApplicationMailer.send(:remove_method, method_name)
+                end
+            end
+        end
+
+        def prepend_theme_views(theme_name)
+            ApplicationMailer.class_eval do
+                view_paths.unshift File.join(Rails.root, 'spec', 'fixtures', 'theme_views', theme_name)
+            end
+        end
+
+        def append_theme_views(theme_name)
+            ApplicationMailer.class_eval do
+                view_paths << File.join(Rails.root, 'spec', 'fixtures', 'theme_views', theme_name)
+            end
+        end
+
+        def reset_views
+            ApplicationMailer.class_eval do
+                 self.view_paths = @previous_view_paths
+            end
+        end
+
+        def create_multipart_method(method_name)
+            ApplicationMailer.send(:define_method, method_name) do
+                attachment :content_type => 'message/rfc822',
+                           :body => 'xxx',
+                           :filename => "original.eml",
+                           :transfer_encoding => '7bit',
+                           :content_disposition => 'inline'
+            end
+        end
+
+        before do
+            set_base_views
+            add_mail_methods(['simple', 'theme_only', 'core_only', 'neither'])
+        end
+
+        describe 'when a plugin prepends its mail templates to the view paths' do
+
+            it 'should render a theme template in preference to a core template' do
+                prepend_theme_views('theme_one')
+                @mail = ApplicationMailer.create_simple()
+                @mail.body.should match('Theme simple')
+            end
+
+            it 'should render the template provided by the theme if no template is available in core' do
+                prepend_theme_views('theme_one')
+                @mail = ApplicationMailer.create_theme_only()
+                @mail.body.should match('Theme only')
+            end
+
+            it 'should render the template provided by core if there is no theme template' do
+                prepend_theme_views('theme_one')
+                @mail = ApplicationMailer.create_core_only()
+                @mail.body.should match('Core only')
+            end
+
+            it 'should raise an error if the template is in neither core nor theme' do
+                prepend_theme_views('theme_one')
+                lambda{ ApplicationMailer.create_neither() }.should raise_error('Missing template application_mailer/neither.erb in view path spec/fixtures/theme_views/theme_one:spec/fixtures/theme_views/core')
+            end
+
+            it 'should render a multipart email using a theme template' do
+                prepend_theme_views('theme_one')
+                create_multipart_method('multipart_theme_only')
+                @mail = ApplicationMailer.create_multipart_theme_only()
+                @mail.parts.size.should == 2
+                message_part = @mail.parts[0].to_s
+                message_part.should match("Theme multipart")
+            end
+
+            it 'should render a multipart email using a core template' do
+                prepend_theme_views('theme_one')
+                create_multipart_method('multipart_core_only')
+                @mail = ApplicationMailer.create_multipart_core_only()
+                @mail.parts.size.should == 2
+                message_part = @mail.parts[0].to_s
+                message_part.should match("Core multipart")
+            end
+
+        end
+
+        describe 'when a plugin appends its mail templates to the view paths' do
+
+            it 'should render a core template in preference to a theme template' do
+                append_theme_views('theme_one')
+                @mail = ApplicationMailer.create_simple()
+                @mail.body.should match('Core simple')
+            end
+
+            it 'should render the template provided by the theme if no template is available in core' do
+                append_theme_views('theme_one')
+                @mail = ApplicationMailer.create_theme_only()
+                @mail.body.should match('Theme only')
+            end
+
+            it 'should render the template provided by core if there is no theme template' do
+                append_theme_views('theme_one')
+                @mail = ApplicationMailer.create_core_only()
+                @mail.body.should match('Core only')
+            end
+
+            it 'should raise an error if the template is in neither core nor theme' do
+                append_theme_views('theme_one')
+                lambda{ ApplicationMailer.create_neither() }.should raise_error('Missing template application_mailer/neither.erb in view path spec/fixtures/theme_views/core:spec/fixtures/theme_views/theme_one')
+            end
+
+            it 'should render a multipart email using a core template' do
+                append_theme_views('theme_one')
+                create_multipart_method('multipart_core_only')
+                @mail = ApplicationMailer.create_multipart_core_only()
+                @mail.parts.size.should == 2
+                message_part = @mail.parts[0].to_s
+                message_part.should match("Core multipart")
+            end
+
+            it 'should render a multipart email using a theme template' do
+                append_theme_views('theme_one')
+                create_multipart_method('multipart_theme_only')
+                @mail = ApplicationMailer.create_multipart_theme_only()
+                @mail.parts.size.should == 2
+                message_part = @mail.parts[0].to_s
+                message_part.should match("Theme multipart")
+            end
+
+        end
+
+        after do
+            reset_views
+            remove_mail_methods(['simple', 'theme_only', 'core_only', 'neither', 'multipart'])
+        end
     end
+
 end
 
 
+
diff --git a/spec/models/request_mailer_spec.rb b/spec/models/request_mailer_spec.rb
index 98681a9e9..eec362ad3 100644
--- a/spec/models/request_mailer_spec.rb
+++ b/spec/models/request_mailer_spec.rb
@@ -97,10 +97,12 @@ describe RequestMailer, " when receiving incoming mail" do
         # check attached bounce is good copy of incoming-request-plain.email
         mail.multipart?.should == true
         mail.parts.size.should == 2
+        message_part = mail.parts[0].to_s
         bounced_mail = TMail::Mail.parse(mail.parts[1].body)
         bounced_mail.to.should == [ ir.incoming_email ]
         bounced_mail.from.should == [ 'geraldinequango@localhost' ]
-        bounced_mail.body.include?("That's so totally a rubbish question")
+        bounced_mail.body.include?("That's so totally a rubbish question").should be_true
+        message_part.include?("marked to no longer receive responses").should be_true
         deliveries.clear
     end
 
@@ -324,3 +326,4 @@ describe RequestMailer, 'when sending mail when someone has updated an old uncla
     end
 
 end
+