From 4e5809b99c3d583de3009bfd4c1f6383bc0575bb Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 25 Mar 2024 09:50:52 -0600 Subject: [PATCH 01/14] Style simplification --- src/registrar/assets/sass/_theme/_admin.scss | 30 ------------------- .../admin/includes/detail_table_fieldset.html | 2 +- 2 files changed, 1 insertion(+), 31 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index c711e296e..8dd2a59f3 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -347,27 +347,6 @@ input.admin-confirm-button { color: $dhs-blue-70; } -details.dja-detail-table { - background-color: var(--darkened-bg); - display: inline-table; - .dja-details-summary { - color: var(--header-link-color); - background-color: var(--primary); - } - .dja-details-contents { - tr { - background-color: var(--darkened-bg); - } - } -} - -table.dja-user-detail-table { - margin-left: 160px; - tr { - background-color: var(--body-bg); - } -} - .admin-icon-group { position: relative; display: flex; @@ -391,19 +370,10 @@ table.dja-user-detail-table { } } - - details.dja-detail-table { - background-color: var(--darkened-bg); display: inline-table; .dja-details-summary { color: var(--header-link-color); - background-color: var(--primary); - } - .dja-details-contents { - tr { - background-color: var(--darkened-bg); - } } } 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 f5a5b71ee..6ebb74245 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -38,7 +38,7 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% elif field.field.name == "other_contacts" and original.other_contacts.all %}
Details -
+
{% for contact in original.other_contacts.all %} From c54bffca4c962869dc82be2867f6f79c40f77036 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 25 Mar 2024 10:07:31 -0600 Subject: [PATCH 02/14] Further simplification --- src/registrar/assets/sass/_theme/_admin.scss | 9 +++++++++ .../django/admin/includes/detail_table_fieldset.html | 10 +++++----- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 8dd2a59f3..9532f622d 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -372,9 +372,18 @@ input.admin-confirm-button { details.dja-detail-table { display: inline-table; + background-color: var(--body-bg); .dja-details-summary { color: var(--header-link-color); } + + tr { + background-color: transparent; + } + + td, th { + padding-left: 12px; + } } table.dja-user-detail-table { 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 6ebb74245..6784c1f3b 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -37,7 +37,7 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% 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 + Details
@@ -47,10 +47,10 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) Then we can link these two fields using javascript. {% endcomment %} - - - - + + + + {% endfor %} From 55fd53fb5630c78799aee8bae5c508325ed931ad Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 25 Mar 2024 14:13:52 -0600 Subject: [PATCH 03/14] PR suggestions --- src/registrar/assets/js/get-gov-reports.js | 1 - src/registrar/assets/sass/_theme/_admin.scss | 68 ++++++------------- src/registrar/templates/admin/fieldset.html | 10 ++- .../admin/domain_information_change_form.html | 7 ++ .../admin/domain_request_change_form.html | 10 ++- .../admin/includes/contact_detail_table.html | 25 ++++--- .../admin/includes/detail_table_fieldset.html | 16 ++++- .../includes/domain_information_fieldset.html | 2 - .../includes/domain_request_fieldset.html | 2 - 9 files changed, 75 insertions(+), 66 deletions(-) delete mode 100644 src/registrar/templates/django/admin/includes/domain_information_fieldset.html delete mode 100644 src/registrar/templates/django/admin/includes/domain_request_fieldset.html diff --git a/src/registrar/assets/js/get-gov-reports.js b/src/registrar/assets/js/get-gov-reports.js index 735681777..92bba4a1f 100644 --- a/src/registrar/assets/js/get-gov-reports.js +++ b/src/registrar/assets/js/get-gov-reports.js @@ -61,7 +61,6 @@ function createComparativeColumnChart(canvasId, title, labelOne, labelTwo) { var canvas = document.getElementById(canvasId); if (!canvas) { - console.log("Could not find canvas") return } diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 9532f622d..912a67f10 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -347,29 +347,6 @@ input.admin-confirm-button { color: $dhs-blue-70; } -.admin-icon-group { - position: relative; - display: flex; - align-items: center; - - .usa-button__icon { - position: absolute; - right: 0; - height: 100%; - } - - input { - // Allow for padding around the copy button - padding-right: 35px; - // Match the height of other inputs - min-height: 2.25rem; - } - - .no-outline-on-click:focus { - outline: none !important; - } -} - details.dja-detail-table { display: inline-table; background-color: var(--body-bg); @@ -383,40 +360,35 @@ details.dja-detail-table { td, th { padding-left: 12px; + border: none } + + thead > tr > th { + border-radius: 4px; + border-top: none; + border-bottom: none; + } + } table.dja-user-detail-table { - margin-left: 160px; tr { background-color: var(--body-bg); } + + // Make the head of the user detail table "float" + thead > tr > th { + border-radius: 4px; + border-top: none; + border-bottom: none; + } + + th, td { + border: none + } + } -.admin-icon-group { - position: relative; - display: flex; - align-items: center; - - .usa-button__icon { - position: absolute; - right: 0; - height: 100%; - } - - input { - // Allow for padding around the copy button - padding-right: 35px; - // Match the height of other inputs - min-height: 2.25rem; - } - - .no-outline-on-click:focus { - outline: none !important; - } -} - - .errors span.select2-selection { border: 1px solid var(--error-fg) !important; } \ No newline at end of file diff --git a/src/registrar/templates/admin/fieldset.html b/src/registrar/templates/admin/fieldset.html index 37f79ab46..579ad7033 100644 --- a/src/registrar/templates/admin/fieldset.html +++ b/src/registrar/templates/admin/fieldset.html @@ -6,10 +6,12 @@ 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/includes/fieldset.html {% endcomment %}
+ {# .gov override #} {% block fieldset_title %} {% if fieldset.name %}

{{ fieldset.name }}

{% endif %} {% endblock fieldset_title %} + {# .gov override #} {% block fieldset_description %} {% if fieldset.description %}
{{ fieldset.description|safe }}
@@ -24,16 +26,19 @@ https://github.com/django/django/blob/main/django/contrib/admin/templates/admin/ {% if not line.fields|length == 1 and not field.is_readonly %}{{ field.errors }}{% endif %}
{% if field.is_checkbox %} + {# .gov override #} {% block field_checkbox %} {{ field.field }}{{ field.label_tag }} {% endblock field_checkbox%} {% else %} {{ field.label_tag }} {% if field.is_readonly %} + {# .gov override #} {% block field_readonly %}
{{ field.contents }}
{% endblock field_readonly%} {% else %} + {# .gov override #} {% block field_other %} {{ field.field }} {% endblock field_other%} @@ -41,18 +46,21 @@ https://github.com/django/django/blob/main/django/contrib/admin/templates/admin/ {% endif %}
+ {# .gov addition #} {% block before_help_text %} {# For templating purposes #} {% endblock before_help_text %} {% if field.field.help_text %} + {# .gov override #} {% block help_text %}
{{ field.field.help_text|safe }}
{% endblock help_text %} {% endif %} - + + {# .gov addition #} {% block after_help_text %} {# For templating purposes #} {% endblock after_help_text %} diff --git a/src/registrar/templates/django/admin/domain_information_change_form.html b/src/registrar/templates/django/admin/domain_information_change_form.html index 86475890d..f58ee2239 100644 --- a/src/registrar/templates/django/admin/domain_information_change_form.html +++ b/src/registrar/templates/django/admin/domain_information_change_form.html @@ -3,6 +3,13 @@ {% block field_sets %} {% for fieldset in adminform %} + {% comment %} + TODO: this will eventually need to be changed to something like this + if we ever want to customize this file: {% include "django/admin/includes/domain_information_fieldset.html" %} + + Use detail_table_fieldset as an example, or just extend it. + {% endcomment %} + {% include "django/admin/includes/detail_table_fieldset.html" %} {% endfor %} {% endblock %} diff --git a/src/registrar/templates/django/admin/domain_request_change_form.html b/src/registrar/templates/django/admin/domain_request_change_form.html index 5d5e6ed60..b0ee6e044 100644 --- a/src/registrar/templates/django/admin/domain_request_change_form.html +++ b/src/registrar/templates/django/admin/domain_request_change_form.html @@ -4,8 +4,16 @@ {% block field_sets %} {# Create an invisible tag so that we can use a click event to toggle the modal. #} + {% for fieldset in adminform %} - {% include "django/admin/includes/domain_request_fieldset.html" %} + {% comment %} + TODO: this will eventually need to be changed to something like this + if we ever want to customize this file: + {% include "django/admin/includes/domain_information_fieldset.html" %} + + Use detail_table_fieldset as an example, or just extend it. + {% endcomment %} + {% include "django/admin/includes/detail_table_fieldset.html" %} {% endfor %} {% endblock %} diff --git a/src/registrar/templates/django/admin/includes/contact_detail_table.html b/src/registrar/templates/django/admin/includes/contact_detail_table.html index c25812b4d..68dbedb24 100644 --- a/src/registrar/templates/django/admin/includes/contact_detail_table.html +++ b/src/registrar/templates/django/admin/includes/contact_detail_table.html @@ -1,9 +1,14 @@ {% load i18n static %} -
{{contact.first_name}} {{contact.last_name}}{{ contact.title }}{{ contact.email }}{{ contact.phone }}{{contact.first_name}} {{contact.last_name}}{{ contact.title }}{{ contact.email }}{{ contact.phone }}
+
+ + + + + - + {% if user.title or user.contact.title %} {% if user.contact.title %} @@ -15,27 +20,27 @@ {% endif %} - + {% if user.email or user.contact.email %} {% if user.contact.email %} - + {% else %} - + {% endif %} {% else %} - + {% endif %} - + {% if user.phone or user.contact.phone %} {% if user.contact.phone %} - + {% else %} - + {% endif %} {% else %} - + {% endif %} 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 6784c1f3b..25b2826db 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -30,16 +30,30 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% 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
Contact details
TitleTitle{{ user.contact.title }}
EmailEmail{{ user.contact.email }}{{ user.contact.email }}{{ user.email }}{{ user.email }}Nothing foundNothing found
PhonePhone{{ user.contact.phone }}{{ user.contact.phone }}{{ user.phone }}{{ user.phone }}Nothing foundNothing found
+ + + + + {% for contact in original.other_contacts.all %} {% comment %} diff --git a/src/registrar/templates/django/admin/includes/domain_information_fieldset.html b/src/registrar/templates/django/admin/includes/domain_information_fieldset.html deleted file mode 100644 index b42873c3c..000000000 --- a/src/registrar/templates/django/admin/includes/domain_information_fieldset.html +++ /dev/null @@ -1,2 +0,0 @@ -{% 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 deleted file mode 100644 index b42873c3c..000000000 --- a/src/registrar/templates/django/admin/includes/domain_request_fieldset.html +++ /dev/null @@ -1,2 +0,0 @@ -{% extends "django/admin/includes/detail_table_fieldset.html" %} -{# Stubbed file for future expansion #} From a70214e46763b3f821862d6c88934caa87b61f86 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 26 Mar 2024 13:21:40 -0600 Subject: [PATCH 04/14] PR suggestions --- src/registrar/admin.py | 6 ++ .../admin/includes/contact_detail_table.html | 46 ++++++++--- .../admin/includes/detail_table_fieldset.html | 79 ++++++++++++------- 3 files changed, 88 insertions(+), 43 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index febd8123c..e4a94e20d 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -910,6 +910,9 @@ class DomainInformationAdmin(ListHeaderAdmin): ), ] + # Readonly fields for analysts and superusers + readonly_fields = ("other_contacts",) + # Read only that we'll leverage for CISA Analysts analyst_readonly_fields = [ "creator", @@ -1120,6 +1123,9 @@ class DomainRequestAdmin(ListHeaderAdmin): ), ] + # Readonly fields for analysts and superusers + readonly_fields = ("other_contacts", "current_websites") + # Read only that we'll leverage for CISA Analysts analyst_readonly_fields = [ "creator", diff --git a/src/registrar/templates/django/admin/includes/contact_detail_table.html b/src/registrar/templates/django/admin/includes/contact_detail_table.html index 68dbedb24..decce0f43 100644 --- a/src/registrar/templates/django/admin/includes/contact_detail_table.html +++ b/src/registrar/templates/django/admin/includes/contact_detail_table.html @@ -1,47 +1,67 @@ {% load i18n static %} -
Contact information
+
+ {% if show_table_details %} + {% endif %} + + {% if user.title or user.contact.title or user.email or user.contact.email or user.phone or user.contact.phone %} - + {% if show_table_details %} + + {% endif %} + {% if user.title or user.contact.title %} {% if user.contact.title %} - + {% else %} - + {% endif %} {% else %} - + {% endif %} - + {% if show_table_details %} + + {% endif %} + {% if user.email or user.contact.email %} {% if user.contact.email %} - + {% else %} - + {% endif %} {% else %} - + {% endif %} - + {% if show_table_details %} + + {% endif %} + {% if user.phone or user.contact.phone %} {% if user.contact.phone %} - + {% else %} - + {% endif %} {% else %} - + {% endif %} + {% else %} + + + + + + {% endif %}
Contact details
TitleTitle{{ user.contact.title }}{{ user.contact.title }}{{ user.title }}{{ user.title }}Nothing foundNone
EmailEmail{{ user.contact.email }}{{ user.contact.email }}{{ user.email }}{{ user.email }}Nothing foundNone
PhonePhone{{ user.contact.phone }}{{ user.contact.phone }}{{ user.phone }}{{ user.phone }}Nothing foundNone
No additional contact information found.
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 25b2826db..4ca577688 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -20,9 +20,17 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) This ONLY applies to analysts. For superusers, its business as usual. {% endcomment %} - {% for website in field.contents|split:", " %} +
+ {% with total_websites=field.contents|split:", " %} + {% for website in total_websites %} {{ website }}{% if not forloop.last %}, {% endif %} + {# Acts as a
#} + {% if total_websites|length < 5 %} +
+ {% endif %} {% endfor %} + {% endwith %} +
{% else %}
{{ field.contents }}
{% endif %} @@ -32,44 +40,55 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% if field.field.name == "creator" %}
- {% include "django/admin/includes/contact_detail_table.html" with user=original.creator field_name="creator" %} + {% include "django/admin/includes/contact_detail_table.html" with user=original.creator field_name="creator" no_title_top_padding=field.is_readonly %}
{% elif field.field.name == "submitter" %}
- {% include "django/admin/includes/contact_detail_table.html" with user=original.submitter field_name="submitter" %} + {% include "django/admin/includes/contact_detail_table.html" with user=original.submitter field_name="submitter" no_title_top_padding=field.is_readonly %}
{% elif field.field.name == "authorizing_official" %}
- {% include "django/admin/includes/contact_detail_table.html" with user=original.authorizing_official field_name="authorizing_official" %} + {% include "django/admin/includes/contact_detail_table.html" with user=original.authorizing_official field_name="authorizing_official" no_title_top_padding=field.is_readonly %}
{% 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 information
{{contact.first_name}} {{contact.last_name}}{{ contact.title }}{{ contact.email }}{{ contact.phone }}
-
-
+ {% with all_contacts=original.other_contacts.all %} + {% if all_contacts.count == 1 %} + {% for contact in all_contacts %} +
+ + {% include "django/admin/includes/contact_detail_table.html" with user=contact field_name="other_contact" no_title_top_padding=field.is_readonly %} +
+ {% endfor %} + {% else %} +
+ Details +
+ + + + + + + + {% for contact in all_contacts %} + {% 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 information
{{contact.first_name}} {{contact.last_name}}{{ contact.title }}{{ contact.email }}{{ contact.phone }}
+
+
+ {% endif %} + {% endwith %} {% endif %} {% endblock after_help_text %} From a512dcd1b17d8035ed3fad2b0bff53b149c64a18 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 26 Mar 2024 14:45:26 -0600 Subject: [PATCH 05/14] Fix unit tests --- src/registrar/admin.py | 6 +-- src/registrar/assets/js/get-gov-admin.js | 6 +++ .../admin/includes/detail_table_fieldset.html | 1 + src/registrar/tests/common.py | 1 - src/registrar/tests/test_admin.py | 41 +++++++------------ 5 files changed, 22 insertions(+), 33 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index e4a94e20d..b3a603d84 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -924,7 +924,6 @@ class DomainInformationAdmin(ListHeaderAdmin): "no_other_contacts_rationale", "anything_else", "is_policy_acknowledged", - "other_contacts", ] # For each filter_horizontal, init in admin js extendFilterHorizontalWidgets @@ -1133,8 +1132,6 @@ class DomainRequestAdmin(ListHeaderAdmin): "requested_domain", "approved_domain", "alternative_domains", - "other_contacts", - "current_websites", "purpose", "submitter", "no_other_contacts_rationale", @@ -1310,7 +1307,7 @@ class DomainRequestAdmin(ListHeaderAdmin): readonly_fields.extend([field.name for field in self.model._meta.fields]) # Add the multi-select fields to readonly_fields: # Complex fields like ManyToManyField require special handling - readonly_fields.extend(["current_websites", "other_contacts", "alternative_domains"]) + readonly_fields.extend(["alternative_domains"]) if request.user.has_perm("registrar.full_access_permission"): return readonly_fields @@ -1421,7 +1418,6 @@ class DomainAdmin(ListHeaderAdmin): ) def queryset(self, request, queryset): - logger.debug(self.value()) if self.value() == "1": return queryset.filter(domain_info__is_election_board=True) if self.value() == "0": diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 3d72332fd..f3208e346 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -175,6 +175,12 @@ function checkToListThenInitWidget(toListId, attempts) { document.addEventListener('DOMContentLoaded', function() { // Select all table rows that have a data-contact-id attribute let contactRows = document.querySelectorAll("tr[data-contact-url]"); + console.log(`contact rows at this point: ${contactRows}`) + console.log(`length of ont ${contactRows.length}`) + if (!contactRows || contactRows.length == 0) { + console.log("in if statement") + contactRows = document.querySelectorAll(".other-contact-id") + } if (contactRows){ // Add a click event listener to each row 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 4ca577688..577bd9f41 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -58,6 +58,7 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% for contact in all_contacts %}
+ {% include "django/admin/includes/contact_detail_table.html" with user=contact field_name="other_contact" no_title_top_padding=field.is_readonly %}
{% endfor %} diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 8aa696aaf..50fdde35d 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -204,7 +204,6 @@ class GenericTestHelper(TestCase): {"action": "delete_selected", "select_across": selected_across, "index": index, "_selected_action": "23"}, follow=True, ) - print(f"what is the response? {response}") return response diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index a6b2d9777..bc2eb7ca1 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1335,9 +1335,9 @@ class TestDomainRequestAdmin(MockEppLib): ] 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) + # count=5 because the underlying domain has two users with this name. + # The dropdown has 4 of these. + self.assertContains(response, "Testy Tester", count=5) # Check for table titles. We only need to check for the end tag # (Otherwise this test will fail if we change classes, etc) @@ -1446,6 +1446,7 @@ class TestDomainRequestAdmin(MockEppLib): self.assertContains(response, "Yes, select ineligible status") def test_readonly_when_restricted_creator(self): + self.maxDiff = None with less_console_noise(): domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW) with boto3_mocking.clients.handler_for("sesv2", self.mock_client): @@ -1458,6 +1459,8 @@ class TestDomainRequestAdmin(MockEppLib): readonly_fields = self.admin.get_readonly_fields(request, domain_request) expected_fields = [ + "other_contacts", + "current_websites", "id", "created_at", "updated_at", @@ -1490,8 +1493,6 @@ class TestDomainRequestAdmin(MockEppLib): "is_policy_acknowledged", "submission_date", "notes", - "current_websites", - "other_contacts", "alternative_domains", ] @@ -1505,13 +1506,13 @@ class TestDomainRequestAdmin(MockEppLib): readonly_fields = self.admin.get_readonly_fields(request) expected_fields = [ + "other_contacts", + "current_websites", "creator", "about_your_organization", "requested_domain", "approved_domain", "alternative_domains", - "other_contacts", - "current_websites", "purpose", "submitter", "no_other_contacts_rationale", @@ -1528,7 +1529,7 @@ class TestDomainRequestAdmin(MockEppLib): readonly_fields = self.admin.get_readonly_fields(request) - expected_fields = [] + expected_fields = ['other_contacts', 'current_websites'] self.assertEqual(readonly_fields, expected_fields) @@ -1972,6 +1973,7 @@ class TestDomainInformationAdmin(TestCase): p = "userpass" self.client.login(username="staffuser", password=p) + response = self.client.get( "/admin/registrar/domaininformation/{}/change/".format(domain_info.pk), follow=True, @@ -2063,21 +2065,9 @@ class TestDomainInformationAdmin(TestCase): ] 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) + # count=5 because the underlying domain has two users with this name. + # The dropdown has 4 of these. + self.assertContains(response, "Testy Tester", count=5) # == Test the other_employees field == # expected_other_employees_fields = [ @@ -2088,9 +2078,6 @@ class TestDomainInformationAdmin(TestCase): ] 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(): @@ -2100,6 +2087,7 @@ class TestDomainInformationAdmin(TestCase): readonly_fields = self.admin.get_readonly_fields(request) expected_fields = [ + "other_contacts", "creator", "type_of_work", "more_organization_information", @@ -2109,7 +2097,6 @@ class TestDomainInformationAdmin(TestCase): "no_other_contacts_rationale", "anything_else", "is_policy_acknowledged", - "other_contacts", ] self.assertEqual(readonly_fields, expected_fields) From bd9c6213e864aed93a65c82adf20e81188390c0b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 26 Mar 2024 14:50:00 -0600 Subject: [PATCH 06/14] Linting --- src/registrar/tests/test_admin.py | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index bc2eb7ca1..fbdf8de7e 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1339,18 +1339,6 @@ class TestDomainRequestAdmin(MockEppLib): # The dropdown has 4 of these. self.assertContains(response, "Testy Tester", count=5) - # 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 @@ -1360,9 +1348,6 @@ class TestDomainRequestAdmin(MockEppLib): ] 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_save_model_sets_restricted_status_on_user(self): with less_console_noise(): # make sure there is no user with this email @@ -1529,7 +1514,7 @@ class TestDomainRequestAdmin(MockEppLib): readonly_fields = self.admin.get_readonly_fields(request) - expected_fields = ['other_contacts', 'current_websites'] + expected_fields = ["other_contacts", "current_websites"] self.assertEqual(readonly_fields, expected_fields) From 22412c1958ccba7ec0bf0718298c461d7e4985ec Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 27 Mar 2024 08:17:52 -0600 Subject: [PATCH 07/14] Change background color --- src/registrar/assets/sass/_theme/_admin.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 912a67f10..979d17ed3 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -351,7 +351,7 @@ details.dja-detail-table { display: inline-table; background-color: var(--body-bg); .dja-details-summary { - color: var(--header-link-color); + color: var(--body-quiet-color); } tr { From d9b4d58e552353d64044e545e1573b8a1d44f9f2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 27 Mar 2024 13:52:44 -0600 Subject: [PATCH 08/14] Refactor --- src/registrar/assets/sass/_theme/_admin.scss | 40 +++++++++ .../admin/includes/contact_detail_list.html | 47 ++++++++++ .../admin/includes/contact_detail_table.html | 67 --------------- .../admin/includes/detail_table_fieldset.html | 86 +++++++++++-------- 4 files changed, 137 insertions(+), 103 deletions(-) create mode 100644 src/registrar/templates/django/admin/includes/contact_detail_list.html delete mode 100644 src/registrar/templates/django/admin/includes/contact_detail_table.html diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 979d17ed3..e98c27174 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -389,6 +389,46 @@ table.dja-user-detail-table { } +ul.padding-left-0__detail-list { + padding-left: 0 !important; +} + +ul.margin-left-0__detail-list { + margin-left: 0 !important; +} + +ul.margin-top-neg-1__detail-list { + margin-top: -8px !important; +} + +.dja-detail-list { + dl { + padding-left: 0px !important; + } + // Mimic the normal label size + dt { + font-size: 0.8125rem; + color: var(--body-quiet-color); + } + + address { + font-size: 0.8125rem; + color: var(--body-quiet-color); + } +} + +// Mimic the normal label size +@media (max-width: 1024px){ + .dja-detail-list dt { + font-size: 0.875rem; + color: var(--body-quiet-color); + } + .dja-detail-list address { + font-size: 0.875rem; + color: var(--body-quiet-color); + } +} + .errors span.select2-selection { border: 1px solid var(--error-fg) !important; } \ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/contact_detail_list.html b/src/registrar/templates/django/admin/includes/contact_detail_list.html new file mode 100644 index 000000000..f01c5680f --- /dev/null +++ b/src/registrar/templates/django/admin/includes/contact_detail_list.html @@ -0,0 +1,47 @@ +{% load i18n static %} + + +
    + {% if user.title or user.contact.title or user.email or user.contact.email or user.phone or user.contact.phone %} + {# Title #} +
  • + {% if user.title or user.contact.title %} + {% if user.contact.title %} + {{ user.contact.title }} + {% else %} + {{ user.title }} + {% endif %} + {% else %} + None + {% endif %} +
    • + {# Email #} +
  • + {% if user.email or user.contact.email %} + {% if user.contact.email %} + {{ user.contact.email }} + {% else %} + {{ user.email }} + {% endif %} + {% else %} + None + {% endif %} +
    • + {# Phone #} +
  • + {% if user.phone or user.contact.phone %} + {% if user.contact.phone %} + {{ user.contact.phone.as_national }} + {% else %} + {{ user.phone.as_national }} + {% endif %} + {% else %} + None + {% endif %} +
    • + {% else %} +
  • + No additional contact information found. +
    • + {% endif %} +
diff --git a/src/registrar/templates/django/admin/includes/contact_detail_table.html b/src/registrar/templates/django/admin/includes/contact_detail_table.html deleted file mode 100644 index decce0f43..000000000 --- a/src/registrar/templates/django/admin/includes/contact_detail_table.html +++ /dev/null @@ -1,67 +0,0 @@ -{% load i18n static %} - - - {% if show_table_details %} - - - - - - {% endif %} - - {% if user.title or user.contact.title or user.email or user.contact.email or user.phone or user.contact.phone %} - - - {% if show_table_details %} - - {% endif %} - - {% if user.title or user.contact.title %} - {% if user.contact.title %} - - {% else %} - - {% endif %} - {% else %} - - {% endif %} - - - {% if show_table_details %} - - {% endif %} - - {% if user.email or user.contact.email %} - {% if user.contact.email %} - - {% else %} - - {% endif %} - {% else %} - - {% endif %} - - - {% if show_table_details %} - - {% endif %} - - {% if user.phone or user.contact.phone %} - {% if user.contact.phone %} - - {% else %} - - {% endif %} - {% else %} - - {% endif %} - - - {% else %} - - - - - - {% endif %} -
Contact details
Title{{ user.contact.title }}{{ user.title }}None
Email{{ user.contact.email }}{{ user.email }}None
Phone{{ user.contact.phone }}{{ user.phone }}None
No additional contact information found.
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 577bd9f41..01b9e6a8c 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -5,64 +5,78 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% endcomment %} {% block field_readonly %} + {% with all_contacts=original.other_contacts.all %} {% 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 %} -
- {% with total_websites=field.contents|split:", " %} - {% for website in total_websites %} - {{ website }}{% if not forloop.last %}, {% endif %} - {# Acts as a
#} - {% if total_websites|length < 5 %} -
+ {% if all_contacts.count > 2 %} +
+ {% for contact in field.contents|split:", " %} + {{ contact }}{% if not forloop.last %}, {% endif %} + {% endfor %} +
+ {% else %} +
+
+ {% for contact in all_contacts %} +
+ Organization contact {{forloop.counter}} +
+
+
+ {% if contact.get_formatted_name %}{{ contact.get_formatted_name }}
{% else %}None
{% endif %} + {% if contact.title %}{{ contact.title }}
{% else %}None
{% endif %} + {% if contact.email %}{{ contact.email }}
{% else %}None
{% endif %} + {% if contact.phone %}{{ contact.phone.as_national }}{% else %}None{% endif %} +
+
+ {% endfor %} +
+
{% endif %} - {% endfor %} - {% endwith %} -
+ {% 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 %} +
+ {% with total_websites=field.contents|split:", " %} + {% for website in total_websites %} + {{ website }}{% if not forloop.last %}, {% endif %} + {# Acts as a
#} + {% if total_websites|length < 5 %} +
+ {% endif %} + {% endfor %} + {% endwith %} +
{% else %}
{{ field.contents }}
{% endif %} + {% endwith %} {% 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" no_title_top_padding=field.is_readonly %} + {% include "django/admin/includes/contact_detail_list.html" with user=original.creator field_name="creator" no_title_top_padding=field.is_readonly %}
{% elif field.field.name == "submitter" %}
- {% include "django/admin/includes/contact_detail_table.html" with user=original.submitter field_name="submitter" no_title_top_padding=field.is_readonly %} + {% include "django/admin/includes/contact_detail_list.html" with user=original.submitter field_name="submitter" no_title_top_padding=field.is_readonly %}
{% elif field.field.name == "authorizing_official" %}
- {% include "django/admin/includes/contact_detail_table.html" with user=original.authorizing_official field_name="authorizing_official" no_title_top_padding=field.is_readonly %} + {% include "django/admin/includes/contact_detail_list.html" with user=original.authorizing_official field_name="authorizing_official" no_title_top_padding=field.is_readonly %}
{% elif field.field.name == "other_contacts" and original.other_contacts.all %} {% with all_contacts=original.other_contacts.all %} - {% if all_contacts.count == 1 %} - {% for contact in all_contacts %} -
- - - {% include "django/admin/includes/contact_detail_table.html" with user=contact field_name="other_contact" no_title_top_padding=field.is_readonly %} -
- {% endfor %} - {% else %} + {% if all_contacts.count > 2 %}
Details
From f53a4414eff163af76074241896b5c147c9df78d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 27 Mar 2024 14:57:41 -0600 Subject: [PATCH 09/14] Fix tests, cleanup --- src/registrar/assets/js/get-gov-admin.js | 3 - src/registrar/assets/sass/_theme/_admin.scss | 37 +++++------- src/registrar/templates/admin/fieldset.html | 19 ++---- .../admin/includes/contact_detail_list.html | 49 +++++++-------- .../admin/includes/detail_table_fieldset.html | 8 +-- src/registrar/tests/test_admin.py | 60 +++++++++---------- 6 files changed, 74 insertions(+), 102 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index f3208e346..f7ea0fa1d 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -175,10 +175,7 @@ function checkToListThenInitWidget(toListId, attempts) { document.addEventListener('DOMContentLoaded', function() { // Select all table rows that have a data-contact-id attribute let contactRows = document.querySelectorAll("tr[data-contact-url]"); - console.log(`contact rows at this point: ${contactRows}`) - console.log(`length of ont ${contactRows.length}`) if (!contactRows || contactRows.length == 0) { - console.log("in if statement") contactRows = document.querySelectorAll(".other-contact-id") } diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index e98c27174..fe3fcb89c 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -354,6 +354,13 @@ details.dja-detail-table { color: var(--body-quiet-color); } + @media (max-width: 1024px){ + .dja-detail-contents { + max-width: 400px !important; + overflow-x: scroll !important; + } + } + tr { background-color: transparent; } @@ -371,34 +378,18 @@ details.dja-detail-table { } -table.dja-user-detail-table { - tr { - background-color: var(--body-bg); +ul { + .padding-left-0__detail-list { + padding-left: 0 !important; } - // Make the head of the user detail table "float" - thead > tr > th { - border-radius: 4px; - border-top: none; - border-bottom: none; + .margin-left-0__detail-list { + margin-left: 0 !important; } - th, td { - border: none + .margin-top-neg-1__detail-list { + margin-top: -8px !important; } - -} - -ul.padding-left-0__detail-list { - padding-left: 0 !important; -} - -ul.margin-left-0__detail-list { - margin-left: 0 !important; -} - -ul.margin-top-neg-1__detail-list { - margin-top: -8px !important; } .dja-detail-list { diff --git a/src/registrar/templates/admin/fieldset.html b/src/registrar/templates/admin/fieldset.html index 579ad7033..8b8972e08 100644 --- a/src/registrar/templates/admin/fieldset.html +++ b/src/registrar/templates/admin/fieldset.html @@ -6,17 +6,11 @@ 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/includes/fieldset.html {% endcomment %}
- {# .gov override #} - {% block fieldset_title %} - {% if fieldset.name %}

{{ fieldset.name }}

{% endif %} - {% endblock fieldset_title %} + {% if fieldset.name %}

{{ fieldset.name }}

{% endif %} - {# .gov override #} - {% block fieldset_description %} - {% if fieldset.description %} -
{{ fieldset.description|safe }}
- {% endif %} - {% endblock fieldset_description %} + {% if fieldset.description %} +
{{ fieldset.description|safe }}
+ {% endif %} {% for line in fieldset %}
@@ -45,11 +39,6 @@ https://github.com/django/django/blob/main/django/contrib/admin/templates/admin/ {% endif %} {% endif %}
- - {# .gov addition #} - {% block before_help_text %} - {# For templating purposes #} - {% endblock before_help_text %} {% if field.field.help_text %} {# .gov override #} diff --git a/src/registrar/templates/django/admin/includes/contact_detail_list.html b/src/registrar/templates/django/admin/includes/contact_detail_list.html index f01c5680f..b0b21ea8e 100644 --- a/src/registrar/templates/django/admin/includes/contact_detail_list.html +++ b/src/registrar/templates/django/admin/includes/contact_detail_list.html @@ -4,41 +4,36 @@
    {% if user.title or user.contact.title or user.email or user.contact.email or user.phone or user.contact.phone %} {# Title #} -
  • - {% if user.title or user.contact.title %} - {% if user.contact.title %} - {{ user.contact.title }} - {% else %} - {{ user.title }} - {% endif %} + {% if user.title or user.contact.title %} + {% if user.contact.title %} +
  • {{ user.contact.title }}
    • {% else %} - None +
  • {{ user.title }}
    • {% endif %} - + {% else %} +
  • None
    • + {% endif %} {# Email #} -
  • - {% if user.email or user.contact.email %} - {% if user.contact.email %} - {{ user.contact.email }} - {% else %} - {{ user.email }} - {% endif %} + {% if user.email or user.contact.email %} + {% if user.contact.email %} +
  • {{ user.contact.email }}
    • {% else %} - None +
  • {{ user.email }}
    • {% endif %} - + {% else %} +
  • None
    • + {% endif %} + {# Phone #} -
  • - {% if user.phone or user.contact.phone %} - {% if user.contact.phone %} - {{ user.contact.phone.as_national }} - {% else %} - {{ user.phone.as_national }} - {% endif %} + {% if user.phone or user.contact.phone %} + {% if user.contact.phone %}
    • +
  • {{ user.contact.phone }}
    • {% else %} - None +
  • {{ user.phone }}
    • {% endif %} - + {% else %} +
  • None
    • + {% endif %} {% else %}
  • No additional contact information found. 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 01b9e6a8c..cf8e0f752 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -22,10 +22,10 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
    - {% if contact.get_formatted_name %}{{ contact.get_formatted_name }}
    {% else %}None
    {% endif %} - {% if contact.title %}{{ contact.title }}
    {% else %}None
    {% endif %} - {% if contact.email %}{{ contact.email }}
    {% else %}None
    {% endif %} - {% if contact.phone %}{{ contact.phone.as_national }}{% else %}None{% endif %} + {% if contact.get_formatted_name %}{{ contact.get_formatted_name }}
    {% else %}None
    {% endif %} + {% if contact.title %}{{ contact.title }}
    {% else %}None
    {% endif %} + {% if contact.email %}{{ contact.email }}
    {% else %}None
    {% endif %} + {% if contact.phone %}{{ contact.phone }}{% else %}None{% endif %}
    {% endfor %} diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index fbdf8de7e..df3d760a8 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1307,9 +1307,9 @@ class TestDomainRequestAdmin(MockEppLib): # (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"), + ("title", "Treat inspector
    • "), + ("email", "meoward.jones@igorville.gov"), + ("phone", "(555) 123 12345"), ] self.test_helper.assert_response_contains_distinct_values(response, expected_creator_fields) @@ -1319,9 +1319,9 @@ class TestDomainRequestAdmin(MockEppLib): # == Check for the submitter == # expected_submitter_fields = [ # Field, expected value - ("title", "Admin Tester"), - ("email", "mayor@igorville.gov"), - ("phone", "(555) 555 5556"), + ("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") @@ -1329,22 +1329,22 @@ class TestDomainRequestAdmin(MockEppLib): # == Check for the authorizing_official == # expected_ao_fields = [ # Field, expected value - ("title", "Chief Tester"), - ("email", "testy@town.com"), - ("phone", "(555) 555 5555"), + ("title", "Chief Tester"), + ("email", "testy@town.com"), + ("phone", "(555) 555 5555"), ] self.test_helper.assert_response_contains_distinct_values(response, expected_ao_fields) - # count=5 because the underlying domain has two users with this name. - # The dropdown has 4 of these. - self.assertContains(response, "Testy Tester", count=5) + # count=3 because the underlying domain has two users with this name. + # The dropdown has 2 of these. + self.assertContains(response, "Testy Tester", 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"), + ("title", "Another Tester"), + ("email", "testy2@town.com"), + ("phone", "(555) 555 5557"), ] self.test_helper.assert_response_contains_distinct_values(response, expected_other_employees_fields) @@ -2022,9 +2022,9 @@ class TestDomainInformationAdmin(TestCase): # (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"), + ("title", "Treat inspector"), + ("email", "meoward.jones@igorville.gov"), + ("phone", "(555) 123 12345"), ] self.test_helper.assert_response_contains_distinct_values(response, expected_creator_fields) @@ -2034,9 +2034,9 @@ class TestDomainInformationAdmin(TestCase): # == Check for the submitter == # expected_submitter_fields = [ # Field, expected value - ("title", "Admin Tester"), - ("email", "mayor@igorville.gov"), - ("phone", "(555) 555 5556"), + ("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") @@ -2044,22 +2044,22 @@ class TestDomainInformationAdmin(TestCase): # == Check for the authorizing_official == # expected_ao_fields = [ # Field, expected value - ("title", "Chief Tester"), - ("email", "testy@town.com"), - ("phone", "(555) 555 5555"), + ("title", "Chief Tester"), + ("email", "testy@town.com"), + ("phone", "(555) 555 5555"), ] self.test_helper.assert_response_contains_distinct_values(response, expected_ao_fields) - # count=5 because the underlying domain has two users with this name. - # The dropdown has 4 of these. - self.assertContains(response, "Testy Tester", count=5) + # count=3 because the underlying domain has two users with this name. + # The dropdown has 2 of these. + self.assertContains(response, "Testy Tester", 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"), + ("title", "Another Tester"), + ("email", "testy2@town.com"), + ("phone", "(555) 555 5557"), ] self.test_helper.assert_response_contains_distinct_values(response, expected_other_employees_fields) From 25704e2814e5f98962f39362823a706822bb60ec Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 27 Mar 2024 15:04:40 -0600 Subject: [PATCH 10/14] Update _admin.scss --- src/registrar/assets/sass/_theme/_admin.scss | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index fe3fcb89c..6310fa08e 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -378,18 +378,16 @@ details.dja-detail-table { } -ul { - .padding-left-0__detail-list { - padding-left: 0 !important; - } +ul.padding-left-0__detail-list { + padding-left: 0 !important; +} - .margin-left-0__detail-list { - margin-left: 0 !important; - } +ul.margin-left-0__detail-list { + margin-left: 0 !important; +} - .margin-top-neg-1__detail-list { - margin-top: -8px !important; - } +ul.margin-top-neg-1__detail-list { + margin-top: -8px !important; } .dja-detail-list { From 3e740f46684d672ca417da873091857dad5490a2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 27 Mar 2024 15:46:21 -0600 Subject: [PATCH 11/14] Update detail_table_fieldset.html --- .../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 cf8e0f752..28883207f 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -77,7 +77,7 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% elif field.field.name == "other_contacts" and original.other_contacts.all %} {% with all_contacts=original.other_contacts.all %} {% if all_contacts.count > 2 %} -
    +
    Details
    From ea59232cefc90d03bb7f473efa0a7f4c261311ab Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 28 Mar 2024 09:15:48 -0600 Subject: [PATCH 12/14] PR suggestions (slight refactor) --- src/registrar/admin.py | 2 +- src/registrar/assets/js/get-gov-admin.js | 29 ----------- src/registrar/assets/sass/_theme/_admin.scss | 16 +++--- .../admin/includes/contact_detail_list.html | 39 +++++++++------ .../admin/includes/detail_table_fieldset.html | 35 ++++++------- src/registrar/tests/test_admin.py | 50 +++++++++---------- 6 files changed, 73 insertions(+), 98 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index b3a603d84..c5dbb152b 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1123,7 +1123,7 @@ class DomainRequestAdmin(ListHeaderAdmin): ] # Readonly fields for analysts and superusers - readonly_fields = ("other_contacts", "current_websites") + readonly_fields = ("other_contacts", "current_websites", "alternative_domains") # Read only that we'll leverage for CISA Analysts analyst_readonly_fields = [ diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index f7ea0fa1d..8c60c534f 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -170,35 +170,6 @@ function checkToListThenInitWidget(toListId, attempts) { } } -(function() { - function addUrlsToOtherContacts() { - document.addEventListener('DOMContentLoaded', function() { - // Select all table rows that have a data-contact-id attribute - let contactRows = document.querySelectorAll("tr[data-contact-url]"); - if (!contactRows || contactRows.length == 0) { - contactRows = document.querySelectorAll(".other-contact-id") - } - - if (contactRows){ - // Add a click event listener to each row - let index = 1; - contactRows.forEach(function(row) { - let otherContactUrl = row.getAttribute("data-contact-url"); - if (otherContactUrl){ - let otherContact = document.querySelector(`.other-contact__${index}`); - if (otherContact) { - otherContact.href = otherContactUrl; - } - } - index++; - }); - } - }); - } - - addUrlsToOtherContacts() -})(); - // Initialize the widget: // add related buttons to the widget for edit, delete and view // add event listeners on the from list, the to list, and selector buttons which either enable or disable the related buttons diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 6310fa08e..4bddfd1df 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -351,6 +351,7 @@ details.dja-detail-table { display: inline-table; background-color: var(--body-bg); .dja-details-summary { + cursor: pointer; color: var(--body-quiet-color); } @@ -378,21 +379,15 @@ details.dja-detail-table { } -ul.padding-left-0__detail-list { - padding-left: 0 !important; -} -ul.margin-left-0__detail-list { - margin-left: 0 !important; -} - -ul.margin-top-neg-1__detail-list { +address.margin-top-neg-1__detail-list { margin-top: -8px !important; } .dja-detail-list { dl { padding-left: 0px !important; + margin-top: 5px !important; } // Mimic the normal label size dt { @@ -406,6 +401,11 @@ ul.margin-top-neg-1__detail-list { } } +address.dja-address-contact-list { + font-size: 0.8125rem; + color: var(--body-quiet-color); +} + // Mimic the normal label size @media (max-width: 1024px){ .dja-detail-list dt { diff --git a/src/registrar/templates/django/admin/includes/contact_detail_list.html b/src/registrar/templates/django/admin/includes/contact_detail_list.html index b0b21ea8e..6afe4c8d6 100644 --- a/src/registrar/templates/django/admin/includes/contact_detail_list.html +++ b/src/registrar/templates/django/admin/includes/contact_detail_list.html @@ -1,42 +1,51 @@ {% load i18n static %} +
    + + {% if show_formatted_name %} + {% if contact.get_formatted_name %} + {{ user.get_formatted_name }}
    + {% else %} + None
    + {% endif %} + {% endif %} -
      {% if user.title or user.contact.title or user.email or user.contact.email or user.phone or user.contact.phone %} {# Title #} {% if user.title or user.contact.title %} {% if user.contact.title %} -
    • {{ user.contact.title }}
      • + {{ user.contact.title }} {% else %} -
    • {{ user.title }}
      • + {{ user.title }} {% endif %} +
      {% else %} -
    • None
      • + None
      {% endif %} {# Email #} {% if user.email or user.contact.email %} {% if user.contact.email %} -
    • {{ user.contact.email }}
      • + {{ user.contact.email }} {% else %} -
    • {{ user.email }}
      • + {{ user.email }} {% endif %} +
      {% else %} -
    • None
      • + None
      {% endif %} {# Phone #} {% if user.phone or user.contact.phone %} - {% if user.contact.phone %} -
    • {{ user.contact.phone }}
      • + {% if user.contact.phone %} + {{ user.contact.phone }} {% else %} -
    • {{ user.phone }}
      • + {{ user.phone }} {% endif %} +
      {% else %} -
    • None
      • + None
      {% endif %} {% else %} -
    • - No additional contact information found. -
      • + No additional contact information found. {% endif %} -
    +
    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 28883207f..47145faf2 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -9,24 +9,19 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% if field.field.name == "other_contacts" %} {% if all_contacts.count > 2 %}
    - {% for contact in field.contents|split:", " %} - {{ contact }}{% if not forloop.last %}, {% endif %} + {% for contact in all_contacts %} + {{ contact.get_formatted_name }}{% if not forloop.last %}, {% endif %} {% endfor %}
    {% else %}
    -
    +
    {% for contact in all_contacts %}
    Organization contact {{forloop.counter}}
    -
    - {% if contact.get_formatted_name %}{{ contact.get_formatted_name }}
    {% else %}None
    {% endif %} - {% if contact.title %}{{ contact.title }}
    {% else %}None
    {% endif %} - {% if contact.email %}{{ contact.email }}
    {% else %}None
    {% endif %} - {% if contact.phone %}{{ contact.phone }}{% else %}None{% endif %} -
    + {% include "django/admin/includes/contact_detail_list.html" with user=contact show_formatted_name=True %}
    {% endfor %}
    @@ -52,6 +47,12 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% endfor %} {% endwith %}
    + {% elif field.field.name == "alternative_domains" %} +
    + {% for alt_domain in original.alternative_domains.all %} + {{ alt_domain }}{% if not forloop.last %}, {% endif %} + {% endfor %} +
    {% else %}
    {{ field.contents }}
    {% endif %} @@ -62,17 +63,17 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% if field.field.name == "creator" %}
    - {% include "django/admin/includes/contact_detail_list.html" with user=original.creator field_name="creator" no_title_top_padding=field.is_readonly %} + {% include "django/admin/includes/contact_detail_list.html" with user=original.creator no_title_top_padding=field.is_readonly %}
    {% elif field.field.name == "submitter" %}
    - {% include "django/admin/includes/contact_detail_list.html" with user=original.submitter field_name="submitter" no_title_top_padding=field.is_readonly %} + {% include "django/admin/includes/contact_detail_list.html" with user=original.submitter no_title_top_padding=field.is_readonly %}
    {% elif field.field.name == "authorizing_official" %}
    - {% include "django/admin/includes/contact_detail_list.html" with user=original.authorizing_official field_name="authorizing_official" no_title_top_padding=field.is_readonly %} + {% include "django/admin/includes/contact_detail_list.html" with user=original.authorizing_official no_title_top_padding=field.is_readonly %}
    {% elif field.field.name == "other_contacts" and original.other_contacts.all %} {% with all_contacts=original.other_contacts.all %} @@ -83,17 +84,13 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
    - + {% for contact in all_contacts %} - {% 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 %} - - + + diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index df3d760a8..3d88bd8bd 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1303,13 +1303,11 @@ class TestDomainRequestAdmin(MockEppLib): # == 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"), + ("title", "Treat inspector"), + ("email", "meoward.jones@igorville.gov"), + ("phone", "(555) 123 12345"), ] self.test_helper.assert_response_contains_distinct_values(response, expected_creator_fields) @@ -1319,9 +1317,9 @@ class TestDomainRequestAdmin(MockEppLib): # == Check for the submitter == # expected_submitter_fields = [ # Field, expected value - ("title", "Admin Tester"), - ("email", "mayor@igorville.gov"), - ("phone", "(555) 555 5556"), + ("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") @@ -1329,15 +1327,15 @@ class TestDomainRequestAdmin(MockEppLib): # == Check for the authorizing_official == # expected_ao_fields = [ # Field, expected value - ("title", "Chief Tester"), - ("email", "testy@town.com"), - ("phone", "(555) 555 5555"), + ("title", "Chief Tester"), + ("email", "testy@town.com"), + ("phone", "(555) 555 5555"), ] self.test_helper.assert_response_contains_distinct_values(response, expected_ao_fields) - # count=3 because the underlying domain has two users with this name. - # The dropdown has 2 of these. - self.assertContains(response, "Testy Tester", count=3) + # count=5 because the underlying domain has two users with this name. + # The dropdown has 3 of these. + self.assertContains(response, "Testy Tester", count=5) # == Test the other_employees field == # expected_other_employees_fields = [ @@ -2022,9 +2020,9 @@ class TestDomainInformationAdmin(TestCase): # (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"), + ("title", "Treat inspector"), + ("email", "meoward.jones@igorville.gov"), + ("phone", "(555) 123 12345"), ] self.test_helper.assert_response_contains_distinct_values(response, expected_creator_fields) @@ -2034,9 +2032,9 @@ class TestDomainInformationAdmin(TestCase): # == Check for the submitter == # expected_submitter_fields = [ # Field, expected value - ("title", "Admin Tester"), - ("email", "mayor@igorville.gov"), - ("phone", "(555) 555 5556"), + ("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") @@ -2044,15 +2042,15 @@ class TestDomainInformationAdmin(TestCase): # == Check for the authorizing_official == # expected_ao_fields = [ # Field, expected value - ("title", "Chief Tester"), - ("email", "testy@town.com"), - ("phone", "(555) 555 5555"), + ("title", "Chief Tester"), + ("email", "testy@town.com"), + ("phone", "(555) 555 5555"), ] self.test_helper.assert_response_contains_distinct_values(response, expected_ao_fields) - # count=3 because the underlying domain has two users with this name. - # The dropdown has 2 of these. - self.assertContains(response, "Testy Tester", count=3) + # count=5 because the underlying domain has two users with this name. + # The dropdown has 3 of these. + self.assertContains(response, "Testy Tester", count=5) # == Test the other_employees field == # expected_other_employees_fields = [ From cf6b8283b6fe7aebfa130c14269ee459c12b9be9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 28 Mar 2024 09:17:13 -0600 Subject: [PATCH 13/14] Tests --- src/registrar/tests/test_admin.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 3d88bd8bd..e8937e822 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1444,6 +1444,7 @@ class TestDomainRequestAdmin(MockEppLib): expected_fields = [ "other_contacts", "current_websites", + "alternative_domains", "id", "created_at", "updated_at", @@ -1491,6 +1492,7 @@ class TestDomainRequestAdmin(MockEppLib): expected_fields = [ "other_contacts", "current_websites", + "alternative_domains", "creator", "about_your_organization", "requested_domain", @@ -1512,7 +1514,7 @@ class TestDomainRequestAdmin(MockEppLib): readonly_fields = self.admin.get_readonly_fields(request) - expected_fields = ["other_contacts", "current_websites"] + expected_fields = ["other_contacts", "current_websites", "alternative_domains",] self.assertEqual(readonly_fields, expected_fields) From 3a519f75ab28c1c11543c8391b54f6589cea68f4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 28 Mar 2024 09:25:15 -0600 Subject: [PATCH 14/14] Linting --- src/registrar/tests/test_admin.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index e8937e822..a53cfe9b8 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1514,7 +1514,11 @@ class TestDomainRequestAdmin(MockEppLib): readonly_fields = self.admin.get_readonly_fields(request) - expected_fields = ["other_contacts", "current_websites", "alternative_domains",] + expected_fields = [ + "other_contacts", + "current_websites", + "alternative_domains", + ] self.assertEqual(readonly_fields, expected_fields)
    Contact informationOther contact information
    {{contact.first_name}} {{contact.last_name}}
    {{ contact.get_formatted_name }} {{ contact.title }} {{ contact.email }} {{ contact.phone }}