Move the reminder invite email into member invite mailer

- better separation of concerns
- enables easier maintainability
- see #466555

.rubocop_todo/gitlab/rails/safe_format.yml

@@ -13,7 +13,6 @@ Gitlab/Rails/SafeFormat:
     - 'app/helpers/merge_requests_helper.rb'
     - 'app/helpers/profiles_helper.rb'
     - 'app/helpers/projects_helper.rb'
-    - 'app/helpers/reminder_emails_helper.rb'
     - 'app/helpers/search_helper.rb'
     - 'app/helpers/sourcegraph_helper.rb'
     - 'app/helpers/whats_new_helper.rb'

.rubocop_todo/layout/line_length.yml

@@ -218,7 +218,6 @@ Layout/LineLength:
     - 'app/helpers/projects/security/configuration_helper.rb'
     - 'app/helpers/projects_helper.rb'
     - 'app/helpers/registrations_helper.rb'
-    - 'app/helpers/reminder_emails_helper.rb'
     - 'app/helpers/repository_languages_helper.rb'
     - 'app/helpers/routing/pseudonymization_helper.rb'
     - 'app/helpers/search_helper.rb'

.rubocop_todo/rails/output_safety.yml

@@ -31,7 +31,6 @@ Rails/OutputSafety:
     - 'app/helpers/page_layout_helper.rb'
     - 'app/helpers/profiles_helper.rb'
     - 'app/helpers/projects_helper.rb'
-    - 'app/helpers/reminder_emails_helper.rb'
     - 'app/helpers/safe_format_helper.rb'
     - 'app/helpers/search_helper.rb'
     - 'app/helpers/sidebars_helper.rb'

.rubocop_todo/style/format_string.yml

@@ -44,7 +44,6 @@ Style/FormatString:
     - 'app/helpers/projects/project_members_helper.rb'
     - 'app/helpers/projects_helper.rb'
     - 'app/helpers/registrations_helper.rb'
-    - 'app/helpers/reminder_emails_helper.rb'
     - 'app/helpers/search_helper.rb'
     - 'app/helpers/ssh_keys_helper.rb'
     - 'app/helpers/tags_helper.rb'

.rubocop_todo/style/if_unless_modifier.yml

@@ -21,7 +21,6 @@ Style/IfUnlessModifier:
     - 'app/helpers/preferences_helper.rb'
     - 'app/helpers/projects_helper.rb'
     - 'app/helpers/releases_helper.rb'
-    - 'app/helpers/reminder_emails_helper.rb'
     - 'app/helpers/routing/artifacts_helper.rb'
     - 'app/helpers/search_helper.rb'
     - 'app/helpers/snippets_helper.rb'

app/helpers/emails_helper.rb

@@ -57,6 +57,8 @@ def sanitize_name(name)
     end
   end
 
+  module_function :sanitize_name
+
   def password_reset_token_valid_time
     valid_hours = Devise.reset_password_within / 60 / 60
     if valid_hours >= 24

app/helpers/reminder_emails_helper.rb

