Merge branch 'feat/unify-html-email-layouts' into 'master'

feat: unify html layouts for members emails

Closes #16703

See merge request gitlab-org/gitlab!17699

app/assets/stylesheets/mailer.scss

+@import 'framework/variables';
+
+// Do not use 3-letter hex codes, bgcolor vs css background-color is problematic in emails
+// See https://stackoverflow.com/questions/28551981/why-are-3-digit-hex-color-code-values-interpreted-differently-in-internet-explor
+//
+// stylelint-disable color-hex-length
+
+$mailer-font: 'Helvetica Neue', Helvetica, Arial, sans-serif;
+$mailer-text-color: #333333;
+$mailer-bg-color: #fafafa;
+$mailer-link-color: #3777b0;
+$mailer-link-muted-color: #333333;
+$mailer-line-cell-bg-color: #6b4fbb;
+$mailer-wrapper-cell-bg-color: #ffffff;
+$mailer-wrapper-cell-border-color: #ededed;
+$mailer-header-footer-text-color: #5c5c5c;
+
+body {
+  margin: 0 !important;
+  background-color: $mailer-bg-color;
+  padding: 0;
+  text-align: center;
+  min-width: 640px;
+  width: 100%;
+  height: 100%;
+  font-family: $mailer-font;
+}
+
+table#body {
+  background-color: $mailer-bg-color;
+  margin: 0;
+  padding: 0;
+  text-align: center;
+  min-width: 640px;
+  width: 100%;
+}
+
+a {
+  color: $mailer-link-color;
+  text-decoration: none;
+
+  &.muted {
+    color: $mailer-link-muted-color;
+  }
+}
+
+.highlight {
+  font-weight: 500;
+}
+
+tr td {
+  font-family: $mailer-font;
+}
+
+tr.line td {
+  font-family: $mailer-font;
+  background-color: $mailer-line-cell-bg-color;
+  height: 4px;
+  font-size: 4px;
+  line-height: 4px;
+}
+
+tr.header td,
+tr.footer td,
+td.footer-message {
+  font-family: $mailer-font;
+  padding: 25px 0;
+  font-size: 13px;
+  line-height: 1.6;
+  color: $mailer-header-footer-text-color;
+}
+
+table.wrapper {
+  width: 640px;
+  margin: 0 auto;
+  border-collapse: separate;
+  border-spacing: 0;
+
+  td.wrapper-cell {
+    font-family: $mailer-font;
+    background-color: $mailer-wrapper-cell-bg-color;
+    text-align: left;
+    padding: 18px 25px;
+    border: 1px solid $mailer-wrapper-cell-border-color;
+    border-radius: 3px;
+    overflow: hidden;
+  }
+}
+
+table.content {
+  width: 100%;
+  border-collapse: separate;
+  border-spacing: 0;
+
+  td.text-content {
+    font-family: $mailer-font;
+    color: $mailer-text-color;
+    font-size: 15px;
+    font-weight: 400;
+    line-height: 1.4;
+    padding: 15px 5px;
+    text-align: center;
+  }
+}
+
+tr.footer td {
+  img {
+    display: block;
+    margin: 0 auto 1em;
+  }
+
+  .mng-notif-link,
+  .help-link {
+    color: $mailer-link-color;
+    text-decoration: none;
+  }
+}

app/assets/stylesheets/mailer_client_specific.scss

