From 549e8e7baf62eb4ecb4dd039e301e6280889218d Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Fri, 17 Nov 2023 04:50:19 -0500 Subject: [PATCH] Add `email_spec` and speedup/cleanup to `spec/mailers` (#27902) --- Gemfile | 3 + Gemfile.lock | 5 + spec/mailers/admin_mailer_spec.rb | 97 +++++++------- spec/mailers/notification_mailer_spec.rb | 109 ++++++++-------- spec/mailers/user_mailer_spec.rb | 119 ++++++++++++------ spec/rails_helper.rb | 1 + .../api/v1/admin/account_actions_spec.rb | 12 +- 7 files changed, 194 insertions(+), 152 deletions(-) diff --git a/Gemfile b/Gemfile index add7b3606..74672ad06 100644 --- a/Gemfile +++ b/Gemfile @@ -109,6 +109,9 @@ group :test do # RSpec progress bar formatter gem 'fuubar', '~> 2.5' + # RSpec helpers for email specs + gem 'email_spec' + # Extra RSpec extenion methods and helpers for sidekiq gem 'rspec-sidekiq', '~> 4.0' diff --git a/Gemfile.lock b/Gemfile.lock index 20c958e2e..beec8b39c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -260,6 +260,10 @@ GEM elasticsearch-transport (7.13.3) faraday (~> 1) multi_json + email_spec (2.2.2) + htmlentities (~> 4.3.3) + launchy (~> 2.1) + mail (~> 2.7) encryptor (3.0.0) erubi (1.12.0) et-orbi (1.2.7) @@ -853,6 +857,7 @@ DEPENDENCIES doorkeeper (~> 5.6) dotenv-rails (~> 2.8) ed25519 (~> 1.3) + email_spec fabrication (~> 2.30) faker (~> 3.2) fast_blank (~> 1.0) diff --git a/spec/mailers/admin_mailer_spec.rb b/spec/mailers/admin_mailer_spec.rb index 423dce88a..9f0d89996 100644 --- a/spec/mailers/admin_mailer_spec.rb +++ b/spec/mailers/admin_mailer_spec.rb @@ -13,14 +13,13 @@ RSpec.describe AdminMailer do recipient.user.update(locale: :en) end - it 'renders the headers' do - expect(mail.subject).to eq("New report for cb6e6126.ngrok.io (##{report.id})") - expect(mail.to).to eq [recipient.user_email] - expect(mail.from).to eq ['notifications@localhost'] - end - - it 'renders the body' do - expect(mail.body.encoded).to eq("Mike,\r\n\r\nJohn has reported Mike\r\n\r\nView: https://cb6e6126.ngrok.io/admin/reports/#{report.id}\r\n") + it 'renders the email' do + expect(mail) + .to be_present + .and(deliver_to(recipient.user_email)) + .and(deliver_from('notifications@localhost')) + .and(have_subject("New report for cb6e6126.ngrok.io (##{report.id})")) + .and(have_body_text("Mike,\r\n\r\nJohn has reported Mike\r\n\r\nView: https://cb6e6126.ngrok.io/admin/reports/#{report.id}\r\n")) end end @@ -33,14 +32,13 @@ RSpec.describe AdminMailer do recipient.user.update(locale: :en) end - it 'renders the headers' do - expect(mail.subject).to eq("#{appeal.account.username} is appealing a moderation decision on cb6e6126.ngrok.io") - expect(mail.to).to eq [recipient.user_email] - expect(mail.from).to eq ['notifications@localhost'] - end - - it 'renders the body' do - expect(mail.body.encoded).to match "#{appeal.account.username} is appealing a moderation decision by #{appeal.strike.account.username}" + it 'renders the email' do + expect(mail) + .to be_present + .and(deliver_to(recipient.user_email)) + .and(deliver_from('notifications@localhost')) + .and(have_subject("#{appeal.account.username} is appealing a moderation decision on cb6e6126.ngrok.io")) + .and(have_body_text("#{appeal.account.username} is appealing a moderation decision by #{appeal.strike.account.username}")) end end @@ -53,14 +51,13 @@ RSpec.describe AdminMailer do recipient.user.update(locale: :en) end - it 'renders the headers' do - expect(mail.subject).to eq("New account up for review on cb6e6126.ngrok.io (#{user.account.username})") - expect(mail.to).to eq [recipient.user_email] - expect(mail.from).to eq ['notifications@localhost'] - end - - it 'renders the body' do - expect(mail.body.encoded).to match 'The details of the new account are below. You can approve or reject this application.' + it 'renders the email' do + expect(mail) + .to be_present + .and(deliver_to(recipient.user_email)) + .and(deliver_from('notifications@localhost')) + .and(have_subject("New account up for review on cb6e6126.ngrok.io (#{user.account.username})")) + .and(have_body_text('The details of the new account are below. You can approve or reject this application.')) end end @@ -75,14 +72,13 @@ RSpec.describe AdminMailer do recipient.user.update(locale: :en) end - it 'renders the headers' do - expect(mail.subject).to eq('New trends up for review on cb6e6126.ngrok.io') - expect(mail.to).to eq [recipient.user_email] - expect(mail.from).to eq ['notifications@localhost'] - end - - it 'renders the body' do - expect(mail.body.encoded).to match 'The following items need a review before they can be displayed publicly' + it 'renders the email' do + expect(mail) + .to be_present + .and(deliver_to(recipient.user_email)) + .and(deliver_from('notifications@localhost')) + .and(have_subject('New trends up for review on cb6e6126.ngrok.io')) + .and(have_body_text('The following items need a review before they can be displayed publicly')) end end @@ -94,14 +90,13 @@ RSpec.describe AdminMailer do recipient.user.update(locale: :en) end - it 'renders the headers' do - expect(mail.subject).to eq('New Mastodon versions are available for cb6e6126.ngrok.io!') - expect(mail.to).to eq [recipient.user_email] - expect(mail.from).to eq ['notifications@localhost'] - end - - it 'renders the body' do - expect(mail.body.encoded).to match 'New Mastodon versions have been released, you may want to update!' + it 'renders the email' do + expect(mail) + .to be_present + .and(deliver_to(recipient.user_email)) + .and(deliver_from('notifications@localhost')) + .and(have_subject('New Mastodon versions are available for cb6e6126.ngrok.io!')) + .and(have_body_text('New Mastodon versions have been released, you may want to update!')) end end @@ -113,18 +108,16 @@ RSpec.describe AdminMailer do recipient.user.update(locale: :en) end - it 'renders the headers', :aggregate_failures do - expect(mail.subject).to eq('Critical Mastodon updates are available for cb6e6126.ngrok.io!') - expect(mail.to).to eq [recipient.user_email] - expect(mail.from).to eq ['notifications@localhost'] - - expect(mail['Importance'].value).to eq 'high' - expect(mail['Priority'].value).to eq 'urgent' - expect(mail['X-Priority'].value).to eq '1' - end - - it 'renders the body' do - expect(mail.body.encoded).to match 'New critical versions of Mastodon have been released, you may want to update as soon as possible!' + it 'renders the email' do + expect(mail) + .to be_present + .and(deliver_to(recipient.user_email)) + .and(deliver_from('notifications@localhost')) + .and(have_subject('Critical Mastodon updates are available for cb6e6126.ngrok.io!')) + .and(have_body_text('New critical versions of Mastodon have been released, you may want to update as soon as possible!')) + .and(have_header('Importance', 'high')) + .and(have_header('Priority', 'urgent')) + .and(have_header('X-Priority', '1')) end end end diff --git a/spec/mailers/notification_mailer_spec.rb b/spec/mailers/notification_mailer_spec.rb index 78a497c06..eab196166 100644 --- a/spec/mailers/notification_mailer_spec.rb +++ b/spec/mailers/notification_mailer_spec.rb @@ -8,24 +8,27 @@ RSpec.describe NotificationMailer do let(:foreign_status) { Fabricate(:status, account: sender, text: 'The body of the foreign status') } let(:own_status) { Fabricate(:status, account: receiver.account, text: 'The body of the own status') } - shared_examples 'headers' do |type, thread| - it 'renders the to and from headers' do - expect(mail[:to].value).to eq "#{receiver.account.username} <#{receiver.email}>" - expect(mail.from).to eq ['notifications@localhost'] + shared_examples 'standard headers' do |type| + it 'renders the email' do + expect(mail) + .to be_present + .and(have_header('To', "#{receiver.account.username} <#{receiver.email}>")) + .and(have_header('List-ID', "<#{type}.alice.cb6e6126.ngrok.io>")) + .and(have_header('List-Unsubscribe', %r{})) + .and(have_header('List-Unsubscribe', /&type=#{type}/)) + .and(have_header('List-Unsubscribe-Post', 'List-Unsubscribe=One-Click')) + .and(deliver_to("#{receiver.account.username} <#{receiver.email}>")) + .and(deliver_from('notifications@localhost')) end + end - it 'renders the list headers' do - expect(mail['List-ID'].value).to eq "<#{type}.alice.cb6e6126.ngrok.io>" - expect(mail['List-Unsubscribe'].value).to match(%r{}) - expect(mail['List-Unsubscribe'].value).to match("&type=#{type}") - expect(mail['List-Unsubscribe-Post'].value).to eq 'List-Unsubscribe=One-Click' - end - - if thread - it 'renders the thread headers' do - expect(mail['In-Reply-To'].value).to match(//) - expect(mail['References'].value).to match(//) - end + shared_examples 'thread headers' do + it 'renders the email with conversation thread headers' do + conversation_header_regex = // + expect(mail) + .to be_present + .and(have_header('In-Reply-To', conversation_header_regex)) + .and(have_header('References', conversation_header_regex)) end end @@ -35,15 +38,15 @@ RSpec.describe NotificationMailer do let(:mail) { prepared_mailer_for(receiver.account).mention } include_examples 'localized subject', 'notification_mailer.mention.subject', name: 'bob' - include_examples 'headers', 'mention', true + include_examples 'standard headers', 'mention' + include_examples 'thread headers' - it 'renders the subject' do - expect(mail.subject).to eq('You were mentioned by bob') - end - - it 'renders the body' do - expect(mail.body.encoded).to match('You were mentioned by bob') - expect(mail.body.encoded).to include 'The body of the foreign status' + it 'renders the email' do + expect(mail) + .to be_present + .and(have_subject('You were mentioned by bob')) + .and(have_body_text('You were mentioned by bob')) + .and(have_body_text('The body of the foreign status')) end end @@ -53,14 +56,13 @@ RSpec.describe NotificationMailer do let(:mail) { prepared_mailer_for(receiver.account).follow } include_examples 'localized subject', 'notification_mailer.follow.subject', name: 'bob' - include_examples 'headers', 'follow', false + include_examples 'standard headers', 'follow' - it 'renders the subject' do - expect(mail.subject).to eq('bob is now following you') - end - - it 'renders the body' do - expect(mail.body.encoded).to match('bob is now following you') + it 'renders the email' do + expect(mail) + .to be_present + .and(have_subject('bob is now following you')) + .and(have_body_text('bob is now following you')) end end @@ -70,15 +72,15 @@ RSpec.describe NotificationMailer do let(:mail) { prepared_mailer_for(own_status.account).favourite } include_examples 'localized subject', 'notification_mailer.favourite.subject', name: 'bob' - include_examples 'headers', 'favourite', true + include_examples 'standard headers', 'favourite' + include_examples 'thread headers' - it 'renders the subject' do - expect(mail.subject).to eq('bob favorited your post') - end - - it 'renders the body' do - expect(mail.body.encoded).to match('Your post was favorited by bob') - expect(mail.body.encoded).to include 'The body of the own status' + it 'renders the email' do + expect(mail) + .to be_present + .and(have_subject('bob favorited your post')) + .and(have_body_text('Your post was favorited by bob')) + .and(have_body_text('The body of the own status')) end end @@ -88,15 +90,15 @@ RSpec.describe NotificationMailer do let(:mail) { prepared_mailer_for(own_status.account).reblog } include_examples 'localized subject', 'notification_mailer.reblog.subject', name: 'bob' - include_examples 'headers', 'reblog', true + include_examples 'standard headers', 'reblog' + include_examples 'thread headers' - it 'renders the subject' do - expect(mail.subject).to eq('bob boosted your post') - end - - it 'renders the body' do - expect(mail.body.encoded).to match('Your post was boosted by bob') - expect(mail.body.encoded).to include 'The body of the own status' + it 'renders the email' do + expect(mail) + .to be_present + .and(have_subject('bob boosted your post')) + .and(have_body_text('Your post was boosted by bob')) + .and(have_body_text('The body of the own status')) end end @@ -106,14 +108,13 @@ RSpec.describe NotificationMailer do let(:mail) { prepared_mailer_for(receiver.account).follow_request } include_examples 'localized subject', 'notification_mailer.follow_request.subject', name: 'bob' - include_examples 'headers', 'follow_request', false + include_examples 'standard headers', 'follow_request' - it 'renders the subject' do - expect(mail.subject).to eq('Pending follower: bob') - end - - it 'renders the body' do - expect(mail.body.encoded).to match('bob has requested to follow you') + it 'renders the email' do + expect(mail) + .to be_present + .and(have_subject('Pending follower: bob')) + .and(have_body_text('bob has requested to follow you')) end end diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index c661f5bbd..4a4392824 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -10,9 +10,12 @@ describe UserMailer do it 'renders confirmation instructions' do receiver.update!(locale: nil) - expect(mail.body.encoded).to include I18n.t('devise.mailer.confirmation_instructions.title') - expect(mail.body.encoded).to include 'spec' - expect(mail.body.encoded).to include Rails.configuration.x.local_domain + + expect(mail) + .to be_present + .and(have_body_text(I18n.t('devise.mailer.confirmation_instructions.title'))) + .and(have_body_text('spec')) + .and(have_body_text(Rails.configuration.x.local_domain)) end include_examples 'localized subject', @@ -25,13 +28,17 @@ describe UserMailer do it 'renders reconfirmation instructions' do receiver.update!(email: 'new-email@example.com', locale: nil) - expect(mail.body.encoded).to include I18n.t('devise.mailer.reconfirmation_instructions.title') - expect(mail.body.encoded).to include 'spec' - expect(mail.body.encoded).to include Rails.configuration.x.local_domain - expect(mail.subject).to eq I18n.t('devise.mailer.reconfirmation_instructions.subject', - instance: Rails.configuration.x.local_domain, - locale: I18n.default_locale) + + expect(mail) + .to be_present + .and(have_body_text(I18n.t('devise.mailer.reconfirmation_instructions.title'))) + .and(have_body_text('spec')) + .and(have_body_text(Rails.configuration.x.local_domain)) end + + include_examples 'localized subject', + 'devise.mailer.confirmation_instructions.subject', + instance: Rails.configuration.x.local_domain end describe '#reset_password_instructions' do @@ -39,8 +46,11 @@ describe UserMailer do it 'renders reset password instructions' do receiver.update!(locale: nil) - expect(mail.body.encoded).to include I18n.t('devise.mailer.reset_password_instructions.title') - expect(mail.body.encoded).to include 'spec' + + expect(mail) + .to be_present + .and(have_body_text(I18n.t('devise.mailer.reset_password_instructions.title'))) + .and(have_body_text('spec')) end include_examples 'localized subject', @@ -52,7 +62,10 @@ describe UserMailer do it 'renders password change notification' do receiver.update!(locale: nil) - expect(mail.body.encoded).to include I18n.t('devise.mailer.password_change.title') + + expect(mail) + .to be_present + .and(have_body_text(I18n.t('devise.mailer.password_change.title'))) end include_examples 'localized subject', @@ -64,7 +77,10 @@ describe UserMailer do it 'renders email change notification' do receiver.update!(locale: nil) - expect(mail.body.encoded).to include I18n.t('devise.mailer.email_changed.title') + + expect(mail) + .to be_present + .and(have_body_text(I18n.t('devise.mailer.email_changed.title'))) end include_examples 'localized subject', @@ -77,8 +93,11 @@ describe UserMailer do it 'renders warning notification' do receiver.update!(locale: nil) - expect(mail.body.encoded).to include I18n.t('user_mailer.warning.title.suspend', acct: receiver.account.acct) - expect(mail.body.encoded).to include strike.text + + expect(mail) + .to be_present + .and(have_body_text(I18n.t('user_mailer.warning.title.suspend', acct: receiver.account.acct))) + .and(have_body_text(strike.text)) end end @@ -88,7 +107,10 @@ describe UserMailer do it 'renders webauthn credential deleted notification' do receiver.update!(locale: nil) - expect(mail.body.encoded).to include I18n.t('devise.mailer.webauthn_credential.deleted.title') + + expect(mail) + .to be_present + .and(have_body_text(I18n.t('devise.mailer.webauthn_credential.deleted.title'))) end include_examples 'localized subject', @@ -103,7 +125,10 @@ describe UserMailer do it 'renders suspicious sign in notification' do receiver.update!(locale: nil) - expect(mail.body.encoded).to include I18n.t('user_mailer.suspicious_sign_in.explanation') + + expect(mail) + .to be_present + .and(have_body_text(I18n.t('user_mailer.suspicious_sign_in.explanation'))) end include_examples 'localized subject', @@ -115,8 +140,10 @@ describe UserMailer do let(:mail) { described_class.appeal_approved(receiver, appeal) } it 'renders appeal_approved notification' do - expect(mail.subject).to eq I18n.t('user_mailer.appeal_approved.subject', date: I18n.l(appeal.created_at)) - expect(mail.body.encoded).to include I18n.t('user_mailer.appeal_approved.title') + expect(mail) + .to be_present + .and(have_subject(I18n.t('user_mailer.appeal_approved.subject', date: I18n.l(appeal.created_at)))) + .and(have_body_text(I18n.t('user_mailer.appeal_approved.title'))) end end @@ -125,8 +152,10 @@ describe UserMailer do let(:mail) { described_class.appeal_rejected(receiver, appeal) } it 'renders appeal_rejected notification' do - expect(mail.subject).to eq I18n.t('user_mailer.appeal_rejected.subject', date: I18n.l(appeal.created_at)) - expect(mail.body.encoded).to include I18n.t('user_mailer.appeal_rejected.title') + expect(mail) + .to be_present + .and(have_subject(I18n.t('user_mailer.appeal_rejected.subject', date: I18n.l(appeal.created_at)))) + .and(have_body_text(I18n.t('user_mailer.appeal_rejected.title'))) end end @@ -134,8 +163,10 @@ describe UserMailer do let(:mail) { described_class.two_factor_enabled(receiver) } it 'renders two_factor_enabled mail' do - expect(mail.subject).to eq I18n.t('devise.mailer.two_factor_enabled.subject') - expect(mail.body.encoded).to include I18n.t('devise.mailer.two_factor_enabled.explanation') + expect(mail) + .to be_present + .and(have_subject(I18n.t('devise.mailer.two_factor_enabled.subject'))) + .and(have_body_text(I18n.t('devise.mailer.two_factor_enabled.explanation'))) end end @@ -143,8 +174,10 @@ describe UserMailer do let(:mail) { described_class.two_factor_disabled(receiver) } it 'renders two_factor_disabled mail' do - expect(mail.subject).to eq I18n.t('devise.mailer.two_factor_disabled.subject') - expect(mail.body.encoded).to include I18n.t('devise.mailer.two_factor_disabled.explanation') + expect(mail) + .to be_present + .and(have_subject(I18n.t('devise.mailer.two_factor_disabled.subject'))) + .and(have_body_text(I18n.t('devise.mailer.two_factor_disabled.explanation'))) end end @@ -152,8 +185,10 @@ describe UserMailer do let(:mail) { described_class.webauthn_enabled(receiver) } it 'renders webauthn_enabled mail' do - expect(mail.subject).to eq I18n.t('devise.mailer.webauthn_enabled.subject') - expect(mail.body.encoded).to include I18n.t('devise.mailer.webauthn_enabled.explanation') + expect(mail) + .to be_present + .and(have_subject(I18n.t('devise.mailer.webauthn_enabled.subject'))) + .and(have_body_text(I18n.t('devise.mailer.webauthn_enabled.explanation'))) end end @@ -161,8 +196,10 @@ describe UserMailer do let(:mail) { described_class.webauthn_disabled(receiver) } it 'renders webauthn_disabled mail' do - expect(mail.subject).to eq I18n.t('devise.mailer.webauthn_disabled.subject') - expect(mail.body.encoded).to include I18n.t('devise.mailer.webauthn_disabled.explanation') + expect(mail) + .to be_present + .and(have_subject(I18n.t('devise.mailer.webauthn_disabled.subject'))) + .and(have_body_text(I18n.t('devise.mailer.webauthn_disabled.explanation'))) end end @@ -170,8 +207,10 @@ describe UserMailer do let(:mail) { described_class.two_factor_recovery_codes_changed(receiver) } it 'renders two_factor_recovery_codes_changed mail' do - expect(mail.subject).to eq I18n.t('devise.mailer.two_factor_recovery_codes_changed.subject') - expect(mail.body.encoded).to include I18n.t('devise.mailer.two_factor_recovery_codes_changed.explanation') + expect(mail) + .to be_present + .and(have_subject(I18n.t('devise.mailer.two_factor_recovery_codes_changed.subject'))) + .and(have_body_text(I18n.t('devise.mailer.two_factor_recovery_codes_changed.explanation'))) end end @@ -180,8 +219,10 @@ describe UserMailer do let(:mail) { described_class.webauthn_credential_added(receiver, credential) } it 'renders webauthn_credential_added mail' do - expect(mail.subject).to eq I18n.t('devise.mailer.webauthn_credential.added.subject') - expect(mail.body.encoded).to include I18n.t('devise.mailer.webauthn_credential.added.explanation') + expect(mail) + .to be_present + .and(have_subject(I18n.t('devise.mailer.webauthn_credential.added.subject'))) + .and(have_body_text(I18n.t('devise.mailer.webauthn_credential.added.explanation'))) end end @@ -189,8 +230,10 @@ describe UserMailer do let(:mail) { described_class.welcome(receiver) } it 'renders welcome mail' do - expect(mail.subject).to eq I18n.t('user_mailer.welcome.subject') - expect(mail.body.encoded).to include I18n.t('user_mailer.welcome.explanation') + expect(mail) + .to be_present + .and(have_subject(I18n.t('user_mailer.welcome.subject'))) + .and(have_body_text(I18n.t('user_mailer.welcome.explanation'))) end end @@ -199,8 +242,10 @@ describe UserMailer do let(:mail) { described_class.backup_ready(receiver, backup) } it 'renders backup_ready mail' do - expect(mail.subject).to eq I18n.t('user_mailer.backup_ready.subject') - expect(mail.body.encoded).to include I18n.t('user_mailer.backup_ready.explanation') + expect(mail) + .to be_present + .and(have_subject(I18n.t('user_mailer.backup_ready.subject'))) + .and(have_body_text(I18n.t('user_mailer.backup_ready.explanation'))) end end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 79f98f2e2..68023b70d 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -21,6 +21,7 @@ require 'webmock/rspec' require 'paperclip/matchers' require 'capybara/rspec' require 'chewy/rspec' +require 'email_spec/rspec' Dir[Rails.root.join('spec', 'support', '**', '*.rb')].each { |f| require f } diff --git a/spec/requests/api/v1/admin/account_actions_spec.rb b/spec/requests/api/v1/admin/account_actions_spec.rb index bdf1f08e4..c14e08c21 100644 --- a/spec/requests/api/v1/admin/account_actions_spec.rb +++ b/spec/requests/api/v1/admin/account_actions_spec.rb @@ -8,18 +8,12 @@ RSpec.describe 'Account actions' do let(:scopes) { 'admin:write admin:write:accounts' } let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: scopes) } let(:headers) { { 'Authorization' => "Bearer #{token.token}" } } - let(:mailer) { instance_double(ActionMailer::MessageDelivery, deliver_later!: nil) } - - before do - allow(UserMailer).to receive(:warning).with(target_account.user, anything).and_return(mailer) - end shared_examples 'a successful notification delivery' do it 'notifies the user about the action taken' do - subject - - expect(UserMailer).to have_received(:warning).with(target_account.user, anything).once - expect(mailer).to have_received(:deliver_later!).once + expect { subject } + .to have_enqueued_job(ActionMailer::MailDeliveryJob) + .with('UserMailer', 'warning', 'deliver_now!', args: [User, AccountWarning]) end end