diff --git a/app/controllers/password_resets_controller.rb b/app/controllers/password_resets_controller.rb index ff09b5c70..202c54930 100644 --- a/app/controllers/password_resets_controller.rb +++ b/app/controllers/password_resets_controller.rb @@ -9,7 +9,7 @@ class PasswordResetsController < ApplicationController if @user.blank? flash[:notice] = "That account does not exist" redirect_to password_reset_path - elsif @user.can_receive_email?(require_verification: false) + elsif @user.can_receive_email?(require_verified_email: false) UserMailer.password_reset(@user).deliver_later UserEvent.create_from_request!(@user, :password_reset, request) flash[:notice] = "Password reset email sent. Check your email" diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index f2bbf88e1..95b8a8be8 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -90,7 +90,7 @@ class UsersController < ApplicationController flash[:notice] = "Sign up failed: #{@user.errors.full_messages.join("; ")}" else session[:user_id] = @user.id - UserMailer.welcome_user(@user).deliver_later if @user.can_receive_email?(require_verification: false) + UserMailer.welcome_user(@user).deliver_later set_current_user end diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index ae19ae2c1..6dbb708de 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -5,4 +5,11 @@ # @see https://guides.rubyonrails.org/action_mailer_basics.html class ApplicationMailer < ActionMailer::Base default from: "#{Danbooru.config.canonical_app_name} <#{Danbooru.config.contact_email}>", content_type: "text/html" + + def mail(user, require_verified_email:, **options) + to = email_address_with_name(user.email_address&.address, user.name) + message = super(to: to, **options) + message.perform_deliveries = user.can_receive_email?(require_verified_email: require_verified_email) + message + end end diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 7dec38cab..0e176df0f 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -7,24 +7,24 @@ class UserMailer < ApplicationMailer # The email sent when a user receives a DMail. def dmail_notice(dmail) @dmail = dmail - mail to: dmail.to.email_with_name, subject: "#{Danbooru.config.app_name} - Message received from #{dmail.from.name}" + mail(dmail.to, require_verified_email: true, subject: "#{Danbooru.config.app_name} - Message received from #{dmail.from.name}") end # The email sent when a user requests a password reset. def password_reset(user) @user = user - mail to: @user.email_with_name, subject: "#{Danbooru.config.app_name} password reset request" + mail(@user, require_verified_email: false, subject: "#{Danbooru.config.app_name} password reset request") end # The email sent when a user changes their email address. def email_change_confirmation(user) @user = user - mail to: @user.email_with_name, subject: "Confirm your email address" + mail(@user, require_verified_email: false, subject: "Confirm your email address") end # The email sent when a new user signs up with an email address. def welcome_user(user) @user = user - mail to: @user.email_with_name, subject: "Welcome to #{Danbooru.config.app_name}! Confirm your email address" + mail(@user, require_verified_email: false, subject: "Welcome to #{Danbooru.config.app_name}! Confirm your email address") end end diff --git a/app/models/dmail.rb b/app/models/dmail.rb index 8fdb661cd..57aa89cac 100644 --- a/app/models/dmail.rb +++ b/app/models/dmail.rb @@ -141,7 +141,7 @@ class Dmail < ApplicationRecord end def send_email - if is_recipient? && !is_deleted? && to.receive_email_notifications? && to.can_receive_email? + if is_recipient? && !is_deleted? && to.receive_email_notifications? UserMailer.dmail_notice(self).deliver_later end end diff --git a/app/models/user.rb b/app/models/user.rb index 5e54b50c0..a6594285f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -361,11 +361,7 @@ class User < ApplicationRecord end module EmailMethods - def email_with_name - "#{name} <#{email_address.address}>" - end - - def can_receive_email?(require_verification: true) + def can_receive_email?(require_verified_email: true) email_address.present? && email_address.is_deliverable? && (email_address.is_verified? || !require_verification) end diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb index 8053f0ddb..9a4a111e2 100644 --- a/test/functional/users_controller_test.rb +++ b/test/functional/users_controller_test.rb @@ -259,7 +259,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest assert_equal(User::Levels::MEMBER, User.last.level) assert_equal(User.last, User.last.authenticate_password("xxxxx1")) assert_nil(User.last.email_address) - assert_no_enqueued_emails + assert_enqueued_email_with UserMailer, :welcome_user, args: [User.last], queue: "default" assert_equal(true, User.last.user_events.user_creation.exists?) end diff --git a/test/unit/user_mailer_test.rb b/test/unit/user_mailer_test.rb index 7b96b86c4..4242d60fb 100644 --- a/test/unit/user_mailer_test.rb +++ b/test/unit/user_mailer_test.rb @@ -12,6 +12,20 @@ class UserMailerTest < ActionMailer::TestCase mail = UserMailer.dmail_notice(@dmail) assert_emails(1) { mail.deliver_now } end + + should "not send an email for a user without an email address" do + @user = create(:user) + @dmail = create(:dmail, owner: @user, to: @user) + mail = UserMailer.dmail_notice(@dmail) + assert_emails(0) { mail.deliver_now } + end + + should "not send an email for a user with an undeliverable address" do + @user = create(:user, email_address: build(:email_address, is_deliverable: false)) + @dmail = create(:dmail, owner: @user, to: @user) + mail = UserMailer.dmail_notice(@dmail) + assert_emails(0) { mail.deliver_now } + end end context "password_reset method" do