accessible markup and unit test coverage

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

@@ -342,6 +342,40 @@ div#content > h2 {
     }
 }
 
+.module {
+    .margin-left-0 {
+        margin-left: 0;
+    }
+    .margin-top-0 {
+        margin-top: 0;
+    }
+    .padding-left-0 {
+        padding-left: 0;
+    }
+}
+
+.admin-list-inline {
+    li {
+        float: left;
+        padding-top: 0;
+        margin-right: 4px;
+    }
+    li:not(:last-child)::after {
+        content: ",";
+    }
+}
+
+.form-row {
+    .margin-y-0 {
+        margin-top: 0;
+        margin-bottom: 0;
+    }
+    .padding-y-0 {
+        padding-top: 0;
+        padding-bottom: 0;
+    }
+}
+
 // Fixes a display issue where the list was entirely white, or had too much whitespace
 .select2-dropdown {
     display: inline-grid !important;

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

@@ -110,21 +110,37 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
         {% endcomment %}
         <div class="readonly">
             {% with total_websites=field.contents|split:", " %}
+                {% if total_websites|length == 1 %}
+                    <p class="margin-y-0 padding-y-0">
+                        <a href="{{ total_websites.0 }}" target="_blank">
+                            {{ total_websites.0 }}
+                        </a>
+                    </p>
+                {% elif total_websites|length > 1 %}
+                    <ul class="margin-top-0 margin-left-0 padding-left-0{% if total_websites|length > 5 %} admin-list-inline{% endif %}">
                         {% for website in total_websites %}
-                <a href="{{ website }}" target="_blank" class="padding-top-1 current-website__{{forloop.counter}}">{{ website }}</a>{% if not forloop.last %}, {% endif %}
+                            {% comment %}White space matters: do NOT reformat the following line{% endcomment %}
-                {# Acts as a <br> #}
+                            <li><a href="{{ website }}" target="_blank">{{ website }}</a></li>
-                {% if total_websites|length < 5 %}
-                <div class="display-block margin-top-1"></div>
-                {% endif %}
                         {% endfor %}
+                    </ul>
+                {% endif %}
             {% endwith %}
         </div>
     {% elif field.field.name == "alternative_domains" %}
         <div class="readonly">
             {% with current_path=request.get_full_path %}
+                {% if original_object.alternative_domains.all|length == 1 %}
+                    <p class="margin-y-0 padding-y-0">
+                        <a href="{% url 'admin:registrar_website_change' original_object.alternative_domains.all.0.id %}?{{ 'return_path='|add:current_path }}" target="_blank">{{ original_object.alternative_domains.all.0 }}</a>
+                    </p>
+                {% elif original_object.alternative_domains.all|length > 1 %}
+                    <ul class="margin-top-0 margin-left-0 padding-left-0 admin-list-inline">
                         {% for alt_domain in original_object.alternative_domains.all %}
-                <a href="{% url 'admin:registrar_website_change' alt_domain.id %}?{{ 'return_path='|add:current_path }}">{{ alt_domain }}</a>{% if not forloop.last %}, {% endif %}
+                            {% comment %}White space matters: do NOT reformat the following line{% endcomment %}
+                            <li><a href="{% url 'admin:registrar_website_change' alt_domain.id %}?{{ 'return_path='|add:current_path }}" target="_blank">{{alt_domain}}</a></li>
                         {% endfor %}
+                    </ul>
+                {% endif %}
             {% endwith %}
         </div>
     {% elif field.field.name == "domain_managers" or field.field.name == "invited_domain_managers" %}

src/registrar/tests/common.py

@@ -13,6 +13,7 @@ from django.contrib.auth import get_user_model, login
 from django.utils.timezone import make_aware
 from datetime import date, datetime, timedelta
 from django.utils import timezone
+from django.utils.html import strip_spaces_between_tags
 
 from registrar.models import (
     Contact,
@@ -107,6 +108,11 @@ def get_time_aware_date(date=datetime(2023, 11, 1)):
     return timezone.make_aware(date)
 
 
+def normalize_html(html):
+    """Normalize HTML by removing newlines and extra spaces."""
+    return strip_spaces_between_tags(" ".join(html.split()))
+
+
 class GenericTestHelper(TestCase):
     """A helper class that contains various helper functions for TestCases"""
 
@@ -1017,8 +1023,9 @@ def create_ready_domain():
 # TODO in 1793: Remove the federal agency/updated federal agency fields
 def completed_domain_request(  # noqa
     has_other_contacts=True,
-    has_current_website=True,
+    # pass empty [] if you want current_websites or alternative_domains set to None
-    has_alternative_gov_domain=True,
+    current_websites=["city.com"],
+    alternative_domains=["city1.gov"],
     has_about_your_organization=True,
     has_anything_else=True,
     has_cisa_representative=True,
@@ -1046,8 +1053,6 @@ def completed_domain_request(  # noqa
         phone="(555) 555 5555",
     )
     domain, _ = DraftDomain.objects.get_or_create(name=name)
-    alt, _ = Website.objects.get_or_create(website="city1.gov")
-    current, _ = Website.objects.get_or_create(website="city.com")
     other, _ = Contact.objects.get_or_create(
         first_name="Testy",
         last_name="Tester",
@@ -1102,9 +1107,13 @@ def completed_domain_request(  # noqa
 
     if has_other_contacts:
         domain_request.other_contacts.add(other)
-    if has_current_website:
+    if len(current_websites) > 0:
+        for website in current_websites:
+            current, _ = Website.objects.get_or_create(website=website)
             domain_request.current_websites.add(current)
-    if has_alternative_gov_domain:
+    if len(alternative_domains) > 0:
+        for alternative_domain in alternative_domains:
+            alt, _ = Website.objects.get_or_create(website=alternative_domain)
             domain_request.alternative_domains.add(alt)
     if has_cisa_representative:
         domain_request.cisa_representative_first_name = "CISA-first-name"

src/registrar/tests/test_admin_request.py

@@ -35,6 +35,7 @@ from .common import (
     multiple_unalphabetical_domain_objects,
     MockEppLib,
     GenericTestHelper,
+    normalize_html,
 )
 from unittest.mock import patch
 
@@ -1445,8 +1446,9 @@ class TestDomainRequestAdmin(MockEppLib):
         self.assertContains(response, expected_url)
 
     @less_console_noise_decorator
-    def test_other_websites_has_readonly_link(self):
+    def test_other_websites_has_one_readonly_link(self):
-        """Tests if the readonly other_websites field has links"""
+        """Tests if the readonly other_websites field has links.
+        Test markup for one website."""
 
         # Create a fake domain request
         domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW)
@@ -1462,8 +1464,179 @@ class TestDomainRequestAdmin(MockEppLib):
         self.assertContains(response, domain_request.requested_domain.name)
 
         # Check that the page contains the link we expect.
-        expected_url = '<a href="city.com" target="_blank" class="padding-top-1 current-website__1">city.com</a>'
+        expected_markup = """
-        self.assertContains(response, expected_url)
+            <p class="margin-y-0 padding-y-0">
+                <a href="city.com" target="_blank">
+                    city.com
+                </a>
+            </p>
+            """
+
+        normalized_expected = normalize_html(expected_markup)
+        normalized_response = normalize_html(response.content.decode("utf-8"))
+
+        self.assertIn(normalized_expected, normalized_response)
+
+    @less_console_noise_decorator
+    def test_other_websites_has_few_readonly_links(self):
+        """Tests if the readonly other_websites field has links.
+        Test markup for l5 or less websites."""
+
+        # Create a domain request with 4 current websites
+        domain_request = completed_domain_request(
+            status=DomainRequest.DomainRequestStatus.IN_REVIEW,
+            current_websites=["city.gov", "city2.gov", "city3.gov", "city4.gov"],
+        )
+
+        self.client.force_login(self.staffuser)
+        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_markup = """
+            <ul class="margin-top-0 margin-left-0 padding-left-0">
+                <li><a href="city.gov" target="_blank">city.gov</a></li>
+                <li><a href="city2.gov" target="_blank">city2.gov</a></li>
+                <li><a href="city3.gov" target="_blank">city3.gov</a></li>
+                <li><a href="city4.gov" target="_blank">city4.gov</a></li>
+            </ul>
+            """
+
+        normalized_expected = normalize_html(expected_markup)
+        normalized_response = normalize_html(response.content.decode("utf-8"))
+
+        self.assertIn(normalized_expected, normalized_response)
+
+    @less_console_noise_decorator
+    def test_other_websites_has_lots_readonly_links(self):
+        """Tests if the readonly other_websites field has links.
+        Test markup for 6 or more websites."""
+
+        # Create a domain requests with 6 current websites
+        domain_request = completed_domain_request(
+            status=DomainRequest.DomainRequestStatus.IN_REVIEW,
+            current_websites=["city.gov", "city2.gov", "city3.gov", "city4.gov", "city5.gov", "city6.gov"],
+        )
+
+        self.client.force_login(self.staffuser)
+        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_markup = """
+            <ul class="margin-top-0 margin-left-0 padding-left-0 admin-list-inline">
+                <li><a href="city.gov" target="_blank">city.gov</a></li>
+                <li><a href="city2.gov" target="_blank">city2.gov</a></li>
+                <li><a href="city3.gov" target="_blank">city3.gov</a></li>
+                <li><a href="city4.gov" target="_blank">city4.gov</a></li>
+                <li><a href="city5.gov" target="_blank">city5.gov</a></li>
+                <li><a href="city6.gov" target="_blank">city6.gov</a></li>
+            </ul>
+            """
+
+        normalized_expected = normalize_html(expected_markup)
+        normalized_response = normalize_html(response.content.decode("utf-8"))
+
+        self.assertIn(normalized_expected, normalized_response)
+
+    @less_console_noise_decorator
+    def test_alternative_domains_has_one_readonly_link(self):
+        """Tests if the readonly alternative_domains field has links.
+        Test markup for one website."""
+
+        # Create a fake domain request
+        domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW)
+
+        self.client.force_login(self.staffuser)
+        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.
+        website = Website.objects.filter(website="city1.gov").first()
+        base_url = "/admin/registrar/website"
+        return_path = f"/admin/registrar/domainrequest/{domain_request.pk}/change/"
+        expected_markup = f"""
+            <p class="margin-y-0 padding-y-0">
+            <a href="{base_url}/{website.pk}/change/?return_path={return_path}" target="_blank">city1.gov</a>
+            </p>
+        """
+
+        normalized_expected = normalize_html(expected_markup)
+        normalized_response = normalize_html(response.content.decode("utf-8"))
+
+        self.assertIn(normalized_expected, normalized_response)
+
+    @less_console_noise_decorator
+    def test_alternative_domains_has_lots_readonly_link(self):
+        """Tests if the readonly other_websites field has links.
+        Test markup for 6 or more websites."""
+
+        # Create a domain request with 6 alternative domains
+        domain_request = completed_domain_request(
+            status=DomainRequest.DomainRequestStatus.IN_REVIEW,
+            alternative_domains=[
+                "altcity1.gov",
+                "altcity2.gov",
+                "altcity3.gov",
+                "altcity4.gov",
+                "altcity5.gov",
+                "altcity6.gov",
+            ],
+        )
+
+        self.client.force_login(self.staffuser)
+        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.
+        website1 = Website.objects.filter(website="altcity1.gov").first()
+        website2 = Website.objects.filter(website="altcity2.gov").first()
+        website3 = Website.objects.filter(website="altcity3.gov").first()
+        website4 = Website.objects.filter(website="altcity4.gov").first()
+        website5 = Website.objects.filter(website="altcity5.gov").first()
+        website6 = Website.objects.filter(website="altcity6.gov").first()
+        base_url = "/admin/registrar/website"
+        return_path = f"/admin/registrar/domainrequest/{domain_request.pk}/change/"
+        attr = 'target="_blank"'
+        expected_markup = f"""
+            <ul class="margin-top-0 margin-left-0 padding-left-0 admin-list-inline">
+                <li><a href="{base_url}/{website1.pk}/change/?return_path={return_path}" {attr}>altcity1.gov</a></li>
+                <li><a href="{base_url}/{website2.pk}/change/?return_path={return_path}" {attr}>altcity2.gov</a></li>
+                <li><a href="{base_url}/{website3.pk}/change/?return_path={return_path}" {attr}>altcity3.gov</a></li>
+                <li><a href="{base_url}/{website4.pk}/change/?return_path={return_path}" {attr}>altcity4.gov</a></li>
+                <li><a href="{base_url}/{website5.pk}/change/?return_path={return_path}" {attr}>altcity5.gov</a></li>
+                <li><a href="{base_url}/{website6.pk}/change/?return_path={return_path}" {attr}>altcity6.gov</a></li>
+            </ul>
+            """
+
+        normalized_expected = normalize_html(expected_markup)
+        normalized_response = normalize_html(response.content.decode("utf-8"))
+
+        self.assertIn(normalized_expected, normalized_response)
 
     @less_console_noise_decorator
     def test_contact_fields_have_detail_table(self):

src/registrar/tests/test_emails.py

@@ -150,7 +150,7 @@ class TestEmails(TestCase):
     def test_submission_confirmation_no_current_website_spacing(self):
         """Test line spacing without current_website."""
         domain_request = completed_domain_request(
-            has_current_website=False, user=User.objects.create(username="test", email="testy@town.com")
+            current_websites=[], user=User.objects.create(username="test", email="testy@town.com")
         )
         with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class):
             domain_request.submit()
@@ -164,9 +164,7 @@ class TestEmails(TestCase):
     @less_console_noise_decorator
     def test_submission_confirmation_current_website_spacing(self):
         """Test line spacing with current_website."""
-        domain_request = completed_domain_request(
+        domain_request = completed_domain_request(user=User.objects.create(username="test", email="testy@town.com"))
-            has_current_website=True, user=User.objects.create(username="test", email="testy@town.com")
-        )
         with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class):
             domain_request.submit()
         _, kwargs = self.mock_client.send_email.call_args
@@ -218,9 +216,7 @@ class TestEmails(TestCase):
     @less_console_noise_decorator
     def test_submission_confirmation_alternative_govdomain_spacing(self):
         """Test line spacing with alternative .gov domain."""
-        domain_request = completed_domain_request(
+        domain_request = completed_domain_request(user=User.objects.create(username="test", email="testy@town.com"))
-            has_alternative_gov_domain=True, user=User.objects.create(username="test", email="testy@town.com")
-        )
         with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class):
             domain_request.submit()
         _, kwargs = self.mock_client.send_email.call_args
@@ -234,7 +230,7 @@ class TestEmails(TestCase):
     def test_submission_confirmation_no_alternative_govdomain_spacing(self):
         """Test line spacing without alternative .gov domain."""
         domain_request = completed_domain_request(
-            has_alternative_gov_domain=False, user=User.objects.create(username="test", email="testy@town.com")
+            alternative_domains=[], user=User.objects.create(username="test", email="testy@town.com")
         )
         with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class):
             domain_request.submit()