Merge branch '43275-improve-variables-validation-message' into 'master'

Resolve "Improve Variables validation message"

Closes #43275

See merge request gitlab-org/gitlab-ce!17135

7 files changed, 19 insertions, 4 deletions

diff --git a/app/models/ci/group_variable.rb b/app/models/ci/group_variable.rb
index afeae69ba39..1dd0e050ba9 100644
--- a/app/models/ci/group_variable.rb
+++ b/app/models/ci/group_variable.rb
@@ -6,7 +6,10 @@ module Ci
 
     belongs_to :group
 
-    validates :key, uniqueness: { scope: :group_id }
+    validates :key, uniqueness: {
+      scope: :group_id,
+      message: "(%{value}) has already been taken"
+    }
 
     scope :unprotected, -> { where(protected: false) }
   end
diff --git a/app/models/ci/variable.rb b/app/models/ci/variable.rb
index 67d3ec81b6f..7c71291de84 100644
--- a/app/models/ci/variable.rb
+++ b/app/models/ci/variable.rb
@@ -6,7 +6,10 @@ module Ci
 
     belongs_to :project
 
-    validates :key, uniqueness: { scope: [:project_id, :environment_scope] }
+    validates :key, uniqueness: {
+      scope: [:project_id, :environment_scope],
+      message: "(%{value}) has already been taken"
+    }
 
     scope :unprotected, -> { where(protected: false) }
   end
diff --git a/app/validators/variable_duplicates_validator.rb b/app/validators/variable_duplicates_validator.rb
index 4bfa3c45303..72660be6c43 100644
--- a/app/validators/variable_duplicates_validator.rb
+++ b/app/validators/variable_duplicates_validator.rb
@@ -5,6 +5,8 @@
 # - Use `validates :xxx, uniqueness: { scope: :xxx_id }` in a child model
 class VariableDuplicatesValidator < ActiveModel::EachValidator
   def validate_each(record, attribute, value)
+    return if record.errors.include?(:"#{attribute}.key")
+
     if options[:scope]
       scoped = value.group_by do |variable|
         Array(options[:scope]).map { |attr| variable.send(attr) } # rubocop:disable GitlabSecurity/PublicSend
diff --git a/changelogs/unreleased/43275-improve-variables-validation-message.yml b/changelogs/unreleased/43275-improve-variables-validation-message.yml
new file mode 100644
index 00000000000..88ef93123a0
--- /dev/null
+++ b/changelogs/unreleased/43275-improve-variables-validation-message.yml
@@ -0,0 +1,5 @@
+---
+title: Remove duplicated error message on duplicate variable validation
+merge_request: 17135
+author:
+type: fixed
diff --git a/spec/models/ci/group_variable_spec.rb b/spec/models/ci/group_variable_spec.rb
index 145189e7469..1b10501701c 100644
--- a/spec/models/ci/group_variable_spec.rb
+++ b/spec/models/ci/group_variable_spec.rb
@@ -5,7 +5,7 @@ describe Ci::GroupVariable do
 
   it { is_expected.to include_module(HasVariable) }
   it { is_expected.to include_module(Presentable) }
-  it { is_expected.to validate_uniqueness_of(:key).scoped_to(:group_id) }
+  it { is_expected.to validate_uniqueness_of(:key).scoped_to(:group_id).with_message(/\(\w+\) has already been taken/) }
 
   describe '.unprotected' do
     subject { described_class.unprotected }
diff --git a/spec/models/ci/variable_spec.rb b/spec/models/ci/variable_spec.rb
index e4ff551151e..875e8b2b682 100644
--- a/spec/models/ci/variable_spec.rb
+++ b/spec/models/ci/variable_spec.rb
@@ -6,7 +6,7 @@ describe Ci::Variable do
   describe 'validations' do
     it { is_expected.to include_module(HasVariable) }
     it { is_expected.to include_module(Presentable) }
-    it { is_expected.to validate_uniqueness_of(:key).scoped_to(:project_id, :environment_scope) }
+    it { is_expected.to validate_uniqueness_of(:key).scoped_to(:project_id, :environment_scope).with_message(/\(\w+\) has already been taken/) }
   end
 
   describe '.unprotected' do
diff --git a/spec/support/features/variable_list_shared_examples.rb b/spec/support/features/variable_list_shared_examples.rb
index 0d8f7a7aae6..f7f851eb1eb 100644
--- a/spec/support/features/variable_list_shared_examples.rb
+++ b/spec/support/features/variable_list_shared_examples.rb
@@ -261,6 +261,8 @@ shared_examples 'variable list' do
     click_button('Save variables')
     wait_for_requests
 
+    expect(all('.js-ci-variable-list-section .js-ci-variable-error-box ul li').count).to eq(1)
+
     # We check the first row because it re-sorts to alphabetical order on refresh
     page.within('.js-ci-variable-list-section') do
       expect(find('.js-ci-variable-error-box')).to have_content(/Validation failed Variables have duplicate values \(.+\)/)