From caed970e14b02b2994c849421d1d4007921d5e5f Mon Sep 17 00:00:00 2001
From: CocoByte <nicolle.leclair@gmail.com>
Date: Sun, 17 Mar 2024 22:01:28 -0600
Subject: [PATCH 01/31] Made other contacts and current websites read-only for
 analysts

---
 src/registrar/admin.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 4968da527..10ea1af29 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -1098,6 +1098,8 @@ class DomainRequestAdmin(ListHeaderAdmin):
         "requested_domain",
         "approved_domain",
         "alternative_domains",
+        "other_contacts",
+        "current_websites",
         "purpose",
         "submitter",
         "no_other_contacts_rationale",

From 2ad571354e16046a53ddba402be2ea8cfb1017ce Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Tue, 19 Mar 2024 11:15:05 -0600
Subject: [PATCH 02/31] Basics

---
 src/registrar/admin.py                        |  4 ++
 src/registrar/templates/admin/fieldset.html   | 49 ++++++++++++++
 .../admin/domain_request_change_form.html     |  8 +++
 .../includes/domain_request_fieldset.html     | 65 +++++++++++++++++++
 4 files changed, 126 insertions(+)
 create mode 100644 src/registrar/templates/admin/fieldset.html
 create mode 100644 src/registrar/templates/django/admin/domain_request_change_form.html
 create mode 100644 src/registrar/templates/django/admin/includes/domain_request_fieldset.html

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 5f571ac73..a5ad62492 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -1099,6 +1099,8 @@ class DomainRequestAdmin(ListHeaderAdmin):
         "about_your_organization",
         "requested_domain",
         "approved_domain",
+        "other_contacts",
+        "current_websites",
         "alternative_domains",
         "purpose",
         "submitter",
@@ -1119,6 +1121,8 @@ class DomainRequestAdmin(ListHeaderAdmin):
     # Table ordering
     ordering = ["requested_domain__name"]
 
+    change_form_template = "django/admin/domain_request_change_form.html"
+
     # Trigger action when a fieldset is changed
     def save_model(self, request, obj, form, change):
         """Custom save_model definition that handles edge cases"""
diff --git a/src/registrar/templates/admin/fieldset.html b/src/registrar/templates/admin/fieldset.html
new file mode 100644
index 000000000..dadae5811
--- /dev/null
+++ b/src/registrar/templates/admin/fieldset.html
@@ -0,0 +1,49 @@
+{% 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/includes/fieldset.html
+{% endcomment %}
+<fieldset class="module aligned {{ fieldset.classes }}">
+    {% block fieldset_title %}
+        {% if fieldset.name %}<h2>{{ fieldset.name }}</h2>{% endif %}
+    {% endblock fieldset_title %}
+
+    {% block fieldset_description %}
+        {% if fieldset.description %}
+            <div class="description">{{ fieldset.description|safe }}</div>
+        {% endif %}
+    {% endblock fieldset_description %}
+
+    {% block fieldset_lines %}
+        {% for line in fieldset %}
+            <div class="form-row{% if line.fields|length == 1 and line.errors %} errors{% endif %}{% if not line.has_visible_field %} hidden{% endif %}{% for field in line %}{% if field.field.name %} field-{{ field.field.name }}{% endif %}{% endfor %}">
+                {% if line.fields|length == 1 %}{{ line.errors }}{% else %}<div class="flex-container form-multiline">{% endif %}
+                {% for field in line %}
+                    <div>
+                        {% if not line.fields|length == 1 and not field.is_readonly %}{{ field.errors }}{% endif %}
+                            <div class="flex-container{% if not line.fields|length == 1 %} fieldBox{% if field.field.name %} field-{{ field.field.name }}{% endif %}{% if not field.is_readonly and field.errors %} errors{% endif %}{% if field.field.is_hidden %} hidden{% endif %}{% elif field.is_checkbox %} checkbox-row{% endif %}">
+                                {% if field.is_checkbox %}
+                                    {{ field.field }}{{ field.label_tag }}
+                                {% else %}
+                                    {{ field.label_tag }}
+                                    {% if field.is_readonly %}
+                                        <div class="readonly">{{ field.contents }}</div>
+                                    {% else %}
+                                        {{ field.field }}
+                                    {% endif %}
+                                {% endif %}
+                            </div>
+                        {% if field.field.help_text %}
+                            <div class="help"{% if field.field.id_for_label %} id="{{ field.field.id_for_label }}_helptext"{% endif %}>
+                                <div>{{ field.field.help_text|safe }}</div>
+                            </div>
+                        {% endif %}
+                    </div>
+                {% endfor %}
+                {% if not line.fields|length == 1 %}</div>{% endif %}
+            </div>
+        {% endfor %}
+    {% endblock fieldset_lines %}
+</fieldset>
\ No newline at end of file
diff --git a/src/registrar/templates/django/admin/domain_request_change_form.html b/src/registrar/templates/django/admin/domain_request_change_form.html
new file mode 100644
index 000000000..e48ab37a0
--- /dev/null
+++ b/src/registrar/templates/django/admin/domain_request_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_request_fieldset.html" %}
+    {% endfor %}
+{% endblock %}
diff --git a/src/registrar/templates/django/admin/includes/domain_request_fieldset.html b/src/registrar/templates/django/admin/includes/domain_request_fieldset.html
new file mode 100644
index 000000000..fb7232a45
--- /dev/null
+++ b/src/registrar/templates/django/admin/includes/domain_request_fieldset.html
@@ -0,0 +1,65 @@
+{% extends "admin/fieldset.html" %}
+
+{% comment %}
+This is using a custom implementation fieldset.html (see admin/fieldset.html)
+{% endcomment %}
+{% block fieldset_lines %}
+{% for line in fieldset %}
+    <div class="form-row{% if line.fields|length == 1 and line.errors %} errors{% endif %}{% if not line.has_visible_field %} hidden{% endif %}{% for field in line %}{% if field.field.name %} field-{{ field.field.name }}{% endif %}{% endfor %}">
+        {% if line.fields|length == 1 %}{{ line.errors }}{% else %}<div class="flex-container form-multiline">{% endif %}
+        {% for field in line %}
+            <div>
+                {% if not line.fields|length == 1 and not field.is_readonly %}{{ field.errors }}{% endif %}
+                    <div class="flex-container{% if not line.fields|length == 1 %} fieldBox{% if field.field.name %} field-{{ field.field.name }}{% endif %}{% if not field.is_readonly and field.errors %} errors{% endif %}{% if field.field.is_hidden %} hidden{% endif %}{% elif field.is_checkbox %} checkbox-row{% endif %}">
+                        {% if field.is_checkbox %}
+                            {{ field.field }}{{ field.label_tag }}
+                        {% else %}
+                            {{ field.label_tag }}
+                            {% if field.is_readonly %}
+                                <div class="readonly">{{ field.contents }}</div>
+                            {% else %}
+                                {{ field.field }}
+                            {% endif %}
+                        {% endif %}
+                    </div>
+                {% if field.field.name == "creator" %}
+                    <ul>
+                        {% if original.creator.title %}
+                        <li>{{ original.creator.title }}</li>
+                        {% endif %}
+
+                        {% if original.creator.email %}
+                            <li>{{ original.creator.email }}</li>
+                        {% endif %}
+
+                        {% if original.creator.phone %}
+                            <li>{{ original.creator.phone }}</li>
+                        {% endif %}
+                    </ul>
+                {% elif field.field.name == "submitter" %}
+                    <ul>
+                        {% if original.submitter.title %}
+                        <li>{{ original.submitter.title }}</li>
+                        {% endif %}
+
+                        {% if original.submitter.email %}
+                            <li>{{ original.submitter.email }}</li>
+                        {% endif %}
+
+                        {% if original.submitter.phone %}
+                            <li>{{ original.submitter.phone }}</li>
+                        {% endif %}
+                    </ul>
+                {% endif %}
+                {% if field.field.help_text %}
+                    <div class="help"{% if field.field.id_for_label %} id="{{ field.field.id_for_label }}_helptext"{% endif %}>
+                        <div>{{ field.field.help_text|safe }}</div>
+                    </div>
+                {% endif %}
+            </div>
+        {% endfor %}
+        {% if not line.fields|length == 1 %}</div>{% endif %}
+    </div>
+{% endfor %}
+
+{% endblock fieldset_lines %}

From 1841b2f728ae1675f6b5729774eee88bcb85147c Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Tue, 19 Mar 2024 12:46:22 -0600
Subject: [PATCH 03/31] Styling

---
 src/registrar/assets/sass/_theme/_admin.scss  | 14 +++++
 .../includes/domain_request_fieldset.html     | 52 ++++++++++++++-----
 2 files changed, 53 insertions(+), 13 deletions(-)

diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss
index 4f715de3c..596de17ac 100644
--- a/src/registrar/assets/sass/_theme/_admin.scss
+++ b/src/registrar/assets/sass/_theme/_admin.scss
@@ -316,3 +316,17 @@ input.admin-confirm-button {
 .usa-summary-box__dhs-color {
     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);
+        }
+    }
+}
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 fb7232a45..a05a80f46 100644
--- a/src/registrar/templates/django/admin/includes/domain_request_fieldset.html
+++ b/src/registrar/templates/django/admin/includes/domain_request_fieldset.html
@@ -37,19 +37,45 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
                         {% endif %}
                     </ul>
                 {% elif field.field.name == "submitter" %}
-                    <ul>
-                        {% if original.submitter.title %}
-                        <li>{{ original.submitter.title }}</li>
-                        {% endif %}
-
-                        {% if original.submitter.email %}
-                            <li>{{ original.submitter.email }}</li>
-                        {% endif %}
-
-                        {% if original.submitter.phone %}
-                            <li>{{ original.submitter.phone }}</li>
-                        {% endif %}
-                    </ul>
+                    <details class="dja-detail-table" open>
+                        <summary class="padding-1 dja-details-summary">Details</summary>
+                        <div class="grid-container padding-left-0 padding-right-0 dja-details-contents">
+                            <table>
+                                <tbody>
+                                    <tr>
+                                        <th scope="row">Title</th>
+                                        <td>{{ original.submitter.title }}</td>
+                                    </tr>
+                                    <tr>
+                                        <th scope="row">Email</th>
+                                        <td>{{ original.submitter.email }}</td>
+                                    </tr>
+                                    <tr>
+                                        <th scope="row">Phone</th>
+                                        <td>{{ original.submitter.phone }}</td>
+                                    </tr>
+                                </tbody>
+                            </table>
+                        </div>
+                    </details>
+                {% elif field.field.name == "other_contacts" %}
+                    <details class="dja-detail-table">
+                        <summary class="padding-1 dja-details-summary">Details</summary>
+                        <div class="grid-container padding-left-0 padding-right-0 dja-details-contents">
+                            <table>
+                                <tbody>
+                                    {% for contact in original.other_contacts.all %}
+                                    <tr>
+                                        <th scope="row">{{contact.first_name}} {{contact.last_name}}</th>
+                                        <td>{{ contact.title }}</td>
+                                        <td>{{ contact.email }}</td>
+                                        <td>{{ contact.phone }}</td>
+                                    </tr>
+                                    {% endfor %}
+                                </tbody>
+                            </table>
+                        </div>
+                    </details>
                 {% endif %}
                 {% if field.field.help_text %}
                     <div class="help"{% if field.field.id_for_label %} id="{{ field.field.id_for_label }}_helptext"{% endif %}>

From 7ea27e3aa287be3c6a1c1039e3127f5e08bc62d1 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Tue, 19 Mar 2024 13:50:25 -0600
Subject: [PATCH 04/31] Slight abstraction

---
 .../includes/domain_request_detail_table.html | 27 ++++++++++++
 .../includes/domain_request_fieldset.html     | 41 +++----------------
 2 files changed, 33 insertions(+), 35 deletions(-)
 create mode 100644 src/registrar/templates/django/admin/includes/domain_request_detail_table.html

diff --git a/src/registrar/templates/django/admin/includes/domain_request_detail_table.html b/src/registrar/templates/django/admin/includes/domain_request_detail_table.html
new file mode 100644
index 000000000..5fbffa425
--- /dev/null
+++ b/src/registrar/templates/django/admin/includes/domain_request_detail_table.html
@@ -0,0 +1,27 @@
+{% load i18n static %}
+
+{% if start_opened %}
+<details class="margin-top-1 dja-detail-table" open>
+{% else %}
+<details class="margin-top-1 dja-detail-table">
+{% endif%}
+    <summary class="padding-1 dja-details-summary">Details</summary>
+    <div class="grid-container padding-left-0 padding-right-0 dja-details-contents">
+        <table>
+            <tbody>
+                <tr>
+                    <th scope="row">Title</th>
+                    <td>{{ user.title }}</td>
+                </tr>
+                <tr>
+                    <th scope="row">Email</th>
+                    <td>{{ user.email }}</td>
+                </tr>
+                <tr>
+                    <th scope="row">Phone</th>
+                    <td>{{ user.phone }}</td>
+                </tr>
+            </tbody>
+        </table>
+    </div>
+</details>
\ No newline at end of file
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 a05a80f46..d034d2441 100644
--- a/src/registrar/templates/django/admin/includes/domain_request_fieldset.html
+++ b/src/registrar/templates/django/admin/includes/domain_request_fieldset.html
@@ -23,43 +23,13 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
                         {% endif %}
                     </div>
                 {% if field.field.name == "creator" %}
