Merge branch 'nicolasdular/new-invite-mail' into 'master'

Add invite email experiment

See merge request gitlab-org/gitlab!39628

.haml-lint_todo.yml

@@ -117,6 +117,7 @@ linters:
       - "app/views/import/bitbucket_server/status.html.haml"
       - "app/views/invites/show.html.haml"
       - "app/views/layouts/_mailer.html.haml"
+      - "app/views/layouts/experiment_mailer.html.haml"
       - "app/views/layouts/header/_default.html.haml"
       - "app/views/layouts/header/_new_dropdown.haml"
       - "app/views/layouts/notify.html.haml"

app/assets/stylesheets/mailer.scss

@@ -48,6 +48,22 @@ a {
   font-weight: 500;
 }
 
+.invite-header {
+  margin-top: 0;
+}
+
+.invite-actions {
+  margin-top: 24px;
+}
+
+.invite-btn-join {
+  border-radius: $border-radius-default;
+  padding: $gl-btn-vert-padding $gl-btn-horz-padding;
+  cursor: pointer;
+  background-color: $purple;
+  color: $white;
+}
+
 tr td {
   font-family: $mailer-font;
 }

app/controllers/invites_controller.rb

@@ -12,11 +12,13 @@ class InvitesController < ApplicationController
   respond_to :html
 
   def show
+    track_experiment('opened')
     accept if skip_invitation_prompt?
   end
 
   def accept
     if member.accept_invite!(current_user)
+      track_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
@@ -96,4 +98,17 @@ class InvitesController < ApplicationController
                           }
                         end
   end
+
+  def track_experiment(action)
+    return unless params[:new_user_invite]
+
+    property = params[:new_user_invite] == 'experiment' ? 'experiment_group' : 'control_group'
+
+    Gitlab::Tracking.event(
+      Gitlab::Experimentation::EXPERIMENTS[:invite_email][:tracking_category],
+      action,
+      property: property,
+      value: Digest::MD5.hexdigest(member.to_global_id.to_s)
+    )
+  end
 end

app/mailers/emails/members.rb

@@ -51,9 +51,32 @@ module Emails
 
       return unless member_exists?
 
-      member_email_with_layout(
-        to: member.invite_email,
-        subject: subject("Invitation to join the #{member_source.human_name} #{member_source.model_name.singular}"))
+      subject_line = subject("Invitation to join the #{member_source.human_name} #{member_source.model_name.singular}")
+
+      if member.invite_to_unknown_user? && Feature.enabled?(:invite_email_experiment)
+        subject_line = subject("#{member.created_by.name} invited you to join GitLab") if member.created_by
+        @invite_url_params = { new_user_invite: 'experiment' }
+
+        member_email_with_layout(
+          to: member.invite_email,
+          subject: subject_line,
+          template: 'member_invited_email_experiment',
+          layout: 'experiment_mailer'
+        )
+
+        Gitlab::Tracking.event(Gitlab::Experimentation::EXPERIMENTS[:invite_email][:tracking_category], 'sent', property: 'experiment_group')
+      else
+        @invite_url_params = member.invite_to_unknown_user? ? { new_user_invite: 'control' } : {}
+
+        member_email_with_layout(
+          to: member.invite_email,
+          subject: subject_line
+        )
+
+        if member.invite_to_unknown_user?
+          Gitlab::Tracking.event(Gitlab::Experimentation::EXPERIMENTS[:invite_email][:tracking_category], 'sent', property: 'control_group')
+        end
+      end
     end
 
     def member_invite_accepted_email(member_source_type, member_id)
@@ -107,10 +130,15 @@ module Emails
       @member_source_type.classify.constantize
     end
 
-    def member_email_with_layout(to:, subject:)
+    def member_email_with_layout(to:, subject:, template: nil, layout: 'mailer')
       mail(to: to, subject: subject) do |format|
