Commit c98cb46f authored by Jonas Wälter's avatar Jonas Wälter Committed by charlie ablett

Allow only topic names that are case insensitive unique

Changelog: changed
parent 4d9be902
...@@ -2826,7 +2826,9 @@ class Project < ApplicationRecord ...@@ -2826,7 +2826,9 @@ class Project < ApplicationRecord
if @topic_list != self.topic_list if @topic_list != self.topic_list
self.topics.delete_all 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 end
@topic_list = nil @topic_list = nil
......
...@@ -7,7 +7,8 @@ module Projects ...@@ -7,7 +7,8 @@ module Projects
include Avatarable include Avatarable
include Gitlab::SQL::Pattern 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 } validates :description, length: { maximum: 1024 }
has_many :project_topics, class_name: 'Projects::ProjectTopic' has_many :project_topics, class_name: 'Projects::ProjectTopic'
......
# 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
5bb52cc70aada72e0e569006fd05de0c0d7629559d78bfd361009c91482f02cf
\ No newline at end of file
...@@ -27975,6 +27975,8 @@ CREATE UNIQUE INDEX index_token_with_ivs_on_hashed_token ON token_with_ivs USING ...@@ -27975,6 +27975,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_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 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); CREATE INDEX index_topics_on_name_trigram ON topics USING gin (name gin_trgm_ops);
...@@ -88,6 +88,13 @@ RSpec.describe Admin::TopicsController do ...@@ -88,6 +88,13 @@ RSpec.describe Admin::TopicsController do
expect(errors).to contain_exactly(errors.full_message(:name, I18n.t('errors.messages.blank'))) expect(errors).to contain_exactly(errors.full_message(:name, I18n.t('errors.messages.blank')))
end 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 context 'as a normal user' do
before do before do
sign_in(user) sign_in(user)
...@@ -116,6 +123,15 @@ RSpec.describe Admin::TopicsController do ...@@ -116,6 +123,15 @@ RSpec.describe Admin::TopicsController do
expect(errors).to contain_exactly(errors.full_message(:name, I18n.t('errors.messages.blank'))) expect(errors).to contain_exactly(errors.full_message(:name, I18n.t('errors.messages.blank')))
end 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 context 'as a normal user' do
before do before do
sign_in(user) sign_in(user)
......
...@@ -7489,6 +7489,14 @@ RSpec.describe Project, factory_default: :keep do ...@@ -7489,6 +7489,14 @@ RSpec.describe Project, factory_default: :keep do
expect(project.save).to be_falsy expect(project.save).to be_falsy
expect(project.reload.topics.map(&:name)).to eq(%w[topic1 topic2 topic3]) expect(project.reload.topics.map(&:name)).to eq(%w[topic1 topic2 topic3])
end 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 end
context 'public topics counter' do context 'public topics counter' do
......
...@@ -22,7 +22,7 @@ RSpec.describe Projects::Topic do ...@@ -22,7 +22,7 @@ RSpec.describe Projects::Topic do
describe 'validations' do describe 'validations' do
it { is_expected.to validate_presence_of(:name) } 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(:name).is_at_most(255) }
it { is_expected.to validate_length_of(:description).is_at_most(1024) } it { is_expected.to validate_length_of(:description).is_at_most(1024) }
end end
......
...@@ -142,6 +142,13 @@ RSpec.describe API::Topics do ...@@ -142,6 +142,13 @@ RSpec.describe API::Topics do
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['error']).to eql('name is missing') expect(json_response['error']).to eql('name is missing')
end 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 end
context 'as normal user' do context 'as normal user' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment