Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Issue #4209 - Add deliver_later option in send_devise_notification #4224

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions lib/devise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,10 @@ module Test
mattr_accessor :token_generator
@@token_generator = nil

# Store devise notification delivery method, default is deliver now
mattr_accessor :deliver_later_option
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add a new commenting section to the initializer template to document this for new apps.

@@deliver_later_option = false

# Default way to set up Devise. Run rails generate devise_install to create
# a fresh initializer with all configuration values.
def self.setup
Expand Down
7 changes: 6 additions & 1 deletion lib/devise/models/authenticatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,10 @@ def devise_mailer
Devise.mailer
end

def delivery_method
Devise.deliver_later_option ? :deliver_later : :deliver_now
end

# This is an internal method called every time Devise needs
# to send a notification/mail. This can be overridden if you
# need to customize the e-mail delivery logic. For instance,
Expand Down Expand Up @@ -187,8 +191,9 @@ def devise_mailer
def send_devise_notification(notification, *args)
message = devise_mailer.send(notification, self, *args)
# Remove once we move to Rails 4.2+ only.

if message.respond_to?(:deliver_now)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove this if now, right? 😄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HexterCH what do you think of this? I guess that's the only pending piece we have here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lucasmazza , but if remove this if, that meanings if user's environment don't support deliver_now or deliver_later will failed.

And really sorry about that I respond so lately. 😭

message.deliver_now
message.send(delivery_method)
else
message.deliver
end
Expand Down
5 changes: 5 additions & 0 deletions lib/generators/templates/devise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -271,4 +271,9 @@
# When using OmniAuth, Devise cannot automatically set OmniAuth path,
# so you need to do it manually. For the users scope, it would be:
# config.omniauth_path_prefix = '/my_engine/users/auth'

# ===> ActiveJob configuration
# If you want to use ActiveJob to diliver ActionMailer messages in the backgound,
# you can config deliver_later_option to true. Default is false
# config.deliver_later_option = true
end
8 changes: 8 additions & 0 deletions test/mailers/mailer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,12 @@ def confirmation_instructions(record, token, opts = {})

assert mail.content_transfer_encoding, "7bit"
end

test "devise mailer should use deliver_later if deliver_later_option is true" do
swap Devise, deliver_later_option: true do
user = create_user
deliver_method = user.send(:delivery_method).to_s
assert_equal deliver_method, "deliver_later"
end
end
end