send activation email after the commit in transactional databases #130
Conversation
| @@ -33,7 +33,15 @@ def define_field(name, type, options={}) | |||
| end | |||
|
|
|||
| def define_callback(time, event, method_name, options={}) | |||
| @klass.send "#{time}_#{event}", method_name, options.slice(:if) | |||
| callback_name = if event == :commit && options[:on] == :create | |||
There was a problem hiding this comment.
it's better to exclude it to a method and use case instead of if/elsif
|
@youzik thanks for the comment, I've moved code to separate method. Not sure about the case statement, because a condition does not depend on one parameter. Can you suggest me how to reproduce specs failing locally? https://travis-ci.org/Sorcery/sorcery/jobs/391800300 . I tried: |
|
@anaumov I believe the issue you're having is that Fixnum and Bignum were refactored in ruby itself in either Try switching rvm to use the same version as the failing travis build: |
|
@athix I don't care about Fixnum and Bignum issues :) I worry that next line fails without any backtrace |
|
I turned off Thanks @athix, I set ruby to 2.9 and specs work fine. |
| @@ -45,7 +45,7 @@ def self.included(base) | |||
| # don't setup activation if no password supplied - this user is created automatically | |||
| sorcery_adapter.define_callback :before, :create, :setup_activation, if: proc { |user| user.send(sorcery_config.password_attribute_name).present? } | |||
| # don't send activation needed email if no crypted password created - this user is external (OAuth etc.) | |||
| sorcery_adapter.define_callback :after, :create, :send_activation_needed_email!, if: :send_activation_needed_email? | |||
| sorcery_adapter.define_callback :after, :commit, :send_activation_needed_email!, on: :create, if: :send_activation_needed_email? | |||
There was a problem hiding this comment.
Interesting, I honestly didn't realize rails had a callback for specifically after the DB commit.
👍
| @@ -29,7 +29,7 @@ class TestMailer < ActionMailer::Base; end | |||
| config.include RSpec::Rails::ControllerExampleGroup, file_path: /controller(.)*_spec.rb$/ | |||
| config.mock_with :rspec | |||
|
|
|||
| config.use_transactional_fixtures = true | |||
| config.use_transactional_fixtures = false | |||
There was a problem hiding this comment.
@anaumov is this to fix the callbacks not working due to it waiting for a commit that never happens? I'd be worried that this will break specs for end-users as well.
There was a problem hiding this comment.
Yes, if it sets to use_transactional_fixtures = true specs running with rails ~> 4.0 didn't commit transaction. No emails will be sent and two specs are failed. use_transactional_fixtures = false works fine (the build is green). I read that it could slow down specs, but they are still pretty fast.
|
Ok, looks good to me. Might fix #134? |
|
Cool! thanks @Ch4s3 |
Use
after_commit on: :createinstead ofafter_createcallback for sending activation email. It's necessary because ofafter_createis located in a transaction. There is no guarantee that user will exist when email will start building.More details http://codebeerstartups.com/2012/11/use-after_commit-instead-of-active-record-callbacks-to-avoid-unexpected-errors/
Also added a patch to mongo
define callback. Let me know if you need more info about this issue.