From 2465c9c724c3051477499756d1f9fc5e2bcc587b Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Tue, 4 Jun 2013 17:58:25 -0400 Subject: [PATCH] Add min_private_message_title_length site setting so private messages can have short titles --- .../javascripts/discourse/models/composer.js | 26 ++++++-- .../discourse/views/composer_view.js | 5 +- app/models/site_setting.rb | 5 ++ app/models/topic.rb | 3 +- config/locales/server.en.yml | 1 + .../topic_title_length_validator.rb | 16 +++++ .../topic_title_length_validator_spec.rb | 60 +++++++++++++++++++ spec/models/site_setting_spec.rb | 7 +++ spec/models/topic_spec.rb | 17 ++++++ 9 files changed, 131 insertions(+), 9 deletions(-) create mode 100644 lib/validators/topic_title_length_validator.rb create mode 100644 spec/components/validators/topic_title_length_validator_spec.rb diff --git a/app/assets/javascripts/discourse/models/composer.js b/app/assets/javascripts/discourse/models/composer.js index fb3f8984541..d7092eaa810 100644 --- a/app/assets/javascripts/discourse/models/composer.js +++ b/app/assets/javascripts/discourse/models/composer.js @@ -68,6 +68,10 @@ Discourse.Composer = Discourse.Model.extend({ return false; }.property('editingPost', 'creatingTopic', 'post.post_number'), + canCategorize: function() { + return (this.get('editTitle') && !this.get('creatingPrivateMessage')); + }.property('editTitle', 'creatingPrivateMessage'), + showAdminOptions: function() { if (this.get('creatingTopic') && Discourse.User.current('staff')) return true; return false; @@ -164,9 +168,8 @@ Discourse.Composer = Discourse.Model.extend({ // - creating a new topic // - editing the 1st post // - creating a private message - if (this.get('editTitle') && - (this.get('titleLength') < Discourse.SiteSettings.min_topic_title_length || - this.get('titleLength') > Discourse.SiteSettings.max_topic_title_length) ) return true; + + if (this.get('editTitle') && !this.get('titleLengthValid')) return true; // Need at least one user when sending a private message if ( this.get('creatingPrivateMessage') && @@ -178,11 +181,20 @@ Discourse.Composer = Discourse.Model.extend({ // reply is always required if (this.get('replyLength') < Discourse.SiteSettings.min_post_length) return true; - if (this.get('editTitle') && !Discourse.SiteSettings.allow_uncategorized_topics && !this.get('categoryName')) return true; + if (this.get('canCategorize') && !Discourse.SiteSettings.allow_uncategorized_topics && !this.get('categoryName')) return true; return false; }.property('loading', 'editTitle', 'titleLength', 'targetUsernames', 'replyLength', 'categoryName'), + titleLengthValid: function() { + if (this.get('creatingPrivateMessage')) { + if (this.get('titleLength') < Discourse.SiteSettings.min_private_message_title_length) return false; + } else { + if (this.get('titleLength') < Discourse.SiteSettings.min_topic_title_length) return false; + } + return (this.get('titleLength') <= Discourse.SiteSettings.max_topic_title_length); + }.property('titleLength'), + // The text for the save button saveText: function() { switch (this.get('action')) { @@ -519,7 +531,11 @@ Discourse.Composer = Discourse.Model.extend({ @property missingTitleCharacters **/ missingTitleCharacters: function() { - return Discourse.SiteSettings.min_topic_title_length - this.get('titleLength'); + if (this.get('creatingPrivateMessage')) { + return Discourse.SiteSettings.min_private_message_title_length - this.get('titleLength'); + } else { + return Discourse.SiteSettings.min_topic_title_length - this.get('titleLength'); + } }.property('titleLength'), diff --git a/app/assets/javascripts/discourse/views/composer_view.js b/app/assets/javascripts/discourse/views/composer_view.js index 0db67248135..35d1994e564 100644 --- a/app/assets/javascripts/discourse/views/composer_view.js +++ b/app/assets/javascripts/discourse/views/composer_view.js @@ -373,10 +373,11 @@ Discourse.ComposerView = Discourse.View.extend({ titleValidation: function() { var title = this.get('content.title'), reason; + var minLength = (this.get('content.creatingPrivateMessage') ? Discourse.SiteSettings.min_private_message_title_length : Discourse.SiteSettings.min_topic_title_length); if( !title || title.length < 1 ){ reason = Em.String.i18n('composer.error.title_missing'); - } else if( title.length < Discourse.SiteSettings.min_topic_title_length ) { - reason = Em.String.i18n('composer.error.title_too_short', {min: Discourse.SiteSettings.min_topic_title_length}) + } else if( title.length < minLength ) { + reason = Em.String.i18n('composer.error.title_too_short', {min: minLength}) } else if( title.length > Discourse.SiteSettings.max_topic_title_length ) { reason = Em.String.i18n('composer.error.title_too_long', {max: Discourse.SiteSettings.max_topic_title_length}) } diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index c47782f040d..829993e388f 100644 --- a/app/models/site_setting.rb +++ b/app/models/site_setting.rb @@ -34,6 +34,7 @@ class SiteSetting < ActiveRecord::Base client_setting(:max_post_length, 16000) client_setting(:min_topic_title_length, 15) client_setting(:max_topic_title_length, 255) + client_setting(:min_private_message_title_length, 2) client_setting(:allow_uncategorized_topics, true) client_setting(:min_search_term_length, 3) client_setting(:flush_timings_secs, 5) @@ -227,6 +228,10 @@ class SiteSetting < ActiveRecord::Base min_topic_title_length..max_topic_title_length end + def self.private_message_title_length + min_private_message_title_length..max_topic_title_length + end + def self.post_length min_post_length..max_post_length end diff --git a/app/models/topic.rb b/app/models/topic.rb index 80b3a30d4c8..139b2850adb 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -39,8 +39,7 @@ class Topic < ActiveRecord::Base before_validation :sanitize_title validates :title, :presence => true, - :length => { :in => SiteSetting.topic_title_length, - :allow_blank => true }, + :topic_title_length => true, :quality_title => { :unless => :private_message? }, :unique_among => { :unless => Proc.new { |t| (SiteSetting.allow_duplicate_topic_titles? || t.private_message?) }, :message => :has_already_been_used, diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 8cd7aa048c7..eed9b0cee20 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -418,6 +418,7 @@ en: 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" + min_private_message_title_length: "Minimum title length for a private message in characters" min_search_term_length: "Minimum search term length in characters" allow_uncategorized_topics: "Allow topics to be created without a category" allow_duplicate_topic_titles: "Allow topics with identical titles" diff --git a/lib/validators/topic_title_length_validator.rb b/lib/validators/topic_title_length_validator.rb new file mode 100644 index 00000000000..73783e83141 --- /dev/null +++ b/lib/validators/topic_title_length_validator.rb @@ -0,0 +1,16 @@ +class TopicTitleLengthValidator < ActiveModel::EachValidator + + def initialize(options) + @topic_title_validator = ActiveModel::Validations::LengthValidator.new({attributes: :title, in: SiteSetting.topic_title_length, allow_blank: true}) + @private_message_title_validator = ActiveModel::Validations::LengthValidator.new({attributes: :title, in: SiteSetting.private_message_title_length, allow_blank: true}) + super + end + + def validate_each(record, attribute, value) + if record.private_message? + @private_message_title_validator.validate_each(record, attribute, value) + else + @topic_title_validator.validate_each(record, attribute, value) + 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 new file mode 100644 index 00000000000..9b623162424 --- /dev/null +++ b/spec/components/validators/topic_title_length_validator_spec.rb @@ -0,0 +1,60 @@ +# encoding: UTF-8 + +require 'spec_helper' +require 'validators/topic_title_length_validator' + +describe TopicTitleLengthValidator do + + let(:validator) { TopicTitleLengthValidator.new({attributes: :title}) } + subject(:validate) { validator.validate_each(record,:title,record.title) } + + shared_examples "validating any topic title" do + it 'adds an error when topic title is greater than SiteSetting.max_topic_title_length' do + record.title = 'a' * (SiteSetting.max_topic_title_length + 1) + validate + expect(record.errors[:title]).to be_present + end + end + + describe 'topic' do + let(:record) { Fabricate.build(:topic) } + + it 'adds an error when topic title is shorter than SiteSetting.min_topic_title_length' do + record.title = 'a' * (SiteSetting.min_topic_title_length - 1) + validate + expect(record.errors[:title]).to be_present + end + + it 'does not add an error when length is good' do + record.title = 'a' * (SiteSetting.min_topic_title_length) + validate + expect(record.errors[:title]).to_not be_present + end + + include_examples "validating any topic title" + end + + describe 'private message' do + before do + SiteSetting.stubs(:min_private_message_title_length).returns(3) + end + + let(:record) { Fabricate.build(:private_message_topic) } + + it 'adds an error when topic title is shorter than SiteSetting.min_private_message_title_length' do + record.title = 'a' * (SiteSetting.min_private_message_title_length - 1) + validate + expect(record.errors[:title]).to be_present + end + + it 'does not add an error when topic title is shorter than SiteSetting.min_topic_title_length' do + SiteSetting.stubs(:min_topic_title_length).returns(15) + record.title = 'a' * (SiteSetting.min_private_message_title_length) + validate + expect(record.errors[:title]).to_not be_present + end + + include_examples "validating any topic title" + end + +end diff --git a/spec/models/site_setting_spec.rb b/spec/models/site_setting_spec.rb index 95d2ca9d83e..31584f82b7f 100644 --- a/spec/models/site_setting_spec.rb +++ b/spec/models/site_setting_spec.rb @@ -156,4 +156,11 @@ describe SiteSetting do SiteSetting.post_length.should == (SiteSetting.defaults[:min_post_length]..SiteSetting.defaults[:max_post_length]) end end + + describe 'private_message_title_length' do + it 'returns a range of min/max pm topic title length' do + expect(SiteSetting.private_message_title_length).to eq(SiteSetting.defaults[:min_private_message_title_length]..SiteSetting.defaults[:max_topic_title_length]) + end + end + end diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index e65ab4cb4fd..f22d7a98e89 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -52,6 +52,23 @@ describe Topic do end end + context 'private message title' do + before do + SiteSetting.stubs(:min_topic_title_length).returns(15) + SiteSetting.stubs(:min_private_message_title_length).returns(3) + end + + it 'allows shorter titles' do + pm = Fabricate.build(:private_message_topic, title: 'a' * SiteSetting.min_private_message_title_length) + expect(pm).to be_valid + end + + it 'but not too short' do + pm = Fabricate.build(:private_message_topic, title: 'a') + expect(pm).to_not be_valid + end + end + context 'topic title uniqueness' do let!(:topic) { Fabricate(:topic) }