Merge pull request #300 from goshakkk/cleanup

Minor cleanup, using AR querying DSL over raw SQL in some places

app/models/category.rb

@@ -1,6 +1,5 @@
 class Category < ActiveRecord::Base
-
+  belongs_to :topic, dependent: :destroy
-  belongs_to :topic
   belongs_to :user
 
   has_many :topics
@@ -18,15 +17,13 @@ class Category < ActiveRecord::Base
   after_save :invalidate_site_cache
   after_destroy :invalidate_site_cache
 
+  scope :popular, lambda { order('topic_count desc') }
+
   def uncategorized_validator
     return errors.add(:name, I18n.t(:is_reserved)) if name == SiteSetting.uncategorized_name
     return errors.add(:slug, I18n.t(:is_reserved)) if slug == SiteSetting.uncategorized_name
   end
 
-  def self.popular
-    order('topic_count desc')
-  end
-
   # Recalculates `topics_year`, `topics_month`, and `topics_week`
   # for each Category.
   def self.update_stats
@@ -69,9 +66,4 @@ class Category < ActiveRecord::Base
   def invalidate_site_cache
     Site.invalidate_cache
   end
-
-  before_destroy do
-    topic.destroy
-  end
-
 end

app/models/category_featured_topic.rb

@@ -4,19 +4,16 @@ class CategoryFeaturedTopic < ActiveRecord::Base
 
   # Populates the category featured topics
   def self.feature_topics
-
     transaction do
       Category.all.each do |c|
         feature_topics_for(c)
         CategoryFeaturedUser.feature_users_in(c)
       end
     end
-
-    nil
   end
 
   def self.feature_topics_for(c)
-    return unless c.present?
+    return if c.blank?
 
     CategoryFeaturedTopic.transaction do
       exec_sql "DELETE FROM category_featured_topics WHERE category_id = :category_id", category_id: c.id

app/models/category_featured_user.rb

@@ -20,7 +20,7 @@ class CategoryFeaturedUser < ActiveRecord::Base
     ", category_id: category.id, max_featured_users: max_featured_users
 
     transaction do
-      CategoryFeaturedUser.delete_all ['category_id = ?', category.id]
+      CategoryFeaturedUser.delete_all category_id: category.id
       user_counts.each do |uc|
         create(category_id: category.id, user_id: uc['user_id'])
       end

app/models/category_list.rb

@@ -47,25 +47,22 @@ class CategoryList
 
     # Remove categories with no featured topics unless we have the ability to edit one
     unless Guardian.new(current_user).can_create?(Category)
-      @categories.delete_if {|c| c.featured_topics.blank? }
+      @categories.delete_if { |c| c.featured_topics.blank? }
     end
 
     # Get forum topic user records if appropriate
     if current_user.present?
       topics = []
-      @categories.each {|c| topics << c.featured_topics}
+      @categories.each { |c| topics << c.featured_topics }
       topics << @uncategorized
 
       topics.flatten! if topics.present?
       topics.compact! if topics.present?
 
-
       topic_lookup = TopicUser.lookup_for(current_user, topics)
 
       # Attach some data for serialization to each topic
-      topics.each {|ft| ft.user_data = topic_lookup[ft.id]}
+      topics.each { |ft| ft.user_data = topic_lookup[ft.id] }
     end
-
   end
-
 end

app/models/draft.rb

@@ -1,5 +1,4 @@
 class Draft < ActiveRecord::Base
-
   NEW_TOPIC = 'new_topic'
   NEW_PRIVATE_MESSAGE = 'new_private_message'
   EXISTING_TOPIC = 'topic_'
@@ -20,8 +19,6 @@ class Draft < ActiveRecord::Base
     d = find_draft(user,key)
     if d && d.sequence == sequence
       d.data
-    else
-      nil
     end
   end
 
@@ -29,8 +26,6 @@ class Draft < ActiveRecord::Base
     d = find_draft(user,key)
     if d && d.sequence <= sequence
       d.destroy
-    else
-      nil
     end
   end
 
@@ -41,5 +36,4 @@ class Draft < ActiveRecord::Base
     user_id = user.id if User === user
     Draft.where(user_id: user_id, draft_key: key).first
   end
-
 end

app/models/draft_sequence.rb

@@ -2,7 +2,7 @@ class DraftSequence < ActiveRecord::Base
   def self.next!(user,key)
     user_id = user
     user_id = user.id unless user.class == Fixnum
-    h = {user_id: user_id, draft_key: key}
+    h = { user_id: user_id, draft_key: key }
     c = DraftSequence.where(h).first
     c ||= DraftSequence.new(h)
     c.sequence ||= 0
@@ -20,10 +20,6 @@ class DraftSequence < ActiveRecord::Base
     # perf critical path
     r = exec_sql('select sequence from draft_sequences where user_id = ? and draft_key = ?', user_id, key).values
 
-    if r.length == 0
+    r.length.zero? ? 0 : r[0][0].to_i
-      0
-    else
-      r[0][0].to_i
-    end
   end
 end

app/models/email_log.rb

@@ -1,12 +1,10 @@
 class EmailLog < ActiveRecord::Base
-
   belongs_to :user
   validates_presence_of :email_type
   validates_presence_of :to_address
 
   after_create do
     # Update last_emailed_at if the user_id is present
-    User.update_all("last_emailed_at = CURRENT_TIMESTAMP", ["id = ?", user_id]) if user_id.present?
+    User.update_all("last_emailed_at = CURRENT_TIMESTAMP", id: user_id) if user_id.present?
   end
-
 end

app/models/email_token.rb

@@ -5,7 +5,7 @@ class EmailToken < ActiveRecord::Base
   validates_presence_of :user_id
   validates_presence_of :email
 
-  before_validation(:on => :create) do
+  before_validation(on: :create) do
     self.token = EmailToken.generate_token
   end
 
@@ -38,14 +38,13 @@ class EmailToken < ActiveRecord::Base
     return unless token.present?
     return unless token.length/2 == EmailToken.token_length
 
-    email_token = EmailToken.where("token = ? AND expired = FALSE and created_at >= ?", token, EmailToken.valid_after).includes(:user).first
+    email_token = EmailToken.where("token = ? and expired = FALSE and created_at >= ?", token, EmailToken.valid_after).includes(:user).first
     return if email_token.blank?
 
     user = email_token.user
     User.transaction do
-      row_count = EmailToken.update_all 'confirmed = true', ['id = ? AND confirmed = false', email_token.id]
+      row_count = EmailToken.update_all 'confirmed = true', id: email_token.id, expired: false
       if row_count == 1
-
         # If we are activating the user, send the welcome message
         user.send_welcome_message = !user.active?
 
@@ -57,7 +56,5 @@ class EmailToken < ActiveRecord::Base
     user
   rescue ActiveRecord::RecordInvalid
     # If the user's email is already taken, just return nil (failure)
-    nil
   end
-
 end

app/models/error_log.rb

@@ -2,7 +2,6 @@
 # a mechanism to iterate through errors in reverse
 # async logging should queue, if dupe stack traces are found in batch error should be merged into prev one
 
-
 class ErrorLog
 
   @lock = Mutex.new
@@ -12,7 +11,7 @@ class ErrorLog
   end
 
   def self.clear!(guid)
-    raise "not implemented"
+    raise NotImplementedError
   end
 
   def self.clear_all!()
@@ -21,22 +20,22 @@ class ErrorLog
 
   def self.report_async!(exception, controller, request, user)
     Thread.new do
-      self.report!(exception, controller, request, user)
+      report!(exception, controller, request, user)
     end
   end
 
   def self.report!(exception, controller, request, user)
     add_row!(
-      :date => DateTime.now,
+      date: DateTime.now,
-      :guid => SecureRandom.uuid,
+      guid: SecureRandom.uuid,
-      :user_id => user && user.id,
+      user_id: user && user.id,
-      :request => filter_sensitive_post_data_parameters(controller, request.parameters).inspect,
+      request: filter_sensitive_post_data_parameters(controller, request.parameters).inspect,
-      :action => controller.action_name,
+      action: controller.action_name,
-      :controller => controller.controller_name,
+      controller: controller.controller_name,
-      :backtrace => sanitize_backtrace(exception.backtrace).join("\n"),
+      backtrace: sanitize_backtrace(exception.backtrace).join("\n"),
-      :message => exception.message,
+      message: exception.message,
-      :url => "#{request.protocol}#{request.env["HTTP_X_FORWARDED_HOST"] || request.env["HTTP_HOST"]}#{request.fullpath}",
+      url: "#{request.protocol}#{request.env["HTTP_X_FORWARDED_HOST"] || request.env["HTTP_HOST"]}#{request.fullpath}",
-      :exception_class => exception.class.to_s
+      exception_class: exception.class.to_s
     )
   end
 
