Merge pull request #3290 from cisagov/el/2232-admin-list-markup

#2232: Accessible markup and unit test coverage for admin lists

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

@@ -351,6 +351,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

@@ -109,22 +109,38 @@
         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 }}" target="_blank" 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>
+            {% 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 %}
+                            {% comment %}White space matters: do NOT reformat the following line{% endcomment %}
+                            <li><a href="{{ website }}" target="_blank">{{ website }}</a></li>
+                        {% endfor %}
+                    </ul>
                 {% endif %}
-            {% endfor %}
-        {% endwith %}
+            {% endwith %}
         </div>
     {% elif field.field.name == "alternative_domains" %}
         <div class="readonly">
             {% with current_path=request.get_full_path %}
-            {% 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 %}
-            {% endfor %}
+                {% 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 %}
+                            {% 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.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,
-    has_alternative_gov_domain=True,
+    # pass empty [] if you want current_websites or alternative_domains set to None
+    current_websites=["city.com"],
+    alternative_domains=["city1.gov"],
     has_about_your_organization=True,
     has_anything_else=True,
     has_cisa_representative=True,
@@ -1050,8 +1057,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",
@@ -1118,10 +1123,14 @@ def completed_domain_request(  # noqa
 
     if has_other_contacts:
         domain_request.other_contacts.add(other)
-    if has_current_website:
-        domain_request.current_websites.add(current)
-    if has_alternative_gov_domain:
-        domain_request.alternative_domains.add(alt)
+    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 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"
         domain_request.cisa_representative_last_name = "CISA-last-name"

src/registrar/tests/test_admin_request.py

@@ -40,6 +40,7 @@
     multiple_unalphabetical_domain_objects,
     MockEppLib,
     GenericTestHelper,
+    normalize_html,
 )
 from unittest.mock import ANY, patch
 
@@ -1530,8 +1531,9 @@ def test_other_contacts_has_readonly_link(self):
         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"""
+    def test_other_websites_has_one_readonly_link(self):
+        """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)
@@ -1547,8 +1549,224 @@ def test_other_websites_has_readonly_link(self):
         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>'
-        self.assertContains(response, expected_url)
+        expected_markup = """
+            <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"))
+
+        index = normalized_response.find(normalized_expected)
+
+        # Assert that the expected markup is found in the response
+        if index == -1:
+            self.fail(
+                f"Expected markup not found in the response.\n\n"
+                f"Expected:\n{normalized_expected}\n\n"
+                f"Start index of mismatch: {index}\n\n"
+                f"Consider checking the surrounding response for context."
+            )
+
+    @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 5 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"))
+
+        index = normalized_response.find(normalized_expected)
+
+        # Assert that the expected markup is found in the response
+        if index == -1:
+            self.fail(
+                f"Expected markup not found in the response.\n\n"
+                f"Expected:\n{normalized_expected}\n\n"
+                f"Start index of mismatch: {index}\n\n"
+                f"Consider checking the surrounding response for context."
+            )
+
+    @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"))
+
+        index = normalized_response.find(normalized_expected)
+
+        # Assert that the expected markup is found in the response
+        if index == -1:
+            self.fail(
+                f"Expected markup not found in the response.\n\n"
+                f"Expected:\n{normalized_expected}\n\n"
+                f"Start index of mismatch: {index}\n\n"
+                f"Consider checking the surrounding response for context."
+            )
+
+    @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"))
+
+        index = normalized_response.find(normalized_expected)
+
+        # Assert that the expected markup is found in the response
+        if index == -1:
+            self.fail(
+                f"Expected markup not found in the response.\n\n"
+                f"Expected:\n{normalized_expected}\n\n"
+                f"Start index of mismatch: {index}\n\n"
+                f"Consider checking the surrounding response for context."
+            )
+
+    @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"))
+
+        index = normalized_response.find(normalized_expected)
+
+        # Assert that the expected markup is found in the response
+        if index == -1:
+            self.fail(
+                f"Expected markup not found in the response.\n\n"
+                f"Expected:\n{normalized_expected}\n\n"
+                f"Start index of mismatch: {index}\n\n"
+                f"Consider checking the surrounding response for context."
+            )
 
     @less_console_noise_decorator
     def test_contact_fields_have_detail_table(self):