Merge branch 'cleanup-invitation-reminders-experiment' into 'master'

Cleanup invitation_reminders experiment code

See merge request gitlab-org/gitlab!47920

app/controllers/invites_controller.rb

@@ -20,7 +20,6 @@ class InvitesController < ApplicationController
 
   def accept
     if member.accept_invite!(current_user)
-      track_invitation_reminders_experiment('accepted')
       redirect_to invite_details[:path], notice: _("You have been granted %{member_human_access} access to %{title} %{name}.") %
         { member_human_access: member.human_access, title: invite_details[:title], name: invite_details[:name] }
     else
@@ -107,8 +106,4 @@ class InvitesController < ApplicationController
                           }
                         end
   end
-
-  def track_invitation_reminders_experiment(action)
-    track_experiment_event(:invitation_reminders, action, subject: member)
-  end
 end

app/mailers/emails/members.rb

@@ -63,15 +63,6 @@ module Emails
         subject: subject_line,
         layout: 'unknown_user_mailer'
       )
-
-      if Gitlab::Experimentation.active?(:invitation_reminders)
-        Gitlab::Tracking.event(
-          Gitlab::Experimentation.get_experiment(:invitation_reminders).tracking_category,
-          'sent',
-          property: Gitlab::Experimentation.in_experiment_group?(:invitation_reminders, subject: member.invite_email) ? 'experimental_group' : 'control_group',
-          label: Digest::MD5.hexdigest(member.to_global_id.to_s)
-        )
-      end
     end
 
     def member_invited_reminder_email(member_source_type, member_id, token, reminder_index)

app/services/members/invitation_reminder_email_service.rb

@@ -14,8 +14,6 @@ module Members
     end
 
     def execute
-      return unless experiment_enabled?
-
       reminder_index = days_on_which_to_send_reminders.index(days_after_invitation_sent)
       return unless reminder_index
 
@@ -24,10 +22,6 @@ module Members
 
     private
 
-    def experiment_enabled?
-      Gitlab::Experimentation.in_experiment_group?(:invitation_reminders, subject: invitation.invite_email)
-    end
-
     def days_after_invitation_sent
       (Date.today - invitation.created_at.to_date).to_i
     end

app/workers/member_invitation_reminder_emails_worker.rb

@@ -8,8 +8,6 @@ class MemberInvitationReminderEmailsWorker # rubocop:disable Scalability/Idempot
   urgency :low
 
   def perform
-    return unless Gitlab::Experimentation.active?(:invitation_reminders)
-
     Member.not_accepted_invitations.not_expired.last_ten_days_excluding_today.find_in_batches do |invitations|
       invitations.each do |invitation|
         Members::InvitationReminderEmailService.new(invitation).execute

changelogs/unreleased/cleanup-invitation-reminders-experiment.yml

+---
+title: Add invitation reminders
+merge_request: 47920
+author:
+type: added

doc/user/project/members/index.md

@@ -96,6 +96,9 @@ invitation, change their access level, or even delete them.
 
 ![Invite user members list](img/add_user_email_accept.png)
 
+While unaccepted, the system will automatically send reminder emails on the second, fifth,
+and tenth day after the invitation was initially sent.
+
 After the user accepts the invitation, they are prompted to create a new
 GitLab account using the same e-mail address the invitation was sent to.
 

lib/gitlab/experimentation.rb

@@ -66,10 +66,6 @@ module Gitlab
         tracking_category: 'Growth::Acquisition::Experiment::InviteEmail',
         use_backwards_compatible_subject_index: true
       },