-        format.html { render layout: 'mailer' }
-        format.text { render layout: 'mailer' }
+        if template
+          format.html { render template, layout: layout }
+          format.text { render template, layout: layout }
+        else
+          format.html { render layout: layout }
+          format.text { render layout: layout }
+        end
       end
     end
   end

app/models/member.rb

@@ -395,6 +395,10 @@ class Member < ApplicationRecord
     end
   end
 
+  def invite_to_unknown_user?
+    invite? && user_id.nil?
+  end
+
   private
 
   def send_invite

app/views/invites/show.html.haml

@@ -25,5 +25,5 @@
 
 - if !member?
   .actions
-    = link_to _("Accept invitation"), accept_invite_url(@token), method: :post, class: "btn btn-success"
+    = link_to _("Accept invitation"), accept_invite_url(@token, new_user_invite: params[:new_user_invite]), method: :post, class: "btn btn-success"
     = link_to _("Decline"), decline_invite_url(@token), method: :post, class: "btn btn-danger gl-ml-3"

app/views/layouts/experiment_mailer.html.haml

+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
+%html{ lang: "en" }
+  %head
+    %meta{ content: "text/html; charset=UTF-8", "http-equiv" => "Content-Type" }/
+    %meta{ content: "width=device-width, initial-scale=1", name: "viewport" }/
+    %meta{ content: "IE=edge", "http-equiv" => "X-UA-Compatible" }/
+    %title= message.subject
+
+    -# Avoid premailer processing of client-specific styles (@media tag not supported)
+    -# We need to inline the contents here because mail clients (e.g. iOS Mail, Outlook)
+    -# do not support linked stylesheets.
+    %style{ type: 'text/css', 'data-premailer': 'ignore' }
+      = asset_to_string('mailer_client_specific.css').html_safe
+
+    = stylesheet_link_tag 'mailer.css'
+  %body
+    %table#body{ border: "0", cellpadding: "0", cellspacing: "0" }
+      %tbody
+        %tr.line
+          %td
+        %tr.header
+          %td
+            = html_header_message
+            = header_logo
+        %tr
+          %td
+            %table.wrapper{ border: "0", cellpadding: "0", cellspacing: "0" }
+              %tbody
+                %tr
+                  %td.wrapper-cell{ style: "padding: 0" }
+                    %table.content{ border: "0", cellpadding: "0", cellspacing: "0" }
+                      %tbody
+                        = yield
+
+        = render_if_exists 'layouts/mailer/additional_text'
+
+        %tr.footer
+          %td{ style: "padding: 24px 0" }
+            %img{ alt: "GitLab", height: "33", width: "90", src: image_url('mailers/gitlab_footer_logo.gif') }
+            %p{ style: "color: #949ba5; max-width: 640px; margin: 0 auto; text-align: left; font-size: 12px;" }
+              GitLab is a complete DevOps platform, delivered as a single application, fundamentally changing the way
+              %br
+              Development, Security, and Ops teams collaborate.
+
+        = yield :additional_footer
+        %tr
+          %td.footer-message
+            = html_footer_message

app/views/notify/member_invited_email.html.haml

@@ -10,7 +10,7 @@
       #{member_source.model_name.singular} as #{content_tag :span, member.human_access, class: :highlight}.
 
     %p
-      = link_to 'Accept invitation', invite_url(@token)
+      = link_to 'Accept invitation', invite_url(@token, @invite_url_params)
       or
       = link_to 'decline', decline_invite_url(@token)
 

app/views/notify/member_invited_email.text.erb

 You have been invited <%= "by #{sanitize_name(member.created_by.name)} " if member.created_by %>to join the <%= member_source.human_name %> <%= member_source.model_name.singular %> as <%= member.human_access %>.
 
-Accept invitation: <%= invite_url(@token) %>
+Accept invitation: <%= invite_url(@token, @invite_url_params) %>
 Decline invitation: <%= decline_invite_url(@token) %>

