Merge branch...

Merge branch '337534-experiment-continuous-onboarding-iteration-2-change-destination-urls' into 'master'

Continuous onboarding experiment iteration 2: change destination URLs

See merge request gitlab-org/gitlab!71408

app/helpers/learn_gitlab_helper.rb

@@ -10,7 +10,14 @@ module LearnGitlabHelper
   def onboarding_actions_data(project)
     attributes = onboarding_progress(project).attributes.symbolize_keys
 
-    action_urls.to_h do |action, url|
+    urls_to_use = nil
+
+    experiment(:change_continuous_onboarding_link_urls) do |e|
+      e.use { urls_to_use = action_urls }
+      e.try { urls_to_use = new_action_urls(project) }
+    end
+
+    urls_to_use.to_h do |action, url|
       [
         action,
         url: url,
@@ -46,6 +53,17 @@ module LearnGitlabHelper
       .merge(LearnGitlab::Onboarding::ACTION_DOC_URLS)
   end
 
+  def new_action_urls(project)
+    action_urls.merge(
+      issue_created: project_issues_path(project),
+      git_write: project_path(project),
+      pipeline_created: project_pipelines_path(project),
+      merge_request_created: project_merge_requests_path(project),
+      user_added: project_members_url(project),
+      security_scan_enabled: project_security_configuration_path(project)
+    )
+  end
+
   def learn_gitlab_project
     @learn_gitlab_project ||= LearnGitlab::Project.new(current_user).project
   end
@@ -54,3 +72,5 @@ module LearnGitlabHelper
     OnboardingProgress.find_by(namespace: project.namespace) # rubocop: disable CodeReuse/ActiveRecord
   end
 end
+
+LearnGitlabHelper.prepend_mod_with('LearnGitlabHelper')

config/feature_flags/experiment/change_continuous_onboarding_link_urls.yml

+---
+name: change_continuous_onboarding_link_urls
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/71408
+rollout_issue_url:
+milestone: '14.5'
+type: experiment
+group: group::conversion
+default_enabled: false

ee/app/helpers/ee/learn_gitlab_helper.rb

+# frozen_string_literal: true
+
+module EE
+  module LearnGitlabHelper
+    extend ::Gitlab::Utils::Override
+
+    GITLAB_COM = 'gitlab.com'
+    ONBOARDING_START_TRIAL = 'onboarding-start-trial'
+    ONBOARDING_REQUIRE_MR_APPROVALS = 'onboarding-require-merge-approvals'
+    ONBOARDING_CODE_OWNERS = 'onboarding-code-owners'
+
+    private
+
+    override :new_action_urls
+    def new_action_urls(project)
+      urls = super(project)
+
+      return urls unless ::Gitlab::CurrentSettings.should_check_namespace_plan?
+
+      glm_params = { glm_source: GITLAB_COM }
+
+      urls.merge(
+        trial_started: new_trial_path(glm_params.merge(glm_content: ONBOARDING_START_TRIAL)),
+        required_mr_approvals_enabled: new_trial_path(glm_params.merge(glm_content: ONBOARDING_REQUIRE_MR_APPROVALS)),
+        code_owners_enabled: new_trial_path(glm_params.merge(glm_content: ONBOARDING_CODE_OWNERS))
+      )
+    end
+  end
+end

ee/spec/helpers/ee/learn_gitlab_helper_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe LearnGitlabHelper do
+  include Devise::Test::ControllerHelpers
+
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:project, name: LearnGitlab::Project::PROJECT_NAME, namespace: user.namespace) }
+  let_it_be(:namespace) { project.namespace }
+
+  before do
+    allow_next_instance_of(LearnGitlab::Project) do |learn_gitlab|
+      allow(learn_gitlab).to receive(:project).and_return(project)
+    end
+
+    OnboardingProgress.onboard(namespace)
+  end
+
+  describe '#onboarding_actions_data' do
+    subject(:onboarding_actions_data) { helper.onboarding_actions_data(project) }
+
+    context 'when in the new action URLs experiment' do
+      before do
+        stub_experiments(change_continuous_onboarding_link_urls: :candidate)
+      end
+
+      context 'for trial- and subscription-related actions' do
+        context 'when namespace plans are not enabled' do
+          before do
+            stub_application_setting(check_namespace_plan: false)
+          end
+
+          it 'provides the default URLs' do
+            expect(onboarding_actions_data).to include(
+              trial_started: a_hash_including(
+                url: a_string_matching(%r{#{namespace.path}/learn_gitlab/-/issues/2})
+              ),
+              code_owners_enabled: a_hash_including(
+                url: a_string_matching(%r{#{namespace.path}/learn_gitlab/-/issues/10})
+              ),
+              required_mr_approvals_enabled: a_hash_including(
+                url: a_string_matching(%r{#{namespace.path}/learn_gitlab/-/issues/11})
+              )
+            )
+          end
+        end
+
+        context 'when namespace plans are enabled' do
+          before do
+            stub_application_setting(check_namespace_plan: true)
+          end
+
+          it 'provides URLs to start a trial for the appropariate actions' do
+            expect(onboarding_actions_data).to include(
+              trial_started: a_hash_including(
+                url: new_trial_path(glm_source: 'gitlab.com', glm_content: 'onboarding-start-trial')
+              ),
+              code_owners_enabled: a_hash_including(
+                url: new_trial_path(glm_source: 'gitlab.com', glm_content: 'onboarding-code-owners')
+              ),
+              required_mr_approvals_enabled: a_hash_including(
+                url: new_trial_path(glm_source: 'gitlab.com', glm_content: 'onboarding-require-merge-approvals')
+              )
+            )
+          end
+        end
+      end
+    end
+  end
+end

spec/helpers/learn_gitlab_helper_spec.rb

@@ -11,9 +11,6 @@ RSpec.describe LearnGitlabHelper do
   let_it_be(:namespace) { project.namespace }
 
   before do
-    project.add_developer(user)
-
-    allow(helper).to receive(:user).and_return(user)
     allow_next_instance_of(LearnGitlab::Project) do |learn_gitlab|
       allow(learn_gitlab).to receive(:project).and_return(project)
     end
@@ -22,38 +19,115 @@ RSpec.describe LearnGitlabHelper do
     OnboardingProgress.register(namespace, :git_write)
   end
 
-  describe '.onboarding_actions_data' do
+  describe '#onboarding_actions_data' do
     subject(:onboarding_actions_data) { helper.onboarding_actions_data(project) }
 
-    it 'has all actions' do
-      expect(onboarding_actions_data.keys).to contain_exactly(
-        :issue_created,
-        :git_write,
-        :pipeline_created,
-        :merge_request_created,
-        :user_added,
-        :trial_started,
-        :required_mr_approvals_enabled,
-        :code_owners_enabled,
-        :security_scan_enabled
-      )
+    shared_examples 'has all actions' do
+      it 'has all actions' do
+        expect(onboarding_actions_data.keys).to contain_exactly(
+          :issue_created,
+          :git_write,
+          :pipeline_created,
+          :merge_request_created,
+          :user_added,
+          :trial_started,
+          :required_mr_approvals_enabled,
+          :code_owners_enabled,
+          :security_scan_enabled
+        )
+      end
     end
 
-    it 'sets correct path and completion status' do
-      expect(onboarding_actions_data[:git_write]).to eq({
-        url: project_issue_url(project, LearnGitlab::Onboarding::ACTION_ISSUE_IDS[:git_write]),
-        completed: true,
-        svg: helper.image_path("learn_gitlab/git_write.svg")
+    it_behaves_like 'has all actions'
+
+    it 'sets correct paths' do
+      expect(onboarding_actions_data).to match({
+        trial_started: a_hash_including(
+          url: a_string_matching(%r{/learn_gitlab/-/issues/2\z})
+        ),
+        issue_created: a_hash_including(
+          url: a_string_matching(%r{/learn_gitlab/-/issues/4\z})
+        ),
+        git_write: a_hash_including(
+          url: a_string_matching(%r{/learn_gitlab/-/issues/6\z})
+        ),
+        pipeline_created: a_hash_including(
+          url: a_string_matching(%r{/learn_gitlab/-/issues/7\z})
+        ),
+        user_added: a_hash_including(
+          url: a_string_matching(%r{/learn_gitlab/-/issues/8\z})
+        ),
+        merge_request_created: a_hash_including(
+          url: a_string_matching(%r{/learn_gitlab/-/issues/9\z})
+        ),
+        code_owners_enabled: a_hash_including(
+          url: a_string_matching(%r{/learn_gitlab/-/issues/10\z})
+        ),
+        required_mr_approvals_enabled: a_hash_including(
+          url: a_string_matching(%r{/learn_gitlab/-/issues/11\z})
+        ),
+        security_scan_enabled: a_hash_including(
+          url: a_string_matching(%r{docs\.gitlab\.com/ee/user/application_security/security_dashboard/#gitlab-security-dashboard-security-center-and-vulnerability-reports\z})
+        )
       })
-      expect(onboarding_actions_data[:pipeline_created]).to eq({
-        url: project_issue_url(project, LearnGitlab::Onboarding::ACTION_ISSUE_IDS[:pipeline_created]),
-        completed: false,
-        svg: helper.image_path("learn_gitlab/pipeline_created.svg")
+    end
+
+    it 'sets correct completion statuses' do
+      expect(onboarding_actions_data).to match({
+        issue_created: a_hash_including(completed: false),
+        git_write: a_hash_including(completed: true),
+        pipeline_created: a_hash_including(completed: false),
+        merge_request_created: a_hash_including(completed: false),
+        user_added: a_hash_including(completed: false),
+        trial_started: a_hash_including(completed: false),
+        required_mr_approvals_enabled: a_hash_including(completed: false),
+        code_owners_enabled: a_hash_including(completed: false),
+        security_scan_enabled: a_hash_including(completed: false)
       })
     end
+
+    context 'when in the new action URLs experiment' do
+      before do
+        stub_experiments(change_continuous_onboarding_link_urls: :candidate)
+      end
+
+      it_behaves_like 'has all actions'
+
+      it 'sets mostly new paths' do
+        expect(onboarding_actions_data).to match({
+          trial_started: a_hash_including(
+            url: a_string_matching(%r{/learn_gitlab/-/issues/2\z})
+          ),
+          issue_created: a_hash_including(
+            url: a_string_matching(%r{/learn_gitlab/-/issues\z})
+          ),
+          git_write: a_hash_including(
+            url: a_string_matching(%r{/learn_gitlab\z})
+          ),
+          pipeline_created: a_hash_including(
+            url: a_string_matching(%r{/learn_gitlab/-/pipelines\z})
+          ),
+          user_added: a_hash_including(
+            url: a_string_matching(%r{/learn_gitlab/-/project_members\z})
+          ),
+          merge_request_created: a_hash_including(
+            url: a_string_matching(%r{/learn_gitlab/-/merge_requests\z})
+          ),
+          code_owners_enabled: a_hash_including(
+            url: a_string_matching(%r{/learn_gitlab/-/issues/10\z})
+          ),
+          required_mr_approvals_enabled: a_hash_including(
+            url: a_string_matching(%r{/learn_gitlab/-/issues/11\z})
+          ),
+          security_scan_enabled: a_hash_including(
+            url: a_string_matching(%r{/learn_gitlab/-/security/configuration\z})
+          )
+        })
+      end
+    end
   end
 
-  describe '.learn_gitlab_enabled?' do
+  describe '#learn_gitlab_enabled?' do
     using RSpec::Parameterized::TableSyntax
 
     let_it_be(:user) { create(:user) }
@@ -89,7 +163,7 @@ RSpec.describe LearnGitlabHelper do
     end
   end
 
-  describe '.onboarding_sections_data' do
+  describe '#onboarding_sections_data' do
     subject(:sections) { helper.onboarding_sections_data }
 
     it 'has the right keys' do