Merge branch '82-add-new-onboarding-to-flipper-a-b-testing' into 'master'

Add new onboarding to flipper A/B testing

Closes growth#82

See merge request gitlab-org/gitlab-ee!14424

app/controllers/dashboard/projects_controller.rb

@@ -3,6 +3,7 @@
 class Dashboard::ProjectsController < Dashboard::ApplicationController
   include ParamsBackwardCompatibility
   include RendersMemberAccess
+  include OnboardingExperimentHelper
 
   prepend_before_action(only: [:index]) { authenticate_sessionless_user!(:rss) }
   before_action :set_non_archived_param

app/helpers/onboarding_experiment_helper.rb

+# frozen_string_literal: true
+
+module OnboardingExperimentHelper
+  def allow_access_to_onboarding?
+    ::Gitlab.com? && Feature.enabled?(:user_onboarding)
+  end
+end
+
+OnboardingExperimentHelper.prepend(EE::OnboardingExperimentHelper)

ee/app/controllers/ee/dashboard/projects_controller.rb

@@ -5,23 +5,26 @@ module EE
     module ProjectsController
       extend ActiveSupport::Concern
       extend ::Gitlab::Utils::Override
+      include ::OnboardingExperimentHelper
 
       private
 
       override :render_projects
       def render_projects
-        if show_onboarding_welcome_page?
-          redirect_to explore_onboarding_index_path
-        else
-          super
-        end
+        return redirect_to explore_onboarding_index_path if show_onboarding_welcome_page?
+
+        super
       end
 
       def show_onboarding_welcome_page?
-        return false unless ::Gitlab.com?
-        return false if cookies['onboarding_dismissed'] == 'true'
+        return false if onboarding_cookie_set?
+        return false unless allow_access_to_onboarding?
+
+        !show_projects?(projects, params)
+      end
 
-        ::Feature.enabled?(:user_onboarding) && !show_projects?(projects, params)
+      def onboarding_cookie_set?
+        cookies['onboarding_dismissed'] == 'true'
       end
     end
   end

ee/app/controllers/explore/onboarding_controller.rb

 # frozen_string_literal: true
 
 class Explore::OnboardingController < Explore::ApplicationController
+  include ::OnboardingExperimentHelper
+
   before_action :authenticate_user!
   before_action :set_project!
 
@@ -17,8 +19,6 @@ class Explore::OnboardingController < Explore::ApplicationController
   end
 
   def get_onboarding_demo_project
-    if Gitlab.com? && Feature.enabled?(:user_onboarding)
-      Project.find_by_full_path("gitlab-org/gitlab-ce")
-    end
+    Project.find_by_full_path("gitlab-org/gitlab-ce") if allow_access_to_onboarding?
   end
 end

ee/app/helpers/ee/application_helper.rb

@@ -3,6 +3,8 @@
 module EE
   module ApplicationHelper
     extend ::Gitlab::Utils::Override
+    include ::OnboardingExperimentHelper
+
     DB_LAG_SHOW_THRESHOLD = 60 # seconds
     LOG_CURSOR_CHECK_TIME = ::Gitlab::Geo::LogCursor::Daemon::SECONDARY_CHECK_INTERVAL
     EVENT_PROCESSING_TIME = 60.seconds
@@ -118,7 +120,7 @@ module EE
     end
 
     def user_onboarding_enabled?
-      ::Gitlab.com? && ::Feature.enabled?(:user_onboarding)
+      allow_access_to_onboarding?
     end
 
     private

ee/app/helpers/ee/onboarding_experiment_helper.rb