-# frozen_string_literal: true
-
-module ReminderEmailsHelper
-  def invitation_reminder_salutation(reminder_index, format: nil)
-    case reminder_index
-    when 0
-      s_('InviteReminderEmail|Invitation pending')
-    when 1
-      if format == :html
-        wave_emoji_tag = Gitlab::Emoji.gl_emoji_tag(TanukiEmoji.find_by_alpha_code('wave'))
-        s_('InviteReminderEmail|Hey there %{wave_emoji}').html_safe % { wave_emoji: wave_emoji_tag }
-      else
-        s_('InviteReminderEmail|Hey there!')
-      end
-    when 2
-      s_('InviteReminderEmail|In case you missed it...')
-    end
-  end
-
-  def invitation_reminder_body(member, reminder_index, format: nil)
-    options = {
-      inviter: sanitize_name(member.created_by.name),
-      strong_start: '',
-      strong_end: '',
-      project_or_group_name: member_source.human_name,
-      project_or_group: member_source.model_name.singular,
-      role: member.human_access.downcase
-    }
-
-    if format == :html
-      options.merge!(
-        inviter: (link_to member.created_by.name, user_url(member.created_by)).html_safe,
-        strong_start: '<strong>'.html_safe,
-        strong_end: '</strong>'.html_safe
-      )
-    end
-
-    if reminder_index == 2
-      options[:invitation_age] = (Date.current - member.created_at.to_date).to_i
-    end
-
-    body = invitation_reminder_body_text(reminder_index)
-
-    (format == :html ? ERB::Util.html_escape(body) : body) % options
-  end
-
-  def invitation_reminder_accept_link(token, format: nil)
-    case format
-    when :html
-      link_to s_('InviteReminderEmail|Accept invitation'), invite_url(token), class: 'invite-btn-join'
-    else
-      s_('InviteReminderEmail|Accept invitation: %{invite_url}') % { invite_url: invite_url(token) }
-    end
-  end
-
-  def invitation_reminder_decline_link(token, format: nil)
-    case format
-    when :html
-      link_to s_('InviteReminderEmail|Decline invitation'), decline_invite_url(token), class: 'invite-btn-decline'
-    else
-      s_('InviteReminderEmail|Decline invitation: %{decline_url}') % { decline_url: decline_invite_url(token) }
-    end
-  end
-
-  private
-
-  def invitation_reminder_body_text(reminder_index)
-    case reminder_index
-    when 0
-      s_('InviteReminderEmail|%{inviter} is waiting for you to join the %{strong_start}%{project_or_group_name}%{strong_end} %{project_or_group} as a %{role}.')
-    when 1
-      s_('InviteReminderEmail|This is a friendly reminder that %{inviter} invited you to join the %{strong_start}%{project_or_group_name}%{strong_end} %{project_or_group} as a %{role}.')
-    when 2
-      s_("InviteReminderEmail|It's been %{invitation_age} days since %{inviter} invited you to join the %{strong_start}%{project_or_group_name}%{strong_end} %{project_or_group} as a %{role}. What would you like to do?")
-    end
-  end
-end

app/mailers/emails/members.rb

@@ -50,29 +50,6 @@ def member_access_denied_email(member_source_type, source_id, user_id)
         subject: subject("Access to the #{human_name} #{member_source.model_name.singular} was denied"))
     end
 
-    def member_invited_reminder_email(member_source_type, member_id, token, reminder_index)
-      @member_source_type = member_source_type
-      @member_id = member_id
-      @token = token
-      @reminder_index = reminder_index
-
-      return unless member_exists? && member.created_by && member.invite_to_unknown_user?
-
-      subjects = {
-        0 => s_("InviteReminderEmail|%{inviter}'s invitation to GitLab is pending"),
-        1 => s_('InviteReminderEmail|%{inviter} is waiting for you to join GitLab'),
-        2 => s_('InviteReminderEmail|%{inviter} is still waiting for you to join GitLab')
-      }
-
-      subject_line = subjects[reminder_index] % { inviter: member.created_by.name }
-
-      email_with_layout(
-        layout: 'unknown_user_mailer',
-        to: member.invite_email,
-        subject: subject(subject_line)
-      )
-    end
-
     def member_invite_accepted_email(member_source_type, member_id)
       @member_source_type = member_source_type
       @member_id = member_id

app/mailers/members/invite_mailer.rb

@@ -7,7 +7,7 @@ class InviteMailer < ApplicationMailer
     helper EmailsHelper
     helper AvatarsHelper
 
-    helper_method :member_source, :member, :invited_to_description
+    helper_method :member_source, :member, :invited_to_description, :reminder_salutation, :reminder_body
 
     layout 'unknown_user_mailer'
 
@@ -26,9 +26,27 @@ def initial_email(member, token)
       )
     end
 
