Merge branch 'mc/fix/project-variables-scope' into 'master'

Resolve "Project variables validate without any scopes disregarding environment_scope"

Closes #43191

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

3 files changed, 70 insertions, 2 deletions

diff --git a/spec/fixtures/api/schemas/variable.json b/spec/fixtures/api/schemas/variable.json
index 78977118b0a..6f6b044115b 100644
--- a/spec/fixtures/api/schemas/variable.json
+++ b/spec/fixtures/api/schemas/variable.json
@@ -10,7 +10,8 @@
     "id": { "type": "integer" },
     "key": { "type": "string" },
     "value": { "type": "string" },
-    "protected": { "type": "boolean" }
+    "protected": { "type": "boolean" },
+    "environment_scope": { "type": "string", "optional": true }
   },
   "additionalProperties": false
 }
diff --git a/spec/support/features/variable_list_shared_examples.rb b/spec/support/features/variable_list_shared_examples.rb
index 4315bf5d037..0d8f7a7aae6 100644
--- a/spec/support/features/variable_list_shared_examples.rb
+++ b/spec/support/features/variable_list_shared_examples.rb
@@ -263,7 +263,7 @@ shared_examples 'variable list' do
 
     # 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 Duplicate variables: samekey')
+      expect(find('.js-ci-variable-error-box')).to have_content(/Validation failed Variables have duplicate values \(.+\)/)
     end
   end
 end
diff --git a/spec/validators/variable_duplicates_validator_spec.rb b/spec/validators/variable_duplicates_validator_spec.rb
new file mode 100644
index 00000000000..0b71a67f94d
--- /dev/null
+++ b/spec/validators/variable_duplicates_validator_spec.rb
@@ -0,0 +1,67 @@
+require 'spec_helper'
+
+describe VariableDuplicatesValidator do
+  let(:validator) { described_class.new(attributes: [:variables], **options) }
+
+  describe '#validate_each' do
+    let(:project) { build(:project) }
+
+    subject { validator.validate_each(project, :variables, project.variables) }
+
+    context 'with no scope' do
+      let(:options) { {} }
+      let(:variables) { build_list(:ci_variable, 2, project: project) }
+
+      before do
+        project.variables << variables
+      end
+
+      it 'does not have any errors' do
+        subject
+
+        expect(project.errors.empty?).to be true
+      end
+
+      context 'with duplicates' do
+        before do
+          project.variables.build(key: variables.first.key, value: 'dummy_value')
+        end
+
+        it 'has a duplicate key error' do
+          subject
+
+          expect(project.errors).to have_key(:variables)
+        end
+      end
+    end
+
+    context 'with a scope attribute' do
+      let(:options) { { scope: :environment_scope } }
+      let(:first_variable) { build(:ci_variable, key: 'test_key', environment_scope: '*', project: project) }
+      let(:second_variable) { build(:ci_variable, key: 'test_key', environment_scope: 'prod', project: project) }
+
+      before do
+        project.variables << first_variable
+        project.variables << second_variable
+      end
+
+      it 'does not have any errors' do
+        subject
+
+        expect(project.errors.empty?).to be true
+      end
+
+      context 'with duplicates' do
+        before do
+          project.variables.build(key: second_variable.key, value: 'dummy_value', environment_scope: second_variable.environment_scope)
+        end
+
+        it 'has a duplicate key error' do
+          subject
+
+          expect(project.errors).to have_key(:variables)
+        end
+      end
+    end
+  end
+end