From a9b82b8d6a91e63b713f44e2eec30f8852a61761 Mon Sep 17 00:00:00 2001 From: Vasilii Iakliushin <viakliushin@gitlab.com> Date: Fri, 15 Nov 2024 14:52:54 +0100 Subject: [PATCH] Add missing email suffix for Emails On Push and Admin emails Contributes to https://gitlab.com/gitlab-org/gitlab/-/issues/213383 **Problem** A custom email suffix is missing for `Emails on Push` and `Admin notifications`. **Solution** Use `#subject` method to wrap subject text. It automatically applies email suffix if it's missing. Changelog: fixed --- app/mailers/emails/admin_notification.rb | 6 ++--- app/mailers/emails/projects.rb | 2 +- .../mailers/emails/admin_notification_spec.rb | 22 +++++++++++++++++++ spec/mailers/emails/projects_spec.rb | 18 +++++++++++++++ .../mailers/notify_shared_examples.rb | 10 +++++++++ 5 files changed, 54 insertions(+), 4 deletions(-) diff --git a/app/mailers/emails/admin_notification.rb b/app/mailers/emails/admin_notification.rb index 5c5497d8eb556..35814eb5647b5 100644 --- a/app/mailers/emails/admin_notification.rb +++ b/app/mailers/emails/admin_notification.rb @@ -2,18 +2,18 @@ module Emails module AdminNotification - def send_admin_notification(user_id, subject, body) + def send_admin_notification(user_id, subj, body) user = User.find(user_id) email = user.notification_email_or_default @unsubscribe_url = unsubscribe_url(email: Base64.urlsafe_encode64(email)) @body = body - mail_with_locale to: email, subject: subject + mail_with_locale to: email, subject: subject(subj) end def send_unsubscribed_notification(user_id) user = User.find(user_id) email = user.notification_email_or_default - mail_with_locale to: email, subject: "Unsubscribed from GitLab administrator notifications" + mail_with_locale to: email, subject: subject("Unsubscribed from GitLab administrator notifications") end end end diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb index ef555d69c4a27..dde6ee69a99a7 100644 --- a/app/mailers/emails/projects.rb +++ b/app/mailers/emails/projects.rb @@ -82,7 +82,7 @@ def repository_push_email(project_id, opts = {}) mail_with_locale( from: sender(@message.author_id, send_from_user_email: @message.send_from_committer_email?), reply_to: @message.reply_to, - subject: @message.subject + subject: subject_with_suffix([@message.subject]) ) end diff --git a/spec/mailers/emails/admin_notification_spec.rb b/spec/mailers/emails/admin_notification_spec.rb index 33b8558bfa3ab..3fc71e8d30e9d 100644 --- a/spec/mailers/emails/admin_notification_spec.rb +++ b/spec/mailers/emails/admin_notification_spec.rb @@ -11,4 +11,26 @@ expect(Notify).to be_respond_to(email_method) end end + + describe '#send_admin_notification' do + subject { Notify.send_admin_notification(recipient.id, 'Subject', 'Body') } + + it 'sends an email' do + expect(subject).to have_subject 'Subject' + expect(subject).to have_body_text 'Body' + end + + it_behaves_like 'an email with suffix' + end + + describe '#send_unsubscribed_notification' do + subject { Notify.send_unsubscribed_notification(recipient.id) } + + it 'sends an email' do + expect(subject).to have_subject 'Unsubscribed from GitLab administrator notifications' + expect(subject).to have_body_text 'You have been unsubscribed' + end + + it_behaves_like 'an email with suffix' + end end diff --git a/spec/mailers/emails/projects_spec.rb b/spec/mailers/emails/projects_spec.rb index fc33b7fb865b3..f39c2efd8a66d 100644 --- a/spec/mailers/emails/projects_spec.rb +++ b/spec/mailers/emails/projects_spec.rb @@ -180,6 +180,24 @@ end end + describe '#repository_push_email' do + let(:recipient) { user } + + subject { Notify.repository_push_email(project.id, { author_id: user.id, ref: 'main', action: :create }) } + + it_behaves_like 'it should not have Gmail Actions links' + it_behaves_like 'a user cannot unsubscribe through footer link' + it_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' + it_behaves_like 'an email with suffix' + + it 'has the correct subject and body' do + is_expected.to have_subject("[Git][#{project.full_path}] Pushed new branch main") + is_expected.to have_body_text(project.full_path) + is_expected.to have_body_text("#{user.name} pushed new branch main at") + end + end + describe '.inactive_project_deletion_warning_email' do let(:recipient) { user } let(:deletion_date) { "2022-01-10" } diff --git a/spec/support/shared_examples/mailers/notify_shared_examples.rb b/spec/support/shared_examples/mailers/notify_shared_examples.rb index 0ffab4b7a479b..e965787cf2dfe 100644 --- a/spec/support/shared_examples/mailers/notify_shared_examples.rb +++ b/spec/support/shared_examples/mailers/notify_shared_examples.rb @@ -36,6 +36,16 @@ end end +RSpec.shared_examples 'an email with suffix' do + before do + stub_config_setting(email_subject_suffix: 'Suffix') + end + + it 'adds a correct suffix to subject' do + expect(subject.subject).to end_with('| Suffix') + end +end + RSpec.shared_examples 'an email that contains a header with author username' do it 'has X-GitLab-Author header containing author\'s username' do is_expected.to have_header 'X-GitLab-Author', user.username -- GitLab