+# frozen_string_literal: true
+
+module EE
+  module OnboardingExperimentHelper
+    extend ::Gitlab::Utils::Override
+
+    EXPERIMENT_GROWTH_ONBOARDING_FEATURE_NAME = :experiment_growth_onboarding
+
+    override :allow_access_to_onboarding?
+    def allow_access_to_onboarding?
+      super && experiment_enabled_for_user?
+    end
+
+    private
+
+    def experiment_enabled_for_user?
+      return true unless current_user
+      # If EXPERIMENT_GROWTH_ONBOARDING_FEATURE_NAME is not set, we should return
+      # true which means available for all
+      return true unless onboarding_sign_up_experiment_set?
+
+      ::Feature.enabled?(EXPERIMENT_GROWTH_ONBOARDING_FEATURE_NAME, current_user)
+    end
+
+    def onboarding_sign_up_experiment_set?
+      ::Feature.persisted?(onboarding_sign_up_experiment_feature)
+    end
+
+    def onboarding_sign_up_experiment_feature
+      ::Feature.get(EXPERIMENT_GROWTH_ONBOARDING_FEATURE_NAME)
+    end
+  end
+end

ee/spec/helpers/ee/onboarding_experiment_helper_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe OnboardingExperimentHelper, type: :helper do
+  using RSpec::Parameterized::TableSyntax
+
+  describe '.allow_access_to_onboarding?' do
+    context "when we're not gitlab.com" do
+      it 'returns false' do
+        allow(::Gitlab).to receive(:com?).and_return(false)
+
+        expect(helper.allow_access_to_onboarding?).to be(false)
+      end
+    end
+
+    context "when we're gitlab.com" do
+      before do
+        allow(::Gitlab).to receive(:com?).and_return(true)
+      end
+
+      context 'and the :user_onboarding feature is not enabled' do
+        it 'returns false' do
+          stub_feature_flags(user_onboarding: false)
+
+          expect(helper.allow_access_to_onboarding?).to be(false)
+        end
+      end
+
+      context 'and the :user_onboarding feature is enabled' do
+        before do
+          stub_feature_flags(user_onboarding: true)
+        end
+
+        context 'but there is no current_user' do
+          it 'returns true' do
+            allow(helper).to receive(:current_user).and_return(nil)
+
+            expect(helper.allow_access_to_onboarding?).to be(true)
+          end
+        end
+
+        context 'and there is a current_user' do
+          let!(:user) { create(:user, id: 2) }
+
+          before do
+            allow(helper).to receive(:current_user).and_return(user)
+          end
+
+          context 'but experiment_growth_onboarding has not been set' do
+            it 'returns true' do
+              expect(helper.allow_access_to_onboarding?).to be(true)
+            end
+          end
+
+          context 'and experiment_growth_onboarding has been set' do
+            it 'checks if feature is enabled for current_user' do
+              percentage = ::Feature.flipper.actors(50)
+              ::Feature.flipper[described_class::EXPERIMENT_GROWTH_ONBOARDING_FEATURE_NAME].enable(percentage)
+
+              expect(Feature).to receive(:enabled?).with(described_class::EXPERIMENT_GROWTH_ONBOARDING_FEATURE_NAME, user).and_call_original
+              expect(helper.allow_access_to_onboarding?).to eq(true)
+            end
+          end
+        end
+      end
+    end
+  end
+end

spec/helpers/onboarding_experiment_helper_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe OnboardingExperimentHelper, type: :helper do
+  describe '.allow_access_to_onboarding?' do
+    context "when we're not gitlab.com" do
+      it 'returns false' do
+        allow(::Gitlab).to receive(:com?).and_return(false)
+
+        expect(helper.allow_access_to_onboarding?).to be(false)
+      end
+    end
+
+    context "when we're gitlab.com" do
+      before do
+        allow(::Gitlab).to receive(:com?).and_return(true)
+      end
+
+      context 'and the :user_onboarding feature is not enabled' do
+        it 'returns false' do
+          stub_feature_flags(user_onboarding: false)
+
+          expect(helper.allow_access_to_onboarding?).to be(false)
+        end
+      end
+
+      context 'and the :user_onboarding feature is enabled' do
+        it 'returns true' do
+          stub_feature_flags(user_onboarding: true)
+          allow(helper).to receive(:current_user).and_return(create(:user))
+
+          expect(helper.allow_access_to_onboarding?).to be(true)
+        end
+      end
+    end
+  end
+end