app/views/notify/member_invited_email_experiment.html.haml

+%tr
+  %td.text-content
+    %h2.invite-header
+      = s_('InviteEmail|You are invited!')
+    %p
+      - if member.created_by
+        = html_escape(s_("InviteEmail|%{inviter} invited you")) % { inviter: (link_to member.created_by.name, user_url(member.created_by)).html_safe }
+      = html_escape(s_("InviteEmail|to join the %{strong_start}%{project_or_group_name}%{strong_end}")) % { strong_start: '<strong>'.html_safe, strong_end: '</strong>'.html_safe, project_or_group_name: member_source.human_name }
+      %br
+      = s_("InviteEmail|%{project_or_group} as a %{role}") % { project_or_group: member_source.model_name.singular, role: member.human_access.downcase }
+    %p.invite-actions
+      = link_to s_('InviteEmail|Join now'), invite_url(@token, @invite_url_params), class: 'invite-btn-join'

app/views/notify/member_invited_email_experiment.text.erb

+<% project_and_role = s_('InviteEmail|to join the %{project_or_group_name} %{project_or_group} as a %{role}') \
+  % { project_or_group_name: member_source.human_name, project_or_group: member_source.model_name.singular, role: member.human_access.downcase } %>
+
+<% if member.created_by %>
+<%= s_('InviteEmail|%{inviter} invited you') % { inviter: sanitize_name(member.created_by.name) } %> <%= project_and_role %>
+<% else %>
+<%= s_('InviteEmail|You have been invited') %> <%= project_and_role %>
+<% end %>
+
+Join now: <%= invite_url(@token, @invite_url_params) %>

lib/gitlab/experimentation.rb

@@ -62,6 +62,9 @@ module Gitlab
       },
       customize_homepage: {
         tracking_category: 'Growth::Expansion::Experiment::CustomizeHomepage'
+      },
+      invite_email: {
+        tracking_category: 'Growth::Acquisition::Experiment::InviteEmail'
       }
     }.freeze
 

locale/gitlab.pot

@@ -13417,6 +13417,27 @@ msgstr ""
 msgid "Invite teammates (optional)"
 msgstr ""
 
+msgid "InviteEmail|%{inviter} invited you"
+msgstr ""
+
+msgid "InviteEmail|%{project_or_group} as a %{role}"
+msgstr ""
+
+msgid "InviteEmail|Join now"
+msgstr ""
+
+msgid "InviteEmail|You are invited!"
+msgstr ""
+
+msgid "InviteEmail|You have been invited"
+msgstr ""
+
+msgid "InviteEmail|to join the %{project_or_group_name} %{project_or_group} as a %{role}"
+msgstr ""
+
+msgid "InviteEmail|to join the %{strong_start}%{project_or_group_name}%{strong_end}"
+msgstr ""
+
 msgid "Invited"
 msgstr ""
 

spec/controllers/invites_controller_spec.rb

@@ -7,6 +7,7 @@ RSpec.describe InvitesController do
   let_it_be(:user) { create(:user) }
   let(:member) { create(:project_member, :invited, invite_token: token, invite_email: user.email) }
   let(:project_members) { member.source.users }
+  let(:md5_member_global_id) { Digest::MD5.hexdigest(member.to_global_id.to_s) }
 
   before do
     controller.instance_variable_set(:@member, member)
@@ -14,9 +15,13 @@ RSpec.describe InvitesController do
   end
 
   describe 'GET #show' do
+    let(:params) { { id: token } }
+
+    subject(:request) { get :show, params: params }
+
     it 'accepts user if invite email matches signed in user' do
       expect do
-        get :show, params: { id: token }
+        request
       end.to change { project_members.include?(user) }.from(false).to(true)
 
       expect(response).to have_gitlab_http_status(:found)
@@ -27,11 +32,105 @@ RSpec.describe InvitesController do
       member.invite_email = 'bogus@email.com'
 
       expect do