@@ -44,7 +43,7 @@ class ErrorLog
     data = hash.to_xml(skip_instruct: true)
     # use background thread to write the log cause it may block if it gets backed up
     @lock.synchronize do
-      File.open(self.filename, "a") do |f|
+      File.open(filename, "a") do |f|
         f.flock(File::LOCK_EX)
         f.write(data)
         f.close
@@ -54,13 +53,12 @@ class ErrorLog
 
 
   def self.each(&blk)
-    skip(0,&blk)
+    skip(0, &blk)
   end
 
   def self.skip(skip=0)
-    data = nil
     pos = 0
-    return [] unless File.exists?(self.filename)
+    return [] unless File.exists?(filename)
 
     loop do
       lines = ""
@@ -107,5 +105,4 @@ class ErrorLog
     return PARAM_FILTER_REPLACEMENT if (env_key =~ /RAW_POST_DATA/i)
     return controller.__send__(:filter_parameters, {env_key => env_value}).values[0]
   end
-
 end

app/models/incoming_link.rb

@@ -1,13 +1,12 @@
 class IncomingLink < ActiveRecord::Base
   belongs_to :topic
 
-  validates :domain, :length => { :in => 1..100 }
+  validates :domain, length: { in: 1..100 }
-  validates :referer, :length => { :in => 3..1000 }
+  validates :referer, length: { in: 3..1000 }
   validates_presence_of :url
 
   # Extract the domain
   before_validation do
-
     # Referer (remote URL)
     if referer.present?
       parsed = URI.parse(referer)
@@ -27,7 +26,6 @@ class IncomingLink < ActiveRecord::Base
         # If we can't route to the url, that's OK. Don't save those two fields.
       end
     end
-
   end
 
   # Update appropriate incoming link counts
@@ -43,5 +41,4 @@ class IncomingLink < ActiveRecord::Base
       end
     end
   end
-
 end

app/models/invite.rb

@@ -2,7 +2,7 @@ class Invite < ActiveRecord::Base
 
   belongs_to :user
   belongs_to :topic
-  belongs_to :invited_by, class_name: User
+  belongs_to :invited_by, class_name: 'User'
 
   has_many :topic_invites
   has_many :topics, through: :topic_invites, source: :topic
@@ -17,7 +17,7 @@ class Invite < ActiveRecord::Base
   end
 
   before_save do
-    self.email = self.email.downcase
+    self.email = email.downcase
   end
 
   validate :user_doesnt_already_exist
@@ -45,13 +45,13 @@ class Invite < ActiveRecord::Base
     Invite.transaction do
       # Avoid a race condition
       row_count = Invite.update_all('redeemed_at = CURRENT_TIMESTAMP',
-                                    ['id = ? AND redeemed_at IS NULL AND created_at >= ?', self.id, SiteSetting.invite_expiry_days.days.ago])
+                                    ['id = ? AND redeemed_at IS NULL AND created_at >= ?', id, SiteSetting.invite_expiry_days.days.ago])
 
       if row_count == 1
 
         # Create the user if we are redeeming the invite and the user doesn't exist
         result = User.where(email: email).first
-        result = User.create_for_email(email, trust_level: SiteSetting.default_invitee_trust_level) if result.blank?
+        result ||= User.create_for_email(email, trust_level: SiteSetting.default_invitee_trust_level)
         result.send_welcome_message = false
 
         # If there are topic invites for private topics
@@ -61,19 +61,19 @@ class Invite < ActiveRecord::Base
 
         # Check for other invites by the same email. Don't redeem them, but approve their
         # topics.
-        Invite.where('invites.email = ? and invites.id != ?', self.email, self.id).includes(:topics).where('topics.archetype = ?', Archetype::private_message).each do |i|
+        Invite.where('invites.email = ? and invites.id != ?', email, id).includes(:topics).where(topics: { archetype: Archetype::private_message }).each do |i|
           i.topics.each do |t|
             t.topic_allowed_users.create(user_id: result.id)
           end
         end
 
-        if Invite.update_all(['user_id = ?', result.id], ['email = ?', self.email]) == 1
+        if Invite.update_all(['user_id = ?', result.id], ['email = ?', email]) == 1
           result.send_welcome_message = true
         end
 
           # Notify the invitee
           invited_by.notifications.create(notification_type: Notification.Types[:invitee_accepted],
-                                          data: {display_username: result.username}.to_json)
+                                          data: { display_username: result.username }.to_json)
 
       else
         # Otherwise return the existing user

app/models/notification.rb

@@ -6,6 +6,9 @@ class Notification < ActiveRecord::Base
   validates_presence_of :data
   validates_presence_of :notification_type
 
