From e31471585a6101ebcbe728f7fa51fa9c0d6a6c8c Mon Sep 17 00:00:00 2001 From: Bernhard Suttner Date: Thu, 18 Jun 2020 17:19:47 +0200 Subject: [PATCH] DEV: allow to have duplicate topic titles if categegory is different (#10034) Co-authored-by: Robin Ward Co-authored-by: Robin Ward --- app/models/topic.rb | 7 +++++- config/locales/server.en.yml | 1 + config/site_settings.yml | 1 + lib/validators/unique_among_validator.rb | 2 +- script/import_scripts/base.rb | 1 + spec/models/topic_spec.rb | 32 +++++++++++++++++++++--- 6 files changed, 38 insertions(+), 6 deletions(-) diff --git a/app/models/topic.rb b/app/models/topic.rb index 49d14cbf229..a26998897df 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -155,7 +155,12 @@ class Topic < ActiveRecord::Base message: :has_already_been_used, allow_blank: true, case_sensitive: false, - collection: Proc.new { Topic.listable_topics } } + collection: Proc.new { |t| + SiteSetting.allow_duplicate_topic_titles_category? ? + Topic.listable_topics.where("category_id = ?", t.category_id) : + Topic.listable_topics + } + } validates :category_id, presence: true, diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 5d79648f50b..5a3bc5ada40 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1449,6 +1449,7 @@ en: category_search_priority_very_high_weight: "Weight applied to ranking for very high category search priority." allow_uncategorized_topics: "Allow topics to be created without a category. WARNING: If there are any uncategorized topics, you must recategorize them before turning this off." allow_duplicate_topic_titles: "Allow topics with identical, duplicate titles." + allow_duplicate_topic_titles_category: "Allow topics with identical, duplicate titles if the category is different. allow_duplicate_topic_titles must be false." unique_posts_mins: "How many minutes before a user can make a post with the same content again" educate_until_posts: "When the user starts typing their first (n) new posts, show the pop-up new user education panel in the composer." title: "The name of this site, as used in the title tag." diff --git a/config/site_settings.yml b/config/site_settings.yml index 2948b8793f2..502d71cbc11 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -716,6 +716,7 @@ posting: default: true refresh: true allow_duplicate_topic_titles: false + allow_duplicate_topic_titles_category: false min_title_similar_length: client: true default: 10 diff --git a/lib/validators/unique_among_validator.rb b/lib/validators/unique_among_validator.rb index 82fed5c206c..ed3d1420145 100644 --- a/lib/validators/unique_among_validator.rb +++ b/lib/validators/unique_among_validator.rb @@ -12,7 +12,7 @@ class UniqueAmongValidator < ActiveRecord::Validations::UniquenessValidator # do nothing further unless there were some duplicates. unless new_errors == 0 # now look only in the collection we care about. - dupes = options[:collection].call.where("lower(#{attribute}) = ?", value.downcase) + dupes = options[:collection].call(record).where("lower(#{attribute}) = ?", value.downcase) dupes = dupes.where("id != ?", record.id) if record.persisted? # pop off the error, if it was a false positive diff --git a/script/import_scripts/base.rb b/script/import_scripts/base.rb index a42b8c3dda9..64de94ba75f 100644 --- a/script/import_scripts/base.rb +++ b/script/import_scripts/base.rb @@ -77,6 +77,7 @@ class ImportScripts::Base min_personal_message_post_length: 1, min_personal_message_title_length: 1, allow_duplicate_topic_titles: true, + allow_duplicate_topic_titles_category: false, disable_emails: 'yes', max_attachment_size_kb: 102400, max_image_size_kb: 102400, diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index d1d5d1cc911..ead96218b2d 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -255,19 +255,27 @@ describe Topic do end context 'topic title uniqueness' do + let!(:category1) { Fabricate(:category) } + let!(:category2) { Fabricate(:category) } - let!(:topic) { Fabricate(:topic) } - let(:new_topic) { Fabricate.build(:topic, title: topic.title) } + let!(:topic) { Fabricate(:topic, category: category1) } + let(:new_topic) { Fabricate.build(:topic, title: topic.title, category: category1) } + let(:new_topic_different_cat) { Fabricate.build(:topic, title: topic.title, category: category2) } context "when duplicates aren't allowed" do before do - SiteSetting.expects(:allow_duplicate_topic_titles?).returns(false) + SiteSetting.allow_duplicate_topic_titles = false + SiteSetting.allow_duplicate_topic_titles_category = false end it "won't allow another topic to be created with the same name" do expect(new_topic).not_to be_valid end + it "won't even allow another topic to be created with the same name but different category" do + expect(new_topic_different_cat).not_to be_valid + end + it "won't allow another topic with an upper case title to be created" do new_topic.title = new_topic.title.upcase expect(new_topic).not_to be_valid @@ -286,7 +294,8 @@ describe Topic do context "when duplicates are allowed" do before do - SiteSetting.expects(:allow_duplicate_topic_titles?).returns(true) + SiteSetting.allow_duplicate_topic_titles = true + SiteSetting.allow_duplicate_topic_titles_category = false end it "will allow another topic to be created with the same name" do @@ -294,6 +303,21 @@ describe Topic do end end + context "when duplicates are allowed if the category is different" do + before do + SiteSetting.allow_duplicate_topic_titles = false + SiteSetting.allow_duplicate_topic_titles_category = true + end + + it "will allow another topic to be created with the same name but different category" do + expect(new_topic_different_cat).to be_valid + end + + it "won't allow another topic to be created with the same name in same category" do + expect(new_topic).not_to be_valid + end + end + end context 'html in title' do