-        get :show, params: { id: token }
+        request
       end.not_to change { project_members.include?(user) }
 
       expect(response).to have_gitlab_http_status(:ok)
       expect(flash[:notice]).to be_nil
     end
+
+    context 'when new_user_invite is not set' do
+      it 'does not track the user as experiment group' do
+        expect(Gitlab::Tracking).not_to receive(:event)
+
+        request
+      end
+    end
+
+    context 'when new_user_invite is experiment' do
+      let(:params) { { id: token, new_user_invite: 'experiment' } }
+
+      it 'tracks the user as experiment group' do
+        expect(Gitlab::Tracking).to receive(:event).with(
+          'Growth::Acquisition::Experiment::InviteEmail',
+          'opened',
+          property: 'experiment_group',
+          value: md5_member_global_id
+        )
+        expect(Gitlab::Tracking).to receive(:event).with(
+          'Growth::Acquisition::Experiment::InviteEmail',
+          'accepted',
+          property: 'experiment_group',
+          value: md5_member_global_id
+        )
+
+        request
+      end
+    end
+
+    context 'when new_user_invite is control' do
+      let(:params) { { id: token, new_user_invite: 'control' } }
+
+      it 'tracks the user as control group' do
+        expect(Gitlab::Tracking).to receive(:event).with(
+          'Growth::Acquisition::Experiment::InviteEmail',
+          'opened',
+          property: 'control_group',
+          value: md5_member_global_id
+        )
+        expect(Gitlab::Tracking).to receive(:event).with(
+          'Growth::Acquisition::Experiment::InviteEmail',
+          'accepted',
+          property: 'control_group',
+          value: md5_member_global_id
+        )
+
+        request
+      end
+    end
+  end
+
+  describe 'POST #accept' do
+    let(:params) { { id: token } }
+
+    subject(:request) { post :accept, params: params }
+
+    context 'when new_user_invite is not set' do
+      it 'does not track an event' do
+        expect(Gitlab::Tracking).not_to receive(:event)
+
+        request
+      end
+    end
+
+    context 'when new_user_invite is experiment' do
+      let(:params) { { id: token, new_user_invite: 'experiment' } }
+
+      it 'tracks the user as experiment group' do
+        expect(Gitlab::Tracking).to receive(:event).with(
+          'Growth::Acquisition::Experiment::InviteEmail',
+          'accepted',
+          property: 'experiment_group',
+          value: md5_member_global_id
+        )
+
+        request
+      end
+    end
+
+    context 'when new_user_invite is control' do
+      let(:params) { { id: token, new_user_invite: 'control' } }
+
+      it 'tracks the user as control group' do
+        expect(Gitlab::Tracking).to receive(:event).with(
+          'Growth::Acquisition::Experiment::InviteEmail',
+          'accepted',
+          property: 'control_group',
+          value: md5_member_global_id
+        )
+
+        request
+      end
+    end
   end
 end

spec/mailers/notify_spec.rb

@@ -868,36 +868,116 @@ RSpec.describe Notify do
       end
     end
 
-    def invite_to_project(project, inviter:)
+    def invite_to_project(project, inviter:, user: nil)
       create(
         :project_member,
         :developer,
         project: project,
         invite_token: '1234',
         invite_email: 'toto@example.com',
-        user: nil,
+        user: user,
         created_by: inviter
       )
     end
 
     describe 'project invitation' do
       let(:maintainer) { create(:user).tap { |u| project.add_maintainer(u) } }
-      let(:project_member) { invite_to_project(project, inviter: maintainer) }
+      let(:project_member) { invite_to_project(project, inviter: inviter) }
+      let(:inviter) { maintainer }
 
       subject { described_class.member_invited_email('project', project_member.id, project_member.invite_token) }
 
