From eb061be71bf34ec3f75fabe282089e5c80d80457 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 21 Aug 2024 12:25:27 -0400 Subject: [PATCH 01/37] Django admin portfolio members section --- src/registrar/admin.py | 110 ++++++++++++++++-- src/registrar/models/user.py | 43 +++++++ .../admin/includes/detail_table_fieldset.html | 11 ++ .../django/admin/portfolio_change_form.html | 3 +- src/registrar/tests/test_admin.py | 80 +++++++++++++ src/registrar/tests/test_models.py | 55 +++++++++ 6 files changed, 288 insertions(+), 14 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 423c0a01b..12c5d8cf8 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2867,11 +2867,7 @@ class PortfolioAdmin(ListHeaderAdmin): fieldsets = [ # created_on is the created_at field, and portfolio_type is f"{organization_type} - {federal_type}" (None, {"fields": ["portfolio_type", "organization_name", "creator", "created_on", "notes"]}), - # TODO - uncomment in #2521 - # ("Portfolio members", { - # "classes": ("collapse", "closed"), - # "fields": ["administrators", "members"]} - # ), + ("Portfolio members", {"fields": ["display_admins", "display_members"]}), ("Portfolio domains", {"classes": ("collapse", "closed"), "fields": ["domains", "domain_requests"]}), ("Type of organization", {"fields": ["organization_type", "federal_type"]}), ( @@ -2925,8 +2921,94 @@ class PortfolioAdmin(ListHeaderAdmin): "domain_requests", "suborganizations", "portfolio_type", + # Django admin doesn't allow methods to be directly listed in fieldsets. We can + # display the custom methods display_admins amd display_members in the admin form if + # they are readonly. + "display_admins", + "display_members", ] + def display_admins(self, obj): + """Get joined users who are Admin, unpack and return an HTML block. + + 'DJA readonly can't handle querysets, so we need to unpack and return html here. + Alternatively, we could return querysets in context but that would limit where this + data would display in a custom change form without extensive template customization. + + Will be used in the field_readonly block""" + admins = [user for user in obj.user.all() if "Admin" in user.portfolio_role_summary] + if not admins: + return format_html("

No admins found.

") + + admin_details = "" + for portfolio_admin in admins: + change_url = reverse("admin:registrar_user_change", args=[portfolio_admin.pk]) + admin_details += "
" + admin_details += f'{portfolio_admin}
' + admin_details += f"{portfolio_admin.title}
" + admin_details += f"{portfolio_admin.email}" + admin_details += "
" + admin_details += f"" + admin_details += ( + "" + admin_details += "

" + admin_details += f"{portfolio_admin.phone}" + admin_details += "
" + return format_html(admin_details) + + display_admins.short_description = "Administrators" # type: ignore + + def display_members(self, obj): + """Get joined users who have roles/perms that are not Admin, unpack and return an HTML block. + + DJA readonly can't handle querysets, so we need to unpack and return html here. + Alternatively, we could return querysets in context but that would limit where this + data would display in a custom change form without extensive template customization. + + Will be used in the after_help_text block.""" + members = [user for user in obj.user.all() if "Admin" not in user.portfolio_role_summary] + if not members: + return format_html("

No members found.

") + + member_details = ( + "" + + "" + ) + for member in members: + full_name = ( + f"{member.first_name} {member.middle_name} {member.last_name}" + if member.middle_name + else f"{member.first_name} {member.last_name}" + ) + member_details += "" + member_details += f"" + member_details += f"" + member_details += f"" + member_details += f"" + member_details += "" + member_details += "
NameTitleEmailPhoneRoles
{full_name}{member.title}{member.email}{member.phone}" + for role in member.portfolio_role_summary: + member_details += f"{role} " + member_details += "
" + return format_html(member_details) + + display_members.short_description = "Members" # type: ignore + + def display_members_summary(self, obj): + """Will be passed as context and used in the field_readonly block.""" + members = [user for user in obj.user.all() if "Admin" not in user.portfolio_role_summary] + if not members: + return {} + + return self.get_field_links_as_list(members, "user", separator=", ") + def federal_type(self, obj: models.Portfolio): """Returns the federal_type field""" return BranchChoices.get_branch_label(obj.federal_type) if obj.federal_type else "-" @@ -2977,7 +3059,7 @@ class PortfolioAdmin(ListHeaderAdmin): ] def get_field_links_as_list( - self, queryset, model_name, attribute_name=None, link_info_attribute=None, seperator=None + self, queryset, model_name, attribute_name=None, link_info_attribute=None, separator=None ): """ Generate HTML links for items in a queryset, using a specified attribute for link text. @@ -3009,14 +3091,14 @@ class PortfolioAdmin(ListHeaderAdmin): if link_info_attribute: link += f" ({self.value_of_attribute(item, link_info_attribute)})" - if seperator: + if separator: links.append(link) else: links.append(f"
  • {link}
    • ") - # If no seperator is specified, just return an unordered list. - if seperator: - return format_html(seperator.join(links)) if links else "-" + # If no separator is specified, just return an unordered list. + if separator: + return format_html(separator.join(links)) if links else "-" else: links = "".join(links) return format_html(f'') if links else "-" @@ -3059,8 +3141,12 @@ class PortfolioAdmin(ListHeaderAdmin): return readonly_fields def change_view(self, request, object_id, form_url="", extra_context=None): - """Add related suborganizations and domain groups""" - extra_context = {"skip_additional_contact_info": True} + """Add related suborganizations and domain groups. + Add the summary for the portfolio members field (list of members that link to change_forms).""" + obj = self.get_object(request, object_id) + extra_context = extra_context or {} + extra_context["skip_additional_contact_info"] = True + extra_context["display_members_summary"] = self.display_members_summary(obj) return super().change_view(request, object_id, form_url, extra_context) def save_model(self, request, obj, form, change): diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 81d3b9b61..a6026b63a 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -293,6 +293,49 @@ class User(AbstractUser): def has_edit_suborganization(self): return self._has_portfolio_permission(UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION) + def has_edit_requests(self): + return self._has_portfolio_permission(UserPortfolioPermissionChoices.EDIT_REQUESTS) + + @property + def portfolio_role_summary(self): + """Returns a list of roles based on the user's permissions.""" + roles = [] + + # Define the conditions and their corresponding roles + conditions_roles = [ + (self.has_edit_suborganization(), ["Admin"]), + ( + self.has_view_all_domains_permission() + and self.has_domain_requests_portfolio_permission() + and self.has_edit_requests(), + ["View-only admin", "Domain requestor"], + ), + ( + self.has_view_all_domains_permission() and self.has_domain_requests_portfolio_permission(), + ["View-only admin"], + ), + ( + self.has_base_portfolio_permission() + and self.has_edit_requests() + and self.has_domains_portfolio_permission(), + ["Member", "Domain requestor", "Domain manager"], + ), + (self.has_base_portfolio_permission() and self.has_edit_requests(), ["Member", "Domain requestor"]), + ( + self.has_base_portfolio_permission() and self.has_domains_portfolio_permission(), + ["Member", "Domain manager"], + ), + (self.has_base_portfolio_permission(), ["Member"]), + ] + + # Evaluate conditions and add roles + for condition, role_list in conditions_roles: + if condition: + roles.extend(role_list) + break + + return roles + @classmethod def needs_identity_verification(cls, email, uuid): """A method used by our oidc classes to test whether a user needs email/uuid verification diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index 683f33117..5da6c2ca2 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -137,6 +137,10 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% endfor %} {% endwith %} + {% elif field.field.name == "display_admins" %} +
    {{ field.contents|safe }}
    + {% elif field.field.name == "display_members" %} +
    {{ display_members_summary }}
    {% else %}
    {{ field.contents }}
    {% endif %} @@ -240,6 +244,13 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% endif %} {% endwith %} + {% elif field.field.name == "display_members" %} +
    + Details +
    + {{ field.contents|safe }} +
    +
    {% elif field.field.name == "state_territory" %}
    diff --git a/src/registrar/templates/django/admin/portfolio_change_form.html b/src/registrar/templates/django/admin/portfolio_change_form.html index 9d59aae42..a2d70611c 100644 --- a/src/registrar/templates/django/admin/portfolio_change_form.html +++ b/src/registrar/templates/django/admin/portfolio_change_form.html @@ -14,8 +14,7 @@ This is a placeholder for now. Disclaimer: - When extending the fieldset view - *make a new one* that extends from detail_table_fieldset. - For instance, "portfolio_fieldset.html". + 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/detail_table_fieldset.html" with original_object=original %} diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 827742ef1..7ef1a4a97 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -45,6 +45,7 @@ from registrar.models import ( from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.models.senior_official import SeniorOfficial from registrar.models.user_domain_role import UserDomainRole +from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from registrar.models.verified_by_staff import VerifiedByStaff from .common import ( MockDbForSharedTests, @@ -2066,6 +2067,7 @@ class TestPortfolioAdmin(TestCase): DomainRequest.objects.all().delete() Domain.objects.all().delete() Portfolio.objects.all().delete() + User.objects.all().delete() @less_console_noise_decorator def test_created_on_display(self): @@ -2121,3 +2123,81 @@ class TestPortfolioAdmin(TestCase): self.assertIn("request1.gov", domain_requests) self.assertIn("request2.gov", domain_requests) self.assertIn('
    @@ -25,7 +25,7 @@ Template for an input field with a clipboard
    {% endif %} \ No newline at end of file diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 02cd6e0f6..93e611c1a 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -45,6 +45,7 @@ from registrar.models import ( from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.models.senior_official import SeniorOfficial from registrar.models.user_domain_role import UserDomainRole +from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from registrar.models.verified_by_staff import VerifiedByStaff from .common import ( @@ -2129,8 +2130,10 @@ class TestPortfolioAdmin(TestCase): last_name="Meoward", title="Captain", email="meaoward@gov.gov", - portfolio=self.portfolio, - portfolio_roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + ) + + UserPortfolioPermission.objects.all().create( + user=admin_user_1, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] ) admin_user_2 = User.objects.create( @@ -2139,8 +2142,10 @@ class TestPortfolioAdmin(TestCase): last_name="Poopy", title="Major", email="poopy@gov.gov", - portfolio=self.portfolio, - portfolio_roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + ) + + UserPortfolioPermission.objects.all().create( + user=admin_user_2, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] ) admin_user_3 = User.objects.create( @@ -2149,8 +2154,10 @@ class TestPortfolioAdmin(TestCase): last_name="Max", title="Road warrior", email="madmax@gov.gov", - portfolio=self.portfolio, - portfolio_roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + ) + + UserPortfolioPermission.objects.all().create( + user=admin_user_3, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER] ) admin_user_4 = User.objects.create( @@ -2159,8 +2166,12 @@ class TestPortfolioAdmin(TestCase): last_name="Smith", title="Program", email="thematrix@gov.gov", + ) + + UserPortfolioPermission.objects.all().create( + user=admin_user_4, portfolio=self.portfolio, - portfolio_additional_permissions=[ + additional_permissions=[ UserPortfolioPermissionChoices.VIEW_PORTFOLIO, UserPortfolioPermissionChoices.EDIT_REQUESTS, ], From ef2adaaf288e7a386ce67e2cbec0b5632ed8fcad Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 30 Aug 2024 17:25:56 -0400 Subject: [PATCH 32/37] No additional members found --- .../templates/django/admin/includes/detail_table_fieldset.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index da2753b4c..7eea7b72b 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -144,7 +144,7 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% if display_members_summary %} {{ display_members_summary }} {% else %} -

    No members found.

    +

    No additional members found.

    {% endif %} {% else %} From ca1c9bbb0424313425848f5e004b64968994112b Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 30 Aug 2024 17:35:57 -0400 Subject: [PATCH 33/37] fix model tests? --- src/registrar/tests/test_models.py | 39 +++++++++++++++--------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 19f9d46c3..3510ff65e 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1308,6 +1308,7 @@ class TestUser(TestCase): self.domain, _ = Domain.objects.get_or_create(name="igorville.gov") self.user, _ = User.objects.get_or_create(email=self.email) self.factory = RequestFactory() + self.portfolio = Portfolio.objects.create(organization_name="Test Portfolio", creator=self.user) def tearDown(self): super().tearDown() @@ -1325,57 +1326,57 @@ class TestUser(TestCase): @patch.object(User, "has_edit_suborganization", return_value=True) def test_portfolio_role_summary_admin(self, mock_edit_suborganization): # Test if the user is recognized as an Admin - self.assertEqual(self.user.portfolio_role_summary, ["Admin"]) + self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Admin"]) @patch.multiple( User, - has_view_all_domains_permission=lambda self: True, - has_domain_requests_portfolio_permission=lambda self: True, - has_edit_requests=lambda self: True, + has_view_all_domains_permission=lambda self, portfolio: True, + has_domain_requests_portfolio_permission=lambda self, portfolio: True, + has_edit_requests=lambda self, portfolio: True, ) def test_portfolio_role_summary_view_only_admin_and_domain_requestor(self): # Test if the user has both 'View-only admin' and 'Domain requestor' roles - self.assertEqual(self.user.portfolio_role_summary, ["View-only admin", "Domain requestor"]) + self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["View-only admin", "Domain requestor"]) @patch.multiple( User, - has_view_all_domains_permission=lambda self: True, - has_domain_requests_portfolio_permission=lambda self: True, + has_view_all_domains_permission=lambda self, portfolio: True, + has_domain_requests_portfolio_permission=lambda self, portfolio: True, ) def test_portfolio_role_summary_view_only_admin(self): # Test if the user is recognized as a View-only admin - self.assertEqual(self.user.portfolio_role_summary, ["View-only admin"]) + self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["View-only admin"]) @patch.multiple( User, - has_base_portfolio_permission=lambda self: True, - has_edit_requests=lambda self: True, - has_domains_portfolio_permission=lambda self: True, + has_base_portfolio_permission=lambda self, portfolio: True, + has_edit_requests=lambda self, portfolio: True, + has_domains_portfolio_permission=lambda self, portfolio: True, ) def test_portfolio_role_summary_member_domain_requestor_domain_manager(self): # Test if the user has 'Member', 'Domain requestor', and 'Domain manager' roles - self.assertEqual(self.user.portfolio_role_summary, ["Domain requestor", "Domain manager"]) + self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Domain requestor", "Domain manager"]) - @patch.multiple(User, has_base_portfolio_permission=lambda self: True, has_edit_requests=lambda self: True) + @patch.multiple(User, has_base_portfolio_permission=lambda self, portfolio: True, has_edit_requests=lambda self, portfolio: True) def test_portfolio_role_summary_member_domain_requestor(self): # Test if the user has 'Member' and 'Domain requestor' roles - self.assertEqual(self.user.portfolio_role_summary, ["Domain requestor"]) + self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Domain requestor"]) @patch.multiple( - User, has_base_portfolio_permission=lambda self: True, has_domains_portfolio_permission=lambda self: True + User, has_base_portfolio_permission=lambda self, portfolio: True, has_domains_portfolio_permission=lambda self, portfolio: True ) def test_portfolio_role_summary_member_domain_manager(self): # Test if the user has 'Member' and 'Domain manager' roles - self.assertEqual(self.user.portfolio_role_summary, ["Domain manager"]) + self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Domain manager"]) - @patch.multiple(User, has_base_portfolio_permission=lambda self: True) + @patch.multiple(User, has_base_portfolio_permission=lambda self, portfolio: True) def test_portfolio_role_summary_member(self): # Test if the user is recognized as a Member - self.assertEqual(self.user.portfolio_role_summary, ["Member"]) + self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Member"]) def test_portfolio_role_summary_empty(self): # Test if the user has no roles - self.assertEqual(self.user.portfolio_role_summary, []) + self.assertEqual(self.user.portfolio_role_summary(self.portfolio), []) @less_console_noise_decorator def test_check_transition_domains_without_domains_on_login(self): From e20bfa55678ee12c7ba897ecf52f4c3cad0ad84b Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 30 Aug 2024 17:44:09 -0400 Subject: [PATCH 34/37] lint --- src/registrar/admin.py | 6 +----- src/registrar/tests/test_models.py | 8 ++++++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index a47617c28..c8be04715 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -3085,11 +3085,7 @@ class PortfolioAdmin(ListHeaderAdmin): + "PhoneRoles" ) for member in members: - full_name = ( - f"{member.first_name} {member.middle_name} {member.last_name}" - if member.middle_name - else f"{member.first_name} {member.last_name}" - ) + full_name = member.get_formatted_name() member_details += "" member_details += f"{full_name}" member_details += f"{member.title}" diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 3510ff65e..7983fed36 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1357,13 +1357,17 @@ class TestUser(TestCase): # Test if the user has 'Member', 'Domain requestor', and 'Domain manager' roles self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Domain requestor", "Domain manager"]) - @patch.multiple(User, has_base_portfolio_permission=lambda self, portfolio: True, has_edit_requests=lambda self, portfolio: True) + @patch.multiple( + User, has_base_portfolio_permission=lambda self, portfolio: True, has_edit_requests=lambda self, portfolio: True + ) def test_portfolio_role_summary_member_domain_requestor(self): # Test if the user has 'Member' and 'Domain requestor' roles self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Domain requestor"]) @patch.multiple( - User, has_base_portfolio_permission=lambda self, portfolio: True, has_domains_portfolio_permission=lambda self, portfolio: True + User, + has_base_portfolio_permission=lambda self, portfolio: True, + has_domains_portfolio_permission=lambda self, portfolio: True, ) def test_portfolio_role_summary_member_domain_manager(self): # Test if the user has 'Member' and 'Domain manager' roles From 79fbc5c54026c7516e39347a7d7be50d0f9a6263 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 30 Aug 2024 17:46:05 -0400 Subject: [PATCH 35/37] cleanup --- src/registrar/admin.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index c8be04715..fd32238f7 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2943,11 +2943,6 @@ class PortfolioAdmin(ListHeaderAdmin): # created_on is the created_at field, and portfolio_type is f"{organization_type} - {federal_type}" (None, {"fields": ["portfolio_type", "organization_name", "creator", "created_on", "notes"]}), ("Portfolio members", {"fields": ["display_admins", "display_members"]}), - # TODO - uncomment in #2521 - # ("Portfolio members", { - # "classes": ("collapse", "closed"), - # "fields": ["administrators", "members"]} - # ), ("Portfolio domains", {"fields": ["domains", "domain_requests"]}), ("Type of organization", {"fields": ["organization_type", "federal_type"]}), ( @@ -2995,15 +2990,14 @@ class PortfolioAdmin(ListHeaderAdmin): readonly_fields = [ # This is the created_at field "created_on", - # Custom fields such as these must be defined as readonly. + # Django admin doesn't allow methods to be directly listed in fieldsets. We can + # display the custom methods display_admins amd display_members in the admin form if + # they are readonly. "federal_type", "domains", "domain_requests", "suborganizations", "portfolio_type", - # Django admin doesn't allow methods to be directly listed in fieldsets. We can - # display the custom methods display_admins amd display_members in the admin form if - # they are readonly. "display_admins", "display_members", "creator", From cba2b27d79122951ee67c680a62021a72beebe48 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 4 Sep 2024 13:44:45 -0400 Subject: [PATCH 36/37] escape dynamic values --- src/registrar/admin.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 4b36c9589..e3bd5c9f7 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -3068,11 +3068,11 @@ class PortfolioAdmin(ListHeaderAdmin): for portfolio_admin in admins: change_url = reverse("admin:registrar_user_change", args=[portfolio_admin.pk]) admin_details += "
    " - admin_details += f'{portfolio_admin}
    ' - admin_details += f"{portfolio_admin.title}
    " - admin_details += f"{portfolio_admin.email}" + admin_details += f'    {escape(portfolio_admin)}
    ' + admin_details += f"{escape(portfolio_admin.title)}
    " + admin_details += f"{escape(portfolio_admin.email)}" admin_details += "
    " - admin_details += f"" + admin_details += f"" admin_details += ( "" admin_details += "

    " - admin_details += f"{portfolio_admin.phone}" + admin_details += f"{escape(portfolio_admin.phone)}" admin_details += "
    " return format_html(admin_details) @@ -3108,13 +3108,13 @@ class PortfolioAdmin(ListHeaderAdmin): for member in members: full_name = member.get_formatted_name() member_details += "" - member_details += f"{full_name}" - member_details += f"{member.title}" - member_details += f"{member.email}" - member_details += f"{member.phone}" + member_details += f"{escape(full_name)}" + member_details += f"{escape(member.title)}" + member_details += f"{escape(member.email)}" + member_details += f"{escape(member.phone)}" member_details += "" for role in member.portfolio_role_summary(obj): - member_details += f"{role} " + member_details += f"{escape(role)} " member_details += "" member_details += "" return format_html(member_details) From a6b9d79fad37af6173f89942df95dd48d2df09ba Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 5 Sep 2024 12:07:57 -0400 Subject: [PATCH 37/37] cleanup comments --- src/registrar/assets/js/get-gov.js | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 7b676eeb3..e6b5a039f 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -2044,17 +2044,9 @@ document.addEventListener('DOMContentLoaded', function() { // Use a MutationObserver to watch for changes in the dropdown list const dropdownList = comboBox.querySelector(`#${input.id}--list`); - // TODO: customize blank value - // const isSuborgComboBox = comboBox.querySelector('#id_sub_organization--list'); const observer = new MutationObserver(function(mutations) { mutations.forEach(function(mutation) { if (mutation.type === "childList") { - // TODO: customize blank value - // if (isSuborgComboBox) { - // addBlankOption(clearInputButton, dropdownList, initialValue, 'No suborganization'); - // } else { - // addBlankOption(clearInputButton, dropdownList, initialValue); - // } addBlankOption(clearInputButton, dropdownList, initialValue); } }); @@ -2110,8 +2102,6 @@ document.addEventListener('DOMContentLoaded', function() { } } - // TODO: customize blank value - // function addBlankOption(clearInputButton, dropdownList, initialValue, customBlank) { function addBlankOption(clearInputButton, dropdownList, initialValue) { if (dropdownList && !dropdownList.querySelector('[data-value=""]') && !isTyping) { const blankOption = document.createElement("li"); @@ -2121,8 +2111,6 @@ document.addEventListener('DOMContentLoaded', function() { if (!initialValue){ blankOption.classList.add("usa-combo-box__list-option--selected") } - // TODO: customize blank value - // customBlank ? blankOption.textContent = customBlank : blankOption.textContent = "---------"; blankOption.textContent = "⎯"; dropdownList.insertBefore(blankOption, dropdownList.firstChild);