Add latest changes from gitlab-org/gitlab@14-5-stable-eev14.5.0-rc42

2 files changed, 160 insertions, 130 deletions

diff --git a/spec/models/operations/feature_flags/strategy_spec.rb b/spec/models/operations/feature_flags/strategy_spec.rb
index 9289e3beab5..de1b9d2c855 100644
--- a/spec/models/operations/feature_flags/strategy_spec.rb
+++ b/spec/models/operations/feature_flags/strategy_spec.rb
@@ -20,8 +20,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do
         end
         with_them do
           it 'skips parameters validation' do
-            strategy = described_class.create(feature_flag: feature_flag,
-                                              name: invalid_name, parameters: { bad: 'params' })
+            strategy = build(:operations_strategy,
+                             feature_flag: feature_flag,
+                             name: invalid_name,
+                             parameters: { bad: 'params' })
+
+            expect(strategy).to be_invalid
 
             expect(strategy.errors[:name]).to eq(['strategy name is invalid'])
             expect(strategy.errors[:parameters]).to be_empty
@@ -36,19 +40,24 @@ RSpec.describe Operations::FeatureFlags::Strategy do
         end
         with_them do
           it 'must have valid parameters for the strategy' do
-            strategy = described_class.create(feature_flag: feature_flag,
-                                              name: 'gradualRolloutUserId', parameters: invalid_parameters)
+            strategy = build(:operations_strategy,
+                             :gradual_rollout,
+                             feature_flag: feature_flag,
+                             parameters: invalid_parameters)
+
+            expect(strategy).to be_invalid
 
             expect(strategy.errors[:parameters]).to eq(['parameters are invalid'])
           end
         end
 
         it 'allows the parameters in any order' do
-          strategy = described_class.create(feature_flag: feature_flag,
-                                            name: 'gradualRolloutUserId',
-                                            parameters: { percentage: '10', groupId: 'mygroup' })
+          strategy = build(:operations_strategy,
+                           :gradual_rollout,
+                           feature_flag: feature_flag,
+                           parameters: { percentage: '10', groupId: 'mygroup' })
 
-          expect(strategy.errors[:parameters]).to be_empty
+          expect(strategy).to be_valid
         end
 
         describe 'percentage' do
@@ -59,9 +68,12 @@ 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
-              strategy = described_class.create(feature_flag: feature_flag,
-                                                name: 'gradualRolloutUserId',
-                                                parameters: { groupId: 'mygroup', percentage: invalid_value })
+              strategy = build(:operations_strategy,
+                               :gradual_rollout,
+                               feature_flag: feature_flag,
+                               parameters: { groupId: 'mygroup', percentage: invalid_value })
+
+              expect(strategy).to be_invalid
 
               expect(strategy.errors[:parameters]).to eq(['percentage must be a string between 0 and 100 inclusive'])
             end
@@ -72,11 +84,12 @@ 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
-              strategy = described_class.create(feature_flag: feature_flag,
-                                                name: 'gradualRolloutUserId',
-                                                parameters: { groupId: 'mygroup', percentage: valid_value })
+              strategy = build(:operations_strategy,
+                               :gradual_rollout,
+                               feature_flag: feature_flag,
+                               parameters: { groupId: 'mygroup', percentage: valid_value })
 
-              expect(strategy.errors[:parameters]).to eq([])
+              expect(strategy).to be_valid
             end
           end
         end
@@ -88,9 +101,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do
           end
           with_them do
             it 'must be a string value of up to 32 lowercase characters' do
-              strategy = described_class.create(feature_flag: feature_flag,
-                                                name: 'gradualRolloutUserId',
-                                                parameters: { groupId: invalid_value, percentage: '40' })
+              strategy = build(:operations_strategy,
+                               :gradual_rollout,
+                               feature_flag: feature_flag,
+                               parameters: { groupId: invalid_value, percentage: '40' })
+
+              expect(strategy).to be_invalid
 
               expect(strategy.errors[:parameters]).to eq(['groupId parameter is invalid'])
             end
@@ -101,11 +117,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do
           end
           with_them do
             it 'must be a string value of up to 32 lowercase characters' do
-              strategy = described_class.create(feature_flag: feature_flag,
-                                                name: 'gradualRolloutUserId',
-                                                parameters: { groupId: valid_value, percentage: '40' })
+              strategy = build(:operations_strategy,
+                               :gradual_rollout,
+                               feature_flag: feature_flag,
+                               parameters: { groupId: valid_value, percentage: '40' })
 
