From cce68d18820f9feed1927d8f5f3ee91202d10c98 Mon Sep 17 00:00:00 2001
From: Doug Stull <dstull@gitlab.com>
Date: Wed, 25 Aug 2021 08:57:06 +0000
Subject: [PATCH] Experiment: change invite email to be from inviter
---
app/controllers/invites_controller.rb | 7 +++-
app/controllers/registrations_controller.rb | 1 +
app/helpers/notify_helper.rb | 9 ++++-
app/mailers/emails/members.rb | 29 ++++++++++++++--
.../experiment/invite_email_from.yml | 8 +++++
locale/gitlab.pot | 3 ++
spec/controllers/invites_controller_spec.rb | 26 +++++++++++++-
.../registrations_controller_spec.rb | 34 +++++++++++++++++++
spec/features/invites_spec.rb | 14 ++++++++
spec/helpers/notify_helper_spec.rb | 22 ++++++++++++
spec/mailers/notify_spec.rb | 29 ++++++++++++++++
11 files changed, 177 insertions(+), 5 deletions(-)
create mode 100644 config/feature_flags/experiment/invite_email_from.yml
diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb
index 7f5750d20114..4242f918ea0b 100644
--- a/app/controllers/invites_controller.rb
+++ b/app/controllers/invites_controller.rb
@@ -77,7 +77,12 @@ def ensure_member_exists
def track_invite_join_click
return unless member && initial_invite_email?
- experiment(:invite_email_preview_text, actor: member).track(:join_clicked) if params[:experiment_name] == 'invite_email_preview_text'
+ if params[:experiment_name] == 'invite_email_preview_text'
+ experiment(:invite_email_preview_text, actor: member).track(:join_clicked)
+ elsif params[:experiment_name] == 'invite_email_from'
+ experiment(:invite_email_from, actor: member).track(:join_clicked)
+ end
+
Gitlab::Tracking.event(self.class.name, 'join_clicked', label: 'invite_email', property: member.id.to_s)
end
diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb
index cc985e845426..3dffbbf3108c 100644
--- a/app/controllers/registrations_controller.rb
+++ b/app/controllers/registrations_controller.rb
@@ -201,6 +201,7 @@ def after_pending_invitations_hook
experiment_name = session.delete(:invite_email_experiment_name)
experiment(:invite_email_preview_text, actor: member).track(:accepted) if experiment_name == 'invite_email_preview_text'
+ experiment(:invite_email_from, actor: member).track(:accepted) if experiment_name == 'invite_email_from'
Gitlab::Tracking.event(self.class.name, 'accepted', label: 'invite_email', property: member.id.to_s)
end
diff --git a/app/helpers/notify_helper.rb b/app/helpers/notify_helper.rb
index da42740f9939..ed96f3cef4fc 100644
--- a/app/helpers/notify_helper.rb
+++ b/app/helpers/notify_helper.rb
@@ -24,7 +24,14 @@ def invited_to_description(source)
def invited_join_url(token, member)
additional_params = { invite_type: Emails::Members::INITIAL_INVITE }
- if experiment(:invite_email_preview_text, actor: member).enabled?
+ # order important below to our scheduled testing of these
+ # `from` experiment will be after the `text` on, but we may not cleanup
+ # from the `text` one by the time we run the `from` experiment,
+ # therefore we want to support `text` being fully enabled
+ # but if `from` is also enabled, then we only care about `from`
+ if experiment(:invite_email_from, actor: member).enabled?
+ additional_params[:experiment_name] = 'invite_email_from'
+ elsif experiment(:invite_email_preview_text, actor: member).enabled?
additional_params[:experiment_name] = 'invite_email_preview_text'
end
diff --git a/app/mailers/emails/members.rb b/app/mailers/emails/members.rb
index fe2d28915477..53b38cc59725 100644
--- a/app/mailers/emails/members.rb
+++ b/app/mailers/emails/members.rb
@@ -57,7 +57,7 @@ def member_invited_email(member_source_type, member_id, token)
Gitlab::Tracking.event(self.class.name, 'invite_email_sent', label: 'invite_email', property: member_id.to_s)
- mail(to: member.invite_email, subject: invite_email_subject, **invite_email_headers) do |format|
+ mail(to: member.invite_email, subject: invite_email_subject, **invite_email_headers.merge(additional_invite_settings)) do |format|
format.html { render layout: 'unknown_user_mailer' }
format.text { render layout: 'unknown_user_mailer' }
end
@@ -147,7 +147,17 @@ def notification_group
def invite_email_subject
if member.created_by
- subject(s_("MemberInviteEmail|%{member_name} invited you to join GitLab") % { member_name: member.created_by.name })
+ experiment(:invite_email_from, actor: member) do |experiment_instance|
+ experiment_instance.use do
+ subject(s_("MemberInviteEmail|%{member_name} invited you to join GitLab") % { member_name: member.created_by.name })
+ end
+
+ experiment_instance.candidate do
+ subject(s_("MemberInviteEmail|I've invited you to join me in GitLab"))
+ end
+
+ experiment_instance.run
+ end
else
subject(s_("MemberInviteEmail|Invitation to join the %{project_or_group} %{project_or_group_name}") % { project_or_group: member_source.human_name, project_or_group_name: member_source.model_name.singular })
end
@@ -164,6 +174,21 @@ def invite_email_headers
end
end
+ def additional_invite_settings
+ return {} unless member.created_by
+
+ experiment(:invite_email_from, actor: member) do |experiment_instance|
+ experiment_instance.use { {} }
+ experiment_instance.candidate do
+ {
+ from: "#{member.created_by.name} <#{member.created_by.email}>"
+ }
+ end
+
+ experiment_instance.run
+ end
+ end
+
def member_exists?
Gitlab::AppLogger.info("Tried to send an email invitation for a deleted group. Member id: #{@member_id}") if member.blank?
member.present?
diff --git a/config/feature_flags/experiment/invite_email_from.yml b/config/feature_flags/experiment/invite_email_from.yml
new file mode 100644
index 000000000000..59baf2493411
--- /dev/null
+++ b/config/feature_flags/experiment/invite_email_from.yml
@@ -0,0 +1,8 @@
+---
+name: invite_email_from
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68376
+rollout_issue_url: https://gitlab.com/gitlab-org/growth/team-tasks/-/issues/429
+milestone: '14.3'
+type: experiment
+group: group::expansion
+default_enabled: false
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 33ee854e7a92..16369188a581 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -20784,6 +20784,9 @@ msgstr ""
msgid "MemberInviteEmail|%{member_name} invited you to join GitLab"
msgstr ""
+msgid "MemberInviteEmail|I've invited you to join me in GitLab"
+msgstr ""
+
msgid "MemberInviteEmail|Invitation to join the %{project_or_group} %{project_or_group_name}"
msgstr ""
diff --git a/spec/controllers/invites_controller_spec.rb b/spec/controllers/invites_controller_spec.rb
index dc1fb0454dfc..d4091461062c 100644
--- a/spec/controllers/invites_controller_spec.rb
+++ b/spec/controllers/invites_controller_spec.rb
@@ -120,6 +120,29 @@
end
end
+ context 'when it is part of the invite_email_from experiment' do
+ let(:extra_params) { { invite_type: 'initial_email', experiment_name: 'invite_email_from' } }
+
+ it 'tracks the initial join click from email' do
+ experiment = double(track: true)
+ allow(controller).to receive(:experiment).with(:invite_email_from, actor: member).and_return(experiment)
+
+ request
+
+ expect(experiment).to have_received(:track).with(:join_clicked)
+ end
+
+ context 'when member does not exist' do
+ let(:raw_invite_token) { '_bogus_token_' }
+
+ it 'does not track the experiment' do
+ expect(controller).not_to receive(:experiment).with(:invite_email_from, actor: member)
+
+ request
+ end
+ end
+ end
+
context 'when member does not exist' do
let(:raw_invite_token) { '_bogus_token_' }
@@ -147,8 +170,9 @@
end
context 'when it is not part of our invite email experiment' do
- it 'does not track via experiment' do
+ it 'does not track via experiment', :aggregate_failures do
expect(controller).not_to receive(:experiment).with(:invite_email_preview_text, actor: member)
+ expect(controller).not_to receive(:experiment).with(:invite_email_from, actor: member)
request
end
diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb
index 301c60e89c8a..a5a0f16f2b17 100644
--- a/spec/controllers/registrations_controller_spec.rb
+++ b/spec/controllers/registrations_controller_spec.rb
@@ -227,6 +227,40 @@
end
end
end
+
+ context 'with the invite_email_preview_text experiment', :experiment do
+ let(:extra_session_params) { { invite_email_experiment_name: 'invite_email_from' } }
+
+ context 'when member and invite_email_experiment_name exists from the session key value' do
+ it 'tracks the invite acceptance' do
+ expect(experiment(:invite_email_from)).to track(:accepted)
+ .with_context(actor: member)
+ .on_next_instance
+
+ subject
+ end
+ end
+
+ context 'when member does not exist from the session key value' do
+ let(:originating_member_id) { -1 }
+
+ it 'does not track invite acceptance' do
+ expect(experiment(:invite_email_from)).not_to track(:accepted)
+
+ subject
+ end
+ end
+
+ context 'when invite_email_experiment_name does not exist from the session key value' do
+ let(:extra_session_params) { {} }
+
+ it 'does not track invite acceptance' do
+ expect(experiment(:invite_email_from)).not_to track(:accepted)
+
+ subject
+ end
+ end
+ end
end
context 'when invite email matches email used on registration' do
diff --git a/spec/features/invites_spec.rb b/spec/features/invites_spec.rb
index d56bedd4852b..583daba37f15 100644
--- a/spec/features/invites_spec.rb
+++ b/spec/features/invites_spec.rb
@@ -216,6 +216,20 @@ def fill_in_welcome_form
end
end
+ context 'with invite email acceptance for the invite_email_from experiment', :experiment do
+ let(:extra_params) do
+ { invite_type: Emails::Members::INITIAL_INVITE, experiment_name: 'invite_email_from' }
+ end
+
+ it 'tracks the accepted invite' do
+ expect(experiment(:invite_email_from)).to track(:accepted)
+ .with_context(actor: group_invite)
+ .on_next_instance
+
+ fill_in_sign_up_form(new_user)
+ end
+ end
+
it 'signs up and redirects to the group activity page with all the project/groups invitation automatically accepted' do
fill_in_sign_up_form(new_user)
fill_in_welcome_form
diff --git a/spec/helpers/notify_helper_spec.rb b/spec/helpers/notify_helper_spec.rb
index 633a4b65139f..a4193444528f 100644
--- a/spec/helpers/notify_helper_spec.rb
+++ b/spec/helpers/notify_helper_spec.rb
@@ -70,6 +70,28 @@ def reference_link(entity, url)
expect(helper.invited_join_url(token, member))
.to eq("http://test.host/-/invites/#{token}?experiment_name=invite_email_preview_text&invite_type=initial_email")
end
+
+ context 'when invite_email_from is enabled' do
+ before do
+ stub_experiments(invite_email_from: :control)
+ end
+
+ it 'has correct params' do
+ expect(helper.invited_join_url(token, member))
+ .to eq("http://test.host/-/invites/#{token}?experiment_name=invite_email_from&invite_type=initial_email")
+ end
+ end
+ end
+
+ context 'when invite_email_from is enabled' do
+ before do
+ stub_experiments(invite_email_from: :control)
+ end
+
+ it 'has correct params' do
+ expect(helper.invited_join_url(token, member))
+ .to eq("http://test.host/-/invites/#{token}?experiment_name=invite_email_from&invite_type=initial_email")
+ end
end
context 'when invite_email_preview_text is disabled' do
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index 5d2b136043ec..ecff5c15816b 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -834,6 +834,35 @@ def invite_to_project(project, inviter:, user: nil)
invite_type: Emails::Members::INITIAL_INVITE,
experiment_name: 'invite_email_preview_text'))
end
+
+ it 'tracks the sent invite' do
+ expect(experiment(:invite_email_preview_text)).to track(:assignment)
+ .with_context(actor: project_member)
+ .on_next_instance
+
+ invite_email.deliver_now
+ end
+ end
+
+ context 'with invite_email_from enabled', :experiment do
+ before do
+ stub_experiments(invite_email_from: :control)
+ end
+
+ it 'has the correct invite_url with params' do
+ is_expected.to have_link('Join now',
+ href: invite_url(project_member.invite_token,
+ invite_type: Emails::Members::INITIAL_INVITE,
+ experiment_name: 'invite_email_from'))
+ end
+
+ it 'tracks the sent invite' do
+ expect(experiment(:invite_email_from)).to track(:assignment)
+ .with_context(actor: project_member)
+ .on_next_instance
+
+ invite_email.deliver_now
+ end
end
context 'when invite email sent is tracked', :snowplow do
--
GitLab