diff --git a/app/assets/javascripts/discourse/controllers/flag_action_type_controller.js b/app/assets/javascripts/discourse/controllers/flag_action_type_controller.js index c099bd022d7..ca97cd3f1d2 100644 --- a/app/assets/javascripts/discourse/controllers/flag_action_type_controller.js +++ b/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) { diff --git a/app/assets/javascripts/discourse/controllers/flag_controller.js b/app/assets/javascripts/discourse/controllers/flag_controller.js index 3331287dc13..ab8615dc41f 100644 --- a/app/assets/javascripts/discourse/controllers/flag_controller.js +++ b/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; diff --git a/app/assets/javascripts/discourse/models/post_action_type.js b/app/assets/javascripts/discourse/models/post_action_type.js index 4ff10880aae..20b7cb42e43 100644 --- a/app/assets/javascripts/discourse/models/post_action_type.js +++ b/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 }) diff --git a/app/models/post.rb b/app/models/post.rb index 08991e6eb80..25d5fbe51b7 100644 --- a/app/models/post.rb +++ b/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 :raw, stripped_length: { in: -> { SiteSetting.post_length } } - validates_with PostValidator + validates_with ::Validators::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 diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index ecaf743b5c6..c92029be626 100644 --- a/app/models/site_setting.rb +++ b/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) @@ -161,7 +162,7 @@ class SiteSetting < ActiveRecord::Base setting(:enforce_global_nicknames, true) setting(:discourse_org_access_key, '') - + setting(:enable_s3_uploads, false) setting(:s3_access_key_id, '') setting(:s3_secret_access_key, '') @@ -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] diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 28253325596..eb25e477e76 100644 --- a/config/locales/server.en.yml +++ b/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" diff --git a/lib/text_sentinel.rb b/lib/text_sentinel.rb index cb0d36393d8..f4361ebfb75 100644 --- a/lib/text_sentinel.rb +++ b/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) - TextSentinel.new(text, min_entropy: SiteSetting.body_min_entropy) + def self.body_sentinel(text, opts={}) + 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) diff --git a/lib/validators/post_validator.rb b/lib/validators/post_validator.rb index 7197c7bd6bd..941585c5130 100644 --- a/lib/validators/post_validator.rb +++ b/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 diff --git a/lib/validators/stripped_length_validator.rb b/lib/validators/stripped_length_validator.rb index b77e786ee9e..37628a2a487 100644 --- a/lib/validators/stripped_length_validator.rb +++ b/lib/validators/stripped_length_validator.rb @@ -1,15 +1,20 @@ -class StrippedLengthValidator < ActiveModel::EachValidator - def validate_each(record, attribute, value) +module Validators; end +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 - 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 + record.errors.add attribute, (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 diff --git a/spec/components/validators/post_validator_spec.rb b/spec/components/validators/post_validator_spec.rb new file mode 100644 index 00000000000..cc6e4afb1f1 --- /dev/null +++ b/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 diff --git a/spec/components/validators/topic_title_length_validator_spec.rb b/spec/components/validators/topic_title_length_validator_spec.rb index 9b623162424..4b33e0c8135 100644 --- a/spec/components/validators/topic_title_length_validator_spec.rb +++ b/spec/components/validators/topic_title_length_validator_spec.rb @@ -53,7 +53,7 @@ describe TopicTitleLengthValidator do validate expect(record.errors[:title]).to_not be_present end - + include_examples "validating any topic title" end diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index 7b2b573919f..a2d0e8559ba 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -365,8 +365,14 @@ describe Post do end - it 'validates' do - Fabricate.build(:post, post_args).should be_valid + 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