Merge branch '292376-experiment-cleanup-mvc-add-contact-sales-option-in-app' into 'master'

Cleanup the "Add contact sales option in app" experiment

See merge request gitlab-org/gitlab!65598

ee/app/controllers/groups/billings_controller.rb

@@ -21,10 +21,7 @@ class Groups::BillingsController < Groups::ApplicationController
       .new(plan: current_plan, namespace_id: relevant_group.id)
       .execute
 
-    if @plans_data
-      track_experiment_event(:contact_sales_btn_in_app, 'page_view:billing_plans:group')
-      record_experiment_user(:contact_sales_btn_in_app)
-    else
+    unless @plans_data
       render 'shared/billings/customers_dot_unavailable'
     end
   end

ee/app/controllers/profiles/billings_controller.rb

@@ -11,10 +11,7 @@ class Profiles::BillingsController < Profiles::ApplicationController
       .new(plan: current_user.namespace.plan_name_for_upgrading, namespace_id: current_user.namespace_id)
       .execute
 
-    if @plans_data
-      track_experiment_event(:contact_sales_btn_in_app, 'page_view:billing_plans:profile')
-      record_experiment_user(:contact_sales_btn_in_app)
-    else
+    unless @plans_data
       render 'shared/billings/customers_dot_unavailable'
     end
   end

ee/app/helpers/billing_plans_helper.rb

@@ -19,19 +19,19 @@ module BillingPlansHelper
   end
 
   def has_upgrade?(upgrade_offer)
-    upgrade_offer == :upgrade_for_free || upgrade_offer == :upgrade_for_offer
+    [:upgrade_for_free, :upgrade_for_offer].include?(upgrade_offer)
   end
 
   def show_contact_sales_button?(purchase_link_action, upgrade_offer)
     return false unless purchase_link_action == 'upgrade'
 
-    upgrade_offer == :upgrade_for_offer ||
-      (experiment_enabled?(:contact_sales_btn_in_app) && upgrade_offer == :no_offer)
+    [:upgrade_for_offer, :no_offer].include?(upgrade_offer)
   end
 
   def show_upgrade_button?(purchase_link_action, upgrade_offer)
-    purchase_link_action == 'upgrade' &&
-      (upgrade_offer == :no_offer || upgrade_offer == :upgrade_for_free)
+    return false unless purchase_link_action == 'upgrade'
+
+    [:no_offer, :upgrade_for_free].include?(upgrade_offer)
   end
 
   def subscription_plan_data_attributes(namespace, plan)
@@ -62,18 +62,6 @@ module BillingPlansHelper
     namespace.group? && (namespace.actual_plan_name == Plan::FREE || namespace.trial_active?)
   end
 
-  def experiment_tracking_data_for_button_click(button_label)
-    return {} unless Gitlab::Experimentation.active?(:contact_sales_btn_in_app)
-
-    {
-      track: {
-        event: 'click_button',
-        label: button_label,
-        property: experiment_tracking_category_and_group(:contact_sales_btn_in_app)
-      }
-    }
-  end
-
   def plan_feature_list(plan)
     return [] unless plan.features
 
@@ -116,11 +104,16 @@ module BillingPlansHelper
     _('Seats usage data is updated every day at 12:00pm UTC')
   end
 
+  def upgrade_button_text(plan_offer_type)
+    plan_offer_type === :upgrade_for_free ? s_('BillingPlan|Upgrade for free') : s_('BillingPlan|Upgrade')
+  end
+
   def upgrade_button_css_classes(namespace, plan, is_current_plan)
-    css_classes = %w[btn btn-success gl-button]
+    css_classes = []
 
     css_classes << 'disabled' if is_current_plan && !namespace.trial_active?
     css_classes << 'invisible' if ::Feature.enabled?(:hide_deprecated_billing_plans) && plan.deprecated?
+    css_classes << "billing-cta-purchase#{'-new' if use_new_purchase_flow?(namespace)}"
 
     css_classes.join(' ')
   end

ee/app/views/shared/billings/_billing_plan.html.haml

@@ -63,13 +63,9 @@
               = link_to s_("BillingPlans|See all %{plan_name} features") % { plan_name: plan.name }, EE::SUBSCRIPTIONS_COMPARISON_URL
 
     .card-footer
