Commit f554f4a3 authored by Lin Jen-Shin's avatar Lin Jen-Shin

Follow review feedback:

* Just use ClassMethods
* Ignore the value if variable_environment_scope is not available
* Use a partial
parent 09800138
......@@ -3,15 +3,13 @@ module EE
module Variable
extend ActiveSupport::Concern
module VariableClassMethods
module ClassMethods
def key_uniqueness_scope
%i[project_id environment_scope]
end
end
prepended do
singleton_class.prepend(VariableClassMethods)
validates(
:environment_scope,
presence: true,
......@@ -19,15 +17,16 @@ module EE
message: ::Gitlab::Regex.environment_scope_regex_message }
)
validate :validate_updating_environment_scope
before_save :verify_updating_environment_scope
private
def validate_updating_environment_scope
def verify_updating_environment_scope
return unless environment_scope_changed?
unless project.feature_available?(:variable_environment_scope)
errors.add(:environment_scope, 'is not enabled for this project')
# Ignore the changes to this value to mimic CE behaviour
self.environment_scope = environment_scope_was
end
end
end
......
......@@ -7,14 +7,7 @@
.form-group
= f.label :value, "Value", class: "label-light"
= f.text_area :value, class: "form-control", placeholder: "PROJECT_VARIABLE"
- if @project.feature_available?(:variable_environment_scope)
.form-group
= f.label :environment_scope, "Environment scope", class: "label-light"
= f.text_field :environment_scope, class: "form-control", placeholder: "*"
.help-block
This variable will be passed only to jobs with a matching environment name.
<code>*</code> is a wildcard that matches all environments (existing or not).
= link_to icon('question-circle'), help_page_path('ci/variables/README', anchor: 'limiting-environment-scopes-of-secret-variables'), target: '_blank'
= render 'projects/variables/ee/environment_scope', f: f
.form-group
.checkbox
= f.label :protected do
......
- if @project.feature_available?(:variable_environment_scope)
.form-group
= f.label :environment_scope, "Environment scope", class: "label-light"
= f.text_field :environment_scope, class: "form-control", placeholder: "*"
.help-block
This variable will be passed only to jobs with a matching environment name.
<code>*</code> is a wildcard that matches all environments (existing or not).
= link_to icon('question-circle'), help_page_path('ci/variables/README', anchor: 'limiting-environment-scopes-of-secret-variables'), target: '_blank'
......@@ -1422,7 +1422,7 @@ describe Ci::Build, :models do
.merge(project: project, environment_scope: 'stag*'))
# Skip this validation so that we could test for existing data
allow(variable).to receive(:validate_updating_environment_scope)
allow(variable).to receive(:verify_updating_environment_scope)
.and_return(true)
variable.save!
......
......@@ -20,6 +20,11 @@ describe Ci::Variable, models: true do
it { is_expected.to allow_value('*').for(:environment_scope) }
it { is_expected.to allow_value('review/*').for(:environment_scope) }
it { is_expected.not_to allow_value('').for(:environment_scope) }
it do
is_expected.to validate_uniqueness_of(:key)
.scoped_to(:project_id, :environment_scope)
end
end
# EE
......@@ -29,15 +34,14 @@ describe Ci::Variable, models: true do
end
it { is_expected.to allow_value('*').for(:environment_scope) }
it { is_expected.not_to allow_value('review/*').for(:environment_scope) }
end
let(:key_scope) do
[:project_id, :environment_scope] # EE
it 'ignores the changes to environment_scope' do
expect do
subject.update!(environment_scope: 'review/*')
end.not_to change { subject.environment_scope }
end
end
it { is_expected.to validate_uniqueness_of(:key).scoped_to(key_scope) }
describe '.unprotected' do
subject { described_class.unprotected }
......
......@@ -2296,7 +2296,7 @@ describe Project, models: true do
before do
# Skip this validation so that we could test for existing data
allow_any_instance_of(Ci::Variable)
.to receive(:validate_updating_environment_scope).and_return(true)
.to receive(:verify_updating_environment_scope).and_return(true)
end
shared_examples 'matching environment scope' 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