Skip to content

Send Devise emails triggered from the 'Email' model asynchronously

Manoj M J requested to merge async-secondary-email-devise-mailers into master

What does this MR do?

This change sends Devise emails triggered from the Email model asynchronously.

This change has been made because of the error observed @ #217831 (closed)

Background

Currently, User is the primary model that is backed by Devise modules like lockable, confirmable etc.

Devise triggers emails on certain events (eg: Confirm your email email when your email is updated), and by default these emails are triggered along with the request.

To trigger these emails in async fashion, we currently use this method, which pushes the ActionMailer job via ActiveJob to Sidekiq, thus triggering the emails outside of the web request.

Problem

We store secondary emails in the Email model and this model also contains the Devise module confirmable included in it.

Which means that an email will be triggered when a new secondary email is added for a user.

However, this model does not contain the overridden method send_devise_notification, to send these emails in asynchronous fashion.

Problem

The lack of the overridden method makes Devise trigger these emails synchronously and this can be problematic because:

  1. The web-request takes longer to complete - user has to wait a longer period for the request to be successful.
  2. Any problem that arises during the sending of these email causes a 500 error to the user. This has been observed here, hence the reason to make this change.
  3. Had the email been async, Sidekiq would have put the failed email in retry queue and re-tried sending later. However, now since the email is part of the request-response cycle, we lose the ability to retry a failed email.

Changes:

  1. The current send_devise_notification has been extracted to a separate module so that it can be reused across models.
  2. This new module has been added in Email model, making the Devise emails triggered from this model asynchronous.
  3. Added specs for testing Devise emails are being enqueued in the mailer queue from both the User and Email model.
  4. Changed some specs that relied on the behaviour of confirmation emails being delivered syncronously.

Screenshots

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Manoj M J

Merge request reports