Merge branch '300728-add-tracking-to-the-trial-status-sidebar-widget-experiment' into 'master'

Record trial status widget experiment variant assignments

See merge request gitlab-org/gitlab!53301

ee/app/helpers/trial_status_widget_helper.rb

 # frozen_string_literal: true
 
 module TrialStatusWidgetHelper
-  def show_trial_status_widget?(group)
-    billing_plans_and_trials_available? &&
-      trial_status_widget_experiment_enabled?(group) &&
-      group.trial_active? &&
-      user_can_administer_group?(group)
-  end
-
   def plan_title_for_group(group)
     group.gitlab_subscription&.plan_title
   end
 
+  def show_trial_status_widget?(group)
+    billing_plans_and_trials_available? && eligible_for_trial_status_widget?(group)
+  end
+
   private
 
   def billing_plans_and_trials_available?
     ::Gitlab::CurrentSettings.should_check_namespace_plan?
   end
 
-  def trial_status_widget_experiment_enabled?(group)
-    experiment_enabled?(:show_trial_status_in_sidebar, subject: group)
-  end
-
-  def user_can_administer_group?(group)
-    can?(current_user, :admin_namespace, group)
+  def eligible_for_trial_status_widget?(group)
+    group.trial_active? && can?(current_user, :admin_namespace, group)
   end
 end

ee/app/views/layouts/nav/sidebar/_group_trial_status_widget.html.haml

+-# Only top-level groups can have trials & plans
 - root_group = group.root_ancestor
 - return unless show_trial_status_widget?(root_group)
 
+-# For the current Growth::Conversion experiment
+- experiment_key = :show_trial_status_in_sidebar
+- record_experiment_group(experiment_key, root_group)
+- return unless experiment_enabled?(experiment_key, subject: root_group)
+
 = nav_link do
   #js-trial-status-widget{ data: { container_id: 'trial-status-sidebar-widget',
     days_remaining: root_group.trial_days_remaining,
@@ -13,6 +19,6 @@
     group_name: root_group.name,
     plan_name: plan_title_for_group(root_group),
     plans_href: group_billings_path(root_group),
-    purchase_href: new_subscriptions_path(namespace_id: group.id, plan_id: '2c92a0fc5a83f01d015aa6db83c45aac'),
+    purchase_href: new_subscriptions_path(namespace_id: root_group.id, plan_id: '2c92a0fc5a83f01d015aa6db83c45aac'),
     target_id: 'trial-status-sidebar-widget',
     trial_end_date: root_group.trial_ends_on } }

ee/spec/helpers/trial_status_widget_helper_spec.rb

@@ -3,61 +3,6 @@
 require 'spec_helper'
 
 RSpec.describe TrialStatusWidgetHelper do
-  describe '#show_trial_status_widget?' do
-    let_it_be(:user) { create(:user) }
-
-    let(:trials_available) { true }
-    let(:experiment_enabled) { true }
-    let(:trial_active) { true }
-    let(:user_can_admin_group) { true }
-    let(:group) { instance_double(Group, trial_active?: trial_active) }
-
-    before do
-      # current_user
-      allow(helper).to receive(:current_user).and_return(user)
-
-      # billing_plans_and_trials_available?
-      stub_application_setting(check_namespace_plan: trials_available)
-
-      # trial_status_widget_experiment_enabled?(group)
-      allow(helper).to receive(:experiment_enabled?).with(:show_trial_status_in_sidebar, subject: group).and_return(experiment_enabled)
-
-      # user_can_administer_group?(group)
-      allow(helper).to receive(:can?).and_call_original
-      allow(helper).to receive(:can?).with(user, :admin_namespace, group).and_return(user_can_admin_group)
-    end
-
-    subject { helper.show_trial_status_widget?(group) }
-
-    context 'when all requirements are met for the widget to be shown' do
-      it { is_expected.to be_truthy }
-    end
-
-    context 'when the app is not configured for billing plans & trials' do
-      let(:trials_available) { false }
-
-      it { is_expected.to be_falsey }
-    end
-
-    context 'when the experiment is not active or not enabled for the group' do
-      let(:experiment_enabled) { false }
-
-      it { is_expected.to be_falsey }
-    end
-
-    context 'when the group is not in an active trial' do
-      let(:trial_active) { false }
-
-      it { is_expected.to be_falsey }
-    end
-
-    context 'when the user is not an admin/owner of the group' do
-      let(:user_can_admin_group) { false }
-
-      it { is_expected.to be_falsey }
-    end
-  end
-
   describe '#plan_title_for_group' do
     using RSpec::Parameterized::TableSyntax
 