+  scope :unread, lambda { where(read: false) }
+  scope :recent, lambda { order('created_at desc').limit(10) }
+
   def self.Types
     {:mentioned => 1,
      :replied => 2,
@@ -23,16 +26,8 @@ class Notification < ActiveRecord::Base
     @inverted_types ||= Notification.Types.invert
   end
 
-  def self.unread
-    where(read: false)
-  end
-
   def self.mark_posts_read(user, topic_id, post_numbers)
-    Notification.update_all "read = 't'", ["user_id = ? and topic_id = ? and post_number in (?) and read = ?", user.id, topic_id, post_numbers, false]
+    Notification.update_all "read = 't'", user_id: user.id, topic_id: topic_id, post_number: post_numbers, read: false
-  end
-
-  def self.recent
-    order('created_at desc').limit(10)
   end
 
   def self.interesting_after(min_date)
@@ -68,7 +63,7 @@ class Notification < ActiveRecord::Base
   def data_hash
     @data_hash ||= begin
       return nil if data.blank?
-      ::JSON.parse(data).with_indifferent_access
+      JSON.parse(data).with_indifferent_access
     end
   end
 
@@ -81,15 +76,12 @@ class Notification < ActiveRecord::Base
     if topic.present?
       return topic.relative_url(post_number)
     end
-    nil
   end
 
   def post
-    return nil unless topic_id.present?
+    return if topic_id.blank? || post_number.blank?
-    return nil unless post_number.present?
 
     Post.where(topic_id: topic_id, post_number: post_number).first
   end
-
 end
 

app/models/onebox_render.rb

@@ -1,10 +1,8 @@
 class OneboxRender < ActiveRecord::Base
-
   validates_presence_of :url
   validates_presence_of :cooked
   validates_presence_of :expires_at
 
-  has_many :post_onebox_renders, :dependent => :delete_all
+  has_many :post_onebox_renders, dependent: :delete_all
   has_many :posts, through: :post_onebox_renders
-
 end

app/models/post.rb

@@ -29,7 +29,7 @@ class Post < ActiveRecord::Base
   has_many :post_actions
 
   validates_presence_of :raw, :user_id, :topic_id
-  validates :raw, stripped_length: {in: SiteSetting.min_post_length..SiteSetting.max_post_length}
+  validates :raw, stripped_length: { in: SiteSetting.post_length }
   validate :raw_quality
   validate :max_mention_validator
   validate :max_images_validator
@@ -54,15 +54,14 @@ class Post < ActiveRecord::Base
   after_commit :store_unique_post_key, on: :create
 
   after_create do
-    TopicUser.auto_track(self.user_id, self.topic_id, TopicUser::NotificationReasons::CREATED_POST)
+    TopicUser.auto_track(user_id, topic_id, TopicUser::NotificationReasons::CREATED_POST)
   end
 
   scope :by_newest, order('created_at desc, id desc')
   scope :with_user, includes(:user)
 
   def raw_quality
-
+    sentinel = TextSentinel.new(raw, min_entropy: SiteSetting.body_min_entropy)
-    sentinel = TextSentinel.new(self.raw, min_entropy: SiteSetting.body_min_entropy)
     if sentinel.valid?
       # It's possible the sentinel has cleaned up the title a bit
       self.raw = sentinel.text
@@ -75,7 +74,7 @@ class Post < ActiveRecord::Base
   # Stop us from posting the same thing too quickly
   def unique_post_validator
     return if SiteSetting.unique_posts_mins == 0
-    return if user.admin? or user.has_trust_level?(:moderator)
+    return if user.admin? || user.moderator?
 
     # If the post is empty, default to the validates_presence_of
     return if raw.blank?
@@ -97,13 +96,13 @@ class Post < ActiveRecord::Base
   end
 
   def raw_hash
-    return nil if raw.blank?
+    return if raw.blank?
     Digest::SHA1.hexdigest(raw.gsub(/\s+/, "").downcase)
   end
 
   def cooked_document
-    self.cooked ||= cook(self.raw, topic_id: topic_id)
+    self.cooked ||= cook(raw, topic_id: topic_id)
-    @cooked_document ||= Nokogiri::HTML.fragment(self.cooked)
+    @cooked_document ||= Nokogiri::HTML.fragment(cooked)
   end
 
   def reset_cooked
@@ -116,20 +115,18 @@ class Post < ActiveRecord::Base
   end
 
   def image_count
-    return 0 unless self.raw.present?
+    return 0 unless raw.present?
 
-    cooked_document.search("img").reject{ |t|
+    cooked_document.search("img").reject do |t|
       dom_class = t["class"]
       if dom_class
         (Post.white_listed_image_classes & dom_class.split(" ")).count > 0
-      else
-        false
       end
-    }.count
+    end.count
   end
 
   def link_count
-    return 0 unless self.raw.present?
+    return 0 unless raw.present?
     cooked_document.search("a[href]").count
   end
 
@@ -138,12 +135,12 @@ class Post < ActiveRecord::Base
   end
 
   def max_images_validator
-    return if user.present? and user.has_trust_level?(:basic)
+    return if user.present? && user.has_trust_level?(:basic)
     errors.add(:raw, I18n.t(:too_many_images)) if image_count > 0
   end
 
   def max_links_validator
-    return if user.present? and user.has_trust_level?(:basic)
+    return if user.present? && user.has_trust_level?(:basic)
     errors.add(:raw, I18n.t(:too_many_links)) if link_count > 1
   end
 
@@ -161,22 +158,18 @@ class Post < ActiveRecord::Base
     doc.search("code").remove
 
     results = doc.to_html.scan(PrettyText.mention_matcher)
-    if results.present?
+    @raw_mentions = results.uniq.map { |un| un.first.downcase.gsub!(/^@/, '') }
-      @raw_mentions = results.uniq.map {|un| un.first.downcase.gsub!(/^@/, '')}
-    else
-      @raw_mentions = []
-    end
   end
 
   # The rules for deletion change depending on who is doing it.
   def delete_by(deleted_by)
-    if deleted_by.has_trust_level?(:moderator)
+    if deleted_by.moderator?
       # As a moderator, delete the post.
       Post.transaction do
         self.destroy
-        Topic.reset_highest(self.topic_id)
+        Topic.reset_highest(topic_id)
       end
-    elsif deleted_by.id == self.user_id
+    elsif deleted_by.id == user_id
       # As the poster, make a revision that says deleted.
       Post.transaction do
         revise(deleted_by, I18n.t('js.post.deleted_by_author'), force_new_version: true)
@@ -205,7 +198,7 @@ class Post < ActiveRecord::Base
     PostAction.update_flagged_posts_count
   end
 
-  def filter_quotes(parent_post=nil)
+  def filter_quotes(parent_post = nil)
     return cooked if parent_post.blank?
 
     # We only filter quotes when there is exactly 1
@@ -229,31 +222,30 @@ class Post < ActiveRecord::Base
   end
 
   def quoteless?
-    (quote_count == 0) and (reply_to_post_number.present?)
+    (quote_count == 0) && (reply_to_post_number.present?)
   end
 
   # Get the post that we reply to.
   def reply_to_user
-    return nil unless reply_to_post_number.present?
+    return if reply_to_post_number.blank?
-    User.where('id = (select user_id from posts where topic_id = ? and post_number = ?)', topic_id, reply_to_post_number).first
+    Post.where(topic_id: topic_id, post_number: reply_to_post_number).first.try(:user)
   end
 
   def reply_notification_target
-    return nil unless reply_to_post_number.present?
+    return if reply_to_post_number.blank?
-    reply_post = Post.where("topic_id = :topic_id AND post_number = :post_number AND user_id <> :user_id",
+    Post.where("topic_id = :topic_id AND post_number = :post_number AND user_id <> :user_id",
                 topic_id: topic_id,
                 post_number: reply_to_post_number,
-                            user_id: user_id).first
+                user_id: user_id).first.try(:user)
-    return reply_post.try(:user)
   end
 
-  def self.excerpt(cooked, maxlength=nil)
+  def self.excerpt(cooked, maxlength = nil)
     maxlength ||= SiteSetting.post_excerpt_maxlength
     PrettyText.excerpt(cooked, maxlength)
   end
 
   # Strip out most of the markup
-  def excerpt(maxlength=nil)
+  def excerpt(maxlength = nil)
     Post.excerpt(cooked, maxlength)
   end
 
@@ -279,22 +271,22 @@ class Post < ActiveRecord::Base
   # A list of versions including the initial version
   def all_versions
     result = []
-    result << {number: 1, display_username: user.name, created_at: created_at}
+    result << { number: 1, display_username: user.name, created_at: created_at }
     versions.order(:number).includes(:user).each do |v|
-      result << {number: v.number, display_username: v.user.name, created_at: v.created_at}
+      result << { number: v.number, display_username: v.user.name, created_at: v.created_at }
     end
     result
   end
 
   def is_flagged?
-    post_actions.where('post_action_type_id in (?) and deleted_at is null', PostActionType.FlagTypes).count != 0
+    post_actions.where(post_action_type_id: PostActionType.FlagTypes, deleted_at: nil).count != 0
   end
 
   def unhide!
     self.hidden = false
     self.hidden_reason_id = nil
     self.topic.update_attributes(visible: true)
-    self.save
+    save
   end
 
   def url
@@ -305,7 +297,7 @@ class Post < ActiveRecord::Base
     user.readable_name
   end
 
-  def revise(updated_by, new_raw, opts={})
+  def revise(updated_by, new_raw, opts = {})
     PostRevisor.new(self).revise!(updated_by, new_raw, opts)
   end
 
@@ -320,21 +312,20 @@ class Post < ActiveRecord::Base
 
   # TODO: Move some of this into an asynchronous job?
   after_create do
-
     # Update attributes on the topic - featured users and last posted.
-    attrs = {last_posted_at: self.created_at, last_post_user_id: self.user_id}
+    attrs = {last_posted_at: created_at, last_post_user_id: user_id}
-    attrs[:bumped_at] = self.created_at unless no_bump
+    attrs[:bumped_at] = created_at unless no_bump
     topic.update_attributes(attrs)
 
     # Update the user's last posted at date
-    user.update_column(:last_posted_at, self.created_at)
+    user.update_column(:last_posted_at, created_at)
 
     # Update topic user data
     TopicUser.change(user,
                      topic.id,
                      posted: true,
-                           last_read_post_number: self.post_number,
+                     last_read_post_number: post_number,
-                           seen_post_count: self.post_number)
+                     seen_post_count: post_number)
   end
 
   def email_private_message
@@ -371,57 +362,54 @@ class Post < ActiveRecord::Base
   end
 
   before_save do
-    self.last_editor_id ||= self.user_id
+    self.last_editor_id ||= user_id
     self.cooked = cook(raw, topic_id: topic_id) unless new_record?
   end
 
   before_destroy do
 
     # Update the last post id to the previous post if it exists
-    last_post = Post.where("topic_id = ? and id <> ?", self.topic_id, self.id).order('created_at desc').limit(1).first
+    last_post = Post.where("topic_id = ? and id <> ?", topic_id, id).order('created_at desc').limit(1).first
     if last_post.present?
       topic.update_attributes(last_posted_at: last_post.created_at,
                               last_post_user_id: last_post.user_id,
                               highest_post_number: last_post.post_number)
 
       # If the poster doesn't have any other posts in the topic, clear their posted flag
-      unless Post.exists?(["topic_id = ? and user_id = ? and id <> ?", self.topic_id, self.user_id, self.id])
+      unless Post.exists?(["topic_id = ? and user_id = ? and id <> ?", topic_id, user_id, id])
-        TopicUser.update_all 'posted = false', ['topic_id = ? and user_id = ?', self.topic_id, self.user_id]
+        TopicUser.update_all 'posted = false', topic_id: topic_id, user_id: user_id
       end
     end
 
     # Feature users in the topic
