From a61aad62e9d319f3a8e6a90490145e2d23a74299 Mon Sep 17 00:00:00 2001
From: David Fischer <david@readthedocs.org>
Date: Sat, 1 Nov 2025 09:58:35 -0700
Subject: [PATCH] User advertiser member check and constraint

I hit this extremely minor bug in production. If you try to add the same
user again as an authorized user on an advertiser, you hit a DB
constraint (500 error).

That's because the constraint added here exists
in production but not in the model. That is because at first the
UserAdvertiserMember model was just an automatic table and then we added
a new column to the table via migration.

This adds a secondary check to ensure we don't try to add the same user
again and adds the constraint in development. Hopefully there isn't an
issue adding that constraint again in prod or we could fake the
migration if desired since the constraint already exists.
---
 ...ter_useradvertisermember_unique_together.py | 18 ++++++++++++++++++
 adserver/auth/models.py                        |  2 ++
 adserver/tests/test_advertiser_dashboard.py    |  1 +
 adserver/views.py                              | 17 ++++++++++-------
 4 files changed, 31 insertions(+), 7 deletions(-)
 create mode 100644 adserver/auth/migrations/0011_alter_useradvertisermember_unique_together.py

diff --git a/adserver/auth/migrations/0011_alter_useradvertisermember_unique_together.py b/adserver/auth/migrations/0011_alter_useradvertisermember_unique_together.py
new file mode 100644
index 00000000..fc40684f
--- /dev/null
+++ b/adserver/auth/migrations/0011_alter_useradvertisermember_unique_together.py
@@ -0,0 +1,18 @@
+# Generated by Django 5.2.7 on 2025-11-01 16:57
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('adserver', '0104_auto_renew_payment_method'),
+        ('adserver_auth', '0010_data_all_emails_verified'),
+    ]
+
+    operations = [
+        migrations.AlterUniqueTogether(
+            name='useradvertisermember',
+            unique_together={('user', 'advertiser')},
+        ),
+    ]
diff --git a/adserver/auth/models.py b/adserver/auth/models.py
index b7cf224b..8ee67e8c 100644
--- a/adserver/auth/models.py
+++ b/adserver/auth/models.py
@@ -255,6 +255,8 @@ class Meta:
         # To do that, we needed to start with the same table
         db_table = "adserver_auth_user_advertisers"
 
+        unique_together = ("user", "advertiser")
+
 
 class UserPublisherMember(models.Model):
     """User-Publisher 'through' model."""
diff --git a/adserver/tests/test_advertiser_dashboard.py b/adserver/tests/test_advertiser_dashboard.py
index bf4f40ab..1e68f03d 100644
--- a/adserver/tests/test_advertiser_dashboard.py
+++ b/adserver/tests/test_advertiser_dashboard.py
@@ -1252,6 +1252,7 @@ def test_authorized_users_invite_existing(self):
         self.assertContains(response, "Successfully invited")
         self.assertEqual(User.objects.filter(email=email).count(), 1)
         self.assertEqual(User.objects.filter(name=name).count(), 1)
+        self.assertEqual(UserAdvertiserMember.objects.filter(advertiser=self.advertiser, user__email=email).count(), 1)
 
         # The 2nd request didn't create a user or update the user's name
         self.assertEqual(User.objects.filter(name="Yet Another User").count(), 0)
diff --git a/adserver/views.py b/adserver/views.py
index 6acdfd04..05dc1e24 100644
--- a/adserver/views.py
+++ b/adserver/views.py
@@ -1857,13 +1857,16 @@ def dispatch(self, request, *args, **kwargs):
     def form_valid(self, form):
         result = super().form_valid(form)
 
-        # Add m2m and role
-        role = form.cleaned_data["role"]
-        UserAdvertiserMember.objects.create(
-            advertiser=self.advertiser,
-            user=self.object,
-            role=role,
-        )
+        if not UserAdvertiserMember.objects.filter(
+            advertiser=self.advertiser, user=self.object
+        ).exists():
+            # Add m2m and role
+            role = form.cleaned_data["role"]
+            UserAdvertiserMember.objects.create(
+                advertiser=self.advertiser,
+                user=self.object,
+                role=role,
+            )
 
         messages.success(
             self.request,