@@ -79,4 +24,28 @@ RSpec.describe TrialStatusWidgetHelper do
       it { is_expected.to eq(title) }
     end
   end
+
+  describe '#show_trial_status_widget?' do
+    let(:user) { instance_double(User) }
+    let(:group) { instance_double(Group, trial_active?: trial_active) }
+
+    before do
+      stub_application_setting(check_namespace_plan: trials_available)
+      allow(helper).to receive(:current_user).and_return(user)
+      allow(helper).to receive(:can?).and_call_original
+      allow(helper).to receive(:can?).with(user, :admin_namespace, group).and_return(user_can_admin_group)
+    end
+
+    subject { helper.show_trial_status_widget?(group) }
+
+    where(
+      trials_available: [true, false],
+      trial_active: [true, false],
+      user_can_admin_group: [true, false]
+    )
+
+    with_them do
+      it { is_expected.to eq(trials_available && trial_active && user_can_admin_group) }
+    end
+  end
 end

ee/spec/views/layouts/nav/sidebar/_group.html.haml_spec.rb

@@ -12,26 +12,33 @@ RSpec.describe 'layouts/nav/sidebar/_group' do
   let(:user) { create(:user) }
 
   describe 'trial status widget', :aggregate_failures do
+    using RSpec::Parameterized::TableSyntax
+
     let!(:gitlab_subscription) { create(:gitlab_subscription, :active_trial, namespace: group) }
+
+    let(:experiment_key) { :show_trial_status_in_sidebar }
     let(:show_widget) { false }
+    let(:experiment_enabled) { false }
 
     before do
       allow(view).to receive(:show_trial_status_widget?).and_return(show_widget)
-      render
+      allow(view).to receive(:experiment_enabled?).with(experiment_key, subject: group).and_return(experiment_enabled)
+      allow(view).to receive(:record_experiment_group)
     end
 
-    subject { rendered }
+    subject do
+      render
+      rendered
+    end
 
-    context 'when the widget should not be shown' do
+    shared_examples 'does not render the widget & popover' do
       it 'does not render' do
         is_expected.not_to have_selector '#js-trial-status-widget'
         is_expected.not_to have_selector '#js-trial-status-popover'
       end
     end
 
-    context 'when the widget should be shown' do
-      let(:show_widget) { true }
-
+    shared_examples 'renders the widget & popover' do
       it 'renders both the widget & popover component initialization elements' do
         is_expected.to have_selector '#js-trial-status-widget'
         is_expected.to have_selector '#js-trial-status-popover'
@@ -44,6 +51,35 @@ RSpec.describe 'layouts/nav/sidebar/_group' do
         is_expected.to have_selector "[data-target-id=#{expected_id}]"
       end
     end
+
+    shared_examples 'does record experiment subject' do
+      it 'records the group as an experiment subject' do
+        expect(view).to receive(:record_experiment_group).with(experiment_key, group)
+
+        subject
+      end
+    end
+
+    shared_examples 'does not record experiment subject' do
+      it 'does not record the group as an experiment subject' do
+        expect(view).not_to receive(:record_experiment_group)
+
+        subject
+      end
+    end
+
+    where :show_widget, :experiment_enabled, :examples_to_include do
+      true  | true  | ['does record experiment subject', 'renders the widget & popover']
+      true  | false | ['does record experiment subject', 'does not render the widget & popover']
+      false | true  | ['does not record experiment subject', 'does not render the widget & popover']
+      false | false | ['does not record experiment subject', 'does not render the widget & popover']
+    end
+
+    with_them do
+      params[:examples_to_include].each do |example_set|
+        include_examples(example_set)
+      end
+    end
   end
 
   describe 'DevOps adoption link' do