-              expect(strategy.errors[:parameters]).to eq([])
+              expect(strategy).to be_valid
             end
           end
         end
@@ -123,9 +140,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do
         ])
         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)
+            strategy = build(:operations_strategy,
+                             :flexible_rollout,
+                             feature_flag: feature_flag,
+                             parameters: invalid_parameters)
+
+            expect(strategy).to be_invalid
 
             expect(strategy.errors[:parameters]).to eq(['parameters are invalid'])
           end
@@ -137,11 +157,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do
           [: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])
+            strategy = build(:operations_strategy,
+                             :flexible_rollout,
+                             feature_flag: feature_flag,
+                             parameters: Hash[parameters])
 
-            expect(strategy.errors[:parameters]).to be_empty
+            expect(strategy).to be_valid
           end
         end
 
@@ -152,9 +173,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do
           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)
+              strategy = build(:operations_strategy,
+                               :flexible_rollout,
+                               feature_flag: feature_flag,
+                               parameters: parameters)
+
+              expect(strategy).to be_invalid
 
               expect(strategy.errors[:parameters]).to eq([
                 'rollout must be a string between 0 and 100 inclusive'
@@ -166,11 +190,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do
           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)
+              strategy = build(:operations_strategy,
+                               :flexible_rollout,
+                               feature_flag: feature_flag,
+                               parameters: parameters)
 
-              expect(strategy.errors[:parameters]).to eq([])
+              expect(strategy).to be_valid
             end
           end
         end
@@ -181,9 +206,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do
           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)
+              strategy = build(:operations_strategy,
+                               :flexible_rollout,
+                               feature_flag: feature_flag,
+                               parameters: parameters)
+
+              expect(strategy).to be_invalid
 
               expect(strategy.errors[:parameters]).to eq(['groupId parameter is invalid'])
             end
@@ -193,11 +221,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do
           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)
+              strategy = build(:operations_strategy,
+                               :flexible_rollout,
+                               feature_flag: feature_flag,
+                               parameters: parameters)
 
-              expect(strategy.errors[:parameters]).to eq([])
+              expect(strategy).to be_valid
             end
           end
         end
@@ -207,9 +236,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do
           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)