-      invitation_reminders: {
-        tracking_category: 'Growth::Acquisition::Experiment::InvitationReminders',
-        use_backwards_compatible_subject_index: true
-      },
       group_only_trials: {
         tracking_category: 'Growth::Conversion::Experiment::GroupOnlyTrials',
         use_backwards_compatible_subject_index: true

spec/controllers/invites_controller_spec.rb

@@ -22,43 +22,6 @@ RSpec.describe InvitesController, :snowplow do
     end
   end
 
-  shared_examples "tracks the 'accepted' event for the invitation reminders experiment" do
-    before do
-      stub_experiment(invitation_reminders: true)
-      stub_experiment_for_subject(invitation_reminders: experimental_group)
-    end
-
-    context 'when in the control group' do
-      let(:experimental_group) { false }
-
-      it "tracks the 'accepted' event" do
-        request
-
-        expect_snowplow_event(
-          category: 'Growth::Acquisition::Experiment::InvitationReminders',
-          label: md5_member_global_id,
-          property: 'control_group',
-          action: 'accepted'
-        )
-      end
-    end
-
-    context 'when in the experimental group' do
-      let(:experimental_group) { true }
-
-      it "tracks the 'accepted' event" do
-        request
-
-        expect_snowplow_event(
-          category: 'Growth::Acquisition::Experiment::InvitationReminders',
-          label: md5_member_global_id,
-          property: 'experimental_group',
-          action: 'accepted'
-        )
-      end
-    end
-  end
-
   describe 'GET #show' do
     subject(:request) { get :show, params: params }
 
@@ -87,7 +50,6 @@ RSpec.describe InvitesController, :snowplow do
         expect(flash[:notice]).to be_nil
       end
 
-      it_behaves_like "tracks the 'accepted' event for the invitation reminders experiment"
       it_behaves_like 'invalid token'
     end
 
@@ -119,7 +81,6 @@ RSpec.describe InvitesController, :snowplow do
 
     subject(:request) { post :accept, params: params }
 
-    it_behaves_like "tracks the 'accepted' event for the invitation reminders experiment"
     it_behaves_like 'invalid token'
   end
 

spec/lib/gitlab/experimentation_spec.rb

@@ -16,7 +16,6 @@ RSpec.describe Gitlab::Experimentation::EXPERIMENTS do
       :contact_sales_btn_in_app,
       :customize_homepage,
       :invite_email,
-      :invitation_reminders,
       :group_only_trials,
       :default_to_issues_board
     ]

spec/mailers/notify_spec.rb

@@ -1450,38 +1450,6 @@ RSpec.describe Notify do
 
       subject { described_class.member_invited_email('Group', group_member.id, group_member.invite_token) }
 
-      shared_examples "tracks the 'sent' event for the invitation reminders experiment" do
-        before do
-          stub_experiment(invitation_reminders: true)
-          allow(Gitlab::Experimentation).to receive(:in_experiment_group?).with(:invitation_reminders, subject: group_member.invite_email).and_return(experimental_group)
-        end
-
-        it "tracks the 'sent' event", :snowplow do
-          subject.deliver_now
-
-          expect_snowplow_event(
-            category: 'Growth::Acquisition::Experiment::InvitationReminders',
-            label: Digest::MD5.hexdigest(group_member.to_global_id.to_s),
-            property: experimental_group ? 'experimental_group' : 'control_group',
-            action: 'sent'
-          )
-        end
-      end
-
-      describe 'tracking for the invitation reminders experiment' do
-        context 'when invite email is in the experimental group' do
-          let(:experimental_group) { true }
-
-          it_behaves_like "tracks the 'sent' event for the invitation reminders experiment"
-        end
-
-        context 'when invite email is in the control group' do
-          let(:experimental_group) { false }
-
-          it_behaves_like "tracks the 'sent' event for the invitation reminders experiment"
-        end
-      end
-
       it_behaves_like 'an email sent from GitLab'
       it_behaves_like 'it should not have Gmail Actions links'
       it_behaves_like "a user cannot unsubscribe through footer link"

spec/services/members/invitation_reminder_email_service_spec.rb

@@ -9,67 +9,49 @@ RSpec.describe Members::InvitationReminderEmailService do
     let_it_be(:frozen_time) { Date.today.beginning_of_day }
     let_it_be(:invitation) { build(:group_member, :invited, created_at: frozen_time) }
 
-    context 'when the experiment is disabled' do
-      before do
-        allow(Gitlab::Experimentation).to receive(:in_experiment_group?).and_return(false)
-        invitation.expires_at = frozen_time + 2.days
-      end
-
-      it 'does not send an invitation' do
-        travel_to(frozen_time + 1.day) do
-          expect(invitation).not_to receive(:send_invitation_reminder)
-
-          subject
-        end
-      end
+    before do
+      invitation.expires_at = frozen_time + expires_at_days.days if expires_at_days
     end
 
