From 1ebff9fc45560c27a98051a2e644b5b5d7f98f52 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 23 Aug 2024 10:45:32 -0400 Subject: [PATCH 1/9] some incremental changes --- src/registrar/admin.py | 47 ++++++++++++++++--- .../templates/admin/stacked_no_heading.html | 42 +++++++++++++++++ .../includes/domain_info_inline_stacked.html | 2 +- 3 files changed, 84 insertions(+), 7 deletions(-) create mode 100644 src/registrar/templates/admin/stacked_no_heading.html diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 3ad5e3ea0..5b55eee12 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2272,6 +2272,17 @@ class DomainInformationInline(admin.StackedInline): analyst_readonly_fields = DomainInformationAdmin.analyst_readonly_fields autocomplete_fields = DomainInformationAdmin.autocomplete_fields + # Removing specific fields from the first fieldset dynamically + fieldsets[0][1]["fields"] = [ + field for field in fieldsets[0][1]["fields"] if field not in ["creator", "submitter", "domain_request", "notes"] + ] + fieldsets[2][1]["fields"] = [ + field for field in fieldsets[2][1]["fields"] if field not in ["other_contacts", "no_other_contacts_rationale"] + ] + fieldsets[3][1]["fields"].extend(["other_contacts", "no_other_contacts_rationale"]) # type: ignore + fieldsets_to_move = fieldsets.pop(3) + fieldsets.append(fieldsets_to_move) + def has_change_permission(self, request, obj=None): """Custom has_change_permission override so that we can specify that analysts can edit this through this inline, but not through the model normally""" @@ -2382,14 +2393,11 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): fieldsets = ( ( - None, - {"fields": ["name", "state", "expiration_date", "first_ready", "deleted"]}, + "Domain Information", + {"fields": ["state", "expiration_date", "first_ready", "deleted", "dnssecdata", "nameservers"]}, ), ) - # this ordering effects the ordering of results in autocomplete_fields for domain - ordering = ["name"] - def generic_org_type(self, obj): return obj.domain_info.get_generic_org_type_display() @@ -2410,6 +2418,25 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): organization_name.admin_order_field = "domain_info__organization_name" # type: ignore + def dnssecdata(self, obj): + return "Yes" if obj.dnssecdata else "No" + + dnssecdata.short_description = "DNS Sec Enabled" # type: ignore + + # Custom method to display formatted nameservers + def nameservers(self, obj): + if not obj.nameservers: + return "No nameservers" + + formatted_nameservers = [] + for server, ip_list in obj.nameservers: + formatted_nameservers.append(f"{server} [{', '.join(ip_list)}]") + + # Join the formatted strings with line breaks + return "\n".join(formatted_nameservers) + + nameservers.short_description = "Nameservers" # type: ignore + def custom_election_board(self, obj): domain_info = getattr(obj, "domain_info", None) if domain_info: @@ -2436,7 +2463,15 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): search_fields = ["name"] search_help_text = "Search by domain name." change_form_template = "django/admin/domain_change_form.html" - readonly_fields = ("state", "expiration_date", "first_ready", "deleted", "federal_agency") + readonly_fields = ( + "state", + "expiration_date", + "first_ready", + "deleted", + "federal_agency", + "dnssecdata", + "nameservers", + ) # Table ordering ordering = ["name"] diff --git a/src/registrar/templates/admin/stacked_no_heading.html b/src/registrar/templates/admin/stacked_no_heading.html new file mode 100644 index 000000000..60e37d757 --- /dev/null +++ b/src/registrar/templates/admin/stacked_no_heading.html @@ -0,0 +1,42 @@ +{% load i18n admin_urls %} +{% load i18n static %} + +{% comment %} +This is copied from Djangos implementation of this template, with added "blocks" +It is not inherently customizable on its own, so we can modify this instead. +https://github.com/django/django/blob/main/django/contrib/admin/templates/admin/edit_inline/stacked.html +{% endcomment %} + +
+ +
+ {{ inline_admin_formset.formset.management_form }} + {{ inline_admin_formset.formset.non_form_errors }} + + {% for inline_admin_form in inline_admin_formset %}
+ {% if inline_admin_form.form.non_field_errors %} + {{ inline_admin_form.form.non_field_errors }} + {% endif %} + + {% for fieldset in inline_admin_form %} + {# .gov override #} + {% block fieldset %} + {% include "admin/includes/fieldset.html" %} + {% endblock fieldset%} + {# End of .gov override #} + {% endfor %} + + {% if inline_admin_form.needs_explicit_pk_field %} + {{ inline_admin_form.pk_field.field }} + {% endif %} + {% if inline_admin_form.fk_field %} + {{ inline_admin_form.fk_field.field }} + {% endif %} +
+ {% endfor %} +
+ +
diff --git a/src/registrar/templates/django/admin/includes/domain_info_inline_stacked.html b/src/registrar/templates/django/admin/includes/domain_info_inline_stacked.html index 414c485e5..9638bb7cb 100644 --- a/src/registrar/templates/django/admin/includes/domain_info_inline_stacked.html +++ b/src/registrar/templates/django/admin/includes/domain_info_inline_stacked.html @@ -1,4 +1,4 @@ -{% extends 'admin/stacked.html' %} +{% extends 'admin/stacked_no_heading.html' %} {% load i18n static %} {% block fieldset %} From bec50b4e08c8629eac47d83485483818f66c20b5 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 17 Sep 2024 10:54:29 -0400 Subject: [PATCH 2/9] checking in updates --- src/registrar/admin.py | 72 +++++++++- .../admin/includes/detail_table_fieldset.html | 2 +- .../includes/domain_info_inline_stacked.html | 2 +- src/registrar/tests/test_admin_domain.py | 125 ++++++++++++++++++ 4 files changed, 195 insertions(+), 6 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index c1b90083e..84649913d 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -11,6 +11,7 @@ from django.conf import settings from django.shortcuts import redirect from django_fsm import get_available_FIELD_transitions, FSMField from registrar.models.domain_information import DomainInformation +from registrar.models.domain_invitation import DomainInvitation from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from waffle.decorators import flag_is_active @@ -2340,11 +2341,12 @@ class DomainInformationInline(admin.StackedInline): template = "django/admin/includes/domain_info_inline_stacked.html" model = models.DomainInformation - fieldsets = DomainInformationAdmin.fieldsets - readonly_fields = DomainInformationAdmin.readonly_fields + fieldsets = list(DomainInformationAdmin.fieldsets) + readonly_fields = list(DomainInformationAdmin.readonly_fields) analyst_readonly_fields = DomainInformationAdmin.analyst_readonly_fields autocomplete_fields = DomainInformationAdmin.autocomplete_fields + readonly_fields.extend(["domain_managers", "invited_domain_managers"]) # type: ignore # Removing specific fields from the first fieldset dynamically fieldsets[0][1]["fields"] = [ field for field in fieldsets[0][1]["fields"] if field not in ["creator", "submitter", "domain_request", "notes"] @@ -2352,9 +2354,71 @@ class DomainInformationInline(admin.StackedInline): fieldsets[2][1]["fields"] = [ field for field in fieldsets[2][1]["fields"] if field not in ["other_contacts", "no_other_contacts_rationale"] ] - fieldsets[3][1]["fields"].extend(["other_contacts", "no_other_contacts_rationale"]) # type: ignore + fieldsets[2][1]["fields"].extend(["domain_managers", "invited_domain_managers"]) # type: ignore fieldsets_to_move = fieldsets.pop(3) fieldsets.append(fieldsets_to_move) + + + def get_domain_managers(self, obj): + user_domain_roles = UserDomainRole.objects.filter(domain=obj.domain) + user_ids = user_domain_roles.values_list("user_id", flat=True) + domain_managers = User.objects.filter(id__in=user_ids) + return domain_managers + + def get_domain_invitations(self, obj): + domain_invitations = DomainInvitation.objects.filter( + domain=obj.domain, status=DomainInvitation.DomainInvitationStatus.INVITED + ) + return domain_invitations + + def domain_managers(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.""" + domain_managers = self.get_domain_managers(obj) + if not domain_managers: + return "No domain managers found." + + domain_manager_details = "" + "" + for domain_manager in domain_managers: + full_name = domain_manager.get_formatted_name() + change_url = reverse("admin:registrar_user_change", args=[domain_manager.pk]) + domain_manager_details += "" + domain_manager_details += f'" + domain_manager_details += f"" + domain_manager_details += "" + domain_manager_details += "
UIDNameEmail
{escape(domain_manager.username)}' + domain_manager_details += f"{escape(full_name)}{escape(domain_manager.email)}
" + return format_html(domain_manager_details) + + domain_managers.short_description = "Domain Managers" # type: ignore + + def invited_domain_managers(self, obj): + """Get emails which have been invited to the domain, 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.""" + domain_invitations = self.get_domain_invitations(obj) + if not domain_invitations: + return "No invited domain managers found." + + domain_invitation_details = "" + "" + for domain_invitation in domain_invitations: + domain_invitation_details += "" + domain_invitation_details += f"" + domain_invitation_details += f"" + domain_invitation_details += "" + domain_invitation_details += "
EmailStatus
{escape(domain_invitation.email)}{escape(domain_invitation.status.capitalize())}
" + return format_html(domain_invitation_details) + + invited_domain_managers.short_description = "Invited Domain Managers" # type: ignore def has_change_permission(self, request, obj=None): """Custom has_change_permission override so that we can specify that @@ -2466,7 +2530,7 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): fieldsets = ( ( - "Domain Information", + None, {"fields": ["state", "expiration_date", "first_ready", "deleted", "dnssecdata", "nameservers"]}, ), ) 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 e22bcb571..a02162167 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -137,7 +137,7 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% endfor %} {% endwith %} - {% elif field.field.name == "display_admins" %} + {% elif field.field.name == "display_admins" or field.field.name == "domain_managers" or field.field.namd == "invited_domain_managers" %}
{{ field.contents|safe }}
{% elif field.field.name == "display_members" %}
diff --git a/src/registrar/templates/django/admin/includes/domain_info_inline_stacked.html b/src/registrar/templates/django/admin/includes/domain_info_inline_stacked.html index 9638bb7cb..414c485e5 100644 --- a/src/registrar/templates/django/admin/includes/domain_info_inline_stacked.html +++ b/src/registrar/templates/django/admin/includes/domain_info_inline_stacked.html @@ -1,4 +1,4 @@ -{% extends 'admin/stacked_no_heading.html' %} +{% extends 'admin/stacked.html' %} {% load i18n static %} {% block fieldset %} diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py index 49f095a25..bfbdad81d 100644 --- a/src/registrar/tests/test_admin_domain.py +++ b/src/registrar/tests/test_admin_domain.py @@ -334,6 +334,131 @@ class TestDomainAdminAsStaff(MockEppLib): domain.delete() +class TestDomainInformationInline(MockEppLib): + """Test DomainAdmin class, specifically the DomainInformationInline class, as staff user. + + Notes: + all tests share staffuser; do not change staffuser model in tests + tests have available staffuser, client, and admin + """ + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.staffuser = create_user() + cls.site = AdminSite() + cls.admin = DomainAdmin(model=Domain, admin_site=cls.site) + cls.factory = RequestFactory() + + def setUp(self): + self.client = Client(HTTP_HOST="localhost:8080") + self.client.force_login(self.staffuser) + super().setUp() + + 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() + + @classmethod + def tearDownClass(cls): + User.objects.all().delete() + super().tearDownClass() + + @less_console_noise_decorator + def test_domain_managers_display(self): + """Tests the custom domain managers field""" + admin_user_1 = User.objects.create( + username="testuser1", + first_name="Gerald", + last_name="Meoward", + email="meoward@gov.gov", + ) + + domain_request = completed_domain_request( + status=DomainRequest.DomainRequestStatus.IN_REVIEW, user=self.staffuser, name="fake.gov" + ) + domain_request.approve() + _domain_info = DomainInformation.objects.filter(domain=domain_request.approved_domain).get() + domain = Domain.objects.filter(domain_info=_domain_info).get() + + UserDomainRole.objects.get_or_create(user=admin_user_1, domain=domain, role=UserDomainRole.Roles.MANAGER) + + admin_user_2 = User.objects.create( + username="testuser2", + first_name="Arnold", + last_name="Poopy", + email="poopy@gov.gov", + ) + + UserDomainRole.objects.get_or_create(user=admin_user_2, domain=domain, role=UserDomainRole.Roles.MANAGER) + + # Get the first inline (DomainInformationInline) + inline_instance = self.admin.inlines[0](self.admin.model, self.admin.admin_site) + + # Call the domain_managers method + domain_managers = inline_instance.domain_managers(domain.domain_info) + + self.assertIn( + f'testuser1', + domain_managers, + ) + self.assertIn("Gerald Meoward", domain_managers) + self.assertIn("meoward@gov.gov", domain_managers) + self.assertIn(f'testuser2', domain_managers) + self.assertIn("Arnold Poopy", domain_managers) + self.assertIn("poopy@gov.gov", domain_managers) + + @less_console_noise_decorator + def test_invited_domain_managers_display(self): + """Tests the custom invited domain managers field""" + admin_user_1 = User.objects.create( + username="testuser1", + first_name="Gerald", + last_name="Meoward", + email="meoward@gov.gov", + ) + + domain_request = completed_domain_request( + status=DomainRequest.DomainRequestStatus.IN_REVIEW, user=self.staffuser, name="fake.gov" + ) + domain_request.approve() + _domain_info = DomainInformation.objects.filter(domain=domain_request.approved_domain).get() + domain = Domain.objects.filter(domain_info=_domain_info).get() + + # domain, _ = Domain.objects.get_or_create(name="fake.gov", state=Domain.State.READY) + UserDomainRole.objects.get_or_create(user=admin_user_1, domain=domain, role=UserDomainRole.Roles.MANAGER) + + admin_user_2 = User.objects.create( + username="testuser2", + first_name="Arnold", + last_name="Poopy", + email="poopy@gov.gov", + ) + + UserDomainRole.objects.get_or_create(user=admin_user_2, domain=domain, role=UserDomainRole.Roles.MANAGER) + + # Get the first inline (DomainInformationInline) + inline_instance = self.admin.inlines[0](self.admin.model, self.admin.admin_site) + + # Call the domain_managers method + domain_managers = inline_instance.domain_managers(domain.domain_info) + # domain_managers = self.admin.get_inlinesdomain_managers(self.domain) + + self.assertIn( + f'testuser1', + domain_managers, + ) + self.assertIn("Gerald Meoward", domain_managers) + self.assertIn("meoward@gov.gov", domain_managers) + self.assertIn(f'testuser2', domain_managers) + self.assertIn("Arnold Poopy", domain_managers) + self.assertIn("poopy@gov.gov", domain_managers) + + class TestDomainAdminWithClient(TestCase): """Test DomainAdmin class as super user. From a772678d76a2e7055950f2eb0099bd8010ac7c47 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 20 Sep 2024 07:52:54 -0400 Subject: [PATCH 3/9] added values to readonly_fields in DomainInformationAdmin and redundant method definitions --- src/registrar/admin.py | 73 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 69 insertions(+), 4 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 84649913d..ac9f01925 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1511,7 +1511,7 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): ] # Readonly fields for analysts and superusers - readonly_fields = ("other_contacts", "is_election_board") + readonly_fields = ("other_contacts", "is_election_board", "domain_managers", "invited_domain_managers") # Read only that we'll leverage for CISA Analysts analyst_readonly_fields = [ @@ -1606,6 +1606,70 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): return super().formfield_for_foreignkey(db_field, request, use_admin_sort_fields=use_sort, **kwargs) + def get_domain_managers(self, obj): + user_domain_roles = UserDomainRole.objects.filter(domain=obj.domain) + user_ids = user_domain_roles.values_list("user_id", flat=True) + domain_managers = User.objects.filter(id__in=user_ids) + return domain_managers + + def get_domain_invitations(self, obj): + domain_invitations = DomainInvitation.objects.filter( + domain=obj.domain, status=DomainInvitation.DomainInvitationStatus.INVITED + ) + return domain_invitations + + @admin.display(description='Domain managers') + def domain_managers(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.""" + domain_managers = self.get_domain_managers(obj) + if not domain_managers: + return "No domain managers found." + + domain_manager_details = "" + "" + for domain_manager in domain_managers: + full_name = domain_manager.get_formatted_name() + change_url = reverse("admin:registrar_user_change", args=[domain_manager.pk]) + domain_manager_details += "" + domain_manager_details += f'" + domain_manager_details += f"" + domain_manager_details += "" + domain_manager_details += "
UIDNameEmail
{escape(domain_manager.username)}' + domain_manager_details += f"{escape(full_name)}{escape(domain_manager.email)}
" + return format_html(domain_manager_details) + + domain_managers.short_description = "Domain managers" # type: ignore + + @admin.display(description='Invited domain managers') + def invited_domain_managers(self, obj): + """Get emails which have been invited to the domain, 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.""" + domain_invitations = self.get_domain_invitations(obj) + if not domain_invitations: + return "No invited domain managers found." + + domain_invitation_details = "" + "" + for domain_invitation in domain_invitations: + domain_invitation_details += "" + domain_invitation_details += f"" + domain_invitation_details += f"" + domain_invitation_details += "" + domain_invitation_details += "
EmailStatus
{escape(domain_invitation.email)}{escape(domain_invitation.status.capitalize())}
" + return format_html(domain_invitation_details) + + invited_domain_managers.short_description = "Invited domain managers" # type: ignore + + class DomainRequestResource(FsmModelResource): """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" @@ -2358,7 +2422,6 @@ class DomainInformationInline(admin.StackedInline): fieldsets_to_move = fieldsets.pop(3) fieldsets.append(fieldsets_to_move) - def get_domain_managers(self, obj): user_domain_roles = UserDomainRole.objects.filter(domain=obj.domain) user_ids = user_domain_roles.values_list("user_id", flat=True) @@ -2371,6 +2434,7 @@ class DomainInformationInline(admin.StackedInline): ) return domain_invitations + @admin.display(description='Domain managers') def domain_managers(self, obj): """Get joined users who have roles/perms that are not Admin, unpack and return an HTML block. @@ -2395,8 +2459,9 @@ class DomainInformationInline(admin.StackedInline): domain_manager_details += "" return format_html(domain_manager_details) - domain_managers.short_description = "Domain Managers" # type: ignore + domain_managers.short_description = "Domain managers" # type: ignore + @admin.display(description='Invited domain managers') def invited_domain_managers(self, obj): """Get emails which have been invited to the domain, unpack and return an HTML block. @@ -2418,7 +2483,7 @@ class DomainInformationInline(admin.StackedInline): domain_invitation_details += "" return format_html(domain_invitation_details) - invited_domain_managers.short_description = "Invited Domain Managers" # type: ignore + invited_domain_managers.short_description = "Invited domain managers" # type: ignore def has_change_permission(self, request, obj=None): """Custom has_change_permission override so that we can specify that From bf4df511bdeb1ccf0f7b24f8730409d4b1461f2d Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 20 Sep 2024 12:27:47 -0400 Subject: [PATCH 4/9] working properly --- src/registrar/admin.py | 117 ++++++----------------- src/registrar/tests/test_admin_domain.py | 31 ------ 2 files changed, 28 insertions(+), 120 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ac9f01925..f3287be27 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1511,7 +1511,7 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): ] # Readonly fields for analysts and superusers - readonly_fields = ("other_contacts", "is_election_board", "domain_managers", "invited_domain_managers") + readonly_fields = ("other_contacts", "is_election_board") # Read only that we'll leverage for CISA Analysts analyst_readonly_fields = [ @@ -1606,70 +1606,6 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): return super().formfield_for_foreignkey(db_field, request, use_admin_sort_fields=use_sort, **kwargs) - def get_domain_managers(self, obj): - user_domain_roles = UserDomainRole.objects.filter(domain=obj.domain) - user_ids = user_domain_roles.values_list("user_id", flat=True) - domain_managers = User.objects.filter(id__in=user_ids) - return domain_managers - - def get_domain_invitations(self, obj): - domain_invitations = DomainInvitation.objects.filter( - domain=obj.domain, status=DomainInvitation.DomainInvitationStatus.INVITED - ) - return domain_invitations - - @admin.display(description='Domain managers') - def domain_managers(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.""" - domain_managers = self.get_domain_managers(obj) - if not domain_managers: - return "No domain managers found." - - domain_manager_details = "" + "" - for domain_manager in domain_managers: - full_name = domain_manager.get_formatted_name() - change_url = reverse("admin:registrar_user_change", args=[domain_manager.pk]) - domain_manager_details += "" - domain_manager_details += f'" - domain_manager_details += f"" - domain_manager_details += "" - domain_manager_details += "
UIDNameEmail
{escape(domain_manager.username)}' - domain_manager_details += f"{escape(full_name)}{escape(domain_manager.email)}
" - return format_html(domain_manager_details) - - domain_managers.short_description = "Domain managers" # type: ignore - - @admin.display(description='Invited domain managers') - def invited_domain_managers(self, obj): - """Get emails which have been invited to the domain, 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.""" - domain_invitations = self.get_domain_invitations(obj) - if not domain_invitations: - return "No invited domain managers found." - - domain_invitation_details = "" + "" - for domain_invitation in domain_invitations: - domain_invitation_details += "" - domain_invitation_details += f"" - domain_invitation_details += f"" - domain_invitation_details += "" - domain_invitation_details += "
EmailStatus
{escape(domain_invitation.email)}{escape(domain_invitation.status.capitalize())}
" - return format_html(domain_invitation_details) - - invited_domain_managers.short_description = "Invited domain managers" # type: ignore - - class DomainRequestResource(FsmModelResource): """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" @@ -2405,23 +2341,10 @@ class DomainInformationInline(admin.StackedInline): template = "django/admin/includes/domain_info_inline_stacked.html" model = models.DomainInformation - fieldsets = list(DomainInformationAdmin.fieldsets) - readonly_fields = list(DomainInformationAdmin.readonly_fields) - analyst_readonly_fields = DomainInformationAdmin.analyst_readonly_fields - autocomplete_fields = DomainInformationAdmin.autocomplete_fields + fieldsets = copy.deepcopy(list(DomainInformationAdmin.fieldsets)) + analyst_readonly_fields = copy.deepcopy(DomainInformationAdmin.analyst_readonly_fields) + autocomplete_fields = copy.deepcopy(DomainInformationAdmin.autocomplete_fields) - readonly_fields.extend(["domain_managers", "invited_domain_managers"]) # type: ignore - # Removing specific fields from the first fieldset dynamically - fieldsets[0][1]["fields"] = [ - field for field in fieldsets[0][1]["fields"] if field not in ["creator", "submitter", "domain_request", "notes"] - ] - fieldsets[2][1]["fields"] = [ - field for field in fieldsets[2][1]["fields"] if field not in ["other_contacts", "no_other_contacts_rationale"] - ] - fieldsets[2][1]["fields"].extend(["domain_managers", "invited_domain_managers"]) # type: ignore - fieldsets_to_move = fieldsets.pop(3) - fieldsets.append(fieldsets_to_move) - def get_domain_managers(self, obj): user_domain_roles = UserDomainRole.objects.filter(domain=obj.domain) user_ids = user_domain_roles.values_list("user_id", flat=True) @@ -2434,7 +2357,6 @@ class DomainInformationInline(admin.StackedInline): ) return domain_invitations - @admin.display(description='Domain managers') def domain_managers(self, obj): """Get joined users who have roles/perms that are not Admin, unpack and return an HTML block. @@ -2461,7 +2383,6 @@ class DomainInformationInline(admin.StackedInline): domain_managers.short_description = "Domain managers" # type: ignore - @admin.display(description='Invited domain managers') def invited_domain_managers(self, obj): """Get emails which have been invited to the domain, unpack and return an HTML block. @@ -2523,7 +2444,9 @@ class DomainInformationInline(admin.StackedInline): return super().formfield_for_foreignkey(db_field, request, **kwargs) def get_readonly_fields(self, request, obj=None): - return DomainInformationAdmin.get_readonly_fields(self, request, obj=None) + readonly_fields = copy.deepcopy(DomainInformationAdmin.get_readonly_fields(self, request, obj=None)) + readonly_fields.extend(["domain_managers", "invited_domain_managers"]) # type: ignore + return readonly_fields # Re-route the get_fieldsets method to utilize DomainInformationAdmin.get_fieldsets # since that has all the logic for excluding certain fields according to user permissions. @@ -2532,14 +2455,30 @@ class DomainInformationInline(admin.StackedInline): def get_fieldsets(self, request, obj=None): # Grab fieldsets from DomainInformationAdmin so that it handles all logic # for permission-based field visibility. - modified_fieldsets = DomainInformationAdmin.get_fieldsets(self, request, obj=None) + modified_fieldsets = copy.deepcopy(DomainInformationAdmin.get_fieldsets(self, request, obj=None)) - # remove .gov domain from fieldset + # Modify fieldset sections in place + for index, (title, f) in enumerate(modified_fieldsets): + if title is None: + modified_fieldsets[index][1]["fields"] = [ + field for field in modified_fieldsets[index][1]["fields"] if field not in ["creator", "domain_request", "notes"] + ] + elif title == "Contacts": + modified_fieldsets[index][1]["fields"] = [ + field for field in modified_fieldsets[index][1]["fields"] if field not in ["other_contacts", "no_other_contacts_rationale"] + ] + modified_fieldsets[index][1]["fields"].extend(["domain_managers", "invited_domain_managers"]) # type: ignore + + # Remove or remove fieldset sections for index, (title, f) in enumerate(modified_fieldsets): if title == ".gov domain": - del modified_fieldsets[index] - break - + # remove .gov domain from fieldset + modified_fieldsets.pop(index) + elif title == "Background info": + # move Background info to the bottom of the list + fieldsets_to_move = modified_fieldsets.pop(index) + modified_fieldsets.append(fieldsets_to_move) + return modified_fieldsets diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py index bfbdad81d..a9b94781f 100644 --- a/src/registrar/tests/test_admin_domain.py +++ b/src/registrar/tests/test_admin_domain.py @@ -167,12 +167,6 @@ class TestDomainAdminAsStaff(MockEppLib): expected_organization_name = "MonkeySeeMonkeyDo" self.assertContains(response, expected_organization_name) - # clean up this test's data - domain.delete() - domain_information.delete() - _domain_request.delete() - _creator.delete() - @less_console_noise_decorator def test_deletion_is_successful(self): """ @@ -227,9 +221,6 @@ class TestDomainAdminAsStaff(MockEppLib): self.assertEqual(domain.state, Domain.State.DELETED) - # clean up data within this test - domain.delete() - @less_console_noise_decorator def test_deletion_ready_fsm_failure(self): """ @@ -269,9 +260,6 @@ class TestDomainAdminAsStaff(MockEppLib): self.assertEqual(domain.state, Domain.State.READY) - # delete data created in this test - domain.delete() - @less_console_noise_decorator def test_analyst_deletes_domain_idempotent(self): """ @@ -330,9 +318,6 @@ class TestDomainAdminAsStaff(MockEppLib): ) self.assertEqual(domain.state, Domain.State.DELETED) - # delete data created in this test - domain.delete() - class TestDomainInformationInline(MockEppLib): """Test DomainAdmin class, specifically the DomainInformationInline class, as staff user. @@ -540,17 +525,6 @@ class TestDomainAdminWithClient(TestCase): self.assertContains(response, domain.name) # Check that the fields have the right values. - # == Check for the creator == # - - # Check for the right title, email, and phone number in the response. - # We only need to check for the end tag - # (Otherwise this test will fail if we change classes, etc) - self.assertContains(response, "Treat inspector") - self.assertContains(response, "meoward.jones@igorville.gov") - self.assertContains(response, "(555) 123 12345") - - # Check for the field itself - self.assertContains(response, "Meoward Jones") # == Check for the senior_official == # self.assertContains(response, "testy@town.com") @@ -560,11 +534,6 @@ class TestDomainAdminWithClient(TestCase): # Includes things like readonly fields self.assertContains(response, "Testy Tester") - # == Test the other_employees field == # - self.assertContains(response, "testy2@town.com") - self.assertContains(response, "Another Tester") - self.assertContains(response, "(555) 555 5557") - # Test for the copy link self.assertContains(response, "button--clipboard") From 410bdc8238adc1232471ed07031c9c1d60decb32 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 20 Sep 2024 12:35:16 -0400 Subject: [PATCH 5/9] cleaned up --- .../templates/admin/stacked_no_heading.html | 42 ------------------- 1 file changed, 42 deletions(-) delete mode 100644 src/registrar/templates/admin/stacked_no_heading.html diff --git a/src/registrar/templates/admin/stacked_no_heading.html b/src/registrar/templates/admin/stacked_no_heading.html deleted file mode 100644 index 60e37d757..000000000 --- a/src/registrar/templates/admin/stacked_no_heading.html +++ /dev/null @@ -1,42 +0,0 @@ -{% load i18n admin_urls %} -{% load i18n static %} - -{% comment %} -This is copied from Djangos implementation of this template, with added "blocks" -It is not inherently customizable on its own, so we can modify this instead. -https://github.com/django/django/blob/main/django/contrib/admin/templates/admin/edit_inline/stacked.html -{% endcomment %} - -
- -
- {{ inline_admin_formset.formset.management_form }} - {{ inline_admin_formset.formset.non_form_errors }} - - {% for inline_admin_form in inline_admin_formset %}
- {% if inline_admin_form.form.non_field_errors %} - {{ inline_admin_form.form.non_field_errors }} - {% endif %} - - {% for fieldset in inline_admin_form %} - {# .gov override #} - {% block fieldset %} - {% include "admin/includes/fieldset.html" %} - {% endblock fieldset%} - {# End of .gov override #} - {% endfor %} - - {% if inline_admin_form.needs_explicit_pk_field %} - {{ inline_admin_form.pk_field.field }} - {% endif %} - {% if inline_admin_form.fk_field %} - {{ inline_admin_form.fk_field.field }} - {% endif %} -
- {% endfor %} -
- -
From e4df5c83a411d28255456304ee6c43c177ae721f Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 20 Sep 2024 12:38:29 -0400 Subject: [PATCH 6/9] lint --- src/registrar/admin.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index f3287be27..a4551c728 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2444,7 +2444,7 @@ class DomainInformationInline(admin.StackedInline): return super().formfield_for_foreignkey(db_field, request, **kwargs) def get_readonly_fields(self, request, obj=None): - readonly_fields = copy.deepcopy(DomainInformationAdmin.get_readonly_fields(self, request, obj=None)) + readonly_fields = copy.deepcopy(DomainInformationAdmin.get_readonly_fields(self, request, obj=None)) readonly_fields.extend(["domain_managers", "invited_domain_managers"]) # type: ignore return readonly_fields @@ -2461,13 +2461,19 @@ class DomainInformationInline(admin.StackedInline): for index, (title, f) in enumerate(modified_fieldsets): if title is None: modified_fieldsets[index][1]["fields"] = [ - field for field in modified_fieldsets[index][1]["fields"] if field not in ["creator", "domain_request", "notes"] + field + for field in modified_fieldsets[index][1]["fields"] + if field not in ["creator", "domain_request", "notes"] ] elif title == "Contacts": modified_fieldsets[index][1]["fields"] = [ - field for field in modified_fieldsets[index][1]["fields"] if field not in ["other_contacts", "no_other_contacts_rationale"] + field + for field in modified_fieldsets[index][1]["fields"] + if field not in ["other_contacts", "no_other_contacts_rationale"] ] - modified_fieldsets[index][1]["fields"].extend(["domain_managers", "invited_domain_managers"]) # type: ignore + modified_fieldsets[index][1]["fields"].extend( + ["domain_managers", "invited_domain_managers"] + ) # type: ignore # Remove or remove fieldset sections for index, (title, f) in enumerate(modified_fieldsets): @@ -2478,7 +2484,7 @@ class DomainInformationInline(admin.StackedInline): # move Background info to the bottom of the list fieldsets_to_move = modified_fieldsets.pop(index) modified_fieldsets.append(fieldsets_to_move) - + return modified_fieldsets From 861f5d2fa6e074357624a7887c7e387fac02b6df Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 20 Sep 2024 15:02:10 -0400 Subject: [PATCH 7/9] request and notes added to background section --- src/registrar/admin.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index a4551c728..e7e8cfb7a 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2474,6 +2474,11 @@ class DomainInformationInline(admin.StackedInline): modified_fieldsets[index][1]["fields"].extend( ["domain_managers", "invited_domain_managers"] ) # type: ignore + elif title == "Background info": + # move domain request and notes to background + modified_fieldsets[index][1]["fields"].extend( + ["domain_request", "notes"] + ) # type: ignore # Remove or remove fieldset sections for index, (title, f) in enumerate(modified_fieldsets): From 4ab41f82787115b0f99540f51db8c7b5720b5eae Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 23 Sep 2024 12:08:21 -0400 Subject: [PATCH 8/9] code cleanup, formatting of nameservers in display --- src/registrar/admin.py | 45 ++++++++++++++---------------------------- 1 file changed, 15 insertions(+), 30 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index e7e8cfb7a..397c4a3ed 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1566,7 +1566,7 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): modified_fieldsets = [] for name, data in fieldsets: fields = data.get("fields", []) - fields = tuple(field for field in fields if field not in DomainInformationAdmin.superuser_only_fields) + fields = [field for field in fields if field not in DomainInformationAdmin.superuser_only_fields] modified_fieldsets.append((name, {**data, "fields": fields})) return modified_fieldsets return fieldsets @@ -2358,18 +2358,12 @@ class DomainInformationInline(admin.StackedInline): return domain_invitations def domain_managers(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.""" + """Get domain managers for the domain, unpack and return an HTML block.""" domain_managers = self.get_domain_managers(obj) if not domain_managers: return "No domain managers found." - domain_manager_details = "" + "" + domain_manager_details = "
UIDNameEmail
" for domain_manager in domain_managers: full_name = domain_manager.get_formatted_name() change_url = reverse("admin:registrar_user_change", args=[domain_manager.pk]) @@ -2384,13 +2378,7 @@ class DomainInformationInline(admin.StackedInline): domain_managers.short_description = "Domain managers" # type: ignore def invited_domain_managers(self, obj): - """Get emails which have been invited to the domain, 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.""" + """Get emails which have been invited to the domain, unpack and return an HTML block.""" domain_invitations = self.get_domain_invitations(obj) if not domain_invitations: return "No invited domain managers found." @@ -2458,27 +2446,21 @@ class DomainInformationInline(admin.StackedInline): modified_fieldsets = copy.deepcopy(DomainInformationAdmin.get_fieldsets(self, request, obj=None)) # Modify fieldset sections in place - for index, (title, f) in enumerate(modified_fieldsets): + for index, (title, options) in enumerate(modified_fieldsets): if title is None: - modified_fieldsets[index][1]["fields"] = [ - field - for field in modified_fieldsets[index][1]["fields"] - if field not in ["creator", "domain_request", "notes"] + options["fields"] = [ + field for field in options["fields"] if field not in ["creator", "domain_request", "notes"] ] elif title == "Contacts": - modified_fieldsets[index][1]["fields"] = [ + options["fields"] = [ field - for field in modified_fieldsets[index][1]["fields"] + for field in options["fields"] if field not in ["other_contacts", "no_other_contacts_rationale"] ] - modified_fieldsets[index][1]["fields"].extend( - ["domain_managers", "invited_domain_managers"] - ) # type: ignore + options["fields"].extend(["domain_managers", "invited_domain_managers"]) # type: ignore elif title == "Background info": # move domain request and notes to background - modified_fieldsets[index][1]["fields"].extend( - ["domain_request", "notes"] - ) # type: ignore + options["fields"].extend(["domain_request", "notes"]) # type: ignore # Remove or remove fieldset sections for index, (title, f) in enumerate(modified_fieldsets): @@ -2582,7 +2564,10 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): formatted_nameservers = [] for server, ip_list in obj.nameservers: - formatted_nameservers.append(f"{server} [{', '.join(ip_list)}]") + server_display = str(server) + if ip_list: + server_display += f" [{', '.join(ip_list)}]" + formatted_nameservers.append(server_display) # Join the formatted strings with line breaks return "\n".join(formatted_nameservers) From 3f57a57089f254a816b5bfa41d8acd89bb028e64 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Thu, 26 Sep 2024 18:57:38 -0400 Subject: [PATCH 9/9] Fixed text labels --- src/registrar/admin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 397c4a3ed..00b037b65 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2555,7 +2555,7 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): def dnssecdata(self, obj): return "Yes" if obj.dnssecdata else "No" - dnssecdata.short_description = "DNS Sec Enabled" # type: ignore + dnssecdata.short_description = "DNSSEC enabled" # type: ignore # Custom method to display formatted nameservers def nameservers(self, obj): @@ -2572,7 +2572,7 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): # Join the formatted strings with line breaks return "\n".join(formatted_nameservers) - nameservers.short_description = "Nameservers" # type: ignore + nameservers.short_description = "Name servers" # type: ignore def custom_election_board(self, obj): domain_info = getattr(obj, "domain_info", None)
UIDNameEmail