+    def reminder_email(member, token, reminder_index)
+      @member = member
+      @token = token
+      @reminder_index = reminder_index
+
+      return unless member_exists? && member.created_by && member.invite_to_unknown_user?
+
+      subjects = {
+        0 => s_("InviteReminderEmail|%{inviter}'s invitation to GitLab is pending"),
+        1 => s_('InviteReminderEmail|%{inviter} is waiting for you to join GitLab'),
+        2 => s_('InviteReminderEmail|%{inviter} is still waiting for you to join GitLab')
+      }
+
+      subject_line = subjects[reminder_index] % { inviter: member.created_by.name }
+
+      mail_with_locale(to: member.invite_email, subject: EmailsHelper.subject_with_suffix([subject_line]))
+    end
+
     private
 
-    attr_reader :token, :member
+    attr_reader :token, :member, :reminder_index
 
     def member_source
       member.source
@@ -82,5 +100,68 @@ def invited_to_description(source)
 
       (source.description || default_description).truncate(200, separator: ' ')
     end
+
+    def reminder_salutation(reminder_index, format: nil)
+      case reminder_index
+      when 0
+        s_('InviteReminderEmail|Invitation pending')
+      when 1
+        if format == :html
+          wave_emoji_tag = Gitlab::Emoji.gl_emoji_tag(TanukiEmoji.find_by_alpha_code('wave'))
+          s_('InviteReminderEmail|Hey there %{wave_emoji}').html_safe % { wave_emoji: wave_emoji_tag }
+        else
+          s_('InviteReminderEmail|Hey there!')
+        end
+      when 2
+        s_('InviteReminderEmail|In case you missed it...')
+      end
+    end
+
+    def reminder_body(member, reminder_index, format: nil)
+      options = {
+        inviter: EmailsHelper.sanitize_name(member.created_by.name),
+        strong_start: '',
+        strong_end: '',
+        project_or_group_name: member_source.human_name,
+        project_or_group: member_source.model_name.singular,
+        role: member.human_access.downcase
+      }
+
+      if format == :html
+        options.merge!(
+          inviter: (
+            ActionController::Base.helpers.link_to member.created_by.name, user_url(member.created_by)
+          ).html_safe,
+          strong_start: '<strong>'.html_safe,
+          strong_end: '</strong>'.html_safe
+        )
+      end
+
+      options[:invitation_age] = (Date.current - member.created_at.to_date).to_i if reminder_index == 2
+      body = reminder_body_text(reminder_index)
+
+      (format == :html ? ERB::Util.html_escape(body) : body) % options
+    end
+
+    def reminder_body_text(reminder_index)
+      case reminder_index
+      when 0
+        s_(
+          'InviteReminderEmail|%{inviter} is waiting for you to join the ' \
+            '%{strong_start}%{project_or_group_name}%{strong_end} %{project_or_group} as a %{role}.'
+        )
+      when 1
+        s_(
+          'InviteReminderEmail|This is a friendly reminder that %{inviter} invited you to join the ' \
+            '%{strong_start}%{project_or_group_name}%{strong_end} %{project_or_group} as a %{role}.'
+        )
+      when 2
+        s_(
+          "InviteReminderEmail|It's been %{invitation_age} days since %{inviter} invited you to join the " \
+            "%{strong_start}%{project_or_group_name}%{strong_end} %{project_or_group} as a %{role}. " \
+            "What would you like to do?"
+        )
+      end
+    end
   end
 end

app/mailers/notify.rb

@@ -4,7 +4,6 @@ class Notify < ApplicationMailer
   include ActionDispatch::Routing::PolymorphicRoutes
   include GitlabRoutingHelper
   include EmailsHelper
-  include ReminderEmailsHelper
   include IssuablesHelper
 
   mattr_accessor :override_layout_lookup_table, default: {}
@@ -34,7 +33,6 @@ class Notify < ApplicationMailer
   helper DiffHelper
   helper BlobHelper
   helper EmailsHelper
-  helper ReminderEmailsHelper
   helper MembersHelper
   helper AvatarsHelper
   helper GitlabRoutingHelper

app/mailers/previews/members/invite_mailer_preview.rb

