diff options
author | Alexandru Croitor <acroitor@gitlab.com> | 2019-02-20 17:18:15 +0200 |
---|---|---|
committer | Alexandru Croitor <acroitor@gitlab.com> | 2019-02-27 22:07:50 +0200 |
commit | 77985826d94454514c40b8da926e13b3b3791841 (patch) | |
tree | 00da4c2f5b72001e38b56e9d994cabc278ec0d7c | |
parent | 177f9ca88b4d080e91b6c4ce2bba04d2fba95c07 (diff) | |
download | gitlab-ce-77985826d94454514c40b8da926e13b3b3791841.tar.gz |
Show header and footer system messages in email
* Add email_header_and_footer_enabled flag to appearances table
* Set email_header_and_footer_enabled default value to false
* Add checkbox to appearance to toggle show header and footer in emails
* Add email_header_and_footer_enabled to allowed params in controller
* Add header and footer messages to the html and text email layouts
* Remove the color styling for emails header and footer
* Add empty_mailer layout for emails without layout,
to have the header and footer applied
30 files changed, 420 insertions, 22 deletions
diff --git a/app/controllers/admin/appearances_controller.rb b/app/controllers/admin/appearances_controller.rb index 2b9cae21da2..189fee98aa0 100644 --- a/app/controllers/admin/appearances_controller.rb +++ b/app/controllers/admin/appearances_controller.rb @@ -78,6 +78,7 @@ class Admin::AppearancesController < Admin::ApplicationController footer_message message_background_color message_font_color + email_header_and_footer_enabled ] end end diff --git a/app/helpers/appearances_helper.rb b/app/helpers/appearances_helper.rb index 023e44258b7..c0db9910143 100644 --- a/app/helpers/appearances_helper.rb +++ b/app/helpers/appearances_helper.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true module AppearancesHelper + include MarkupHelper + def brand_title current_appearance&.title.presence || default_brand_title end @@ -47,7 +49,7 @@ module AppearancesHelper class_names = [] class_names << 'with-performance-bar' if performance_bar_enabled? - render_message(:header_message, class_names) + render_message(:header_message, class_names: class_names) end def footer_message @@ -58,10 +60,10 @@ module AppearancesHelper private - def render_message(field_sym, class_names = []) + def render_message(field_sym, class_names: [], style: message_style) class_names << field_sym.to_s.dasherize - content_tag :div, class: class_names, style: message_style do + content_tag :div, class: class_names, style: style do markdown_field(current_appearance, field_sym) end end diff --git a/app/helpers/emails_helper.rb b/app/helpers/emails_helper.rb index dedc58f482b..96471d15aac 100644 --- a/app/helpers/emails_helper.rb +++ b/app/helpers/emails_helper.rb @@ -131,4 +131,42 @@ module EmailsHelper project.id.to_s + "." + project_path_as_domain + "." + Gitlab.config.gitlab.host end + + def html_header_message + return unless show_header? + + render_message(:header_message, style: '') + end + + def html_footer_message + return unless show_footer? + + render_message(:footer_message, style: '') + end + + def text_header_message + return unless show_header? + + strip_tags(render_message(:header_message, style: '')) + end + + def text_footer_message + return unless show_footer? + + strip_tags(render_message(:footer_message, style: '')) + end + + private + + def show_footer? + email_header_and_footer_enabled? && current_appearance&.show_footer? + end + + def show_header? + email_header_and_footer_enabled? && current_appearance&.show_header? + end + + def email_header_and_footer_enabled? + current_appearance&.email_header_and_footer_enabled? + end end diff --git a/app/mailers/abuse_report_mailer.rb b/app/mailers/abuse_report_mailer.rb index e032f568913..e0aa66e6de3 100644 --- a/app/mailers/abuse_report_mailer.rb +++ b/app/mailers/abuse_report_mailer.rb @@ -1,6 +1,10 @@ # frozen_string_literal: true class AbuseReportMailer < BaseMailer + layout 'empty_mailer' + + helper EmailsHelper + def notify(abuse_report_id) return unless deliverable? diff --git a/app/mailers/email_rejection_mailer.rb b/app/mailers/email_rejection_mailer.rb index 45fc5a6c383..d743533b1bc 100644 --- a/app/mailers/email_rejection_mailer.rb +++ b/app/mailers/email_rejection_mailer.rb @@ -1,6 +1,10 @@ # frozen_string_literal: true class EmailRejectionMailer < BaseMailer + layout 'empty_mailer' + + helper EmailsHelper + def rejection(reason, original_raw, can_retry = false) @reason = reason @original_message = Mail::Message.new(original_raw) diff --git a/app/mailers/repository_check_mailer.rb b/app/mailers/repository_check_mailer.rb index 145169be8a6..a24d3476d0e 100644 --- a/app/mailers/repository_check_mailer.rb +++ b/app/mailers/repository_check_mailer.rb @@ -2,6 +2,10 @@ class RepositoryCheckMailer < BaseMailer # rubocop: disable CodeReuse/ActiveRecord + layout 'empty_mailer' + + helper EmailsHelper + def notify(failed_count) @message = if failed_count == 1 diff --git a/app/models/appearance.rb b/app/models/appearance.rb index b9ad676ca47..bdee9b2b73c 100644 --- a/app/models/appearance.rb +++ b/app/models/appearance.rb @@ -20,6 +20,7 @@ class Appearance < ActiveRecord::Base default_value_for :message_background_color, '#E75E40' default_value_for :message_font_color, '#FFFFFF' + default_value_for :email_header_and_footer_enabled, false mount_uploader :logo, AttachmentUploader mount_uploader :header_logo, AttachmentUploader diff --git a/app/views/admin/appearances/_system_header_footer_form.html.haml b/app/views/admin/appearances/_system_header_footer_form.html.haml index ca9d6adebeb..4301ebd05af 100644 --- a/app/views/admin/appearances/_system_header_footer_form.html.haml +++ b/app/views/admin/appearances/_system_header_footer_form.html.haml @@ -13,6 +13,15 @@ .form-group = form.label :footer_message, _('Footer message'), class: 'col-form-label label-bold' = form.text_area :footer_message, placeholder: _('State your message to activate'), class: "form-control js-autosize" + .form-group + .form-check + = form.check_box :email_header_and_footer_enabled, class: 'form-check-input' + = form.label :email_header_and_footer_enabled, class: 'label-bold' do + = _('Enable header and footer in emails') + + .hint + = _('Add header and footer to emails. Please note that color settings will only be applied within the application interface') + .form-group.js-toggle-colors-container %button.btn.btn-link.js-toggle-colors-link{ type: 'button' } = _('Customize colors') diff --git a/app/views/layouts/_mailer.html.haml b/app/views/layouts/_mailer.html.haml index 26fd34347ec..e13490ed410 100644 --- a/app/views/layouts/_mailer.html.haml +++ b/app/views/layouts/_mailer.html.haml @@ -52,6 +52,7 @@ %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;background-color:#6b4fbb;height:4px;font-size:4px;line-height:4px;" } %tr.header %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;padding:25px 0;font-size:13px;line-height:1.6;color:#5c5c5c;" } + = html_header_message = header_logo %tr %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;" } @@ -72,3 +73,6 @@ = _("You're receiving this email because of your account on %{host}. %{manage_notifications_link} · %{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;" } + = html_footer_message diff --git a/app/views/layouts/empty_mailer.html.haml b/app/views/layouts/empty_mailer.html.haml new file mode 100644 index 00000000000..a25dcefd445 --- /dev/null +++ b/app/views/layouts/empty_mailer.html.haml @@ -0,0 +1,5 @@ += html_header_message + += yield + += html_footer_message diff --git a/app/views/layouts/empty_mailer.text.erb b/app/views/layouts/empty_mailer.text.erb new file mode 100644 index 00000000000..6ab0dbead07 --- /dev/null +++ b/app/views/layouts/empty_mailer.text.erb @@ -0,0 +1,5 @@ +<%= text_header_message %> + +<%= yield -%> + +<%= text_footer_message %> diff --git a/app/views/layouts/mailer.text.erb b/app/views/layouts/mailer.text.erb index 8e11174f8d7..f8032f3262b 100644 --- a/app/views/layouts/mailer.text.erb +++ b/app/views/layouts/mailer.text.erb @@ -1,4 +1,8 @@ +<%= text_header_message %> + <%= yield -%> -- <%# signature marker %> <%= _("You're receiving this email because of your account on %{host}.") % { host: Gitlab.config.gitlab.host } %> + +<%= text_footer_message %> diff --git a/app/views/layouts/notify.html.haml b/app/views/layouts/notify.html.haml index 1c3e05e07f4..8dff12c1b7f 100644 --- a/app/views/layouts/notify.html.haml +++ b/app/views/layouts/notify.html.haml @@ -7,6 +7,7 @@ = yield :head %body .content + = html_header_message = yield .footer{ style: "margin-top: 10px;" } %p @@ -30,3 +31,4 @@ adjust your notification settings. = email_action @target_url + = html_footer_message diff --git a/app/views/layouts/notify.text.erb b/app/views/layouts/notify.text.erb index 9dc490efa9a..248916fba63 100644 --- a/app/views/layouts/notify.text.erb +++ b/app/views/layouts/notify.text.erb @@ -1,3 +1,5 @@ +<%= text_header_message %> + <%= yield -%> -- <%# signature marker %> @@ -10,3 +12,5 @@ <% end -%> <%= "You're receiving this email because #{notification_reason_text(@reason)}." %> + +<%= text_footer_message -%> diff --git a/changelogs/unreleased/56863-system-messages-in-email.yml b/changelogs/unreleased/56863-system-messages-in-email.yml new file mode 100644 index 00000000000..21a90aa95ee --- /dev/null +++ b/changelogs/unreleased/56863-system-messages-in-email.yml @@ -0,0 +1,5 @@ +--- +title: Show header and footer system messages in email +merge_request: 25474 +author: +type: added diff --git a/db/migrate/20190220142344_add_email_header_and_footer_enabled_flag_to_appearances_table.rb b/db/migrate/20190220142344_add_email_header_and_footer_enabled_flag_to_appearances_table.rb new file mode 100644 index 00000000000..85b9e0580f4 --- /dev/null +++ b/db/migrate/20190220142344_add_email_header_and_footer_enabled_flag_to_appearances_table.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddEmailHeaderAndFooterEnabledFlagToAppearancesTable < ActiveRecord::Migration[5.0] + include Gitlab::Database::MigrationHelpers + + disable_ddl_transaction! + + DOWNTIME = false + + def up + add_column_with_default(:appearances, :email_header_and_footer_enabled, :boolean, default: false) + end + + def down + remove_column(:appearances, :email_header_and_footer_enabled) + end +end diff --git a/db/schema.rb b/db/schema.rb index a7a83475679..efb0aa7bf7c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -44,6 +44,7 @@ ActiveRecord::Schema.define(version: 20190220150130) do t.text "message_background_color" t.text "message_font_color" t.string "favicon" + t.boolean "email_header_and_footer_enabled", default: false, null: false end create_table "application_setting_terms", force: :cascade do |t| diff --git a/doc/customization/system_header_and_footer_messages.md b/doc/customization/system_header_and_footer_messages.md index cf7d8b3f3e8..9d6931c730d 100644 --- a/doc/customization/system_header_and_footer_messages.md +++ b/doc/customization/system_header_and_footer_messages.md @@ -7,6 +7,10 @@ Navigate to the **Admin** area and go to the **Appearance** page. Under **System header and footer** insert your header message and/or footer message. Both background and font color of the header and footer are customizable. +You can also apply the header and footer messages to gitlab emails, +by checking the **Enable header and footer in emails** checkbox. +Note that color settings will only be applied within the app interface and not to emails + ![appearance](system_header_and_footer_messages/appearance.png) After saving, all GitLab pages will contain the custom system header and/or footer messages: diff --git a/doc/customization/system_header_and_footer_messages/appearance.png b/doc/customization/system_header_and_footer_messages/appearance.png Binary files differindex 14667f751c4..fd315bb6c07 100644 --- a/doc/customization/system_header_and_footer_messages/appearance.png +++ b/doc/customization/system_header_and_footer_messages/appearance.png diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 588dd766bdc..c699097be4a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -393,6 +393,9 @@ msgstr "" msgid "Add a table" msgstr "" +msgid "Add header and footer to emails. Please note that color settings will only be applied within the application interface" +msgstr "" + msgid "Add image comment" msgstr "" @@ -2959,6 +2962,9 @@ msgstr "" msgid "Enable group Runners" msgstr "" +msgid "Enable header and footer in emails" +msgstr "" + msgid "Enable or disable version check and usage ping." msgstr "" diff --git a/spec/controllers/admin/appearances_controller_spec.rb b/spec/controllers/admin/appearances_controller_spec.rb index 4ddd0953267..621aa148301 100644 --- a/spec/controllers/admin/appearances_controller_spec.rb +++ b/spec/controllers/admin/appearances_controller_spec.rb @@ -1,15 +1,17 @@ +# frozen_string_literal: true + require 'spec_helper' describe Admin::AppearancesController do let(:admin) { create(:admin) } - let(:header_message) { "Header message" } - let(:footer_message) { "Footer" } + let(:header_message) { 'Header message' } + let(:footer_message) { 'Footer' } describe 'POST #create' do let(:create_params) do { - title: "Foo", - description: "Bar", + title: 'Foo', + description: 'Bar', header_message: header_message, footer_message: footer_message } @@ -24,9 +26,26 @@ describe Admin::AppearancesController do expect(Appearance.current).to have_attributes( header_message: header_message, - footer_message: footer_message + footer_message: footer_message, + email_header_and_footer_enabled: false, + message_background_color: '#E75E40', + message_font_color: '#FFFFFF' ) end + + context 'when enabling header and footer in email' do + it 'creates appearance with enabled flag' do + create_params[:email_header_and_footer_enabled] = true + + post :create, params: { appearance: create_params } + + expect(Appearance.current).to have_attributes( + header_message: header_message, + footer_message: footer_message, + email_header_and_footer_enabled: true + ) + end + end end describe 'PUT #update' do @@ -48,8 +67,25 @@ describe Admin::AppearancesController do expect(Appearance.current).to have_attributes( header_message: header_message, - footer_message: footer_message + footer_message: footer_message, + email_header_and_footer_enabled: false, + message_background_color: '#E75E40', + message_font_color: '#FFFFFF' ) end + + context 'when enabling header and footer in email' do + it 'updates appearance with enabled flag' do + update_params[:email_header_and_footer_enabled] = true + + post :update, params: { appearance: update_params } + + expect(Appearance.current).to have_attributes( + header_message: header_message, + footer_message: footer_message, + email_header_and_footer_enabled: true + ) + end + end end end diff --git a/spec/helpers/emails_helper_spec.rb b/spec/helpers/emails_helper_spec.rb index 23d7e41803e..03b4c19ec22 100644 --- a/spec/helpers/emails_helper_spec.rb +++ b/spec/helpers/emails_helper_spec.rb @@ -142,4 +142,58 @@ describe EmailsHelper do end end end + + describe 'header and footer messages' do + context 'when email_header_and_footer_enabled is enabled' do + it 'returns header and footer messages' do + create :appearance, header_message: 'Foo', footer_message: 'Bar', email_header_and_footer_enabled: true + + aggregate_failures do + expect(html_header_message).to eq(%{<div class="header-message" style=""><p>Foo</p></div>}) + expect(html_footer_message).to eq(%{<div class="footer-message" style=""><p>Bar</p></div>}) + expect(text_header_message).to eq('Foo') + expect(text_footer_message).to eq('Bar') + end + end + + context 'when header and footer messages are empty' do + it 'returns nil' do + create :appearance, header_message: '', footer_message: '', email_header_and_footer_enabled: true + + aggregate_failures do + expect(html_header_message).to eq(nil) + expect(html_footer_message).to eq(nil) + expect(text_header_message).to eq(nil) + expect(text_footer_message).to eq(nil) + end + end + end + + context 'when header and footer messages are nil' do + it 'returns nil' do + create :appearance, header_message: nil, footer_message: nil, email_header_and_footer_enabled: true + + aggregate_failures do + expect(html_header_message).to eq(nil) + expect(html_footer_message).to eq(nil) + expect(text_header_message).to eq(nil) + expect(text_footer_message).to eq(nil) + end + end + end + end + + context 'when email_header_and_footer_enabled is disabled' do + it 'returns header and footer messages' do + create :appearance, header_message: 'Foo', footer_message: 'Bar', email_header_and_footer_enabled: false + + aggregate_failures do + expect(html_header_message).to eq(nil) + expect(html_footer_message).to eq(nil) + expect(text_header_message).to eq(nil) + expect(text_footer_message).to eq(nil) + end + end + end + end end diff --git a/spec/mailers/abuse_report_mailer_spec.rb b/spec/mailers/abuse_report_mailer_spec.rb index bda892083b3..f96870cc112 100644 --- a/spec/mailers/abuse_report_mailer_spec.rb +++ b/spec/mailers/abuse_report_mailer_spec.rb @@ -4,25 +4,24 @@ describe AbuseReportMailer do include EmailSpec::Matchers describe '.notify' do - context 'with admin_notification_email set' do - before do - stub_application_setting(admin_notification_email: 'admin@example.com') - end + before do + stub_application_setting(admin_notification_email: 'admin@example.com') + end - it 'sends to the admin_notification_email' do - report = create(:abuse_report) + let(:report) { create(:abuse_report) } + + subject { described_class.notify(report.id) } - mail = described_class.notify(report.id) + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' - expect(mail).to deliver_to 'admin@example.com' + context 'with admin_notification_email set' do + it 'sends to the admin_notification_email' do + is_expected.to deliver_to 'admin@example.com' end it 'includes the user in the subject' do - report = create(:abuse_report) - - mail = described_class.notify(report.id) - - expect(mail).to have_subject "#{report.user.name} (#{report.user.username}) was reported for abuse" + is_expected.to have_subject "#{report.user.name} (#{report.user.username}) was reported for abuse" end end diff --git a/spec/mailers/email_rejection_mailer_spec.rb b/spec/mailers/email_rejection_mailer_spec.rb new file mode 100644 index 00000000000..bbe0a50ae8e --- /dev/null +++ b/spec/mailers/email_rejection_mailer_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe EmailRejectionMailer do + include EmailSpec::Matchers + + describe '#rejection' do + let(:raw_email) { 'From: someone@example.com\nraw email here' } + + subject { described_class.rejection('some rejection reason', raw_email) } + + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' + end +end diff --git a/spec/mailers/emails/auto_devops_spec.rb b/spec/mailers/emails/auto_devops_spec.rb index 839caf3f50e..dd7c12c3143 100644 --- a/spec/mailers/emails/auto_devops_spec.rb +++ b/spec/mailers/emails/auto_devops_spec.rb @@ -13,6 +13,9 @@ describe Emails::AutoDevops do subject { Notify.autodevops_disabled_email(pipeline, owner.email) } + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' + it 'sents email with correct subject' do is_expected.to have_subject("#{project.name} | Auto DevOps pipeline was disabled for #{project.name}") end diff --git a/spec/mailers/emails/issues_spec.rb b/spec/mailers/emails/issues_spec.rb index 09253cf8003..5b5bd6f4308 100644 --- a/spec/mailers/emails/issues_spec.rb +++ b/spec/mailers/emails/issues_spec.rb @@ -29,5 +29,14 @@ describe Emails::Issues do expect(subject).to have_body_text "23, 34, 58" end + + context 'with header and footer' do + let(:results) { { success: 165, error_lines: [], parse_error: false } } + + subject { Notify.import_issues_csv_email(user.id, project.id, results) } + + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' + end end end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 4f578c48d5b..15b04c9d066 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -53,6 +53,8 @@ describe Notify do end it_behaves_like 'it should show Gmail Actions View Issue link' it_behaves_like 'an unsubscribeable thread' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'has the correct subject and body' do aggregate_failures do @@ -72,6 +74,9 @@ describe Notify do context 'when sent with a reason' do subject { described_class.new_issue_email(issue.assignees.first.id, issue.id, NotificationReason::ASSIGNED) } + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' + it 'includes the reason in a header' do is_expected.to have_header('X-GitLab-NotificationReason', NotificationReason::ASSIGNED) end @@ -99,6 +104,8 @@ describe Notify do end it_behaves_like 'it should show Gmail Actions View Issue link' it_behaves_like 'an unsubscribeable thread' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -118,6 +125,9 @@ describe Notify do context 'when sent with a reason' do subject { described_class.reassigned_issue_email(recipient.id, issue.id, [previous_assignee.id], current_user.id, NotificationReason::ASSIGNED) } + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' + it 'includes the reason in a header' do is_expected.to have_header('X-GitLab-NotificationReason', NotificationReason::ASSIGNED) end @@ -134,6 +144,8 @@ describe Notify do it_behaves_like 'it should show Gmail Actions View Issue link' it_behaves_like 'a user cannot unsubscribe through footer link' it_behaves_like 'an email with a labels subscriptions link in its footer' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -173,6 +185,8 @@ describe Notify do end it_behaves_like 'it should show Gmail Actions View Issue link' it_behaves_like 'an unsubscribeable thread' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -199,6 +213,8 @@ describe Notify do end it_behaves_like 'it should show Gmail Actions View Issue link' it_behaves_like 'an unsubscribeable thread' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'contains description about action taken' do is_expected.to have_body_text 'Issue was moved to another project' @@ -226,6 +242,8 @@ describe Notify do end it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like 'an unsubscribeable thread' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'has the correct subject and body' do aggregate_failures do @@ -243,6 +261,9 @@ describe Notify do context 'when sent with a reason' do subject { described_class.new_merge_request_email(merge_request.assignee_id, merge_request.id, NotificationReason::ASSIGNED) } + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' + it 'includes the reason in a header' do is_expected.to have_header('X-GitLab-NotificationReason', NotificationReason::ASSIGNED) end @@ -270,6 +291,8 @@ describe Notify do end it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like "an unsubscribeable thread" + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -289,6 +312,9 @@ describe Notify do context 'when sent with a reason' do subject { described_class.reassigned_merge_request_email(recipient.id, merge_request.id, previous_assignee.id, current_user.id, NotificationReason::ASSIGNED) } + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' + it 'includes the reason in a header' do is_expected.to have_header('X-GitLab-NotificationReason', NotificationReason::ASSIGNED) end @@ -313,6 +339,8 @@ describe Notify do it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like "an unsubscribeable thread" + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'contains the description' do is_expected.to have_body_text(merge_request.description) @@ -329,6 +357,8 @@ describe Notify do it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like 'a user cannot unsubscribe through footer link' it_behaves_like 'an email with a labels subscriptions link in its footer' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -352,6 +382,8 @@ describe Notify do end it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like 'an unsubscribeable thread' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -379,6 +411,8 @@ describe Notify do end it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like 'an unsubscribeable thread' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'is sent as the merge author' do sender = subject.header[:from].addrs[0] @@ -413,6 +447,8 @@ describe Notify do end it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like 'an unsubscribeable thread' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'is sent as the merge request author' do sender = subject.header[:from].addrs[0] @@ -442,6 +478,8 @@ describe Notify do end it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like 'an unsubscribeable thread' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'is sent as the push user' do sender = subject.header[:from].addrs[0] @@ -482,6 +520,9 @@ describe Notify do subject { described_class.note_issue_email(recipient.id, third_note.id) } + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' + it 'has In-Reply-To header pointing to previous note in discussion' do expect(subject.header['In-Reply-To'].message_ids).to eq(["note_#{second_note.id}@#{host}"]) end @@ -502,6 +543,9 @@ describe Notify do subject { described_class.note_issue_email(recipient.id, note.id) } + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' + it 'has In-Reply-To header pointing to the issue' do expect(subject.header['In-Reply-To'].message_ids).to eq(["issue_#{note.noteable.id}@#{host}"]) end @@ -518,6 +562,9 @@ describe Notify do subject { described_class.note_project_snippet_email(project_snippet_note.author_id, project_snippet_note.id) } + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' + it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do let(:model) { project_snippet } end @@ -535,6 +582,8 @@ describe Notify 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 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'has the correct subject and body' do is_expected.to have_subject("#{project.name} | Project was moved") @@ -559,6 +608,8 @@ describe Notify 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 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'contains all the useful information' do to_emails = subject.header[:to].addrs.map(&:address) @@ -582,6 +633,8 @@ describe Notify 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 '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 "Access to the #{project.full_name} project was denied" @@ -599,6 +652,8 @@ describe Notify 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 '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 "Access to the #{project.full_name} project was granted" @@ -629,6 +684,8 @@ describe Notify 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 '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" @@ -653,6 +710,8 @@ describe Notify 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 '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 accepted' @@ -676,6 +735,8 @@ describe Notify 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 '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 declined' @@ -708,6 +769,8 @@ describe Notify do end it_behaves_like 'it should show Gmail Actions View Commit 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' it 'has the correct subject and body' do aggregate_failures do @@ -732,6 +795,8 @@ describe Notify do end it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like 'an unsubscribeable thread' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'has the correct subject and body' do aggregate_failures do @@ -756,6 +821,8 @@ describe Notify do end it_behaves_like 'it should show Gmail Actions View Issue link' it_behaves_like 'an unsubscribeable thread' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'has the correct subject and body' do aggregate_failures do @@ -819,6 +886,8 @@ describe Notify do end it_behaves_like 'it should show Gmail Actions View Commit 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' it 'has the correct subject' do is_expected.to have_subject "Re: #{project.name} | #{commit.title} (#{commit.short_id})" @@ -845,6 +914,8 @@ describe Notify do end it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like 'an unsubscribeable thread' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'has the correct subject' do is_expected.to have_referable_subject(merge_request, reply: true) @@ -871,6 +942,8 @@ describe Notify do end it_behaves_like 'it should show Gmail Actions View Issue link' it_behaves_like 'an unsubscribeable thread' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'has the correct subject' do is_expected.to have_referable_subject(issue, reply: true) @@ -948,6 +1021,8 @@ describe Notify do it_behaves_like 'an email for a note on a diff discussion', :diff_note_on_commit it_behaves_like 'it should show Gmail Actions View Commit 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' end describe 'on a merge request' do @@ -958,6 +1033,8 @@ describe Notify do it_behaves_like 'an email for a note on a diff discussion', :diff_note_on_merge_request it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like 'an unsubscribeable thread' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' end end end @@ -976,6 +1053,8 @@ describe Notify 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 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'contains all the useful information' do to_emails = subject.header[:to].addrs.map(&:address) @@ -998,6 +1077,8 @@ describe Notify 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 '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 "Access to the #{group.name} group was denied" @@ -1014,6 +1095,8 @@ describe Notify 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 '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 "Access to the #{group.name} group was granted" @@ -1044,6 +1127,8 @@ describe Notify 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 '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 #{group.name} group" @@ -1068,6 +1153,8 @@ describe Notify 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 '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 accepted' @@ -1091,6 +1178,8 @@ describe Notify 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 '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 declined' @@ -1140,6 +1229,8 @@ describe Notify do it_behaves_like 'a user cannot unsubscribe through footer link' it_behaves_like 'an email with X-GitLab headers containing project details' it_behaves_like 'an email that contains a header with author username' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -1165,6 +1256,8 @@ describe Notify do it_behaves_like "a user cannot unsubscribe through footer link" it_behaves_like 'an email with X-GitLab headers containing project details' it_behaves_like 'an email that contains a header with author username' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -1189,6 +1282,8 @@ describe Notify do it_behaves_like 'a user cannot unsubscribe through footer link' it_behaves_like 'an email with X-GitLab headers containing project details' it_behaves_like 'an email that contains a header with author username' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -1210,6 +1305,8 @@ describe Notify do it_behaves_like 'a user cannot unsubscribe through footer link' it_behaves_like 'an email with X-GitLab headers containing project details' it_behaves_like 'an email that contains a header with author username' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -1237,6 +1334,8 @@ describe Notify do it_behaves_like 'a user cannot unsubscribe through footer link' it_behaves_like 'an email with X-GitLab headers containing project details' it_behaves_like 'an email that contains a header with author username' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -1328,6 +1427,8 @@ describe Notify do it_behaves_like 'a user cannot unsubscribe through footer link' it_behaves_like 'an email with X-GitLab headers containing project details' it_behaves_like 'an email that contains a header with author username' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -1348,6 +1449,11 @@ describe Notify do describe 'HTML emails setting' do let(:multipart_mail) { described_class.project_was_moved_email(project.id, user.id, "gitlab/gitlab") } + subject { multipart_mail } + + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' + context 'when disabled' do it 'only sends the text template' do stub_application_setting(html_emails_enabled: false) @@ -1386,6 +1492,8 @@ describe Notify do subject { described_class.note_personal_snippet_email(personal_snippet_note.author_id, personal_snippet_note.id) } 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 'has the correct subject and body' do is_expected.to have_referable_subject(personal_snippet, reply: true) diff --git a/spec/mailers/repository_check_mailer_spec.rb b/spec/mailers/repository_check_mailer_spec.rb index 00613c7b671..384660f7221 100644 --- a/spec/mailers/repository_check_mailer_spec.rb +++ b/spec/mailers/repository_check_mailer_spec.rb @@ -17,5 +17,12 @@ describe RepositoryCheckMailer do expect(mail).to have_subject 'GitLab Admin | 3 projects failed their last repository check' end + + context 'with footer and header' do + subject { described_class.notify(1) } + + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' + end end end diff --git a/spec/models/appearance_spec.rb b/spec/models/appearance_spec.rb index 28d482adebf..3e95aa2b5dd 100644 --- a/spec/models/appearance_spec.rb +++ b/spec/models/appearance_spec.rb @@ -78,4 +78,22 @@ describe Appearance do it { is_expected.to allow_value(hex).for(:message_font_color) } it { is_expected.not_to allow_value('000').for(:message_font_color) } end + + describe 'email_header_and_footer_enabled' do + context 'default email_header_and_footer_enabled flag value' do + it 'returns email_header_and_footer_enabled as true' do + appearance = build(:appearance) + + expect(appearance.email_header_and_footer_enabled?).to eq(false) + end + end + + context 'when setting email_header_and_footer_enabled flag value' do + it 'returns email_header_and_footer_enabled as true' do + appearance = build(:appearance, email_header_and_footer_enabled: true) + + expect(appearance.email_header_and_footer_enabled?).to eq(true) + end + end + end end diff --git a/spec/support/shared_examples/notify_shared_examples.rb b/spec/support/shared_examples/notify_shared_examples.rb index a38354060cf..4fff1c4e228 100644 --- a/spec/support/shared_examples/notify_shared_examples.rb +++ b/spec/support/shared_examples/notify_shared_examples.rb @@ -252,3 +252,31 @@ shared_examples 'a note email' do end end end + +shared_examples 'appearance header and footer enabled' do + it "contains header and footer" do + create :appearance, header_message: "Foo", footer_message: "Bar", email_header_and_footer_enabled: true + + aggregate_failures do + expect(subject.html_part).to have_body_text("<div class=\"header-message\" style=\"\"><p>Foo</p></div>") + expect(subject.html_part).to have_body_text("<div class=\"footer-message\" style=\"\"><p>Bar</p></div>") + + expect(subject.text_part).to have_body_text(/^Foo/) + expect(subject.text_part).to have_body_text(/Bar$/) + end + end +end + +shared_examples 'appearance header and footer not enabled' do + it "does not contain header and footer" do + create :appearance, header_message: "Foo", footer_message: "Bar", email_header_and_footer_enabled: false + + aggregate_failures do + expect(subject.html_part).not_to have_body_text("<div class=\"header-message\" style=\"\"><p>Foo</p></div>") + expect(subject.html_part).not_to have_body_text("<div class=\"footer-message\" style=\"\"><p>Bar</p></div>") + + expect(subject.text_part).not_to have_body_text(/^Foo/) + expect(subject.text_part).not_to have_body_text(/Bar$/) + end + end +end |