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:

ba27ee1637/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.
This commit is contained in:
Osama Sayegh 2022-10-04 21:55:21 +03:00 committed by GitHub
parent cf646b2061
commit 2d391565e4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 34 additions and 22 deletions

View File

@ -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?

View File

@ -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