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.

Sign up for GitHub

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

send activation email after the commit in transactional databases #130

Merged
merged 5 commits into from
Jul 9, 2018
Merged

send activation email after the commit in transactional databases #130

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fa2bf9f
send activation email after the commit in transactional databases
anaumov Jun 13, 2018
0985df9
extract callback_name to separate method
anaumov Jun 19, 2018
658990f
fix typo in parameter
anaumov Jun 28, 2018
bdec3d2
disable transactional fixtures to be able to test after_commit
anaumov Jun 29, 2018
dd94983
Raise on failure when change_password!
dapi Jul 6, 2018
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
2 changes: 1 addition & 1 deletion lib/sorcery/adapters/active_record_adapter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def define_field(name, type, options = {})
end

def define_callback(time, event, method_name, options = {})
@klass.send "#{time}_#{event}", method_name, options.slice(:if)
@klass.send "#{time}_#{event}", method_name, options.slice(:if, :on)
end

def find_by_oauth_credentials(provider, uid)
Expand Down
10 changes: 9 additions & 1 deletion lib/sorcery/adapters/mongoid_adapter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
@klass.send callback_name(time, event, options), method_name, options.slice(:if)
end

def callback_name(time, event, options)
if event == :commit
options[:on] == :create ? "#{time}_create" : "#{time}_save"
else
"#{time}_#{event}"
end
end

def credential_regex(credential)
Expand Down
2 changes: 1 addition & 1 deletion lib/sorcery/model/submodules/user_activation.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Copy link
Member

Choose a reason for hiding this comment

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

Interesting, I honestly didn't realize rails had a callback for specifically after the DB commit.

👍

end

base.sorcery_config.after_config << :validate_mailer_defined
Expand Down
2 changes: 1 addition & 1 deletion spec/spec_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

@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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.


config.before(:suite) { setup_orm }
config.after(:suite) { teardown_orm }
Expand Down