Merge remote-tracking branch 'mysociety/develop' into rails-3-develop

Conflicts:
	Gemfile
	Gemfile.lock
	app/controllers/admin_request_controller.rb
	app/controllers/admin_track_controller.rb
	app/controllers/request_controller.rb
	app/controllers/services_controller.rb
	app/helpers/link_to_helper.rb
	app/mailers/request_mailer.rb
	app/models/application_mailer.rb
	app/models/info_request.rb
	app/views/admin_censor_rule/edit.html.erb
	app/views/admin_censor_rule/new.html.erb
	app/views/admin_public_body/_form.html.erb
	app/views/admin_public_body/_locale_selector.html.erb
	app/views/admin_public_body/_one_list.html.erb
	app/views/admin_public_body/edit.html.erb
	app/views/admin_public_body/list.html.erb
	app/views/admin_public_body/new.html.erb
	app/views/admin_request/_incoming_message_actions.html.erb
	app/views/admin_request/edit.html.erb
	app/views/admin_request/edit_comment.html.erb
	app/views/admin_request/edit_outgoing.html.erb
	app/views/admin_request/list.html.erb
	app/views/admin_request/list_old_unclassified.html.erb
	app/views/admin_request/show.html.erb
	app/views/admin_track/_some_tracks.html.erb
	app/views/admin_track/list.html.erb
	app/views/admin_user/edit.html.erb
	app/views/admin_user/list.html.erb
	app/views/admin_user/show.html.erb
	app/views/general/_footer.html.erb
	app/views/general/exception_caught.html.erb
	app/views/help/contact.html.erb
	app/views/layouts/default.html.erb
	app/views/public_body/_alphabet.html.erb
	app/views/request/_request_listing_single.html.erb
	app/views/request/_sidebar.html.erb
	app/views/request/new.html.erb
	app/views/request/show.html.erb
	app/views/request_mailer/external_response.rhtml
	app/views/request_mailer/fake_response.rhtml
	config/environment.rb
	config/environments/production.rb
	config/routes.rb
	spec/controllers/admin_censor_rule_controller_spec.rb
	spec/controllers/request_controller_spec.rb
	spec/controllers/track_controller_spec.rb
	spec/helpers/link_to_helper_spec.rb
	spec/mailers/request_mailer_spec.rb
	spec/models/info_request_spec.rb
	spec/spec_helper.rb
	spec/views/public_body/show.html.erb_spec.rb
	spec/views/request/show.html.erb_spec.rb
	vendor/plugins/rails_xss/lib/rails_xss/erubis.rb

1 files changed, 80 insertions, 76 deletions

diff --git a/app/models/info_request.rb b/app/models/info_request.rb
index 5f5cc28c7..130c5b02c 100644
--- a/app/models/info_request.rb
+++ b/app/models/info_request.rb
@@ -160,8 +160,31 @@ class InfoRequest < ActiveRecord::Base
     rescue MissingSourceFile, NameError
     end
 
+    # only check on create, so existing models with mixed case are allowed
+    def validate_on_create
+        if !self.title.nil? && !MySociety::Validate.uses_mixed_capitals(self.title, 10)
+            errors.add(:title, _('Please write the summary using a mixture of capital and lower case letters. This makes it easier for others to read.'))
+        end
+        if !self.title.nil? && title.size > 200
+            errors.add(:title, _('Please keep the summary short, like in the subject of an email. You can use a phrase, rather than a full sentence.'))
+        end
+        if !self.title.nil? && self.title =~ /^(FOI|Freedom of Information)\s*requests?$/i
+            errors.add(:title, _('Please describe more what the request is about in the subject. There is no need to say it is an FOI request, we add that on anyway.'))
+        end
+    end
+
     OLD_AGE_IN_DAYS = 21.days
 
+    def after_initialize
+        if self.described_state.nil?
+            self.described_state = 'waiting_response'
+        end
+        # FOI or EIR?
+        if !self.public_body.nil? && self.public_body.eir_only?
+            self.law_used = 'eir'
+        end
+    end
+
     def visible_comments
         self.comments.find(:all, :conditions => 'visible')
     end
@@ -529,15 +552,15 @@ public
     end
 
     def requires_admin?
-        return true if InfoRequest.requires_admin_states.include?(described_state)
-        return false
+        ['requires_admin', 'error_message', 'attention_requested'].include?(described_state)
     end
 
     # change status, including for last event for later historical purposes
-    def set_described_state(new_state, set_by = nil)
+    def set_described_state(new_state, set_by = nil, message = "")
+        old_described_state = described_state
         ActiveRecord::Base.transaction do
             self.awaiting_description = false
-            last_event = self.get_last_event
+            last_event = self.info_request_events.last
             last_event.described_state = new_state
             self.described_state = new_state
             last_event.save!
@@ -549,9 +572,23 @@ public
         if self.requires_admin?
             # Check there is someone to send the message "from"
             if !set_by.nil? || !self.user.nil?
-                RequestMailer.requires_admin(self, set_by).deliver
+                RequestMailer.requires_admin(self, set_by, message).deliver
             end
         end
