Merge pull request #995 from novemberkilo/master

Refactoring Post model

app/models/post.rb

@@ -30,20 +30,16 @@ class Post < ActiveRecord::Base
 
   validates_presence_of :raw, :user_id, :topic_id
   validates :raw, stripped_length: { in: -> { SiteSetting.post_length } }
-  validate :raw_quality
+  validates_with PostValidator
-  validate :max_mention_validator
-  validate :max_images_validator
-  validate :max_links_validator
-  validate :unique_post_validator
 
   # We can pass a hash of image sizes when saving to prevent crawling those images
   attr_accessor :image_sizes, :quoted_post_numbers, :no_bump, :invalidate_oneboxes
 
   SHORT_POST_CHARS = 1200
 
-  scope :by_newest, order('created_at desc, id desc')
+  scope :by_newest, -> { order('created_at desc, id desc') }
-  scope :by_post_number, order('post_number ASC')
+  scope :by_post_number, -> { order('post_number ASC') }
-  scope :with_user, includes(:user)
+  scope :with_user, -> { includes(:user) }
   scope :public_posts, -> { joins(:topic).where('topics.archetype <> ?', Archetype.private_message) }
   scope :private_posts, -> { joins(:topic).where('topics.archetype = ?', Archetype.private_message) }
   scope :with_topic_subtype, ->(subtype) { joins(:topic).where('topics.subtype = ?', subtype) }
@@ -61,24 +57,6 @@ class Post < ActiveRecord::Base
     update_flagged_posts_count
   end
 
-  def raw_quality
-    sentinel = TextSentinel.body_sentinel(raw)
-    errors.add(:raw, I18n.t(:is_invalid)) unless sentinel.valid?
-  end
-
-  # Stop us from posting the same thing too quickly
-  def unique_post_validator
-    return if SiteSetting.unique_posts_mins == 0
-    return if acting_user.admin? || acting_user.moderator?
-
-    # If the post is empty, default to the validates_presence_of
-    return if raw.blank?
-
-    if $redis.exists(unique_post_key)
-      errors.add(:raw, I18n.t(:just_posted_that))
-    end
-  end
-
   # The key we use in redis to ensure unique posts
   def unique_post_key
     "post-#{user_id}:#{raw_hash}"
@@ -124,25 +102,6 @@ class Post < ActiveRecord::Base
     @acting_user = pu
   end
 
-  # Ensure maximum amount of mentions in a post
-  def max_mention_validator
-    if acting_user_is_trusted?
-      add_error_if_count_exceeded(:too_many_mentions, raw_mentions.size, SiteSetting.max_mentions_per_post)
-    else
-      add_error_if_count_exceeded(:too_many_mentions_newuser, raw_mentions.size, SiteSetting.newuser_max_mentions_per_post)
-    end
-  end
-
-  # Ensure new users can not put too many images in a post
-  def max_images_validator
-    add_error_if_count_exceeded(:too_many_images, image_count, SiteSetting.newuser_max_images) unless acting_user_is_trusted?
-  end
-
-  # Ensure new users can not put too many links in a post
-  def max_links_validator
-    add_error_if_count_exceeded(:too_many_links, link_count, SiteSetting.newuser_max_links) unless acting_user_is_trusted?
-  end
-
   def total_hosts_usage
     hosts = linked_hosts.clone
 
@@ -291,37 +250,14 @@ class Post < ActiveRecord::Base
     PostRevisor.new(self).revise!(updated_by, new_raw, opts)
   end
 
-
-  # TODO: move into PostCreator
-  # Various callbacks
   before_create do
-    if reply_to_post_number.present?
+    PostCreator.before_create_tasks(self)
-      self.reply_to_user_id ||= Post.select(:user_id).where(topic_id: topic_id, post_number: reply_to_post_number).first.try(:user_id)
-    end
-
-    self.post_number ||= Topic.next_post_number(topic_id, reply_to_post_number.present?)
-    self.cooked ||= cook(raw, topic_id: topic_id)
-    self.sort_order = post_number
-    DiscourseEvent.trigger(:before_create_post, self)
-    self.last_version_at ||= Time.now
   end
 
   # TODO: Move some of this into an asynchronous job?
   # TODO: Move into PostCreator
   after_create do
-
+    PostCreator.after_create_tasks(self)
-    Rails.logger.info (">" * 30) + "#{no_bump} #{created_at}"
-    # Update attributes on the topic - featured users and last posted.
-    attrs = {last_posted_at: created_at, last_post_user_id: user_id}
-    attrs[:bumped_at] = created_at unless no_bump
-    topic.update_attributes(attrs)
-
-    # Update topic user data
-    TopicUser.change(user,
-                     topic.id,
-                     posted: true,
-                     last_read_post_number: post_number,
-                     seen_post_count: post_number)
   end
 
   # This calculates the geometric mean of the post timings and stores it along with