-                    <ul>
-                        {% if original.creator.title %}
-                        <li>{{ original.creator.title }}</li>
-                        {% endif %}
-
-                        {% if original.creator.email %}
-                            <li>{{ original.creator.email }}</li>
-                        {% endif %}
-
-                        {% if original.creator.phone %}
-                            <li>{{ original.creator.phone }}</li>
-                        {% endif %}
-                    </ul>
+                    {% include "django/admin/includes/domain_request_detail_table.html" with user=original.creator start_opened=True %}
                 {% elif field.field.name == "submitter" %}
-                    <details class="dja-detail-table" open>
-                        <summary class="padding-1 dja-details-summary">Details</summary>
-                        <div class="grid-container padding-left-0 padding-right-0 dja-details-contents">
-                            <table>
-                                <tbody>
-                                    <tr>
-                                        <th scope="row">Title</th>
-                                        <td>{{ original.submitter.title }}</td>
-                                    </tr>
-                                    <tr>
-                                        <th scope="row">Email</th>
-                                        <td>{{ original.submitter.email }}</td>
-                                    </tr>
-                                    <tr>
-                                        <th scope="row">Phone</th>
-                                        <td>{{ original.submitter.phone }}</td>
-                                    </tr>
-                                </tbody>
-                            </table>
-                        </div>
-                    </details>
+                    {% include "django/admin/includes/domain_request_detail_table.html" with user=original.submitter start_opened=True %}
+                {% elif field.field.name == "authorizing_official" %}
+                    {% include "django/admin/includes/domain_request_detail_table.html" with user=original.authorizing_official start_opened=True %}
                 {% elif field.field.name == "other_contacts" %}
-                    <details class="dja-detail-table">
+                    <details class="margin-top-1 dja-detail-table">
                         <summary class="padding-1 dja-details-summary">Details</summary>
                         <div class="grid-container padding-left-0 padding-right-0 dja-details-contents">
                             <table>
@@ -77,6 +47,7 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
                         </div>
                     </details>
                 {% endif %}
+
                 {% if field.field.help_text %}
                     <div class="help"{% if field.field.id_for_label %} id="{{ field.field.id_for_label }}_helptext"{% endif %}>
                         <div>{{ field.field.help_text|safe }}</div>

From 983f1bc00ee1dbca0cb278450b3f495f713f7b92 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Tue, 19 Mar 2024 14:06:00 -0600
Subject: [PATCH 05/31] Update domain_request_detail_table.html

---
 .../admin/includes/domain_request_detail_table.html   | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/registrar/templates/django/admin/includes/domain_request_detail_table.html b/src/registrar/templates/django/admin/includes/domain_request_detail_table.html
index 5fbffa425..f34ffa491 100644
--- a/src/registrar/templates/django/admin/includes/domain_request_detail_table.html
+++ b/src/registrar/templates/django/admin/includes/domain_request_detail_table.html
@@ -1,12 +1,10 @@
 {% load i18n static %}
 
-{% if start_opened %}
-<details class="margin-top-1 dja-detail-table" open>
-{% else %}
-<details class="margin-top-1 dja-detail-table">
-{% endif%}
+
+<details class="margin-top-1 dja-detail-table" {% if start_opened %}open{% endif %}>
     <summary class="padding-1 dja-details-summary">Details</summary>
     <div class="grid-container padding-left-0 padding-right-0 dja-details-contents">
+        {% if user.title or user.email or user.phone %}
         <table>
             <tbody>
                 <tr>
@@ -23,5 +21,8 @@
                 </tr>
             </tbody>
         </table>
+        {% else %}
+        <div class="padding-1">No details found</div>
+        {% endif %}
     </div>
 </details>
\ No newline at end of file

From 9952d248f6dd3fba0808debb354b3d9ad563c6ea Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Tue, 19 Mar 2024 14:07:02 -0600
Subject: [PATCH 06/31] Update admin.py

---
 src/registrar/admin.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 2059d1d69..38f3ecb2c 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -1099,8 +1099,6 @@ class DomainRequestAdmin(ListHeaderAdmin):
         "about_your_organization",
         "requested_domain",
         "approved_domain",
-        "other_contacts",
-        "current_websites",
         "alternative_domains",
         "other_contacts",
         "current_websites",

From a8c10816b8e6913b57ba9369d7adae4fcbb0eb10 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Tue, 19 Mar 2024 15:49:32 -0600
Subject: [PATCH 07/31] Add links to urls

---
 src/registrar/assets/js/get-gov-admin.js      | 44 +++++++++++++++++++
 src/registrar/assets/sass/_theme/_admin.scss  | 32 ++++++++++++++
 .../includes/domain_request_detail_table.html | 42 +++++++-----------
 .../includes/domain_request_fieldset.html     | 24 +++++++---
 src/registrar/templatetags/url_helpers.py     |  7 +++
 5 files changed, 117 insertions(+), 32 deletions(-)

diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js
index 4ed00c33f..28149999c 100644
--- a/src/registrar/assets/js/get-gov-admin.js
+++ b/src/registrar/assets/js/get-gov-admin.js
@@ -170,6 +170,50 @@ 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){
+                // 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()
+})();
+
+document.addEventListener('DOMContentLoaded', function() {
+    // Select all table rows that have a data-contact-id attribute
+    let contactRows = document.querySelectorAll("tr[data-contact-url]");
+
+    // Add a click event listener to each row
+    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++
+    });
+});
+
 // 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 596de17ac..98e5009db 100644
--- a/src/registrar/assets/sass/_theme/_admin.scss
+++ b/src/registrar/assets/sass/_theme/_admin.scss
@@ -330,3 +330,35 @@ details.dja-detail-table {
         }
     }
 }
+
+table.dja-user-detail-table {
+    margin-left: 160px;
+    tr {
+        background-color: var(--body-bg);
+    }
+}
+
+.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;
+    }
+}
+
+
diff --git a/src/registrar/templates/django/admin/includes/domain_request_detail_table.html b/src/registrar/templates/django/admin/includes/domain_request_detail_table.html
index f34ffa491..fba588b61 100644
--- a/src/registrar/templates/django/admin/includes/domain_request_detail_table.html
+++ b/src/registrar/templates/django/admin/includes/domain_request_detail_table.html
@@ -1,28 +1,18 @@
 {% load i18n static %}
 
-
-<details class="margin-top-1 dja-detail-table" {% if start_opened %}open{% endif %}>
-    <summary class="padding-1 dja-details-summary">Details</summary>
-    <div class="grid-container padding-left-0 padding-right-0 dja-details-contents">
-        {% if user.title or user.email or user.phone %}
-        <table>
-            <tbody>
-                <tr>
-                    <th scope="row">Title</th>
-                    <td>{{ user.title }}</td>
-                </tr>
-                <tr>
-                    <th scope="row">Email</th>
-                    <td>{{ user.email }}</td>
-                </tr>
-                <tr>
-                    <th scope="row">Phone</th>
-                    <td>{{ user.phone }}</td>
-                </tr>
-            </tbody>
-        </table>
-        {% else %}
-        <div class="padding-1">No details found</div>
-        {% endif %}
-    </div>
-</details>
\ No newline at end of file
+<table class="dja-user-detail-table">
+    <tbody>
+        <tr>
+            <th class="padding-left-0" scope="row">Title</th>
+            <td>{{ user.title }}</td>
+        </tr>
+        <tr>
+            <th class="padding-left-0" scope="row">Email</th>
+            <td>{{ user.email }}</td>
+        </tr>
+        <tr>
+            <th class="padding-left-0" scope="row">Phone</th>
+            <td>{{ user.phone }}</td>
+        </tr>
+    </tbody>
+</table>
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 d034d2441..c90423474 100644
--- a/src/registrar/templates/django/admin/includes/domain_request_fieldset.html
+++ b/src/registrar/templates/django/admin/includes/domain_request_fieldset.html
@@ -1,5 +1,5 @@
 {% extends "admin/fieldset.html" %}
-
+{% load static url_helpers %}
 {% comment %}
 This is using a custom implementation fieldset.html (see admin/fieldset.html)
 {% endcomment %}
@@ -16,18 +16,26 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
                         {% else %}
                             {{ field.label_tag }}
                             {% if field.is_readonly %}
-                                <div class="readonly">{{ field.contents }}</div>
+                                {% if field.field.name == "other_contacts" %}
+                                    <div class="readonly">
+                                        {% for contact in field.contents|split:", " %}
+                                            <a href="#" class="other-contact__{{forloop.counter}}">{{ contact }}</a>{% if not forloop.last %}, {% endif %}
+                                        {% endfor %}
+                                    </div>
+                                {% else %}
+                                    <div class="readonly">{{ field.contents }}</div>
+                                {% endif %}
                             {% else %}
                                 {{ field.field }}
                             {% endif %}
                         {% endif %}
                     </div>
                 {% if field.field.name == "creator" %}
-                    {% include "django/admin/includes/domain_request_detail_table.html" with user=original.creator start_opened=True %}
+                    {% include "django/admin/includes/domain_request_detail_table.html" with user=original.creator %}
                 {% elif field.field.name == "submitter" %}
-                    {% include "django/admin/includes/domain_request_detail_table.html" with user=original.submitter start_opened=True %}
+                    {% include "django/admin/includes/domain_request_detail_table.html" with user=original.submitter %}
                 {% elif field.field.name == "authorizing_official" %}
-                    {% include "django/admin/includes/domain_request_detail_table.html" with user=original.authorizing_official start_opened=True %}
+                    {% include "django/admin/includes/domain_request_detail_table.html" with user=original.authorizing_official %}
                 {% elif field.field.name == "other_contacts" %}
                     <details class="margin-top-1 dja-detail-table">
                         <summary class="padding-1 dja-details-summary">Details</summary>
@@ -35,7 +43,11 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
                             <table>
                                 <tbody>
                                     {% for contact in original.other_contacts.all %}
-                                    <tr>
+                                    {% 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 %}
+                                    <tr data-contact-id="{{ contact.id }}" data-contact-url="{% url 'admin:registrar_contact_change' contact.id %}">
                                         <th scope="row">{{contact.first_name}} {{contact.last_name}}</th>
                                         <td>{{ contact.title }}</td>
                                         <td>{{ contact.email }}</td>
diff --git a/src/registrar/templatetags/url_helpers.py b/src/registrar/templatetags/url_helpers.py
index 931eedc92..9413892aa 100644
--- a/src/registrar/templatetags/url_helpers.py
+++ b/src/registrar/templatetags/url_helpers.py
@@ -26,6 +26,13 @@ def endswith(text, ends):
     return False
 
 