+/* CLIENT-SPECIFIC STYLES */
+
+// These are client-specific rules, ignore some linting rules
+//
+// stylelint-disable property-no-vendor-prefix, property-no-unknown, length-zero-no-unit
+// scss-lint:disable PropertySpelling, ZeroUnit
+
+body,
+table,
+td,
+a {
+  -webkit-text-size-adjust: 100%;
+  -ms-text-size-adjust: 100%;
+}
+
+table,
+td {
+  mso-table-lspace: 0pt;
+  mso-table-rspace: 0pt;
+}
+
+img {
+  -ms-interpolation-mode: bicubic;
+}
+
+.hidden {
+  display: none !important;
+  visibility: hidden !important;
+}
+
+/* iOS BLUE LINKS */
+a[x-apple-data-detectors] {
+  color: inherit !important;
+  text-decoration: none !important;
+  font-size: inherit !important;
+  font-family: inherit !important;
+  font-weight: inherit !important;
+  line-height: inherit !important;
+}
+
+/* ANDROID MARGIN HACK */
+div[style*='margin: 16px 0'] {
+  margin: 0 !important;
+}
+
+@media only screen and (max-width: 639px) {
+  body,
+  #body {
+    min-width: 320px !important;
+  }
+
+  table.wrapper {
+    width: 100% !important;
+    min-width: 320px !important;
+  }
+
+  table.wrapper td.wrapper-cell {
+    border-left: 0 !important;
+    border-right: 0 !important;
+    border-radius: 0 !important;
+    padding-left: 10px !important;
+    padding-right: 10px !important;
+  }
+}
+

app/helpers/application_helper.rb

@@ -324,6 +324,15 @@ module ApplicationHelper
     }
   end
 
+  def asset_to_string(name)
+    app = Rails.application
+    if Rails.configuration.assets.compile
+      app.assets.find_asset(name).to_s
+    else
+      controller.view_context.render(file: File.join('public/assets', app.assets_manifest.assets[name]))
+    end
+  end
+
   private
 
   def appearance

app/mailers/emails/members.rb

@@ -15,16 +15,18 @@ module Emails
 
       user = User.find(recipient_id)
 
-      mail(to: user.notification_email_for(notification_group),
-           subject: subject("Request to join the #{member_source.human_name} #{member_source.model_name.singular}"))
+      member_email_with_layout(
+        to: user.notification_email_for(notification_group),
+        subject: subject("Request to join the #{member_source.human_name} #{member_source.model_name.singular}"))
     end
 
     def member_access_granted_email(member_source_type, member_id)
       @member_source_type = member_source_type
       @member_id = member_id
 
-      mail(to: member.user.notification_email_for(notification_group),
-           subject: subject("Access to the #{member_source.human_name} #{member_source.model_name.singular} was granted"))
+      member_email_with_layout(
+        to: member.user.notification_email_for(notification_group),
+        subject: subject("Access to the #{member_source.human_name} #{member_source.model_name.singular} was granted"))
     end
 
     def member_access_denied_email(member_source_type, source_id, user_id)
@@ -33,8 +35,9 @@ module Emails
 
       user = User.find(user_id)
 
-      mail(to: user.notification_email_for(notification_group),
-           subject: subject("Access to the #{member_source.human_name} #{member_source.model_name.singular} was denied"))
+      member_email_with_layout(
+        to: user.notification_email_for(notification_group),
+        subject: subject("Access to the #{member_source.human_name} #{member_source.model_name.singular} was denied"))
     end
 
     def member_invited_email(member_source_type, member_id, token)
@@ -42,8 +45,9 @@ module Emails
       @member_id = member_id
       @token = token
 
-      mail(to: member.invite_email,
-           subject: subject("Invitation to join the #{member_source.human_name} #{member_source.model_name.singular}"))
+      member_email_with_layout(
+        to: member.invite_email,
+        subject: subject("Invitation to join the #{member_source.human_name} #{member_source.model_name.singular}"))
     end
 
     def member_invite_accepted_email(member_source_type, member_id)
@@ -51,8 +55,9 @@ module Emails
       @member_id = member_id
       return unless member.created_by
 
-      mail(to: member.created_by.notification_email_for(notification_group),
-           subject: subject('Invitation accepted'))
+      member_email_with_layout(
+        to: member.created_by.notification_email_for(notification_group),
+        subject: subject('Invitation accepted'))
     end
 
     def member_invite_declined_email(member_source_type, source_id, invite_email, created_by_id)
@@ -64,8 +69,9 @@ module Emails
 
       user = User.find(created_by_id)
 
