Repository: carpentries/amy
Issue: Bulk add persons error

Bulk add persons is throwing a Server Error (500).

On the preview screen it looks fine but it returns an error when you click "Verify" or "Confirm and Save" 

Changed files:
amy/workshops/util.py, amy/workshops/tests/test_bulk_add.py

diff --git a/amy/workshops/tests/test_bulk_add.py b/amy/workshops/tests/test_bulk_add.py
index 34258dd6..d7e889dd 100644
--- a/amy/workshops/tests/test_bulk_add.py
+++ b/amy/workshops/tests/test_bulk_add.py
@@ -178,6 +178,49 @@ class VerifyUploadPersonTask(CSVBulkUploadTestBase):
             has_errors = verify_upload_person_task(bad_data, match=True)
             self.assertFalse(has_errors, 'Bad email: {}'.format(email))
 
+    def test_email_missing(self):
+        """This tests against regression in:
+        https://github.com/swcarpentry/amy/issues/1394
+
+        The issue: entries without emails (email=None) were caught in
+        `Person.objects.get(email=email)`, which returned multiple objects
+        (because we have many users with empty emails, and it doesn't violate
+        UNIQUE constraint."""
+
+        # make existing users loose their emails
+        usernames = ['potter_harry', 'granger_hermione', 'weasley_ron']
+        Person.objects.filter(username__in=usernames).update(email=None)
+
+        bad_data = [
+            {
+                'email': None,
+                'personal': 'Harry',
+                'family': 'Potter',
+                'event': 'foobar',
+                'role': 'learner'
+            },
+            {
+                'email': None,
+                'personal': 'Hermione',
+                'family': 'Granger',
+                'event': 'foobar',
+                'role': 'learner'
+            },
+            {
+                'email': None,
+                'personal': 'Ron',
+                'family': 'Weasley',
+                'event': 'foobar',
+                'role': 'learner'
+            },
+        ]
+        # test for first occurrence of the error
+        has_errors = verify_upload_person_task(bad_data, match=True)
+        self.assertFalse(has_errors)
+        # test for second occurrence of the error
+        has_errors = verify_upload_person_task(bad_data, match=False)
+        self.assertFalse(has_errors)
+
     def test_verify_existing_user_has_workshop_role_provided(self):
         bad_data = [
             {
diff --git a/amy/workshops/util.py b/amy/workshops/util.py
index ce28e046..8785d610 100644
--- a/amy/workshops/util.py
+++ b/amy/workshops/util.py
@@ -152,7 +152,7 @@ def verify_upload_person_task(data, match=False):
         if match is True:
             try:
                 person = Person.objects.get(email=email)
-            except Person.DoesNotExist:
+            except (Person.DoesNotExist, Person.MultipleObjectsReturned):
                 person = None
             else:
                 info.append('Existing record for person will be used.')
@@ -171,7 +171,7 @@ def verify_upload_person_task(data, match=False):
         elif not person_id:
             try:
                 Person.objects.get(email=email)
-            except Person.DoesNotExist:
+            except (Person.DoesNotExist, Person.MultipleObjectsReturned):
                 pass
             else:
                 errors.append('Person with this email address already exists.')