+@register.filter("split")
+def split_string(value, key):
+    """
+    Splits a given string
+    """
+    return value.split(key)
+
 @register.simple_tag
 def public_site_url(url_path):
     """Make a full URL for this path at our public site.

From e0aa729133043d7ca74c348b2b3cf8fe03ab78b3 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 20 Mar 2024 09:11:51 -0600
Subject: [PATCH 08/31] Fix merge conflicts

---
 src/registrar/assets/js/get-gov-admin.js      |  18 ---
 src/registrar/assets/js/get-gov-reports.js    | 122 ++++++++++--------
 .../admin/domain_application_change_form.html |  96 --------------
 .../admin/domain_request_change_form.html     |  90 +++++++++++++
 src/registrar/tests/test_admin.py             |   2 +
 5 files changed, 160 insertions(+), 168 deletions(-)
 delete mode 100644 src/registrar/templates/django/admin/domain_application_change_form.html

diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js
index fe03d6864..3d72332fd 100644
--- a/src/registrar/assets/js/get-gov-admin.js
+++ b/src/registrar/assets/js/get-gov-admin.js
@@ -196,24 +196,6 @@ function checkToListThenInitWidget(toListId, attempts) {
     addUrlsToOtherContacts()
 })();
 
-document.addEventListener('DOMContentLoaded', function() {
-    // Select all table rows that have a data-contact-id attribute
-    let contactRows = document.querySelectorAll("tr[data-contact-url]");
-
-    // Add a click event listener to each row
-    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++
-    });
-});
-
 // 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/js/get-gov-reports.js b/src/registrar/assets/js/get-gov-reports.js
index d10cf2dc6..2fc9b6219 100644
--- a/src/registrar/assets/js/get-gov-reports.js
+++ b/src/registrar/assets/js/get-gov-reports.js
@@ -54,64 +54,78 @@
 
 })();
 
-document.addEventListener("DOMContentLoaded", function () {
-    createComparativeColumnChart("myChart1", "Managed domains", "Start Date", "End Date");
-    createComparativeColumnChart("myChart2", "Unmanaged domains", "Start Date", "End Date");
-    createComparativeColumnChart("myChart3", "Deleted domains", "Start Date", "End Date");
-    createComparativeColumnChart("myChart4", "Ready domains", "Start Date", "End Date");
-    createComparativeColumnChart("myChart5", "Submitted requests", "Start Date", "End Date");
-    createComparativeColumnChart("myChart6", "All requests", "Start Date", "End Date");
-});
 
-function createComparativeColumnChart(canvasId, title, labelOne, labelTwo) {
-    var canvas = document.getElementById(canvasId);
-    var ctx = canvas.getContext("2d");
+/** An IIFE to initialize the analytics page
+*/
+(function () {
+    function createComparativeColumnChart(canvasId, title, labelOne, labelTwo) {
+        var canvas = document.getElementById(canvasId);
+        if (!canvas) {
+            console.log("Could not find canvas")
+            return
+        }
 
-    var listOne = JSON.parse(canvas.getAttribute('data-list-one'));
-    var listTwo = JSON.parse(canvas.getAttribute('data-list-two'));
+        var ctx = canvas.getContext("2d");
 
-    var data = {
-        labels: ["Total", "Federal", "Interstate", "State/Territory", "Tribal", "County", "City", "Special District", "School District", "Election Board"],
-        datasets: [
-            {
-                label: labelOne,
-                backgroundColor: "rgba(255, 99, 132, 0.2)",
-                borderColor: "rgba(255, 99, 132, 1)",
-                borderWidth: 1,
-                data: listOne,
+        var listOne = JSON.parse(canvas.getAttribute('data-list-one'));
+        var listTwo = JSON.parse(canvas.getAttribute('data-list-two'));
+
+        var data = {
+            labels: ["Total", "Federal", "Interstate", "State/Territory", "Tribal", "County", "City", "Special District", "School District", "Election Board"],
+            datasets: [
+                {
+                    label: labelOne,
+                    backgroundColor: "rgba(255, 99, 132, 0.2)",
+                    borderColor: "rgba(255, 99, 132, 1)",
+                    borderWidth: 1,
+                    data: listOne,
+                },
+                {
+                    label: labelTwo,
+                    backgroundColor: "rgba(75, 192, 192, 0.2)",
+                    borderColor: "rgba(75, 192, 192, 1)",
+                    borderWidth: 1,
+                    data: listTwo,
+                },
+            ],
+        };
+
+        var options = {
+            responsive: true,
+            maintainAspectRatio: false,
+            plugins: {
+                legend: {
+                    position: 'top',
+                },
+                title: {
+                    display: true,
+                    text: title
+                }
             },
-            {
-                label: labelTwo,
-                backgroundColor: "rgba(75, 192, 192, 0.2)",
-                borderColor: "rgba(75, 192, 192, 1)",
-                borderWidth: 1,
-                data: listTwo,
+            scales: {
+                y: {
+                    beginAtZero: true,
+                },
             },
-        ],
+        };
+
+        new Chart(ctx, {
+            type: "bar",
+            data: data,
+            options: options,
+        });
+    }
+
+    function initComparativeColumnCharts() {
+        document.addEventListener("DOMContentLoaded", function () {
+            createComparativeColumnChart("myChart1", "Managed domains", "Start Date", "End Date");
+            createComparativeColumnChart("myChart2", "Unmanaged domains", "Start Date", "End Date");
+            createComparativeColumnChart("myChart3", "Deleted domains", "Start Date", "End Date");
+            createComparativeColumnChart("myChart4", "Ready domains", "Start Date", "End Date");
+            createComparativeColumnChart("myChart5", "Submitted requests", "Start Date", "End Date");
+            createComparativeColumnChart("myChart6", "All requests", "Start Date", "End Date");
+        });
     };
 
-    var options = {
-        responsive: true,
-        maintainAspectRatio: false,
-        plugins: {
-            legend: {
-                position: 'top',
-            },
-            title: {
-                display: true,
-                text: title
-            }
-        },
-        scales: {
-            y: {
-                beginAtZero: true,
-            },
-        },
-    };
-
-    new Chart(ctx, {
-        type: "bar",
-        data: data,
-        options: options,
-    });
-}
\ No newline at end of file
+    initComparativeColumnCharts();
+})();
\ No newline at end of file
diff --git a/src/registrar/templates/django/admin/domain_application_change_form.html b/src/registrar/templates/django/admin/domain_application_change_form.html
deleted file mode 100644
index 95392da1e..000000000
--- a/src/registrar/templates/django/admin/domain_application_change_form.html
+++ /dev/null
@@ -1,96 +0,0 @@
-{% extends 'admin/change_form.html' %}
-{% load i18n static %}
-
-{% block field_sets %}
-    {# Create an invisible <a> tag so that we can use a click event to toggle the modal. #}
-    <a id="invisible-ineligible-modal-toggler" class="display-none" href="#toggle-set-ineligible" aria-controls="toggle-set-ineligible" data-open-modal></a>
-    {{ block.super }}
-{% endblock %}
-
-{% block submit_buttons_bottom %}
-    {% comment %}
-    Modals behave very weirdly in django admin.
-    They tend to "strip out" any injected form elements, leaving only the main form.
-    In addition, USWDS handles modals by first destroying the element, then repopulating it toward the end of the page.
-    In effect, this means that the modal is not, and cannot, be surrounded by any form element at compile time.
-
-    The current workaround for this is to use javascript to inject a hidden input, and bind submit of that
-    element to the click of the confirmation button within this modal.
-
-    This is controlled by the class `dja-form-placeholder` on the button.
-
-    In addition, the modal element MUST be placed low in the DOM. The script loads slower on DJA than on other portions
-    of the application, so this means that it will briefly "populate", causing unintended visual effects.
-    {% endcomment %}
-    {# Create a modal for when a domain is marked as ineligible #}
-    <div
-    class="usa-modal"
-    id="toggle-set-ineligible"
-    aria-labelledby="Are you sure you want to select ineligible status?"
-    aria-describedby="This request will be marked as ineligible."
-    >
-        <div class="usa-modal__content">
-            <div class="usa-modal__main">
-                <h2 class="usa-modal__heading" id="modal-1-heading">
-                    Are you sure you want to select ineligible status?
-                </h2>
-                <div class="usa-prose">
-                    <p>
-                        When a domain request is in ineligible status, the registrant's permissions within the registrar are restricted as follows: 
-                    </p>
-                    <ul>
-                        <li class="font-body-sm">They cannot edit the ineligible request or any other pending requests.</li>
-                        <li class="font-body-sm">They cannot manage any of their approved domains.</li>
-                        <li class="font-body-sm">They cannot initiate a new domain request.</li>
-                    </ul>
-                    <p>
-                        The restrictions will not take effect until you “save” the changes for this domain request. 
-                        This action can be reversed, if needed.
-                    </p>
-                    <p>
-                        Domain: <b>{{ original.requested_domain.name }}</b>
-                        {# Acts as a <br> #}
-                        <div class="display-inline"></div>
-                        New status: <b>{{ original.DomainRequestStatus.INELIGIBLE|capfirst }}</b>
-                    </p>
-                </div>
-
-                <div class="usa-modal__footer">
-                    <ul class="usa-button-group">
-                        <li class="usa-button-group__item">
-                            <button
-                            type="submit" 
-                            class="usa-button" 
-                            name="_set_domain_request_ineligible"
-                            data-close-modal
-                            >
-                                Yes, select ineligible status
-                            </button>
-                        </li>
-                        <li class="usa-button-group__item">
-                            <button
-                            type="button"
-                            class="usa-button usa-button--unstyled padding-105 text-center"
-                            name="_cancel_domain_request_ineligible"
-                            data-close-modal
-                            >
-                                Cancel
-                            </button>
-                        </li>
-                    </ul>
-                </div>
-            </div>
-            <button
-            type="button"
-            class="usa-button usa-modal__close"
-            aria-label="Close this window"
-            data-close-modal
-            >
-                <svg class="usa-icon" aria-hidden="true" focusable="false" role="img">
-                    <use xlink:href="{%static 'img/sprite.svg'%}#close"></use>
-                </svg>
-            </button>
-        </div>
-</div>
-{{ block.super }}
-{% endblock %}
\ No newline at end of file
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 e48ab37a0..5d5e6ed60 100644
--- a/src/registrar/templates/django/admin/domain_request_change_form.html
+++ b/src/registrar/templates/django/admin/domain_request_change_form.html
@@ -2,7 +2,97 @@
 {% load i18n static %}
 
 {% block field_sets %}
+    {# Create an invisible <a> tag so that we can use a click event to toggle the modal. #}
+    <a id="invisible-ineligible-modal-toggler" class="display-none" href="#toggle-set-ineligible" aria-controls="toggle-set-ineligible" data-open-modal></a>
     {% for fieldset in adminform %}
         {% include "django/admin/includes/domain_request_fieldset.html" %}
     {% endfor %}
 {% endblock %}
+
+{% block submit_buttons_bottom %}
+    {% comment %}
+    Modals behave very weirdly in django admin.
+    They tend to "strip out" any injected form elements, leaving only the main form.
+    In addition, USWDS handles modals by first destroying the element, then repopulating it toward the end of the page.
+    In effect, this means that the modal is not, and cannot, be surrounded by any form element at compile time.
+
+    The current workaround for this is to use javascript to inject a hidden input, and bind submit of that
+    element to the click of the confirmation button within this modal.
+
+    This is controlled by the class `dja-form-placeholder` on the button.
+
+    In addition, the modal element MUST be placed low in the DOM. The script loads slower on DJA than on other portions
+    of the application, so this means that it will briefly "populate", causing unintended visual effects.
+    {% endcomment %}
+    {# Create a modal for when a domain is marked as ineligible #}
+    <div
+    class="usa-modal"
+    id="toggle-set-ineligible"
+    aria-labelledby="Are you sure you want to select ineligible status?"
+    aria-describedby="This request will be marked as ineligible."
+    >
+        <div class="usa-modal__content">
+            <div class="usa-modal__main">
+                <h2 class="usa-modal__heading" id="modal-1-heading">
+                    Are you sure you want to select ineligible status?
+                </h2>
+                <div class="usa-prose">
+                    <p>
+                        When a domain request is in ineligible status, the registrant's permissions within the registrar are restricted as follows: 
+                    </p>
+                    <ul>
+                        <li class="font-body-sm">They cannot edit the ineligible request or any other pending requests.</li>
+                        <li class="font-body-sm">They cannot manage any of their approved domains.</li>
+                        <li class="font-body-sm">They cannot initiate a new domain request.</li>
+                    </ul>
+                    <p>
+                        The restrictions will not take effect until you “save” the changes for this domain request. 
+                        This action can be reversed, if needed.
+                    </p>
+                    <p>
+                        Domain: <b>{{ original.requested_domain.name }}</b>
+                        {# Acts as a <br> #}
+                        <div class="display-inline"></div>
+                        New status: <b>{{ original.DomainRequestStatus.INELIGIBLE|capfirst }}</b>
+                    </p>
+                </div>
+
+                <div class="usa-modal__footer">
+                    <ul class="usa-button-group">
+                        <li class="usa-button-group__item">
+                            <button
+                            type="submit" 
+                            class="usa-button" 
+                            name="_set_domain_request_ineligible"
+                            data-close-modal
+                            >
+                                Yes, select ineligible status
+                            </button>
+                        </li>
+                        <li class="usa-button-group__item">
+                            <button
+                            type="button"
+                            class="usa-button usa-button--unstyled padding-105 text-center"
+                            name="_cancel_domain_request_ineligible"
+                            data-close-modal
+                            >
+                                Cancel
+                            </button>
+                        </li>
+                    </ul>
+                </div>
+            </div>
+            <button
+            type="button"
+            class="usa-button usa-modal__close"
+            aria-label="Close this window"
+            data-close-modal
+            >
+                <svg class="usa-icon" aria-hidden="true" focusable="false" role="img">
+                    <use xlink:href="{%static 'img/sprite.svg'%}#close"></use>
+                </svg>
+            </button>
+        </div>
+</div>
+{{ block.super }}
+{% endblock %}
diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index 9666633e9..d22973971 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -1339,6 +1339,8 @@ class TestDomainRequestAdmin(MockEppLib):
                 "about_your_organization",
                 "requested_domain",
                 "approved_domain",
+                "other_contacts",
+                "current_websites",
                 "alternative_domains",
                 "purpose",
                 "submitter",

From 54603cbfad06d667781ff9b733149363813b0517 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 20 Mar 2024 11:02:25 -0600
Subject: [PATCH 09/31] Unit tests, refactoring

---
 docs/developer/README.md                      | 10 ++++
 .../includes/domain_request_detail_table.html | 32 ++++++++--
 .../includes/domain_request_fieldset.html     |  6 +-
 src/registrar/tests/common.py                 |  1 +
 src/registrar/tests/test_admin.py             | 60 ++++++++++++++++++-
 5 files changed, 101 insertions(+), 8 deletions(-)

diff --git a/docs/developer/README.md b/docs/developer/README.md
index 9f2e131b6..a06202d8d 100644
--- a/docs/developer/README.md
+++ b/docs/developer/README.md
@@ -204,6 +204,16 @@ from .common import less_console_noise
             # <test code goes here>
 ```
 
+Or alternatively, if you prefer using a decorator, just use:
+
+```python
+from .common import less_console_noise_decorator
+
+@less_console_noise_decorator
+def some_function():
+  # <test code goes here>
+```
+
 ### Accessibility Testing in the browser
 
 We use the [ANDI](https://www.ssa.gov/accessibility/andi/help/install.html) browser extension 
diff --git a/src/registrar/templates/django/admin/includes/domain_request_detail_table.html b/src/registrar/templates/django/admin/includes/domain_request_detail_table.html
index fba588b61..c25812b4d 100644
--- a/src/registrar/templates/django/admin/includes/domain_request_detail_table.html
+++ b/src/registrar/templates/django/admin/includes/domain_request_detail_table.html
@@ -1,18 +1,42 @@
 {% load i18n static %}
 
-<table class="dja-user-detail-table">
+<table class="dja-user-detail-table" {% if field_name %} id="id_detail_table__{{field_name}}" {% endif %}>
     <tbody>
         <tr>
             <th class="padding-left-0" scope="row">Title</th>
-            <td>{{ user.title }}</td>
+            {% if user.title or user.contact.title %}
+                {% if user.contact.title %}
+                <td>{{ user.contact.title }}</td>
+                {% else %}
+                <td>{{ user.title }}</td>
+                {% endif %}
+            {% else %}
+            <td>Nothing found</td>
+            {% endif %}
         </tr>
         <tr>
             <th class="padding-left-0" scope="row">Email</th>
-            <td>{{ user.email }}</td>
+            {% if user.email or user.contact.email %}
+                {% if user.contact.email %}
+                <td>{{ user.contact.email }}</td>
+                {% else %}
+                <td>{{ user.email }}</td>
+                {% endif %}
+            {% else %}
+            <td>Nothing found</td>
+            {% endif %}
         </tr>
         <tr>
             <th class="padding-left-0" scope="row">Phone</th>
-            <td>{{ user.phone }}</td>
+            {% if user.phone or user.contact.phone %}
+                {% if user.contact.phone %}
+                <td>{{ user.contact.phone }}</td>
+                {% else %}
+                <td>{{ user.phone }}</td>
+                {% endif %}
+            {% else %}
+            <td>Nothing found</td>
+            {% endif %}
         </tr>
     </tbody>
 </table>
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 c90423474..efd90b909 100644
--- a/src/registrar/templates/django/admin/includes/domain_request_fieldset.html
+++ b/src/registrar/templates/django/admin/includes/domain_request_fieldset.html
@@ -31,11 +31,11 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
                         {% endif %}
                     </div>
                 {% if field.field.name == "creator" %}
-                    {% include "django/admin/includes/domain_request_detail_table.html" with user=original.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 %}
+                    {% 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 %}
+                    {% 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 class="margin-top-1 dja-detail-table">
                         <summary class="padding-1 dja-details-summary">Details</summary>
diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py
index 04eb4c82d..c7846d49d 100644
--- a/src/registrar/tests/common.py
+++ b/src/registrar/tests/common.py
@@ -734,6 +734,7 @@ def completed_domain_request(
     """A completed domain request."""
     if not user:
         user = get_user_model().objects.create(username="username" + str(uuid.uuid4())[:8])
+
     ao, _ = Contact.objects.get_or_create(
         first_name="Testy",
         last_name="Tester",
diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index d22973971..fce279416 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -2,6 +2,7 @@ from datetime import date
 from django.test import TestCase, RequestFactory, Client, override_settings
 from django.contrib.admin.sites import AdminSite
 from contextlib import ExitStack
+from api.tests.common import less_console_noise_decorator
 from django_webtest import WebTest  # type: ignore
 from django.contrib import messages
 from django.urls import reverse
@@ -1193,6 +1194,63 @@ class TestDomainRequestAdmin(MockEppLib):
             # Test that approved domain exists and equals requested domain
             self.assertEqual(domain_request.requested_domain.name, domain_request.approved_domain.name)
 
+    @less_console_noise_decorator
+    def test_creator_has_detail_table(self):
+        """Tests if the "creator" field has the detail table which displays title, email, and phone"""
+
+        # Create fake creator
+        _creator = User.objects.create(
+            username="MrMeoward",
+            first_name = "Meoward",
+            last_name = "Jones",
+            email="meoward.jones@igorville.gov",
+            phone="(555) 123 12345",
+            title="Treat inspector"
+        )
+        # Create a fake domain request
+        domain_request = completed_domain_request(
+            status=DomainRequest.DomainRequestStatus.IN_REVIEW,
+            creator=_creator
+        )
+
+        p = "userpass"
+        self.client.login(username="staffuser", password=p)
+        response = self.client.get(
+            "/admin/registrar/domainrequest/{}/change/".format(domain_request.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_request.requested_domain.name)
+
+        # Check that the modal has the right content
+        # Check for the header
+        
+        # Check for the creator
+        self.assertContains(response, "Meoward Jones")
+        # Check that it is readonly
+        self.assertContains(response, "")
+
+        # Check for the right title, email, and phone number in the response
+
+        # Title
+        self.assertContains(response, "Treat inspector")
+
+        # Email
+        self.assertContains(response, "meoward.jones@igorville.gov")
+
+        # Phone number
+        self.assertContains(response, "(555) 123 12345")
+
+        # Check for table titles
+
+        # Title. We only need to check for the end tag
+        # (Otherwise this test will fail if we change classes, etc)
+        self.assertContains(response, "Title</th>")
+        self.assertContains(response, "Email</th>")
+        self.assertContains(response, "Phone</th>")
+
     def test_save_model_sets_restricted_status_on_user(self):
         with less_console_noise():
             # make sure there is no user with this email
@@ -1339,9 +1397,9 @@ class TestDomainRequestAdmin(MockEppLib):
                 "about_your_organization",
                 "requested_domain",
                 "approved_domain",
+                "alternative_domains",
                 "other_contacts",
                 "current_websites",
-                "alternative_domains",
                 "purpose",
                 "submitter",
                 "no_other_contacts_rationale",

From b8b97245a460e8bbbcad749885d0f0f018f8d140 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 20 Mar 2024 11:47:06 -0600
Subject: [PATCH 10/31] More tests

---
 src/registrar/tests/test_admin.py | 127 ++++++++++++++++++++++++------
 1 file changed, 101 insertions(+), 26 deletions(-)

diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index fce279416..ef4d05e0b 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -1194,23 +1194,74 @@ class TestDomainRequestAdmin(MockEppLib):
             # Test that approved domain exists and equals requested domain
             self.assertEqual(domain_request.requested_domain.name, domain_request.approved_domain.name)
 
+    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</td>"),
+            ("email", "meoward.jones@igorville.gov</td>"),
+        ]
+        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_other_contacts_readonly_has_link(self):
+        """Tests if the readonly other_contacts field has links"""
+
+        # Create a fake domain request
+        domain_request = completed_domain_request(
+            status=DomainRequest.DomainRequestStatus.IN_REVIEW,
+            user=_creator
+        )
+
+        p = "userpass"
+        self.client.login(username="staffuser", password=p)
+        response = self.client.get(
+            "/admin/registrar/domainrequest/{}/change/".format(domain_request.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_request.requested_domain.name)
+
     @less_console_noise_decorator
-    def test_creator_has_detail_table(self):
-        """Tests if the "creator" field has the detail table which displays title, email, and phone"""
+    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",
-            email="meoward.jones@igorville.gov",
-            phone="(555) 123 12345",
-            title="Treat inspector"
         )
+
+        # 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,
-            creator=_creator
+            user=_creator
         )
 
         p = "userpass"
@@ -1227,29 +1278,53 @@ class TestDomainRequestAdmin(MockEppLib):
         # Check that the modal has the right content
         # Check for the header
         
-        # Check for the creator
+        # == Check for the creator == # 
         self.assertContains(response, "Meoward Jones")
-        # Check that it is readonly
-        self.assertContains(response, "")
 
-        # Check for the right title, email, and phone number in the response
-
-        # Title
-        self.assertContains(response, "Treat inspector")
-
-        # Email
-        self.assertContains(response, "meoward.jones@igorville.gov")
-
-        # Phone number
-        self.assertContains(response, "(555) 123 12345")
-
-        # Check for table titles
-
-        # Title. We only need to check for the end tag
+        # 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, "Title</th>")
-        self.assertContains(response, "Email</th>")
-        self.assertContains(response, "Phone</th>")
+        expected_creator_fields = [
+            # Field, expected value
+            ("title", "Treat inspector</td>"),
+            ("email", "meoward.jones@igorville.gov</td>"),
+            ("phone", "(555) 123 12345</td>")
+        ]
+        self.assert_response_contains_distinct_values(response, expected_creator_fields)
+
+        # == Check for the submitter == # 
+        self.assertContains(response, "Testy2 Tester2")
+
+        expected_submitter_fields = [
+            # Field, expected value
+            ("title", "Admin Tester</td>"),
+            ("email", "mayor@igorville.gov</td>"),
+            ("phone", "(555) 555 5556</td>")
+        ]
+        self.assert_response_contains_distinct_values(response, expected_submitter_fields)
+
+        # == Check for the authorizing_official == # 
+        self.assertContains(response, "Testy Tester")
+
+        expected_ao_fields = [
+            # Field, expected value
+            ("title", "Chief Tester</td>"),
+            ("email", "testy@town.com</td>"),
+            ("phone", "(555) 555 5555</td>")
+        ]
+        self.assert_response_contains_distinct_values(response, expected_ao_fields)
+
+        # 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</th>", count=3)
+
+        # Email. Count=3 because this table appears on three records.
+        self.assertContains(response, "Email</th>", count=3)
+
+        # Phone. Count=3 because this table appears on three records.
+        self.assertContains(response, "Phone</th>", count=3)
 
     def test_save_model_sets_restricted_status_on_user(self):
         with less_console_noise():

From 4830f5630f057b42b99d46127856c8bda393bbe2 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 20 Mar 2024 12:11:11 -0600
Subject: [PATCH 11/31] Add unit tests + linting

---
 src/registrar/templatetags/url_helpers.py |  1 +
 src/registrar/tests/test_admin.py         | 53 +++++++++++++----------
 2 files changed, 31 insertions(+), 23 deletions(-)

diff --git a/src/registrar/templatetags/url_helpers.py b/src/registrar/templatetags/url_helpers.py
index 9413892aa..e75b99ff3 100644
--- a/src/registrar/templatetags/url_helpers.py
+++ b/src/registrar/templatetags/url_helpers.py
@@ -33,6 +33,7 @@ def split_string(value, key):
     """
     return value.split(key)
 
