diff --git a/src/registrar/admin.py b/src/registrar/admin.py index f2e9a65e8..49cf08935 100644 --- a/src/registrar/admin.py +++ b/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() diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index 55ca64da5..9acec8c64 100644 --- a/src/registrar/models/portfolio.py +++ b/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 diff --git a/src/registrar/models/user_group.py b/src/registrar/models/user_group.py index e6bcaa4f4..41ae67c06 100644 --- a/src/registrar/models/user_group.py +++ b/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", + ], }, ] diff --git a/src/registrar/templates/django/admin/includes/details_button.html b/src/registrar/templates/django/admin/includes/details_button.html index 9ae039b04..73748f170 100644 --- a/src/registrar/templates/django/admin/includes/details_button.html +++ b/src/registrar/templates/django/admin/includes/details_button.html @@ -6,4 +6,4 @@ {% block detail_content %} {% endblock detail_content%} - \ No newline at end of file + diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_domain_requests_table.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_domain_requests_table.html index 5086721f7..46303efce 100644 --- a/src/registrar/templates/django/admin/includes/portfolio/portfolio_domain_requests_table.html +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_domain_requests_table.html @@ -23,4 +23,4 @@ {% endfor %} -{% endblock detail_content %} \ No newline at end of file +{% endblock detail_content %} diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_domains_table.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_domains_table.html index 8b2aa018c..56621b769 100644 --- a/src/registrar/templates/django/admin/includes/portfolio/portfolio_domains_table.html +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_domains_table.html @@ -27,4 +27,4 @@ {% endfor %} -{% endblock detail_content%} \ No newline at end of file +{% endblock detail_content%} diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html index 26c84800f..c5c6f510a 100644 --- a/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html +++ b/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 %} \ No newline at end of file +{% endblock after_help_text %} diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html index 2f3389032..136fe3a5a 100644 --- a/src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html @@ -52,4 +52,4 @@ {% endfor %} -{% endblock %} \ No newline at end of file +{% endblock %} diff --git a/src/registrar/templates/django/admin/portfolio_change_form.html b/src/registrar/templates/django/admin/portfolio_change_form.html index 0a87f8e49..fec1538d9 100644 --- a/src/registrar/templates/django/admin/portfolio_change_form.html +++ b/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 %} diff --git a/src/registrar/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index de9b7bfa1..0bde40bb9 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/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 [] \ No newline at end of file + return [] diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index e125baf60..cdc3c97de 100644 --- a/src/registrar/tests/test_admin.py +++ b/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'2 administrators', 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'Mad Max madmax@gov.gov', - display_members_summary, - ) - - url = reverse("admin:registrar_userportfoliopermission_change", args=[perm_4.pk]) - self.assertIn( - f'Agent Smith thematrix@gov.gov', - display_members_summary, - ) - display_members = self.admin.display_members(self.portfolio) - - self.assertIn("Mad Max", display_members) - self.assertIn("Member", display_members) - self.assertIn("Road warrior", display_members) - self.assertIn("Agent Smith", display_members) - self.assertIn("Domain requestor", display_members) - self.assertIn("Program", display_members) + self.assertIn(f'2 members', display_members) class TestTransferUser(WebTest): diff --git a/src/registrar/tests/test_migrations.py b/src/registrar/tests/test_migrations.py index 5dde4831c..d646a03de 100644 --- a/src/registrar/tests/test_migrations.py +++ b/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)