-    context 'when the experiment is enabled' do
-      before do
-        allow(Gitlab::Experimentation).to receive(:in_experiment_group?).and_return(true)
-        invitation.expires_at = frozen_time + expires_at_days.days if expires_at_days
-      end
-
-      using RSpec::Parameterized::TableSyntax
-
-      where(:expires_at_days, :send_reminder_at_days) do
-        0   | []
-        1   | []
-        2   | [1]
-        3   | [1, 2]
-        4   | [1, 2, 3]
-        5   | [1, 2, 4]
-        6   | [1, 3, 5]
-        7   | [1, 3, 5]
-        8   | [2, 3, 6]
-        9   | [2, 4, 7]
-        10  | [2, 4, 8]
-        11  | [2, 4, 8]
-        12  | [2, 5, 9]
-        13  | [2, 5, 10]
-        14  | [2, 5, 10]
-        15  | [2, 5, 10]
-        nil | [2, 5, 10]
-      end
-
-      with_them do
-        # Create an invitation today with an expiration date from 0 to 10 days in the future or without an expiration date
-        # We chose 10 days here, because we fetch invitations that were created at most 10 days ago.
-        (0..10).each do |day|
-          it 'sends an invitation reminder only on the expected days' do
-            next if day > (expires_at_days || 10) # We don't need to test after the invitation has already expired
-
-            # We are traveling in a loop from today to 10 days from now
-            travel_to(frozen_time + day.days) do
-              # Given an expiration date and the number of days after the creation of the invitation based on the current day in the loop, a reminder may be sent
-              if (reminder_index = send_reminder_at_days.index(day))
-                expect(invitation).to receive(:send_invitation_reminder).with(reminder_index)
-              else
-                expect(invitation).not_to receive(:send_invitation_reminder)
-              end
+    using RSpec::Parameterized::TableSyntax
+
+    where(:expires_at_days, :send_reminder_at_days) do
+      0   | []
+      1   | []
+      2   | [1]
+      3   | [1, 2]
+      4   | [1, 2, 3]
+      5   | [1, 2, 4]
+      6   | [1, 3, 5]
+      7   | [1, 3, 5]
+      8   | [2, 3, 6]
+      9   | [2, 4, 7]
+      10  | [2, 4, 8]
+      11  | [2, 4, 8]
+      12  | [2, 5, 9]
+      13  | [2, 5, 10]
+      14  | [2, 5, 10]
+      15  | [2, 5, 10]
+      nil | [2, 5, 10]
+    end
 
-              subject
+    with_them do
+      # Create an invitation today with an expiration date from 0 to 10 days in the future or without an expiration date
+      # We chose 10 days here, because we fetch invitations that were created at most 10 days ago.
+      (0..10).each do |day|
+        it 'sends an invitation reminder only on the expected days' do
+          next if day > (expires_at_days || 10) # We don't need to test after the invitation has already expired
+
+          # We are traveling in a loop from today to 10 days from now
+          travel_to(frozen_time + day.days) do
+            # Given an expiration date and the number of days after the creation of the invitation based on the current day in the loop, a reminder may be sent
+            if (reminder_index = send_reminder_at_days.index(day))
+              expect(invitation).to receive(:send_invitation_reminder).with(reminder_index)
+            else
+              expect(invitation).not_to receive(:send_invitation_reminder)
             end
+
+            subject
           end
         end
       end

spec/workers/member_invitation_reminder_emails_worker_spec.rb

@@ -10,30 +10,12 @@ RSpec.describe MemberInvitationReminderEmailsWorker do
       create(:group_member, :invited, created_at: 2.days.ago)
     end
 
-    context 'feature flag disabled' do
-      before do
-        stub_experiment(invitation_reminders: false)
+    it 'executes the invitation reminder email service' do
+      expect_next_instance_of(Members::InvitationReminderEmailService) do |service|
+        expect(service).to receive(:execute)
       end
 
-      it 'does not attempt to execute the invitation reminder service' do
-        expect(Members::InvitationReminderEmailService).not_to receive(:new)
-
-        subject
-      end
-    end
-
-    context 'feature flag enabled' do
-      before do
-        stub_experiment(invitation_reminders: true)
-      end
-
-      it 'executes the invitation reminder email service' do
-        expect_next_instance_of(Members::InvitationReminderEmailService) do |service|
-          expect(service).to receive(:execute)
-        end
-
-        subject
-      end
+      subject
     end
   end
 end