+
 @register.simple_tag
 def public_site_url(url_path):
     """Make a full URL for this path at our public site.
diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index ef4d05e0b..970742ae3 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -1219,15 +1219,15 @@ class TestDomainRequestAdmin(MockEppLib):
             with self.subTest(field=field, expected_value=value):
                 self.assertContains(response, value, count=1)
 
-        @less_console_noise_decorator
-    def test_other_contacts_readonly_has_link(self):
+    @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
-        domain_request = completed_domain_request(
-            status=DomainRequest.DomainRequestStatus.IN_REVIEW,
-            user=_creator
-        )
+        domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW)
+
+        # Get the other contact
+        other_contact = domain_request.other_contacts.all().first()
 
         p = "userpass"
         self.client.login(username="staffuser", password=p)
@@ -1240,6 +1240,16 @@ class TestDomainRequestAdmin(MockEppLib):
         self.assertEqual(response.status_code, 200)
         self.assertContains(response, domain_request.requested_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 = f"Testy Tester</a>"
+        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"""
@@ -1247,8 +1257,8 @@ class TestDomainRequestAdmin(MockEppLib):
         # Create fake creator
         _creator = User.objects.create(
             username="MrMeoward",
-            first_name = "Meoward",
-            last_name = "Jones",
+            first_name="Meoward",
+            last_name="Jones",
         )
 
         # Due to the relation between User <==> Contact,
@@ -1259,10 +1269,7 @@ class TestDomainRequestAdmin(MockEppLib):
         _creator.contact.save()
 
         # Create a fake domain request
-        domain_request = completed_domain_request(
-            status=DomainRequest.DomainRequestStatus.IN_REVIEW,
-            user=_creator
-        )
+        domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW, user=_creator)
 
         p = "userpass"
         self.client.login(username="staffuser", password=p)
@@ -1277,9 +1284,8 @@ class TestDomainRequestAdmin(MockEppLib):
 
         # Check that the modal has the right content
         # Check for the header
-        
-        # == Check for the creator == # 
-        self.assertContains(response, "Meoward Jones")
+
+        # == 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
@@ -1288,31 +1294,32 @@ class TestDomainRequestAdmin(MockEppLib):
             # Field, expected value
             ("title", "Treat inspector</td>"),
             ("email", "meoward.jones@igorville.gov</td>"),
-            ("phone", "(555) 123 12345</td>")
+            ("phone", "(555) 123 12345</td>"),
         ]
         self.assert_response_contains_distinct_values(response, expected_creator_fields)
 
-        # == Check for the submitter == # 
-        self.assertContains(response, "Testy2 Tester2")
+        # Check for the field itself
+        self.assertContains(response, "Meoward Jones")
 
+        # == Check for the submitter == #
         expected_submitter_fields = [
             # Field, expected value
             ("title", "Admin Tester</td>"),
             ("email", "mayor@igorville.gov</td>"),
-            ("phone", "(555) 555 5556</td>")
+            ("phone", "(555) 555 5556</td>"),
         ]
         self.assert_response_contains_distinct_values(response, expected_submitter_fields)
+        self.assertContains(response, "Testy2 Tester2")
 
-        # == Check for the authorizing_official == # 
-        self.assertContains(response, "Testy Tester")
-
+        # == Check for the authorizing_official == #
         expected_ao_fields = [
             # Field, expected value
             ("title", "Chief Tester</td>"),
             ("email", "testy@town.com</td>"),
-            ("phone", "(555) 555 5555</td>")
+            ("phone", "(555) 555 5555</td>"),
         ]
         self.assert_response_contains_distinct_values(response, expected_ao_fields)
+        self.assertContains(response, "Testy Tester")
 
         # Check for table titles. We only need to check for the end tag
         # (Otherwise this test will fail if we change classes, etc)

From bdc03c66a91b6e441dcc7683d84117b1d3c46b11 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 20 Mar 2024 12:33:28 -0600
Subject: [PATCH 12/31] Final unit test

---
 .../includes/domain_request_fieldset.html     | 12 ++++++++++
 src/registrar/tests/test_admin.py             | 24 ++++++++++++++++++-
 2 files changed, 35 insertions(+), 1 deletion(-)

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 efd90b909..980fe0bfd 100644
--- a/src/registrar/templates/django/admin/includes/domain_request_fieldset.html
+++ b/src/registrar/templates/django/admin/includes/domain_request_fieldset.html
@@ -22,6 +22,18 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
                                             <a href="#" class="other-contact__{{forloop.counter}}">{{ contact }}</a>{% if not forloop.last %}, {% endif %}
                                         {% endfor %}
                                     </div>