-      - cta_class = '-new' if use_new_purchase_flow?(namespace)
-      - upgrade_button_classes = upgrade_button_css_classes(namespace, plan, is_current)
-      - upgrade_button_text = plan_offer_type === :upgrade_for_free ? s_('BillingPlan|Upgrade for free') : s_('BillingPlan|Upgrade')
       - show_upgrade_button = show_upgrade_button?(purchase_link.action, plan_offer_type)
-      - show_contact_sales_button = show_contact_sales_button?(purchase_link.action, plan_offer_type)
       .gl-min-h-7.gl-display-flex.gl-flex-wrap.gl-justify-content-end
-        - if show_contact_sales_button
-          = link_to s_('BillingPlan|Contact sales'), "#{contact_sales_url}?test=inappcontactsales#{plan.code}", class: ["btn gl-button", show_upgrade_button ? "btn-success-secondary" : "btn-success"], data: { **experiment_tracking_data_for_button_click('contact_sales') }
+        - if show_contact_sales_button?(purchase_link.action, plan_offer_type)
+          = link_to s_('BillingPlan|Contact sales'), "#{contact_sales_url}?test=inappcontactsales#{plan.code}", class: ["btn gl-button", show_upgrade_button ? "btn-success-secondary" : "btn-success"], data: { 'track-action': 'click_button', 'track-label': 'contact_sales', 'track-property': plan.code }
         - if show_upgrade_button
-          = link_to upgrade_button_text, plan_purchase_or_upgrade_url(namespace, plan), class: "#{upgrade_button_classes} billing-cta-purchase#{cta_class} gl-ml-3", data: { **experiment_tracking_data_for_button_click('upgrade'), qa_selector: "upgrade_to_#{plan.code}" }
+          = link_to upgrade_button_text(plan_offer_type), plan_purchase_or_upgrade_url(namespace, plan), class: ["btn btn-success gl-button", upgrade_button_css_classes(namespace, plan, is_current), "gl-ml-3"], data: { 'track-action': 'click_button', 'track-label': 'upgrade', 'track-property': plan.code, qa_selector: "upgrade_to_#{plan.code}" }

ee/config/feature_flags/experiment/contact_sales_btn_in_app_experiment_percentage.yml

----
-name: contact_sales_btn_in_app_experiment_percentage
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38508
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/238207
-milestone: '13.3'
-type: experiment
-group: group::conversion
-default_enabled: true

ee/spec/controllers/groups/billings_controller_spec.rb

@@ -50,18 +50,6 @@ RSpec.describe Groups::BillingsController do
         expect(assigns(:plans_data)).to eq(data)
       end
 
-      it 'tracks the page view for the contact_sales_btn_in_app experiment' do
-        expect(controller).to receive(:track_experiment_event).with(:contact_sales_btn_in_app, 'page_view:billing_plans:group')
-
-        get_index
-      end
-
-      it 'records user for the contact_sales_btn_in_app experiment' do
-        expect(controller).to receive(:record_experiment_user).with(:contact_sales_btn_in_app)
-
-        get_index
-      end
-
       context 'when CustomersDot is unavailable' do
         before do
           allow_next_instance_of(GitlabSubscriptions::FetchSubscriptionPlansService) do |instance|
@@ -69,17 +57,7 @@ RSpec.describe Groups::BillingsController do
           end
         end
 
-        it 'does not track the page view for the contact_sales_btn_in_app experiment' do
-          expect(controller).not_to receive(:track_experiment_event)
-
-          get_index
-
-          expect(response).to render_template('shared/billings/customers_dot_unavailable')
-        end
-
-        it 'does not record the user for the contact_sales_btn_in_app experiment' do
-          expect(controller).not_to receive(:record_experiment_user)
-
+        it 'renders a different partial' do
           get_index
 
           expect(response).to render_template('shared/billings/customers_dot_unavailable')

ee/spec/controllers/profiles/billings_controller_spec.rb

@@ -40,18 +40,6 @@ RSpec.describe Profiles::BillingsController do
       expect(assigns(:plans_data)).to eq(data)
     end
 