-    Jobs.enqueue(:feature_topic_users, topic_id: topic_id, except_post_id: self.id)
+    Jobs.enqueue(:feature_topic_users, topic_id: topic_id, except_post_id: id)
 
   end
 
   after_destroy do
-
     # Remove any reply records that point to deleted posts
-    post_ids = PostReply.select(:post_id).where(reply_id: self.id).map(&:post_id)
+    post_ids = PostReply.select(:post_id).where(reply_id: id).map(&:post_id)
-    PostReply.delete_all ["reply_id = ?", self.id]
+    PostReply.delete_all reply_id: id
 
     if post_ids.present?
-      Post.where(id: post_ids).each {|p| p.update_column :reply_count, p.replies.count}
+      Post.where(id: post_ids).each { |p| p.update_column :reply_count, p.replies.count }
     end
 
     # Remove any notifications that point to this deleted post
-    Notification.delete_all ["topic_id = ? and post_number = ?", self.topic_id, self.post_number]
+    Notification.delete_all topic_id: topic_id, post_number: post_number
   end
 
   after_save do
-
+    DraftSequence.next! last_editor_id, topic.draft_key if topic # could be deleted
-    DraftSequence.next! self.last_editor_id, self.topic.draft_key if self.topic # could be deleted
 
     quoted_post_numbers << reply_to_post_number if reply_to_post_number.present?
 
     # Create a reply relationship between quoted posts and this new post
-    if self.quoted_post_numbers.present?
+    if quoted_post_numbers.present?
-      self.quoted_post_numbers.map! {|pid| pid.to_i}.uniq!
+      quoted_post_numbers.map(&:to_i).uniq.each do |p|
-      self.quoted_post_numbers.each do |p|
         post = Post.where(topic_id: topic_id, post_number: p).first
         if post.present?
-          post_reply = post.post_replies.new(reply_id: self.id)
+          post_reply = post.post_replies.new(reply_id: id)
           if post_reply.save
             Post.update_all ['reply_count = reply_count + 1'], id: post.id
           end
@@ -443,7 +431,7 @@ class Post < ActiveRecord::Base
 
         if args[:topic].present?
           # If the topic attribute is present, ensure it's the same topic
-          self.quoted_post_numbers << args[:post] if self.topic_id == args[:topic]
+          self.quoted_post_numbers << args[:post] if topic_id == args[:topic]
         else
           self.quoted_post_numbers << args[:post]
         end
@@ -452,15 +440,14 @@ class Post < ActiveRecord::Base
     end
 
     self.quoted_post_numbers.uniq!
-    self.quote_count = self.quoted_post_numbers.size
+    self.quote_count = quoted_post_numbers.size
   end
 
   # Process this post after comitting it
   def trigger_post_process
-    args = {post_id: self.id}
+    args = { post_id: id }
-    args[:image_sizes] = self.image_sizes if self.image_sizes.present?
+    args[:image_sizes] = image_sizes if image_sizes.present?
-    args[:invalidate_oneboxes] = true if self.invalidate_oneboxes.present?
+    args[:invalidate_oneboxes] = true if invalidate_oneboxes.present?
     Jobs.enqueue(:process_post, args)
   end
-
 end

app/models/post_action.rb

@@ -19,7 +19,6 @@ class PostAction < ActiveRecord::Base
   validate :message_quality
 
   def self.update_flagged_posts_count
