a few improvements to code, comments, and tests

src/registrar/admin.py

@@ -433,11 +433,9 @@ class MultiFieldSortableChangeList(admin.views.main.ChangeList):
 
     def get_filters_params(self, params=None):
         """
-        Overrides the default behavior which gets filter_params, except
+        Add portfolio to ignored params to allow the portfolio filter while not
-        those in IGNORED_PARAMS. The override is to also include
+        listing it as a filter option on the right side of Change List on the
-        portfolio in the overrides. This allows the portfolio filter
+        portfolio list.
-        not to throw an error as a valid filter while not listing the
-        portfolio filter on the right side of the Change List view.
         """
         params = params or self.params
         lookup_params = params.copy()  # a dictionary of the query string
@@ -3018,7 +3016,7 @@ class PortfolioAdmin(ListHeaderAdmin):
         if domain_count > 0:
             # Construct the URL to the admin page, filtered by portfolio
             url = reverse("admin:registrar_domain_changelist") + f"?portfolio={obj.id}"
-            label = "Domain" if domain_count == 1 else "Domains"
+            label = "Domain" if domain_count == 1 else "No domains"
             # Create a clickable link with the domain count
             return format_html('<a href="{}">{} {}</a>', url, domain_count, label)
         return "No Domains"
@@ -3033,7 +3031,7 @@ class PortfolioAdmin(ListHeaderAdmin):
             url = reverse("admin:registrar_domainrequest_changelist") + f"?portfolio={obj.id}"
             # Create a clickable link with the domain request count
             return format_html('<a href="{}">{} Domain Requests</a>', url, domain_request_count)
-        return "No Domain Requests"
+        return "No domain requests"
 
     domain_requests.short_description = "Domain requests"  # type: ignore
 

src/registrar/tests/test_admin_domain.py

@@ -16,6 +16,7 @@ from registrar.models import (
     Host,
     Portfolio,
 )
+from registrar.models.user_domain_role import UserDomainRole
 from .common import (
     MockSESClient,
     completed_domain_request,
@@ -357,6 +358,7 @@ class TestDomainAdminWithClient(TestCase):
     def tearDown(self):
         super().tearDown()
         Host.objects.all().delete()
+        UserDomainRole.objects.all().delete()
         Domain.objects.all().delete()
         DomainInformation.objects.all().delete()
         DomainRequest.objects.all().delete()
@@ -457,7 +459,7 @@ class TestDomainAdminWithClient(TestCase):
     @less_console_noise_decorator
     def test_domains_by_portfolio(self):
         """
-        Tests that domains display for a portfolio.
+        Tests that domains display for a portfolio.  And that domains outside the portfolio do not display.
         """
 
         portfolio, _ = Portfolio.objects.get_or_create(organization_name="Test Portfolio", creator=self.superuser)
@@ -468,6 +470,9 @@ class TestDomainAdminWithClient(TestCase):
         _domain_request.approve()
 
         domain = _domain_request.approved_domain
+        domain2, _ = Domain.objects.get_or_create(name="fake.gov", state=Domain.State.READY)
+        UserDomainRole.objects.get_or_create()
+        UserDomainRole.objects.get_or_create(user=self.superuser, domain=domain2, role=UserDomainRole.Roles.MANAGER)
 
         self.client.force_login(self.superuser)
         response = self.client.get(
@@ -478,6 +483,7 @@ class TestDomainAdminWithClient(TestCase):
         # Make sure the page loaded, and that we're on the right page
         self.assertEqual(response.status_code, 200)
         self.assertContains(response, domain.name)
+        self.assertNotContains(response, domain2.name)
         self.assertContains(response, portfolio.organization_name)
 
     @less_console_noise_decorator

src/registrar/tests/test_admin_request.py

@@ -268,7 +268,7 @@ class TestDomainRequestAdmin(MockEppLib):
     @less_console_noise_decorator
     def test_domain_requests_by_portfolio(self):
         """
-        Tests that domain_requests display for a portfolio.
+        Tests that domain_requests display for a portfolio. And requests not in portfolio do not display.
         """
 
         portfolio, _ = Portfolio.objects.get_or_create(organization_name="Test Portfolio", creator=self.superuser)
@@ -276,6 +276,9 @@ class TestDomainRequestAdmin(MockEppLib):
         domain_request = completed_domain_request(
             status=DomainRequest.DomainRequestStatus.IN_REVIEW, portfolio=portfolio
         )
+        domain_request2 = completed_domain_request(
+            name="testdomain2.gov", status=DomainRequest.DomainRequestStatus.IN_REVIEW            
+        )
 
         self.client.force_login(self.superuser)
         response = self.client.get(
@@ -286,6 +289,7 @@ class TestDomainRequestAdmin(MockEppLib):
         # Make sure the page loaded, and that we're on the right page
         self.assertEqual(response.status_code, 200)
         self.assertContains(response, domain_request.requested_domain.name)
+        self.assertNotContains(response, domain_request2.requested_domain.name)
         self.assertContains(response, portfolio.organization_name)
 
     @less_console_noise_decorator