This fixes a race condition between the web process creating the user record and a worker process picking up the job to use the user record to add them to SendGrid.
The race shows itself in an error from Sidekiq:
ActiveJob::DeserializationError: Error while trying to deserialize
arguments: Couldn't find User with 'id'=1234
Description of changes
An alternative to #402 to fix the same AddToSendGridJob errors seen in production.
The welcome_user() method is the thing scheduling a job that is trying to find a user. Making the ActiveRecord lifecycle callbacks responsible for business processes can be painful to test and maintain. This change removes the welcoming of a user from the User model's callbacks and makes the controller action that creates users responsible for welcoming them. Asserting that the job is enqueued is then possible in the User model test without the double-enqueue that would occur before: once when the test user instance was created (saved and committed) and again when calling the welcome_user() method to test it.
Summary of changes:
User's after_create callback to welcome_user is removed.
User's tests are updated to only confirm that the job is enqueued when welcome_user() is called.
UsersController now calls welcome_user after a user is successfully saved to the database which avoids the race condition between save and job execution.
UsersController tests are updated to:
assert that User's welcome_user is called during create controller action
share a common set of valid user_params
dry the tests slightly by using the valid_user_params with a removal to test invalid user params
BONUS
set test environment Rails output to WARN so that things like deprecation notices don't get lost in the noise of database logging
per a minitest dep notice: update nil checks to use assert_nil thingie instead of assert_equal nil, thingie
Suggested Follow-on Work
move the calls to UserMailer.welcome(user).deliver into the User.welcome_user() method
This email is sent from two places right now: api/v1/users_controller#create and models/user#fetch_social_user_and_redirect_path called by api/v1/social_users_controller#create. The English in the mailer method implies that this email is part of welcoming a new user. It might be easier to maintain that welcoming process in one place and use it consistently for new users whether they are signing up with email+password or through social media.
send mail in the background with UserMailer.welcome(user).deliver_later
ActionMailer and ActiveJob are pals. Mailer's deliver_later turns the sending of an email into a job so that the work of rendering and sending (and any errors that may occur) is Somebody Else's Problem from the perspective of a web request/response cycle. This will speed up the response time for sign-up and prevent email rendering or send errors from causing the web response to error.
This would require updating the Sidekiq execution or config file—which appears to live outside this repo?—to include mailers in its list of queues to process.
rename User.welcome_user() to User.onboard()
This method starts to encapsulate the business process of onboarding a new user which includes welcoming but also other things (like adding to SendGrid, which doesn't appear to be a welcome)
Issue Resolved
This fixes a race condition between the web process creating the user record and a worker process picking up the job to use the user record to add them to SendGrid.
The race shows itself in an error from Sidekiq:
Description of changes
An alternative to #402 to fix the same AddToSendGridJob errors seen in production.
The
welcome_user()method is the thing scheduling a job that is trying to find a user. Making the ActiveRecord lifecycle callbacks responsible for business processes can be painful to test and maintain. This change removes the welcoming of a user from the User model's callbacks and makes the controller action that creates users responsible for welcoming them. Asserting that the job is enqueued is then possible in the User model test without the double-enqueue that would occur before: once when the test user instance was created (saved and committed) and again when calling thewelcome_user()method to test it.Summary of changes:
assert_nil thingieinstead ofassert_equal nil, thingieSuggested Follow-on Work
UserMailer.welcome(user).deliverinto theUser.welcome_user()methodThis email is sent from two places right now:
api/v1/users_controller#createandmodels/user#fetch_social_user_and_redirect_pathcalled byapi/v1/social_users_controller#create. The English in the mailer method implies that this email is part of welcoming a new user. It might be easier to maintain that welcoming process in one place and use it consistently for new users whether they are signing up with email+password or through social media.UserMailer.welcome(user).deliver_laterActionMailer and ActiveJob are pals. Mailer's
deliver_laterturns the sending of an email into a job so that the work of rendering and sending (and any errors that may occur) is Somebody Else's Problem from the perspective of a web request/response cycle. This will speed up the response time for sign-up and prevent email rendering or send errors from causing the web response to error.This would require updating the Sidekiq execution or config file—which appears to live outside this repo?—to include
mailersin its list of queues to process.User.welcome_user()toUser.onboard()This method starts to encapsulate the business process of onboarding a new user which includes welcoming but also other things (like adding to SendGrid, which doesn't appear to be a welcome)