-    it 'tracks the page view for the contact_sales_btn_in_app experiment' do
-      expect(controller).to receive(:track_experiment_event).with(:contact_sales_btn_in_app, 'page_view:billing_plans:profile')
-
-      get_index
-    end
-
-    it 'records user for the contact_sales_btn_in_app experiment' do
-      expect(controller).to receive(:record_experiment_user).with(:contact_sales_btn_in_app)
-
-      get_index
-    end
-
     context 'when CustomersDot is unavailable' do
       before do
         allow_next_instance_of(GitlabSubscriptions::FetchSubscriptionPlansService) do |instance|
@@ -59,17 +47,7 @@ RSpec.describe Profiles::BillingsController do
         end
       end
 
-      it 'does not track the page view for the contact_sales_btn_in_app experiment' do
-        expect(controller).not_to receive(:track_experiment_event)
-
-        get_index
-
-        expect(response).to render_template('shared/billings/customers_dot_unavailable')
-      end
-
-      it 'does not record the user for the contact_sales_btn_in_app experiment' do
-        expect(controller).not_to receive(:record_experiment_user)
-
+      it 'renders a different partial' do
         get_index
 
         expect(response).to render_template('shared/billings/customers_dot_unavailable')

ee/spec/features/billings/billing_plans_spec.rb

@@ -17,7 +17,6 @@ RSpec.describe 'Billing plan pages', :feature, :js do
   before do
     stub_feature_flags(show_billing_eoa_banner: true)
     stub_feature_flags(hide_deprecated_billing_plans: false)
-    stub_experiment_for_subject(contact_sales_btn_in_app: true)
     stub_billing_plans(namespace.id, plan.name, plans_data.to_json)
     stub_eoa_eligibility_request(namespace.id)
     stub_application_setting(check_namespace_plan: true)

ee/spec/helpers/billing_plans_helper_spec.rb

@@ -277,22 +277,16 @@ RSpec.describe BillingPlansHelper do
   describe '#show_contact_sales_button?' do
     using RSpec::Parameterized::TableSyntax
 
-    where(:experiment_enabled, :link_action, :upgrade_offer, :result) do
-      true  | 'upgrade'     | :no_offer           | true
-      true  | 'upgrade'     | :upgrade_for_offer  | true
-      true  | 'no_upgrade'  | :no_offer           | false
-      true  | 'no_upgrade'  | :upgrade_for_offer  | false
-      false | 'upgrade'     | :no_offer           | false
-      false | 'upgrade'     | :upgrade_for_offer  | true
-      false | 'no_upgrade'  | :no_offer           | false
-      false | 'no_upgrade'  | :upgrade_for_offer  | false
+    where(:link_action, :upgrade_offer, :result) do
+      'upgrade'     | :no_offer           | true
+      'upgrade'     | :upgrade_for_free   | false
+      'upgrade'     | :upgrade_for_offer  | true
+      'no_upgrade'  | :no_offer           | false
+      'no_upgrade'  | :upgrade_for_free   | false
+      'no_upgrade'  | :upgrade_for_offer  | false
     end
 
     with_them do
-      before do
-        allow(helper).to receive(:experiment_enabled?).with(:contact_sales_btn_in_app).and_return(experiment_enabled)
-      end
-
       subject { helper.show_contact_sales_button?(link_action, upgrade_offer) }
 
       it { is_expected.to eq(result) }
@@ -318,39 +312,6 @@ RSpec.describe BillingPlansHelper do
     end
   end
 
-  describe '#experiment_tracking_data_for_button_click' do
-    let(:button_label) { 'some_label' }
-    let(:experiment_enabled) { false }
-
-    subject { helper.experiment_tracking_data_for_button_click(button_label) }
-
-    before do
-      stub_experiment(contact_sales_btn_in_app: experiment_enabled)
-    end
-
-    context 'when the experiment is not enabled' do
-      it { is_expected.to eq({}) }
-    end
-
-    context 'when the experiment is enabled' do
-      let(:experiment_enabled) { true }
-
-      before do
-        allow(helper).to receive(:experiment_tracking_category_and_group).with(:contact_sales_btn_in_app).and_return("Category:control_group")
-      end
-
-      it 'returns a hash to be used as data-attributes in a view' do
-        is_expected.to eq({
-          track: {
-            event: 'click_button',
-            label: button_label,
-            property: 'Category:control_group'
-          }
-        })
-      end
-    end
-  end
-
   describe '#plan_feature_list' do
     let(:plan) do
       Hashie::Mash.new(features: (1..3).map { |i| { title: "feat 0#{i}", highlight: i.even? } })