-      mail(to: user.notification_email_for(notification_group),
-           subject: subject('Invitation declined'))
+      member_email_with_layout(
+        to: user.notification_email_for(notification_group),
+        subject: subject('Invitation declined'))
     end
 
     def member
@@ -85,5 +91,12 @@ module Emails
     def member_source_class
       @member_source_type.classify.constantize
     end
+
+    def member_email_with_layout(to:, subject:)
+      mail(to: to, subject: subject) do |format|
+        format.html { render layout: 'mailer' }
+        format.text { render layout: 'mailer' }
+      end
+    end
   end
 end

app/mailers/emails/pipelines.rb

@@ -18,12 +18,11 @@ module Emails
       @merge_request = pipeline.all_merge_requests.first
       add_headers
 
-      # We use bcc here because we don't want to generate this emails for a
+      # We use bcc here because we don't want to generate these emails for a
       # thousand times. This could be potentially expensive in a loop, and
       # recipients would contain all project watchers so it could be a lot.
       mail(bcc: recipients,
-           subject: pipeline_subject(status),
-           skip_premailer: true) do |format|
+           subject: pipeline_subject(status)) do |format|
         format.html { render layout: 'mailer' }
         format.text { render layout: 'mailer' }
       end

app/mailers/previews/notify_preview.rb

@@ -77,7 +77,7 @@ class NotifyPreview < ActionMailer::Preview
   end
 
   def import_issues_csv_email
-    Notify.import_issues_csv_email(user, project, { success: 3, errors: [5, 6, 7], valid_file: true })
+    Notify.import_issues_csv_email(user.id, project.id, { success: 3, errors: [5, 6, 7], valid_file: true })
   end
 
   def closed_merge_request_email
@@ -109,11 +109,11 @@ class NotifyPreview < ActionMailer::Preview
   end
 
   def member_access_requested_email
-    Notify.member_access_requested_email('group', user.id, user.id).message
+    Notify.member_access_requested_email(member.source_type, member.id, user.id).message
   end
 
   def member_invite_accepted_email
-    Notify.member_invite_accepted_email('project', user.id).message
+    Notify.member_invite_accepted_email(member.source_type, member.id).message
   end
 
   def member_invite_declined_email

app/views/layouts/_mailer.html.haml

-<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional //EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
+<!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
-    :css
-      /* CLIENT-SPECIFIC STYLES */
-      body, table, td, a { -webkit-text-size-adjust: 100%; -ms-text-size-adjust: 100%; }
-      table, td { mso-table-lspace: 0pt; mso-table-rspace: 0pt; }
-      img { -ms-interpolation-mode: bicubic; }
-      .hidden {
-          display: none !important;
-          visibility: hidden !important;
-      }
 
-      /* iOS BLUE LINKS */
-      a[x-apple-data-detectors] {
-          color: inherit !important;
-          text-decoration: none !important;
-          font-size: inherit !important;
-          font-family: inherit !important;
-          font-weight: inherit !important;
-          line-height: inherit !important;
-      }
+    -# 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
 
-      /* ANDROID MARGIN HACK */
-      body { margin:0 !important; }
-      div[style*="margin: 16px 0"] { margin:0 !important; }
-
-      @media only screen and (max-width: 639px) {
-          body, #body {
-              min-width: 320px !important;
-          }
-          table.wrapper {
-              width: 100% !important;
-              min-width: 320px !important;
-          }
-          table.wrapper > tbody > tr > td {
-              border-left: 0 !important;
-              border-right: 0 !important;
-              border-radius: 0 !important;
-              padding-left: 10px !important;
-              padding-right: 10px !important;
-          }
-      }
-  %body{ style: "background-color:#fafafa;margin:0;padding:0;text-align:center;min-width:640px;width:100%;height:100%;font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;" }
-    %table#body{ border: "0", cellpadding: "0", cellspacing: "0", style: "background-color:#fafafa;margin:0;padding:0;text-align:center;min-width:640px;width:100%;" }
+    = stylesheet_link_tag 'mailer.css'
+  %body
+    %table#body{ border: "0", cellpadding: "0", cellspacing: "0" }
       %tbody
         %tr.line
