Merge branch '352066-cleanup-change_continuous_onboarding_link_urls' into 'master'

Experiment Clean up: change_continuous_onboarding_link_urls

See merge request gitlab-org/gitlab!80906

app/assets/javascripts/pages/projects/learn_gitlab/components/learn_gitlab_section_link.vue

@@ -32,7 +32,7 @@ export default {
       );
     },
     openInNewTab() {
-      return ACTION_LABELS[this.action]?.openInNewTab === true;
+      return ACTION_LABELS[this.action]?.openInNewTab === true || this.value.openInNewTab === true;
     },
   },
   methods: {
@@ -65,8 +65,6 @@ export default {
       data-testid="uncompleted-learn-gitlab-link"
       data-track-action="click_link"
       :data-track-label="$options.i18n.ACTION_LABELS[action].title"
-      data-track-property="Growth::Conversion::Experiment::LearnGitLab"
-      data-track-experiment="change_continuous_onboarding_link_urls"
     >
       {{ $options.i18n.ACTION_LABELS[action].title }}
     </gl-link>

app/assets/javascripts/pages/projects/learn_gitlab/constants/index.js

@@ -62,7 +62,6 @@ export const ACTION_LABELS = {
     description: s__('LearnGitLab|Scan your code to uncover vulnerabilities before deploying.'),
     section: 'deploy',
     position: 1,
-    openInNewTab: true,
   },
   issueCreated: {
     title: s__('LearnGitLab|Create an issue'),

app/helpers/learn_gitlab_helper.rb

 # frozen_string_literal: true
 
 module LearnGitlabHelper
+  IMAGE_PATH_PLAN = "learn_gitlab/section_plan.svg"
+  IMAGE_PATH_DEPLOY = "learn_gitlab/section_deploy.svg"
+  IMAGE_PATH_WORKSPACE = "learn_gitlab/section_workspace.svg"
+
   def learn_gitlab_enabled?(project)
     return false unless current_user
 
@@ -25,19 +29,7 @@ module LearnGitlabHelper
   def onboarding_actions_data(project)
     attributes = onboarding_progress(project).attributes.symbolize_keys
 
-    urls_to_use = nil
-
-    experiment(
-      :change_continuous_onboarding_link_urls,
-      namespace: project.namespace,
-      actor: current_user,
-      sticky_to: project.namespace
-    ) do |e|
-      e.control { urls_to_use = action_urls }
-      e.candidate { urls_to_use = new_action_urls(project) }
-    end
-
-    urls_to_use.to_h do |action, url|
+    action_urls(project).to_h do |action, url|
       [
         action,
         url: url,
@@ -50,13 +42,13 @@ module LearnGitlabHelper
   def onboarding_sections_data
     {
       workspace: {
-        svg: image_path("learn_gitlab/section_workspace.svg")
+        svg: image_path(IMAGE_PATH_WORKSPACE)
       },
       plan: {
-        svg: image_path("learn_gitlab/section_plan.svg")
+        svg: image_path(IMAGE_PATH_PLAN)
       },
       deploy: {
-        svg: image_path("learn_gitlab/section_deploy.svg")
+        svg: image_path(IMAGE_PATH_DEPLOY)
       }
     }
   end
@@ -65,22 +57,20 @@ module LearnGitlabHelper
     { name: project.name }
   end
 
-  def action_urls
-    LearnGitlab::Onboarding::ACTION_ISSUE_IDS.transform_values { |id| project_issue_url(learn_gitlab_project, id) }
-      .merge(LearnGitlab::Onboarding::ACTION_DOC_URLS)
-  end
-
-  def new_action_urls(project)
-    action_urls.merge(
+  def action_urls(project)
+    action_issue_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 action_issue_urls
+    LearnGitlab::Onboarding::ACTION_ISSUE_IDS.transform_values { |id| project_issue_url(learn_gitlab_project, id) }
+  end
+
   def learn_gitlab_project
     @learn_gitlab_project ||= LearnGitlab::Project.new(current_user).project
   end

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

@@ -11,19 +11,21 @@ module EE
 
     private
 
-    override :new_action_urls
-    def new_action_urls(project)
+    override :action_urls
+    def 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))
+        trial_started: new_trial_path_with_glm(content: ONBOARDING_START_TRIAL),
+        required_mr_approvals_enabled: new_trial_path_with_glm(content: ONBOARDING_REQUIRE_MR_APPROVALS),
+        code_owners_enabled: new_trial_path_with_glm(content: ONBOARDING_CODE_OWNERS)
       )
     end
+
+    def new_trial_path_with_glm(content:, source: GITLAB_COM)
+      new_trial_path({ glm_source: source, glm_content: content })
+    end
   end
 end

ee/spec/features/registrations/user_sees_new_onboarding_flow_spec.rb

@@ -42,4 +42,45 @@ RSpec.describe 'User sees new onboarding flow', :js do
     expect(page).to have_content('Learn GitLab')
     expect(page).to have_content('GitLab is better with colleagues!')
   end
+
+  context 'with confetti_post_signup experiment candidate experience', :experiment do
+    before do
+      stub_experiments(combined_registration: :control,
+                       confetti_post_signup: :candidate)
+    end
+
+    it 'shows continuous onboarding flow pages with celebration invite modal' do
+      sign_in(create(:user))
+      visit users_sign_up_welcome_path
+
+      expect(page).to have_content('Welcome to GitLab')
+
+      choose 'Just me'
+      click_on 'Continue'
+
+      expect(page).to have_content('Create your group')
+
+      fill_in 'group_name', with: 'test'
+
+      expect(page).to have_field('group_path', with: 'test')
+
+      click_on 'Create group'
+
+      expect(page).to have_content('Create/import your first project')
+
+      fill_in 'project_name', with: 'test'
+
+      expect(page).to have_field('project_path', with: 'test')
+
+      click_on 'Create project'
+
+      expect(page).to have_content('Get started with GitLab')
+
+      Sidekiq::Worker.drain_all
+      click_on "Ok, let's go"
+
+      expect(page).to have_content('Learn GitLab')
+      expect(page).to have_content('GitLab is better with colleagues!')
+    end
+  end
 end

ee/spec/features/uncompleted_learn_gitlab_link_spec.rb

@@ -7,62 +7,56 @@ RSpec.describe 'Uncompleted learn gitlab link', :feature, :js do
   let_it_be(:project) { create(:project, name: LearnGitlab::Project::PROJECT_NAME, namespace: user.namespace) }
   let_it_be(:namespace) { project.namespace }
 
-  context 'change_continuous_onboarding_link_urls experiment' do
-    before do
-      allow_next_instance_of(LearnGitlab::Project) do |learn_gitlab|
-        allow(learn_gitlab).to receive(:available?).and_return(true)
-      end
+  before do
+    allow_next_instance_of(LearnGitlab::Project) do |learn_gitlab|
+      allow(learn_gitlab).to receive(:available?).and_return(true)
     end
+  end
 
-    context 'when control' do
-      before do
-        create(:onboarding_progress,
-          namespace: namespace,
-          issue_created_at: Date.yesterday,
-          git_write_at: Date.yesterday,
-          pipeline_created_at: Date.yesterday,
-          merge_request_created_at: Date.yesterday,
-          user_added_at: Date.yesterday,
-          security_scan_enabled_at: Date.yesterday
-        )
-
-        stub_experiments(change_continuous_onboarding_link_urls: :control)
-      end
+  context 'with completed links' do
+    before do
+      yesterday = Date.yesterday
+      create(:onboarding_progress,
+        namespace: namespace,
+        issue_created_at: yesterday,
+        git_write_at: yesterday,
+        pipeline_created_at: yesterday,
+        merge_request_created_at: yesterday,
+        user_added_at: yesterday,
+        security_scan_enabled_at: yesterday
+      )
+    end
 
-      it 'renders correct completed sections' do
-        sign_in(user)
-        visit namespace_project_learn_gitlab_path(namespace, project)
+    it 'renders correct completed sections' do
+      sign_in(user)
+      visit namespace_project_learn_gitlab_path(namespace, project)
 
-        expect_completed_section('Create an issue')
-        expect_completed_section('Create or import a repository')
-        expect_completed_section('Set up CI/CD')
-        expect_completed_section('Submit a merge request')
-        expect_completed_section('Invite your colleagues')
-        expect_completed_section('Run a Security scan using CI/CD')
-      end
+      expect_completed_section('Set up CI/CD')
+      expect_completed_section('Submit a merge request')
+      expect_completed_section('Invite your colleagues')
+      expect_completed_section('Run a Security scan using CI/CD')
     end
+  end
 
-    context 'when candidate' do
-      before do
-        create(:onboarding_progress, namespace: namespace)
-        stub_experiments(change_continuous_onboarding_link_urls: :candidate)
-      end
+  context 'without completion progress' do
+    before do
+      create(:onboarding_progress, namespace: namespace)
+    end
 
-      it 'renders correct links' do
-        sign_in(user)
-        visit namespace_project_learn_gitlab_path(namespace, project)
-        issue_link = find_link('Create an issue')
+    it 'renders correct links' do
+      sign_in(user)
+      visit namespace_project_learn_gitlab_path(namespace, project)
+      issue_link = find_link('Create an issue')
 
-        expect_correct_candidate_link(issue_link, project_issues_path(project))
-        expect_correct_candidate_link(find_link('Create or import a repository'), project_path(project))
-        expect_correct_candidate_link(find_link('Set up CI/CD'), project_pipelines_path(project))
-        expect_correct_candidate_link(find_link('Submit a merge request'), project_merge_requests_path(project))
-        expect_correct_candidate_link(find_link('Invite your colleagues'), URI(project_members_url(project)).path)
-        expect_correct_candidate_link(find_link('Run a Security scan using CI/CD'), project_security_configuration_path(project))
+      expect_correct_candidate_link(issue_link, project_issues_path(project))
+      expect_correct_candidate_link(find_link('Create or import a repository'), project_path(project))
+      expect_correct_candidate_link(find_link('Set up CI/CD'), project_issues_path(project, 7))
+      expect_correct_candidate_link(find_link('Submit a merge request'), project_merge_requests_path(project))
+      expect_correct_candidate_link(find_link('Invite your colleagues'), URI(project_members_url(project)).path)
+      expect_correct_candidate_link(find_link('Run a Security scan using CI/CD'), project_security_configuration_path(project))
 
-        issue_link.click
-        expect(page).to have_current_path(project_issues_path(project))
-      end
+      issue_link.click
+      expect(page).to have_current_path(project_issues_path(project))
     end
   end
 

ee/spec/helpers/ee/learn_gitlab_helper_spec.rb

@@ -23,10 +23,6 @@ RSpec.describe LearnGitlabHelper do
     end
 
     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

lib/learn_gitlab/onboarding.rb

@@ -5,19 +5,19 @@ module LearnGitlab
     include Gitlab::Utils::StrongMemoize
 
     ACTION_ISSUE_IDS = {
-      issue_created: 4,
-      git_write: 6,
       pipeline_created: 7,
-      merge_request_created: 9,
-      user_added: 8,
       trial_started: 2,
       required_mr_approvals_enabled: 11,
       code_owners_enabled: 10
     }.freeze
 
-    ACTION_DOC_URLS = {
-      security_scan_enabled: 'https://docs.gitlab.com/ee/user/application_security/security_dashboard/#gitlab-security-dashboard-security-center-and-vulnerability-reports'
-    }.freeze
+    ACTION_PATHS = [
+      :issue_created,
+      :git_write,
+      :merge_request_created,
+      :user_added,
+      :security_scan_enabled
+    ].freeze
 
     def initialize(namespace)
       @namespace = namespace
@@ -49,7 +49,7 @@ module LearnGitlab
     end
 
     def tracked_actions
-      ACTION_ISSUE_IDS.keys + ACTION_DOC_URLS.keys
+      ACTION_ISSUE_IDS.keys + ACTION_PATHS
     end
 
     attr_reader :namespace

spec/frontend/pages/projects/learn_gitlab/components/__snapshots__/learn_gitlab_spec.js.snap

@@ -137,9 +137,7 @@ exports[`Learn GitLab renders correctly 1`] = `
               class="gl-link"
               data-testid="uncompleted-learn-gitlab-link"
               data-track-action="click_link"
-              data-track-experiment="change_continuous_onboarding_link_urls"
               data-track-label="Set up CI/CD"
-              data-track-property="Growth::Conversion::Experiment::LearnGitLab"
               href="http://example.com/"
               target="_self"
             >
@@ -157,9 +155,7 @@ exports[`Learn GitLab renders correctly 1`] = `
               class="gl-link"
               data-testid="uncompleted-learn-gitlab-link"
               data-track-action="click_link"
-              data-track-experiment="change_continuous_onboarding_link_urls"
               data-track-label="Start a free Ultimate trial"
-              data-track-property="Growth::Conversion::Experiment::LearnGitLab"
               href="http://example.com/"
               target="_self"
             >
@@ -177,9 +173,7 @@ exports[`Learn GitLab renders correctly 1`] = `
               class="gl-link"
               data-testid="uncompleted-learn-gitlab-link"
               data-track-action="click_link"
-              data-track-experiment="change_continuous_onboarding_link_urls"
               data-track-label="Add code owners"
-              data-track-property="Growth::Conversion::Experiment::LearnGitLab"
               href="http://example.com/"
               target="_self"
             >
@@ -204,9 +198,7 @@ exports[`Learn GitLab renders correctly 1`] = `
               class="gl-link"
               data-testid="uncompleted-learn-gitlab-link"
               data-track-action="click_link"
-              data-track-experiment="change_continuous_onboarding_link_urls"
               data-track-label="Add merge request approval"
-              data-track-property="Growth::Conversion::Experiment::LearnGitLab"
               href="http://example.com/"
               target="_self"
             >
@@ -267,9 +259,7 @@ exports[`Learn GitLab renders correctly 1`] = `
               class="gl-link"
               data-testid="uncompleted-learn-gitlab-link"
               data-track-action="click_link"
-              data-track-experiment="change_continuous_onboarding_link_urls"
               data-track-label="Create an issue"
-              data-track-property="Growth::Conversion::Experiment::LearnGitLab"
               href="http://example.com/"
               target="_self"
             >
@@ -287,9 +277,7 @@ exports[`Learn GitLab renders correctly 1`] = `
               class="gl-link"
               data-testid="uncompleted-learn-gitlab-link"
               data-track-action="click_link"
-              data-track-experiment="change_continuous_onboarding_link_urls"
               data-track-label="Submit a merge request"
-              data-track-property="Growth::Conversion::Experiment::LearnGitLab"
               href="http://example.com/"
               target="_self"
             >
@@ -343,9 +331,7 @@ exports[`Learn GitLab renders correctly 1`] = `
               class="gl-link"
               data-testid="uncompleted-learn-gitlab-link"
               data-track-action="click_link"
-              data-track-experiment="change_continuous_onboarding_link_urls"
               data-track-label="Run a Security scan using CI/CD"
-              data-track-property="Growth::Conversion::Experiment::LearnGitLab"
               href="https://docs.gitlab.com/ee/foobar/"
               rel="noopener noreferrer"
               target="_blank"

spec/frontend/pages/projects/learn_gitlab/components/learn_gitlab_section_link_spec.js

@@ -12,8 +12,9 @@ const defaultProps = {
   completed: false,
 };
 
-const docLinkProps = {
+const openInNewTabProps = {
   url: 'https://docs.gitlab.com/ee/user/application_security/security_dashboard/',
+  openInNewTab: true,
 };
 
 describe('Learn GitLab Section Link', () => {
@@ -59,9 +60,9 @@ describe('Learn GitLab Section Link', () => {
     expect(wrapper.find('[data-testid="trial-only"]').exists()).toBe(true);
   });
 
-  describe('doc links', () => {
+  describe('links marked with openInNewTab', () => {
     beforeEach(() => {
-      createWrapper('securityScanEnabled', docLinkProps);
+      createWrapper('securityScanEnabled', openInNewTabProps);
     });
 
     it('renders links with blank target', () => {
@@ -78,7 +79,6 @@ describe('Learn GitLab Section Link', () => {
 
       expect(trackingSpy).toHaveBeenCalledWith('_category_', 'click_link', {
         label: 'Run a Security scan using CI/CD',
-        property: 'Growth::Conversion::Experiment::LearnGitLab',
       });
 
       unmockTracking();

spec/frontend/pages/projects/learn_gitlab/components/mock_data.js

@@ -38,6 +38,7 @@ export const testActions = {
     url: 'https://docs.gitlab.com/ee/foobar/',
     completed: false,
     svg: 'http://example.com/images/illustration.svg',
+    openInNewTab: true,
   },
   issueCreated: {
     url: 'http://example.com/',

spec/helpers/learn_gitlab_helper_spec.rb

@@ -97,29 +97,29 @@ RSpec.describe LearnGitlabHelper do
                                                  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})
                                                  ),
+                                                 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})
+                                                 ),
+                                                 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})
+                                                 ),
                                                  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})
+                                                   url: a_string_matching(%r{/learn_gitlab/-/security/configuration\z})
                                                  )
                                                })
     end
@@ -137,58 +137,5 @@ RSpec.describe LearnGitlabHelper do
                                                  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 data'
-
-      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
-
-      it 'calls experiment with expected context & options' do
-        allow(helper).to receive(:current_user).and_return(user)
-
-        expect(helper).to receive(:experiment).with(
-          :change_continuous_onboarding_link_urls,
-          namespace: namespace,
-          actor: user,
-          sticky_to: namespace
-        )
-
-        learn_gitlab_data
-      end
-    end
   end
 end

spec/lib/learn_gitlab/onboarding_spec.rb

@@ -9,7 +9,7 @@ RSpec.describe LearnGitlab::Onboarding do
     let(:namespace) { build(:namespace) }
 
     let_it_be(:tracked_action_columns) do
-      tracked_actions = described_class::ACTION_ISSUE_IDS.keys + described_class::ACTION_DOC_URLS.keys
+      tracked_actions = described_class::ACTION_ISSUE_IDS.keys + described_class::ACTION_PATHS
       tracked_actions.map { |key| OnboardingProgress.column_name(key) }
     end