linting + fix tests

src/registrar/admin.py

@@ -5,7 +5,6 @@ import json
 from django.template.loader import get_template
 from django import forms
 from django.db.models import Value, CharField, Q
-from django.template.loader import render_to_string
 from django.db.models.functions import Concat, Coalesce
 from django.http import HttpResponseRedirect
 from django.conf import settings
@@ -2981,28 +2980,6 @@ class PortfolioAdmin(ListHeaderAdmin):
         "organization_name",
     ]
 
-    def get_readonly_fields(self, request, obj=None):
-        """Set the read-only state on form elements.
-        We have 2 conditions that determine which fields are read-only:
-        admin user permissions and the creator's status, so
-        we'll use the baseline readonly_fields and extend it as needed.
-        """
-        readonly_fields = list(self.readonly_fields)
-
-        # Check if the creator is restricted
-        if obj and obj.creator.status == models.User.RESTRICTED:
-            # For fields like CharField, IntegerField, etc., the widget used is
-            # straightforward and the readonly_fields list can control their behavior
-            readonly_fields.extend([field.name for field in self.model._meta.fields])
-
-        if request.user.has_perm("registrar.full_access_permission"):
-            return readonly_fields
-
-        # Return restrictive Read-only fields for analysts and
-        # users who might not belong to groups
-        readonly_fields.extend([field for field in self.analyst_readonly_fields])
-        return readonly_fields
-
     def get_admin_users(self, obj):
         # Filter UserPortfolioPermission objects related to the portfolio
         admin_permissions = self.get_user_portfolio_permission_admins(obj)
@@ -3015,7 +2992,9 @@ class PortfolioAdmin(ListHeaderAdmin):
     def get_user_portfolio_permission_admins(self, obj):
         """Returns each admin on UserPortfolioPermission for a given portfolio."""
         if obj:
-            return obj.portfolio_users.filter(portfolio=obj, roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN])
+            return obj.portfolio_users.filter(
+                portfolio=obj, roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]
+            )
         else:
             return []
 
@@ -3177,6 +3156,28 @@ class PortfolioAdmin(ListHeaderAdmin):
             return self.add_fieldsets
         return super().get_fieldsets(request, obj)
 
+    def get_readonly_fields(self, request, obj=None):
+        """Set the read-only state on form elements.
+        We have 2 conditions that determine which fields are read-only:
+        admin user permissions and the creator's status, so
+        we'll use the baseline readonly_fields and extend it as needed.
+        """
+        readonly_fields = list(self.readonly_fields)
+
+        # Check if the creator is restricted
+        if obj and obj.creator.status == models.User.RESTRICTED:
+            # For fields like CharField, IntegerField, etc., the widget used is
+            # straightforward and the readonly_fields list can control their behavior
+            readonly_fields.extend([field.name for field in self.model._meta.fields])
+
+        if request.user.has_perm("registrar.full_access_permission"):
+            return readonly_fields
+
+        # Return restrictive Read-only fields for analysts and
+        # users who might not belong to groups
+        readonly_fields.extend([field for field in self.analyst_readonly_fields])
+        return readonly_fields
+
     def change_view(self, request, object_id, form_url="", extra_context=None):
         """Add related suborganizations and domain groups.
         Add the summary for the portfolio members field (list of members that link to change_forms)."""
@@ -3184,7 +3185,7 @@ class PortfolioAdmin(ListHeaderAdmin):
         extra_context = extra_context or {}
         extra_context["skip_additional_contact_info"] = True
 
-        # We repeat these calls twice. 
+        # We repeat these calls twice.
         extra_context["members"] = self.get_user_portfolio_permission_non_admins(obj)
         extra_context["admins"] = self.get_user_portfolio_permission_admins(obj)
         extra_context["domains"] = obj.get_domains()
@@ -3205,7 +3206,7 @@ class PortfolioAdmin(ListHeaderAdmin):
         is_federal = obj.organization_type == DomainRequest.OrganizationChoices.FEDERAL
         if is_federal and obj.organization_name is None:
             obj.organization_name = obj.federal_agency.agency
-        
+
         # Remove this line when senior_official is no longer readonly in /admin.
         if obj.federal_agency and obj.federal_agency.so_federal_agency.exists():
             obj.senior_official = obj.federal_agency.so_federal_agency.first()

src/registrar/models/portfolio.py

@@ -2,7 +2,6 @@ from django.db import models
 
 from registrar.models.domain_request import DomainRequest
 from registrar.models.federal_agency import FederalAgency
-from registrar.utility.constants import BranchChoices
 
 from .utility.time_stamped_model import TimeStampedModel
 

src/registrar/models/user_group.py

@@ -79,7 +79,11 @@ class UserGroup(Group):
             {
                 "app_label": "registrar",
                 "model": "userportfoliopermission",
-                "permissions": ["add_userportfoliopermission", "change_userportfoliopermission", "delete_userportfoliopermission"],
+                "permissions": [
+                    "add_userportfoliopermission",
+                    "change_userportfoliopermission",
+                    "delete_userportfoliopermission",
+                ],
             },
         ]
 

src/registrar/templates/django/admin/includes/details_button.html

@@ -6,4 +6,4 @@
         {% block detail_content %}
         {% endblock detail_content%}
     </div>
-</details>
+</details>