-          %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;background-color:#6b4fbb;height:4px;font-size:4px;line-height:4px;" }
+          %td
         %tr.header
-          %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;padding:25px 0;font-size:13px;line-height:1.6;color:#5c5c5c;" }
+          %td
             = html_header_message
             = header_logo
         %tr
-          %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;" }
-            %table.wrapper{ border: "0", cellpadding: "0", cellspacing: "0", style: "width:640px;margin:0 auto;border-collapse:separate;border-spacing:0;" }
+          %td
+            %table.wrapper{ border: "0", cellpadding: "0", cellspacing: "0" }
               %tbody
                 %tr
-                  %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;background-color:#ffffff;text-align:left;padding:18px 25px;border:1px solid #ededed;border-radius:3px;overflow:hidden;" }
-                    %table.content{ border: "0", cellpadding: "0", cellspacing: "0", style: "width:100%;border-collapse:separate;border-spacing:0;" }
+                  %td.wrapper-cell
+                    %table.content{ border: "0", cellpadding: "0", cellspacing: "0" }
                       %tbody
                         = yield
 
         = render_if_exists 'layouts/mailer/additional_text'
 
         %tr.footer
-          %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;padding:25px 0;font-size:13px;line-height:1.6;color:#5c5c5c;" }
-            %img{ alt: "GitLab", height: "33", src: image_url('mailers/gitlab_footer_logo.gif'), style: "display:block;margin:0 auto 1em;", width: "90" }/
+          %td
+            %img{ alt: "GitLab", height: "33", width: "90", src: image_url('mailers/gitlab_footer_logo.gif') }
             %div
-              - manage_notifications_link = link_to(_("Manage all notifications"), profile_notifications_url, style: "color:#3777b0;text-decoration:none;")
-              - help_link = link_to(_("Help"), help_url, style: "color:#3777b0;text-decoration:none;")
+              - manage_notifications_link = link_to(_("Manage all notifications"), profile_notifications_url, class: 'mng-notif-link')
+              - help_link = link_to(_("Help"), help_url, class: 'help-link')
               = _("You're receiving this email because of your account on %{host}. %{manage_notifications_link} &middot; %{help_link}").html_safe % { host: Gitlab.config.gitlab.host, manage_notifications_link: manage_notifications_link, help_link: help_link }
 
         = yield :additional_footer
         %tr
-          %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;padding:25px 0;font-size:13px;line-height:1.6;color:#5c5c5c;" }
+          %td.footer-message
             = html_footer_message

app/views/notify/member_access_denied_email.html.haml

-%p
-  Your request to join the
-  #{link_to member_source.human_name, member_source.web_url} #{member_source.model_name.singular}
-  has been denied.
+%tr
+  %td.text-content
+    %p
+      Your request to join the
+      #{link_to member_source.human_name, member_source.web_url, class: :highlight} #{member_source.model_name.singular}
+      has been #{content_tag :span, 'denied', class: :highlight}.
+

app/views/notify/member_access_granted_email.html.haml

 - link_end = '</a>'.html_safe
 - source_type = member_source.model_name.singular
 - leave_link = polymorphic_url([member_source], leave: 1)
-- source_link = link_to(member_source.human_name, member_source.web_url, target: '_blank', rel: 'noopener noreferrer')
+- source_link = link_to(member_source.human_name, member_source.web_url, target: '_blank', rel: 'noopener noreferrer', class: :highlight)
+- access_level = content_tag(:span, member.human_access, class: :highlight)
+
+%tr
+  %td.text-content
+    %p
+      = _('You have been granted %{access_level} access to the %{source_link} %{source_type}.').html_safe % { access_level: access_level, source_link: source_link, source_type: source_type }
+    %p
+      - leave_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: leave_link }
+      = _('If this was a mistake you can %{leave_link_start}leave the %{source_type}%{link_end}.').html_safe % { source_type: source_type, leave_link_start: leave_link_start, link_end: link_end }
 