@@ -5,5 +5,17 @@ class InviteMailerPreview < ActionMailer::Preview
     def initial_email
       Members::InviteMailer.initial_email(Member.last, '1234').message
     end
+
+    def first_reminder_email
+      Members::InviteMailer.reminder_email(Member.not_accepted_invitations.with_created_by.last, '1234', 0).message
+    end
+
+    def second_reminder_email
+      Members::InviteMailer.reminder_email(Member.not_accepted_invitations.with_created_by.last, '1234', 1).message
+    end
+
+    def last_reminder_email
+      Members::InviteMailer.reminder_email(Member.not_accepted_invitations.with_created_by.last, '1234', 2).message
+    end
   end
 end

app/mailers/previews/notify_preview.rb

@@ -455,7 +455,7 @@ def group
   end
 
   def member
-    @member ||= Member.last
+    @member ||= Member.non_invite.last
   end
 
   def key

app/models/member.rb

@@ -154,6 +154,7 @@ class Member < ApplicationRecord
   scope :not_accepted_invitations_by_user, ->(user) { not_accepted_invitations.where(created_by: user) }
   scope :not_expired, ->(today = Date.current) { where(arel_table[:expires_at].gt(today).or(arel_table[:expires_at].eq(nil))) }
   scope :expiring_and_not_notified, ->(date) { where("expiry_notified_at is null AND expires_at >= ? AND expires_at <= ?", Date.current, date) }
+  scope :with_created_by, -> { where.associated(:created_by) }
 
   scope :created_today, -> do
     now = Date.current
@@ -519,7 +520,9 @@ def send_invitation_reminder(reminder_index)
 
     generate_invite_token! unless @raw_invite_token
 
-    run_after_commit_or_now { notification_service.invite_member_reminder(self, @raw_invite_token, reminder_index) }
+    run_after_commit_or_now do
+      Members::InviteMailer.reminder_email(self, @raw_invite_token, reminder_index).deliver_later
+    end
   end
 
   def create_notification_setting

app/services/notification_service.rb

@@ -564,10 +564,6 @@ def member_about_to_expire(member)
     mailer.member_about_to_expire_email(member.real_source_type, member.id).deliver_later
   end
 
-  def invite_member_reminder(group_member, token, reminder_index)
-    mailer.member_invited_reminder_email(group_member.real_source_type, group_member.id, token, reminder_index).deliver_later
-  end
-
   def project_was_moved(project, old_path_with_namespace)
     recipients = project_moved_recipients(project)
     recipients = notifiable_users(recipients, :custom, custom_action: :moved_project, project: project)

app/views/members/invite_mailer/reminder_email.html.haml

+%tr
+  %td.text-content
+    %h2.invite-header
+      = reminder_salutation(@reminder_index, format: :html)
+    %p.invite-body
+      = reminder_body(member, @reminder_index, format: :html)
+    %p.invite-actions
+      = link_to s_('InviteReminderEmail|Accept invitation'), invite_url(@token), class: 'invite-btn-join'
+      = link_to s_('InviteReminderEmail|Decline invitation'), decline_invite_url(@token), class: 'invite-btn-decline'

app/views/members/invite_mailer/reminder_email.text.erb

+<%= reminder_salutation(@reminder_index) %>
+
+<%= reminder_body(member, @reminder_index) %>
+
+<%= s_('InviteReminderEmail|Accept invitation: %{invite_url}') % { invite_url: invite_url(@token) } %>
+<%= s_('InviteReminderEmail|Decline invitation: %{decline_url}') % { decline_url: decline_invite_url(@token) } %>

app/views/notify/member_invited_reminder_email.html.haml

-%tr
-  %td.text-content
-    %h2.invite-header
-      = invitation_reminder_salutation(@reminder_index, format: :html)
-    %p.invite-body
-      = invitation_reminder_body(member, @reminder_index, format: :html)
-    %p.invite-actions
-      = invitation_reminder_accept_link(@token, format: :html)
-      = invitation_reminder_decline_link(@token, format: :html)

