Add latest changes from gitlab-org/gitlab@13-5-stable-eev13.5.0-rc42

2 files changed, 159 insertions, 35 deletions

diff --git a/spec/models/operations/feature_flag_spec.rb b/spec/models/operations/feature_flag_spec.rb
index db432e73355..b4e941f2856 100644
--- a/spec/models/operations/feature_flag_spec.rb
+++ b/spec/models/operations/feature_flag_spec.rb
@@ -25,7 +25,7 @@ RSpec.describe Operations::FeatureFlag do
     context 'a version 1 feature flag' do
       it 'is valid if associated with Operations::FeatureFlagScope models' do
         project = create(:project)
-        feature_flag = described_class.create({ name: 'test', project: project, version: 1,
+        feature_flag = described_class.create!({ name: 'test', project: project, version: 1,
                                  scopes_attributes: [{ environment_scope: '*', active: false }] })
 
         expect(feature_flag).to be_valid
@@ -33,9 +33,10 @@ RSpec.describe Operations::FeatureFlag do
 
       it 'is invalid if associated with Operations::FeatureFlags::Strategy models' do
         project = create(:project)
-        feature_flag = described_class.create({ name: 'test', project: project, version: 1,
+        feature_flag = described_class.new({ name: 'test', project: project, version: 1,
                                  strategies_attributes: [{ name: 'default', parameters: {} }] })
 
+        expect(feature_flag.valid?).to eq(false)
         expect(feature_flag.errors.messages).to eq({
           version_associations: ["version 1 feature flags may not have strategies"]
         })
@@ -45,9 +46,10 @@ RSpec.describe Operations::FeatureFlag do
     context 'a version 2 feature flag' do
       it 'is invalid if associated with Operations::FeatureFlagScope models' do
         project = create(:project)
-        feature_flag = described_class.create({ name: 'test', project: project, version: 2,
+        feature_flag = described_class.new({ name: 'test', project: project, version: 2,
                                  scopes_attributes: [{ environment_scope: '*', active: false }] })
 
+        expect(feature_flag.valid?).to eq(false)
         expect(feature_flag.errors.messages).to eq({
           version_associations: ["version 2 feature flags may not have scopes"]
         })
@@ -55,7 +57,7 @@ RSpec.describe Operations::FeatureFlag do
 
       it 'is valid if associated with Operations::FeatureFlags::Strategy models' do
         project = create(:project)
-        feature_flag = described_class.create({ name: 'test', project: project, version: 2,
+        feature_flag = described_class.create!({ name: 'test', project: project, version: 2,
                                                 strategies_attributes: [{ name: 'default', parameters: {} }] })
 
         expect(feature_flag).to be_valid
@@ -75,7 +77,7 @@ RSpec.describe Operations::FeatureFlag do
     it 'defaults to 1 if unspecified' do
       project = create(:project)
 
-      feature_flag = described_class.create(name: 'my_flag', project: project, active: true)
+      feature_flag = described_class.create!(name: 'my_flag', project: project, active: true)
 
       expect(feature_flag).to be_valid
       expect(feature_flag.version_before_type_cast).to eq(1)
@@ -113,14 +115,14 @@ RSpec.describe Operations::FeatureFlag do
 
     context 'with a version 1 feature flag' do
       it 'creates a default scope' do
-        feature_flag = described_class.create({ name: 'test', project: project, scopes_attributes: [], version: 1 })
+        feature_flag = described_class.create!({ name: 'test', project: project, scopes_attributes: [], version: 1 })
 
         expect(feature_flag.scopes.count).to eq(1)
         expect(feature_flag.scopes.first.environment_scope).to eq('*')
       end
 
       it 'allows specifying the default scope in the parameters' do
-        feature_flag = described_class.create({ name: 'test', project: project,
+        feature_flag = described_class.create!({ name: 'test', project: project,
                                                 scopes_attributes: [{ environment_scope: '*', active: false },
                                                                     { environment_scope: 'review/*', active: true }], version: 1 })
 
@@ -131,7 +133,7 @@ RSpec.describe Operations::FeatureFlag do
 
     context 'with a version 2 feature flag' do
       it 'does not create a default scope' do
-        feature_flag = described_class.create({ name: 'test', project: project, scopes_attributes: [], version: 2 })
+        feature_flag = described_class.create!({ name: 'test', project: project, scopes_attributes: [], version: 2 })
 
         expect(feature_flag.scopes).to eq([])
       end
diff --git a/spec/models/operations/feature_flags/strategy_spec.rb b/spec/models/operations/feature_flags/strategy_spec.rb
index 04e3ef26e9d..0ecb49e75f3 100644
--- a/spec/models/operations/feature_flags/strategy_spec.rb
+++ b/spec/models/operations/feature_flags/strategy_spec.rb
@@ -4,11 +4,12 @@ require 'spec_helper'
 
 RSpec.describe Operations::FeatureFlags::Strategy do
   let_it_be(:project) { create(:project) }
+  let_it_be(:feature_flag) { create(:operations_feature_flag, project: project) }
 
   describe 'validations' do
     it do
       is_expected.to validate_inclusion_of(:name)
-        .in_array(%w[default gradualRolloutUserId userWithId gitlabUserList])
+        .in_array(%w[default gradualRolloutUserId flexibleRollout userWithId gitlabUserList])
         .with_message('strategy name is invalid')
     end
 
@@ -19,7 +20,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do
         end
         with_them do
           it 'skips parameters validation' do
-            feature_flag = create(:operations_feature_flag, project: project)
             strategy = described_class.create(feature_flag: feature_flag,
                                               name: invalid_name, parameters: { bad: 'params' })
 
@@ -36,7 +36,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do
         end
         with_them do
           it 'must have valid parameters for the strategy' do
-            feature_flag = create(:operations_feature_flag, project: project)
             strategy = described_class.create(feature_flag: feature_flag,
                                               name: 'gradualRolloutUserId', parameters: invalid_parameters)
 
@@ -45,7 +44,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do
         end
 
         it 'allows the parameters in any order' do
-          feature_flag = create(:operations_feature_flag, project: project)
           strategy = described_class.create(feature_flag: feature_flag,
                                             name: 'gradualRolloutUserId',
                                             parameters: { percentage: '10', groupId: 'mygroup' })
@@ -55,13 +53,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do
 
         describe 'percentage' do
           where(:invalid_value) do
-            [50, 40.0, { key: "value" }, "garbage", "00", "01", "101", "-1", "-10", "0100",
-             "1000", "10.0", "5%", "25%", "100hi", "e100", "30m", " ", "\r\n", "\n", "\t",
-             "\n10", "20\n", "\n100", "100\n", "\n  ", nil]
+            [50, 40.0, { key: "value" }, "garbage", "101", "-1", "-10", "1000", "10.0", "5%", "25%",
+             "100hi", "e100", "30m", " ", "\r\n", "\n", "\t", "\n10", "20\n", "\n100", "100\n",
+             "\n  ", nil]
           end
           with_them do
             it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do
-              feature_flag = create(:operations_feature_flag, project: project)
               strategy = described_class.create(feature_flag: feature_flag,
                                                 name: 'gradualRolloutUserId',
                                                 parameters: { groupId: 'mygroup', percentage: invalid_value })
@@ -75,7 +72,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do
           end
           with_them do
             it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do
-              feature_flag = create(:operations_feature_flag, project: project)
               strategy = described_class.create(feature_flag: feature_flag,
                                                 name: 'gradualRolloutUserId',
                                                 parameters: { groupId: 'mygroup', percentage: valid_value })
@@ -92,7 +88,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do
           end
           with_them do
             it 'must be a string value of up to 32 lowercase characters' do
-              feature_flag = create(:operations_feature_flag, project: project)
               strategy = described_class.create(feature_flag: feature_flag,
                                                 name: 'gradualRolloutUserId',
                                                 parameters: { groupId: invalid_value, percentage: '40' })
@@ -106,7 +101,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do
           end
           with_them do
             it 'must be a string value of up to 32 lowercase characters' do
-              feature_flag = create(:operations_feature_flag, project: project)
               strategy = described_class.create(feature_flag: feature_flag,
                                                 name: 'gradualRolloutUserId',
                                                 parameters: { groupId: valid_value, percentage: '40' })
@@ -117,13 +111,132 @@ RSpec.describe Operations::FeatureFlags::Strategy do
         end
       end
 
+      context 'when the strategy name is flexibleRollout' do
+        valid_parameters = { rollout: '40', groupId: 'mygroup', stickiness: 'DEFAULT' }
+        where(invalid_parameters: [
+          nil,
+          {},
+          *valid_parameters.to_a.combination(1).to_a.map { |p| p.to_h },
+          *valid_parameters.to_a.combination(2).to_a.map { |p| p.to_h },
+          { **valid_parameters, userIds: '4' },
+          { **valid_parameters, extra: nil }
+        ])
+        with_them do
+          it 'must have valid parameters for the strategy' do
+            strategy = described_class.create(feature_flag: feature_flag,
+                                              name: 'flexibleRollout',
+                                              parameters: invalid_parameters)
+
+            expect(strategy.errors[:parameters]).to eq(['parameters are invalid'])
+          end
+        end
+
+        [
+          [:rollout, '10'],
+          [:stickiness, 'DEFAULT'],
+          [:groupId, 'mygroup']
+        ].permutation(3).each do |parameters|
+          it "allows the parameters in the order #{parameters.map { |p| p.first }.join(', ')}" do
+            strategy = described_class.create(feature_flag: feature_flag,
+                                              name: 'flexibleRollout',
+                                              parameters: Hash[parameters])
+
+            expect(strategy.errors[:parameters]).to be_empty
+          end
+        end
+
+        describe 'rollout' do
+          where(invalid_value: [50, 40.0, { key: "value" }, "garbage", "101", "-1", " ", "-10",
+                                "1000", "10.0", "5%", "25%", "100hi", "e100", "30m", "\r\n",
+                                "\n", "\t", "\n10", "20\n", "\n100", "100\n", "\n  ", nil])
+          with_them do
+            it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do
+              parameters = { stickiness: 'DEFAULT', groupId: 'mygroup', rollout: invalid_value }
+              strategy = described_class.create(feature_flag: feature_flag,
+                                                name: 'flexibleRollout',
+                                                parameters: parameters)
+
+              expect(strategy.errors[:parameters]).to eq([
+                'rollout must be a string between 0 and 100 inclusive'
+              ])
+            end
+          end
+
+          where(valid_value: %w[0 1 10 38 100 93])
+          with_them do
+            it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do
+              parameters = { stickiness: 'DEFAULT', groupId: 'mygroup', rollout: valid_value }
+              strategy = described_class.create(feature_flag: feature_flag,
+                                                name: 'flexibleRollout',
+                                                parameters: parameters)
+
+              expect(strategy.errors[:parameters]).to eq([])
+            end
+          end
+        end
+
+        describe 'groupId' do
+          where(invalid_value: [nil, 4, 50.0, {}, 'spaces bad', 'bad$', '%bad', '<bad', 'bad>',
+                                '!bad', '.bad', 'Bad', 'bad1', "", " ", "b" * 33, "ba_d", "ba\nd"])
+          with_them do
+            it 'must be a string value of up to 32 lowercase characters' do
+              parameters = { stickiness: 'DEFAULT', groupId: invalid_value, rollout: '40' }
+              strategy = described_class.create(feature_flag: feature_flag,
+                                                name: 'flexibleRollout',
+                                                parameters: parameters)
+
+              expect(strategy.errors[:parameters]).to eq(['groupId parameter is invalid'])
+            end
+          end
+
+          where(valid_value: ["somegroup", "anothergroup", "okay", "g", "a" * 32])
+          with_them do
+            it 'must be a string value of up to 32 lowercase characters' do
+              parameters = { stickiness: 'DEFAULT', groupId: valid_value, rollout: '40' }
+              strategy = described_class.create(feature_flag: feature_flag,
+                                                name: 'flexibleRollout',
+                                                parameters: parameters)
+
+              expect(strategy.errors[:parameters]).to eq([])
+            end
+          end
+        end
+
+        describe 'stickiness' do
+          where(invalid_value: [nil, " ", "default", "DEFAULT\n", "UserId", "USER", "USERID "])
+          with_them do
+            it 'must be a string representing a supported stickiness setting' do
+              parameters = { stickiness: invalid_value, groupId: 'mygroup', rollout: '40' }
+              strategy = described_class.create(feature_flag: feature_flag,
+                                                name: 'flexibleRollout',
+                                                parameters: parameters)
+
+              expect(strategy.errors[:parameters]).to eq([
+                'stickiness parameter must be DEFAULT, USERID, SESSIONID, or RANDOM'
+              ])
+            end
+          end
+
+          where(valid_value: %w[DEFAULT USERID SESSIONID RANDOM])
+          with_them do
+            it 'must be a string representing a supported stickiness setting' do
+              parameters = { stickiness: valid_value, groupId: 'mygroup', rollout: '40' }
+              strategy = described_class.create(feature_flag: feature_flag,
+                                                name: 'flexibleRollout',
+                                                parameters: parameters)
+
+              expect(strategy.errors[:parameters]).to eq([])
+            end
+          end
+        end
+      end
+
       context 'when the strategy name is userWithId' do
         where(:invalid_parameters) do
           [nil, { userIds: 'sam', percentage: '40' }, { userIds: 'sam', some: 'param' }, { percentage: '40' }, {}]
         end
         with_them do
           it 'must have valid parameters for the strategy' do
-            feature_flag = create(:operations_feature_flag, project: project)
             strategy = described_class.create(feature_flag: feature_flag,
                                               name: 'userWithId', parameters: invalid_parameters)
 
@@ -140,7 +253,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do
           end
           with_them do
             it 'is valid with a string of comma separated values' do
-              feature_flag = create(:operations_feature_flag, project: project)
               strategy = described_class.create(feature_flag: feature_flag,
                                                 name: 'userWithId', parameters: { userIds: valid_value })
 
@@ -155,7 +267,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do
           end
           with_them do
             it 'is invalid' do
-              feature_flag = create(:operations_feature_flag, project: project)
               strategy = described_class.create(feature_flag: feature_flag,
                                                 name: 'userWithId', parameters: { userIds: invalid_value })
 
@@ -173,7 +284,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do
         end
         with_them do
           it 'must be empty' do
-            feature_flag = create(:operations_feature_flag, project: project)
             strategy = described_class.create(feature_flag: feature_flag,
                                               name: 'default',
                                               parameters: invalid_value)
@@ -183,7 +293,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do
         end
 
         it 'must be empty' do
-          feature_flag = create(:operations_feature_flag, project: project)
           strategy = described_class.create(feature_flag: feature_flag,
                                             name: 'default',
                                             parameters: {})
@@ -198,7 +307,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do
         end
         with_them do
           it 'must be empty' do
-            feature_flag = create(:operations_feature_flag, project: project)
             strategy = described_class.create(feature_flag: feature_flag,
                                               name: 'gitlabUserList',
                                               parameters: invalid_value)
@@ -208,7 +316,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do
         end
 
         it 'must be empty' do
-          feature_flag = create(:operations_feature_flag, project: project)
           strategy = described_class.create(feature_flag: feature_flag,
                                             name: 'gitlabUserList',
                                             parameters: {})
@@ -221,7 +328,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do
     describe 'associations' do
       context 'when name is gitlabUserList' do
         it 'is valid when associated with a user list' do
-          feature_flag = create(:operations_feature_flag, project: project)
           user_list = create(:operations_feature_flag_user_list, project: project)
           strategy = described_class.create(feature_flag: feature_flag,
                                             name: 'gitlabUserList',
@@ -232,7 +338,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do
         end
 
         it 'is invalid without a user list' do
-          feature_flag = create(:operations_feature_flag, project: project)
           strategy = described_class.create(feature_flag: feature_flag,
                                             name: 'gitlabUserList',
                                             parameters: {})
@@ -242,7 +347,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do
 
         it 'is invalid when associated with a user list from another project' do
           other_project = create(:project)
-          feature_flag = create(:operations_feature_flag, project: project)
           user_list = create(:operations_feature_flag_user_list, project: other_project)
           strategy = described_class.create(feature_flag: feature_flag,
                                             name: 'gitlabUserList',
@@ -255,7 +359,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do
 
       context 'when name is default' do
         it 'is invalid when associated with a user list' do
-          feature_flag = create(:operations_feature_flag, project: project)
           user_list = create(:operations_feature_flag_user_list, project: project)
           strategy = described_class.create(feature_flag: feature_flag,
                                             name: 'default',
@@ -266,7 +369,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do
         end
 
         it 'is valid without a user list' do
-          feature_flag = create(:operations_feature_flag, project: project)
           strategy = described_class.create(feature_flag: feature_flag,
                                             name: 'default',
                                             parameters: {})
@@ -277,7 +379,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do
 
       context 'when name is userWithId' do
         it 'is invalid when associated with a user list' do
-          feature_flag = create(:operations_feature_flag, project: project)
           user_list = create(:operations_feature_flag_user_list, project: project)
           strategy = described_class.create(feature_flag: feature_flag,
                                             name: 'userWithId',
@@ -288,7 +389,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do
         end
 
         it 'is valid without a user list' do
-          feature_flag = create(:operations_feature_flag, project: project)
           strategy = described_class.create(feature_flag: feature_flag,
                                             name: 'userWithId',
                                             parameters: { userIds: 'user1' })
@@ -299,7 +399,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do
 
       context 'when name is gradualRolloutUserId' do
         it 'is invalid when associated with a user list' do
-          feature_flag = create(:operations_feature_flag, project: project)
           user_list = create(:operations_feature_flag_user_list, project: project)
           strategy = described_class.create(feature_flag: feature_flag,
                                             name: 'gradualRolloutUserId',
@@ -310,7 +409,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do
         end
 
         it 'is valid without a user list' do
-          feature_flag = create(:operations_feature_flag, project: project)
           strategy = described_class.create(feature_flag: feature_flag,
                                             name: 'gradualRolloutUserId',
                                             parameters: { groupId: 'default', percentage: '10' })
@@ -318,6 +416,30 @@ RSpec.describe Operations::FeatureFlags::Strategy do
           expect(strategy.errors[:user_list]).to be_empty
         end
       end
+
+      context 'when name is flexibleRollout' do
+        it 'is invalid when associated with a user list' do
+          user_list = create(:operations_feature_flag_user_list, project: project)
+          strategy = described_class.create(feature_flag: feature_flag,
+                                            name: 'flexibleRollout',
+                                            user_list: user_list,
+                                            parameters: { groupId: 'default',
+                                                          rollout: '10',
+                                                          stickiness: 'DEFAULT' })
+
+          expect(strategy.errors[:user_list]).to eq(['must be blank'])
+        end
+
+        it 'is valid without a user list' do
+          strategy = described_class.create(feature_flag: feature_flag,
+                                            name: 'flexibleRollout',
+                                            parameters: { groupId: 'default',
+                                                          rollout: '10',
+                                                          stickiness: 'DEFAULT' })
+
+          expect(strategy.errors[:user_list]).to be_empty
+        end
+      end
     end
   end
 end