1 files changed, 37 insertions, 29 deletions

diff --git a/app/models/comment.rb b/app/models/comment.rb
index a62c086d5..cc8d0e94b 100644
--- a/app/models/comment.rb
+++ b/app/models/comment.rb
@@ -28,15 +28,32 @@ class Comment < ActiveRecord::Base
 
     #validates_presence_of :user # breaks during construction of new ones :(
     validates_inclusion_of :comment_type, :in => [ 'request' ]
-    validate :body_of_comment
+    validate :check_body_has_content,
+             :check_body_uses_mixed_capitals
+
+    after_save :event_xapian_update
+
+    # When posting a new comment, use this to check user hasn't double
+    # submitted.
+    def self.find_existing(info_request_id, body)
+        # TODO: can add other databases here which have regexp_replace
+        if ActiveRecord::Base.connection.adapter_name == "PostgreSQL"
+            # Exclude spaces from the body comparison using regexp_replace
+            regex_replace_sql = "regexp_replace(body, '[[:space:]]', '', 'g') = regexp_replace(?, '[[:space:]]', '', 'g')"
+            Comment.where(["info_request_id = ? AND #{ regex_replace_sql }", info_request_id, body ]).first
+        else
+            # For other databases (e.g. SQLite) not the end of the world being
+            # space-sensitive for this check
+            Comment.where(:info_request_id => info_request_id, :body => body).first
+        end
+    end
 
     def body
         ret = read_attribute(:body)
-        if ret.nil?
-            return ret
-        end
+        return ret if ret.nil?
         ret = ret.strip
-        ret = ret.gsub(/(?:\n\s*){2,}/, "\n\n") # remove excess linebreaks that unnecessarily space it out
+        # remove excess linebreaks that unnecessarily space it out
+        ret = ret.gsub(/(?:\n\s*){2,}/, "\n\n")
         ret
     end
 
@@ -45,48 +62,39 @@ class Comment < ActiveRecord::Base
     end
 
     # So when takes changes it updates, or when made invisble it vanishes
-    after_save :event_xapian_update
     def event_xapian_update
-        for event in self.info_request_events
-            event.xapian_mark_needs_index
-        end
+        info_request_events.each { |event| event.xapian_mark_needs_index }
     end
 
     # Return body for display as HTML
     def get_body_for_html_display
-        text = self.body.strip
+        text = body.strip
         text = CGI.escapeHTML(text)
         text = MySociety::Format.make_clickable(text, :contract => 1)
         text = text.gsub(/\n/, '<br>')
-        return text.html_safe
-    end
-
-    # When posting a new comment, use this to check user hasn't double submitted.
-    def Comment.find_existing(info_request_id, body)
-        # TODO: can add other databases here which have regexp_replace
-        if ActiveRecord::Base.connection.adapter_name == "PostgreSQL"
-            # Exclude spaces from the body comparison using regexp_replace
-            return Comment.find(:first, :conditions => [ "info_request_id = ? and regexp_replace(body, '[[:space:]]', '', 'g') = regexp_replace(?, '[[:space:]]', '', 'g')", info_request_id, body ])
-        else
-            # For other databases (e.g. SQLite) not the end of the world being space-sensitive for this check
-            return Comment.find(:first, :conditions => [ "info_request_id = ? and body = ?", info_request_id, body ])
-        end
+        text.html_safe
     end
 
     def for_admin_column
         self.class.content_columns.each do |column|
-            yield(column.human_name, self.send(column.name), column.type.to_s, column.name)
+            yield(column.human_name, send(column.name), column.type.to_s, column.name)
         end
     end
 
-  private
+    private
 
-    def body_of_comment
-        if self.body.empty? || self.body =~ /^\s+$/
+    def check_body_has_content
+        if body.empty? || body =~ /^\s+$/
             errors.add(:body, _("Please enter your annotation"))
         end
-        if !MySociety::Validate.uses_mixed_capitals(self.body)
-            errors.add(:body, _('Please write your annotation using a mixture of capital and lower case letters. This makes it easier for others to read.'))
+    end
+
+    def check_body_uses_mixed_capitals
+        unless MySociety::Validate.uses_mixed_capitals(body)
+            msg = _('Please write your annotation using a mixture of capital and ' \
+                    'lower case letters. This makes it easier for others to read.')
+            errors.add(:body, msg)
         end
     end
+
 end