+
+        unless set_by.nil? || is_actual_owning_user?(set_by) || described_state == 'attention_requested'
+            # Log the status change by someone other than the requester
+            event = log_event("status_update",
+                { :user_id => set_by.id,
+                  :old_described_state => old_described_state,
+                  :described_state => described_state,
+                })
+            # Create a classification event for league tables
+            RequestClassification.create!(:user_id => set_by.id,
+                                          :info_request_event_id => event.id)
+
+            RequestMailer.deliver_old_unclassified_updated(self) if !is_external?
+        end
     end
 
     # Work out what the situation of the request is. In addition to values of
@@ -700,41 +737,28 @@ public
         self.info_request_events.create!(:event_type => type, :params => params)
     end
 
+    def response_events
+        self.info_request_events.select{|e| e.response?}
+    end
+
     # The last response is the default one people might want to reply to
     def get_last_response_event_id
-        for e in self.info_request_events.reverse
-            if e.event_type == 'response'
-                return e.id
-            end
-        end
-        return nil
-
+        get_last_response_event.id if get_last_response_event
     end
     def get_last_response_event
-        for e in self.info_request_events.reverse
-            if e.event_type == 'response'
-                return e
-            end
-        end
-        return nil
+        response_events.last
     end
     def get_last_response
-        last_response_event = self.get_last_response_event
-        if last_response_event.nil?
-            return nil
-        else
-            return last_response_event.incoming_message
-        end
+        get_last_response_event.incoming_message if get_last_response_event
+    end
+
+    def outgoing_events
+        info_request_events.select{|e| e.outgoing? }
     end
 
     # The last outgoing message
     def get_last_outgoing_event
-        for e in self.info_request_events.reverse
-            if [ 'sent', 'followup_sent' ].include?(e.event_type)
-                return e
-            end
-        end
-        return nil
+        outgoing_events.last
     end
 
     # Text from the the initial request, for use in summary display
@@ -802,46 +826,31 @@ public
 
     # Display version of status
     def InfoRequest.get_status_description(status)
-        if status == 'waiting_classification'
-            _("Awaiting classification.")
-        elsif status == 'waiting_response'
-            _("Awaiting response.")
-        elsif status == 'waiting_response_overdue'
-            _("Delayed.")
-        elsif status == 'waiting_response_very_overdue'
-            _("Long overdue.")
-        elsif status == 'not_held'
-            _("Information not held.")
-        elsif status == 'rejected'
-            _("Refused.")
-        elsif status == 'partially_successful'
-            _("Partially successful.")
-        elsif status == 'successful'
-            _("Successful.")
-        elsif status == 'waiting_clarification'
-            _("Waiting clarification.")
-        elsif status == 'gone_postal'
-            _("Handled by post.")
-        elsif status == 'internal_review'
-            _("Awaiting internal review.")
-        elsif status == 'error_message'
-            _("Delivery error")
-        elsif status == 'requires_admin'
-            _("Unusual response.")
-        elsif status == 'attention_requested'
-            _("Reported for administrator attention.")
-        elsif status == 'user_withdrawn'
-            _("Withdrawn by the requester.")
-        elsif status == 'vexatious'
-            _("Considered by administrators as vexatious and hidden from site.")
-        elsif status == 'not_foi'
-            _("Considered by administrators as not an FOI request and hidden from site.")
+        descriptions = {
+            'waiting_classification'        => _("Awaiting classification."),
+            'waiting_response'              => _("Awaiting response."),
+            'waiting_response_overdue'      => _("Delayed."),
+            'waiting_response_very_overdue' => _("Long overdue."),
+            'not_held'                      => _("Information not held."),
+            'rejected'                      => _("Refused."),
+            'partially_successful'          => _("Partially successful."),
+            'successful'                    => _("Successful."),
+            'waiting_clarification'         => _("Waiting clarification."),
+            'gone_postal'                   => _("Handled by post."),
+            'internal_review'               => _("Awaiting internal review."),
+            'error_message'                 => _("Delivery error"),
+            'requires_admin'                => _("Unusual response."),
+            'attention_requested'           => _("Reported for administrator attention."),
+            'user_withdrawn'                => _("Withdrawn by the requester."),
+            'vexatious'                     => _("Considered by administrators as vexatious and hidden from site."),
+            'not_foi'                       => _("Considered by administrators as not an FOI request and hidden from site."),
+        }
+        if descriptions[status]
+            descriptions[status]
+        elsif respond_to?(:theme_display_status)
+            theme_display_status(status)
         else
-            begin
-                return self.theme_display_status(status)
-            rescue NoMethodError
-                raise _("unknown status ") + status
-            end
+            raise _("unknown status ") + status
         end
     end
 
@@ -968,13 +977,8 @@ public
     end
 
     def is_old_unclassified?
-        return false if is_external?
-        return false if !awaiting_description
-        return false if url_title == 'holding_pen'
-        last_response_event = get_last_response_event
-        return false unless last_response_event
-        return false if last_response_event.created_at >= Time.now - OLD_AGE_IN_DAYS
-        return true
+        !is_external? && awaiting_description && url_title != 'holding_pen' && get_last_response_event &&
+            Time.now > get_last_response_event.created_at + OLD_AGE_IN_DAYS
     end
 
     # List of incoming messages to followup, by unique email