+              strategy = build(:operations_strategy,
+                               :flexible_rollout,
+                               feature_flag: feature_flag,
+                               parameters: parameters)
+
+              expect(strategy).to be_invalid
 
               expect(strategy.errors[:parameters]).to eq([
                 'stickiness parameter must be default, userId, sessionId, or random'
@@ -221,11 +253,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do
           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)
+              strategy = build(:operations_strategy,
+                               :flexible_rollout,
+                               feature_flag: feature_flag,
+                               parameters: parameters)
 
-              expect(strategy.errors[:parameters]).to eq([])
+              expect(strategy).to be_valid
             end
           end
         end
@@ -237,8 +270,11 @@ RSpec.describe Operations::FeatureFlags::Strategy do
         end
         with_them do
           it 'must have valid parameters for the strategy' do
-            strategy = described_class.create(feature_flag: feature_flag,
-                                              name: 'userWithId', parameters: invalid_parameters)
+            strategy = build(:operations_strategy,
+                             feature_flag: feature_flag,
+                             name: 'userWithId', parameters: invalid_parameters)
+
+            expect(strategy).to be_invalid
 
             expect(strategy.errors[:parameters]).to eq(['parameters are invalid'])
           end
@@ -253,10 +289,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do
           end
           with_them do
             it 'is valid with a string of comma separated values' do
-              strategy = described_class.create(feature_flag: feature_flag,
-                                                name: 'userWithId', parameters: { userIds: valid_value })
+              strategy = build(:operations_strategy,
+                               feature_flag: feature_flag,
+                               name: 'userWithId',
+                               parameters: { userIds: valid_value })
 
-              expect(strategy.errors[:parameters]).to be_empty
+              expect(strategy).to be_valid
             end
           end
 
@@ -267,8 +305,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do
           end
           with_them do
             it 'is invalid' do
-              strategy = described_class.create(feature_flag: feature_flag,
-                                                name: 'userWithId', parameters: { userIds: invalid_value })
+              strategy = build(:operations_strategy,
+                               feature_flag: feature_flag,
+                               name: 'userWithId',
+                               parameters: { userIds: invalid_value })
+
+              expect(strategy).to be_invalid
 
               expect(strategy.errors[:parameters]).to include(
                 'userIds must be a string of unique comma separated values each 256 characters or less'
@@ -284,43 +326,48 @@ RSpec.describe Operations::FeatureFlags::Strategy do
         end
         with_them do
           it 'must be empty' do
-            strategy = described_class.create(feature_flag: feature_flag,
-                                              name: 'default',
-                                              parameters: invalid_value)
+            strategy = build(:operations_strategy, :default, feature_flag: feature_flag, parameters: invalid_value)
+
+            expect(strategy).to be_invalid
 
             expect(strategy.errors[:parameters]).to eq(['parameters are invalid'])
           end
         end
 
         it 'must be empty' do
-          strategy = described_class.create(feature_flag: feature_flag,
-                                            name: 'default',
-                                            parameters: {})
+          strategy = build(:operations_strategy, :default, feature_flag: feature_flag)
 
-          expect(strategy.errors[:parameters]).to be_empty
+          expect(strategy).to be_valid
         end
       end
 
       context 'when the strategy name is gitlabUserList' do
+        let_it_be(:user_list) { create(:operations_feature_flag_user_list, project: project) }
+
         where(:invalid_value) do
           [{ groupId: "default", percentage: "7" }, "", "nothing", 7, nil, [], 2.5, { userIds: 'user1' }]
         end
         with_them do
-          it 'must be empty' do
-            strategy = described_class.create(feature_flag: feature_flag,
-                                              name: 'gitlabUserList',
-                                              parameters: invalid_value)
+          it 'is invalid' do
+            strategy = build(:operations_strategy,
+                             :gitlab_userlist,
+                             user_list: user_list,
+                             feature_flag: feature_flag,
+                             parameters: invalid_value)
+
+            expect(strategy).to be_invalid
 
             expect(strategy.errors[:parameters]).to eq(['parameters are invalid'])
           end
         end
 
-        it 'must be empty' do
-          strategy = described_class.create(feature_flag: feature_flag,
-                                            name: 'gitlabUserList',
-                                            parameters: {})
+        it 'is valid' do
+          strategy = build(:operations_strategy,
+                           :gitlab_userlist,
+                           user_list: user_list,
+                           feature_flag: feature_flag)
 
-          expect(strategy.errors[:parameters]).to be_empty
+          expect(strategy).to be_valid
         end
       end
     end
@@ -329,18 +376,15 @@ RSpec.describe Operations::FeatureFlags::Strategy do
       context 'when name is gitlabUserList' do
         it 'is valid 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: 'gitlabUserList',
-                                            user_list: user_list,
-                                            parameters: {})
+          strategy = build(:operations_strategy, :gitlab_userlist, feature_flag: feature_flag, user_list: user_list)
 
-          expect(strategy.errors[:user_list]).to be_empty
+          expect(strategy).to be_valid
         end
 
         it 'is invalid without a user list' do
-          strategy = described_class.create(feature_flag: feature_flag,
-                                            name: 'gitlabUserList',
-                                            parameters: {})
+          strategy = build(:operations_strategy, :gitlab_userlist, feature_flag: feature_flag, user_list: nil)
+
+          expect(strategy).to be_invalid
 
           expect(strategy.errors[:user_list]).to eq(["can't be blank"])
         end
@@ -348,10 +392,9 @@ RSpec.describe Operations::FeatureFlags::Strategy do
         it 'is invalid when associated with a user list from another project' do
           other_project = create(:project)
           user_list = create(:operations_feature_flag_user_list, project: other_project)
-          strategy = described_class.create(feature_flag: feature_flag,
-                                            name: 'gitlabUserList',
-                                            user_list: user_list,
-                                            parameters: {})
+          strategy = build(:operations_strategy, :gitlab_userlist, feature_flag: feature_flag, user_list: user_list)
+
+          expect(strategy).to be_invalid
 
           expect(strategy.errors[:user_list]).to eq(['must belong to the same project'])
         end
@@ -360,84 +403,68 @@ RSpec.describe Operations::FeatureFlags::Strategy do
       context 'when name is default' 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: 'default',
-                                            user_list: user_list,
-                                            parameters: {})
+          strategy = build(:operations_strategy, :default, feature_flag: feature_flag, user_list: user_list)
+
+          expect(strategy).to be_invalid
 
           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: 'default',
-                                            parameters: {})
+          strategy = build(:operations_strategy, :default, feature_flag: feature_flag)
 
-          expect(strategy.errors[:user_list]).to be_empty
+          expect(strategy).to be_valid
         end
       end
 
       context 'when name is userWithId' 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: 'userWithId',