+                                {% 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:", " %}
+                                        <a href="{{ website }}" class="padding-top-1 current-website__{{forloop.counter}}">{{ website }}</a>{% if not forloop.last %}, {% endif %}
+                                    {% endfor %}
                                 {% else %}
                                     <div class="readonly">{{ field.contents }}</div>
                                 {% endif %}
diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index 970742ae3..b4b12c56f 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -1247,7 +1247,29 @@ class TestDomainRequestAdmin(MockEppLib):
         # 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 = f"Testy Tester</a>"
+        expected_url = "Testy Tester</a>"
+        self.assertContains(response, expected_url)
+
+    @less_console_noise_decorator
+    def test_other_websites_has_readonly_link(self):
+        """Tests if the readonly other_websites field has links"""
+
+        # Create a fake domain request
+        domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW)
+
+        p = "userpass"
+        self.client.login(username="staffuser", password=p)
+        response = self.client.get(
+            "/admin/registrar/domainrequest/{}/change/".format(domain_request.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_request.requested_domain.name)
+
+        # Check that the page contains the link we expect.
+        expected_url = '<a href="city.com" class="padding-top-1 current-website__1">city.com</a>'
         self.assertContains(response, expected_url)
 
     @less_console_noise_decorator

From 516415d806e562516fd504cbfa880612e7827be2 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 20 Mar 2024 12:44:21 -0600
Subject: [PATCH 13/31] Correct new lines

---
 src/registrar/assets/js/get-gov-reports.js  |  2 +-
 src/registrar/templates/admin/fieldset.html |  2 +-
 src/registrar/tests/common.py               |  1 -
 src/registrar/tests/test_admin.py           | 50 ++++++++++-----------
 4 files changed, 27 insertions(+), 28 deletions(-)

diff --git a/src/registrar/assets/js/get-gov-reports.js b/src/registrar/assets/js/get-gov-reports.js
index 2fc9b6219..735681777 100644
--- a/src/registrar/assets/js/get-gov-reports.js
+++ b/src/registrar/assets/js/get-gov-reports.js
@@ -128,4 +128,4 @@
     };
 
     initComparativeColumnCharts();
-})();
\ No newline at end of file
+})();
diff --git a/src/registrar/templates/admin/fieldset.html b/src/registrar/templates/admin/fieldset.html
index dadae5811..96433e972 100644
--- a/src/registrar/templates/admin/fieldset.html
+++ b/src/registrar/templates/admin/fieldset.html
@@ -46,4 +46,4 @@ https://github.com/django/django/blob/main/django/contrib/admin/templates/admin/
             </div>
         {% endfor %}
     {% endblock fieldset_lines %}
-</fieldset>
\ No newline at end of file
+</fieldset>
diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py
index c7846d49d..04eb4c82d 100644
--- a/src/registrar/tests/common.py
+++ b/src/registrar/tests/common.py
@@ -734,7 +734,6 @@ def completed_domain_request(
     """A completed domain request."""
     if not user:
         user = get_user_model().objects.create(username="username" + str(uuid.uuid4())[:8])
-
     ao, _ = Contact.objects.get_or_create(
         first_name="Testy",
         last_name="Tester",
diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index b4b12c56f..f0a40044b 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -1194,31 +1194,6 @@ class TestDomainRequestAdmin(MockEppLib):
             # Test that approved domain exists and equals requested domain
             self.assertEqual(domain_request.requested_domain.name, domain_request.approved_domain.name)
 
-    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</td>"),
-            ("email", "meoward.jones@igorville.gov</td>"),
-        ]
-        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_other_contacts_has_readonly_link(self):
         """Tests if the readonly other_contacts field has links"""
@@ -1272,6 +1247,31 @@ class TestDomainRequestAdmin(MockEppLib):
         expected_url = '<a href="city.com" class="padding-top-1 current-website__1">city.com</a>'
         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</td>"),
+            ("email", "meoward.jones@igorville.gov</td>"),
+        ]
+        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"""

From 407ff0cb76bdae7c509d5af0b33c867f62ee7e6b Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 20 Mar 2024 13:13:56 -0600
Subject: [PATCH 14/31] missing unit test

---
 src/registrar/tests/test_admin.py | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index f0a40044b..b29776f81 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -1341,7 +1341,10 @@ class TestDomainRequestAdmin(MockEppLib):
             ("phone", "(555) 555 5555</td>"),
         ]
         self.assert_response_contains_distinct_values(response, expected_ao_fields)
-        self.assertContains(response, "Testy Tester")
+
+        # 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)
@@ -1355,6 +1358,18 @@ class TestDomainRequestAdmin(MockEppLib):
         # Phone. Count=3 because this table appears on three records.
         self.assertContains(response, "Phone</th>", count=3)
 
+        # == Test the other_employees field == #
+        expected_other_employees_fields = [
+            # Field, expected value
+            ("title", "Another Tester</td>"),
+            ("email", "testy2@town.com</td>"),
+            ("phone", "(555) 555 5557</td>"),
+        ]
+        self.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</th>", 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

From 7e8f07439e322cf36529ad58d6ec0fd887310547 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 20 Mar 2024 15:21:19 -0600
Subject: [PATCH 15/31] Move order

---
 .../admin/includes/domain_request_fieldset.html     | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

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 980fe0bfd..f4e68bd0c 100644
--- a/src/registrar/templates/django/admin/includes/domain_request_fieldset.html
+++ b/src/registrar/templates/django/admin/includes/domain_request_fieldset.html
@@ -42,6 +42,13 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
                             {% endif %}
                         {% endif %}
                     </div>
+
+                {% if field.field.help_text %}
+                    <div class="help"{% if field.field.id_for_label %} id="{{ field.field.id_for_label }}_helptext"{% endif %}>
+                        <div>{{ field.field.help_text|safe }}</div>
+                    </div>
+                {% 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" %}
@@ -71,12 +78,6 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
                         </div>
                     </details>
                 {% endif %}
-
-                {% if field.field.help_text %}
-                    <div class="help"{% if field.field.id_for_label %} id="{{ field.field.id_for_label }}_helptext"{% endif %}>
-                        <div>{{ field.field.help_text|safe }}</div>
-                    </div>
-                {% endif %}
             </div>
         {% endfor %}
         {% if not line.fields|length == 1 %}</div>{% endif %}

From ed36fc6e3b36fcd6842ea17d10280b6f99c7067a Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Thu, 21 Mar 2024 12:42:21 -0600
Subject: [PATCH 16/31] Code simplification

---
 src/registrar/admin.py                        |  2 +
 src/registrar/templates/admin/fieldset.html   | 71 +++++++++------
 .../admin/domain_information_change_form.html |  8 ++
 ...l_table.html => contact_detail_table.html} |  0
 .../admin/includes/detail_table_fieldset.html | 61 +++++++++++++
 .../includes/domain_information_fieldset.html |  5 ++
 .../includes/domain_request_fieldset.html     | 88 +------------------
 7 files changed, 120 insertions(+), 115 deletions(-)
 create mode 100644 src/registrar/templates/django/admin/domain_information_change_form.html
 rename src/registrar/templates/django/admin/includes/{domain_request_detail_table.html => contact_detail_table.html} (100%)
 create mode 100644 src/registrar/templates/django/admin/includes/detail_table_fieldset.html
 create mode 100644 src/registrar/templates/django/admin/includes/domain_information_fieldset.html

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index a3cc0d2d1..ae925b5c3 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -913,6 +913,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 %}
-            <div class="form-row{% if line.fields|length == 1 and line.errors %} errors{% endif %}{% if not line.has_visible_field %} hidden{% endif %}{% for field in line %}{% if field.field.name %} field-{{ field.field.name }}{% endif %}{% endfor %}">
-                {% if line.fields|length == 1 %}{{ line.errors }}{% else %}<div class="flex-container form-multiline">{% endif %}
-                {% for field in line %}
-                    <div>
-                        {% if not line.fields|length == 1 and not field.is_readonly %}{{ field.errors }}{% endif %}
-                            <div class="flex-container{% if not line.fields|length == 1 %} fieldBox{% if field.field.name %} field-{{ field.field.name }}{% endif %}{% if not field.is_readonly and field.errors %} errors{% endif %}{% if field.field.is_hidden %} hidden{% endif %}{% elif field.is_checkbox %} checkbox-row{% endif %}">
-                                {% if field.is_checkbox %}
-                                    {{ field.field }}{{ field.label_tag }}
+    {% for line in fieldset %}
+        <div class="form-row{% if line.fields|length == 1 and line.errors %} errors{% endif %}{% if not line.has_visible_field %} hidden{% endif %}{% for field in line %}{% if field.field.name %} field-{{ field.field.name }}{% endif %}{% endfor %}">
+            {% if line.fields|length == 1 %}{{ line.errors }}{% else %}<div class="flex-container form-multiline">{% endif %}
+            {% for field in line %}
+                <div>
+                    {% if not line.fields|length == 1 and not field.is_readonly %}{{ field.errors }}{% endif %}
+                        <div class="flex-container{% if not line.fields|length == 1 %} fieldBox{% if field.field.name %} field-{{ field.field.name }}{% endif %}{% if not field.is_readonly and field.errors %} errors{% endif %}{% if field.field.is_hidden %} hidden{% endif %}{% elif field.is_checkbox %} checkbox-row{% 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 %}
+                                    <div class="readonly">{{ field.contents }}</div>
+                                    {% endblock field_readonly%}
                                 {% else %}
-                                    {{ field.label_tag }}
-                                    {% if field.is_readonly %}
-                                        <div class="readonly">{{ field.contents }}</div>
-                                    {% else %}
-                                        {{ field.field }}
-                                    {% endif %}
+                                    {% block field_other %}
+                                    {{ field.field }}
+                                    {% endblock field_other%}
                                 {% endif %}
-                            </div>
-                        {% if field.field.help_text %}
-                            <div class="help"{% if field.field.id_for_label %} id="{{ field.field.id_for_label }}_helptext"{% endif %}>
-                                <div>{{ field.field.help_text|safe }}</div>
-                            </div>
-                        {% endif %}
-                    </div>
-                {% endfor %}
-                {% if not line.fields|length == 1 %}</div>{% endif %}
-            </div>
-        {% endfor %}
-    {% endblock fieldset_lines %}
+                            {% endif %}
+                        </div>
+                    
+                    {% block before_help_text %}
+                        {# For templating purposes #}
+                    {% endblock before_help_text %}
+
+                    {% if field.field.help_text %}
+                        {% block help_text %}
+                        <div class="help"{% if field.field.id_for_label %} id="{{ field.field.id_for_label }}_helptext"{% endif %}>
+                            <div>{{ field.field.help_text|safe }}</div>
+                        </div>
+                        {% endblock help_text %}
+                    {% endif %}
+
+                    {% block after_help_text %}
+                        {# For templating purposes #}
+                    {% endblock after_help_text %}
+                </div>
+            {% endfor %}
+            {% if not line.fields|length == 1 %}</div>{% endif %}
+        </div>
+    {% endfor %}
 </fieldset>
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" %}
+    <div class="readonly">
+        {% for contact in field.contents|split:", " %}
+            <a href="#" class="other-contact__{{forloop.counter}}">{{ contact }}</a>{% if not forloop.last %}, {% endif %}
+        {% endfor %}
+    </div>
+    {% 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:", " %}
+        <a href="{{ website }}" class="padding-top-1 current-website__{{forloop.counter}}">{{ website }}</a>{% if not forloop.last %}, {% endif %}
+    {% endfor %}
+    {% else %}
+    <div class="readonly">{{ field.contents }}</div>
+    {% 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 class="margin-top-1 dja-detail-table">
+            <summary class="padding-1 dja-details-summary">Details</summary>
+            <div class="grid-container padding-left-0 padding-right-0 dja-details-contents">
+                <table>
+                    <tbody>
+                        {% 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 %}
+                        <tr data-contact-id="{{ contact.id }}" data-contact-url="{% url 'admin:registrar_contact_change' contact.id %}">
+                            <th scope="row">{{contact.first_name}} {{contact.last_name}}</th>
+                            <td>{{ contact.title }}</td>
+                            <td>{{ contact.email }}</td>
+                            <td>{{ contact.phone }}</td>
+                        </tr>
+                        {% endfor %}
+                    </tbody>
+                </table>
+            </div>
+        </details>
+    {% 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..564415f24
--- /dev/null
+++ b/src/registrar/templates/django/admin/includes/domain_information_fieldset.html
@@ -0,0 +1,5 @@
+{% extends "django/admin/includes/detail_table_fieldset.html" %}
+
+{% block after_help_text %}
+<div>TESTING123</div>
+{% endblock after_help_text %}
\ No newline at end of file
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 f4e68bd0c..c694eb353 100644
--- a/src/registrar/templates/django/admin/includes/domain_request_fieldset.html
+++ b/src/registrar/templates/django/admin/includes/domain_request_fieldset.html
@@ -1,87 +1 @@
-{% 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 %}
-    <div class="form-row{% if line.fields|length == 1 and line.errors %} errors{% endif %}{% if not line.has_visible_field %} hidden{% endif %}{% for field in line %}{% if field.field.name %} field-{{ field.field.name }}{% endif %}{% endfor %}">
-        {% if line.fields|length == 1 %}{{ line.errors }}{% else %}<div class="flex-container form-multiline">{% endif %}
-        {% for field in line %}
-            <div>
-                {% if not line.fields|length == 1 and not field.is_readonly %}{{ field.errors }}{% endif %}
-                    <div class="flex-container{% if not line.fields|length == 1 %} fieldBox{% if field.field.name %} field-{{ field.field.name }}{% endif %}{% if not field.is_readonly and field.errors %} errors{% endif %}{% if field.field.is_hidden %} hidden{% endif %}{% elif field.is_checkbox %} checkbox-row{% endif %}">
-                        {% if field.is_checkbox %}
-                            {{ field.field }}{{ field.label_tag }}
-                        {% else %}
-                            {{ field.label_tag }}
-                            {% if field.is_readonly %}
-                                {% if field.field.name == "other_contacts" %}
-                                    <div class="readonly">
-                                        {% for contact in field.contents|split:", " %}
-                                            <a href="#" class="other-contact__{{forloop.counter}}">{{ contact }}</a>{% if not forloop.last %}, {% endif %}
-                                        {% endfor %}
-                                    </div>
-                                {% 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:", " %}
-                                        <a href="{{ website }}" class="padding-top-1 current-website__{{forloop.counter}}">{{ website }}</a>{% if not forloop.last %}, {% endif %}
-                                    {% endfor %}
-                                {% else %}
-                                    <div class="readonly">{{ field.contents }}</div>
-                                {% endif %}
-                            {% else %}
-                                {{ field.field }}
-                            {% endif %}
-                        {% endif %}
-                    </div>
-
-                {% if field.field.help_text %}
-                    <div class="help"{% if field.field.id_for_label %} id="{{ field.field.id_for_label }}_helptext"{% endif %}>
-                        <div>{{ field.field.help_text|safe }}</div>
-                    </div>
-                {% 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 class="margin-top-1 dja-detail-table">
-                        <summary class="padding-1 dja-details-summary">Details</summary>
-                        <div class="grid-container padding-left-0 padding-right-0 dja-details-contents">
-                            <table>
-                                <tbody>
-                                    {% 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 %}
-                                    <tr data-contact-id="{{ contact.id }}" data-contact-url="{% url 'admin:registrar_contact_change' contact.id %}">
-                                        <th scope="row">{{contact.first_name}} {{contact.last_name}}</th>
-                                        <td>{{ contact.title }}</td>
-                                        <td>{{ contact.email }}</td>
-                                        <td>{{ contact.phone }}</td>
-                                    </tr>
-                                    {% endfor %}
-                                </tbody>
-                            </table>
-                        </div>
-                    </details>
-                {% endif %}
-            </div>
-        {% endfor %}
-        {% if not line.fields|length == 1 %}</div>{% endif %}
-    </div>
-{% endfor %}
-
-{% endblock fieldset_lines %}
+{% extends "django/admin/includes/detail_table_fieldset.html" %}
\ No newline at end of file

From 98d940386bfc0c4dd67a28ad3edb5a0c96449f69 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Thu, 21 Mar 2024 13:45:06 -0600
Subject: [PATCH 17/31] New test cases

---
 src/registrar/admin.py                        |   1 +
 .../includes/domain_information_fieldset.html |   5 +-
 .../includes/domain_request_fieldset.html     |   3 +-
 src/registrar/tests/common.py                 |  25 +++
 src/registrar/tests/test_admin.py             | 167 +++++++++++++++---
 5 files changed, 167 insertions(+), 34 deletions(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index ae925b5c3..e1b9bd5a9 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -896,6 +896,7 @@ class DomainInformationAdmin(ListHeaderAdmin):
         "no_other_contacts_rationale",
         "anything_else",
         "is_policy_acknowledged",
+        "other_contacts",
     ]
 
     # For each filter_horizontal, init in admin js extendFilterHorizontalWidgets
diff --git a/src/registrar/templates/django/admin/includes/domain_information_fieldset.html b/src/registrar/templates/django/admin/includes/domain_information_fieldset.html
index 564415f24..b42873c3c 100644
--- a/src/registrar/templates/django/admin/includes/domain_information_fieldset.html
+++ b/src/registrar/templates/django/admin/includes/domain_information_fieldset.html
@@ -1,5 +1,2 @@
 {% extends "django/admin/includes/detail_table_fieldset.html" %}
-
-{% block after_help_text %}
-<div>TESTING123</div>
-{% endblock after_help_text %}
\ No newline at end of file
+{# 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 c694eb353..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 +1,2 @@
-{% extends "django/admin/includes/detail_table_fieldset.html" %}
\ No newline at end of file
+{% 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</td>"),
+            ("email", "meoward.jones@igorville.gov</td>"),
+        ]
+        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 b29776f81..042c7261b 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -1247,31 +1247,6 @@ class TestDomainRequestAdmin(MockEppLib):
         expected_url = '<a href="city.com" class="padding-top-1 current-website__1">city.com</a>'
         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</td>"),
-            ("email", "meoward.jones@igorville.gov</td>"),
-        ]
-        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"""
@@ -1318,7 +1293,7 @@ class TestDomainRequestAdmin(MockEppLib):
             ("email", "meoward.jones@igorville.gov</td>"),
             ("phone", "(555) 123 12345</td>"),
         ]
