[ENG-4903] Fixes issue with email confirmation links failing due to d…

…atabase congestion (#10662)

* fix: move send_confirmation_email to post-commit task

* Move send_confirm_email logic to framework/auth/views

* Fix argument passing in enqueue_postcommit_task call in send_confirm_email

* Fix argument passing in enqueue_postcommit_task call in send_confirm_email

* Fix argument passing in enqueue_postcommit_task call in send_confirm_email

* Refactor: Separate send_confirm_email task for post-commit execution

* Refactor: Separate send_confirm_email task for post-commit execution

* Refactor: Separate send_confirm_email task for post-commit execution

---------

Co-authored-by: Uditi Mehta <uditimehta@COSs-MacBook-Pro.local>

api/users/serializers.py

@@ -32,7 +32,7 @@
 from website.profile.views import update_osf_help_mails_subscription, update_mailchimp_subscription
 from api.nodes.serializers import NodeSerializer, RegionRelationshipField
 from api.base.schemas.utils import validate_user_json, from_json
-from framework.auth.views import send_confirm_email
+from framework.auth.views import send_confirm_email_async
 from api.base.versioning import get_kebab_snake_case_field
 
 
@@ -596,7 +596,7 @@ def create(self, validated_data):
  token = user.add_unconfirmed_email(address)
  user.save()
  if CONFIRM_REGISTRATIONS_BY_EMAIL:
- send_confirm_email(user, email=address)
+ send_confirm_email_async(user, email=address)
  user.email_last_sent = timezone.now()
  user.save()
  except ValidationError as e:

api/users/views.py

@@ -60,7 +60,7 @@
 from django.http import JsonResponse
 from django.utils import timezone
 from framework.auth.core import get_user
-from framework.auth.views import send_confirm_email
+from framework.auth.views import send_confirm_email_async
 from framework.auth.oauth_scopes import CoreScopes, normalize_scopes
 from framework.auth.exceptions import ChangePasswordError
 from framework.utils import throttle_period_expired
@@ -900,7 +900,7 @@ def get_object(self):
  if self.request.method == 'GET' and is_truthy(self.request.query_params.get('resend_confirmation')):
  if not confirmed and settings.CONFIRM_REGISTRATIONS_BY_EMAIL:
  if throttle_period_expired(user.email_last_sent, settings.SEND_EMAIL_THROTTLE):
- send_confirm_email(user, email=address, renew=True)
+ send_confirm_email_async(user, email=address, renew=True)
  user.email_last_sent = timezone.now()
  user.save()
 

api_tests/institutions/views/test_institution_auth.py

@@ -12,7 +12,7 @@
 
 from framework.auth import signals, Auth
 from framework.auth.core import get_user
-from framework.auth.views import send_confirm_email
+from framework.auth.views import send_confirm_email_async
 
 from osf.models import OSFUser, InstitutionAffiliation, InstitutionStorageRegion
 from osf.models.institution import SsoFilterCriteriaAction
@@ -454,7 +454,7 @@ def test_user_external_unconfirmed(self, app, institution, url_auth_institution)
  assert user.external_identity
 
  # Send confirm email in order to add new email verifications
- send_confirm_email(
+ send_confirm_email_async(
  user,
  user.username,
  external_id_provider=external_id_provider,

api_tests/users/views/test_user_settings.py

@@ -216,7 +216,7 @@ def test_unconfirmed_email_included(self, app, url, payload, user_one, unconfirm
  assert res.status_code == 200
  assert unconfirmed_address in [result['attributes']['email_address'] for result in res.json['data']]
 
- @mock.patch('api.users.serializers.send_confirm_email')
+ @mock.patch('api.users.serializers.send_confirm_email_async')
  def test_create_new_email_current_user(self, mock_send_confirm_mail, user_one, user_two, app, url, payload):
  new_email = 'hhh@wwe.test'
  payload['data']['attributes']['email_address'] = new_email
@@ -229,7 +229,7 @@ def test_create_new_email_current_user(self, mock_send_confirm_mail, user_one, u
  assert new_email in user_one.unconfirmed_emails
  assert mock_send_confirm_mail.called
 
- @mock.patch('api.users.serializers.send_confirm_email')
+ @mock.patch('api.users.serializers.send_confirm_email_async')
  def test_create_new_email_not_current_user(self, mock_send_confirm_mail, app, url, payload, user_one, user_two):
  new_email = 'HHH@wwe.test'
  payload['data']['attributes']['email_address'] = new_email
@@ -239,7 +239,7 @@ def test_create_new_email_not_current_user(self, mock_send_confirm_mail, app, ur
  assert new_email not in user_one.unconfirmed_emails
  assert not mock_send_confirm_mail.called
 
- @mock.patch('api.users.serializers.send_confirm_email')
+ @mock.patch('api.users.serializers.send_confirm_email_async')
  def test_create_email_already_exists(self, mock_send_confirm_mail, app, url, payload, user_one):
  new_email = 'hello@email.test'
  Email.objects.create(address=new_email, user=user_one)
@@ -578,28 +578,28 @@ def test_updating_verified_for_merge(self, app, user_one, user_two, payload):
  assert res.json['data']['attributes']['confirmed'] is True
  assert res.json['data']['attributes']['is_merge'] is False
 
- @mock.patch('api.users.views.send_confirm_email')
- def test_resend_confirmation_email(self, mock_send_confirm_email, app, user_one, unconfirmed_url, confirmed_url):
+ @mock.patch('api.users.views.send_confirm_email_async')
+ def test_resend_confirmation_email(self, mock_send_confirm_email_async, app, user_one, unconfirmed_url, confirmed_url):
  url = '{}?resend_confirmation=True'.format(unconfirmed_url)
  res = app.get(url, auth=user_one.auth)
  assert res.status_code == 202
- assert mock_send_confirm_email.called
- call_count = mock_send_confirm_email.call_count
+ assert mock_send_confirm_email_async.called
+ call_count = mock_send_confirm_email_async.call_count
 
  # make sure setting false does not send confirm email
  url = '{}?resend_confirmation=False'.format(unconfirmed_url)
  res = app.get(url, auth=user_one.auth)
  # should return 200 instead of 202 because nothing has been done
  assert res.status_code == 200
- assert mock_send_confirm_email.call_count
+ assert mock_send_confirm_email_async.call_count
 
  # make sure normal GET request does not re-send confirmation email
  res = app.get(unconfirmed_url, auth=user_one.auth)
- assert mock_send_confirm_email.call_count == call_count
+ assert mock_send_confirm_email_async.call_count == call_count
  assert res.status_code == 200
 
  # resend confirmation with confirmed email address does not send confirmation email
  url = '{}?resend_confirmation=True'.format(confirmed_url)
  res = app.get(url, auth=user_one.auth)
- assert mock_send_confirm_email.call_count == call_count
+ assert mock_send_confirm_email_async.call_count == call_count
  assert res.status_code == 200

framework/auth/views.py

@@ -37,6 +37,7 @@
 from website.util.metrics import CampaignClaimedTags, CampaignSourceTags
 from website.ember_osf_web.decorators import ember_flag_is_active
 from osf import features
+from framework.postcommit_tasks.handlers import enqueue_postcommit_task
 # from osf.models import PreprintProvider
 
 
@@ -800,7 +801,6 @@ def unconfirmed_email_add(auth=None):
  'removed_email': json_body['address']
  }, 200
 
-
 def send_confirm_email(user, email, renew=False, external_id_provider=None, external_id=None, destination=None):
  """
  Sends `user` a confirmation to the given `email`.
@@ -815,7 +815,6 @@ def send_confirm_email(user, email, renew=False, external_id_provider=None, exte
  :return:
  :raises: KeyError if user does not have a confirmation token for the given email.
  """
-
  confirmation_url = user.get_confirmation_url(
  email,
  external=True,
@@ -872,6 +871,9 @@ def send_confirm_email(user, email, renew=False, external_id_provider=None, exte
  logo=logo if logo else settings.OSF_LOGO
  )
 
+def send_confirm_email_async(user, email, renew=False, external_id_provider=None, external_id=None, destination=None):
+ enqueue_postcommit_task(send_confirm_email, (user, email, renew, external_id_provider, external_id, destination), {})
+
 
 def register_user(**kwargs):
  """
@@ -942,7 +944,7 @@ def register_user(**kwargs):
  )
 
  if settings.CONFIRM_REGISTRATIONS_BY_EMAIL:
- send_confirm_email(user, email=user.username)
+ send_confirm_email_async(user, email=user.username)
  message = language.REGISTRATION_SUCCESS.format(email=user.username)
  return {'message': message}
  else:
@@ -989,7 +991,7 @@ def resend_confirmation_post(auth):
  if user:
  if throttle_period_expired(user.email_last_sent, settings.SEND_EMAIL_THROTTLE):
  try:
- send_confirm_email(user, clean_email, renew=True)
+ send_confirm_email_async(user, clean_email, renew=True)
  except KeyError:
  # already confirmed, redirect to dashboard
  status_message = 'This email {0} has already been confirmed.'.format(clean_email)
@@ -1094,7 +1096,7 @@ def external_login_email_post():
  # 2. add unconfirmed email and send confirmation email
  user.add_unconfirmed_email(clean_email, external_identity=external_identity)
  user.save()
- send_confirm_email(
+ send_confirm_email_async(
  user,
  clean_email,
  external_id_provider=external_id_provider,
@@ -1124,7 +1126,7 @@ def external_login_email_post():
  # TODO: [#OSF-6934] update social fields, verified social fields cannot be modified
  user.save()
  # 3. send confirmation email
- send_confirm_email(
+ send_confirm_email_async(
  user,
  user.username,
  external_id_provider=external_id_provider,

tests/test_views.py

@@ -3325,7 +3325,7 @@ def test_register_email_without_accepted_tos(self, _):
  user = OSFUser.objects.get(username=email)
  assert_equal(user.accepted_terms_of_service, None)
 
- @mock.patch('framework.auth.views.send_confirm_email')
+ @mock.patch('framework.auth.views.send_confirm_email_async')
  def test_register_scrubs_username(self, _):
  url = api_url_for('register_user')
  name = "<i>Eunice</i> O' \"Cornwallis\"<script type='text/javascript' src='http://www.cornify.com/js/cornify.js'></script><script type='text/javascript'>cornify_add()</script>"
@@ -3509,8 +3509,8 @@ def test_register_after_being_invited_as_unreg_contributor(self, mock_update_sea
  assert_true(new_user.check_password(password))
  assert_equal(new_user.fullname, real_name)
 
- @mock.patch('framework.auth.views.send_confirm_email')
- def test_register_sends_user_registered_signal(self, mock_send_confirm_email):
+ @mock.patch('framework.auth.views.send_confirm_email_async')
+ def test_register_sends_user_registered_signal(self, mock_send_confirm_email_async):
  url = api_url_for('register_user')
  name, email, password = fake.name(), fake_email(), 'underpressure'
  with capture_signals() as mock_signals:
@@ -3525,7 +3525,7 @@ def test_register_sends_user_registered_signal(self, mock_send_confirm_email):
  )
  assert_equal(mock_signals.signals_sent(), set([auth.signals.user_registered,
  auth.signals.unconfirmed_user_created]))
- assert_true(mock_send_confirm_email.called)
+ assert_true(mock_send_confirm_email_async.called)
 
  @mock.patch('framework.auth.views.mails.send_mail')
  def test_resend_confirmation(self, send_mail):

website/profile/views.py

@@ -15,7 +15,7 @@
 from framework.auth.decorators import must_be_logged_in
 from framework.auth.decorators import must_be_confirmed
 from framework.auth.exceptions import ChangePasswordError
-from framework.auth.views import send_confirm_email
+from framework.auth.views import send_confirm_email_async
 from framework.auth.signals import (
  user_account_merged,
  user_account_deactivated,
@@ -84,7 +84,7 @@ def resend_confirmation(auth):
 
  # TODO: This setting is now named incorrectly.
  if settings.CONFIRM_REGISTRATIONS_BY_EMAIL:
- send_confirm_email(user, email=address)
+ send_confirm_email_async(user, email=address)
  user.email_last_sent = timezone.now()
 
  user.save()
@@ -167,7 +167,7 @@ def update_user(auth):
  if not throttle_period_expired(user.email_last_sent, settings.SEND_EMAIL_THROTTLE):
  raise HTTPError(http_status.HTTP_400_BAD_REQUEST,
  data={'message_long': 'Too many requests. Please wait a while before adding an email to your account.'})
- send_confirm_email(user, email=address)
+ send_confirm_email_async(user, email=address)
 
  ############
  # Username #