Merge branch 'main' into dk/902-epp-dnssec

src/epplibwrapper/errors.py

@@ -67,6 +67,13 @@ class RegistryError(Exception):
     def should_retry(self):
         return self.code == ErrorCode.COMMAND_FAILED
 
+    # connection errors have error code of None and [Errno 99] in the err message
+    def is_connection_error(self):
+        return self.code is None
+
+    def is_session_error(self):
+        return self.code is not None and (self.code >= 2501 and self.code <= 2502)
+
     def is_server_error(self):
         return self.code is not None and (self.code >= 2400 and self.code <= 2500)
 

src/registrar/admin.py

@@ -745,6 +745,22 @@ class DomainAdmin(ListHeaderAdmin):
             obj.place_client_hold()
             obj.save()
         except Exception as err:
+            # if error is an error from the registry, display useful
+            # and readable error
+            if err.code:
+                self.message_user(
+                    request,
+                    f"Error placing the hold with the registry: {err}",
+                    messages.ERROR,
+                )
+            elif err.is_connection_error():
+                self.message_user(
+                    request,
+                    "Error connecting to the registry",
+                    messages.ERROR,
+                )
+            else:
+                # all other type error messages, display the error
                 self.message_user(request, err, messages.ERROR)
         else:
             self.message_user(
@@ -762,6 +778,22 @@ class DomainAdmin(ListHeaderAdmin):
             obj.revert_client_hold()
             obj.save()
         except Exception as err:
+            # if error is an error from the registry, display useful
+            # and readable error
+            if err.code:
+                self.message_user(
+                    request,
+                    f"Error removing the hold in the registry: {err}",
+                    messages.ERROR,
+                )
+            elif err.is_connection_error():
+                self.message_user(
+                    request,
+                    "Error connecting to the registry",
+                    messages.ERROR,
+                )
+            else:
+                # all other type error messages, display the error
                 self.message_user(request, err, messages.ERROR)
         else:
             self.message_user(

src/registrar/models/domain.py

@@ -634,13 +634,25 @@ class Domain(TimeStampedModel, DomainHelper):
         """This domain should not be active.
         may raises RegistryError, should be caught or handled correctly by caller"""
         request = commands.UpdateDomain(name=self.name, add=[self.clientHoldStatus()])
+        try:
             registry.send(request, cleaned=True)
+            self._invalidate_cache()
+        except RegistryError as err:
+            # if registry error occurs, log the error, and raise it as well
+            logger.error(f"registry error placing client hold: {err}")
+            raise (err)
 
     def _remove_client_hold(self):
         """This domain is okay to be active.
         may raises RegistryError, should be caught or handled correctly by caller"""
         request = commands.UpdateDomain(name=self.name, rem=[self.clientHoldStatus()])
+        try:
             registry.send(request, cleaned=True)
+            self._invalidate_cache()
+        except RegistryError as err:
+            # if registry error occurs, log the error, and raise it as well
+            logger.error(f"registry error removing client hold: {err}")
+            raise (err)
 
     def _delete_domain(self):
         """This domain should be deleted from the registry
@@ -773,7 +785,9 @@ class Domain(TimeStampedModel, DomainHelper):
         administrative_contact.domain = self
         administrative_contact.save()
 
-    @transition(field="state", source=State.READY, target=State.ON_HOLD)
+    @transition(
+        field="state", source=[State.READY, State.ON_HOLD], target=State.ON_HOLD
+    )
     def place_client_hold(self):
         """place a clienthold on a domain (no longer should resolve)"""
         # TODO - ensure all requirements for client hold are made here
@@ -782,7 +796,7 @@ class Domain(TimeStampedModel, DomainHelper):
         self._place_client_hold()
         # TODO -on the client hold ticket any additional error handling here
 
-    @transition(field="state", source=State.ON_HOLD, target=State.READY)
+    @transition(field="state", source=[State.READY, State.ON_HOLD], target=State.READY)
     def revert_client_hold(self):
         """undo a clienthold placed on a domain"""
 

src/registrar/templates/emails/domain_invitation.txt

@@ -1,6 +1,32 @@
-You have been invited to manage the domain {{ domain.name }} on get.gov, 
+{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #}
-the registrar for .gov domain names.
+Hi.
 
-To accept your invitation, go to <{{ domain_url }}>.
+{{ full_name }} has added you as a manager on {{ domain.name }}.
 
-You will need to log in with a Login.gov account using this email address.
+YOU NEED A LOGIN.GOV ACCOUNT
+You’ll need a Login.gov account to manage your .gov domain. Login.gov provides
+a simple and secure process for signing into many government services with one
+account. If you don’t already have one, follow these steps to create your
+Login.gov account <https://login.gov/help/get-started/create-your-account/>.
+
+DOMAIN MANAGEMENT
+As a .gov domain manager you can add or update information about your domain.
+You’ll also serve as a contact for your .gov domain. Please keep your contact
+information updated. Learn more about domain management <https://get.gov/help/>.
+
+SOMETHING WRONG?
+If you’re not affiliated with {{ domain.name }} or think you received this
+message in error, contact the .gov team <https://get.gov/help/#contact-us>.
+
+
+THANK YOU
+
+.Gov helps the public identify official, trusted information. Thank you for
+using a .gov domain.
+
+----------------------------------------------------------------
+
+The .gov team
+Contact us: <https://get.gov/contact/>
+Visit <https://get.gov>
+{% endautoescape %}

src/registrar/templates/emails/domain_invitation_subject.txt

@@ -1 +1 @@
-You are invited to manage {{ domain.name }} on get.gov
+You’ve been added to a .gov domain

src/registrar/tests/common.py

@@ -457,6 +457,7 @@ def completed_application(
     has_anything_else=True,
     status=DomainApplication.STARTED,
     user=False,
+    name="city.gov",
 ):
     """A completed domain application."""
     if not user:
@@ -468,7 +469,7 @@ def completed_application(
         email="testy@town.com",
         phone="(555) 555 5555",
     )
-    domain, _ = DraftDomain.objects.get_or_create(name="city.gov")
+    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")
     you, _ = Contact.objects.get_or_create(

src/registrar/tests/test_models_domain.py

@@ -20,6 +20,8 @@ from .common import MockEppLib
 from epplibwrapper import (
     commands,
     common,
+    RegistryError,
+    ErrorCode,
 )
 
 
@@ -702,7 +704,7 @@ class TestRegistrantDNSSEC(TestCase):
         raise
 
 
-class TestAnalystClientHold(TestCase):
+class TestAnalystClientHold(MockEppLib):
     """Rule: Analysts may suspend or restore a domain by using client hold"""
 
     def setUp(self):
@@ -711,18 +713,50 @@ class TestAnalystClientHold(TestCase):
             Given the analyst is logged in
             And a domain exists in the registry
         """
-        pass
+        super().setUp()
+        # for the tests, need a domain in the ready state
+        self.domain, _ = Domain.objects.get_or_create(
+            name="fake.gov", state=Domain.State.READY
+        )
+        # for the tests, need a domain in the on_hold state
+        self.domain_on_hold, _ = Domain.objects.get_or_create(
+            name="fake-on-hold.gov", state=Domain.State.ON_HOLD
+        )
+
+    def tearDown(self):
+        Domain.objects.all().delete()
+        super().tearDown()
 
-    @skip("not implemented yet")
     def test_analyst_places_client_hold(self):
         """
         Scenario: Analyst takes a domain off the internet
             When `domain.place_client_hold()` is called
             Then `CLIENT_HOLD` is added to the domain's statuses
         """
-        raise
+        self.domain.place_client_hold()
+        self.mockedSendFunction.assert_has_calls(
+            [
+                call(
+                    commands.UpdateDomain(
+                        name="fake.gov",
+                        add=[
+                            common.Status(
+                                state=Domain.Status.CLIENT_HOLD,
+                                description="",
+                                lang="en",
+                            )
+                        ],
+                        nsset=None,
+                        keyset=None,
+                        registrant=None,
+                        auth_info=None,
+                    ),
+                    cleaned=True,
+                )
+            ]
+        )
+        self.assertEquals(self.domain.state, Domain.State.ON_HOLD)
 
-    @skip("not implemented yet")
     def test_analyst_places_client_hold_idempotent(self):
         """
         Scenario: Analyst tries to place client hold twice
@@ -730,9 +764,30 @@ class TestAnalystClientHold(TestCase):
             When `domain.place_client_hold()` is called
             Then Domain returns normally (without error)
         """
-        raise
+        self.domain_on_hold.place_client_hold()
+        self.mockedSendFunction.assert_has_calls(
+            [
+                call(
+                    commands.UpdateDomain(
+                        name="fake-on-hold.gov",
+                        add=[
+                            common.Status(
+                                state=Domain.Status.CLIENT_HOLD,
+                                description="",
+                                lang="en",
+                            )
+                        ],
+                        nsset=None,
+                        keyset=None,
+                        registrant=None,
+                        auth_info=None,
+                    ),
+                    cleaned=True,
+                )
+            ]
+        )
+        self.assertEquals(self.domain_on_hold.state, Domain.State.ON_HOLD)
 
-    @skip("not implemented yet")
     def test_analyst_removes_client_hold(self):
         """
         Scenario: Analyst restores a suspended domain
@@ -740,9 +795,30 @@ class TestAnalystClientHold(TestCase):
             When `domain.remove_client_hold()` is called
             Then `CLIENT_HOLD` is no longer in the domain's statuses
         """
-        raise
+        self.domain_on_hold.revert_client_hold()
+        self.mockedSendFunction.assert_has_calls(
+            [
+                call(
+                    commands.UpdateDomain(
+                        name="fake-on-hold.gov",
+                        rem=[
+                            common.Status(
+                                state=Domain.Status.CLIENT_HOLD,
+                                description="",
+                                lang="en",
+                            )
+                        ],
+                        nsset=None,
+                        keyset=None,
+                        registrant=None,
+                        auth_info=None,
+                    ),
+                    cleaned=True,
+                )
+            ]
+        )
+        self.assertEquals(self.domain_on_hold.state, Domain.State.READY)
 
-    @skip("not implemented yet")
     def test_analyst_removes_client_hold_idempotent(self):
         """
         Scenario: Analyst tries to remove client hold twice
@@ -750,16 +826,54 @@ class TestAnalystClientHold(TestCase):
             When `domain.remove_client_hold()` is called
             Then Domain returns normally (without error)
         """
-        raise
+        self.domain.revert_client_hold()
+        self.mockedSendFunction.assert_has_calls(
+            [
+                call(
+                    commands.UpdateDomain(
+                        name="fake.gov",
+                        rem=[
+                            common.Status(
+                                state=Domain.Status.CLIENT_HOLD,
+                                description="",
+                                lang="en",
+                            )
+                        ],
+                        nsset=None,
+                        keyset=None,
+                        registrant=None,
+                        auth_info=None,
+                    ),
+                    cleaned=True,
+                )
+            ]
+        )
+        self.assertEquals(self.domain.state, Domain.State.READY)
 
-    @skip("not implemented yet")
     def test_update_is_unsuccessful(self):
         """
         Scenario: An update to place or remove client hold is unsuccessful
             When an error is returned from epplibwrapper
             Then a user-friendly error message is returned for displaying on the web
         """
-        raise
+
+        def side_effect(_request, cleaned):
+            raise RegistryError(code=ErrorCode.OBJECT_STATUS_PROHIBITS_OPERATION)
+
+        patcher = patch("registrar.models.domain.registry.send")
+        mocked_send = patcher.start()
+        mocked_send.side_effect = side_effect
+
+        # if RegistryError is raised, admin formats user-friendly
+        # error message if error is_client_error, is_session_error, or
+        # is_server_error; so test for those conditions
+        with self.assertRaises(RegistryError) as err:
+            self.domain.place_client_hold()
+            self.assertTrue(
+                err.is_client_error() or err.is_session_error() or err.is_server_error()
+            )
+
+        patcher.stop()
 
 
 class TestAnalystLock(TestCase):

src/registrar/tests/test_views.py

@@ -1079,6 +1079,7 @@ class TestWithDomainPermissions(TestWithUser):
             self.domain_information.delete()
             if hasattr(self.domain, "contacts"):
                 self.domain.contacts.all().delete()
+            DomainApplication.objects.all().delete()
             self.domain.delete()
             self.role.delete()
         except ValueError:  # pass if already deleted
@@ -1197,6 +1198,10 @@ class TestDomainDetail(TestWithDomainPermissions, WebTest):
         EMAIL = "mayor@igorville.gov"
         User.objects.filter(email=EMAIL).delete()
 
+        self.domain_information, _ = DomainInformation.objects.get_or_create(
+            creator=self.user, domain=self.domain
+        )
+
         add_page = self.app.get(
             reverse("domain-users-add", kwargs={"pk": self.domain.id})
         )
@@ -1218,6 +1223,10 @@ class TestDomainDetail(TestWithDomainPermissions, WebTest):
         EMAIL = "mayor@igorville.gov"
         User.objects.filter(email=EMAIL).delete()
 
+        self.domain_information, _ = DomainInformation.objects.get_or_create(
+            creator=self.user, domain=self.domain
+        )
+
         mock_client = MagicMock()
         mock_client_instance = mock_client.return_value
         with boto3_mocking.clients.handler_for("sesv2", mock_client):
@@ -1270,6 +1279,11 @@ class TestDomainDetail(TestWithDomainPermissions, WebTest):
         add_page = self.app.get(
             reverse("domain-users-add", kwargs={"pk": self.domain.id})
         )
+
+        self.domain_information, _ = DomainInformation.objects.get_or_create(
+            creator=self.user, domain=self.domain
+        )
+
         session_id = self.app.cookies[settings.SESSION_COOKIE_NAME]
         add_page.form["email"] = EMAIL
         self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)

src/registrar/views/domain.py

@@ -16,6 +16,7 @@ from django.views.generic.edit import FormMixin
 
 from registrar.models import (
     Domain,
+    DomainInformation,
     DomainInvitation,
     User,
     UserDomainRole,
@@ -335,6 +336,11 @@ class DomainAddUserView(DomainPermissionView, FormMixin):
             )
         else:
             # created a new invitation in the database, so send an email
+            domaininfo = DomainInformation.objects.filter(domain=self.object)
+            first = domaininfo.first().creator.first_name
+            last = domaininfo.first().creator.last_name
+            full_name = f"{first} {last}"
+
             try:
                 send_templated_email(
                     "emails/domain_invitation.txt",
@@ -343,6 +349,7 @@ class DomainAddUserView(DomainPermissionView, FormMixin):
                     context={
                         "domain_url": self._domain_abs_url(),
                         "domain": self.object,
+                        "full_name": full_name,
                     },
                 )
             except EmailSendingError: