Merge pull request #1345 from edx/usman/lms1222-email-confirm-transaction-errors

Commit/rollback transaction after rendering response to prevent TransactionManagementErrors due to db access in templates.

common/djangoapps/student/tests/test_email.py

@@ -25,6 +25,9 @@ def mock_render_to_string(template_name, context):
 
 def mock_render_to_response(template_name, context):
     """Return an HttpResponse with content that encodes template_name and context"""
+    # View confirm_email_change uses @transaction.commit_manually.
+    # This simulates any db access in the templates.
+    UserProfile.objects.exists()
     return HttpResponse(mock_render_to_string(template_name, context))
 
 

common/djangoapps/student/views.py

@@ -1315,8 +1315,9 @@ def confirm_email_change(request, key):
         try:
             pec = PendingEmailChange.objects.get(activation_key=key)
         except PendingEmailChange.DoesNotExist:
+            response = render_to_response("invalid_email_key.html", {})
             transaction.rollback()
-            return render_to_response("invalid_email_key.html", {})
+            return response
 
         user = pec.user
         address_context = {
@@ -1325,8 +1326,9 @@ def confirm_email_change(request, key):
         }
 
         if len(User.objects.filter(email=pec.new_email)) != 0:
+            response = render_to_response("email_exists.html", {})
             transaction.rollback()
-            return render_to_response("email_exists.html", {})
+            return response
 
         subject = render_to_string('emails/email_change_subject.txt', address_context)
         subject = ''.join(subject.splitlines())
@@ -1342,9 +1344,10 @@ def confirm_email_change(request, key):
         try:
             user.email_user(subject, message, settings.DEFAULT_FROM_EMAIL)
         except Exception:
-            transaction.rollback()
             log.warning('Unable to send confirmation email to old address', exc_info=True)
-            return render_to_response("email_change_failed.html", {'email': user.email})
+            response = render_to_response("email_change_failed.html", {'email': user.email})
+            transaction.rollback()
+            return response
 
         user.email = pec.new_email
         user.save()
@@ -1353,12 +1356,14 @@ def confirm_email_change(request, key):
         try:
             user.email_user(subject, message, settings.DEFAULT_FROM_EMAIL)
         except Exception:
-            transaction.rollback()
             log.warning('Unable to send confirmation email to new address', exc_info=True)
-            return render_to_response("email_change_failed.html", {'email': pec.new_email})
+            response = render_to_response("email_change_failed.html", {'email': pec.new_email})
+            transaction.rollback()
+            return response
 
+        response = render_to_response("email_change_successful.html", address_context)
         transaction.commit()
-        return render_to_response("email_change_successful.html", address_context)
+        return response
     except Exception:
         # If we get an unexpected exception, be sure to rollback the transaction
         transaction.rollback()