Handling filtering environment_scope in controller/API

app/controllers/ee/projects/variables_controller.rb

+module EE
+  module Projects
+    module VariablesController
+      extend ActiveSupport::Concern
+
+      def variable_params_attributes
+        attrs = super
+
+        attrs.unshift(:environment_scope) if
+            project.feature_available?(:variable_environment_scope)
+
+        attrs
+      end
+    end
+  end
+end

app/controllers/projects/variables_controller.rb

 class Projects::VariablesController < Projects::ApplicationController
+  prepend ::EE::Projects::VariablesController
+
   before_action :authorize_admin_build!
 
   layout 'project_settings'
@@ -44,15 +46,10 @@ class Projects::VariablesController < Projects::ApplicationController
   private
 
   def variable_params
-    params.require(:variable)
-      .permit(variable_params_ce.concat(variable_params_ee))
+    params.require(:variable).permit(*variable_params_attributes)
   end
 
-  def variable_params_ce
+  def variable_params_attributes
     %i[id key value protected _destroy]
   end
-
-  def variable_params_ee
-    %i[environment_scope]
-  end
 end

app/models/ee/ci/variable.rb

@@ -10,19 +10,6 @@ module EE
           format: { with: ::Gitlab::Regex.environment_scope_regex,
                     message: ::Gitlab::Regex.environment_scope_regex_message }
         )
-
-        before_save :verify_updating_environment_scope
-      end
-
-      private
-
-      def verify_updating_environment_scope
-        return unless environment_scope_changed?
-
-        unless project.feature_available?(:variable_environment_scope)
-          # Ignore the changes to this value to mimic CE behaviour
-          self.environment_scope = environment_scope_was
-        end
       end
     end
   end

app/models/license.rb

@@ -15,7 +15,7 @@ class License < ActiveRecord::Base
   OBJECT_STORAGE_FEATURE = 'GitLab_ObjectStorage'.freeze
   RELATED_ISSUES_FEATURE = 'RelatedIssues'.freeze
   SERVICE_DESK_FEATURE = 'GitLab_ServiceDesk'.freeze
-  VARIABLE_ENVIRONMENT_SCOPE_FEATURE = 'GitLab_VARIABLE_ENVIRONMENT_SCOPE'.freeze
+  VARIABLE_ENVIRONMENT_SCOPE_FEATURE = 'VariableEnvironmentScope'.freeze
 
   FEATURE_CODES = {
     auditor_user: AUDITOR_USER_FEATURE,

lib/api/variables.rb

@@ -48,7 +48,13 @@ module API
         optional :environment_scope, type: String, desc: 'The environment_scope of the variable'
       end
       post ':id/variables' do
-        variable = user_project.variables.create(declared_params(include_missing: false))
+        variable_params = declared_params(include_missing: false)
+
+        # EE
+        variable_params.delete(:environment_scope) unless
+            user_project.feature_available?(:variable_environment_scope)
+
+        variable = user_project.variables.create(variable_params)
 
         if variable.valid?
           present variable, with: Entities::Variable
@@ -73,7 +79,13 @@ module API
 
         return not_found!('Variable') unless variable
 
-        if variable.update(declared_params(include_missing: false).except(:key))
+        variable_params = declared_params(include_missing: false).except(:key)
+
+        # EE
+        variable_params.delete(:environment_scope) unless
+            user_project.feature_available?(:variable_environment_scope)
+
+        if variable.update(variable_params)
           present variable, with: Entities::Variable
         else
           render_validation_error!(variable)

spec/models/ee/ci/build_spec.rb

@@ -85,10 +85,6 @@ describe Ci::Build, models: true do
                 environment_varialbe.slice(:key, :value)
                   .merge(project: project, environment_scope: 'stag*'))
 
-        # Skip this validation so that we could test for existing data
-        allow(variable).to receive(:verify_updating_environment_scope)
-          .and_return(true)
-
         variable.save!
       end
 

spec/models/ee/ci/variable_spec.rb

@@ -3,32 +3,12 @@ require 'spec_helper'
 describe Ci::Variable, models: true do
   subject { build(:ci_variable) }
 
-  context 'when variable_environment_scope available' do
-    before do
-      stub_licensed_features(variable_environment_scope: true)
-    end
+  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 { 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
-
-  context 'when variable_environment_scope unavailable' do
-    before do
-      stub_licensed_features(variable_environment_scope: false)
-    end
-
-    it { is_expected.to allow_value('*').for(:environment_scope) }
-
-    it 'ignores the changes to environment_scope' do
-      expect do
-        subject.update!(environment_scope: 'review/*')
-      end.not_to change { subject.environment_scope }
-    end
+  it do
+    is_expected.to validate_uniqueness_of(:key)
+      .scoped_to(:project_id, :environment_scope)
   end
 end

spec/models/ee/project_spec.rb

@@ -320,12 +320,6 @@ describe Project, models: true do
         project.secret_variables_for(ref: 'ref', environment: environment)
       end
 
-      before do
-        # Skip this validation so that we could test for existing data
-        allow_any_instance_of(EE::Ci::Variable)
-          .to receive(:verify_updating_environment_scope).and_return(true)
-      end
-
       shared_examples 'matching environment scope' do
         context 'when variable environment scope is available' do
           before do