throwing exception when user has conflict with username or email

common/djangoapps/student/models.py

@@ -2261,20 +2261,22 @@ def strip_if_string(value):
 
 def get_user_by_username_or_email(username_or_email):
     """
-    Return a User object, looking up by email if username_or_email contains a
-    '@', otherwise by username.
+    Return a User object by looking up a user against username_or_email.
 
     Raises:
         User.DoesNotExist if no user object can be found, the user was
         retired, or the user is in the process of being retired.
+
+        MultipleObjectsReturned if one user has same email as username of
+        second user
+
+        MultipleObjectsReturned if more than one user has same email or
+        username
     """
     username_or_email = strip_if_string(username_or_email)
-    user = None
-    if '@' in username_or_email:
-        user = User.objects.filter(email=username_or_email).first()
-
-    if user is None:
-        user = User.objects.get(username=username_or_email)
+    # there should be one user with either username or email equal to username_or_email
+    user = User.objects.get(Q(email=username_or_email) | Q(username=username_or_email))
+    if user is not None and user.username == username_or_email:
         UserRetirementRequest = apps.get_model('user_api', 'UserRetirementRequest')
         if UserRetirementRequest.has_user_requested_retirement(user):
             raise User.DoesNotExist

lms/djangoapps/instructor/tests/test_tools.py

@@ -9,6 +9,7 @@ import unittest
 import mock
 import six
 from django.contrib.auth.models import User
+from django.core.exceptions import MultipleObjectsReturned
 from django.test import TestCase
 from django.test.utils import override_settings
 from pytz import UTC
@@ -365,24 +366,38 @@ class TestStudentFromIdentifier(TestCase):
     """
     Test get_student_from_identifier()
     """
-    def setUp(self):
-        """
-        Fixtures
-        """
-        super(TestStudentFromIdentifier, self).setUp()
-        self.students = [
-            UserFactory.create(username='foo@touchstone'),  # a student with character `@` in user name
-            UserFactory.create()
-        ]
+    @classmethod
+    def setUpClass(cls):
+        super(TestStudentFromIdentifier, cls).setUpClass()
+        cls.valid_student = UserFactory.create(username='baz@touchstone')
+        cls.duplicate_email = UserFactory.create(email='foo@touchstone.com')
+        cls.duplicate_user_name = UserFactory.create(username='foo@touchstone.com')
 
     def test_valid_student_id(self):
-        for student in self.students:
-            assert student == tools.get_student_from_identifier(student.username)
+        """Test with valid username"""
+        assert self.valid_student == tools.get_student_from_identifier(self.valid_student.username)
 
     def test_valid_student_email(self):
-        for student in self.students:
-            assert student == tools.get_student_from_identifier(student.email)
+        """Test with valid email"""
+        assert self.valid_student == tools.get_student_from_identifier(self.valid_student.email)
+
+    def test_student_username_has_conflict_with_others_email(self):
+        """
+        An edge case where there is a user A with username example: foo@touchstone.com and
+        there is user B with email example: foo@touchstone.com
+        """
+        with self.assertRaises(MultipleObjectsReturned):
+            tools.get_student_from_identifier(self.duplicate_user_name.username)
+
+    def test_student_email_has_conflict_with_others_username(self):
+        """
+        An edge case where there is a user A with email example: foo@touchstone.com and
+        there is user B with username example: foo@touchstone.com
+        """
+        with self.assertRaises(MultipleObjectsReturned):
+            tools.get_student_from_identifier(self.duplicate_email.email)
 
     def test_invalid_student_id(self):
+        """Test with invalid identifier"""
         with self.assertRaises(User.DoesNotExist):
             assert tools.get_student_from_identifier("invalid")

lms/djangoapps/instructor/views/api.py

@@ -18,7 +18,12 @@ import time
 import unicodecsv
 from django.conf import settings
 from django.contrib.auth.models import User
-from django.core.exceptions import ObjectDoesNotExist, PermissionDenied, ValidationError
+from django.core.exceptions import (
+    MultipleObjectsReturned,
+    ObjectDoesNotExist,
+    PermissionDenied,
+    ValidationError
+)
 from django.core.mail.message import EmailMessage
 from django.core.urlresolvers import reverse
 from django.core.validators import validate_email
@@ -153,6 +158,8 @@ def common_exceptions_400(func):
             return func(request, *args, **kwargs)
         except User.DoesNotExist:
             message = _('User does not exist.')
+        except MultipleObjectsReturned:
+            message = _('Found a conflict with given identifier. Please try an alternative identifier')
         except (AlreadyRunningError, QueueConnectionError) as err:
             message = unicode(err)