Merge branch '267492-change-to-enabled-sp-ff' into 'master'

Change from suggest_pipeline experiment to enabled ff

See merge request gitlab-org/gitlab!45926

app/assets/javascripts/pages/projects/blob/show/index.js

@@ -7,7 +7,6 @@ import initWebIdeLink from '~/pages/projects/shared/web_ide_link';
 import '~/sourcegraph/load';
 import PipelineTourSuccessModal from '~/blob/pipeline_tour_success_modal.vue';
 import { parseBoolean } from '~/lib/utils/common_utils';
-import { isExperimentEnabled } from '~/lib/utils/experimentation';
 
 const createGitlabCiYmlVisualization = (containerId = '#js-blob-toggle-graph-preview') => {
   const el = document.querySelector(containerId);
@@ -74,7 +73,7 @@ document.addEventListener('DOMContentLoaded', () => {
     );
   }
 
-  if (isExperimentEnabled('suggestPipeline')) {
+  if (gon.features?.suggestPipeline) {
     const successPipelineEl = document.querySelector('.js-success-pipeline-modal');
 
     if (successPipelineEl) {

app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue

@@ -46,7 +46,6 @@ import GroupedTestReportsApp from '../reports/components/grouped_test_reports_ap
 import { setFaviconOverlay } from '../lib/utils/common_utils';
 import GroupedAccessibilityReportsApp from '../reports/accessibility_report/grouped_accessibility_reports_app.vue';
 import getStateQuery from './queries/get_state.query.graphql';
-import { isExperimentEnabled } from '~/lib/utils/experimentation';
 
 export default {
   el: '#js-vue-mr-widget',
@@ -154,7 +153,7 @@ export default {
     },
     shouldSuggestPipelines() {
       return (
-        isExperimentEnabled('suggestPipeline') &&
+        gon.features?.suggestPipeline &&
         !this.mr.hasCI &&
         this.mr.mergeRequestAddCiConfigPath &&
         !this.mr.isDismissedSuggestPipeline

app/controllers/projects/blob_controller.rb

@@ -33,7 +33,7 @@ class Projects::BlobController < Projects::ApplicationController
   before_action :set_last_commit_sha, only: [:edit, :update]
 
   before_action only: :show do
-    push_frontend_experiment(:suggest_pipeline)
+    push_frontend_feature_flag(:suggest_pipeline, default_enabled: true)
     push_frontend_feature_flag(:gitlab_ci_yml_preview, @project, default_enabled: false)
   end
 

app/controllers/projects/merge_requests_controller.rb

@@ -27,7 +27,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
   before_action :authenticate_user!, only: [:assign_related_issues]
   before_action :check_user_can_push_to_source_branch!, only: [:rebase]
   before_action only: [:show] do
-    push_frontend_experiment(:suggest_pipeline)
+    push_frontend_feature_flag(:suggest_pipeline, default_enabled: true)
     push_frontend_feature_flag(:widget_visibility_polling, @project, default_enabled: true)
     push_frontend_feature_flag(:mr_commit_neighbor_nav, @project, default_enabled: true)
     push_frontend_feature_flag(:multiline_comments, @project, default_enabled: true)

app/helpers/blob_helper.rb

@@ -382,7 +382,7 @@ module BlobHelper
   end
 
   def show_suggest_pipeline_creation_celebration?
-    experiment_enabled?(:suggest_pipeline) &&
+    Feature.enabled?(:suggest_pipeline, default_enabled: true) &&
       @blob.path == Gitlab::FileDetector::PATTERNS[:gitlab_ci] &&
       @blob.auxiliary_viewer&.valid?(project: @project, sha: @commit.sha, user: current_user) &&
       @project.uses_default_ci_config? &&

app/helpers/issuables_helper.rb

@@ -76,7 +76,6 @@ module IssuablesHelper
                        when Issue
                          IssueSerializer
                        when MergeRequest
-                         opts[:experiment_enabled] = :suggest_pipeline if experiment_enabled?(:suggest_pipeline) && opts[:serializer] == 'widget'
                          MergeRequestSerializer
                        end
 

app/helpers/suggest_pipeline_helper.rb

@@ -2,7 +2,7 @@
 
 module SuggestPipelineHelper
   def should_suggest_gitlab_ci_yml?
-    experiment_enabled?(:suggest_pipeline) &&
+    Feature.enabled?(:suggest_pipeline, default_enabled: true) &&
       current_user &&
       params[:suggest_gitlab_ci_yml] == 'true'
   end

app/serializers/merge_request_widget_entity.rb

@@ -67,15 +67,15 @@ class MergeRequestWidgetEntity < Grape::Entity
     )
   end
 
-  expose :user_callouts_path, if: -> (_, opts) { opts[:experiment_enabled] == :suggest_pipeline } do |_merge_request|
+  expose :user_callouts_path, if: -> (*) { Feature.enabled?(:suggest_pipeline, default_enabled: true) } do |_merge_request|
     user_callouts_path
   end
 
-  expose :suggest_pipeline_feature_id, if: -> (_, opts) { opts[:experiment_enabled] == :suggest_pipeline } do |_merge_request|
+  expose :suggest_pipeline_feature_id, if: -> (*) { Feature.enabled?(:suggest_pipeline, default_enabled: true) } do |_merge_request|
     SUGGEST_PIPELINE
   end
 
-  expose :is_dismissed_suggest_pipeline, if: -> (_, opts) { opts[:experiment_enabled] == :suggest_pipeline } do |_merge_request|
+  expose :is_dismissed_suggest_pipeline, if: -> (*) { Feature.enabled?(:suggest_pipeline, default_enabled: true) } do |_merge_request|
     current_user && current_user.dismissed_callout?(feature_name: SUGGEST_PIPELINE)
   end
 

changelogs/unreleased/267492-change-to-enabled-sp-ff.yml

+---
+title: Add suggest pipeline for viable merge requests without pipelines
+merge_request: 45926
+author:
+type: changed

config/feature_flags/development/suggest_pipeline.yml

+---
+name: suggest_pipeline
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45926
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/267492
+type: development
+group: group::growth
+default_enabled: true

lib/gitlab/experimentation.rb

@@ -36,9 +36,6 @@ module Gitlab
       onboarding_issues: {
         tracking_category: 'Growth::Conversion::Experiment::OnboardingIssues'
       },
-      suggest_pipeline: {
-        tracking_category: 'Growth::Expansion::Experiment::SuggestPipeline'
-      },
       ci_notification_dot: {
         tracking_category: 'Growth::Expansion::Experiment::CiNotificationDot'
       },

spec/features/merge_request/user_sees_suggest_pipeline_spec.rb

@@ -9,8 +9,6 @@ RSpec.describe 'Merge request > User sees suggest pipeline', :js do
 
   before do
     stub_application_setting(auto_devops_enabled: false)
-    stub_experiment(suggest_pipeline: true)
-    stub_experiment_for_user(suggest_pipeline: true)
     project.add_maintainer(user)
     sign_in(user)
     visit project_merge_request_path(project, merge_request)

spec/features/projects/blobs/user_follows_pipeline_suggest_nudge_spec.rb

@@ -10,7 +10,6 @@ RSpec.describe 'User follows pipeline suggest nudge spec when feature is enabled
 
   describe 'viewing the new blob page' do
     before do
-      stub_experiment_for_user(suggest_pipeline: true)
       sign_in(user)
     end
 

spec/frontend/vue_mr_widget/mr_widget_options_spec.js

 import Vue from 'vue';
 import MockAdapter from 'axios-mock-adapter';
 import mountComponent from 'helpers/vue_mount_component_helper';
-import { withGonExperiment } from 'helpers/experimentation_helper';
 import Api from '~/api';
 import axios from '~/lib/utils/axios_utils';
 import mrWidgetOptions from '~/vue_merge_request_widget/mr_widget_options.vue';
@@ -850,7 +849,7 @@ describe('mrWidgetOptions', () => {
     });
   });
 
-  describe('suggestPipeline Experiment', () => {
+  describe('suggestPipeline feature flag', () => {
     beforeEach(() => {
       mock.onAny().reply(200);
 
@@ -859,10 +858,10 @@ describe('mrWidgetOptions', () => {
       jest.spyOn(console, 'warn').mockImplementation();
     });
 
-    describe('given experiment is enabled', () => {
-      withGonExperiment('suggestPipeline');
-
+    describe('given feature flag is enabled', () => {
       beforeEach(() => {
+        gon.features = { suggestPipeline: true };
+
         createComponent();
 
         vm.mr.hasCI = false;
@@ -893,10 +892,10 @@ describe('mrWidgetOptions', () => {
       });
     });
 
-    describe('given suggestPipeline experiment is not enabled', () => {
-      withGonExperiment('suggestPipeline', false);
-
+    describe('given feature flag is not enabled', () => {
       beforeEach(() => {
+        gon.features = { suggestPipeline: false };
+
         createComponent();
 
         vm.mr.hasCI = false;

spec/helpers/blob_helper_spec.rb

@@ -236,11 +236,7 @@ RSpec.describe BlobHelper do
         let(:data) { File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) }
         let(:blob) { fake_blob(path: Gitlab::FileDetector::PATTERNS[:gitlab_ci], data: data) }
 
-        context 'experiment enabled' do
-          before do
-            allow(helper).to receive(:experiment_enabled?).and_return(true)
-          end
-
+        context 'feature enabled' do
           it 'is true' do
             expect(helper.show_suggest_pipeline_creation_celebration?).to be_truthy
           end
@@ -284,9 +280,9 @@ RSpec.describe BlobHelper do
           end
         end
 
-        context 'experiment disabled' do
+        context 'feature disabled' do
           before do
-            allow(helper).to receive(:experiment_enabled?).and_return(false)
+            stub_feature_flags(suggest_pipeline: false)
           end
 
           it 'is false' do
@@ -298,11 +294,7 @@ RSpec.describe BlobHelper do
       context 'when file is not a pipeline config file' do
         let(:blob) { fake_blob(path: 'LICENSE') }
 
-        context 'experiment enabled' do
-          before do
-            allow(helper).to receive(:experiment_enabled?).and_return(true)
-          end
-
+        context 'feature enabled' do
           it 'is false' do
             expect(helper.show_suggest_pipeline_creation_celebration?).to be_falsey
           end

spec/helpers/issuables_helper_spec.rb

@@ -352,35 +352,4 @@ RSpec.describe IssuablesHelper do
       expect(helper.sidebar_milestone_tooltip_label(milestone)).to eq('&lt;img onerror=alert(1)&gt;<br/>Milestone')
     end
   end
-
-  describe '#serialize_issuable' do
-    context 'when it is a merge request' do
-      let(:merge_request) { build(:merge_request) }
-      let(:user) { build(:user) }
-
-      before do
-        allow(helper).to receive(:current_user) { user }
-      end
-
-      it 'has suggest_pipeline experiment enabled' do
-        allow(helper).to receive(:experiment_enabled?).with(:suggest_pipeline) { true }
-
-        expect_next_instance_of(MergeRequestSerializer) do |serializer|
-          expect(serializer).to receive(:represent).with(merge_request, { serializer: 'widget', experiment_enabled: :suggest_pipeline })
-        end
-
-        helper.serialize_issuable(merge_request, serializer: 'widget')
-      end
-
-      it 'suggest_pipeline experiment disabled' do
-        allow(helper).to receive(:experiment_enabled?).with(:suggest_pipeline) { false }
-
-        expect_next_instance_of(MergeRequestSerializer) do |serializer|
-          expect(serializer).to receive(:represent).with(merge_request, { serializer: 'widget' })
-        end
-
-        helper.serialize_issuable(merge_request, serializer: 'widget')
-      end
-    end
-  end
 end

spec/serializers/merge_request_widget_entity_spec.rb

@@ -333,6 +333,10 @@ RSpec.describe MergeRequestWidgetEntity do
     end
 
     context 'when suggest pipeline feature is not enabled' do
+      before do
+        stub_feature_flags(suggest_pipeline: false)
+      end
+
       it 'provides no valid value for user callout path' do
         expect(subject[:user_callouts_path]).to be_nil
       end