3851 - Make suborg non unique portfolio unique (#3888)

* made changes

* added comma

* migrations

* fixing for tests

* removed an import

* possible change

* removed unique value

* redid the migration

* just migrating the suborg

* with sandbox fixes

* added comma

* added unique true for the suborg

* added test

* cleanup commented out section

* ran docker app black

* added unique portfolio

* this worked

* added the federal agency non duplicates

* updated to allow unique values

* another alternative for non unique federal agencies

* using clean

* updated message

* comment

* updates to the model

* adjusted constraints

* update the filter

* fixed failing tests

* made changes to suborg

* added the pk field

* removed random debugger html

* removed extra logic in a test

* formatted file

* ran formatter

* edited logic for checking federal agency to include non-federal agency

* ran the formatter

* removed the soft delete constraint

* added migration

* added exclude pk

* added test back

* fixed the tests

* formatted files

* reverted changes to the test file

* debugging line

* updated logic

src/registrar/management/commands/create_federal_portfolio.py

@@ -206,7 +206,9 @@ class Command(BaseCommand):
         # == Handle domains and requests == #
         for portfolio_org_name, portfolio in portfolios_to_use_dict.items():
             federal_agency = agencies_dict.get(portfolio_org_name)
-            suborgs = portfolio.portfolio_suborganizations.in_bulk(field_name="name")
+            suborgs = {}
+            for suborg in portfolio.portfolio_suborganizations.all():
+                suborgs[suborg.name] = suborg
 
             if parse_domains:
                 updated_domains = self.update_domains(portfolio, federal_agency, suborgs, debug)

src/registrar/migrations/0152_alter_portfolio_organization_name_and_more.py

@@ -0,0 +1,23 @@
+# Generated by Django 4.2.20 on 2025-06-24 14:16
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("registrar", "0151_alter_suborganization_options"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="portfolio",
+            name="organization_name",
+            field=models.CharField(blank=True, null=True, unique=True),
+        ),
+        migrations.AlterField(
+            model_name="suborganization",
+            name="name",
+            field=models.CharField(max_length=1000, verbose_name="Suborganization"),
+        ),
+    ]

src/registrar/models/portfolio.py

@@ -7,6 +7,7 @@ from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices
 from django.db.models import Q
 
 from .utility.time_stamped_model import TimeStampedModel
+from django.core.exceptions import ValidationError
 
 
 class Portfolio(TimeStampedModel):
@@ -36,6 +37,7 @@ class Portfolio(TimeStampedModel):
     organization_name = models.CharField(
         null=True,
         blank=True,
+        unique=True,
     )
 
     organization_type = models.CharField(
@@ -135,6 +137,19 @@ class Portfolio(TimeStampedModel):
 
         super().save(*args, **kwargs)
 
+    def clean(self):
+
+        # Checks if federal agency already exists in the portfolio table
+        if (
+            self.federal_agency != FederalAgency.get_non_federal_agency()
+            and Portfolio.objects.exclude(pk=self.pk).filter(federal_agency=self.federal_agency).exists()
+        ):
+            raise ValidationError({"federal_agency": "Portfolio with this federal agency already exists"})
+
+        # Checks if organization name already exists in the portfolio table (not case sensitive)
+        if Portfolio.objects.exclude(pk=self.pk).filter(organization_name__iexact=self.organization_name).exists():
+            raise ValidationError({"organization_name": "Portfolio with this name already exists"})
+
     @property
     def federal_type(self):
         """Returns the federal_type value on the underlying federal_agency field"""

src/registrar/models/suborganization.py

@@ -2,6 +2,7 @@ from django.db import models
 
 from registrar.models.domain_request import DomainRequest
 from .utility.time_stamped_model import TimeStampedModel
+from django.core.exceptions import ValidationError
 
 
 class Suborganization(TimeStampedModel):
@@ -13,7 +14,6 @@ class Suborganization(TimeStampedModel):
         ordering = ["name"]
 
     name = models.CharField(
-        unique=True,
         max_length=1000,
         verbose_name="Suborganization",
     )
@@ -39,3 +39,17 @@ class Suborganization(TimeStampedModel):
 
     def __str__(self) -> str:
         return f"{self.name}"
+
+    def clean(self):
+        if (
+            Suborganization.objects.exclude(pk=self.pk)
+            .filter(
+                portfolio=self.portfolio,
+                name__iexact=self.name,
+            )
+            .exists()
+        ):
+            raise ValidationError({"name": "Name already exists in Portfolio"})
+
+    def save(self, *args, **kwargs):
+        super().save(*args, **kwargs)

src/registrar/tests/test_admin.py

@@ -73,6 +73,8 @@ 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 unittest.mock import ANY, call, patch, Mock
 
@@ -4183,6 +4185,15 @@ 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):
+        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(ValidationError):
+            test_dup = Suborganization(name="sub1", portfolio=portfolio)
+            test_dup.full_clean()
+
     @less_console_noise_decorator
     def test_domains_display(self):
         """Tests the custom domains field which displays all related domains"""
@@ -4339,6 +4350,13 @@ class TestPortfolioAdmin(TestCase):
         federal_agency.delete()
         portfolio.delete()
 
+    @less_console_noise_decorator
+    def test_duplicate_portfolio(self):
+        with self.assertRaises(ValidationError):
+            with transaction.atomic():
+                portfolio_dup = Portfolio(organization_name="Test portfolio", creator=self.superuser)
+                portfolio_dup.full_clean()
+
 
 class TestTransferUser(WebTest):
     """User transfer custom admin page"""