-
     posts_flagged_count = PostAction.joins(post: :topic)
                                     .where('post_actions.post_action_type_id' => PostActionType.FlagTypes,
                                            'posts.deleted_at' => nil,
@@ -27,7 +26,7 @@ class PostAction < ActiveRecord::Base
 
     $redis.set('posts_flagged_count', posts_flagged_count)
     admins = User.where(admin: true).select(:id).map {|u| u.id}
-    MessageBus.publish('/flagged_counts', {total: posts_flagged_count}, {user_ids: admins})
+    MessageBus.publish('/flagged_counts', { total: posts_flagged_count }, { user_ids: admins })
   end
 
   def self.flagged_posts_count
@@ -35,7 +34,6 @@ class PostAction < ActiveRecord::Base
   end
 
   def self.counts_for(collection, user)
-
   	return {} if collection.blank?
 
     collection_ids = collection.map {|p| p.id}
@@ -53,7 +51,6 @@ class PostAction < ActiveRecord::Base
   end
 
   def self.clear_flags!(post, moderator_id, action_type_id = nil)
-
     # -1 is the automatic system cleary
     actions = if action_type_id
       [action_type_id]
@@ -61,10 +58,10 @@ class PostAction < ActiveRecord::Base
       moderator_id == -1 ? PostActionType.AutoActionFlagTypes : PostActionType.FlagTypes
     end
 
-    PostAction.update_all({deleted_at: Time.now, deleted_by: moderator_id}, {post_id: post.id, post_action_type_id: actions})
+    PostAction.update_all({ deleted_at: Time.now, deleted_by: moderator_id }, { post_id: post.id, post_action_type_id: actions })
 
     r = PostActionType.Types.invert
-    f = actions.map{|t| ["#{r[t]}_count", 0]}
+    f = actions.map { |t| ["#{r[t]}_count", 0] }
 
     Post.with_deleted.update_all(Hash[*f.flatten], id: post.id)
 
@@ -82,7 +79,7 @@ class PostAction < ActiveRecord::Base
   end
 
   def self.remove_act(user, post, post_action_type_id)
-    if action = self.where(post_id: post.id, user_id: user.id, post_action_type_id: post_action_type_id).first
+    if action = where(post_id: post.id, user_id: user.id, post_action_type_id: post_action_type_id).first
       action.destroy
       action.deleted_at = Time.now
       action.run_callbacks(:save)
@@ -103,7 +100,7 @@ class PostAction < ActiveRecord::Base
 
   # A custom rate limiter for this model
   def post_action_rate_limiter
-    return nil unless is_flag? or is_bookmark? or is_like?
+    return unless is_flag? || is_bookmark? || is_like?
 
     return @rate_limiter if @rate_limiter.present?
 
@@ -127,7 +124,7 @@ class PostAction < ActiveRecord::Base
   end
 
   before_create do
-    raise AlreadyFlagged if is_flag? and PostAction.where(user_id: user_id,
+    raise AlreadyFlagged if is_flag? && PostAction.where(user_id: user_id,
                                                          post_id: post_id,
                                                          post_action_type_id: PostActionType.FlagTypes).exists?
   end
@@ -135,12 +132,12 @@ class PostAction < ActiveRecord::Base
   after_save do
     # Update denormalized counts
     post_action_type = PostActionType.Types.invert[post_action_type_id]
-    column = "#{post_action_type.to_s}_count"
+    column = "#{post_action_type}_count"
     delta = deleted_at.nil? ? 1 : -1
 
     # Voting also changes the sort_order
     if post_action_type == :vote
-      Post.update_all ["vote_count = vote_count + :delta, sort_order = :max - (vote_count + :delta)", delta: delta, max: Topic::MAX_SORT_ORDER], ["id = ?", post_id]
+      Post.update_all ["vote_count = vote_count + :delta, sort_order = :max - (vote_count + :delta)", delta: delta, max: Topic::MAX_SORT_ORDER], id: post_id
     else
       Post.update_all ["#{column} = #{column} + ?", delta], id: post_id
     end
@@ -162,18 +159,16 @@ class PostAction < ActiveRecord::Base
       if new_flags >= SiteSetting.flags_required_to_hide_post
         reason = old_flags > 0 ? Post::HiddenReason::FLAG_THRESHOLD_REACHED_AGAIN : Post::HiddenReason::FLAG_THRESHOLD_REACHED
         Post.update_all(["hidden = true, hidden_reason_id = COALESCE(hidden_reason_id, ?)", reason], id: post_id)
-        Topic.update_all({visible: false},
+        Topic.update_all({ visible: false },
                          ["id = :topic_id AND NOT EXISTS(SELECT 1 FROM POSTS WHERE topic_id = :topic_id AND NOT hidden)", topic_id: post.topic_id])
 
         # inform user
-        if self.post.user
+        if post.user
-          SystemMessage.create(self.post.user, :post_hidden,
+          SystemMessage.create(post.user, :post_hidden,
-                               url: self.post.url,
+                               url: post.url,
                                edit_delay: SiteSetting.cooldown_minutes_after_hiding_posts)
         end
       end
-
     end
-
   end
 end

app/models/post_action_type.rb

@@ -1,19 +1,19 @@
 class PostActionType < ActiveRecord::Base
-
   attr_accessible :id, :is_flag, :name_key, :icon
 
   def self.ordered
-    self.order('position asc').all
+    order('position asc').all
   end
 
   def self.Types
-    @types ||= {:bookmark => 1,
+    {
-     :like => 2,
+      bookmark: 1,
-     :off_topic => 3,
+      like: 2,
-     :inappropriate => 4,
+      off_topic: 3,
-     :vote => 5,
+      inappropriate: 4,
-     :custom_flag => 6,
+      vote: 5,
-     :spam => 8
+      custom_flag: 6,
+      spam: 8
     }
   end
 
@@ -28,5 +28,4 @@ class PostActionType < ActiveRecord::Base
   def self.FlagTypes
     @flag_types ||= self.AutoActionFlagTypes + [self.Types[:custom_flag]]
   end
-
 end

app/models/post_alert_observer.rb

@@ -25,12 +25,11 @@ class PostAlertObserver < ActiveRecord::Observer
 
   def after_save_post_action(post_action)
     # We only care about deleting post actions for now
-    return unless post_action.deleted_at.present?
+    return if post_action.deleted_at.blank?
-    Notification.where(["post_action_id = ?", post_action.id]).each {|n| n.destroy}
+    Notification.where(post_action_id: post_action.id).each(&:destroy)
   end
 
   def after_create_post_action(post_action)
-
     # We only notify on likes for now
     return unless post_action.is_like?
 
@@ -59,7 +58,7 @@ class PostAlertObserver < ActiveRecord::Observer
   def after_create_post(post)
     if post.topic.private_message?
       # If it's a private message, notify the topic_allowed_users
-      post.topic.topic_allowed_users.reject{|a| a.user_id == post.user_id}.each do |a|
+      post.topic.topic_allowed_users.reject { |a| a.user_id == post.user_id }.each do |a|
         create_notification(a.user, Notification.Types[:private_message], post)
       end
     else
@@ -91,13 +90,13 @@ class PostAlertObserver < ActiveRecord::Observer
                                 topic_id: post.topic_id,
                                 post_number: post.post_number,
                                 post_action_id: opts[:post_action_id],
-                                data: {topic_title: post.topic.title,
+                                data: { topic_title: post.topic.title,
-                                       display_username: opts[:display_username] || post.user.username}.to_json)
+                                        display_username: opts[:display_username] || post.user.username }.to_json)
     end
 
     # Returns a list users who have been mentioned
     def extract_mentioned_users(post)
-      User.where("username_lower in (?)", post.raw_mentions).where("id <> ?", post.user_id)
+      User.where(username_lower: post.raw_mentions).where("id <> ?", post.user_id)
     end
 
     # Returns a list of users who were quoted in the post
@@ -121,24 +120,21 @@ class PostAlertObserver < ActiveRecord::Observer
 
     # TODO: This should use javascript for parsing rather than re-doing it this way.
     def notify_post_users(post)
-
       # Is this post a reply to a user?
       reply_to_user = post.reply_notification_target
       notify_users(reply_to_user, :replied, post)
 
-
       # find all users watching
       if post.post_number > 1
         exclude_user_ids = []
         exclude_user_ids << post.user_id
         exclude_user_ids << reply_to_user.id if reply_to_user.present?
-        exclude_user_ids << extract_mentioned_users(post).map{|u| u.id}
+        exclude_user_ids << extract_mentioned_users(post).map(&:id)
-        exclude_user_ids << extract_quoted_users(post).map{|u| u.id}
+        exclude_user_ids << extract_quoted_users(post).map(&:id)
         exclude_user_ids.flatten!
         TopicUser.where(topic_id: post.topic_id, notification_level: TopicUser::NotificationLevel::WATCHING).includes(:user).each do |tu|
           create_notification(tu.user, Notification.Types[:posted], post) unless exclude_user_ids.include?(tu.user_id)
         end
       end
     end
-
 end

app/models/post_reply.rb

@@ -1,5 +1,4 @@
 class PostReply < ActiveRecord::Base
-
   belongs_to :post
   belongs_to :reply, class_name: 'Post'
 

app/models/post_timing.rb

@@ -1,5 +1,4 @@
 class PostTiming < ActiveRecord::Base
-
   belongs_to :topic
   belongs_to :user
 
@@ -9,7 +8,6 @@ class PostTiming < ActiveRecord::Base
 
   # Increases a timer if a row exists, otherwise create it
   def self.record_timing(args)
-
     rows = exec_sql_row_count("UPDATE post_timings
                                SET msecs = msecs + :msecs
                                WHERE topic_id = :topic_id
@@ -28,7 +26,6 @@ class PostTiming < ActiveRecord::Base
                args)
 
     end
-
   end
 
 
@@ -43,7 +40,6 @@ class PostTiming < ActiveRecord::Base
   def self.process_timings(current_user, topic_id, highest_seen, topic_time, timings)
     current_user.update_time_read!
 
-    original_unread = current_user.unread_notifications_by_type
     timings.each do |post_number, time|
       if post_number >= 0
         PostTiming.record_timing(topic_id: topic_id,
@@ -64,7 +60,5 @@ class PostTiming < ActiveRecord::Base
       current_user.reload
       current_user.publish_notifications_state
     end
-
   end
-
 end

app/models/search_observer.rb

@@ -58,7 +58,6 @@ class SearchObserver < ActiveRecord::Observer
   end
 
 
-
   class HtmlScrubber < Nokogiri::XML::SAX::Document
     attr_reader :scrubbed
 
@@ -67,7 +66,7 @@ class SearchObserver < ActiveRecord::Observer
     end
 
     def self.scrub(html)
-      me = self.new
+      me = new
       parser = Nokogiri::HTML::SAX::Parser.new(me)
       begin
         copy = "<div>"
@@ -98,6 +97,5 @@ class SearchObserver < ActiveRecord::Observer
       scrubbed << " "
     end
   end
-
 end
 

app/models/site.rb

@@ -26,7 +26,7 @@ class Site
   end
 
   def archetypes
-    Archetype.list.reject{|t| t.id==Archetype.private_message}
+    Archetype.list.reject { |t| t.id == Archetype.private_message }
   end
 
   def self.cache_key
@@ -45,5 +45,4 @@ class Site
   def self.invalidate_cache
     Rails.cache.delete(Site.cache_key)
   end
-
 end

app/models/site_customization.rb

@@ -1,5 +1,4 @@
 class SiteCustomization < ActiveRecord::Base
-
   ENABLED_KEY = '7e202ef2-56d7-47d5-98d8-a9c8d15e57dd'
   # placing this in uploads to ease deployment rules
   CACHE_PATH = 'uploads/stylesheet-cache'
@@ -13,9 +12,9 @@ class SiteCustomization < ActiveRecord::Base
   end
 
   before_save do
-    if self.stylesheet_changed?
+    if stylesheet_changed?
       begin
-        self.stylesheet_baked = Sass.compile self.stylesheet
+        self.stylesheet_baked = Sass.compile stylesheet
       rescue Sass::SyntaxError => e
         error = e.sass_backtrace_str("custom stylesheet")
         error.gsub!("\n", '\A ')
@@ -30,23 +29,23 @@ footer:after{ content: '#{error}' }"
   end
 
   after_save do
-    if self.stylesheet_changed?
+    if stylesheet_changed?
-      if File.exists?(self.stylesheet_fullpath)
+      if File.exists?(stylesheet_fullpath)
-        File.delete self.stylesheet_fullpath
+        File.delete stylesheet_fullpath
       end
     end
-    self.remove_from_cache!
+    remove_from_cache!
-    if self.stylesheet_changed?
+    if stylesheet_changed?
-      self.ensure_stylesheet_on_disk!
+      ensure_stylesheet_on_disk!
-      MessageBus.publish "/file-change/#{self.key}", self.stylesheet_hash
+      MessageBus.publish "/file-change/#{key}", stylesheet_hash
     end
-    MessageBus.publish "/header-change/#{self.key}", self.header if self.header_changed?
+    MessageBus.publish "/header-change/#{key}", header if header_changed?
 
   end
 
   after_destroy do
-    if File.exists?(self.stylesheet_fullpath)
+    if File.exists?(stylesheet_fullpath)
-      File.delete self.stylesheet_fullpath
+      File.delete stylesheet_fullpath
     end
     self.remove_from_cache!
   end
@@ -57,18 +56,17 @@ footer:after{ content: '#{error}' }"
 
   def self.enabled_style_key
     @cache ||= {}
-    preview_style = @cache[self.enabled_key]
+    preview_style = @cache[enabled_key]
-    return nil if preview_style == :none
+    return if preview_style == :none
     return preview_style if preview_style
 
     @lock.synchronize do
-      style = self.where(enabled: true).first
+      style = where(enabled: true).first
       if style
-        @cache[self.enabled_key] = style.key
+        @cache[enabled_key] = style.key
-        return style.key
       else
-        @cache[self.enabled_key] = :none
+        @cache[enabled_key] = :none
-        return nil
+        nil
       end
     end
   end
@@ -96,7 +94,6 @@ footer:after{ content: '#{error}' }"
   end
 
   def self.lookup_style(key)
-
     return if key.blank?
 
     # cache is cross site resiliant cause key is secure random
@@ -106,7 +103,7 @@ footer:after{ content: '#{error}' }"
     return style if style
 
     @lock.synchronize do
-      style = self.where(key: key).first
+      style = where(key: key).first
       style.ensure_stylesheet_on_disk! if style
       @cache[key] = style
     end
@@ -124,8 +121,8 @@ footer:after{ content: '#{error}' }"
     end
   end
 
-  def self.remove_from_cache!(key, broadcast=true)
+  def self.remove_from_cache!(key, broadcast = true)
-    MessageBus.publish('/site_customization', {key: key}) if broadcast
+    MessageBus.publish('/site_customization', key: key) if broadcast
     if @cache
       @lock.synchronize do
         @cache[key] = nil
@@ -135,11 +132,11 @@ footer:after{ content: '#{error}' }"
 
   def remove_from_cache!
     self.class.remove_from_cache!(self.class.enabled_key)
-    self.class.remove_from_cache!(self.key)
+    self.class.remove_from_cache!(key)
   end
 
   def stylesheet_hash
-    Digest::MD5.hexdigest(self.stylesheet)
+    Digest::MD5.hexdigest(stylesheet)
   end
 
   def cache_fullpath
@@ -152,7 +149,7 @@ footer:after{ content: '#{error}' }"
     FileUtils.mkdir_p(dir)
     unless File.exists?(path)
       File.open(path, "w") do |f|
-        f.puts self.stylesheet_baked
+        f.puts stylesheet_baked
       end
     end
   end
@@ -162,14 +159,13 @@ footer:after{ content: '#{error}' }"
   end
 
   def stylesheet_fullpath
-    "#{self.cache_fullpath}#{self.stylesheet_filename}"
+    "#{cache_fullpath}#{stylesheet_filename}"
   end
 
   def stylesheet_link_tag
-    return "" unless self.stylesheet.present?
+    return "" unless stylesheet.present?
     return @stylesheet_link_tag if @stylesheet_link_tag
     ensure_stylesheet_on_disk!
-    @stylesheet_link_tag = "<link class=\"custom-css\" rel=\"stylesheet\" href=\"/#{CACHE_PATH}#{self.stylesheet_filename}?#{self.stylesheet_hash}\" type=\"text/css\" media=\"screen\">"
+    @stylesheet_link_tag = "<link class=\"custom-css\" rel=\"stylesheet\" href=\"/#{CACHE_PATH}#{stylesheet_filename}?#{stylesheet_hash}\" type=\"text/css\" media=\"screen\">"
   end
-
 end

app/models/site_setting.rb

@@ -162,4 +162,7 @@ class SiteSetting < ActiveRecord::Base
     min_topic_title_length..max_topic_title_length
   end
 
+  def self.post_length
+    min_post_length..max_post_length
+  end
 end

app/models/topic.rb

@@ -68,28 +68,28 @@ class Topic < ActiveRecord::Base
   end
 
   before_validation do
-    if self.title.present?
+    if title.present?
-      self.title = sanitize(self.title)
+      self.title = sanitize(title)
       self.title.strip!
     end
   end
 
   before_create do
     self.bumped_at ||= Time.now
-    self.last_post_user_id ||= self.user_id
+    self.last_post_user_id ||= user_id
   end
 
   after_create do
     changed_to_category(category)
     TopicUser.change(
-                     self.user_id, self.id,
+                     user_id, id,
                      notification_level: TopicUser::NotificationLevel::WATCHING,
                      notifications_reason_id: TopicUser::NotificationReasons::CREATED_TOPIC
                     )
-    if self.archetype == Archetype.private_message
+    if archetype == Archetype.private_message
-      DraftSequence.next!(self.user, Draft::NEW_PRIVATE_MESSAGE)
+      DraftSequence.next!(user, Draft::NEW_PRIVATE_MESSAGE)
     else
-      DraftSequence.next!(self.user, Draft::NEW_TOPIC)
+      DraftSequence.next!(user, Draft::NEW_TOPIC)
     end
   end
 
@@ -170,7 +170,7 @@ class Topic < ActiveRecord::Base
   end
 
   def meta_data_string(key)
-    return nil unless meta_data.present?
+    return unless meta_data.present?
     meta_data[key.to_s]
   end
 
@@ -199,7 +199,7 @@ class Topic < ActiveRecord::Base
               WHERE ftl.topic_id = ?
               GROUP BY ftl.url, ft.title, ftl.link_topic_id, ftl.reflection, ftl.internal
               ORDER BY clicks DESC",
-              self.id).to_a
+              id).to_a
   end
 
   def update_status(property, status, user)
@@ -218,7 +218,7 @@ class Topic < ActiveRecord::Base
   end
 
   # Atomically creates the next post number
-  def self.next_post_number(topic_id, reply=false)
+  def self.next_post_number(topic_id, reply = false)
     highest = exec_sql("select coalesce(max(post_number),0) as max from posts where topic_id = ?", topic_id).first['max'].to_i
 
     reply_sql = reply ? ", reply_count = reply_count + 1" : ""
@@ -265,7 +265,7 @@ class Topic < ActiveRecord::Base
   def changed_to_category(cat)
 
     return if cat.blank?
-    return if Category.where(topic_id: self.id).first.present?
+    return if Category.where(topic_id: id).first.present?
 
     Topic.transaction do
       old_category = category
@@ -275,10 +275,10 @@ class Topic < ActiveRecord::Base
       end
 
       self.category_id = cat.id
-      self.save
+      save
 
       CategoryFeaturedTopic.feature_topics_for(old_category)
-      Category.update_all 'topic_count = topic_count + 1', ['id = ?', cat.id]
+      Category.update_all 'topic_count = topic_count + 1', id: cat.id
       CategoryFeaturedTopic.feature_topics_for(cat) unless old_category.try(:id) == cat.try(:id)
     end
   end
@@ -310,10 +310,10 @@ class Topic < ActiveRecord::Base
     if name.blank?
       if category_id.present?
         CategoryFeaturedTopic.feature_topics_for(category)
-        Category.update_all 'topic_count = topic_count - 1', ['id = ?', category_id]
+        Category.update_all 'topic_count = topic_count - 1', id: category_id
       end
       self.category_id = nil
-      self.save
+      save
       return
     end
 
@@ -335,10 +335,10 @@ class Topic < ActiveRecord::Base
         if topic_allowed_users.create!(user_id: user.id)
           # Notify the user they've been invited
           user.notifications.create(notification_type: Notification.Types[:invited_to_private_message],
-                                    topic_id: self.id,
+                                    topic_id: id,
                                     post_number: 1,
-                                    data: {topic_title: self.title,
+                                    data: { topic_title: title,
-                                           display_username: invited_by.username}.to_json)
+                                            display_username: invited_by.username }.to_json)
           return true
         end
       elsif username_or_email =~ /^.+@.+$/
@@ -371,7 +371,7 @@ class Topic < ActiveRecord::Base
           topic_allowed_users.create!(user_id: user.id)
         end
 
-        return nil
+        return
       end
     end
 
@@ -387,15 +387,14 @@ class Topic < ActiveRecord::Base
     topic = nil
     first_post_number = nil
     Topic.transaction do
-      topic = Topic.create(user: moved_by, title: new_title, category: self.category)
+      topic = Topic.create(user: moved_by, title: new_title, category: category)
 
       to_move = posts.where(id: post_ids).order(:created_at)
       raise Discourse::InvalidParameters.new(:post_ids) if to_move.blank?
 
       to_move.each_with_index do |post, i|
         first_post_number ||= post.post_number
-        row_count = Post.update_all ["post_number = :post_number, topic_id = :topic_id, sort_order = :post_number", post_number: i+1, topic_id: topic.id],
+        row_count = Post.update_all ["post_number = :post_number, topic_id = :topic_id, sort_order = :post_number", post_number: i+1, topic_id: topic.id], id: post.id, topic_id: id
-                                    ['id = ? AND topic_id = ?', post.id, self.id]
 
         # We raise an error if any of the posts can't be moved
         raise Discourse::InvalidParameters.new(:post_ids) if row_count == 0
@@ -403,7 +402,7 @@ class Topic < ActiveRecord::Base
 
       # Update denormalized values since we've manually moved stuff
       Topic.reset_highest(topic.id)
-      Topic.reset_highest(self.id)
+      Topic.reset_highest(id)
     end
 
     # Add a moderator post explaining that the post was moved
@@ -425,7 +424,7 @@ class Topic < ActiveRecord::Base
 
   # Create the summary of the interesting posters in a topic. Cheats to avoid
   # many queries.
-  def posters_summary(topic_user=nil, current_user=nil, opts={})
+  def posters_summary(topic_user = nil, current_user = nil, opts={})
     return @posters_summary if @posters_summary.present?
     descriptions = {}
 
@@ -484,7 +483,7 @@ class Topic < ActiveRecord::Base
   # Enable/disable the star on the topic
   def toggle_star(user, starred)
     Topic.transaction do
-      TopicUser.change(user, self.id, starred: starred, starred_at: starred ? DateTime.now : nil)
+      TopicUser.change(user, id, starred: starred, starred_at: starred ? DateTime.now : nil)
 
       # Update the star count
       exec_sql "UPDATE topics
@@ -492,7 +491,7 @@ class Topic < ActiveRecord::Base
                                   FROM topic_users AS ftu
                                   WHERE ftu.topic_id = topics.id
                                     AND ftu.starred = true)