-        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")
@@ -1330,7 +1305,7 @@ class TestDomainRequestAdmin(MockEppLib):
             ("email", "mayor@igorville.gov</td>"),
             ("phone", "(555) 555 5556</td>"),
         ]
-        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 == #
@@ -1340,7 +1315,7 @@ class TestDomainRequestAdmin(MockEppLib):
             ("email", "testy@town.com</td>"),
             ("phone", "(555) 555 5555</td>"),
         ]
-        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.
@@ -1365,7 +1340,7 @@ class TestDomainRequestAdmin(MockEppLib):
             ("email", "testy2@town.com</td>"),
             ("phone", "(555) 555 5557</td>"),
         ]
-        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</th>", count=1)
@@ -1965,6 +1940,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</a>"
+        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</td>"),
+            ("email", "meoward.jones@igorville.gov</td>"),
+            ("phone", "(555) 123 12345</td>"),
+        ]
+        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</td>"),
+            ("email", "mayor@igorville.gov</td>"),
+            ("phone", "(555) 555 5556</td>"),
+        ]
+        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</td>"),
+            ("email", "testy@town.com</td>"),
+            ("phone", "(555) 555 5555</td>"),
+        ]
+        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</th>", count=3)
+
+        # Email. Count=3 because this table appears on three records.
+        self.assertContains(response, "Email</th>", count=3)
+
+        # Phone. Count=3 because this table appears on three records.
+        self.assertContains(response, "Phone</th>", count=3)
+
+        # == Test the other_employees field == #
+        expected_other_employees_fields = [
+            # Field, expected value
+            ("title", "Another Tester</td>"),
+            ("email", "testy2@town.com</td>"),
+            ("phone", "(555) 555 5557</td>"),
+        ]
+        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</th>", count=1)
+
     def test_readonly_fields_for_analyst(self):
         """Ensures that analysts have their permissions setup correctly"""
         with less_console_noise():
@@ -1983,6 +2091,7 @@ class TestDomainInformationAdmin(TestCase):
                 "no_other_contacts_rationale",
                 "anything_else",
                 "is_policy_acknowledged",
+                "other_contacts",
             ]
 
             self.assertEqual(readonly_fields, expected_fields)

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 18/31] 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 class="margin-top-1 dja-detail-table">
             <summary class="padding-1 dja-details-summary">Details</summary>
-            <div class="grid-container padding-left-0 padding-right-0 dja-details-contents">
+            <div class="grid-container margin-left-0 padding-left-0 padding-right-0 dja-details-contents">
                 <table>
                     <tbody>
                         {% 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 19/31] 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 class="margin-top-1 dja-detail-table">
-            <summary class="padding-1 dja-details-summary">Details</summary>
+            <summary class="padding-1 padding-left-0 dja-details-summary">Details</summary>
             <div class="grid-container margin-left-0 padding-left-0 padding-right-0 dja-details-contents">
                 <table>
                     <tbody>
@@ -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 %}
                         <tr data-contact-id="{{ contact.id }}" data-contact-url="{% url 'admin:registrar_contact_change' contact.id %}">
-                            <th scope="row">{{contact.first_name}} {{contact.last_name}}</th>
-                            <td>{{ contact.title }}</td>
-                            <td>{{ contact.email }}</td>
-                            <td>{{ contact.phone }}</td>
+                            <th class="padding-left-1" scope="row">{{contact.first_name}} {{contact.last_name}}</th>
+                            <td class="padding-left-1">{{ contact.title }}</td>
+                            <td class="padding-left-1">{{ contact.email }}</td>
+                            <td class="padding-left-1">{{ contact.phone }}</td>
                         </tr>
                         {% endfor %}
                     </tbody>

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 20/31] 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 %}
 <fieldset class="module aligned {{ fieldset.classes }}">
