FEATURE: Improve errors when title is invalid (#11149)

It used to simply say "title is invalid" without giving any hint what
the problem could be. This commit adds different errors messages for
all caps titles, low entropy titles or titles with very long words.

config/locales/server.en.yml

@@ -149,6 +149,9 @@ en:
       inclusion: is not included in the list
       invalid: is invalid
       is_invalid: "seems unclear, is it a complete sentence?"
+      is_invalid_meaningful: "seems unclear, most of the words contain the same letters over and over?"
+      is_invalid_unpretentious: "seems unclear, one or more words is very long?"
+      is_invalid_quiet: "seems unclear, did you mean to enter it in ALL CAPS?"
       invalid_timezone: "'%{tz}' is not a valid timezone"
       contains_censored_words: "contains the following censored words: %{censored_words}"
       less_than: must be less than %{count}

lib/text_sentinel.rb

@@ -54,12 +54,6 @@ class TextSentinel
     seems_quiet?
   end
 
-  private
-
-  def symbols_regex
-    /[\ -\/\[-\`\:-\@\{-\~]/m
-  end
-
   def seems_meaningful?
     # Minimum entropy if entropy check required
     @opts[:min_entropy].blank? || (entropy >= @opts[:min_entropy])
@@ -71,10 +65,6 @@ class TextSentinel
     @text.gsub(symbols_regex, '').size > 0
   end
 
-  def skipped_locale
-    %w(zh_CN zh_TW ko ja).freeze
-  end
-
   def seems_unpretentious?
     return true if skipped_locale.include?(SiteSetting.default_locale)
     # Don't allow super long words if there is a word length maximum
@@ -87,4 +77,14 @@ class TextSentinel
     SiteSetting.allow_uppercase_posts || @text == @text.mb_chars.downcase.to_s || @text != @text.mb_chars.upcase.to_s
   end
 
+  private
+
+  def symbols_regex
+    /[\ -\/\[-\`\:-\@\{-\~]/m
+  end
+
+  def skipped_locale
+    %w(zh_CN zh_TW ko ja).freeze
+  end
+
 end

lib/validators/quality_title_validator.rb

@@ -6,6 +6,17 @@ require 'text_cleaner'
 class QualityTitleValidator < ActiveModel::EachValidator
   def validate_each(record, attribute, value)
     sentinel = TextSentinel.title_sentinel(value)
-    record.errors.add(attribute, :is_invalid) unless sentinel.valid?
+
+    if !sentinel.valid?
+      if !sentinel.seems_meaningful?
+        record.errors.add(attribute, :is_invalid_meaningful)
+      elsif !sentinel.seems_unpretentious?
+        record.errors.add(attribute, :is_invalid_unpretentious)
+      elsif !sentinel.seems_quiet?
+        record.errors.add(attribute, :is_invalid_quiet)
+      else
+        record.errors.add(attribute, :is_invalid)
+      end
+    end
   end
 end

spec/components/validators/quality_title_validator_spec.rb

@@ -18,6 +18,10 @@ describe "A record validated with QualityTitleValidator" do
   let(:long_title) { valid_title.center(SiteSetting.max_topic_title_length + 1, 'x') }
   let(:xxxxx_title) { valid_title.gsub(/./, 'x') }
 
+  let(:meaningless_title) { "asdf asdf asdf" }
+  let(:loud_title) { "ALL CAPS INVALID TITLE" }
+  let(:pretentious_title) { "superverylongwordintitlefornoparticularreason" }
+
   subject(:topic) { QualityTitleValidatorSpec::Validatable.new }
 
   before(:each) do
@@ -67,12 +71,21 @@ describe "A record validated with QualityTitleValidator" do
     expect(topic).not_to be_valid
   end
 
-  # describe "with a name" do
-  #   it "is not valid without a name" do
-  #     subject.stub(:name => nil)
-  #     subject.should_not be_valid
-  #     subject.should have(1).error_on(:name)
-  #     subject.errors[:name].first.should == "can't be blank"
-  #   end
-  # end
+  it "doesn't allow a meaningless title" do
+    topic.title = meaningless_title
+    expect(topic).not_to be_valid
+    expect(topic.errors.full_messages.first).to include(I18n.t("errors.messages.is_invalid_meaningful"))
+  end
+
+  it "doesn't allow a pretentious title" do
+    topic.title = pretentious_title
+    expect(topic).not_to be_valid
+    expect(topic.errors.full_messages.first).to include(I18n.t("errors.messages.is_invalid_unpretentious"))
+  end
+
+  it "doesn't allow a loud title" do
+    topic.title = loud_title
+    expect(topic).not_to be_valid
+    expect(topic.errors.full_messages.first).to include(I18n.t("errors.messages.is_invalid_quiet"))
+  end
 end