-%p
-  = _('You have been granted %{access_level} access to the %{source_link} %{source_type}.').html_safe % { access_level: member.human_access, source_link: source_link, source_type: source_type }
-%p
-  - leave_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: leave_link }
-  = _('If this was a mistake you can %{leave_link_start}leave the %{source_type}%{link_end}.').html_safe % { source_type: source_type, leave_link_start: leave_link_start, link_end: link_end }

app/views/notify/member_access_requested_email.html.haml

-%p
-  #{link_to member.user.name, member.user} requested #{member.human_access}
-  access to the #{link_to member_source.human_name, polymorphic_url([member_source, :members])} #{member_source.model_name.singular}.
+%tr
+  %td.text-content
+    %p
+      #{link_to member.user.name, member.user, class: :highlight} requested #{content_tag :span, member.human_access, class: :highlight}
+      access to the #{link_to member_source.human_name, polymorphic_url([member_source, :members]), class: :highlight} #{member_source.model_name.singular}.
+

app/views/notify/member_invite_accepted_email.html.haml

-%p
-  #{member.invite_email}, now known as
-  #{link_to member.user.name, user_url(member.user)},
-  has accepted your invitation to join the
-  #{link_to member_source.human_name, member_source.web_url} #{member_source.model_name.singular}.
+%tr
+  %td.text-content
+    %p
+      #{content_tag :span, member.invite_email, class: :highlight}, now known as
+      #{link_to member.user.name, user_url(member.user)},
+      has accepted your invitation to join the
+      #{link_to member_source.human_name, member_source.web_url, class: :highlight} #{member_source.model_name.singular}.
+

app/views/notify/member_invite_declined_email.html.haml

-%p
-  #{@invite_email}
-  has declined your invitation to join the
-  #{link_to member_source.human_name, member_source.web_url} #{member_source.model_name.singular}.
+%tr
+  %td.text-content
+    %p
+      #{content_tag :span, @invite_email, class: :highlight}
+      has #{content_tag :span, 'declined', class: :highlight} your invitation to join the
+      #{link_to member_source.human_name, member_source.web_url, class: :highlight} #{member_source.model_name.singular}.
+

app/views/notify/member_invited_email.html.haml

-%p
-  You have been invited
-  - if member.created_by
-    by
-    = link_to member.created_by.name, user_url(member.created_by)
-  to join the
-  = link_to member_source.human_name, member_source.public? ? member_source.web_url : invite_url(@token)
-  #{member_source.model_name.singular} as #{member.human_access}.
+%tr
+  %td.text-content
+    %p
+      You have been invited
+      - if member.created_by
+        by
+        = link_to member.created_by.name, user_url(member.created_by)
+      to join the
+      = link_to member_source.human_name, member_source.public? ? member_source.web_url : invite_url(@token), class: :highlight
+      #{member_source.model_name.singular} as #{content_tag :span, member.human_access, class: :highlight}.
+
+    %p
+      = link_to 'Accept invitation', invite_url(@token)
+      or
+      = link_to 'decline', decline_invite_url(@token)
 
-%p
-  = link_to 'Accept invitation', invite_url(@token)
-  or
-  = link_to 'decline', decline_invite_url(@token)

changelogs/unreleased/feat-unify-html-email-layouts.yml

+---
+title: Unify html email layout for member html emails
+merge_request: 17699
+author: Diego Louzán
+type: added

config/application.rb

@@ -157,6 +157,8 @@ module Gitlab
     config.assets.paths << "#{config.root}/vendor/assets/fonts"
 
     config.assets.precompile << "print.css"
+    config.assets.precompile << "mailer.css"
+    config.assets.precompile << "mailer_client_specific.css"
     config.assets.precompile << "notify.css"
     config.assets.precompile << "mailers/*.css"
     config.assets.precompile << "page_bundles/ide.css"