-                WHERE id = ?", self.id
+                WHERE id = ?", id
 
       if starred
         FavoriteLimiter.new(user).performed!
@@ -517,7 +516,7 @@ class Topic < ActiveRecord::Base
 
   def relative_url(post_number=nil)
     url = "/t/#{slug}/#{id}"
-    url << "/#{post_number}" if post_number.present? and post_number.to_i > 1
+    url << "/#{post_number}" if post_number.present? && post_number.to_i > 1
     url
   end
 
@@ -528,23 +527,23 @@ class Topic < ActiveRecord::Base
   end
 
   def draft_key
-    "#{Draft::EXISTING_TOPIC}#{self.id}"
+    "#{Draft::EXISTING_TOPIC}#{id}"
   end
 
   # notification stuff
   def notify_watch!(user)
-    TopicUser.change(user, self.id, notification_level: TopicUser::NotificationLevel::WATCHING)
+    TopicUser.change(user, id, notification_level: TopicUser::NotificationLevel::WATCHING)
   end
 
   def notify_tracking!(user)
-    TopicUser.change(user, self.id, notification_level: TopicUser::NotificationLevel::TRACKING)
+    TopicUser.change(user, id, notification_level: TopicUser::NotificationLevel::TRACKING)
   end
 
   def notify_regular!(user)
