Merge pull request #1927 from cisagov/za/1852-user-contact-info-inline

(on getgov-za) Ticket #1852 / #1836: Contact info inline + Remove unnecessary and "dangerous" multi-selects

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 

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",
@@ -938,6 +941,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:
@@ -1117,6 +1122,9 @@ class DomainRequestAdmin(ListHeaderAdmin):
         ),
     ]
 
+    # Readonly fields for analysts and superusers
+    readonly_fields = ("other_contacts", "current_websites", "alternative_domains")
+
     # Read only that we'll leverage for CISA Analysts
     analyst_readonly_fields = [
         "creator",
@@ -1143,6 +1151,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"""
@@ -1297,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
@@ -1408,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":

src/registrar/assets/js/get-gov-reports.js

@@ -54,64 +54,77 @@
 
 })();
 
-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) {
+/** An IIFE to initialize the analytics page
-    var canvas = document.getElementById(canvasId);
+*/
-    var ctx = canvas.getContext("2d");
+(function () {
+    function createComparativeColumnChart(canvasId, title, labelOne, labelTwo) {
+        var canvas = document.getElementById(canvasId);
+        if (!canvas) {
+            return
+        }
 
-    var listOne = JSON.parse(canvas.getAttribute('data-list-one'));
+        var ctx = canvas.getContext("2d");
-    var listTwo = JSON.parse(canvas.getAttribute('data-list-two'));
 
-    var data = {
+        var listOne = JSON.parse(canvas.getAttribute('data-list-one'));
-        labels: ["Total", "Federal", "Interstate", "State/Territory", "Tribal", "County", "City", "Special District", "School District", "Election Board"],
+        var listTwo = JSON.parse(canvas.getAttribute('data-list-two'));
-        datasets: [
+
-            {
+        var data = {
-                label: labelOne,
+            labels: ["Total", "Federal", "Interstate", "State/Territory", "Tribal", "County", "City", "Special District", "School District", "Election Board"],
-                backgroundColor: "rgba(255, 99, 132, 0.2)",
+            datasets: [
-                borderColor: "rgba(255, 99, 132, 1)",
+                {
-                borderWidth: 1,
+                    label: labelOne,
-                data: listOne,
+                    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
+                }
             },
-            {
+            scales: {
-                label: labelTwo,
+                y: {
-                backgroundColor: "rgba(75, 192, 192, 0.2)",
+                    beginAtZero: true,
-                borderColor: "rgba(75, 192, 192, 1)",
+                },
-                borderWidth: 1,
-                data: listTwo,
             },
-        ],
+        };
+
+        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 = {
+    initComparativeColumnCharts();
-        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,
-    });
-}

src/registrar/assets/sass/_theme/_admin.scss

@@ -347,6 +347,77 @@ input.admin-confirm-button {
     color: $dhs-blue-70;
 }
 
+details.dja-detail-table {
+    display: inline-table;
+    background-color: var(--body-bg);
+    .dja-details-summary {
+        cursor: pointer;
+        color: var(--body-quiet-color);
+    }
+
+    @media (max-width: 1024px){
+        .dja-detail-contents {
+            max-width: 400px !important;
+            overflow-x: scroll !important;
+        }
+    }
+
+    tr {
+        background-color: transparent;
+    }
+
+    td, th {
+        padding-left: 12px;
+        border: none
+    }
+
+    thead > tr > th {
+        border-radius: 4px;
+        border-top: none;
+        border-bottom: none;
+    }
+
+}
+
+
+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 {
+        font-size: 0.8125rem;
+        color: var(--body-quiet-color);
+    }
+
+    address {
+        font-size: 0.8125rem;
+        color: var(--body-quiet-color);
+    }
+}
+
+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 {
+        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;
 }

src/registrar/templates/admin/fieldset.html

@@ -0,0 +1,61 @@
+{% 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 }}">
+    {% if fieldset.name %}<h2>{{ fieldset.name }}</h2>{% endif %}
+
+    {% 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 %}">
+            {% 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 %}
+                                {# .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%}
+                                {% endif %}
+                            {% endif %}
+                        </div>
+
+                    {% 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 %}
+                </div>
+            {% endfor %}
+            {% if not line.fields|length == 1 %}</div>{% endif %}
+        </div>
+    {% endfor %}
+</fieldset>

src/registrar/templates/django/admin/domain_information_change_form.html

@@ -0,0 +1,15 @@
+{% extends 'admin/change_form.html' %}
+{% load i18n static %}
+
+{% 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 %}

src/registrar/templates/django/admin/domain_request_change_form.html

@@ -4,7 +4,17 @@
 {% 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 }}
+
+    {% 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 %}
 
 {% block submit_buttons_bottom %}
@@ -93,4 +103,4 @@
         </div>
 </div>
 {{ block.super }}
-{% endblock %}
+{% endblock %}

src/registrar/templates/django/admin/includes/contact_detail_list.html

@@ -0,0 +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 %}
+
+    {% if user.title or user.contact.title or user.email or user.contact.email or user.phone or user.contact.phone %}
+        {# Title #}
+        {% if user.title or user.contact.title %}
+            {% if user.contact.title %}
+                {{ user.contact.title }}
+            {% else %}
+                {{ user.title }}
+            {% endif %}
+            <br>
+        {% else %}
+            None<br>
+        {% endif %}
+        {# Email #}
+        {% if user.email or user.contact.email %}
+            {% if user.contact.email %}
+                {{ user.contact.email }}
+            {% else %}
+                {{ user.email }}
+            {% endif %}
+            <br>
+        {% else %}
+            None<br>
+        {% endif %}
+
+        {# Phone #}
+        {% if user.phone or user.contact.phone %}
+            {% if user.contact.phone %}
+                {{ user.contact.phone }}
+            {% else %}
+                {{ user.phone }}
+            {% endif %}
+            <br>
+        {% else %}
+            None<br>
+        {% endif %}
+    {% else %}
+        No additional contact information found.
+    {% endif %}
+</address>

src/registrar/templates/django/admin/includes/detail_table_fieldset.html

@@ -0,0 +1,106 @@
+{% 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 %}
+    {% with all_contacts=original.other_contacts.all %}
+    {% if field.field.name == "other_contacts" %}
+        {% if all_contacts.count > 2 %}
+            <div class="readonly">
+                {% 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">
+                    {% for contact in all_contacts %}
+                        <dt class="{% if forloop.counter == 1%}margin-top-0{% endif %}">
+                            Organization contact {{forloop.counter}}
+                        </dt>
+                        <dd>
+                            {% include "django/admin/includes/contact_detail_list.html" with user=contact show_formatted_name=True %}
+                        </dd>
+                    {% endfor %}
+                </dl>
+            </div>
+        {% endif %}
+    {% 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>
+    {% 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 %}
+    {% 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_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 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 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 > 2 %}
+            <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>
+                        <thead>
+                            <tr>
+                                <th colspan="4">Other contact information</th>
+                            <tr>
+                        </thead>
+                        <tbody>
+                            {% for contact in all_contacts %}
+                            <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>
+                            </tr>
+                            {% endfor %}
+                        </tbody>
+                    </table>
+                </div>
+            </details>
+        {% endif %}
+        {% endwith %}
+    {% endif %}
+{% endblock after_help_text %}

src/registrar/templatetags/url_helpers.py

@@ -26,6 +26,14 @@ 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.

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.
@@ -179,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
 
 

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
@@ -1211,6 +1212,140 @@ 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_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)
+
+        # Get the other contact
+        other_contact = domain_request.other_contacts.all().first()
+
+        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 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_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
+    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)
+
+        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 == #
+
+        # Check for the right title, email, and phone number in the response.
+        expected_creator_fields = [
+            # Field, expected value
+            ("title", "Treat inspector"),
+            ("email", "meoward.jones@igorville.gov"),
+            ("phone", "(555) 123 12345"),
+        ]
+        self.test_helper.assert_response_contains_distinct_values(response, expected_creator_fields)
+
+        # Check for the field itself
+        self.assertContains(response, "Meoward Jones")
+
+        # == Check for the submitter == #
+        expected_submitter_fields = [
+            # Field, expected value
+            ("title", "Admin Tester"),
+            ("email", "mayor@igorville.gov"),
+            ("phone", "(555) 555 5556"),
+        ]
+        self.test_helper.assert_response_contains_distinct_values(response, expected_submitter_fields)
+        self.assertContains(response, "Testy2 Tester2")
+
+        # == Check for the authorizing_official == #
+        expected_ao_fields = [
+            # Field, expected value
+            ("title", "Chief Tester"),
+            ("email", "testy@town.com"),
+            ("phone", "(555) 555 5555"),
+        ]
+        self.test_helper.assert_response_contains_distinct_values(response, expected_ao_fields)
+
+        # count=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 = [
+            # Field, expected value
+            ("title", "Another Tester"),
+            ("email", "testy2@town.com"),
+            ("phone", "(555) 555 5557"),
+        ]
+        self.test_helper.assert_response_contains_distinct_values(response, expected_other_employees_fields)
+
     def test_save_model_sets_restricted_status_on_user(self):
         with less_console_noise():
             # make sure there is no user with this email
@@ -1294,6 +1429,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):
@@ -1306,6 +1442,9 @@ class TestDomainRequestAdmin(MockEppLib):
             readonly_fields = self.admin.get_readonly_fields(request, domain_request)
 
             expected_fields = [
+                "other_contacts",
+                "current_websites",
+                "alternative_domains",
                 "id",
                 "created_at",
                 "updated_at",
@@ -1338,8 +1477,6 @@ class TestDomainRequestAdmin(MockEppLib):
                 "is_policy_acknowledged",
                 "submission_date",
                 "notes",
-                "current_websites",
-                "other_contacts",
                 "alternative_domains",
             ]
 
@@ -1353,6 +1490,9 @@ class TestDomainRequestAdmin(MockEppLib):
             readonly_fields = self.admin.get_readonly_fields(request)
 
             expected_fields = [
+                "other_contacts",
+                "current_websites",
+                "alternative_domains",
                 "creator",
                 "about_your_organization",
                 "requested_domain",
@@ -1374,7 +1514,11 @@ class TestDomainRequestAdmin(MockEppLib):
 
             readonly_fields = self.admin.get_readonly_fields(request)
 
-            expected_fields = []
+            expected_fields = [
+                "other_contacts",
+                "current_websites",
+                "alternative_domains",
+            ]
 
             self.assertEqual(readonly_fields, expected_fields)
 
@@ -1804,6 +1948,125 @@ 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"),
+            ("email", "meoward.jones@igorville.gov"),
+            ("phone", "(555) 123 12345"),
+        ]
+        self.test_helper.assert_response_contains_distinct_values(response, expected_creator_fields)
+
+        # Check for the field itself
+        self.assertContains(response, "Meoward Jones")
+
+        # == Check for the submitter == #
+        expected_submitter_fields = [
+            # Field, expected value
+            ("title", "Admin Tester"),
+            ("email", "mayor@igorville.gov"),
+            ("phone", "(555) 555 5556"),
+        ]
+        self.test_helper.assert_response_contains_distinct_values(response, expected_submitter_fields)
+        self.assertContains(response, "Testy2 Tester2")
+
+        # == Check for the authorizing_official == #
+        expected_ao_fields = [
+            # Field, expected value
+            ("title", "Chief Tester"),
+            ("email", "testy@town.com"),
+            ("phone", "(555) 555 5555"),
+        ]
+        self.test_helper.assert_response_contains_distinct_values(response, expected_ao_fields)
+
+        # count=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 = [
+            # Field, expected value
+            ("title", "Another Tester"),
+            ("email", "testy2@town.com"),
+            ("phone", "(555) 555 5557"),
+        ]
+        self.test_helper.assert_response_contains_distinct_values(response, expected_other_employees_fields)
+
     def test_readonly_fields_for_analyst(self):
         """Ensures that analysts have their permissions setup correctly"""
         with less_console_noise():
@@ -1813,6 +2076,7 @@ class TestDomainInformationAdmin(TestCase):
             readonly_fields = self.admin.get_readonly_fields(request)
 
             expected_fields = [
+                "other_contacts",
                 "creator",
                 "type_of_work",
                 "more_organization_information",