Merge branch 'aa-feature-gate-filter' into 'master'

Add feature filter types for users

See merge request gitlab-org/gitlab!24774

db/migrate/20200209131152_add_feature_filter_type_to_user_preferences.rb

+# frozen_string_literal: true
+
+class AddFeatureFilterTypeToUserPreferences < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def change
+    add_column :user_preferences, :feature_filter_type, :bigint
+  end
+end

db/schema.rb

@@ -4182,6 +4182,7 @@ ActiveRecord::Schema.define(version: 2020_02_12_052620) do
     t.boolean "setup_for_company"
     t.boolean "render_whitespace_in_code"
     t.integer "tab_width", limit: 2
+    t.bigint "feature_filter_type"
     t.index ["user_id"], name: "index_user_preferences_on_user_id", unique: true
   end
 

ee/app/helpers/ee/groups_helper.rb

@@ -83,12 +83,14 @@ module EE
     end
 
     def show_discover_group_security?(group)
-      !!::Feature.enabled?(:discover_security) &&
-        ::Gitlab.com? &&
+      security_feature_available_at = DateTime.new(2020, 1, 20)
+
       !!current_user &&
-        current_user.created_at > DateTime.new(2020, 1, 20) &&
+        ::Gitlab.com? &&
+        current_user.created_at > security_feature_available_at &&
         !@group.feature_available?(:security_dashboard) &&
-        can?(current_user, :admin_group, @group)
+        can?(current_user, :admin_group, @group) &&
+        current_user.ab_feature_enabled?(:discover_security)
     end
 
     private

ee/app/helpers/ee/projects_helper.rb

@@ -259,12 +259,14 @@ module EE
     end
 
     def show_discover_project_security?(project)
-      !!::Feature.enabled?(:discover_security) &&
-        ::Gitlab.com? &&
+      security_feature_available_at = DateTime.new(2020, 1, 20)
+
       !!current_user &&
-        current_user.created_at > DateTime.new(2020, 1, 20) &&
+        ::Gitlab.com? &&
+        current_user.created_at > security_feature_available_at &&
         !project.feature_available?(:security_dashboard) &&
-        can?(current_user, :admin_namespace, project.root_ancestor)
+        can?(current_user, :admin_namespace, project.root_ancestor) &&
+        current_user.ab_feature_enabled?(:discover_security)
     end
 
     def settings_operations_available?

ee/app/models/ee/user.rb

@@ -306,6 +306,29 @@ module EE
       super
     end
 
+    def ab_feature_enabled?(feature, percentage: nil)
+      return false unless ::Gitlab.com?
+      return false if ::Gitlab::Geo.secondary?
+
+      raise "Currently only discover_security feature is supported" unless feature == :discover_security
+
+      return false unless ::Feature.enabled?(feature)
+
+      filter = user_preference.feature_filter_type.presence || 0
+
+      # We use a 2nd feature flag for control as enabled and percentage_of_time for chatops
+      flipper_feature = ::Feature.get((feature.to_s + '_control').to_sym)
+      percentage ||= flipper_feature.gate_values[:percentage_of_time] || 0 if flipper_feature
+      return false if percentage <= 0
+
+      if filter == UserPreference::FEATURE_FILTER_UNKNOWN
+        filter = SecureRandom.rand * 100 <= percentage ? UserPreference::FEATURE_FILTER_EXPERIMENT : UserPreference::FEATURE_FILTER_CONTROL
+        user_preference.update_column :feature_filter_type, filter
+      end
+
+      filter == UserPreference::FEATURE_FILTER_EXPERIMENT
+    end
+
     protected
 
     override :password_required?

ee/app/models/ee/user_preference.rb

@@ -11,5 +11,9 @@ module EE
 
       validates :epic_notes_filter, inclusion: { in: ::UserPreference::NOTES_FILTERS.values }, presence: true
     end
+
+    FEATURE_FILTER_UNKNOWN = 0
+    FEATURE_FILTER_CONTROL = 1
+    FEATURE_FILTER_EXPERIMENT = 2
   end
 end