-    TopicUser.change(user, self.id, notification_level: TopicUser::NotificationLevel::REGULAR)
+    TopicUser.change(user, id, notification_level: TopicUser::NotificationLevel::REGULAR)
   end
 
   def notify_muted!(user)
-    TopicUser.change(user, self.id, notification_level: TopicUser::NotificationLevel::MUTED)
+    TopicUser.change(user, id, notification_level: TopicUser::NotificationLevel::MUTED)
   end
 end

app/models/topic_invite.rb

@@ -1,5 +1,4 @@
 class TopicInvite < ActiveRecord::Base
-
   belongs_to :topic
   belongs_to :invite
 

app/models/topic_link.rb

@@ -2,7 +2,6 @@ require 'uri'
 require_dependency 'slug'
 
 class TopicLink < ActiveRecord::Base
-
   belongs_to :topic
   belongs_to :user
   belongs_to :post
@@ -118,8 +117,6 @@ class TopicLink < ActiveRecord::Base
       else
         TopicLink.delete_all ["post_id = :post_id OR link_post_id = :post_id", post_id: post.id]
       end
-
     end
   end
-
 end

app/models/topic_link_click.rb

@@ -2,7 +2,6 @@ require_dependency 'discourse'
 require 'ipaddr'
 
 class TopicLinkClick < ActiveRecord::Base
-
   belongs_to :topic_link, counter_cache: :clicks
   belongs_to :user
 
@@ -54,5 +53,4 @@ class TopicLinkClick < ActiveRecord::Base
 
     result
   end
-
 end

app/models/topic_list.rb

@@ -67,10 +67,8 @@ class TopicList
         query = TopicQuery.new(@current_user, only_category: catSplit[1], limit: false)
         s[name] = query.unread_count + query.new_count
       end
-
     end
 
     s
   end
-
 end

app/models/topic_poster.rb

@@ -14,5 +14,4 @@ class TopicPoster < OpenStruct
   def [](attr)
     send(attr)
   end
-
 end

app/models/topic_user.rb

@@ -1,5 +1,4 @@
 class TopicUser < ActiveRecord::Base
-
   belongs_to :user
   belongs_to :topic
 
@@ -19,7 +18,7 @@ class TopicUser < ActiveRecord::Base
 
   def self.auto_track(user_id, topic_id, reason)
     if TopicUser.where(user_id: user_id, topic_id: topic_id, notifications_reason_id: nil).exists?
-      self.change(user_id, topic_id,
+      change(user_id, topic_id,
           notification_level: NotificationLevel::TRACKING,
           notifications_reason_id: reason
       )
@@ -34,12 +33,10 @@ class TopicUser < ActiveRecord::Base
 
   # Find the information specific to a user in a forum topic
   def self.lookup_for(user, topics)
-
     # If the user isn't logged in, there's no last read posts
-    return {} if user.blank?
+    return {} if user.blank? || topics.blank?
-    return {} if topics.blank?
 
-    topic_ids = topics.map {|ft| ft.id}
+    topic_ids = topics.map(&:id)
     create_lookup(TopicUser.where(topic_id: topic_ids, user_id: user.id))
   end
 
@@ -70,7 +67,6 @@ class TopicUser < ActiveRecord::Base
   # since there's more likely to be an existing record than not. If the update returns 0 rows affected
   # it then creates the row instead.
   def self.change(user_id, topic_id, attrs)
-
     # Sometimes people pass objs instead of the ids. We can handle that.
     topic_id = topic_id.id if topic_id.is_a?(Topic)
     user_id = user_id.id if user_id.is_a?(User)
@@ -85,9 +81,9 @@ class TopicUser < ActiveRecord::Base
       end
       attrs_array = attrs.to_a
 
-      attrs_sql = attrs_array.map {|t| "#{t[0]} = ?"}.join(", ")
+      attrs_sql = attrs_array.map { |t| "#{t[0]} = ?" }.join(", ")
-      vals = attrs_array.map {|t| t[1]}
+      vals = attrs_array.map { |t| t[1] }
-      rows = TopicUser.update_all([attrs_sql, *vals], ["topic_id = ? and user_id = ?", topic_id.to_i, user_id])
+      rows = TopicUser.update_all([attrs_sql, *vals], topic_id: topic_id.to_i, user_id: user_id)
 
       if rows == 0
         now = DateTime.now
@@ -101,7 +97,6 @@ class TopicUser < ActiveRecord::Base
 
         TopicUser.create(attrs.merge!(user_id: user_id, topic_id: topic_id.to_i, first_visited_at: now ,last_visited_at: now))
       end
-
     end
   rescue ActiveRecord::RecordNotUnique
     # In case of a race condition to insert, do nothing
@@ -119,7 +114,6 @@ class TopicUser < ActiveRecord::Base
                values(?,?,?,?)',
                topic.id, user.id, now, now)
     end
-
   end
 
   # Update the last read and the last seen post count, but only if it doesn't exist.
@@ -174,9 +168,6 @@ class TopicUser < ActiveRecord::Base
     end
 
     if rows.length == 0
-
-      self
-
       args[:tracking] = TopicUser::NotificationLevel::TRACKING
       args[:regular] = TopicUser::NotificationLevel::REGULAR
       args[:site_setting] = SiteSetting.auto_track_topics_after
@@ -192,6 +183,4 @@ class TopicUser < ActiveRecord::Base
                 args)
     end
   end
-
-
 end

app/models/upload.rb