+    {# .gov override #}
     {% block fieldset_title %}
         {% if fieldset.name %}<h2>{{ fieldset.name }}</h2>{% endif %}
     {% endblock fieldset_title %}
 
+    {# .gov override #}
     {% block fieldset_description %}
         {% if fieldset.description %}
             <div class="description">{{ fieldset.description|safe }}</div>
@@ -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 %}
                         <div class="flex-container{% if not line.fields|length == 1 %} fieldBox{% if field.field.name %} field-{{ field.field.name }}{% endif %}{% if not field.is_readonly and field.errors %} errors{% endif %}{% if field.field.is_hidden %} hidden{% endif %}{% elif field.is_checkbox %} checkbox-row{% 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 %}
                                     <div class="readonly">{{ field.contents }}</div>
                                     {% 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 %}
                         </div>
                     
+                    {# .gov addition #}
                     {% block before_help_text %}
                         {# For templating purposes #}
                     {% endblock before_help_text %}
 
                     {% if field.field.help_text %}
+                        {# .gov override #}
                         {% block help_text %}
                         <div class="help"{% if field.field.id_for_label %} id="{{ field.field.id_for_label }}_helptext"{% endif %}>
                             <div>{{ field.field.help_text|safe }}</div>
                         </div>
                         {% 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 <a> tag so that we can use a click event to toggle the modal. #}
     <a id="invisible-ineligible-modal-toggler" class="display-none" href="#toggle-set-ineligible" aria-controls="toggle-set-ineligible" data-open-modal></a>
+
     {% 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 %}
 
-<table class="dja-user-detail-table" {% if field_name %} id="id_detail_table__{{field_name}}" {% endif %}>
+<table class="dja-user-detail-table margin-top-1" {% if field_name %} id="id_detail_table__{{field_name}}" {% endif %}>
+    <thead>
+        <tr>
+            <th colspan="2">Contact details</th>
+        <tr>
+    </thead>
     <tbody>
         <tr>
-            <th class="padding-left-0" scope="row">Title</th>
+            <th scope="row">Title</th>
             {% if user.title or user.contact.title %}
                 {% if user.contact.title %}
                 <td>{{ user.contact.title }}</td>
@@ -15,27 +20,27 @@
             {% endif %}
         </tr>
         <tr>
-            <th class="padding-left-0" scope="row">Email</th>
+            <th class="padding-top-0" scope="row">Email</th>
             {% if user.email or user.contact.email %}
                 {% if user.contact.email %}
-                <td>{{ user.contact.email }}</td>
+                <td class="padding-top-0">{{ user.contact.email }}</td>
                 {% else %}
-                <td>{{ user.email }}</td>
+                <td class="padding-top-0">{{ user.email }}</td>
                 {% endif %}
             {% else %}
-            <td>Nothing found</td>
+            <td class="padding-top-0">Nothing found</td>
             {% endif %}
         </tr>
         <tr>
-            <th class="padding-left-0" scope="row">Phone</th>
+            <th class="padding-top-0" scope="row">Phone</th>
             {% if user.phone or user.contact.phone %}
                 {% if user.contact.phone %}
-                <td>{{ user.contact.phone }}</td>
+                <td class="padding-top-0">{{ user.contact.phone }}</td>
                 {% else %}
-                <td>{{ user.phone }}</td>
+                <td class="padding-top-0">{{ user.phone }}</td>
                 {% endif %}
             {% else %}
-            <td>Nothing found</td>
+            <td class="padding-top-0">Nothing found</td>
             {% endif %}
         </tr>
     </tbody>
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" %}
+        <div class="flex-container">
+        <label aria-label="Creator contact details"></label>
         {% include "django/admin/includes/contact_detail_table.html" with user=original.creator field_name="creator" %}
+        </div>
     {% elif field.field.name == "submitter" %}
+        <div class="flex-container">
+        <label aria-label="Submitter contact details"></label>
         {% include "django/admin/includes/contact_detail_table.html" with user=original.submitter field_name="submitter" %}
+        </div>
     {% elif field.field.name == "authorizing_official" %}
+        <div class="flex-container">
+        <label aria-label="Authorizing official contact details"></label>
         {% include "django/admin/includes/contact_detail_table.html" with user=original.authorizing_official field_name="authorizing_official" %}
+        </div>
     {% elif field.field.name == "other_contacts" and original.other_contacts.all %}
-        <details class="margin-top-1 dja-detail-table">
+        <details class="margin-top-1 dja-detail-table" aria-role="button">
             <summary class="padding-1 padding-left-0 dja-details-summary">Details</summary>
             <div class="grid-container margin-left-0 padding-left-0 padding-right-0 dja-details-contents">
                 <table>
+                    <thead>
+                        <tr>
+                            <th colspan="4">Contact information</th>
+                        <tr>
+                    </thead>
                     <tbody>
                         {% 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 21/31] 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 %}
 
-<table class="dja-user-detail-table margin-top-1" {% if field_name %} id="id_detail_table__{{field_name}}" {% endif %}>
+<table class="dja-user-detail-table" {% if field_name %} id="id_detail_table__{{field_name}}" {% endif %}>
+    {% if show_table_details %}
     <thead>
         <tr>
             <th colspan="2">Contact details</th>
         <tr>
     </thead>
+    {% endif %}
+
+    {% if user.title or user.contact.title or user.email or user.contact.email or user.phone or user.contact.phone %}
     <tbody>
         <tr>
-            <th scope="row">Title</th>
+            {% if show_table_details %}
+            <th class="{% if no_title_top_padding %}padding-top-0{% endif %}" scope="row">Title</th>
+            {% endif %}
+
             {% if user.title or user.contact.title %}
                 {% if user.contact.title %}
-                <td>{{ user.contact.title }}</td>
+                <td class="{% if no_title_top_padding %}padding-top-0{% endif %} padding-left-0">{{ user.contact.title }}</td>
                 {% else %}
-                <td>{{ user.title }}</td>
+                <td class="{% if no_title_top_padding %}padding-top-0{% endif %} padding-left-0">{{ user.title }}</td>
                 {% endif %}
             {% else %}
-            <td>Nothing found</td>
+            <td class="{% if no_title_top_padding %}padding-top-0{% endif %} padding-left-0">None</td>
             {% endif %}
         </tr>
         <tr>
-            <th class="padding-top-0" scope="row">Email</th>
+            {% if show_table_details %}
+            <th class="padding-top-0 padding-left-0" scope="row">Email</th>
+            {% endif %}
+
             {% if user.email or user.contact.email %}
                 {% if user.contact.email %}
-                <td class="padding-top-0">{{ user.contact.email }}</td>
+                <td class="padding-top-0 padding-left-0">{{ user.contact.email }}</td>
                 {% else %}
-                <td class="padding-top-0">{{ user.email }}</td>
+                <td class="padding-top-0 padding-left-0">{{ user.email }}</td>
                 {% endif %}
             {% else %}
-            <td class="padding-top-0">Nothing found</td>
+            <td class="padding-top-0 padding-left-0">None</td>
             {% endif %}
         </tr>
         <tr>
-            <th class="padding-top-0" scope="row">Phone</th>
+            {% if show_table_details %}
+            <th class="padding-top-0 padding-left-0" scope="row">Phone</th>
+            {% endif %}
+
             {% if user.phone or user.contact.phone %}
                 {% if user.contact.phone %}
-                <td class="padding-top-0">{{ user.contact.phone }}</td>
+                <td class="padding-top-0 padding-left-0">{{ user.contact.phone }}</td>
                 {% else %}
-                <td class="padding-top-0">{{ user.phone }}</td>
+                <td class="padding-top-0 padding-left-0">{{ user.phone }}</td>
                 {% endif %}
             {% else %}
-            <td class="padding-top-0">Nothing found</td>
+            <td class="padding-top-0 padding-left-0">None</td>
             {% endif %}
         </tr>
     </tbody>
+    {% else %}
+    <tbody>
+        <tr>
+            <td class="padding-left-0">No additional contact information found.</td>
+        </tr>
+    </tbody>
+    {% endif %}
 </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 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:", " %}
+    <div class="readonly">
+    {% with total_websites=field.contents|split:", " %}
+    {% for website in total_websites %}
         <a href="{{ website }}" class="padding-top-1 current-website__{{forloop.counter}}">{{ website }}</a>{% if not forloop.last %}, {% endif %}
+        {# Acts as a <br> #}
+        {% if total_websites|length < 5 %}
+        <div class="display-block margin-top-1"></div>
+        {% endif %}
     {% endfor %}
+    {% endwith %}
+    </div>
     {% else %}
     <div class="readonly">{{ field.contents }}</div>
     {% endif %}
@@ -32,44 +40,55 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
     {% if field.field.name == "creator" %}
         <div class="flex-container">
         <label aria-label="Creator contact details"></label>
-        {% 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 %}
         </div>
     {% elif field.field.name == "submitter" %}
         <div class="flex-container">
         <label aria-label="Submitter contact details"></label>
-        {% 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 %}
         </div>
     {% elif field.field.name == "authorizing_official" %}
         <div class="flex-container">
         <label aria-label="Authorizing official contact details"></label>
-        {% 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 %}
         </div>
     {% elif field.field.name == "other_contacts" and original.other_contacts.all %}
-        <details class="margin-top-1 dja-detail-table" aria-role="button">
-            <summary class="padding-1 padding-left-0 dja-details-summary">Details</summary>
-            <div class="grid-container margin-left-0 padding-left-0 padding-right-0 dja-details-contents">
-                <table>
-                    <thead>
-                        <tr>
-                            <th colspan="4">Contact information</th>
-                        <tr>
-                    </thead>
-                    <tbody>
-                        {% 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 %}
-                        <tr data-contact-id="{{ contact.id }}" data-contact-url="{% url 'admin:registrar_contact_change' contact.id %}">
-                            <th class="padding-left-1" scope="row">{{contact.first_name}} {{contact.last_name}}</th>
-                            <td class="padding-left-1">{{ contact.title }}</td>
-                            <td class="padding-left-1">{{ contact.email }}</td>
-                            <td class="padding-left-1">{{ contact.phone }}</td>
-                        </tr>
-                        {% endfor %}
-                    </tbody>
-                </table>
-            </div>
-        </details>
+        {% with all_contacts=original.other_contacts.all %}
+        {% if all_contacts.count == 1 %}
+            {% for contact in all_contacts %}
+                <div class="flex-container">
+                    <label aria-label="Other contact details"></label>
+                    {% include "django/admin/includes/contact_detail_table.html" with user=contact field_name="other_contact" no_title_top_padding=field.is_readonly %}
+                </div>
+        {% endfor %}
+        {% else %}
+            <details class="margin-top-1 dja-detail-table" aria-role="button">
+                <summary class="padding-1 padding-left-0 dja-details-summary">Details</summary>
+                <div class="grid-container margin-left-0 padding-left-0 padding-right-0 dja-details-contents">
+                    <table>
+                        <thead>
+                            <tr>
+                                <th colspan="4">Contact information</th>
+                            <tr>
+                        </thead>
+                        <tbody>
+                            {% 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 %}
+                            <tr data-contact-id="{{ contact.id }}" data-contact-url="{% url 'admin:registrar_contact_change' contact.id %}">
+                                <th class="padding-left-1" scope="row">{{contact.first_name}} {{contact.last_name}}</th>
+                                <td class="padding-left-1">{{ contact.title }}</td>
+                                <td class="padding-left-1">{{ contact.email }}</td>
+                                <td class="padding-left-1">{{ contact.phone }}</td>
+                            </tr>
+                            {% endfor %}
+                        </tbody>
+                    </table>
+                </div>
+            </details>
+        {% 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 22/31] 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 %}
                 <div class="flex-container">
                     <label aria-label="Other contact details"></label>
+                    <div class="display-none other-contact-id" data-contact-url="{% url 'admin:registrar_contact_change' contact.id %}"></div>
                     {% include "django/admin/includes/contact_detail_table.html" with user=contact field_name="other_contact" no_title_top_padding=field.is_readonly %}
                 </div>
         {% 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</th>", count=3)
-
-        # Email. Count=3 because this table appears on three records.
-        self.assertContains(response, "Email</th>", count=3)
-
-        # Phone. Count=3 because this table appears on three records.
-        self.assertContains(response, "Phone</th>", 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</th>", 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 23/31] 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</th>", count=3)
-
-        # Email. Count=3 because this table appears on three records.
-        self.assertContains(response, "Email</th>", count=3)
-
-        # Phone. Count=3 because this table appears on three records.
-        self.assertContains(response, "Phone</th>", 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</th>", 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 24/31] 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 25/31] 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 %}
+
+
+<ul class="{% if no_title_top_padding %}margin-top-neg-1__detail-list{% endif %} padding-top-0 padding-left-0__detail-list margin-left-0__detail-list" {% if field_name %} id="id_detail_list__{{field_name}}" {% endif %}>
+    {% if user.title or user.contact.title or user.email or user.contact.email or user.phone or user.contact.phone %}
+        {# Title #}
+        <li>
+            {% if user.title or user.contact.title %}
+                {% if user.contact.title %}
+                    {{ user.contact.title }}
+                {% else %}
+                    {{ user.title }}
+                {% endif %}
+            {% else %}
+                None
+            {% endif %}
+        </li>
+        {# Email #}
+        <li>
+            {% if user.email or user.contact.email %}
+                {% if user.contact.email %}
+                    {{ user.contact.email }}
+                {% else %}
+                    {{ user.email }}
+                {% endif %}
+            {% else %}
+                None
+            {% endif %}
+        </li>
+        {# Phone #}
+        <li>
+            {% 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 %}
+        </li>
+    {% else %}
+        <li>
+            No additional contact information found.
+        </li>
+    {% endif %}
+</ul>
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 %}
-
-<table class="dja-user-detail-table" {% if field_name %} id="id_detail_table__{{field_name}}" {% endif %}>
-    {% if show_table_details %}
-    <thead>
-        <tr>
-            <th colspan="2">Contact details</th>
-        <tr>
-    </thead>
-    {% endif %}
-
-    {% if user.title or user.contact.title or user.email or user.contact.email or user.phone or user.contact.phone %}
-    <tbody>
-        <tr>
-            {% if show_table_details %}
-            <th class="{% if no_title_top_padding %}padding-top-0{% endif %}" scope="row">Title</th>
-            {% endif %}
-
-            {% if user.title or user.contact.title %}
-                {% if user.contact.title %}
-                <td class="{% if no_title_top_padding %}padding-top-0{% endif %} padding-left-0">{{ user.contact.title }}</td>
-                {% else %}
-                <td class="{% if no_title_top_padding %}padding-top-0{% endif %} padding-left-0">{{ user.title }}</td>
-                {% endif %}
-            {% else %}
-            <td class="{% if no_title_top_padding %}padding-top-0{% endif %} padding-left-0">None</td>
-            {% endif %}
-        </tr>
-        <tr>
-            {% if show_table_details %}
-            <th class="padding-top-0 padding-left-0" scope="row">Email</th>
-            {% endif %}
-
-            {% if user.email or user.contact.email %}
-                {% if user.contact.email %}
-                <td class="padding-top-0 padding-left-0">{{ user.contact.email }}</td>
-                {% else %}
-                <td class="padding-top-0 padding-left-0">{{ user.email }}</td>
-                {% endif %}
-            {% else %}
-            <td class="padding-top-0 padding-left-0">None</td>
-            {% endif %}
-        </tr>
-        <tr>
-            {% if show_table_details %}
-            <th class="padding-top-0 padding-left-0" scope="row">Phone</th>
-            {% endif %}
-
-            {% if user.phone or user.contact.phone %}
-                {% if user.contact.phone %}
-                <td class="padding-top-0 padding-left-0">{{ user.contact.phone }}</td>
-                {% else %}
-                <td class="padding-top-0 padding-left-0">{{ user.phone }}</td>
-                {% endif %}
-            {% else %}
-            <td class="padding-top-0 padding-left-0">None</td>
-            {% endif %}
-        </tr>
-    </tbody>
-    {% else %}
-    <tbody>
-        <tr>
-            <td class="padding-left-0">No additional contact information found.</td>
-        </tr>
-    </tbody>
-    {% endif %}
-</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 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" %}
-    <div class="readonly">
-        {% for contact in field.contents|split:", " %}
-            <a href="#" class="other-contact__{{forloop.counter}}">{{ contact }}</a>{% if not forloop.last %}, {% endif %}
-        {% endfor %}
-    </div>
-    {% 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 %}
-    <div class="readonly">
-    {% with total_websites=field.contents|split:", " %}
-    {% for website in total_websites %}
-        <a href="{{ website }}" class="padding-top-1 current-website__{{forloop.counter}}">{{ website }}</a>{% if not forloop.last %}, {% endif %}
-        {# Acts as a <br> #}
-        {% if total_websites|length < 5 %}
-        <div class="display-block margin-top-1"></div>
+        {% if all_contacts.count > 2 %}
+            <div class="readonly">
+                {% for contact in field.contents|split:", " %}
+                    <a href="#" class="other-contact__{{forloop.counter}}">{{ contact }}</a>{% if not forloop.last %}, {% endif %}
+                {% endfor %}
+            </div>
+        {% else %}
+            <div class="flex-container dja-detail-list">
+                <dl class="usa-list usa-list--unstyled margin-0 margin-top-1">
+                    {% for contact in all_contacts %}
+                        <dt class="{% if forloop.counter == 1%}margin-top-0{% endif %}">
+                            Organization contact {{forloop.counter}}
+                        </dt>
+                        <dd>
+                            <address>
+                                {% if  contact.get_formatted_name  %}<a href="{% url 'admin:registrar_contact_change' contact.id %}">{{ contact.get_formatted_name }}</a><br />{% else %}None<br />{% endif %}
+                                {% if  contact.title  %}{{ contact.title }}<br />{% else %}None<br />{% endif %}
+                                {% if  contact.email  %}{{ contact.email }}<br />{% else %}None<br />{% endif %}
+                                {% if  contact.phone  %}{{ contact.phone.as_national }}{% else %}None{% endif %}
+                            </address>
+                        </dd>
+                    {% endfor %}
+                </dl>
+            </div>
         {% endif %}
-    {% endfor %}
-    {% endwith %}
-    </div>
+    {% 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 %}
+        <div class="readonly">
+        {% with total_websites=field.contents|split:", " %}
+            {% for website in total_websites %}
+                <a href="{{ website }}" class="padding-top-1 current-website__{{forloop.counter}}">{{ website }}</a>{% if not forloop.last %}, {% endif %}
+                {# Acts as a <br> #}
+                {% if total_websites|length < 5 %}
+                <div class="display-block margin-top-1"></div>
+                {% endif %}
+            {% endfor %}
+        {% endwith %}
+        </div>
     {% else %}
     <div class="readonly">{{ field.contents }}</div>
     {% endif %}
+    {% endwith %}
 {% endblock field_readonly %}
 
 {% block after_help_text %}
     {% if field.field.name == "creator" %}
         <div class="flex-container">
         <label aria-label="Creator contact details"></label>
-        {% 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 %}
         </div>
     {% elif field.field.name == "submitter" %}
         <div class="flex-container">
         <label aria-label="Submitter contact details"></label>
-        {% 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 %}
         </div>
     {% elif field.field.name == "authorizing_official" %}
         <div class="flex-container">
         <label aria-label="Authorizing official contact details"></label>
-        {% 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 %}
         </div>
     {% 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 %}
-                <div class="flex-container">
-                    <label aria-label="Other contact details"></label>
-                    <div class="display-none other-contact-id" data-contact-url="{% url 'admin:registrar_contact_change' contact.id %}"></div>
-                    {% include "django/admin/includes/contact_detail_table.html" with user=contact field_name="other_contact" no_title_top_padding=field.is_readonly %}
-                </div>
-        {% endfor %}
-        {% else %}
+        {% if all_contacts.count > 2 %}
             <details class="margin-top-1 dja-detail-table" aria-role="button">
                 <summary class="padding-1 padding-left-0 dja-details-summary">Details</summary>
                 <div class="grid-container margin-left-0 padding-left-0 padding-right-0 dja-details-contents">

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 26/31] 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 %}
 <fieldset class="module aligned {{ fieldset.classes }}">
-    {# .gov override #}
-    {% block fieldset_title %}
-        {% if fieldset.name %}<h2>{{ fieldset.name }}</h2>{% endif %}
-    {% endblock fieldset_title %}
+    {% if fieldset.name %}<h2>{{ fieldset.name }}</h2>{% endif %}
 
-    {# .gov override #}
-    {% block fieldset_description %}
-        {% if fieldset.description %}
-            <div class="description">{{ fieldset.description|safe }}</div>
-        {% endif %}
-    {% endblock fieldset_description %}
+    {% if fieldset.description %}
+        <div class="description">{{ fieldset.description|safe }}</div>
+    {% endif %}
 
     {% for line in fieldset %}
         <div class="form-row{% if line.fields|length == 1 and line.errors %} errors{% endif %}{% if not line.has_visible_field %} hidden{% endif %}{% for field in line %}{% if field.field.name %} field-{{ field.field.name }}{% endif %}{% endfor %}">
@@ -45,11 +39,6 @@ https://github.com/django/django/blob/main/django/contrib/admin/templates/admin/
                                 {% endif %}
                             {% endif %}
                         </div>
-                    
-                    {# .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 @@
 <ul class="{% if no_title_top_padding %}margin-top-neg-1__detail-list{% endif %} padding-top-0 padding-left-0__detail-list margin-left-0__detail-list" {% if field_name %} id="id_detail_list__{{field_name}}" {% endif %}>
     {% if user.title or user.contact.title or user.email or user.contact.email or user.phone or user.contact.phone %}
         {# Title #}
-        <li>
-            {% 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 %}
+                <li>{{ user.contact.title }}</li>
             {% else %}
-                None
+                <li>{{ user.title }}</li>
             {% endif %}
-        </li>
+        {% else %}
+            <li>None</li>
+        {% endif %}
         {# Email #}
-        <li>
-            {% 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 %}
+                <li>{{ user.contact.email }}</li>
             {% else %}
-                None
+                <li>{{ user.email }}</li>
             {% endif %}
-        </li>
+        {% else %}
+            <li>None</li>
+        {% endif %}
+
         {# Phone #}
-        <li>
-            {% 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 %}</li>
+                <li>{{ user.contact.phone }}</li>
             {% else %}
-            None
+                <li>{{ user.phone }}</li>
             {% endif %}
-        </li>
+        {% else %}
+            <li>None</li>
+        {% endif %}
     {% else %}
         <li>
             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)
                         </dt>
                         <dd>
                             <address>
-                                {% if  contact.get_formatted_name  %}<a href="{% url 'admin:registrar_contact_change' contact.id %}">{{ contact.get_formatted_name }}</a><br />{% else %}None<br />{% endif %}
-                                {% if  contact.title  %}{{ contact.title }}<br />{% else %}None<br />{% endif %}
-                                {% if  contact.email  %}{{ contact.email }}<br />{% else %}None<br />{% endif %}
-                                {% if  contact.phone  %}{{ contact.phone.as_national }}{% else %}None{% endif %}
+                                {% if contact.get_formatted_name %}<a href="{% url 'admin:registrar_contact_change' contact.id %}">{{ contact.get_formatted_name }}</a><br />{% else %}None<br />{% endif %}
+                                {% if contact.title %}{{ contact.title }}<br />{% else %}None<br />{% endif %}
+                                {% if contact.email %}{{ contact.email }}<br />{% else %}None<br />{% endif %}
+                                {% if contact.phone %}{{ contact.phone }}{% else %}None{% endif %}
                             </address>
                         </dd>
                     {% 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</td>"),
-            ("email", "meoward.jones@igorville.gov</td>"),
-            ("phone", "(555) 123 12345</td>"),
+            ("title", "Treat inspector</li>"),
+            ("email", "meoward.jones@igorville.gov</li>"),
+            ("phone", "(555) 123 12345</li>"),
         ]
         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</td>"),
-            ("email", "mayor@igorville.gov</td>"),
-            ("phone", "(555) 555 5556</td>"),
+            ("title", "Admin Tester</li>"),
+            ("email", "mayor@igorville.gov</li>"),
+            ("phone", "(555) 555 5556</li>"),
         ]
         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</td>"),
-            ("email", "testy@town.com</td>"),
-            ("phone", "(555) 555 5555</td>"),
+            ("title", "Chief Tester</li>"),
+            ("email", "testy@town.com</li>"),
+            ("phone", "(555) 555 5555</li>"),
         ]
         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</td>"),
-            ("email", "testy2@town.com</td>"),
-            ("phone", "(555) 555 5557</td>"),
+            ("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</td>"),
-            ("email", "meoward.jones@igorville.gov</td>"),
-            ("phone", "(555) 123 12345</td>"),
+            ("title", "Treat inspector</li>"),
+            ("email", "meoward.jones@igorville.gov</li>"),
+            ("phone", "(555) 123 12345</li>"),
         ]
         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</td>"),
-            ("email", "mayor@igorville.gov</td>"),
-            ("phone", "(555) 555 5556</td>"),
+            ("title", "Admin Tester</li>"),
+            ("email", "mayor@igorville.gov</li>"),
+            ("phone", "(555) 555 5556</li>"),
         ]
         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</td>"),
-            ("email", "testy@town.com</td>"),
-            ("phone", "(555) 555 5555</td>"),
+            ("title", "Chief Tester</li>"),
+            ("email", "testy@town.com</li>"),
+            ("phone", "(555) 555 5555</li>"),
         ]
         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</td>"),
-            ("email", "testy2@town.com</td>"),
-            ("phone", "(555) 555 5557</td>"),
+            ("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 27/31] 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 28/31] 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 class="margin-top-1 dja-detail-table" aria-role="button">
+            <details class="margin-top-1 dja-detail-table" aria-role="button" open>
                 <summary class="padding-1 padding-left-0 dja-details-summary">Details</summary>
                 <div class="grid-container margin-left-0 padding-left-0 padding-right-0 dja-details-contents">
                     <table>

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 29/31] 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 %}
 
+<address class="{% if no_title_top_padding %}margin-top-neg-1__detail-list{% endif %} dja-address-contact-list">
+
+    {% if show_formatted_name %}
+        {% if contact.get_formatted_name %}
+            <a href="{% url 'admin:registrar_contact_change' user.id %}">{{ user.get_formatted_name }}</a><br />
+        {% else %}
+            None<br />
+        {% endif %}
+    {% endif %}
 
-<ul class="{% if no_title_top_padding %}margin-top-neg-1__detail-list{% endif %} padding-top-0 padding-left-0__detail-list margin-left-0__detail-list" {% if field_name %} id="id_detail_list__{{field_name}}" {% 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 %}
-                <li>{{ user.contact.title }}</li>
+                {{ user.contact.title }}
             {% else %}
-                <li>{{ user.title }}</li>
+                {{ user.title }}
             {% endif %}
+            <br>
         {% else %}
-            <li>None</li>
+            None<br>
         {% endif %}
         {# Email #}
         {% if user.email or user.contact.email %}
             {% if user.contact.email %}
-                <li>{{ user.contact.email }}</li>
+                {{ user.contact.email }}
             {% else %}
-                <li>{{ user.email }}</li>
+                {{ user.email }}
             {% endif %}
+            <br>
         {% else %}
-            <li>None</li>
+            None<br>
         {% endif %}
 
         {# Phone #}
         {% if user.phone or user.contact.phone %}
-            {% if user.contact.phone %}</li>
-                <li>{{ user.contact.phone }}</li>
+            {% if user.contact.phone %}
+                {{ user.contact.phone }}
             {% else %}
-                <li>{{ user.phone }}</li>
+                {{ user.phone }}
             {% endif %}
+            <br>
         {% else %}
-            <li>None</li>
+            None<br>
         {% endif %}
     {% else %}
-        <li>
-            No additional contact information found.
-        </li>
+        No additional contact information found.
     {% endif %}
-</ul>
+</address>
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 %}
             <div class="readonly">
-                {% for contact in field.contents|split:", " %}
-                    <a href="#" class="other-contact__{{forloop.counter}}">{{ contact }}</a>{% if not forloop.last %}, {% endif %}
+                {% for contact in all_contacts %}
+                    <a href="{% url 'admin:registrar_contact_change' contact.id %}">{{ contact.get_formatted_name }}</a>{% if not forloop.last %}, {% endif %}
                 {% endfor %}
             </div>
         {% else %}
             <div class="flex-container dja-detail-list">
-                <dl class="usa-list usa-list--unstyled margin-0 margin-top-1">
+                <dl class="usa-list usa-list--unstyled margin-0">
                     {% for contact in all_contacts %}
                         <dt class="{% if forloop.counter == 1%}margin-top-0{% endif %}">
                             Organization contact {{forloop.counter}}
                         </dt>
                         <dd>
-                            <address>
-                                {% if contact.get_formatted_name %}<a href="{% url 'admin:registrar_contact_change' contact.id %}">{{ contact.get_formatted_name }}</a><br />{% else %}None<br />{% endif %}
-                                {% if contact.title %}{{ contact.title }}<br />{% else %}None<br />{% endif %}
-                                {% if contact.email %}{{ contact.email }}<br />{% else %}None<br />{% endif %}
-                                {% if contact.phone %}{{ contact.phone }}{% else %}None{% endif %}
-                            </address>
+                            {% include "django/admin/includes/contact_detail_list.html" with user=contact show_formatted_name=True %}
                         </dd>
                     {% endfor %}
                 </dl>
@@ -52,6 +47,12 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
             {% endfor %}
         {% endwith %}
         </div>
+    {% elif field.field.name == "alternative_domains" %}
+        <div class="readonly">
+            {% for alt_domain in original.alternative_domains.all %}
+                <a href="{% url 'admin:registrar_website_change' alt_domain.id %}">{{ alt_domain }}</a>{% if not forloop.last %}, {% endif %}
+            {% endfor %}
+        </div>
     {% else %}
     <div class="readonly">{{ field.contents }}</div>
     {% endif %}
@@ -62,17 +63,17 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
     {% if field.field.name == "creator" %}
         <div class="flex-container">
         <label aria-label="Creator contact details"></label>
-        {% 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 %}
         </div>
     {% elif field.field.name == "submitter" %}
         <div class="flex-container">
         <label aria-label="Submitter contact details"></label>
-        {% 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 %}
         </div>
     {% elif field.field.name == "authorizing_official" %}
         <div class="flex-container">
         <label aria-label="Authorizing official contact details"></label>
-        {% 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 %}
         </div>
     {% 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)
                     <table>
                         <thead>
                             <tr>
-                                <th colspan="4">Contact information</th>
+                                <th colspan="4">Other contact information</th>
                             <tr>
                         </thead>
                         <tbody>
                             {% 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 %}
-                            <tr data-contact-id="{{ contact.id }}" data-contact-url="{% url 'admin:registrar_contact_change' contact.id %}">
-                                <th class="padding-left-1" scope="row">{{contact.first_name}} {{contact.last_name}}</th>
+                            <tr>
+                                <th class="padding-left-1" scope="row">{{ contact.get_formatted_name }}</th>
                                 <td class="padding-left-1">{{ contact.title }}</td>
                                 <td class="padding-left-1">{{ contact.email }}</td>
                                 <td class="padding-left-1">{{ contact.phone }}</td>
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</li>"),
-            ("email", "meoward.jones@igorville.gov</li>"),
-            ("phone", "(555) 123 12345</li>"),
+            ("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</li>"),
-            ("email", "mayor@igorville.gov</li>"),
-            ("phone", "(555) 555 5556</li>"),
+            ("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</li>"),
-            ("email", "testy@town.com</li>"),
-            ("phone", "(555) 555 5555</li>"),
+            ("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</li>"),
-            ("email", "meoward.jones@igorville.gov</li>"),
-            ("phone", "(555) 123 12345</li>"),
+            ("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</li>"),
-            ("email", "mayor@igorville.gov</li>"),
-            ("phone", "(555) 555 5556</li>"),
+            ("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</li>"),
-            ("email", "testy@town.com</li>"),
-            ("phone", "(555) 555 5555</li>"),
+            ("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 30/31] 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 31/31] 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)