@@ -430,36 +391,48 @@ RSpec.describe BillingPlansHelper do
     end
   end
 
-  describe '#upgrade_button_css_classes' do
+  describe '#upgrade_button_text' do
     using RSpec::Parameterized::TableSyntax
 
-    let(:plan) { double('Plan', deprecated?: false) }
+    subject { helper.upgrade_button_text(plan_offer_type) }
 
-    it 'returns button-related classes only' do
-      expect(helper.upgrade_button_css_classes(nil, plan, false)).to eq('btn btn-success gl-button')
+    where(:plan_offer_type, :result) do
+      :no_offer           | 'Upgrade'
+      :upgrade_for_free   | 'Upgrade for free'
+      :upgrade_for_offer  | 'Upgrade'
     end
 
-    where(:is_current_plan, :trial_active, :result) do
-      false | false | 'btn btn-success gl-button'
-      false | true  | 'btn btn-success gl-button'
-      true  | true  | 'btn btn-success gl-button'
-      true  | false | 'btn btn-success gl-button disabled'
-      false | false | 'btn btn-success gl-button'
+    with_them do
+      it { is_expected.to eq(result) }
+    end
   end
 
-    with_them do
-      let(:namespace) { OpenStruct.new(trial_active: trial_active) }
+  describe '#upgrade_button_css_classes' do
+    let(:plan) { double('Plan', deprecated?: plan_is_deprecated) }
+    let(:namespace) { double('Namespace', trial_active?: trial_active) }
 
     subject { helper.upgrade_button_css_classes(namespace, plan, is_current_plan) }
 
-      it { is_expected.to include(result) }
+    before do
+      allow(helper).to receive(:use_new_purchase_flow?).and_return(use_new_purchase_flow)
     end
 
-    context 'when plan is deprecated' do
-      let(:deprecated_plan) { double('Plan', deprecated?: true) }
+    where(
+      is_current_plan: [true, false],
+      trial_active: [true, false],
+      plan_is_deprecated: [true, false],
+      use_new_purchase_flow: [true, false]
+    )
 
-      it 'returns invisible class' do
-        expect(helper.upgrade_button_css_classes(nil, deprecated_plan, false)).to include('invisible')
+    with_them do
+      it 'returns the expected list of CSS classes' do
+        expected_classes = [].tap do |ary|
+          ary << 'disabled' if is_current_plan && !trial_active
+          ary << 'invisible' if plan_is_deprecated
+          ary << "billing-cta-purchase#{'-new' if use_new_purchase_flow}"
+        end.join(' ')
+
+        is_expected.to eq(expected_classes)
       end
     end
   end

lib/gitlab/experimentation.rb

@@ -38,10 +38,6 @@ module Gitlab
         tracking_category: 'Growth::Expansion::Experiment::InviteMembersEmptyGroupVersionA',
         use_backwards_compatible_subject_index: true
       },
-      contact_sales_btn_in_app: {
-        tracking_category: 'Growth::Conversion::Experiment::ContactSalesInApp',
-        use_backwards_compatible_subject_index: true
-      },
       remove_known_trial_form_fields_welcoming: {
         tracking_category: 'Growth::Conversion::Experiment::RemoveKnownTrialFormFieldsWelcoming',
         rollout_strategy: :user

spec/lib/gitlab/experimentation_spec.rb

@@ -6,10 +6,7 @@ require 'spec_helper'
 # Originally created as part of https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45733 for https://gitlab.com/gitlab-org/gitlab/-/issues/270858.
 RSpec.describe Gitlab::Experimentation::EXPERIMENTS do
   it 'temporarily ensures we know what experiments exist for backwards compatibility' do
-    expected_experiment_keys = [
-      :invite_members_empty_group_version_a,
-      :contact_sales_btn_in_app
-    ]
+    expected_experiment_keys = [:invite_members_empty_group_version_a]
 
     backwards_compatible_experiment_keys = described_class.filter { |_, v| v[:use_backwards_compatible_subject_index] }.keys