app/views/notify/member_invited_reminder_email.text.erb

-<%= invitation_reminder_salutation(@reminder_index) %>
-
-<%= invitation_reminder_body(member, @reminder_index) %>
-
-<%= invitation_reminder_accept_link(@token) %>
-<%= invitation_reminder_decline_link(@token) %>

spec/mailers/notify_spec.rb

@@ -1712,88 +1712,6 @@ def invite_to_group(group, inviter:, user: nil)
       )
     end
 
-    describe 'group invitation reminders' do
-      let_it_be(:inviter) { create(:user, owner_of: group) }
-
-      let(:group_member) { invite_to_group(group, inviter: inviter) }
-
-      subject { described_class.member_invited_reminder_email('Group', group_member.id, group_member.invite_token, reminder_index) }
-
-      describe 'not sending a reminder' do
-        let(:reminder_index) { 0 }
-
-        context 'member does not exist' do
-          let(:group_member) { double(id: nil, invite_token: nil) }
-
-          it_behaves_like 'no email is sent'
-        end
-
-        context 'member is not created by a user' do
-          before do
-            group_member.update!(created_by: nil)
-          end
-
-          it_behaves_like 'no email is sent'
-        end
-
-        context 'member is a known user' do
-          before do
-            group_member.update!(user: create(:user))
-          end
-
-          it_behaves_like 'no email is sent'
-        end
-      end
-
-      describe 'the first reminder' do
-        let(:reminder_index) { 0 }
-
-        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 'contains all the useful information' do
-          is_expected.to have_subject "#{inviter.name}'s invitation to GitLab is pending"
-          is_expected.to have_body_text group.human_name
-          is_expected.to have_body_text group_member.human_access.downcase
-          is_expected.to have_body_text invite_url(group_member.invite_token)
-          is_expected.to have_body_text decline_invite_url(group_member.invite_token)
-        end
-      end
-
-      describe 'the second reminder' do
-        let(:reminder_index) { 1 }
-
-        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 'contains all the useful information' do
-          is_expected.to have_subject "#{inviter.name} is waiting for you to join GitLab"
-          is_expected.to have_body_text group.human_name
-          is_expected.to have_body_text group_member.human_access.downcase
-          is_expected.to have_body_text invite_url(group_member.invite_token)
-          is_expected.to have_body_text decline_invite_url(group_member.invite_token)
-        end
-      end
-
-      describe 'the third reminder' do
-        let(:reminder_index) { 2 }
-
-        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 'contains all the useful information' do
-          is_expected.to have_subject "#{inviter.name} is still waiting for you to join GitLab"
-          is_expected.to have_body_text group.human_name
-          is_expected.to have_body_text group_member.human_access.downcase
-          is_expected.to have_body_text invite_url(group_member.invite_token)
-          is_expected.to have_body_text decline_invite_url(group_member.invite_token)
-        end
-      end
-    end
-
     describe 'group invitation accepted' do
       let(:invited_user) { create(:user, name: 'invited user') }
       let(:owner) { create(:user, owner_of: group) }

spec/mailers/previews_spec.rb

@@ -12,7 +12,6 @@
   let_it_be(:milestone) { create(:milestone, project: project) }
   let_it_be(:issue) { create(:issue, project: project, milestone: milestone) }
   let_it_be(:remote_mirror) { create(:remote_mirror, project: project) }
-  let_it_be(:member) { create(:project_member, :maintainer, project: project, created_by: user) }
   let_it_be(:review) { create(:review, project: project, merge_request: merge_request, author: user) }
   let_it_be(:key) { create(:key, user: user) }
   let_it_be(:bulk_import) { create(:bulk_import, :finished, :with_configuration) }
@@ -33,6 +32,11 @@
     }
   end
 
+  before_all do
+    create(:project_member, :maintainer, source: project, created_by: user)
+    create(:project_member, :invited, source: project, created_by: user)
+  end
+
   subject { preview.call(email) }
 
   where(:preview, :email) do