Merge branch 'main' of https://github.com/cisagov/manage.get.gov into rh/3129-domain-renewal-form

src/registrar/admin.py

@@ -1434,6 +1434,20 @@ class DomainInvitationAdmin(ListHeaderAdmin):
         # Get the filtered values
         return super().changelist_view(request, extra_context=extra_context)
 
+    def save_model(self, request, obj, form, change):
+        """
+        Override the save_model method.
+
+        On creation of a new domain invitation, attempt to retrieve the invitation,
+        which will be successful if a single User exists for that email; otherwise, will
+        just continue to create the invitation.
+        """
+        if not change and User.objects.filter(email=obj.email).count() == 1:
+            # Domain Invitation creation for an existing User
+            obj.retrieve()
+        # Call the parent save method to save the object
+        super().save_model(request, obj, form, change)
+
 
 class PortfolioInvitationAdmin(ListHeaderAdmin):
     """Custom portfolio invitation admin class."""

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 is using a custom implementation fieldset.html (see admin/fieldset.html)
         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.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,
-    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.py

@@ -131,13 +131,11 @@ class TestDomainInvitationAdmin(TestCase):
       tests have available superuser, client, and admin
     """
 
-    @classmethod
-    def setUpClass(cls):
-        cls.factory = RequestFactory()
-        cls.admin = ListHeaderAdmin(model=DomainInvitationAdmin, admin_site=AdminSite())
-        cls.superuser = create_superuser()
-
     def setUp(self):
+        self.factory = RequestFactory()
+        self.admin = ListHeaderAdmin(model=DomainInvitationAdmin, admin_site=AdminSite())
+        self.superuser = create_superuser()
+        self.domain = Domain.objects.create(name="example.com")
         """Create a client object"""
         self.client = Client(HTTP_HOST="localhost:8080")
 
@@ -145,9 +143,6 @@ class TestDomainInvitationAdmin(TestCase):
         """Delete all DomainInvitation objects"""
         DomainInvitation.objects.all().delete()
         Contact.objects.all().delete()
-
-    @classmethod
-    def tearDownClass(self):
         User.objects.all().delete()
 
     @less_console_noise_decorator
@@ -168,6 +163,7 @@ class TestDomainInvitationAdmin(TestCase):
         )
         self.assertContains(response, "Show more")
 
+    @less_console_noise_decorator
     def test_get_filters(self):
         """Ensures that our filters are displaying correctly"""
         with less_console_noise():
@@ -192,6 +188,59 @@ class TestDomainInvitationAdmin(TestCase):
             self.assertContains(response, invited_html, count=1)
             self.assertContains(response, retrieved_html, count=1)
 
+    @less_console_noise_decorator
+    def test_save_model_user_exists(self):
+        """Test saving a domain invitation when the user exists.
+
+        Should attempt to retrieve the domain invitation."""
+        # Create a user with the same email
+        User.objects.create_user(email="test@example.com", username="username")
+
+        # Create a domain invitation instance
+        invitation = DomainInvitation(email="test@example.com", domain=self.domain)
+
+        admin_instance = DomainInvitationAdmin(DomainInvitation, admin_site=None)
+
+        # Create a request object
+        request = self.factory.post("/admin/registrar/DomainInvitation/add/")
+        request.user = self.superuser
+
+        # Patch the retrieve method
+        with patch.object(DomainInvitation, "retrieve") as mock_retrieve:
+            admin_instance.save_model(request, invitation, form=None, change=False)
+
+        # Assert retrieve was called
+        mock_retrieve.assert_called_once()
+
+        # Assert the invitation was saved
+        self.assertEqual(DomainInvitation.objects.count(), 1)
+        self.assertEqual(DomainInvitation.objects.first().email, "test@example.com")
+
+    @less_console_noise_decorator
+    def test_save_model_user_does_not_exist(self):
+        """Test saving a domain invitation when the user does not exist.
+
+        Should not attempt to retrieve the domain invitation."""
+        # Create a domain invitation instance
+        invitation = DomainInvitation(email="nonexistent@example.com", domain=self.domain)
+
+        admin_instance = DomainInvitationAdmin(DomainInvitation, admin_site=None)
+
+        # Create a request object
+        request = self.factory.post("/admin/registrar/DomainInvitation/add/")
+        request.user = self.superuser
+
+        # Patch the retrieve method to ensure it is not called
+        with patch.object(DomainInvitation, "retrieve") as mock_retrieve:
+            admin_instance.save_model(request, invitation, form=None, change=False)
+
+        # Assert retrieve was not called
+        mock_retrieve.assert_not_called()
+
+        # Assert the invitation was saved
+        self.assertEqual(DomainInvitation.objects.count(), 1)
+        self.assertEqual(DomainInvitation.objects.first().email, "nonexistent@example.com")
+
 
 class TestUserPortfolioPermissionAdmin(TestCase):
     """Tests for the PortfolioInivtationAdmin class"""

src/registrar/tests/test_admin_request.py

@@ -40,6 +40,7 @@ from .common import (
     multiple_unalphabetical_domain_objects,
     MockEppLib,
     GenericTestHelper,
+    normalize_html,
 )
 from unittest.mock import ANY, patch
 
@@ -1530,8 +1531,9 @@ class TestDomainRequestAdmin(MockEppLib):
         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 @@ 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>'
-        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):

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(
-            has_current_website=True, user=User.objects.create(username="test", email="testy@town.com")
-        )
+        domain_request = completed_domain_request(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(
-            has_alternative_gov_domain=True, user=User.objects.create(username="test", email="testy@town.com")
-        )
+        domain_request = completed_domain_request(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()