Commit 242d61a9 authored by Vitali Tatarintev's avatar Vitali Tatarintev

Merge branch '327636-engineering-change-invite-email-to-be-from-inviter' into 'master'

Experiment: change invite email to be from inviter

See merge request gitlab-org/gitlab!68376
parents 39a981de cce68d18
......@@ -77,7 +77,12 @@ class InvitesController < ApplicationController
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
......
......@@ -201,6 +201,7 @@ class RegistrationsController < Devise::RegistrationsController
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
......
......@@ -24,7 +24,14 @@ module NotifyHelper
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
......
......@@ -57,7 +57,7 @@ module Emails
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 @@ module Emails
def invite_email_subject
if member.created_by
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 @@ module Emails
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?
......
---
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
......@@ -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 ""
......
......@@ -120,6 +120,29 @@ RSpec.describe InvitesController do
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 @@ RSpec.describe InvitesController do
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
......
......@@ -227,6 +227,40 @@ RSpec.describe RegistrationsController do
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
......
......@@ -216,6 +216,20 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do
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
......
......@@ -70,6 +70,28 @@ RSpec.describe NotifyHelper do
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
......
......@@ -834,6 +834,35 @@ RSpec.describe Notify do
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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment