refactor validators

add a new setting for min pm body length
use that setting for flags
scale entropy check down for pms

app/assets/javascripts/discourse/controllers/flag_action_type_controller.js

@@ -27,12 +27,12 @@ Discourse.FlagActionTypeController = Discourse.ObjectController.extend({
   showDescription: Em.computed.not('showMessageInput'),
 
   customMessageLengthClasses: function() {
-    return (this.get('message.length') < Discourse.PostActionType.MIN_MESSAGE_LENGTH) ? "too-short" : "ok"
+    return (this.get('message.length') < Discourse.SiteSettings.min_private_message_post_length) ? "too-short" : "ok"
   }.property('message.length'),
 
   customMessageLength: function() {
     var len = this.get('message.length') || 0;
-    var minLen = Discourse.PostActionType.MIN_MESSAGE_LENGTH;
+    var minLen = Discourse.SiteSettings.min_private_message_post_length;
     if (len === 0) {
       return Em.String.i18n("flagging.custom_message.at_least", { n: minLen });
     } else if (len < minLen) {

app/assets/javascripts/discourse/controllers/flag_controller.js

@@ -19,7 +19,7 @@ Discourse.FlagController = Discourse.ObjectController.extend(Discourse.ModalFunc
 
     if (selected.get('is_custom_flag')) {
       var len = this.get('message.length') || 0;
-      return len >= Discourse.PostActionType.MIN_MESSAGE_LENGTH &&
+      return len >= Discourse.SiteSettings.min_private_message_post_length &&
              len <= Discourse.PostActionType.MAX_MESSAGE_LENGTH;
     }
     return true;

app/assets/javascripts/discourse/models/post_action_type.js

@@ -9,6 +9,5 @@
 Discourse.PostActionType = Discourse.Model.extend({});
 
 Discourse.PostActionType.reopenClass({
-  MIN_MESSAGE_LENGTH: 10,
   MAX_MESSAGE_LENGTH: 500
 })

app/models/post.rb

@@ -5,6 +5,7 @@ require_dependency 'post_revisor'
 require_dependency 'enum'
 require_dependency 'trashable'
 require_dependency 'post_analyzer'
+require_dependency 'validators/post_validator'
 
 require 'archetype'
 require 'digest/sha1'
@@ -17,7 +18,6 @@ class Post < ActiveRecord::Base
 
   rate_limit
 
-
   belongs_to :user
   belongs_to :topic, counter_cache: :posts_count
   belongs_to :reply_to_user, class_name: "User"
@@ -28,9 +28,7 @@ class Post < ActiveRecord::Base
 
   has_one :post_search_data
 
-  validates_presence_of :raw, :user_id, :topic_id
+  validates_with ::Validators::PostValidator
-  validates :raw, stripped_length: { in: -> { SiteSetting.post_length } }
-  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

app/models/site_setting.rb

@@ -29,6 +29,7 @@ class SiteSetting < ActiveRecord::Base
   client_setting(:polling_interval, 3000)
   client_setting(:anon_polling_interval, 30000)
   client_setting(:min_post_length, Rails.env.test? ? 5 : 20)
+  client_setting(:min_private_message_post_length, Rails.env.test? ? 5 : 10)
   client_setting(:max_post_length, 16000)
   client_setting(:min_topic_title_length, 15)
   client_setting(:max_topic_title_length, 255)
@@ -236,6 +237,10 @@ class SiteSetting < ActiveRecord::Base
     min_post_length..max_post_length
   end
 
+  def self.private_message_post_length
+    min_private_message_post_length..max_post_length
+  end
+
   def self.homepage
     # TODO objectify this
     top_menu.split('|')[0].split(',')[0]

config/locales/server.en.yml

@@ -426,6 +426,7 @@ en:
   site_settings:
     default_locale: "The default language of this Discourse instance (ISO 639-1 Code)"
     min_post_length: "Minimum post length in characters"
+    min_private_message_post_length: "Minimum post length in characters for private messages"
     max_post_length: "Maximum post length in characters"
     min_topic_title_length: "Minimum topic title length in characters"
     max_topic_title_length: "Maximum topic title length in characters"

lib/text_sentinel.rb

@@ -10,8 +10,13 @@ class TextSentinel
     @text = text.to_s.encode('UTF-8', invalid: :replace, undef: :replace, replace: '')
   end
 
-  def self.body_sentinel(text)
+  def self.body_sentinel(text, opts={})
-    TextSentinel.new(text, min_entropy: SiteSetting.body_min_entropy)
+    entropy = SiteSetting.body_min_entropy
+    if opts[:private_message]
+      scale_entropy = SiteSetting.min_private_message_post_length.to_f / SiteSetting.min_post_length.to_f
+      entropy = (entropy * scale_entropy).to_i
+    end
+    TextSentinel.new(text, min_entropy: entropy)
   end
 
   def self.title_sentinel(text)

lib/validators/post_validator.rb

@@ -1,5 +1,9 @@
-class PostValidator < ActiveModel::Validator
+require_dependency 'validators/stripped_length_validator'
+module Validators; end
+class Validators::PostValidator < ActiveModel::Validator
   def validate(record)
+    presence(record)
+    stripped_length(record)
     raw_quality(record)
     max_mention_validator(record)
     max_images_validator(record)
@@ -7,8 +11,19 @@ class PostValidator < ActiveModel::Validator
     unique_post_validator(record)
   end
 
+  def presence(post)
+    [:raw,:user_id,:topic_id].each do |attr_name|
+       post.errors.add(attr_name, :blank, options) if post.send(attr_name).blank?
+    end
+  end
+
+  def stripped_length(post)
+    range = post.topic.try(:private_message?) ? SiteSetting.private_message_post_length : SiteSetting.post_length
+    Validators::StrippedLengthValidator.validate(post, :raw, post.raw, range)
+  end
+
   def raw_quality(post)
-    sentinel = TextSentinel.body_sentinel(post.raw)
+    sentinel = TextSentinel.body_sentinel(post.raw, private_message: post.topic.try(:private_message?))
     post.errors.add(:raw, I18n.t(:is_invalid)) unless sentinel.valid?
   end
 

lib/validators/stripped_length_validator.rb

@@ -1,15 +1,20 @@
-class StrippedLengthValidator < ActiveModel::EachValidator
+module Validators; end
-  def validate_each(record, attribute, value)
+class Validators::StrippedLengthValidator < ActiveModel::EachValidator
+  def self.validate(record, attribute, value, range)
     unless value.nil?
       stripped_length = value.strip.length
-      # the `in` parameter might be a lambda when the range is dynamic
+      record.errors.add attribute, (I18n.t('errors.messages.too_short', count: range.begin)) unless
-      range = options[:in].lambda? ? options[:in].call : options[:in]
-      record.errors.add attribute, (options[:message] || I18n.t('errors.messages.too_short', count: range.begin)) unless
           stripped_length >= range.begin
-      record.errors.add attribute, (options[:message] || I18n.t('errors.messages.too_long', count: range.end)) unless
+      record.errors.add attribute, (I18n.t('errors.messages.too_long', count: range.end)) unless
           stripped_length <= range.end
     else
-      record.errors.add attribute, (options[:message] || I18n.t('errors.messages.blank'))
+      record.errors.add attribute, (I18n.t('errors.messages.blank'))
     end
   end
+
+  def validate_each(record, attribute, value)
+    # the `in` parameter might be a lambda when the range is dynamic
+    range = options[:in].lambda? ? options[:in].call : options[:in]
+    self.class.validate(record, attribute, value, range)
+  end
 end

spec/components/validators/post_validator_spec.rb

@@ -0,0 +1,34 @@
+require 'spec_helper'
+require_dependency 'validators/post_validator'
+
+describe Validators::PostValidator do
+  let :post do
+    build(:post)
+  end
+
+  let :validator do
+    Validators::PostValidator.new({})
+  end
+
+  context "stripped_length" do
+    it "adds an error for short raw" do
+      post.raw = "abc"
+      validator.stripped_length(post)
+      expect(post.errors.count).to eq(1)
+    end
+
+    it "adds no error for long raw" do
+      post.raw = "this is a long topic body testing 123"
+      validator.stripped_length(post)
+      expect(post.errors.count).to eq(0)
+    end
+  end
+
+  context "invalid post" do
+    it "should be invalid" do
+      validator.validate(post)
+      expect(post.errors.count).to be > 0
+    end
+  end
+
+end

spec/models/post_spec.rb

@@ -365,10 +365,16 @@ describe Post do
 
   end
 
-  it 'validates' do
+  context 'validation' do
+    it 'validates our default post' do
       Fabricate.build(:post, post_args).should be_valid
     end
 
+    it 'treate blank posts as invalid' do
+      Fabricate.build(:post, raw: "").should_not be_valid
+    end
+  end
+
   context "raw_hash" do
 
     let(:raw) { "this is our test post body"}