From 2d391565e4876806bbb7a9826aa9a9856908c57f Mon Sep 17 00:00:00 2001
From: Osama Sayegh <asooomaasoooma90@gmail.com>
Date: Tue, 4 Oct 2022 21:55:21 +0300
Subject: [PATCH] FIX: Skip quality title validations for static topics when
 edited by admin (#18468)

Static topics are the seeded topics that are automatically created for every Discourse instance to hold the content for the FAQ, ToS and Privacy pages. These topics are allowed to bypass the minimum title length checks when they're edited by admins:

https://github.com/discourse/discourse/blob/ba27ee16376c961c93a4e3854b038a42f9577613/app/assets/javascripts/discourse/app/models/composer.js#L487-L496

However, on the server-side, the "quality title" validations aren't skipped for static topics and that can cause confusion for admins when they change the title of a static topic to something that's short enough to fail the quality title validations. This commit ignores all quality title validations on static topics when they're edited by admins.

Internal topic: t/75745.
---
 lib/validators/quality_title_validator.rb     |  2 +
 .../quality_title_validator_spec.rb           | 54 +++++++++++--------
 2 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/lib/validators/quality_title_validator.rb b/lib/validators/quality_title_validator.rb
index c05ae07f9de..2fa95624392 100644
--- a/lib/validators/quality_title_validator.rb
+++ b/lib/validators/quality_title_validator.rb
@@ -5,6 +5,8 @@ require 'text_cleaner'
 
 class QualityTitleValidator < ActiveModel::EachValidator
   def validate_each(record, attribute, value)
+    return if Discourse.static_doc_topic_ids.include?(record.id) && record.acting_user&.admin?
+
     sentinel = TextSentinel.title_sentinel(value)
 
     if !sentinel.valid?
diff --git a/spec/lib/validators/quality_title_validator_spec.rb b/spec/lib/validators/quality_title_validator_spec.rb
index 00a0d705244..a5574e29b9e 100644
--- a/spec/lib/validators/quality_title_validator_spec.rb
+++ b/spec/lib/validators/quality_title_validator_spec.rb
@@ -1,29 +1,19 @@
 # encoding: UTF-8
 # frozen_string_literal: true
 
-require 'ostruct'
-
-module QualityTitleValidatorSpec
-  class Validatable < OpenStruct
-    include ActiveModel::Validations
-    validates :title, quality_title: { unless: :private_message? }
-  end
-end
-
 RSpec.describe QualityTitleValidator do
   let(:valid_title) { "hello this is my cool topic! welcome: all;" }
   let(:short_title) { valid_title.slice(0, SiteSetting.min_topic_title_length - 1) }
   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(:meaningless_title) { "asdf asdf asdf asdf" }
   let(:loud_title) { "ALL CAPS INVALID TITLE" }
   let(:pretentious_title) { "superverylongwordintitlefornoparticularreason" }
+  fab!(:topic) { Fabricate(:post).topic }
 
-  subject(:topic) { QualityTitleValidatorSpec::Validatable.new }
-
-  before(:each) do
-    topic.stubs(private_message?: false)
+  before do
+    SiteSetting.title_prettify = false
   end
 
   it "allows a regular title with a few ascii characters" do
@@ -41,32 +31,34 @@ RSpec.describe QualityTitleValidator do
     expect(topic).to be_valid
   end
 
-  it "allows anything in a private message" do
-    topic.stubs(private_message?: true)
-    [short_title, long_title, xxxxx_title].each do |bad_title|
-      topic.title = bad_title
-      expect(topic).to be_valid
-    end
-  end
-
   it "strips a title when identifying length" do
     topic.title = short_title.center(SiteSetting.min_topic_title_length + 1, ' ')
     expect(topic).not_to be_valid
+    expect(topic.errors.full_messages.first).to include(
+      I18n.t("errors.messages.too_short", count: SiteSetting.min_topic_title_length)
+    )
   end
 
   it "doesn't allow a long title" do
     topic.title = long_title
     expect(topic).not_to be_valid
+    expect(topic.errors.full_messages.first).to include(
+      I18n.t("errors.messages.too_long", count: SiteSetting.max_topic_title_length)
+    )
   end
 
   it "doesn't allow a short title" do
     topic.title = short_title
     expect(topic).not_to be_valid
+    expect(topic.errors.full_messages.first).to include(
+      I18n.t("errors.messages.too_short", count: SiteSetting.min_topic_title_length)
+    )
   end
 
   it "doesn't allow a title of one repeated character" do
     topic.title = xxxxx_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 meaningless title" do
@@ -86,4 +78,22 @@ RSpec.describe QualityTitleValidator do
     expect(topic).not_to be_valid
     expect(topic.errors.full_messages.first).to include(I18n.t("errors.messages.is_invalid_quiet"))
   end
+
+  it "bypasses all checks for static docs if the acting user is admin" do
+    SiteSetting.tos_topic_id = topic.id
+    topic.acting_user = Fabricate(:admin)
+    [loud_title, pretentious_title, meaningless_title].each do |bad|
+      topic.title = bad
+      expect(topic).to be_valid
+    end
+  end
+
+  it "doesn't bypass all checks for static docs if the acting user isn't admin" do
+    SiteSetting.tos_topic_id = topic.id
+    topic.acting_user = Fabricate(:moderator)
+    [loud_title, pretentious_title, meaningless_title].each do |bad|
+      topic.title = bad
+      expect(topic).not_to be_valid
+    end
+  end
 end