Send confirmation emails immediately

Using deferred methods on the user model breaks delayed jobs when the
user is deleted while the job still exists. We could create a proper job
referencing a user id for sending these emails instead. But since the
user has to wait for the confirmation email anyway, we can send it
within the current request. This should be revised if performance
becomes an issue.

Sending the email directly also has the advantage that we can tell the
user if emailing failed. See the following commits.

This change impacts a bunch of specs as we now need a working email
setup to create unconfirmed users. This commit introduces a custom
matcher to unify testing for confirmation emails.
This commit is contained in:
Maikel Linke
2018-09-14 15:16:09 +10:00
parent 51f9a0afa1
commit 600c8fcd4c
11 changed files with 48 additions and 18 deletions

View File

@@ -24,8 +24,6 @@ Spree.user_class.class_eval do
# We use the same options as Spree and add :confirmable
devise :confirmable, reconfirmable: true
handle_asynchronously :send_confirmation_instructions
handle_asynchronously :send_on_create_confirmation_instructions
# TODO: Later versions of devise have a dedicated after_confirmation callback, so use that
after_update :welcome_after_confirm, if: lambda { confirmation_token_changed? && confirmation_token.nil? }

View File

@@ -25,13 +25,14 @@ module Admin
context "signing up a new user" do
before do
create(:mail_method)
controller.stub spree_current_user: admin
end
it "creates a new user, sends an invitation email, and returns the user id" do
expect do
spree_post :create, {email: 'un.registered@email.com', enterprise_id: enterprise.id}
end.to enqueue_job Delayed::PerformableMethod
end.to send_confirmation_instructions
new_user = Spree::User.find_by_email('un.registered@email.com')
@@ -45,6 +46,7 @@ module Admin
describe "with enterprise permissions" do
context "as user with proper enterprise permissions" do
before do
create(:mail_method)
controller.stub spree_current_user: enterprise_owner
end

View File

@@ -57,6 +57,8 @@ describe UserConfirmationsController, type: :controller do
end
context "requesting confirmation instructions to be resent" do
before { create(:mail_method) }
it "redirects the user to login" do
spree_post :create, { spree_user: { email: unconfirmed_user.email } }
expect(response).to redirect_to login_path
@@ -66,8 +68,7 @@ describe UserConfirmationsController, type: :controller do
it "sends the confirmation email" do
expect do
spree_post :create, { spree_user: { email: unconfirmed_user.email } }
end.to enqueue_job Delayed::PerformableMethod
expect(Delayed::Job.last.payload_object.method_name).to eq(:send_confirmation_instructions_without_delay)
end.to send_confirmation_instructions
end
end
end

View File

@@ -513,6 +513,16 @@ FactoryBot.modify do
confirmation_sent_at '1970-01-01 00:00:00'
confirmed_at '1970-01-01 00:00:01'
before(:create) do |user, evaluator|
if evaluator.confirmation_sent_at
if evaluator.confirmed_at
user.skip_confirmation!
else
user.skip_confirmation_notification!
end
end
end
after(:create) do |user|
user.spree_roles.clear # Remove admin role
end

View File

@@ -137,6 +137,7 @@ feature %q{
end
it "can invite unregistered users to be managers" do
create(:mail_method)
find('a.button.help-modal').click
expect(page).to have_css '#invite-manager-modal'

View File

@@ -4,7 +4,10 @@ feature "Managing users" do
include AuthenticationWorkflow
context "as super-admin" do
before { quick_login_as_admin }
before do
create(:mail_method)
quick_login_as_admin
end
describe "creating a user" do
it "shows no confirmation message to start with" do
@@ -31,12 +34,11 @@ feature "Managing users" do
it "displays success" do
visit spree.edit_admin_user_path user
# The `a` element doesn't have an href, so we can't use click_link.
find("a", text: "Resend").click
expect(page).to have_text "Resend done"
# And it's successful. (testing it here for reduced test time)
expect(Delayed::Job.last.payload_object.method_name).to eq :send_confirmation_instructions_without_delay
expect do
# The `a` element doesn't have an href, so we can't use click_link.
find("a", text: "Resend").click
expect(page).to have_text "Resend done"
end.to send_confirmation_instructions
end
end
end

View File

@@ -7,6 +7,7 @@ feature "Account Settings", js: true do
let(:user) { create(:user, email: 'old@email.com') }
before do
create(:mail_method)
quick_login_as user
end

View File

@@ -75,6 +75,7 @@ feature "Authentication", js: true, retry: 3 do
end
scenario "Signing up successfully" do
create(:mail_method)
fill_in "Email", with: "test@foo.com"
fill_in "Choose a password", with: "test12345"
fill_in "Confirm password", with: "test12345"
@@ -82,9 +83,7 @@ feature "Authentication", js: true, retry: 3 do
expect do
click_signup_button
expect(page).to have_content I18n.t('devise.user_registrations.spree_user.signed_up_but_unconfirmed')
end.to enqueue_job Delayed::PerformableMethod
expect(Delayed::Job.last.payload_object.method_name).to eq(:send_on_create_confirmation_instructions_without_delay)
end.to send_confirmation_instructions
end
end

View File

@@ -8,6 +8,7 @@ feature "Confirm invitation as manager" do
let(:user) { Spree::User.create(email: email, unconfirmed_email: email, password: "secret") }
before do
create(:mail_method)
user.reset_password_token = Devise.friendly_token
user.reset_password_sent_at = Time.now.utc
user.save!

View File

@@ -72,10 +72,11 @@ describe Spree.user_class do
context "#create" do
it "should send a confirmation email" do
create(:mail_method)
expect do
create(:user, confirmed_at: nil)
end.to enqueue_job Delayed::PerformableMethod
expect(Delayed::Job.last.payload_object.method_name).to eq(:send_on_create_confirmation_instructions_without_delay)
create(:user, confirmation_sent_at: nil, confirmed_at: nil)
end.to send_confirmation_instructions
end
context "with the the same email as existing customers" do
@@ -96,6 +97,8 @@ describe Spree.user_class do
context "confirming email" do
it "should send a welcome email" do
create(:mail_method)
expect do
create(:user, confirmed_at: nil).confirm!
end.to enqueue_job ConfirmSignupJob

View File

@@ -0,0 +1,12 @@
RSpec::Matchers.define :send_confirmation_instructions do
match do |event_proc|
expect(&event_proc).to change { ActionMailer::Base.deliveries.count }.by 1
message = ActionMailer::Base.deliveries.last
expect(message.subject).to eq "Please confirm your OFN account"
end
def supports_block_expectations?
true
end
end