diff --git a/src/registrar/admin.py b/src/registrar/admin.py index a3380eb71..4a2a4515a 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -911,6 +911,7 @@ class DomainInformationAdmin(ListHeaderAdmin): "no_other_contacts_rationale", "anything_else", "is_policy_acknowledged", + "other_contacts", ] # For each filter_horizontal, init in admin js extendFilterHorizontalWidgets @@ -928,6 +929,8 @@ class DomainInformationAdmin(ListHeaderAdmin): # Table ordering ordering = ["domain__name"] + change_form_template = "django/admin/domain_information_change_form.html" + def get_readonly_fields(self, request, obj=None): """Set the read-only state on form elements. We have 1 conditions that determine which fields are read-only: diff --git a/src/registrar/templates/admin/fieldset.html b/src/registrar/templates/admin/fieldset.html index 96433e972..37f79ab46 100644 --- a/src/registrar/templates/admin/fieldset.html +++ b/src/registrar/templates/admin/fieldset.html @@ -16,34 +16,49 @@ https://github.com/django/django/blob/main/django/contrib/admin/templates/admin/ {% endif %} {% endblock fieldset_description %} - {% block fieldset_lines %} - {% for line in fieldset %} -
- {% if line.fields|length == 1 %}{{ line.errors }}{% else %}
{% endif %} - {% for field in line %} -
- {% if not line.fields|length == 1 and not field.is_readonly %}{{ field.errors }}{% endif %} -
- {% if field.is_checkbox %} - {{ field.field }}{{ field.label_tag }} + {% for line in fieldset %} +
+ {% if line.fields|length == 1 %}{{ line.errors }}{% else %}
{% endif %} + {% for field in line %} +
+ {% if not line.fields|length == 1 and not field.is_readonly %}{{ field.errors }}{% endif %} +
+ {% if field.is_checkbox %} + {% block field_checkbox %} + {{ field.field }}{{ field.label_tag }} + {% endblock field_checkbox%} + {% else %} + {{ field.label_tag }} + {% if field.is_readonly %} + {% block field_readonly %} +
{{ field.contents }}
+ {% endblock field_readonly%} {% else %} - {{ field.label_tag }} - {% if field.is_readonly %} -
{{ field.contents }}
- {% else %} - {{ field.field }} - {% endif %} + {% block field_other %} + {{ field.field }} + {% endblock field_other%} {% endif %} -
- {% if field.field.help_text %} -
-
{{ field.field.help_text|safe }}
-
- {% endif %} -
- {% endfor %} - {% if not line.fields|length == 1 %}
{% endif %} -
- {% endfor %} - {% endblock fieldset_lines %} + {% endif %} +
+ + {% block before_help_text %} + {# For templating purposes #} + {% endblock before_help_text %} + + {% if field.field.help_text %} + {% block help_text %} +
+
{{ field.field.help_text|safe }}
+
+ {% endblock help_text %} + {% endif %} + + {% block after_help_text %} + {# For templating purposes #} + {% endblock after_help_text %} +
+ {% endfor %} + {% if not line.fields|length == 1 %}
{% endif %} +
+ {% endfor %} diff --git a/src/registrar/templates/django/admin/domain_information_change_form.html b/src/registrar/templates/django/admin/domain_information_change_form.html new file mode 100644 index 000000000..86475890d --- /dev/null +++ b/src/registrar/templates/django/admin/domain_information_change_form.html @@ -0,0 +1,8 @@ +{% extends 'admin/change_form.html' %} +{% load i18n static %} + +{% block field_sets %} + {% for fieldset in adminform %} + {% include "django/admin/includes/domain_information_fieldset.html" %} + {% endfor %} +{% endblock %} diff --git a/src/registrar/templates/django/admin/includes/domain_request_detail_table.html b/src/registrar/templates/django/admin/includes/contact_detail_table.html similarity index 100% rename from src/registrar/templates/django/admin/includes/domain_request_detail_table.html rename to src/registrar/templates/django/admin/includes/contact_detail_table.html diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html new file mode 100644 index 000000000..f5a5b71ee --- /dev/null +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -0,0 +1,61 @@ +{% extends "admin/fieldset.html" %} +{% load static url_helpers %} + +{% comment %} +This is using a custom implementation fieldset.html (see admin/fieldset.html) +{% endcomment %} +{% block field_readonly %} + {% if field.field.name == "other_contacts" %} +
+ {% for contact in field.contents|split:", " %} + {{ contact }}{% if not forloop.last %}, {% endif %} + {% endfor %} +
+ {% elif field.field.name == "current_websites" %} + {% comment %} + The "website" model is essentially just a text field. + It is not useful to be redirected to the object definition, + rather it is more useful in this scenario to be redirected to the + actual website (as its just a plaintext string otherwise). + + This ONLY applies to analysts. For superusers, its business as usual. + {% endcomment %} + {% for website in field.contents|split:", " %} + {{ website }}{% if not forloop.last %}, {% endif %} + {% endfor %} + {% else %} +
{{ field.contents }}
+ {% endif %} +{% endblock field_readonly %} + +{% block after_help_text %} + {% if field.field.name == "creator" %} + {% include "django/admin/includes/contact_detail_table.html" with user=original.creator field_name="creator" %} + {% elif field.field.name == "submitter" %} + {% include "django/admin/includes/contact_detail_table.html" with user=original.submitter field_name="submitter" %} + {% elif field.field.name == "authorizing_official" %} + {% include "django/admin/includes/contact_detail_table.html" with user=original.authorizing_official field_name="authorizing_official" %} + {% elif field.field.name == "other_contacts" and original.other_contacts.all %} +
+ Details +
+ + + {% for contact in original.other_contacts.all %} + {% comment %} + Since we can't get the id from field, we can embed this information here. + Then we can link these two fields using javascript. + {% endcomment %} + + + + + + + {% endfor %} + +
{{contact.first_name}} {{contact.last_name}}{{ contact.title }}{{ contact.email }}{{ contact.phone }}
+
+
+ {% endif %} +{% endblock after_help_text %} diff --git a/src/registrar/templates/django/admin/includes/domain_information_fieldset.html b/src/registrar/templates/django/admin/includes/domain_information_fieldset.html new file mode 100644 index 000000000..b42873c3c --- /dev/null +++ b/src/registrar/templates/django/admin/includes/domain_information_fieldset.html @@ -0,0 +1,2 @@ +{% extends "django/admin/includes/detail_table_fieldset.html" %} +{# Stubbed file for future expansion #} diff --git a/src/registrar/templates/django/admin/includes/domain_request_fieldset.html b/src/registrar/templates/django/admin/includes/domain_request_fieldset.html index 976969667..b42873c3c 100644 --- a/src/registrar/templates/django/admin/includes/domain_request_fieldset.html +++ b/src/registrar/templates/django/admin/includes/domain_request_fieldset.html @@ -1,101 +1,2 @@ -{% extends "admin/fieldset.html" %} -{% load static url_helpers %} -{% comment %} -This is using a custom implementation fieldset.html (see admin/fieldset.html) -{% endcomment %} -{% block fieldset_lines %} -{% for line in fieldset %} -
- {% if line.fields|length == 1 %}{{ line.errors }}{% else %}
{% endif %} - {% for field in line %} -
- {% if not line.fields|length == 1 and not field.is_readonly %}{{ field.errors }}{% endif %} -
- {% if field.is_checkbox %} - {{ field.field }}{{ field.label_tag }} - {% else %} - {{ field.label_tag }} - {% if field.is_readonly %} - {% if field.field.name == "other_contacts" %} -
- {% for contact in field.contents|split:", " %} - {{ contact }}{% if not forloop.last %}, {% endif %} - {% endfor %} -
- {% elif field.field.name == "current_websites" %} - {% comment %} - The "website" model is essentially just a text field. - It is not useful to be redirected to the object definition, - rather it is more useful in this scenario to be redirected to the - actual website (as its just a plaintext string otherwise). - - This ONLY applies to analysts. For superusers, its business as usual. - {% endcomment %} - {% for website in field.contents|split:", " %} - {{ website }}{% if not forloop.last %}, {% endif %} - {% endfor %} - {% else %} -
{{ field.contents }}
- {% endif %} - {% else %} - {{ field.field }} - {% endif %} - {% endif %} -
- {% if field.field.name == "creator" %} - {% include "django/admin/includes/domain_request_detail_table.html" with user=original.creator field_name="creator" %} - {% elif field.field.name == "submitter" %} - {% include "django/admin/includes/domain_request_detail_table.html" with user=original.submitter field_name="submitter" %} - {% elif field.field.name == "authorizing_official" %} - {% include "django/admin/includes/domain_request_detail_table.html" with user=original.authorizing_official field_name="authorizing_official" %} - {% elif field.field.name == "other_contacts" %} -
- Details -
- - - {% for contact in original.other_contacts.all %} - {% comment %} - Since we can't get the id from field, we can embed this information here. - Then we can link these two fields using javascript. - {% endcomment %} - - - - - - {# Copy button for the email #} - - - {% endfor %} - -
{{contact.first_name}} {{contact.last_name}}{{ contact.title }}{{ contact.email }}{{ contact.phone }} - - -
-
-
- {% endif %} - - {% if field.field.help_text %} -
-
{{ field.field.help_text|safe }}
-
- {% endif %} -
- {% endfor %} - {% if not line.fields|length == 1 %}
{% endif %} -
-{% endfor %} - -{% endblock fieldset_lines %} +{% extends "django/admin/includes/detail_table_fieldset.html" %} +{# Stubbed file for future expansion #} diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 04eb4c82d..655fda02b 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -116,6 +116,31 @@ class GenericTestHelper(TestCase): self.url = url self.client = client + def assert_response_contains_distinct_values(self, response, expected_values): + """ + Asserts that each specified value appears exactly once in the response. + + This method iterates over a list of tuples, where each tuple contains a field name + and its expected value. It then performs an assertContains check for each value, + ensuring that each value appears exactly once in the response. + + Parameters: + - response: The HttpResponse object to inspect. + - expected_values: A list of tuples, where each tuple contains: + - field: The name of the field (used for subTest identification). + - value: The expected value to check for in the response. + + Example usage: + expected_values = [ + ("title", "Treat inspector"), + ("email", "meoward.jones@igorville.gov"), + ] + self.assert_response_contains_distinct_values(response, expected_values) + """ + for field, value in expected_values: + with self.subTest(field=field, expected_value=value): + self.assertContains(response, value, count=1) + def assert_table_sorted(self, o_index, sort_fields): """ This helper function validates the sorting functionality of a Django Admin table view. diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 7bcfc388f..c69759fb2 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1247,31 +1247,6 @@ class TestDomainRequestAdmin(MockEppLib): expected_url = 'city.com' self.assertContains(response, expected_url) - def assert_response_contains_distinct_values(self, response, expected_values): - """ - Asserts that each specified value appears exactly once in the response. - - This method iterates over a list of tuples, where each tuple contains a field name - and its expected value. It then performs an assertContains check for each value, - ensuring that each value appears exactly once in the response. - - Parameters: - - response: The HttpResponse object to inspect. - - expected_values: A list of tuples, where each tuple contains: - - field: The name of the field (used for subTest identification). - - value: The expected value to check for in the response. - - Example usage: - expected_values = [ - ("title", "Treat inspector"), - ("email", "meoward.jones@igorville.gov"), - ] - self.assert_response_contains_distinct_values(response, expected_values) - """ - for field, value in expected_values: - with self.subTest(field=field, expected_value=value): - self.assertContains(response, value, count=1) - @less_console_noise_decorator def test_contact_fields_have_detail_table(self): """Tests if the contact fields have the detail table which displays title, email, and phone""" @@ -1319,7 +1294,7 @@ class TestDomainRequestAdmin(MockEppLib): ("email_copy_button_input", f'"), ] - self.assert_response_contains_distinct_values(response, expected_creator_fields) + self.test_helper.assert_response_contains_distinct_values(response, expected_creator_fields) # Check for the field itself self.assertContains(response, "Meoward Jones") @@ -1333,7 +1308,7 @@ class TestDomainRequestAdmin(MockEppLib): ("email_copy_button_input", f'"), ] - self.assert_response_contains_distinct_values(response, expected_submitter_fields) + self.test_helper.assert_response_contains_distinct_values(response, expected_submitter_fields) self.assertContains(response, "Testy2 Tester2") # == Check for the authorizing_official == # @@ -1345,7 +1320,7 @@ class TestDomainRequestAdmin(MockEppLib): ("email_copy_button_input", f'"), ] - self.assert_response_contains_distinct_values(response, expected_ao_fields) + self.test_helper.assert_response_contains_distinct_values(response, expected_ao_fields) # count=4 because the underlying domain has two users with this name. # The dropdown has 3 of these. @@ -1372,7 +1347,7 @@ class TestDomainRequestAdmin(MockEppLib): ("email_copy_button_input", f'"), ] - self.assert_response_contains_distinct_values(response, expected_other_employees_fields) + self.test_helper.assert_response_contains_distinct_values(response, expected_other_employees_fields) # count=1 as only one should exist in a table self.assertContains(response, "Testy Tester", count=1) @@ -1972,6 +1947,139 @@ class TestDomainInformationAdmin(TestCase): Contact.objects.all().delete() User.objects.all().delete() + @less_console_noise_decorator + def test_other_contacts_has_readonly_link(self): + """Tests if the readonly other_contacts field has links""" + + # Create a fake domain request and domain + domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW) + domain_request.approve() + domain_info = DomainInformation.objects.filter(domain=domain_request.approved_domain).get() + + # Get the other contact + other_contact = domain_info.other_contacts.all().first() + + p = "userpass" + self.client.login(username="staffuser", password=p) + response = self.client.get( + "/admin/registrar/domaininformation/{}/change/".format(domain_info.pk), + follow=True, + ) + + # Make sure the page loaded, and that we're on the right page + self.assertEqual(response.status_code, 200) + self.assertContains(response, domain_info.domain.name) + + # Check that the page contains the url we expect + expected_href = reverse("admin:registrar_contact_change", args=[other_contact.id]) + self.assertContains(response, expected_href) + + # Check that the page contains the link we expect. + # Since the url is dynamic (populated by JS), we can test for its existence + # by checking for the end tag. + expected_url = "Testy Tester" + self.assertContains(response, expected_url) + + @less_console_noise_decorator + def test_contact_fields_have_detail_table(self): + """Tests if the contact fields have the detail table which displays title, email, and phone""" + + # Create fake creator + _creator = User.objects.create( + username="MrMeoward", + first_name="Meoward", + last_name="Jones", + ) + + # Due to the relation between User <==> Contact, + # the underlying contact has to be modified this way. + _creator.contact.email = "meoward.jones@igorville.gov" + _creator.contact.phone = "(555) 123 12345" + _creator.contact.title = "Treat inspector" + _creator.contact.save() + + # Create a fake domain request + domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW, user=_creator) + domain_request.approve() + domain_info = DomainInformation.objects.filter(domain=domain_request.approved_domain).get() + + p = "userpass" + self.client.login(username="staffuser", password=p) + response = self.client.get( + "/admin/registrar/domaininformation/{}/change/".format(domain_info.pk), + follow=True, + ) + + # Make sure the page loaded, and that we're on the right page + self.assertEqual(response.status_code, 200) + self.assertContains(response, domain_info.domain.name) + + # Check that the modal has the right content + # Check for the header + + # == 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) + expected_creator_fields = [ + # Field, expected value + ("title", "Treat inspector"), + ("email", "meoward.jones@igorville.gov"), + ("phone", "(555) 123 12345"), + ] + self.test_helper.assert_response_contains_distinct_values(response, expected_creator_fields) + + # Check for the field itself + self.assertContains(response, "Meoward Jones") + + # == Check for the submitter == # + expected_submitter_fields = [ + # Field, expected value + ("title", "Admin Tester"), + ("email", "mayor@igorville.gov"), + ("phone", "(555) 555 5556"), + ] + self.test_helper.assert_response_contains_distinct_values(response, expected_submitter_fields) + self.assertContains(response, "Testy2 Tester2") + + # == Check for the authorizing_official == # + expected_ao_fields = [ + # Field, expected value + ("title", "Chief Tester"), + ("email", "testy@town.com"), + ("phone", "(555) 555 5555"), + ] + self.test_helper.assert_response_contains_distinct_values(response, expected_ao_fields) + + # count=4 because the underlying domain has two users with this name. + # The dropdown has 3 of these. + self.assertContains(response, "Testy Tester", count=4) + + # Check for table titles. We only need to check for the end tag + # (Otherwise this test will fail if we change classes, etc) + + # Title. Count=3 because this table appears on three records. + self.assertContains(response, "Title", count=3) + + # Email. Count=3 because this table appears on three records. + self.assertContains(response, "Email", count=3) + + # Phone. Count=3 because this table appears on three records. + self.assertContains(response, "Phone", count=3) + + # == Test the other_employees field == # + expected_other_employees_fields = [ + # Field, expected value + ("title", "Another Tester"), + ("email", "testy2@town.com"), + ("phone", "(555) 555 5557"), + ] + self.test_helper.assert_response_contains_distinct_values(response, expected_other_employees_fields) + + # count=1 as only one should exist in a table + self.assertContains(response, "Testy Tester", count=1) + def test_readonly_fields_for_analyst(self): """Ensures that analysts have their permissions setup correctly""" with less_console_noise(): @@ -1990,6 +2098,7 @@ class TestDomainInformationAdmin(TestCase): "no_other_contacts_rationale", "anything_else", "is_policy_acknowledged", + "other_contacts", ] self.assertEqual(readonly_fields, expected_fields)