From af9e9f337ba4f0ce6db6a68e18fd2d853de95290 Mon Sep 17 00:00:00 2001
From: asaki222 <130692870+asaki222@users.noreply.github.com>
Date: Mon, 30 Jun 2025 16:20:55 -0400
Subject: [PATCH] #3851 - Fix for Suborg/Portfolio unique constraints for
 portfolio and non-unique for suborg - [ad] (#3921)

* fix for error

* fix for migration

* updated message

* added unique constraints

* ran the formmatter

* updated the tests

* misspelling

* added a drop constraint

* ran the formatter

* added the reverse sql

* redid migration

---------

Co-authored-by: CuriousX <nicolle.leclair@gmail.com>
---
 ...er_portfolio_organization_name_and_more.py |  6 ++++-
 src/registrar/models/suborganization.py       |  3 ++-
 src/registrar/tests/test_admin.py             | 24 ++++++++++---------
 3 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/src/registrar/migrations/0152_alter_portfolio_organization_name_and_more.py b/src/registrar/migrations/0152_alter_portfolio_organization_name_and_more.py
index 2f0b76284..f4c51e43e 100644
--- a/src/registrar/migrations/0152_alter_portfolio_organization_name_and_more.py
+++ b/src/registrar/migrations/0152_alter_portfolio_organization_name_and_more.py
@@ -1,4 +1,4 @@
-# Generated by Django 4.2.20 on 2025-06-24 14:16
+# Generated by Django 4.2.20 on 2025-06-30 19:30
 
 from django.db import migrations, models
 
@@ -20,4 +20,8 @@ class Migration(migrations.Migration):
             name="name",
             field=models.CharField(max_length=1000, verbose_name="Suborganization"),
         ),
+        migrations.AddConstraint(
+            model_name="suborganization",
+            constraint=models.UniqueConstraint(fields=("name", "portfolio"), name="unique_name_portfolio"),
+        ),
     ]
diff --git a/src/registrar/models/suborganization.py b/src/registrar/models/suborganization.py
index d5cb5eca6..e6e13b5ef 100644
--- a/src/registrar/models/suborganization.py
+++ b/src/registrar/models/suborganization.py
@@ -12,6 +12,7 @@ class Suborganization(TimeStampedModel):
 
     class Meta:
         ordering = ["name"]
+        constraints = [models.UniqueConstraint(fields=["name", "portfolio"], name="unique_name_portfolio")]
 
     name = models.CharField(
         max_length=1000,
@@ -49,7 +50,7 @@ class Suborganization(TimeStampedModel):
             )
             .exists()
         ):
-            raise ValidationError({"name": "Name already exists in Portfolio"})
+            raise ValidationError({"name": "Suborganization name already exists in Portfolio"})
 
     def save(self, *args, **kwargs):
         super().save(*args, **kwargs)
diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index c7ac54d22..7af357da2 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -73,9 +73,7 @@ from registrar.models.utility.portfolio_helper import UserPortfolioPermissionCho
 from django.contrib.sessions.backends.db import SessionStore
 from django.contrib.auth import get_user_model
 from django.contrib import messages
-from django.db import transaction
-from django.core.exceptions import ValidationError
-
+from django.db import transaction, IntegrityError
 from unittest.mock import ANY, call, patch, Mock
 
 import logging
@@ -4185,14 +4183,19 @@ class TestPortfolioAdmin(TestCase):
         suborg_names = [li.text for li in soup.find_all("li")]
         self.assertEqual(suborg_names, ["Sub1", "Sub2", "Sub3", "Sub4", "Sub5"])
 
-    def test_can_have_dup_suborganizatons(self):
+    def test_cannot_have_dup_suborganizations_with_same_portfolio(self):
         portfolio = Portfolio.objects.create(organization_name="Test portfolio too", creator=self.superuser)
         Suborganization.objects.create(name="Sub1", portfolio=portfolio)
-        Suborganization.objects.create(name="Sub1", portfolio=portfolio)
+        with self.assertRaises(IntegrityError):
+            with transaction.atomic():
+                Suborganization.objects.create(name="Sub1", portfolio=portfolio)
 
-        with self.assertRaises(ValidationError):
-            test_dup = Suborganization(name="sub1", portfolio=portfolio)
-            test_dup.full_clean()
+    def test_can_have_dup_suborganizations_with_diff_portfolio(self):
+        portfolio = Portfolio.objects.create(organization_name="Test portfolio too", creator=self.superuser)
+        Suborganization.objects.create(name="Sub1", portfolio=portfolio)
+        Suborganization.objects.create(name="Sub1", portfolio=self.portfolio)
+        num_of_subs = Suborganization.objects.filter(name="Sub1").count()
+        self.assertEqual(num_of_subs, 2)
 
     @less_console_noise_decorator
     def test_domains_display(self):
@@ -4352,10 +4355,9 @@ class TestPortfolioAdmin(TestCase):
 
     @less_console_noise_decorator
     def test_duplicate_portfolio(self):
-        with self.assertRaises(ValidationError):
+        with self.assertRaises(IntegrityError):
             with transaction.atomic():
-                portfolio_dup = Portfolio(organization_name="Test portfolio", creator=self.superuser)
-                portfolio_dup.full_clean()
+                Portfolio.objects.create(organization_name="Test portfolio", creator=self.superuser)
 
 
 class TestTransferUser(WebTest):