Fix invite behavior for existing member and different kind

backend/ohq/invite.py

@@ -22,18 +22,18 @@ def parse_and_send_invites(course, emails, kind):
     # Map of pennkey to invite email (which may be different from the user's email)
     email_map = {email.split("@")[0]: email for email in emails}
 
-    # Remove invitees already in class
+    # Remove invitees already in class with same kind
     existing = Membership.objects.filter(
-        course=course, user__username__in=email_map.keys()
+        course=course, user__username__in=email_map.keys(), kind=kind
     ).values_list("user__username", flat=True)
     existing = [email_map[pennkey] for pennkey in existing]
 
     emails = list(set(emails) - set(existing))
 
-    # Remove users already invited
-    existing = MembershipInvite.objects.filter(course=course, email__in=emails).values_list(
-        "email", flat=True
-    )
+    # Remove users already invited with same kind
+    existing = MembershipInvite.objects.filter(
+        course=course, email__in=emails, kind=kind
+    ).values_list("email", flat=True)
     emails = list(set(emails) - set(existing))
 
     # Generate final map of pennkey to email of users that need to be invited
@@ -42,13 +42,17 @@ def parse_and_send_invites(course, emails, kind):
     # Directly add invitees with existing accounts
     users = User.objects.filter(Q(email__in=emails) | Q(username__in=email_map.keys())).distinct()
     for user in users:
-        membership = Membership.objects.create(course=course, user=user, kind=kind)
+        membership, _ = Membership.objects.update_or_create(
+            course=course, user=user, defaults={"kind": kind}
+        )
         membership.send_email()
         del email_map[user.username]
 
     # Create membership invites for invitees without an account
     for email in email_map.values():
-        invite = MembershipInvite.objects.create(email=email, course=course, kind=kind)
+        invite, _ = MembershipInvite.objects.update_or_create(
+            email=email, course=course, defaults={"kind": kind}
+        )
         invite.send_email()
 
     return (users.count(), len(email_map))

backend/tests/ohq/test_invite.py

@@ -11,17 +11,26 @@
 
 class ParseAndSendInvitesTestCase(TestCase):
     def setUp(self):
-        self.professor = User.objects.create(username="professor", email="[email protected]")
         self.semester = Semester.objects.create(year=2020, term=Semester.TERM_SUMMER)
         self.course = Course.objects.create(
             course_code="000", department="TEST", course_title="Title", semester=self.semester
         )