src/registrar/templates/django/admin/includes/portfolio/portfolio_domain_requests_table.html

@@ -23,4 +23,4 @@
         {% endfor %}
     </tbody>
 </table>
-{% endblock detail_content %}
+{% endblock detail_content %}

src/registrar/templates/django/admin/includes/portfolio/portfolio_domains_table.html

@@ -27,4 +27,4 @@
         {% endfor %}
     </tbody>
 </table>
-{% endblock detail_content%}
+{% endblock detail_content%}

src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html

@@ -58,4 +58,4 @@
             {% include "django/admin/includes/portfolio/portfolio_domain_requests_table.html" with domain_requests=domain_requests %}
         {% endif %}
     {% endif %}
-{% endblock after_help_text %}
+{% endblock after_help_text %}

src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html

@@ -52,4 +52,4 @@
             {% endfor %}
         </tbody>
     </table>
-{% endblock %}
+{% endblock %}

src/registrar/templates/django/admin/portfolio_change_form.html

@@ -13,13 +13,6 @@
 
 {% block field_sets %}
     {% for fieldset in adminform %}
-        {% comment %}
-        This is a placeholder for now.
-
-        Disclaimer:
-        When extending the fieldset view consider whether you need to make a new one that extends from detail_table_fieldset.
-        detail_table_fieldset is used on multiple admin pages, so a change there can have unintended consequences.
-        {% endcomment %}
         {% include "django/admin/includes/portfolio/portfolio_fieldset.html" with original_object=original %}
     {% endfor %}
 {% endblock %}

src/registrar/templatetags/custom_filters.py

@@ -247,4 +247,4 @@ def portfolio_role_summary(user, portfolio):
     if user and portfolio:
         return user.portfolio_role_summary(portfolio)
     else:
-        return []
+        return []

src/registrar/tests/test_admin.py

@@ -2051,7 +2051,7 @@ class TestPortfolioAdmin(TestCase):
             email="meaoward@gov.gov",
         )
 
-        perm_1 = UserPortfolioPermission.objects.all().create(
+        UserPortfolioPermission.objects.all().create(
             user=admin_user_1, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]
         )
 
@@ -2063,7 +2063,7 @@ class TestPortfolioAdmin(TestCase):
             email="poopy@gov.gov",
         )
 
-        perm_2 = UserPortfolioPermission.objects.all().create(
+        UserPortfolioPermission.objects.all().create(
             user=admin_user_2, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]
         )
 
@@ -2075,7 +2075,7 @@ class TestPortfolioAdmin(TestCase):
             email="madmax@gov.gov",
         )
 
-        perm_3 = UserPortfolioPermission.objects.all().create(
+        UserPortfolioPermission.objects.all().create(
             user=admin_user_3, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER]
         )
 
@@ -2087,7 +2087,7 @@ class TestPortfolioAdmin(TestCase):
             email="thematrix@gov.gov",
         )
 
-        perm_4 = UserPortfolioPermission.objects.all().create(
+        UserPortfolioPermission.objects.all().create(
             user=admin_user_4,
             portfolio=self.portfolio,
             additional_permissions=[
@@ -2100,28 +2100,8 @@ class TestPortfolioAdmin(TestCase):
         url = reverse("admin:registrar_userportfoliopermission_changelist") + f"?portfolio={self.portfolio.id}"
         self.assertIn(f'<a href="{url}">2 administrators</a>', display_admins)
 
-        display_members_summary = self.admin.display_members_summary(self.portfolio)
-
-        url = reverse("admin:registrar_userportfoliopermission_change", args=[perm_3.pk])
-        self.assertIn(
-            f'<a href="{url}">Mad Max madmax@gov.gov</a>',
-            display_members_summary,
-        )
-
-        url = reverse("admin:registrar_userportfoliopermission_change", args=[perm_4.pk])
-        self.assertIn(
-            f'<a href="{url}">Agent Smith thematrix@gov.gov</a>',
-            display_members_summary,
-        )
-
         display_members = self.admin.display_members(self.portfolio)
-
+        self.assertIn(f'<a href="{url}">2 members</a>', display_members)
-        self.assertIn("Mad Max", display_members)
-        self.assertIn("<span class='usa-tag'>Member</span>", display_members)
-        self.assertIn("Road warrior", display_members)
-        self.assertIn("Agent Smith", display_members)
-        self.assertIn("<span class='usa-tag'>Domain requestor</span>", display_members)
-        self.assertIn("Program", display_members)
 
 
 class TestTransferUser(WebTest):

src/registrar/tests/test_migrations.py

@@ -50,7 +50,9 @@ class TestGroups(TestCase):
             "change_user",
             "delete_userdomainrole",
             "view_userdomainrole",
-            "view_userportfoliopermission",
+            "add_userportfoliopermission",
+            "change_userportfoliopermission",
+            "delete_userportfoliopermission",
             "add_verifiedbystaff",
             "change_verifiedbystaff",
             "delete_verifiedbystaff",
@@ -58,6 +60,7 @@ class TestGroups(TestCase):
 
         # Get the codenames of actual permissions associated with the group
         actual_permissions = [p.codename for p in cisa_analysts_group.permissions.all()]
+        self.maxDiff = None
 
         # Assert that the actual permissions match the expected permissions
         self.assertListEqual(actual_permissions, expected_permissions)