-      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"
-      it_behaves_like 'appearance header and footer enabled'
-      it_behaves_like 'appearance header and footer not enabled'
+      context 'when invite_email_experiment is disabled' do
+        before do
+          stub_feature_flags(invite_email_experiment: false)
+        end
 
-      it 'contains all the useful information' do
-        is_expected.to have_subject "Invitation to join the #{project.full_name} project"
-        is_expected.to have_body_text project.full_name
-        is_expected.to have_body_text project.full_name
-        is_expected.to have_body_text project_member.human_access
-        is_expected.to have_body_text project_member.invite_token
+        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"
+        it_behaves_like 'appearance header and footer enabled'
+        it_behaves_like 'appearance header and footer not enabled'
+
+        it 'contains all the useful information' do
+          is_expected.to have_subject "Invitation to join the #{project.full_name} project"
+          is_expected.to have_body_text project.full_name
+          is_expected.to have_body_text project_member.human_access
+          is_expected.to have_body_text project_member.invite_token
+        end
+
+        context 'when member is invited via an email address' do
+          it 'does add a param to the invite link' do
+            is_expected.to have_body_text 'new_user_invite=control'
+          end
+
+          it 'tracks an event' do
+            expect(Gitlab::Tracking).to receive(:event).with(
+              'Growth::Acquisition::Experiment::InviteEmail',
+              'sent',
+              property: 'control_group'
+            )
+
+            subject.deliver_now
+          end
+        end
+
+        context 'when member is already a user' do
+          let(:project_member) { invite_to_project(project, inviter: maintainer, user: create(:user)) }
+
+          it 'does not add a param to the invite link' do
+            is_expected.not_to have_body_text 'new_user_invite'
+          end
+
+          it 'does not track an event' do
+            expect(Gitlab::Tracking).not_to receive(:event)
+
+            subject.deliver_now
+          end
+        end
+      end
+
+      context 'when invite_email_experiment is enabled' do
+        before do
+          stub_feature_flags(invite_email_experiment: true)
+        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"
+
+        context 'when there is no inviter' do
+          let(:inviter) { nil }
+
+          it 'contains all the useful information' do
+            is_expected.to have_subject "Invitation to join the #{project.full_name} project"
+            is_expected.to have_body_text project.full_name
+            is_expected.to have_body_text project_member.human_access.downcase
+            is_expected.to have_body_text project_member.invite_token
+          end
+        end
+
+        context 'when there is an inviter' do
+          it 'contains all the useful information' do
+            is_expected.to have_subject "#{inviter.name} invited you to join GitLab"
+            is_expected.to have_body_text project.full_name
+            is_expected.to have_body_text project_member.human_access.downcase
+            is_expected.to have_body_text project_member.invite_token
+          end
+        end
+
+        it 'adds a param to the invite link' do
+          is_expected.to have_body_text 'new_user_invite=experiment'
+        end
+
+        it 'tracks an event' do
+          expect(Gitlab::Tracking).to receive(:event).with(
+            'Growth::Acquisition::Experiment::InviteEmail',
+            'sent',
+            property: 'experiment_group'
+          )
+
+          subject.deliver_now
+        end
       end
     end
 
@@ -1416,37 +1496,115 @@ RSpec.describe Notify do
       end
     end
 
-    def invite_to_group(group, inviter:)
+    def invite_to_group(group, inviter:, user: nil)
       create(
         :group_member,
         :developer,
         group: group,
         invite_token: '1234',
         invite_email: 'toto@example.com',
-        user: nil,
+        user: user,
         created_by: inviter
       )
     end
 
     describe 'group invitation' do
       let(:owner) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } }
-      let(:group_member) { invite_to_group(group, inviter: owner) }
+      let(:group_member) { invite_to_group(group, inviter: inviter) }
+      let(:inviter) { owner }
 
       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 "a user cannot unsubscribe through footer link"
