Bugfix: don't allow duplicate team memberships

CHANGELOG.md

@@ -11,8 +11,9 @@ All notable changes to this project will be documented in this file.
 - Host a read-only dashboard (from github.com/healthchecks/dashboard/)
 
 ## Bug Fixes
-- Handle excessively long email addresses in the signup form.
-- Handle excessively long email addresses in the team member invite form.
+- Handle excessively long email addresses in the signup form
+- Handle excessively long email addresses in the team member invite form
+- Don't allow duplicate team memberships
 
 ## v1.16.0 - 2020-08-04
 

hc/accounts/migrations/0032_auto_20200819_0757.py

@@ -0,0 +1,17 @@
+# Generated by Django 3.1 on 2020-08-19 07:57
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('accounts', '0031_auto_20200803_1413'),
+    ]
+
+    operations = [
+        migrations.AddConstraint(
+            model_name='member',
+            constraint=models.UniqueConstraint(fields=('user', 'project'), name='accounts_member_no_duplicates'),
+        ),
+    ]

hc/accounts/models.py

@@ -319,9 +319,16 @@ class Project(models.Model):
         return used < self.owner_profile.team_limit
 
     def invite(self, user):
+        if Member.objects.filter(user=user, project=self).exists():
+            return False
+
+        if self.owner_id == user.id:
+            return False
+
         Member.objects.create(user=user, project=self)
         checks_url = reverse("hc-checks", args=[self.code])
         user.profile.send_instant_login_link(self, redirect_url=checks_url)
+        return True
 
     def set_next_nag_date(self):
         """ Set next_nag_date on profiles of all members of this project. """
@@ -373,5 +380,12 @@ class Member(models.Model):
     project = models.ForeignKey(Project, models.CASCADE)
     transfer_request_date = models.DateTimeField(null=True, blank=True)
 
+    class Meta:
+        constraints = [
+            models.UniqueConstraint(
+                fields=["user", "project"], name="accounts_member_no_duplicates"
+            )
+        ]
+
     def can_accept(self):
         return self.user.profile.can_accept(self.project)

hc/accounts/tests/test_project.py

@@ -108,6 +108,26 @@ class ProjectTestCase(BaseTestCase):
         q = TokenBucket.objects.filter(value="invite-%d" % self.alice.id)
         self.assertFalse(q.exists())
 
+    def test_it_rejects_duplicate_membership(self):
+        self.client.login(username="[email protected]", password="password")
+
+        form = {"invite_team_member": "1", "email": "[email protected]"}
+        r = self.client.post(self.url, form)
+        self.assertContains(r, "[email protected] is already a member")
+
+        # The number of memberships should have not increased
+        self.assertEqual(self.project.member_set.count(), 1)
+
+    def test_it_rejects_owner_as_a_member(self):
+        self.client.login(username="[email protected]", password="password")
+
+        form = {"invite_team_member": "1", "email": "[email protected]"}
+        r = self.client.post(self.url, form)
+        self.assertContains(r, "[email protected] is already a member")
+
+        # The number of memberships should have not increased
+        self.assertEqual(self.project.member_set.count(), 1)
+
     def test_it_rejects_too_long_email_addresses(self):
         self.client.login(username="[email protected]", password="password")
 

hc/accounts/views.py

@@ -304,9 +304,12 @@ def project(request, code):
                 except User.DoesNotExist:
                     user = _make_user(email, with_project=False)
 
-                project.invite(user)
-                ctx["team_member_invited"] = email
-                ctx["team_status"] = "success"
+                if project.invite(user):
+                    ctx["team_member_invited"] = email
+                    ctx["team_status"] = "success"
+                else:
+                    ctx["team_member_duplicate"] = email
+                    ctx["team_status"] = "info"
 
         elif "remove_team_member" in request.POST:
             if not is_owner:

templates/accounts/project.html

@@ -228,6 +228,12 @@
             </div>
             {% endif %}
 
+           {% if team_member_duplicate %}
+            <div class="panel-footer">
+                {{ team_member_duplicate }} is already a member
+            </div>
+            {% endif %}
+
            {% if team_member_removed %}
             <div class="panel-footer">
                 {{ team_member_removed }} removed from team