-                                            user_list: user_list,
-                                            parameters: { userIds: 'user1' })
+          strategy = build(:operations_strategy, :userwithid, feature_flag: feature_flag, user_list: user_list)
+
+          expect(strategy).to be_invalid
 
           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: 'userWithId',
-                                            parameters: { userIds: 'user1' })
+          strategy = build(:operations_strategy, :userwithid, feature_flag: feature_flag)
 
-          expect(strategy.errors[:user_list]).to be_empty
+          expect(strategy).to be_valid
         end
       end
 
       context 'when name is gradualRolloutUserId' 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: 'gradualRolloutUserId',
-                                            user_list: user_list,
-                                            parameters: { groupId: 'default', percentage: '10' })
+          strategy = build(:operations_strategy, :gradual_rollout, feature_flag: feature_flag, user_list: user_list)
+
+          expect(strategy).to be_invalid
 
           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: 'gradualRolloutUserId',
-                                            parameters: { groupId: 'default', percentage: '10' })
+          strategy = build(:operations_strategy, :gradual_rollout, feature_flag: feature_flag)
 
-          expect(strategy.errors[:user_list]).to be_empty
+          expect(strategy).to be_valid
         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' })
+          strategy = build(:operations_strategy, :flexible_rollout, feature_flag: feature_flag, user_list: user_list)
+
+          expect(strategy).to be_invalid
 
           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' })
+          strategy = build(:operations_strategy, :flexible_rollout, feature_flag: feature_flag)
 
-          expect(strategy.errors[:user_list]).to be_empty
+          expect(strategy).to be_valid
         end
       end
     end
diff --git a/spec/models/operations/feature_flags/user_list_spec.rb b/spec/models/operations/feature_flags/user_list_spec.rb
index 3a48d3389a3..b2dbebb2c0d 100644
--- a/spec/models/operations/feature_flags/user_list_spec.rb
+++ b/spec/models/operations/feature_flags/user_list_spec.rb
@@ -20,9 +20,9 @@ RSpec.describe Operations::FeatureFlags::UserList do
       end
       with_them do
         it 'is valid with a string of comma separated values' do
-          user_list = described_class.create(user_xids: valid_value)
+          user_list = build(:operations_feature_flag_user_list, user_xids: valid_value)
 
-          expect(user_list.errors[:user_xids]).to be_empty
+          expect(user_list).to be_valid
         end
       end
 
@@ -31,9 +31,10 @@ RSpec.describe Operations::FeatureFlags::UserList do
       end
       with_them do
         it 'automatically casts values of other types' do
-          user_list = described_class.create(user_xids: typecast_value)
+          user_list = build(:operations_feature_flag_user_list, user_xids: typecast_value)
+
+          expect(user_list).to be_valid
 
-          expect(user_list.errors[:user_xids]).to be_empty
           expect(user_list.user_xids).to eq(typecast_value.to_s)
         end
       end
@@ -45,7 +46,9 @@ RSpec.describe Operations::FeatureFlags::UserList do
       end
       with_them do
         it 'is invalid' do
-          user_list = described_class.create(user_xids: invalid_value)
+          user_list = build(:operations_feature_flag_user_list, user_xids: invalid_value)
+
+          expect(user_list).to be_invalid
 
           expect(user_list.errors[:user_xids]).to include(
             'user_xids must be a string of unique comma separated values each 256 characters or less'
@@ -70,20 +73,20 @@ RSpec.describe Operations::FeatureFlags::UserList do
   describe '#destroy' do
     it 'deletes the model if it is not associated with any feature flag strategies' do
       project = create(:project)
-      user_list = described_class.create(project: project, name: 'My User List', user_xids: 'user1,user2')
+      user_list = described_class.create!(project: project, name: 'My User List', user_xids: 'user1,user2')
 
-      user_list.destroy
+      user_list.destroy!
 
       expect(described_class.count).to eq(0)
     end
 
     it 'does not delete the model if it is associated with a feature flag strategy' do
       project = create(:project)
-      user_list = described_class.create(project: project, name: 'My User List', user_xids: 'user1,user2')
+      user_list = described_class.create!(project: project, name: 'My User List', user_xids: 'user1,user2')
       feature_flag = create(:operations_feature_flag, :new_version_flag, project: project)
       strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'gitlabUserList', user_list: user_list)
 
-      user_list.destroy
+      user_list.destroy # rubocop:disable Rails/SaveBang
 
       expect(described_class.count).to eq(1)
       expect(::Operations::FeatureFlags::StrategyUserList.count).to eq(1)