lib/gitlab/experimentation/controller_concern.rb

@@ -15,7 +15,7 @@ module Gitlab
 
       included do
         before_action :set_experimentation_subject_id_cookie, unless: :dnt_enabled?
-        helper_method :experiment_enabled?, :experiment_tracking_category_and_group, :tracking_label
+        helper_method :experiment_enabled?, :experiment_tracking_category_and_group, :record_experiment_group, :tracking_label
       end
 
       def set_experimentation_subject_id_cookie
@@ -72,6 +72,16 @@ module Gitlab
         ::Experiment.add_user(experiment_key, tracking_group(experiment_key, nil, subject: subject), current_user, context)
       end
 
+      def record_experiment_group(experiment_key, group)
+        return if dnt_enabled?
+        return unless Experimentation.active?(experiment_key) && group
+
+        variant_subject = Experimentation.rollout_strategy(experiment_key) == :cookie ? nil : group
+        variant = tracking_group(experiment_key, nil, subject: variant_subject)
+
+        ::Experiment.add_group(experiment_key, group: group, variant: variant)
+      end
+
       def record_experiment_conversion_event(experiment_key, context = {})
         return if dnt_enabled?
         return unless current_user

spec/lib/gitlab/experimentation/controller_concern_spec.rb

@@ -520,6 +520,78 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do
     end
   end
 
+  describe '#record_experiment_group' do
+    let(:group) { 'a group object' }
+    let(:experiment_key) { :some_experiment_key }
+    let(:dnt_enabled) { false }
+    let(:experiment_active) { true }
+    let(:rollout_strategy) { :whatever }
+    let(:variant) { 'variant' }
+
+    before do
+      allow(controller).to receive(:dnt_enabled?).and_return(dnt_enabled)
+      allow(::Gitlab::Experimentation).to receive(:active?).and_return(experiment_active)
+      allow(::Gitlab::Experimentation).to receive(:rollout_strategy).and_return(rollout_strategy)
+      allow(controller).to receive(:tracking_group).and_return(variant)
+      allow(::Experiment).to receive(:add_group)
+    end
+
+    subject(:record_experiment_group) { controller.record_experiment_group(experiment_key, group) }
+
+    shared_examples 'exits early without recording' do
+      it 'returns early without recording the group as an ExperimentSubject' do
+        expect(::Experiment).not_to receive(:add_group)
+        record_experiment_group
+      end
+    end
+
+    shared_examples 'calls tracking_group' do |using_cookie_rollout|
+      it "calls tracking_group with #{using_cookie_rollout ? 'a nil' : 'the group as the'} subject" do
+        expect(controller).to receive(:tracking_group).with(experiment_key, nil, subject: using_cookie_rollout ? nil : group).and_return(variant)
+        record_experiment_group
+      end
+    end
+
+    shared_examples 'records the group' do
+      it 'records the group' do
+        expect(::Experiment).to receive(:add_group).with(experiment_key, group: group, variant: variant)
+        record_experiment_group
+      end
+    end
+
+    context 'when DNT is enabled' do
+      let(:dnt_enabled) { true }
+
+      include_examples 'exits early without recording'
+    end
+
+    context 'when the experiment is not active' do
+      let(:experiment_active) { false }
+
+      include_examples 'exits early without recording'
+    end
+
+    context 'when a nil group is given' do
+      let(:group) { nil }
+
+      include_examples 'exits early without recording'
+    end
+
+    context 'when the experiment uses a cookie-based rollout strategy' do
+      let(:rollout_strategy) { :cookie }
+
+      include_examples 'calls tracking_group', true
+      include_examples 'records the group'
+    end
+
+    context 'when the experiment uses a non-cookie-based rollout strategy' do
+      let(:rollout_strategy) { :group }
+
+      include_examples 'calls tracking_group', false
+      include_examples 'records the group'
+    end
+  end
+
   describe '#record_experiment_conversion_event' do
     let(:user) { build(:user) }