@@ -401,13 +337,7 @@ class Post < ActiveRecord::Base
 
   private
 
-  def acting_user_is_trusted?
-    acting_user.present? && acting_user.has_trust_level?(:basic)
-  end
 
-  def add_error_if_count_exceeded(key_for_translation, current_count, max_count)
-    errors.add(:base, I18n.t(key_for_translation, count: max_count)) if current_count > max_count
-  end
 
   def parse_quote_into_arguments(quote)
     return {} unless quote.present?

lib/post_creator.rb

@@ -73,12 +73,37 @@ class PostCreator
     @post
   end
 
-
-  # Shortcut
   def self.create(user, opts)
     PostCreator.new(user, opts).create
   end
 
+  def self.before_create_tasks(post)
+    if post.reply_to_post_number.present?
+      post.reply_to_user_id ||= Post.select(:user_id).where(topic_id: post.topic_id, post_number: post.reply_to_post_number).first.try(:user_id)
+    end
+
+    post.post_number ||= Topic.next_post_number(post.topic_id, post.reply_to_post_number.present?)
+    post.cooked ||= post.cook(post.raw, topic_id: post.topic_id)
+    post.sort_order = post.post_number
+    DiscourseEvent.trigger(:before_create_post, post)
+    post.last_version_at ||= Time.now
+  end
+
+  def self.after_create_tasks(post)
+    Rails.logger.info (">" * 30) + "#{post.no_bump} #{post.created_at}"
+    # Update attributes on the topic - featured users and last posted.
+    attrs = {last_posted_at: post.created_at, last_post_user_id: post.user_id}
+    attrs[:bumped_at] = post.created_at unless post.no_bump
+    post.topic.update_attributes(attrs)
+
+    # Update topic user data
+    TopicUser.change(post.user,
+                     post.topic.id,
+                     posted: true,
+                     last_read_post_number: post.post_number,
+                     seen_post_count: post.post_number)
+  end
+
   protected
 
   def secure_group_ids(topic)

lib/validators/post_validator.rb

@@ -0,0 +1,56 @@
+class PostValidator < ActiveModel::Validator
+  def validate(record)
+    raw_quality(record)
+    max_mention_validator(record)
+    max_images_validator(record)
+    max_links_validator(record)
+    unique_post_validator(record)
+  end
+
+  def raw_quality(post)
+    sentinel = TextSentinel.body_sentinel(post.raw)
+    post.errors.add(:raw, I18n.t(:is_invalid)) unless sentinel.valid?
+  end
+
+  # Ensure maximum amount of mentions in a post
+  def max_mention_validator(post)
+    if acting_user_is_trusted?(post)
+      add_error_if_count_exceeded(post, :too_many_mentions, post.raw_mentions.size, SiteSetting.max_mentions_per_post)
+    else
+      add_error_if_count_exceeded(post, :too_many_mentions_newuser, post.raw_mentions.size, SiteSetting.newuser_max_mentions_per_post)
+    end
+  end
+
+  # Ensure new users can not put too many images in a post
+  def max_images_validator(post)
+    add_error_if_count_exceeded(post, :too_many_images, post.image_count, SiteSetting.newuser_max_images) unless acting_user_is_trusted?(post)
+  end
+
+  # Ensure new users can not put too many links in a post
+  def max_links_validator(post)
+    add_error_if_count_exceeded(post, :too_many_links, post.link_count, SiteSetting.newuser_max_links) unless acting_user_is_trusted?(post)
+  end
+
+  # Stop us from posting the same thing too quickly
+  def unique_post_validator(post)
+    return if SiteSetting.unique_posts_mins == 0
+    return if post.acting_user.admin? || post.acting_user.moderator?
+
+    # If the post is empty, default to the validates_presence_of
+    return if post.raw.blank?
+
+    if $redis.exists(post.unique_post_key)
+      post.errors.add(:raw, I18n.t(:just_posted_that))
+    end
+  end
+
+  private
+
+  def acting_user_is_trusted?(post)
+    post.acting_user.present? && post.acting_user.has_trust_level?(:basic)
+  end
+
+  def add_error_if_count_exceeded(post, key_for_translation, current_count, max_count)
+    post.errors.add(:base, I18n.t(key_for_translation, count: max_count)) if current_count > max_count
+  end
+end