diff --git a/common/djangoapps/student/management/commands/create_user.py b/common/djangoapps/student/management/commands/create_user.py
index d375dedc54a39e6167a3e53e7630e7094f79f8e0..cacc5dea8ea99b691d2176a0a61f127c9c394483 100644
--- a/common/djangoapps/student/management/commands/create_user.py
+++ b/common/djangoapps/student/management/commands/create_user.py
@@ -1,7 +1,7 @@
from optparse import make_option
from django.core.management.base import BaseCommand
-from student.models import CourseEnrollment, Registration
-from student.views import _do_create_account
+from student.models import CourseEnrollment, Registration, create_comments_service_user
+from student.views import _do_create_account, AccountValidationError
from django.contrib.auth.models import User
from track.management.tracked_command import TrackedCommand
@@ -74,17 +74,16 @@ class Command(TrackedCommand):
'honor_code': u'true',
'terms_of_service': u'true',
}
- create_account = _do_create_account(post_data)
- if isinstance(create_account, tuple):
- user = create_account[0]
+ try:
+ user, profile, reg = _do_create_account(post_data)
if options['staff']:
user.is_staff = True
user.save()
- reg = Registration.objects.get(user=user)
reg.activate()
reg.save()
- else:
- print create_account
+ create_comments_service_user(user)
+ except AccountValidationError as e:
+ print e.message
user = User.objects.get(email=options['email'])
if options['course']:
CourseEnrollment.enroll(user, options['course'], mode=options['mode'])
diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py
index c2fcdea35472ce94a8318543068ec58abfde0391..8f2952fdfe4c7b65eceed41e6a9958f2cc21f6c7 100644
--- a/common/djangoapps/student/models.py
+++ b/common/djangoapps/student/models.py
@@ -831,18 +831,18 @@ def add_user_to_default_group(user, group):
utg.save()
-@receiver(post_save, sender=User)
-def update_user_information(sender, instance, created, **kwargs):
+def create_comments_service_user(user):
if not settings.FEATURES['ENABLE_DISCUSSION_SERVICE']:
# Don't try--it won't work, and it will fill the logs with lots of errors
return
try:
- cc_user = cc.User.from_django_user(instance)
+ cc_user = cc.User.from_django_user(user)
cc_user.save()
except Exception as e:
log = logging.getLogger("edx.discussion")
- log.error(unicode(e))
- log.error("update user info to discussion failed for user with id: " + str(instance.id))
+ log.error(
+ "Could not create comments service user with id {}".format(user.id),
+ exc_info=True)
# Define login and logout handlers here in the models file, instead of the views file,
# so that they are more likely to be loaded when a Studio user brings up the Studio admin
diff --git a/common/djangoapps/student/tests/test_create_account.py b/common/djangoapps/student/tests/test_create_account.py
index 1f5f4943063797f09c3c4de6a2d8ef78887d670a..bc9503d1d05c1c4818a4e2dc177d8712d808b4a2 100644
--- a/common/djangoapps/student/tests/test_create_account.py
+++ b/common/djangoapps/student/tests/test_create_account.py
@@ -3,13 +3,17 @@
import ddt
from django.contrib.auth.models import User
from django.core.urlresolvers import reverse
-from django.test import TestCase
+from django.db.transaction import rollback
+from django.test import TestCase, TransactionTestCase
from django.test.utils import override_settings
import mock
from user_api.models import UserPreference
from lang_pref import LANGUAGE_KEY
+import student
+
+TEST_CS_URL = 'https://comments.service.test:123/'
@ddt.ddt
class TestCreateAccount(TestCase):
@@ -41,3 +45,43 @@ class TestCreateAccount(TestCase):
self.assertEqual(response.status_code, 200)
user = User.objects.get(username=self.username)
self.assertEqual(UserPreference.get_preference(user, LANGUAGE_KEY), lang)
+
+
+@mock.patch.dict("student.models.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
+@mock.patch("lms.lib.comment_client.User.base_url", TEST_CS_URL)
+@mock.patch("lms.lib.comment_client.utils.requests.request", return_value=mock.Mock(status_code=200, text='{}'))
+class TestCreateCommentsServiceUser(TransactionTestCase):
+
+ def setUp(self):
+ self.username = "test_user"
+ self.url = reverse("create_account")
+ self.params = {
+ "username": self.username,
+ "email": "test@example.org",
+ "password": "testpass",
+ "name": "Test User",
+ "honor_code": "true",
+ "terms_of_service": "true",
+ }
+
+ def test_cs_user_created(self, request):
+ "If user account creation succeeds, we should create a comments service user"
+ response = self.client.post(self.url, self.params)
+ self.assertEqual(response.status_code, 200)
+ self.assertTrue(request.called)
+ args, kwargs = request.call_args
+ self.assertEqual(args[0], 'put')
+ self.assertTrue(args[1].startswith(TEST_CS_URL))
+ self.assertEqual(kwargs['data']['username'], self.params['username'])
+
+ @mock.patch("student.models.Registration.register", side_effect=Exception)
+ def test_cs_user_not_created(self, register, request):
+ "If user account creation fails, we should not create a comments service user"
+ try:
+ response = self.client.post(self.url, self.params)
+ except:
+ pass
+ with self.assertRaises(User.DoesNotExist):
+ User.objects.get(username=self.username)
+ self.assertTrue(register.called)
+ self.assertFalse(request.called)
diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py
index 61d2aa22d6cc5d9179d3166d32a21c2033d60d92..54f3e2bfb3dbc013a7fda82e982f0dc60f6257d3 100644
--- a/common/djangoapps/student/views.py
+++ b/common/djangoapps/student/views.py
@@ -38,7 +38,8 @@ from course_modes.models import CourseMode
from student.models import (
Registration, UserProfile, PendingNameChange,
PendingEmailChange, CourseEnrollment, unique_id_for_user,
- CourseEnrollmentAllowed, UserStanding, LoginFailures
+ CourseEnrollmentAllowed, UserStanding, LoginFailures,
+ create_comments_service_user
)
from student.forms import PasswordResetFormNoActive
from student.firebase_token_generator import create_token
@@ -951,6 +952,11 @@ def change_setting(request):
})
+class AccountValidationError(Exception):
+ def __init__(self, message, field):
+ super(AccountValidationError, self).__init__(message)
+ self.field = field
+
def _do_create_account(post_vars):
"""
Given cleaned post variables, create the User and UserProfile objects, as well as the
@@ -970,19 +976,19 @@ def _do_create_account(post_vars):
try:
user.save()
except IntegrityError:
- js = {'success': False}
# Figure out the cause of the integrity error
if len(User.objects.filter(username=post_vars['username'])) > 0:
- js['value'] = _("An account with the Public Username '{username}' already exists.").format(username=post_vars['username'])
- js['field'] = 'username'
- return JsonResponse(js, status=400)
-
- if len(User.objects.filter(email=post_vars['email'])) > 0:
- js['value'] = _("An account with the Email '{email}' already exists.").format(email=post_vars['email'])
- js['field'] = 'email'
- return JsonResponse(js, status=400)
-
- raise
+ raise AccountValidationError(
+ _("An account with the Public Username '{username}' already exists.").format(username=post_vars['username']),
+ field="username"
+ )
+ elif len(User.objects.filter(email=post_vars['email'])) > 0:
+ raise AccountValidationError(
+ _("An account with the Email '{email}' already exists.").format(email=post_vars['email']),
+ field="email"
+ )
+ else:
+ raise
registration.register(user)
@@ -1005,6 +1011,7 @@ def _do_create_account(post_vars):
profile.save()
except Exception:
log.exception("UserProfile creation failed for user {id}.".format(id=user.id))
+ raise
UserPreference.set_preference(user, LANGUAGE_KEY, get_language())
@@ -1150,11 +1157,17 @@ def create_account(request, post_override=None):
return JsonResponse(js, status=400)
# Ok, looks like everything is legit. Create the account.
- ret = _do_create_account(post_vars)
- if isinstance(ret, HttpResponse): # if there was an error then return that
- return ret
+ try:
+ with transaction.commit_on_success():
+ ret = _do_create_account(post_vars)
+ except AccountValidationError as e:
+ return JsonResponse({'success': False, 'value': e.message, 'field': e.field}, status=400)
+
(user, profile, registration) = ret
+ dog_stats_api.increment("common.student.account_created")
+ create_comments_service_user(user)
+
context = {
'name': post_vars['name'],
'key': registration.activation_key,
@@ -1213,8 +1226,6 @@ def create_account(request, post_override=None):
login_user.save()
AUDIT_LOG.info(u"Login activated on extauth account - {0} ({1})".format(login_user.username, login_user.email))
- dog_stats_api.increment("common.student.account_created")
-
response = JsonResponse({
'success': True,
'redirect_url': try_change_enrollment(request),
@@ -1283,20 +1294,16 @@ def auto_auth(request):
# Attempt to create the account.
# If successful, this will return a tuple containing
- # the new user object; otherwise it will return an error
- # message.
- result = _do_create_account(post_data)
-
- if isinstance(result, tuple):
- user = result[0]
-
- # If we did not create a new account, the user might already
- # exist. Attempt to retrieve it.
- else:
+ # the new user object.
+ try:
+ user, profile, reg = _do_create_account(post_data)
+ except AccountValidationError:
+ # Attempt to retrieve the existing user.
user = User.objects.get(username=username)
user.email = email
user.set_password(password)
user.save()
+ reg = Registration.objects.get(user=user)
# Set the user's global staff bit
if is_staff is not None:
@@ -1304,7 +1311,6 @@ def auto_auth(request):
user.save()
# Activate the user
- reg = Registration.objects.get(user=user)
reg.activate()
reg.save()
@@ -1321,6 +1327,8 @@ def auto_auth(request):
user = authenticate(username=username, password=password)
login(request, user)
+ create_comments_service_user(user)
+
# Provide the user with a valid CSRF token
# then return a 200 response
success_msg = u"Logged in user {0} ({1}) with password {2} and user_id {3}".format(
diff --git a/lms/lib/comment_client/user.py b/lms/lib/comment_client/user.py
index 014fe0a810ea38014a2dbca029c2e96b7b64d2a5..77e8b0d01169b606011b8788cc15f9869cfd6d93 100644
--- a/lms/lib/comment_client/user.py
+++ b/lms/lib/comment_client/user.py
@@ -80,7 +80,16 @@ class User(models.Model):
retrieve_params = self.default_retrieve_params
if self.attributes.get('course_id'):
retrieve_params['course_id'] = self.course_id
- response = perform_request('get', url, retrieve_params)
+ try:
+ response = perform_request('get', url, retrieve_params)
+ except CommentClientRequestError as e:
+ if e.status_code == 404:
+ # attempt to gracefully recover from a previous failure
+ # to sync this user to the comments service.
+ self.save()
+ response = perform_request('get', url, retrieve_params)
+ else:
+ raise
self.update_attributes(**response)