@@ -20,7 +20,6 @@ class Upload < ActiveRecord::Base
 
   # Store uploads on s3
   def self.create_on_imgur(user, file, topic_id)
-
     @imgur_loaded = require 'imgur' unless @imgur_loaded
 
 
@@ -56,7 +55,6 @@ class Upload < ActiveRecord::Base
   end
 
   def self.create_on_s3(user, file, topic_id)
-
     @fog_loaded = require 'fog' unless @fog_loaded
 
     tempfile = file.tempfile
@@ -90,5 +88,4 @@ class Upload < ActiveRecord::Base
 
     upload
   end
-
 end

app/models/user.rb

@@ -55,7 +55,7 @@ class User < ActiveRecord::Base
   end
 
   def self.suggest_username(name)
-    return nil unless name.present?
+    return unless name.present?
 
     # If it's an email
     if name =~ /([^@]+)@([^\.]+)/
@@ -266,7 +266,7 @@ class User < ActiveRecord::Base
   end
 
   def has_visit_record?(date)
-    user_visits.where(["visited_at =? ", date]).first
+    user_visits.where(visited_at: date).first
   end
 
   def adding_visit_record(date)
@@ -361,7 +361,7 @@ class User < ActiveRecord::Base
   end
 
   def flags_received_count
-    posts.includes(:post_actions).where('post_actions.post_action_type_id in (?)', PostActionType.FlagTypes).count
+    posts.includes(:post_actions).where(post_actions: { post_action_type_id: PostActionType.FlagTypes }).count
   end
 
   def private_topics_count
@@ -435,7 +435,7 @@ class User < ActiveRecord::Base
     if last_seen.present?
       diff = (Time.now.to_f - last_seen.to_f).round
       if diff > 0 && diff < MAX_TIME_READ_DIFF
-        User.update_all ["time_read = time_read + ?", diff], ["id = ? and time_read = ?", id, time_read]
+        User.update_all ["time_read = time_read + ?", diff], id: id, time_read: time_read
       end
     end
     $redis.set(last_seen_key, Time.now.to_f)

app/models/user_action.rb

@@ -55,7 +55,7 @@ class UserAction < ActiveRecord::Base
 
     results = results.to_a
 
-    results.sort!{|a,b| ORDER[a.action_type] <=> ORDER[b.action_type]}
+    results.sort! { |a,b| ORDER[a.action_type] <=> ORDER[b.action_type] }
     results.each do |row|
       row.description = self.description(row.action_type, detailed: true)
     end
@@ -64,13 +64,13 @@ class UserAction < ActiveRecord::Base
   end
 
   def self.stream_item(action_id, guardian)
-    stream(action_id:action_id, guardian: guardian)[0]
+    stream(action_id: action_id, guardian: guardian).first
   end
 
   def self.stream(opts={})
     user_id = opts[:user_id]
-    offset = opts[:offset]||0
+    offset = opts[:offset] || 0
-    limit = opts[:limit] ||60
+    limit = opts[:limit] || 60
     action_id = opts[:action_id]
     action_types = opts[:action_types]
     guardian = opts[:guardian]
@@ -198,7 +198,7 @@ JOIN users pu on pu.id = COALESCE(p.user_id, t.user_id)
     require_parameters(hash, :action_type, :user_id, :acting_user_id, :target_topic_id, :target_post_id)
     transaction(requires_new: true) do
       begin
-        action = self.new(hash)
+        action = new(hash)
 
         if hash[:created_at]
           action.created_at = hash[:created_at]
@@ -225,5 +225,4 @@ JOIN users pu on pu.id = COALESCE(p.user_id, t.user_id)
       raise Discourse::InvalidParameters.new(p) if data[p].nil?
     end
   end
-
 end

app/models/user_action_observer.rb

@@ -1,10 +1,9 @@
 class UserActionObserver < ActiveRecord::Observer
   observe :post_action, :topic, :post, :notification, :topic_user
 
-
   def after_save(model)
     case
-    when (model.is_a?(PostAction) and (model.is_bookmark? or model.is_like?))
+    when (model.is_a?(PostAction) && (model.is_bookmark? || model.is_like?))
       log_post_action(model)
     when (model.is_a?(Topic))
       log_topic(model)
@@ -39,7 +38,6 @@ class UserActionObserver < ActiveRecord::Observer
   end
 
   def log_notification(model)
-
     action =
       case model.notification_type
         when Notification.Types[:quoted]
@@ -77,11 +75,9 @@ class UserActionObserver < ActiveRecord::Observer
   end
 
   def log_post(model)
-
     # first post gets nada
     return if model.post_number == 1
 
-
     row = {
         action_type: UserAction::POST,
         user_id: model.user_id,
@@ -103,11 +99,11 @@ class UserActionObserver < ActiveRecord::Observer
       end
     end
 
-    rows.each do |row|
+    rows.each do |r|
       if model.deleted_at.nil?
-        UserAction.log_action!(row)
+        UserAction.log_action!(r)
       else
-        UserAction.remove_action!(row)
+        UserAction.remove_action!(r)
       end
     end
   end
@@ -125,7 +121,7 @@ class UserActionObserver < ActiveRecord::Observer
     rows = [row]
 
     if model.private_message?
-      model.topic_allowed_users.reject{|a| a.user_id == model.user_id}.each do |ta|
+      model.topic_allowed_users.reject { |a| a.user_id == model.user_id }.each do |ta|
         row = row.dup
         row[:user_id] = ta.user_id
         row[:action_type] = UserAction::GOT_PRIVATE_MESSAGE
@@ -133,11 +129,11 @@ class UserActionObserver < ActiveRecord::Observer
       end
     end
 
-    rows.each do |row|
+    rows.each do |r|
       if model.deleted_at.nil?
-        UserAction.log_action!(row)
+        UserAction.log_action!(r)
       else
-        UserAction.remove_action!(row)
+        UserAction.remove_action!(r)
       end
     end
   end

app/models/user_email_observer.rb

@@ -55,5 +55,4 @@ class UserEmailObserver < ActiveRecord::Observer
                    user_id: notification.user_id,
                    notification_id: notification.id)
   end
-
 end

app/models/user_open_id.rb

@@ -4,5 +4,4 @@ class UserOpenId < ActiveRecord::Base
 
   validates_presence_of :email
   validates_presence_of :url
-
 end

app/models/user_search.rb

@@ -1,5 +1,4 @@
 class UserSearch
-
   def self.search term, topic_id = nil
     sql = User.sql_builder(
 "select id, username, name, email from users u
@@ -34,5 +33,4 @@ class UserSearch
 
     sql.exec
   end
-
 end

app/models/user_visit.rb

@@ -5,5 +5,4 @@ class UserVisit < ActiveRecord::Base
   def self.by_day
     where("visited_at > ?", 1.month.ago).group(:visited_at).order(:visited_at).count
   end
-
 end

app/models/username_validator.rb

@@ -1,5 +1,4 @@
 class UsernameValidator
-
   def initialize(username)
     @username = username
     @errors = []
@@ -56,5 +55,4 @@ class UsernameValidator
       self.errors << I18n.t(:'user.username.must_begin_with_alphanumeric')
     end
   end
-
 end

app/models/view.rb

@@ -1,7 +1,6 @@
 require 'ipaddr'
 
 class View < ActiveRecord::Base
-
   belongs_to :parent, polymorphic: true
   belongs_to :user
   validates_presence_of :parent_type, :parent_id, :ip, :viewed_at
@@ -21,16 +20,13 @@ class View < ActiveRecord::Base
       $redis.expire(redis_key, 1.day.to_i)
 
       View.transaction do
-        view = View.create(parent: parent, ip: IPAddr.new(ip).to_i, viewed_at: Date.today, user: user)
+        View.create(parent: parent, ip: IPAddr.new(ip).to_i, viewed_at: Date.today, user: user)
 
         # Update the views count in the parent, if it exists.
         if parent.respond_to?(:views)
-          parent.class.update_all 'views = views + 1', ['id = ?', parent.id]
+          parent.class.update_all 'views = views + 1', id: parent.id
-        end
+        end
-
+      end
-      end
-
     end
   end
-
 end

spec/models/site_setting_spec.rb

@@ -116,4 +116,12 @@ describe SiteSetting do
     end
   end
 
+  describe 'post_length' do
+    it 'returns a range of min/max post length' do
+      SiteSetting.min_post_length = 1
+      SiteSetting.max_post_length = 2
+
+      SiteSetting.post_length.should == (1..2)
+    end
+  end
 end