From 3fdba0019bd7088356535817f86e3ae3b906a3b3 Mon Sep 17 00:00:00 2001 From: Navin Date: Sun, 9 Jun 2013 18:48:44 +0200 Subject: [PATCH] Extract callbacks and validations for Post Move Post create callbacks to PostCreate Extract Post validations Move stripped_length_validator to lib/validators --- app/models/post.rb | 82 ++----------------- lib/post_creator.rb | 29 ++++++- lib/validators/post_validator.rb | 56 +++++++++++++ .../validators}/stripped_length_validator.rb | 0 4 files changed, 89 insertions(+), 78 deletions(-) create mode 100644 lib/validators/post_validator.rb rename {app/models => lib/validators}/stripped_length_validator.rb (100%) diff --git a/app/models/post.rb b/app/models/post.rb index 992ae4d2b..08991e6eb 100644 --- a/app/models/post.rb +++ b/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 - validate :max_mention_validator - validate :max_images_validator - validate :max_links_validator - validate :unique_post_validator + validates_with PostValidator # 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_post_number, order('post_number ASC') - scope :with_user, includes(:user) + scope :by_newest, -> { order('created_at desc, id desc') } + scope :by_post_number, -> { order('post_number ASC') } + 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? - 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 + PostCreator.before_create_tasks(self) end # TODO: Move some of this into an asynchronous job? # TODO: Move into PostCreator after_create do - - 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) + PostCreator.after_create_tasks(self) 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? diff --git a/lib/post_creator.rb b/lib/post_creator.rb index 0f39dda50..529e35ece 100644 --- a/lib/post_creator.rb +++ b/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) diff --git a/lib/validators/post_validator.rb b/lib/validators/post_validator.rb new file mode 100644 index 000000000..7197c7bd6 --- /dev/null +++ b/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 diff --git a/app/models/stripped_length_validator.rb b/lib/validators/stripped_length_validator.rb similarity index 100% rename from app/models/stripped_length_validator.rb rename to lib/validators/stripped_length_validator.rb