From a93571a45876698c2af4893be1efc01dd347195d Mon Sep 17 00:00:00 2001
From: Doug Stull <dstull@gitlab.com>
Date: Wed, 15 Dec 2021 18:00:22 +0000
Subject: [PATCH] Add preview text to invit emails with goto action
---
app/controllers/invites_controller.rb | 5 ---
app/controllers/registrations_controller.rb | 2 -
app/helpers/notify_helper.rb | 10 -----
.../notify/member_invited_email.html.haml | 20 +++++-----
.../experiment/invite_email_preview_text.yml | 8 ----
spec/controllers/invites_controller_spec.rb | 31 ----------------
.../registrations_controller_spec.rb | 37 +------------------
spec/features/invites_spec.rb | 14 -------
spec/helpers/notify_helper_spec.rb | 27 --------------
spec/mailers/notify_spec.rb | 25 +------------
.../mailers/notify_shared_examples.rb | 6 +++
11 files changed, 18 insertions(+), 167 deletions(-)
delete mode 100644 config/feature_flags/experiment/invite_email_preview_text.yml
diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb
index eb89fe58cc0e9..2a7f2d42e2af9 100644
--- a/app/controllers/invites_controller.rb
+++ b/app/controllers/invites_controller.rb
@@ -77,10 +77,6 @@ def ensure_member_exists
def track_invite_join_click
return unless member && initial_invite_email?
- if params[:experiment_name] == 'invite_email_preview_text'
- experiment(:invite_email_preview_text, actor: member).track(:join_clicked)
- end
-
Gitlab::Tracking.event(self.class.name, 'join_clicked', label: 'invite_email', property: member.id.to_s)
end
@@ -102,7 +98,6 @@ def set_session_invite_params
session[:invite_email] = member.invite_email
session[:originating_member_id] = member.id if initial_invite_email?
- session[:invite_email_experiment_name] = params[:experiment_name] if initial_invite_email? && params[:experiment_name]
end
def initial_invite_email?
diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb
index 6dc5cf57a9e90..ed3facd72c541 100644
--- a/app/controllers/registrations_controller.rb
+++ b/app/controllers/registrations_controller.rb
@@ -210,8 +210,6 @@ def after_pending_invitations_hook
return unless member
- experiment_name = session.delete(:invite_email_experiment_name)
- experiment(:invite_email_preview_text, actor: member).track(:accepted) if experiment_name == 'invite_email_preview_text'
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 da42740f99399..c0ba93f4a30ef 100644
--- a/app/helpers/notify_helper.rb
+++ b/app/helpers/notify_helper.rb
@@ -20,14 +20,4 @@ def invited_to_description(source)
(source.description || default_description).truncate(200, separator: ' ')
end
-
- def invited_join_url(token, member)
- additional_params = { invite_type: Emails::Members::INITIAL_INVITE }
-
- if experiment(:invite_email_preview_text, actor: member).enabled?
- additional_params[:experiment_name] = 'invite_email_preview_text'
- end
-
- invite_url(token, additional_params)
- end
end
diff --git a/app/views/notify/member_invited_email.html.haml b/app/views/notify/member_invited_email.html.haml
index 1d1f696e1b24b..6d5207510da31 100644
--- a/app/views/notify/member_invited_email.html.haml
+++ b/app/views/notify/member_invited_email.html.haml
@@ -6,17 +6,15 @@
role: member.human_access.downcase }
- join_text = s_('InviteEmail|Join now')
- inviter_name = member.created_by.name if member.created_by
+- join_url = invite_url(@token, invite_type: Emails::Members::INITIAL_INVITE)
-- experiment(:invite_email_preview_text, actor: member) do |experiment_instance|
- - experiment_instance.use {}
- - experiment_instance.candidate do
- = content_for :preview_text do
- %div{ style: "display:none;font-size:1px;line-height:1px;max-height:0px;max-width:0px;opacity:0;overflow:hidden;" }
- - if member.created_by
- = s_('InviteEmail|Join your team on GitLab! %{inviter} invited you to %{project_or_group_name}') % { inviter: inviter_name, project_or_group_name: placeholders[:project_or_group_name] }
- - else
- = s_('InviteEmail|Join your team on GitLab! You are invited to %{project_or_group_name}') % { project_or_group_name: placeholders[:project_or_group_name] }
- = gmail_goto_action(join_text, invited_join_url(@token, member))
+= content_for :preview_text do
+ %div{ style: "display:none;font-size:1px;line-height:1px;max-height:0px;max-width:0px;opacity:0;overflow:hidden;" }
+ - if member.created_by
+ = s_('InviteEmail|Join your team on GitLab! %{inviter} invited you to %{project_or_group_name}') % { inviter: inviter_name, project_or_group_name: placeholders[:project_or_group_name] }
+ - else
+ = s_('InviteEmail|Join your team on GitLab! You are invited to %{project_or_group_name}') % { project_or_group_name: placeholders[:project_or_group_name] }
+ = gmail_goto_action(join_text, join_url)
%tr
%td.text-content{ colspan: 2 }
@@ -32,7 +30,7 @@
- else
= html_escape(s_("InviteEmail|You are invited to join the %{strong_start}%{project_or_group_name}%{strong_end}%{br_tag}%{project_or_group} as a %{role}")) % placeholders
%p.invite-actions
- = link_to join_text, invited_join_url(@token, member), class: 'invite-btn-join'
+ = link_to join_text, join_url, class: 'invite-btn-join'
%tr.border-top
%td.text-content.mailer-align-left.half-width
%h4
diff --git a/config/feature_flags/experiment/invite_email_preview_text.yml b/config/feature_flags/experiment/invite_email_preview_text.yml
deleted file mode 100644
index fcb4cda0b1456..0000000000000
--- a/config/feature_flags/experiment/invite_email_preview_text.yml
+++ /dev/null
@@ -1,8 +0,0 @@
----
-name: invite_email_preview_text
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/67236
-rollout_issue_url: https://gitlab.com/gitlab-org/growth/team-tasks/-/issues/421
-milestone: '14.2'
-type: experiment
-group: group::expansion
-default_enabled: false
diff --git a/spec/controllers/invites_controller_spec.rb b/spec/controllers/invites_controller_spec.rb
index dc1fb0454dfca..c5e693e3489c5 100644
--- a/spec/controllers/invites_controller_spec.rb
+++ b/spec/controllers/invites_controller_spec.rb
@@ -97,29 +97,6 @@
)
end
- context 'when it is part of the invite_email_preview_text experiment' do
- let(:extra_params) { { invite_type: 'initial_email', experiment_name: 'invite_email_preview_text' } }
-
- it 'tracks the initial join click from email' do
- experiment = double(track: true)
- allow(controller).to receive(:experiment).with(:invite_email_preview_text, 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_preview_text, actor: member)
-
- request
- end
- end
- end
-
context 'when member does not exist' do
let(:raw_invite_token) { '_bogus_token_' }
@@ -145,14 +122,6 @@
label: 'invite_email'
)
end
-
- context 'when it is not part of our invite email experiment' do
- it 'does not track via experiment' do
- expect(controller).not_to receive(:experiment).with(:invite_email_preview_text, actor: member)
-
- request
- end
- end
end
context 'when not logged in' do
diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb
index 9094d23536674..3f7941b345651 100644
--- a/spec/controllers/registrations_controller_spec.rb
+++ b/spec/controllers/registrations_controller_spec.rb
@@ -159,12 +159,11 @@
let_it_be(:member) { create(:project_member, :invited, invite_email: user_params.dig(:user, :email)) }
let(:originating_member_id) { member.id }
- let(:extra_session_params) { {} }
let(:session_params) do
{
invite_email: user_params.dig(:user, :email),
originating_member_id: originating_member_id
- }.merge extra_session_params
+ }
end
context 'when member exists from the session key value' do
@@ -193,40 +192,6 @@
)
end
end
-
- context 'with the invite_email_preview_text experiment', :experiment do
- let(:extra_session_params) { { invite_email_experiment_name: 'invite_email_preview_text' } }
-
- 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_preview_text)).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_preview_text)).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_preview_text)).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 d2bf35166ac31..9cb9416e7a032 100644
--- a/spec/features/invites_spec.rb
+++ b/spec/features/invites_spec.rb
@@ -226,20 +226,6 @@ def fill_in_welcome_form
end
end
- context 'with invite email acceptance for the invite_email_preview_text experiment', :experiment do
- let(:extra_params) do
- { invite_type: Emails::Members::INITIAL_INVITE, experiment_name: 'invite_email_preview_text' }
- end
-
- it 'tracks the accepted invite' do
- expect(experiment(:invite_email_preview_text)).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 633a4b65139fc..e2a7a212b1b61 100644
--- a/spec/helpers/notify_helper_spec.rb
+++ b/spec/helpers/notify_helper_spec.rb
@@ -55,31 +55,4 @@
def reference_link(entity, url)
"<a href=\"#{url}\">#{entity.to_reference}</a>"
end
-
- describe '#invited_join_url' do
- let_it_be(:member) { create(:project_member) }
-
- let(:token) { '_token_' }
-
- context 'when invite_email_preview_text is enabled', :experiment do
- before do
- stub_experiments(invite_email_preview_text: :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_preview_text&invite_type=initial_email")
- end
- end
-
- context 'when invite_email_preview_text is disabled' do
- before do
- stub_feature_flags(invite_email_preview_text: false)
- end
-
- it 'has correct params' do
- expect(helper.invited_join_url(token, member)).to eq("http://test.host/-/invites/#{token}?invite_type=initial_email")
- end
- end
- end
end
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index ad9bd0faf8af5..44cb18008d2b7 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -829,7 +829,7 @@ def invite_to_project(project, inviter:, user: nil, tasks_to_be_done: [])
end
it_behaves_like 'an email sent from GitLab'
- it_behaves_like 'it should not have Gmail Actions links'
+ it_behaves_like 'it should show Gmail Actions Join now link'
it_behaves_like "a user cannot unsubscribe through footer link"
it_behaves_like 'appearance header and footer enabled'
it_behaves_like 'appearance header and footer not enabled'
@@ -867,27 +867,6 @@ def invite_to_project(project, inviter:, user: nil, tasks_to_be_done: [])
end
end
- context 'with invite_email_preview_text enabled', :experiment do
- before do
- stub_experiments(invite_email_preview_text: :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_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 'when invite email sent is tracked', :snowplow do
it 'tracks the sent invite' do
invite_email.deliver_now
@@ -1461,7 +1440,7 @@ def invite_to_group(group, inviter:, user: nil, tasks_to_be_done: [])
subject { described_class.member_invited_email('Group', group_member.id, group_member.invite_token) }
it_behaves_like 'an email sent from GitLab'
- it_behaves_like 'it should not have Gmail Actions links'
+ it_behaves_like 'it should show Gmail Actions Join now link'
it_behaves_like "a user cannot unsubscribe through footer link"
it_behaves_like 'appearance header and footer enabled'
it_behaves_like 'appearance header and footer not enabled'
diff --git a/spec/support/shared_examples/mailers/notify_shared_examples.rb b/spec/support/shared_examples/mailers/notify_shared_examples.rb
index e1f7a9030e20f..20ed380fb18fd 100644
--- a/spec/support/shared_examples/mailers/notify_shared_examples.rb
+++ b/spec/support/shared_examples/mailers/notify_shared_examples.rb
@@ -161,6 +161,12 @@
end
end
+RSpec.shared_examples 'it should show Gmail Actions Join now link' do
+ it_behaves_like 'it should have Gmail Actions links'
+
+ it { is_expected.to have_body_text('Join now') }
+end
+
RSpec.shared_examples 'it should show Gmail Actions View Issue link' do
it_behaves_like 'it should have Gmail Actions links'
--
GitLab