Merge branch 'topics/name-unique-case-insensitive' into 'master'

Topics: allow only names that are case insensitive unique

See merge request gitlab-org/gitlab!79826

app/models/project.rb

@@ -2826,7 +2826,9 @@ class Project < ApplicationRecord
 
     if @topic_list != self.topic_list
       self.topics.delete_all
-      self.topics = @topic_list.map { |topic| Projects::Topic.find_or_create_by(name: topic) }
+      self.topics = @topic_list.map do |topic|
+        Projects::Topic.where('lower(name) = ?', topic.downcase).order(total_projects_count: :desc).first_or_create(name: topic)
+      end
     end
 
     @topic_list = nil

app/models/projects/topic.rb

@@ -7,7 +7,8 @@ module Projects
     include Avatarable
     include Gitlab::SQL::Pattern
 
-    validates :name, presence: true, uniqueness: true, length: { maximum: 255 }
+    validates :name, presence: true, length: { maximum: 255 }
+    validates :name, uniqueness: { case_sensitive: false }, if: :name_changed?
     validates :description, length: { maximum: 1024 }
 
     has_many :project_topics, class_name: 'Projects::ProjectTopic'

db/migrate/20220203074916_add_topics_lower_name_index.rb

+# frozen_string_literal: true
+
+class AddTopicsLowerNameIndex < Gitlab::Database::Migration[1.0]
+  INDEX_NAME = 'index_topics_on_lower_name'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :topics, 'lower(name)', name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :topics, INDEX_NAME
+  end
+end

db/schema_migrations/20220203074916

+5bb52cc70aada72e0e569006fd05de0c0d7629559d78bfd361009c91482f02cf
\ No newline at end of file

db/structure.sql

@@ -28023,6 +28023,8 @@ CREATE UNIQUE INDEX index_token_with_ivs_on_hashed_token ON token_with_ivs USING
 
 CREATE INDEX index_topics_non_private_projects_count ON topics USING btree (non_private_projects_count DESC, id);
 
+CREATE INDEX index_topics_on_lower_name ON topics USING btree (lower(name));
+
 CREATE UNIQUE INDEX index_topics_on_name ON topics USING btree (name);
 
 CREATE INDEX index_topics_on_name_trigram ON topics USING gin (name gin_trgm_ops);

spec/controllers/admin/topics_controller_spec.rb

@@ -88,6 +88,13 @@ RSpec.describe Admin::TopicsController do
       expect(errors).to contain_exactly(errors.full_message(:name, I18n.t('errors.messages.blank')))
     end
 
+    it 'shows error message if topic not unique (case insensitive)' do
+      post :create, params: { projects_topic: { name: topic.name.upcase } }
+
+      errors = assigns[:topic].errors
+      expect(errors).to contain_exactly(errors.full_message(:name, I18n.t('errors.messages.taken')))
+    end
+
     context 'as a normal user' do
       before do
         sign_in(user)
@@ -116,6 +123,15 @@ RSpec.describe Admin::TopicsController do
       expect(errors).to contain_exactly(errors.full_message(:name, I18n.t('errors.messages.blank')))
     end
 
+    it 'shows error message if topic not unique (case insensitive)' do
+      other_topic = create(:topic, name: 'other-topic')
+
+      put :update, params: { id: topic.id, projects_topic: { name: other_topic.name.upcase } }
+
+      errors = assigns[:topic].errors
+      expect(errors).to contain_exactly(errors.full_message(:name, I18n.t('errors.messages.taken')))
+    end
+
     context 'as a normal user' do
       before do
         sign_in(user)

spec/models/project_spec.rb

@@ -7504,6 +7504,14 @@ RSpec.describe Project, factory_default: :keep do
         expect(project.save).to be_falsy
         expect(project.reload.topics.map(&:name)).to eq(%w[topic1 topic2 topic3])
       end
+
+      it 'does not add new topic if name is not unique (case insensitive)' do
+        project.topic_list = 'topic1, TOPIC2, topic3'
+
+        project.save!
+
+        expect(project.reload.topics.map(&:name)).to eq(%w[topic1 topic2 topic3])
+      end
     end
 
     context 'public topics counter' do

spec/models/projects/topic_spec.rb

@@ -22,7 +22,7 @@ RSpec.describe Projects::Topic do
 
   describe 'validations' do
     it { is_expected.to validate_presence_of(:name) }
-    it { is_expected.to validate_uniqueness_of(:name) }
+    it { is_expected.to validate_uniqueness_of(:name).case_insensitive }
     it { is_expected.to validate_length_of(:name).is_at_most(255) }
     it { is_expected.to validate_length_of(:description).is_at_most(1024) }
   end

spec/requests/api/topics_spec.rb

@@ -142,6 +142,13 @@ RSpec.describe API::Topics do
         expect(response).to have_gitlab_http_status(:bad_request)
         expect(json_response['error']).to eql('name is missing')
       end
+
+      it 'returns 400 if name is not unique (case insensitive)' do
+        post api('/topics/', admin), params: { name: topic_1.name.downcase }
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+        expect(json_response['message']['name']).to eq(['has already been taken'])
+      end
     end
 
     context 'as normal user' do