ee/changelogs/unreleased/aa-feature-filter-user.yml

+---
+title: Add feature filter for users
+merge_request: 24765
+author:
+type: added

ee/spec/helpers/ee/groups_helper_spec.rb

@@ -96,6 +96,7 @@ describe GroupsHelper do
     using RSpec::Parameterized::TableSyntax
 
     where(
+      ab_feature_enabled?: [true, false],
       gitlab_com?: [true, false],
       user?: [true, false],
       created_at: [Time.mktime(2010, 1, 20), Time.mktime(2030, 1, 20)],
@@ -106,15 +107,16 @@ describe GroupsHelper do
 
     with_them do
       it 'returns the expected value' do
-        allow(::Gitlab).to receive(:com?) { gitlab_com? }
         allow(helper).to receive(:current_user) { user? ? user : nil }
+        allow(::Gitlab).to receive(:com?) { gitlab_com? }
+        allow(user).to receive(:ab_feature_enabled?) { ab_feature_enabled? }
         allow(user).to receive(:created_at) { created_at }
         allow(::Feature).to receive(:enabled?).with(:discover_security) { discover_security_feature_enabled? }
         allow(group).to receive(:feature_available?) { security_dashboard_feature_available? }
         allow(helper).to receive(:can?) { can_admin_group? }
 
-        expected_value = gitlab_com? && user? && created_at > DateTime.new(2020, 1, 20) &&
-                         discover_security_feature_enabled? && !security_dashboard_feature_available? && can_admin_group?
+        expected_value = user? && created_at > DateTime.new(2020, 1, 20) && gitlab_com? &&
+                         ab_feature_enabled? && !security_dashboard_feature_available? && can_admin_group?
 
         expect(helper.show_discover_group_security?(group)).to eq(expected_value)
       end

ee/spec/helpers/projects_helper_spec.rb

@@ -193,10 +193,10 @@ describe ProjectsHelper do
     let(:user) { create(:user) }
 
     where(
+      ab_feature_enabled?: [true, false],
       gitlab_com?: [true, false],
        user?: [true, false],
       created_at: [Time.mktime(2010, 1, 20), Time.mktime(2030, 1, 20)],
-      discover_security_feature_enabled?: [true, false],
       security_dashboard_feature_available?: [true, false],
       can_admin_namespace?: [true, false]
     )
@@ -204,14 +204,14 @@ describe ProjectsHelper do
     with_them do
       it 'returns the expected value' do
         allow(::Gitlab).to receive(:com?) { gitlab_com? }
+        allow(user).to receive(:ab_feature_enabled?) { ab_feature_enabled? }
         allow(helper).to receive(:current_user) { user? ? user : nil }
         allow(user).to receive(:created_at) { created_at }
-        allow(::Feature).to receive(:enabled?).with(:discover_security) { discover_security_feature_enabled? }
         allow(project).to receive(:feature_available?) { security_dashboard_feature_available? }
         allow(helper).to receive(:can?) { can_admin_namespace? }
 
-        expected_value = gitlab_com? && user? && created_at > DateTime.new(2020, 1, 20) &&
-                         discover_security_feature_enabled? && !security_dashboard_feature_available? && can_admin_namespace?
+        expected_value = user? && created_at > DateTime.new(2020, 1, 20) && gitlab_com? &&
+                         ab_feature_enabled? && !security_dashboard_feature_available? && can_admin_namespace?
 
         expect(helper.show_discover_project_security?(project)).to eq(expected_value)
       end

ee/spec/models/user_spec.rb

@@ -826,4 +826,114 @@ describe User do
       end
     end
   end
+
+  describe '#ab_feature_enabled?' do
+    let(:experiment_user) { create(:user) }
+    let(:new_user) { create(:user) }
+    let(:new_fresh_user) { create(:user) }
+    let(:control_user) { create(:user) }
+    let(:users_of_different_groups) { [experiment_user, new_user, new_fresh_user, control_user] }
+
+    before do
+      create(:user_preference, user: experiment_user, feature_filter_type: UserPreference::FEATURE_FILTER_EXPERIMENT)
+      create(:user_preference, user: new_user, feature_filter_type: UserPreference::FEATURE_FILTER_UNKNOWN)
+      create(:user_preference, user: new_fresh_user, feature_filter_type: nil)
+      create(:user_preference, user: control_user, feature_filter_type: UserPreference::FEATURE_FILTER_CONTROL)
+    end
+
+    context 'when not on Gitlab.com' do
+      before do
+        allow(Gitlab).to receive(:com?).and_return(false)
+      end
+
+      it 'returns false' do
+        users_of_different_groups.each do |user|
+          expect(user.ab_feature_enabled?(:discover_security, percentage: 100)).to eq(false)
+        end
+      end
+    end
+
+    context 'when on Gitlab.com' do
+      before do
+        allow(Gitlab).to receive(:com?).and_return(true)
+      end
+
+      context 'when on a secondary Geo' do
+        before do
+          allow(Gitlab::Geo).to receive(:secondary?).and_return(true)
+        end
+
+        it 'returns false' do
+          users_of_different_groups.each do |user|
+            expect(user.ab_feature_enabled?(:discover_security, percentage: 100)).to eq(false)
+          end
+        end
+      end
+
+      context 'when not on a secondary Geo' do
+        before do
+          allow(Gitlab::Geo).to receive(:secondary?).and_return(false)
+        end
+
+        context 'for any feature except discover_security' do
+          it 'raises runtime error' do
+            users_of_different_groups.each do |user|
+              expect do
+                user.ab_feature_enabled?(:any_other_feature, percentage: 100)
+              end.to raise_error(RuntimeError, 'Currently only discover_security feature is supported')
+            end
+          end
+        end
+
+        context 'when discover_security feature flag is disabled' do
+          before do
+            stub_feature_flags(discover_security: false)
+          end
+
+          it 'returns false' do
+            users_of_different_groups.each do |user|
+              expect(user.ab_feature_enabled?(:discover_security, percentage: 100)).to eq(false)
+            end
+          end
+        end
+
+        context 'when discover_security feature flag is enabled' do
+          it 'returns false when in control group' do
+            expect(control_user.ab_feature_enabled?(:discover_security, percentage: 100)).to eq(false)
+          end
+
+          it 'returns true for experiment group' do
+            expect(experiment_user.ab_feature_enabled?(:discover_security, percentage: 100)).to eq(true)
+          end
+
+          it 'assigns to control or experiment group when feature_filter_type is nil' do
+            new_fresh_user.ab_feature_enabled?(:discover_security, percentage: 100)
+
+            expect(new_fresh_user.user_preference.feature_filter_type).not_to eq(UserPreference::FEATURE_FILTER_UNKNOWN)
+          end
+
+          it 'assigns to control or experiment group when feature_filter_type is zero' do
+            new_user.ab_feature_enabled?(:discover_security, percentage: 100)
+
+            expect(new_user.user_preference.feature_filter_type).not_to eq(UserPreference::FEATURE_FILTER_UNKNOWN)
+          end
+
+          it 'returns false for zero percentage' do
+            expect(experiment_user.ab_feature_enabled?(:discover_security, percentage: 0)).to eq(false)
+          end
+
+          it 'returns false when no percentage is provided' do
+            expect(experiment_user.ab_feature_enabled?(:discover_security)).to eq(false)
+          end
+
+          it 'returns true when 100% control percentage is provided' do
+            Feature.get(:discover_security_control).enable_percentage_of_time(100)
+
+            expect(experiment_user.ab_feature_enabled?(:discover_security)).to eq(true)
+            expect(experiment_user.user_preference.feature_filter_type).to eq(UserPreference::FEATURE_FILTER_EXPERIMENT)
+          end
+        end
+      end
+    end
+  end
 end