-        Membership.objects.create(
-            course=self.course, user=self.professor, kind=Membership.KIND_PROFESSOR
+
+        self.user1 = User.objects.create(username="user1", email="[email protected]")
+        Membership.objects.create(course=self.course, user=self.user1, kind=Membership.KIND_STUDENT)
+
+        self.user2 = User.objects.create(username="user2", email="[email protected]")
+        Membership.objects.create(course=self.course, user=self.user2, kind=Membership.KIND_TA)
+
+        self.user3 = User.objects.create(username="user3", email="[email protected]")
+
+        MembershipInvite.objects.create(
+            course=self.course, email="[email protected]", kind=Membership.KIND_STUDENT
         )
-        self.user2 = User.objects.create(username="user2", email="[email protected]")
 
-        MembershipInvite.objects.create(course=self.course, email="[email protected]")
+        MembershipInvite.objects.create(
+            course=self.course, email="[email protected]", kind=Membership.KIND_TA
+        )
 
     def test_invalid_email(self):
         with self.assertRaises(ValidationError):
@@ -30,43 +39,61 @@ def test_invalid_email(self):
     def test_valid_emails(self):
         """
         Test situations where
-        * the user is already a member under a different email
-        * the user is not a member of the course and has different email
-        * the email has already been sent an invite
+        * the user is already a member but is different type - user1 (expected: added, not invited)
+        * the user is already a member and is same type - user2 (expected: not added, not invited)
+        * the user is not a member of the course and has different email - user3 (expected: added, not invited)
+        * the email has already been sent an invite but is different type - user4 (expected: not added, invited)
+        * the email has already been sent an invite and is same type - user5 (expected: not added, not invited)
+        * the user is not a member of any course - user6 (expected: not added, invited)
         """
         members_added, invites_sent = parse_and_send_invites(
             self.course,
             [
-                "professor@sas.upenn.edu",
+                "user1@seas.upenn.edu",
                 "[email protected]",
-                "[email protected]",
-                "[email protected]",
+                "[email protected]",
+                "[email protected]",
+                "[email protected]",
+                "[email protected]",
             ],
             Membership.KIND_TA,
         )
 
         # # Correct number of invites and memberships created
-        self.assertEqual(1, members_added)
-        self.assertEqual(1, invites_sent)
+        self.assertEqual(2, members_added)
+        self.assertEqual(2, invites_sent)
+
+        # Membership is created for user1
+        self.assertEqual(
+            1,
+            Membership.objects.filter(
+                user=self.user1, course=self.course, kind=Membership.KIND_TA
+            ).count(),
+        )
 
-        # Membership is created for user2
+        # Membership is created for user3
         self.assertEqual(
             1,
             Membership.objects.filter(
-                user=self.user2, course=self.course, kind=Membership.KIND_TA
+                user=self.user3, course=self.course, kind=Membership.KIND_TA
             ).count(),
         )
 
-        # Duplicate membership for user 1 isn't created
-        self.assertEqual(2, Membership.objects.all().count())
+        # Duplicate membership for user2 isn't created
+        self.assertEqual(3, Membership.objects.all().count())
 
-        # Invite is sent to 4@nursing.upenn.edu
+        # Invite is sent to user4@wharton.upenn.edu
         self.assertEqual(
             1,
             MembershipInvite.objects.filter(
-                email="user4@nursing.upenn.edu", course=self.course, kind=Membership.KIND_TA
+                email="user4@wharton.upenn.edu", course=self.course, kind=Membership.KIND_TA
             ).count(),
         )
 
-        # Duplicate membership invite for [email protected] isn't created
-        self.assertEqual(2, MembershipInvite.objects.all().count())
+        # Invite is sent to [email protected]
+        self.assertEqual(
+            1,
+            MembershipInvite.objects.filter(
+                email="[email protected]", course=self.course, kind=Membership.KIND_TA
+            ).count(),
+        )

backend/tests/ohq/test_views.py

@@ -60,29 +60,43 @@ def test_resend_success(self):
 class MassInviteTestCase(TestCase):
     def setUp(self):
         self.client = APIClient()
-        self.professor = User.objects.create(username="professor", email="[email protected]")
         self.semester = Semester.objects.create(year=2020, term=Semester.TERM_SUMMER)
         self.course = Course.objects.create(
             course_code="000", department="TEST", course_title="Title", semester=self.semester
         )
+
+        self.professor = User.objects.create(username="professor", email="[email protected]")
         Membership.objects.create(
             course=self.course, user=self.professor, kind=Membership.KIND_PROFESSOR
         )
+
+        self.user1 = User.objects.create(username="user1", email="[email protected]")
+        Membership.objects.create(course=self.course, user=self.user1, kind=Membership.KIND_STUDENT)
+
         self.user2 = User.objects.create(username="user2", email="[email protected]")
+        Membership.objects.create(course=self.course, user=self.user2, kind=Membership.KIND_TA)
 
-        MembershipInvite.objects.create(course=self.course, email="[email protected]")
+        self.user3 = User.objects.create(username="user3", email="[email protected]")
+
+        MembershipInvite.objects.create(
+            course=self.course, email="[email protected]", kind=Membership.KIND_STUDENT
+        )
+
+        MembershipInvite.objects.create(
+            course=self.course, email="[email protected]", kind=Membership.KIND_TA
+        )
 
     def test_invalid_email(self):
         self.client.force_authenticate(user=self.professor)
         response = self.client.post(
             reverse("ohq:mass-invite", args=[self.course.id]),
-            data={"emails": "invalidemail", "role": "TA"},
+            data={"emails": "invalidemail", "kind": "TA"},
         )
         self.assertEqual(400, response.status_code)
 
     def test_valid_emails(self):
         self.client.force_authenticate(user=self.professor)
-        emails = "professor@example.com,[email protected],[email protected],[email protected]"
+        emails = "user1@example.com,[email protected],[email protected],user4@example.com,[email protected],user6@example.com"
         response = self.client.post(
             reverse("ohq:mass-invite", args=[self.course.id]),
             data={"emails": emails, "kind": "TA"},
@@ -92,8 +106,8 @@ def test_valid_emails(self):
 
         # Correct number of invites and memberships created
         content = json.loads(response.content)
-        self.assertEqual(1, content["membersAdded"])
-        self.assertEqual(1, content["invitesSent"])
+        self.assertEqual(2, content["membersAdded"])
+        self.assertEqual(2, content["invitesSent"])
 
 
 class QuestionViewTestCase(TestCase):