Merge branch...

Merge branch '330353-eng-experiment-update-new-user-invitation-email-test-variation-5-group-detail-and-activity' into 'master'

Experiment - Update new user invitation email with activity

See merge request gitlab-org/gitlab!63570

app/assets/stylesheets/mailer.scss

@@ -145,6 +145,27 @@ table.content {
     padding: 15px 5px;
     text-align: center;
   }
+
+  td.mailer-align-left {
+    vertical-align: top;
+    padding: 16px 32px;
+    text-align: left;
+
+    h4 {
+      margin: 0;
+    }
+
+    ul {
+      list-style: none;
+      line-height: 1.6;
+      padding-left: 0;
+      margin: 8px 0 16px;
+    }
+
+    .mailer-icon {
+      margin-bottom: -1px;
+    }
+  }
 }
 
 tr.footer td {

app/experiments/members/invite_email_experiment.rb

@@ -12,7 +12,7 @@ module Members
     end
 
     def resolve_variant_name
-      RoundRobin.new(feature_flag_name, %i[avatar permission_info control]).execute
+      RoundRobin.new(feature_flag_name, %i[activity control]).execute
     end
   end
 

app/helpers/notify_helper.rb

@@ -9,29 +9,15 @@ module NotifyHelper
     link_to(entity.to_reference(full: full), issue_url(entity, *args))
   end
 
-  def invited_role_description(role_name)
-    case role_name
-    when "Guest"
-      s_("InviteEmail|As a guest, you can view projects, leave comments, and create issues.")
-    when "Reporter"
-      s_("InviteEmail|As a reporter, you can view projects and reports, and leave comments on issues.")
-    when "Developer"
-      s_("InviteEmail|As a developer, you have full access to projects, so you can take an idea from concept to production.")
-    when "Maintainer"
-      s_("InviteEmail|As a maintainer, you have full access to projects. You can push commits to the default branch and deploy to production.")
-    when "Owner"
-      s_("InviteEmail|As an owner, you have full access to projects and can manage access to the group, including inviting new members.")
-    when "Minimal Access"
-      s_("InviteEmail|As a user with minimal access, you can view the high-level group from the UI and API.")
-    end
-  end
-
   def invited_to_description(source)
-    case source
-    when "project"
-      s_('InviteEmail|Projects can be used to host your code, track issues, collaborate on code, and continuously build, test, and deploy your app with built-in GitLab CI/CD.')
-    when "group"
-      s_('InviteEmail|Groups assemble related projects together and grant members access to several projects at once.')
-    end
+    default_description =
+      case source
+      when Project
+        s_('InviteEmail|Projects are used to host and collaborate on code, track issues, and continuously build, test, and deploy your app with built-in GitLab CI/CD.')
+      when Group
+        s_('InviteEmail|Groups assemble related projects together and grant members access to several projects at once.')
+      end
+
+    (source.description || default_description).truncate(200, separator: ' ')
   end
 end

app/models/group.rb

@@ -722,6 +722,18 @@ class Group < Namespace
     Gitlab::Routing.url_helpers.activity_group_path(self)
   end
 
+  # rubocop: disable CodeReuse/ServiceClass
+  def open_issues_count(current_user = nil)
+    Groups::OpenIssuesCountService.new(self, current_user).count
+  end
+  # rubocop: enable CodeReuse/ServiceClass
+
+  # rubocop: disable CodeReuse/ServiceClass
+  def open_merge_requests_count(current_user = nil)
+    Groups::MergeRequestsCountService.new(self, current_user).count
+  end
+  # rubocop: enable CodeReuse/ServiceClass
+
   private
 
   def max_member_access(user_ids)

app/models/project.rb

@@ -1753,7 +1753,7 @@ class Project < ApplicationRecord
   # rubocop: enable CodeReuse/ServiceClass
 
   # rubocop: disable CodeReuse/ServiceClass
-  def open_merge_requests_count
+  def open_merge_requests_count(_current_user = nil)
     Projects::OpenMergeRequestsCountService.new(self).count
   end
   # rubocop: enable CodeReuse/ServiceClass

app/views/notify/member_invited_email.html.haml

@@ -13,28 +13,48 @@
             = 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 s_('InviteEmail|Join now'), invite_url(@token, invite_type: Members::InviteEmailExperiment::INVITE_TYPE), class: 'invite-btn-join'
-  - experiment_instance.try(:avatar) do
-    %tr
-      %td.text-content
-        %img.mail-avatar{ height: "60", src: avatar_icon_for_user(member.created_by, 60, only_path: false), width: "60", alt: "" }
-        %p
-          = html_escape(s_("InviteEmail|%{inviter} invited you to join the %{strong_start}%{project_or_group_name}%{strong_end}%{br_tag}%{project_or_group} as a %{role}")) % placeholders.merge({ inviter: (link_to member.created_by.name, user_url(member.created_by)).html_safe })
-        %p.invite-actions
-          = link_to s_('InviteEmail|Join now'), invite_url(@token, invite_type: Members::InviteEmailExperiment::INVITE_TYPE), class: 'invite-btn-join'
-  - experiment_instance.try(:permission_info) do
+  - experiment_instance.try(:activity) do
     %tr
       %td.text-content{ colspan: 2 }
         %img.mail-avatar{ height: "60", src: avatar_icon_for_user(member.created_by, 60, only_path: false), width: "60", alt: "" }
         %p
-          = html_escape(s_("InviteEmail|%{inviter} invited you to join the %{strong_start}%{project_or_group_name}%{strong_end}%{br_tag}%{project_or_group} with the %{role} permission level.")) % placeholders.merge({ inviter: (link_to member.created_by.name, user_url(member.created_by)).html_safe })
+          = html_escape(s_("InviteEmail|%{inviter} invited you to join the %{strong_start}%{project_or_group_name}%{strong_end}%{br_tag}%{project_or_group} as a %{role}")) % placeholders.merge({ inviter: (link_to member.created_by.name, user_url(member.created_by)).html_safe })
         %p.invite-actions
           = link_to s_('InviteEmail|Join now'), invite_url(@token, invite_type: Members::InviteEmailExperiment::INVITE_TYPE), class: 'invite-btn-join'
     %tr.border-top
-      %td.text-content.half-width
+      %td.text-content.mailer-align-left.half-width
         %h4
-          = s_('InviteEmail|What is a GitLab %{project_or_group}?') % { project_or_group: member_source.model_name.singular }
-        %p= invited_to_description(member_source.model_name.singular)
-      %td.text-content.half-width
+          = s_('InviteEmail|%{project_or_group} details') % { project_or_group: member_source.model_name.singular.capitalize }
+        %ul
+          %li
+            %div
+              %img.mailer-icon{ alt: '', src: image_url("mailers/members/users.png") }
+              %span
+                - member_count = member_source.members.size
+                = n_('%{bold_start}%{count}%{bold_end} member', '%{bold_start}%{count}%{bold_end} members',
+                  member_count).html_safe % { count: number_with_delimiter(member_count),
+                  bold_start: '<b>'.html_safe,
+                  bold_end: '</b>'.html_safe }
+          %li
+            %div
+              %img.mailer-icon{ alt: '', src: image_url("mailers/members/issues.png") }
+              %span
+                - issue_count = member_source.open_issues_count(member.created_by)
+                = n_('%{bold_start}%{count}%{bold_end} issue', '%{bold_start}%{count}%{bold_end} issues',
+                  issue_count).html_safe % { count: number_with_delimiter(issue_count),
+                  bold_start: '<b>'.html_safe,
+                  bold_end: '</b>'.html_safe }
+          %li
+            %div
+              %img.mailer-icon{ alt: '', src: image_url("mailers/members/merge-request-open.png") }
+              %span
+                - mr_count = member_source.open_merge_requests_count(member.created_by)
+                = n_('%{bold_start}%{count}%{bold_end} opened merge request', '%{bold_start}%{count}%{bold_end} opened merge requests',
+                  mr_count).html_safe % { count: number_with_delimiter(mr_count),
+                  bold_start: '<b>'.html_safe,
+                  bold_end: '</b>'.html_safe }
+      %td.text-content.mailer-align-left.half-width
         %h4
-          = s_('InviteEmail|What can I do with the %{role} permission level?') % { role: member.human_access.downcase }
-        %p= invited_role_description(member.human_access)
+          = s_("InviteEmail|What's it about?")
+        %p
+          = invited_to_description(member_source)

locale/gitlab.pot

@@ -421,6 +421,21 @@ msgstr ""
 msgid "%{board_target} not found"
 msgstr ""
 
+msgid "%{bold_start}%{count}%{bold_end} issue"
+msgid_plural "%{bold_start}%{count}%{bold_end} issues"
+msgstr[0] ""
+msgstr[1] ""
+
+msgid "%{bold_start}%{count}%{bold_end} member"
+msgid_plural "%{bold_start}%{count}%{bold_end} members"
+msgstr[0] ""
+msgstr[1] ""
+
+msgid "%{bold_start}%{count}%{bold_end} opened merge request"
+msgid_plural "%{bold_start}%{count}%{bold_end} opened merge requests"
+msgstr[0] ""
+msgstr[1] ""
+
 msgid "%{code_open}Masked:%{code_close} Hidden in job logs. Must match masking requirements."
 msgstr ""
 
@@ -17923,25 +17938,7 @@ msgstr ""
 msgid "InviteEmail|%{inviter} invited you to join the %{strong_start}%{project_or_group_name}%{strong_end}%{br_tag}%{project_or_group} as a %{role}"
 msgstr ""
 
-msgid "InviteEmail|%{inviter} invited you to join the %{strong_start}%{project_or_group_name}%{strong_end}%{br_tag}%{project_or_group} with the %{role} permission level."
-msgstr ""
-
-msgid "InviteEmail|As a developer, you have full access to projects, so you can take an idea from concept to production."
-msgstr ""
-
-msgid "InviteEmail|As a guest, you can view projects, leave comments, and create issues."
-msgstr ""
-
-msgid "InviteEmail|As a maintainer, you have full access to projects. You can push commits to the default branch and deploy to production."
-msgstr ""
-
-msgid "InviteEmail|As a reporter, you can view projects and reports, and leave comments on issues."
-msgstr ""
-
-msgid "InviteEmail|As a user with minimal access, you can view the high-level group from the UI and API."
-msgstr ""
-
-msgid "InviteEmail|As an owner, you have full access to projects and can manage access to the group, including inviting new members."
+msgid "InviteEmail|%{project_or_group} details"
 msgstr ""
 
 msgid "InviteEmail|Groups assemble related projects together and grant members access to several projects at once."
@@ -17950,13 +17947,10 @@ msgstr ""
 msgid "InviteEmail|Join now"
 msgstr ""
 
-msgid "InviteEmail|Projects can be used to host your code, track issues, collaborate on code, and continuously build, test, and deploy your app with built-in GitLab CI/CD."
-msgstr ""
-
-msgid "InviteEmail|What can I do with the %{role} permission level?"
+msgid "InviteEmail|Projects are used to host and collaborate on code, track issues, and continuously build, test, and deploy your app with built-in GitLab CI/CD."
 msgstr ""
 
-msgid "InviteEmail|What is a GitLab %{project_or_group}?"
+msgid "InviteEmail|What's it about?"
 msgstr ""
 
 msgid "InviteEmail|You are invited to join the %{strong_start}%{project_or_group_name}%{strong_end}%{br_tag}%{project_or_group} as a %{role}"

spec/experiments/members/invite_email_experiment_spec.rb

@@ -39,20 +39,14 @@ RSpec.describe Members::InviteEmailExperiment, :clean_gitlab_redis_shared_state
       allow(instance_1).to receive(:enabled?).and_return(true)
       instance_2 = described_class.new('members/invite_email', **context)
       allow(instance_2).to receive(:enabled?).and_return(true)
-      instance_3 = described_class.new('members/invite_email', **context)
-      allow(instance_3).to receive(:enabled?).and_return(true)
 
       instance_1.try { }
 
-      expect(instance_1.variant.name).to eq('permission_info')
+      expect(instance_1.variant.name).to eq('control')
 
       instance_2.try { }
 
-      expect(instance_2.variant.name).to eq('control')
-
-      instance_3.try { }
-
-      expect(instance_3.variant.name).to eq('avatar')
+      expect(instance_2.variant.name).to eq('activity')
     end
   end
 

spec/helpers/notify_helper_spec.rb

@@ -28,27 +28,12 @@ RSpec.describe NotifyHelper do
     end
   end
 
-  describe '#invited_role_description' do
-    where(:role, :description) do
-      "Guest"          | /As a guest/
-      "Reporter"       | /As a reporter/
-      "Developer"      | /As a developer/
-      "Maintainer"     | /As a maintainer/
-      "Owner"          | /As an owner/
-      "Minimal Access" | /As a user with minimal access/
-    end
-
-    with_them do
-      specify do
-        expect(helper.invited_role_description(role)).to match description
-      end
-    end
-  end
-
   describe '#invited_to_description' do
     where(:source, :description) do
-      "project" | /Projects can/
-      "group"   | /Groups assemble/
+      build(:project, description: nil) | /Projects are/
+      build(:group, description: nil) | /Groups assemble/
+      build(:project, description: '_description_') | '_description_'
+      build(:group, description: '_description_') | '_description_'
     end
 
     with_them do
@@ -56,6 +41,15 @@ RSpec.describe NotifyHelper do
         expect(helper.invited_to_description(source)).to match description
       end
     end
+
+    it 'truncates long descriptions', :aggregate_failures do
+      description = '_description_ ' * 30
+      project = build(:project, description: description)
+
+      result = helper.invited_to_description(project)
+      expect(result).not_to match description
+      expect(result.length).to be <= 200
+    end
   end
 
   def reference_link(entity, url)

spec/mailers/notify_spec.rb

@@ -790,7 +790,7 @@ RSpec.describe Notify do
       it_behaves_like 'appearance header and footer not enabled'
       it_behaves_like 'does not render a manage notifications link'
 
-      context 'when there is an inviter' do
+      context 'when there is an inviter', :aggregate_failures 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
@@ -799,21 +799,16 @@ RSpec.describe Notify do
           is_expected.to have_link('Join now', href: invite_url(project_member.invite_token, invite_type: Members::InviteEmailExperiment::INVITE_TYPE))
         end
 
-        it 'contains invite link for the avatar' do
-          stub_experiments('members/invite_email': :avatar)
+        it 'contains invite link for the group activity' do
+          stub_experiments('members/invite_email': :activity)
 
+          is_expected.to have_content("#{inviter.name} invited you to join the")
+          is_expected.to have_content('Project details')
+          is_expected.to have_content("What's it about?")
           is_expected.not_to have_content('You are invited!')
           is_expected.not_to have_body_text 'What is a GitLab'
         end
 
-        it 'contains invite link for the avatar' do
-          stub_experiments('members/invite_email': :permission_info)
-
-          is_expected.not_to have_content('You are invited!')
-          is_expected.to have_body_text 'What is a GitLab'
-          is_expected.to have_body_text 'What can I do with'
-        end
-
         it 'has invite link for the control group' do
           stub_experiments('members/invite_email': :control)
 
@@ -821,7 +816,7 @@ RSpec.describe Notify do
         end
       end
 
-      context 'when there is no inviter' do
+      context 'when there is no inviter', :aggregate_failures do
         let(:inviter) { nil }
 
         it 'contains all the useful information' do

spec/models/group_spec.rb

@@ -2617,7 +2617,7 @@ RSpec.describe Group do
   context 'with export' do
     let(:group) { create(:group, :with_export) }
 
-    it '#export_file_exists returns true' do
+    it '#export_file_exists? returns true' do
       expect(group.export_file_exists?).to be true
     end
 
@@ -2625,4 +2625,54 @@ RSpec.describe Group do
       expect(group.export_archive_exists?).to be true
     end
   end
+
+  describe '#open_issues_count', :aggregate_failures do
+    let(:group) { build(:group) }
+
+    it 'provides the issue count' do
+      expect(group.open_issues_count).to eq 0
+    end
+
+    it 'invokes the count service with current_user' do
+      user = build(:user)
+      count_service = instance_double(Groups::OpenIssuesCountService)
+      expect(Groups::OpenIssuesCountService).to receive(:new).with(group, user).and_return(count_service)
+      expect(count_service).to receive(:count)
+
+      group.open_issues_count(user)
+    end
+
+    it 'invokes the count service with no current_user' do
+      count_service = instance_double(Groups::OpenIssuesCountService)
+      expect(Groups::OpenIssuesCountService).to receive(:new).with(group, nil).and_return(count_service)
+      expect(count_service).to receive(:count)
+
+      group.open_issues_count
+    end
+  end
+
+  describe '#open_merge_requests_count', :aggregate_failures do
+    let(:group) { build(:group) }
+
+    it 'provides the merge request count' do
+      expect(group.open_merge_requests_count).to eq 0
+    end
+
+    it 'invokes the count service with current_user' do
+      user = build(:user)
+      count_service = instance_double(Groups::MergeRequestsCountService)
+      expect(Groups::MergeRequestsCountService).to receive(:new).with(group, user).and_return(count_service)
+      expect(count_service).to receive(:count)
+
+      group.open_merge_requests_count(user)
+    end
+
+    it 'invokes the count service with no current_user' do
+      count_service = instance_double(Groups::MergeRequestsCountService)
+      expect(Groups::MergeRequestsCountService).to receive(:new).with(group, nil).and_return(count_service)
+      expect(count_service).to receive(:count)
+
+      group.open_merge_requests_count
+    end
+  end
 end

spec/models/project_spec.rb

@@ -994,6 +994,39 @@ RSpec.describe Project, factory_default: :keep do
     end
   end
 
+  describe '#open_issues_count', :aggregate_failures do
+    let(:project) { build(:project) }
+
+    it 'provides the issue count' do
+      expect(project.open_issues_count).to eq 0
+    end
+
+    it 'invokes the count service with current_user' do
+      user = build(:user)
+      count_service = instance_double(Projects::OpenIssuesCountService)
+      expect(Projects::OpenIssuesCountService).to receive(:new).with(project, user).and_return(count_service)
+      expect(count_service).to receive(:count)
+
+      project.open_issues_count(user)
+    end
+
+    it 'invokes the count service with no current_user' do
+      count_service = instance_double(Projects::OpenIssuesCountService)
+      expect(Projects::OpenIssuesCountService).to receive(:new).with(project, nil).and_return(count_service)
+      expect(count_service).to receive(:count)
+
+      project.open_issues_count
+    end
+  end
+
+  describe '#open_merge_requests_count' do
+    it 'provides the merge request count' do
+      project = build(:project)
+
+      expect(project.open_merge_requests_count).to eq 0
+    end
+  end
+
   describe '#issue_exists?' do
     let_it_be(:project) { create(:project) }