-      it_behaves_like 'appearance header and footer enabled'
-      it_behaves_like 'appearance header and footer not enabled'
-      it_behaves_like 'it requires a group'
+      context 'when invite_email_experiment is disabled' do
+        before do
+          stub_feature_flags(invite_email_experiment: false)
+        end
 
-      it 'contains all the useful information' do
-        is_expected.to have_subject "Invitation to join the #{group.name} group"
-        is_expected.to have_body_text group.name
-        is_expected.to have_body_text group.web_url
-        is_expected.to have_body_text group_member.human_access
-        is_expected.to have_body_text 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 "a user cannot unsubscribe through footer link"
+        it_behaves_like 'appearance header and footer enabled'
+        it_behaves_like 'appearance header and footer not enabled'
+        it_behaves_like 'it requires a group'
+
+        it 'contains all the useful information' do
+          is_expected.to have_subject "Invitation to join the #{group.name} group"
+          is_expected.to have_body_text group.name
+          is_expected.to have_body_text group.web_url
+          is_expected.to have_body_text group_member.human_access
+          is_expected.to have_body_text group_member.invite_token
+        end
+
+        context 'when member is invited via an email address' do
+          it 'does add a param to the invite link' do
+            is_expected.to have_body_text 'new_user_invite=control'
+          end
+
+          it 'tracks an event' do
+            expect(Gitlab::Tracking).to receive(:event).with(
+              'Growth::Acquisition::Experiment::InviteEmail',
+              'sent',
+              property: 'control_group'
+            )
+
+            subject.deliver_now
+          end
+        end
+
+        context 'when member is already a user' do
+          let(:group_member) { invite_to_group(group, inviter: owner, user: create(:user)) }
+
+          it 'does not add a param to the invite link' do
+            is_expected.not_to have_body_text 'new_user_invite'
+          end
+
+          it 'does not track an event' do
+            expect(Gitlab::Tracking).not_to receive(:event)
+
+            subject.deliver_now
+          end
+        end
+      end
+
+      context 'when invite_email_experiment is enabled' do
+        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"
+        it_behaves_like 'it requires a group'
+
+        context 'when there is no inviter' do
+          let(:inviter) { nil }
+
+          it 'contains all the useful information' do
+            is_expected.to have_subject "Invitation to join the #{group.name} group"
+            is_expected.to have_body_text group.name
+            is_expected.to have_body_text group_member.human_access.downcase
+            is_expected.to have_body_text group_member.invite_token
+          end
+        end
+
+        context 'when there is an inviter' do
+          it 'contains all the useful information' do
+            is_expected.to have_subject "#{group_member.created_by.name} invited you to join GitLab"
+            is_expected.to have_body_text group.name
+            is_expected.to have_body_text group_member.human_access.downcase
+            is_expected.to have_body_text group_member.invite_token
+          end
+        end
+
+        it 'does add a param to the invite link' do
+          is_expected.to have_body_text 'new_user_invite'
+        end
+
+        it 'tracks an event' do
+          expect(Gitlab::Tracking).to receive(:event).with(
+            'Growth::Acquisition::Experiment::InviteEmail',
+            'sent',
+            property: 'experiment_group'
+          )
+
+          subject.deliver_now
+        end
       end
     end
 

spec/models/member_spec.rb

@@ -617,6 +617,24 @@ RSpec.describe Member do
     end
   end
 
+  describe "#invite_to_unknown_user?" do
+    subject { member.invite_to_unknown_user? }
+
+    let(:member) { create(:project_member, invite_email: "user@example.com", invite_token: '1234', user: user) }
+
+    context 'when user is nil' do
+      let(:user) { nil }
+
+      it { is_expected.to eq(true) }
+    end
+
+    context 'when user is set' do
+      let(:user) { build(:user) }
+
+      it { is_expected.to eq(false) }
+    end
+  end
+
   describe "destroying a record", :delete do
     it "refreshes user's authorized projects" do
       project = create(:project, :private)