From 60834894882dd47271a6f947d2de79cc66e9bd69 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 16 Feb 2024 10:27:54 -0700 Subject: [PATCH 01/23] Add autocompelte fields --- src/registrar/admin.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index f1680c76a..67f93a376 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -758,6 +758,14 @@ class DomainInformationAdmin(ListHeaderAdmin): # to activate the edit/delete/view buttons filter_horizontal = ("other_contacts",) + autocomplete_fields = [ + "creator", + "domain_application", + "authorizing_official", + "domain", + "submitter", + ] + # Table ordering ordering = ["domain__name"] @@ -1129,6 +1137,13 @@ class DomainAdmin(ListHeaderAdmin): ), ) + autocomplete_fields = [ + "creator", + "domain_application", + "authorizing_official", + "submitter", + ] + # this ordering effects the ordering of results # in autocomplete_fields for domain ordering = ["name"] From d72a3fc7d035915a617c247106e73566c578ce23 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 16 Feb 2024 11:36:05 -0700 Subject: [PATCH 02/23] Update admin.py --- src/registrar/admin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 67f93a376..abb9832bc 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1138,10 +1138,10 @@ class DomainAdmin(ListHeaderAdmin): ) autocomplete_fields = [ - "creator", + "domain_information__creator", "domain_application", "authorizing_official", - "submitter", + "domain_information__submitter", ] # this ordering effects the ordering of results From 7b227a26bb8dc0a4297cd63c1bb7c19ea34e569b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 16 Feb 2024 11:48:40 -0700 Subject: [PATCH 03/23] Update admin.py --- src/registrar/admin.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index abb9832bc..9da8fc855 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1137,13 +1137,6 @@ class DomainAdmin(ListHeaderAdmin): ), ) - autocomplete_fields = [ - "domain_information__creator", - "domain_application", - "authorizing_official", - "domain_information__submitter", - ] - # this ordering effects the ordering of results # in autocomplete_fields for domain ordering = ["name"] From 2aeb34c9eb4f8dae0f4c6831e2572e04f10e7749 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 16 Feb 2024 11:58:22 -0700 Subject: [PATCH 04/23] Fix bug with autocomplete on domains --- src/registrar/admin.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 9da8fc855..36d6b4fd9 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1088,6 +1088,14 @@ class DomainInformationInline(admin.StackedInline): # to activate the edit/delete/view buttons filter_horizontal = ("other_contacts",) + autocomplete_fields = [ + "creator", + "domain_application", + "authorizing_official", + "domain", + "submitter", + ] + def formfield_for_manytomany(self, db_field, request, **kwargs): """customize the behavior of formfields with manytomany relationships. the customized behavior includes sorting of objects in lists as well as customizing helper text""" From 448fc6ed16ad7d4f45557db0a78afebeb4cfff5c Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Fri, 16 Feb 2024 12:01:44 -0800 Subject: [PATCH 05/23] Update logic for making sure we clear out ip address field --- src/registrar/models/domain.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 7ebe3dc34..dca0693c2 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -427,7 +427,6 @@ class Domain(TimeStampedModel, DomainHelper): raise NameserverError(code=nsErrorCodes.INVALID_HOST, nameserver=nameserver) elif cls.isSubdomain(name, nameserver) and (ip is None or ip == []): raise NameserverError(code=nsErrorCodes.MISSING_IP, nameserver=nameserver) - elif not cls.isSubdomain(name, nameserver) and (ip is not None and ip != []): raise NameserverError(code=nsErrorCodes.GLUE_RECORD_NOT_ALLOWED, nameserver=nameserver, ip=ip) elif ip is not None and ip != []: @@ -1778,6 +1777,10 @@ class Domain(TimeStampedModel, DomainHelper): for cleaned_host in cleaned_hosts: # Check if the cleaned_host already exists host_in_db, host_created = Host.objects.get_or_create(domain=self, name=cleaned_host["name"]) + # Check if the nameserver is a subdomain of the current domain + # If it is NOT a subdomain, we remove the IP address + if not Domain.isSubdomain(self.name, cleaned_host["name"]): + cleaned_host["addrs"] = [] # or None # Get cleaned list of ips for update cleaned_ips = cleaned_host["addrs"] if not host_created: From 7e1c1f419ed01eee827944035cd1eaa04883ecc5 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Fri, 16 Feb 2024 16:18:17 -0800 Subject: [PATCH 06/23] Fix tests --- src/registrar/tests/common.py | 14 ++++++ src/registrar/tests/test_models_domain.py | 57 ++++++++++++++++++++++- 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 2865bf5c5..336740c02 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -691,6 +691,19 @@ class MockEppLib(TestCase): ], ex_date=datetime.date(2023, 5, 25), ) + + mockDataInfoDomainSubdomain = fakedEppObject( + "fakePw", + cr_date=make_aware(datetime.datetime(2023, 5, 25, 19, 45, 35)), + contacts=[common.DomainContact(contact="123", type=PublicContact.ContactTypeChoices.SECURITY)], + hosts=["fake.meoward.gov"], + statuses=[ + common.Status(state="serverTransferProhibited", description="", lang="en"), + common.Status(state="inactive", description="", lang="en"), + ], + ex_date=datetime.date(2023, 5, 25), + ) + mockDataExtensionDomain = fakedEppObject( "fakePw", cr_date=make_aware(datetime.datetime(2023, 5, 25, 19, 45, 35)), @@ -1083,6 +1096,7 @@ class MockEppLib(TestCase): "adomain2.gov": (self.InfoDomainWithVerisignSecurityContact, None), "defaulttechnical.gov": (self.InfoDomainWithDefaultTechnicalContact, None), "justnameserver.com": (self.justNameserver, None), + "meoward.gov": (self.mockDataInfoDomainSubdomain, None), } # Retrieve the corresponding values from the dictionary diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 1c4d2521e..f4dea616b 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -96,7 +96,7 @@ class TestDomainCache(MockEppLib): self.mockedSendFunction.assert_has_calls(expectedCalls) - def test_cache_nested_elements(self): + def test_cache_nested_elements_not_subdomain(self): """Cache works correctly with the nested objects cache and hosts""" with less_console_noise(): domain, _ = Domain.objects.get_or_create(name="igorville.gov") @@ -113,7 +113,7 @@ class TestDomainCache(MockEppLib): } expectedHostsDict = { "name": self.mockDataInfoDomain.hosts[0], - "addrs": [item.addr for item in self.mockDataInfoHosts.addrs], + "addrs": [], # hould return empty bc fake.host.com is not a subdomain of igorville.gov "cr_date": self.mockDataInfoHosts.cr_date, } @@ -138,6 +138,59 @@ class TestDomainCache(MockEppLib): # invalidate cache domain._cache = {} + # get host + domain._get_property("hosts") + # Should return empty bc fake.host.com is not a subdomain of igorville.gov + self.assertEqual(domain._cache["hosts"], [expectedHostsDict]) + + # get contacts + domain._get_property("contacts") + self.assertEqual(domain._cache["hosts"], [expectedHostsDict]) + self.assertEqual(domain._cache["contacts"], expectedContactsDict) + + def test_cache_nested_elements_is_subdomain(self): + """Cache works correctly with the nested objects cache and hosts""" + with less_console_noise(): + domain, _ = Domain.objects.get_or_create(name="meoward.gov") + + # The contact list will initially contain objects of type 'DomainContact' + # this is then transformed into PublicContact, and cache should NOT + # hold onto the DomainContact object + expectedUnfurledContactsList = [ + common.DomainContact(contact="123", type="security"), + ] + expectedContactsDict = { + PublicContact.ContactTypeChoices.ADMINISTRATIVE: None, + PublicContact.ContactTypeChoices.SECURITY: "123", + PublicContact.ContactTypeChoices.TECHNICAL: None, + } + expectedHostsDict = { + "name": self.mockDataInfoDomainSubdomain.hosts[0], + "addrs": [item.addr for item in self.mockDataInfoHosts.addrs], + "cr_date": self.mockDataInfoHosts.cr_date, + } + + # this can be changed when the getter for contacts is implemented + domain._get_property("contacts") + + # check domain info is still correct and not overridden + self.assertEqual(domain._cache["auth_info"], self.mockDataInfoDomainSubdomain.auth_info) + self.assertEqual(domain._cache["cr_date"], self.mockDataInfoDomainSubdomain.cr_date) + + # check contacts + self.assertEqual(domain._cache["_contacts"], self.mockDataInfoDomainSubdomain.contacts) + # The contact list should not contain what is sent by the registry by default, + # as _fetch_cache will transform the type to PublicContact + self.assertNotEqual(domain._cache["contacts"], expectedUnfurledContactsList) + self.assertEqual(domain._cache["contacts"], expectedContactsDict) + + # get and check hosts is set correctly + domain._get_property("hosts") + self.assertEqual(domain._cache["hosts"], [expectedHostsDict]) + self.assertEqual(domain._cache["contacts"], expectedContactsDict) + # invalidate cache + domain._cache = {} + # get host domain._get_property("hosts") self.assertEqual(domain._cache["hosts"], [expectedHostsDict]) From e27acb2e9303fbd44c246ede37bd25dca6a72b42 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Tue, 20 Feb 2024 16:10:54 -0700 Subject: [PATCH 07/23] Made "required" error message match "invalid" error message for zip code on Organization name and mailing address --- src/registrar/forms/application_wizard.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 1ee7e0036..a76626b1e 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -284,6 +284,9 @@ class OrganizationContactForm(RegistrarForm): message="Enter a zip code in the form of 12345 or 12345-6789.", ) ], + error_messages={ + "required": ("Enter a zip code in the form of 12345 or 12345-6789.") + }, ) urbanization = forms.CharField( required=False, From 3da387ab1c17ba52fb7ae7ec124710360a673165 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Tue, 20 Feb 2024 16:17:48 -0700 Subject: [PATCH 08/23] linted --- src/registrar/forms/application_wizard.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index a76626b1e..df5b195c6 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -284,9 +284,7 @@ class OrganizationContactForm(RegistrarForm): message="Enter a zip code in the form of 12345 or 12345-6789.", ) ], - error_messages={ - "required": ("Enter a zip code in the form of 12345 or 12345-6789.") - }, + error_messages={"required": ("Enter a zip code in the form of 12345 or 12345-6789.")}, ) urbanization = forms.CharField( required=False, From a14a32159c79ce9bb72a121e6646f7d5c633c32e Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 20 Feb 2024 18:04:13 -0800 Subject: [PATCH 09/23] Add more tests in for caching, still WIP --- src/registrar/models/domain.py | 2 + src/registrar/tests/common.py | 79 +++++++++++++++++ src/registrar/tests/test_models_domain.py | 100 ++++++++++++++++++++-- 3 files changed, 173 insertions(+), 8 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index dca0693c2..f86990a5e 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1679,6 +1679,8 @@ class Domain(TimeStampedModel, DomainHelper): try: data_response = self._get_or_create_domain() cache = self._extract_data_from_response(data_response) + # print("!!! cache is") + # print(cache) cleaned = self._clean_cache(cache, data_response) self._update_hosts_and_contacts(cleaned, fetch_hosts, fetch_contacts) if fetch_hosts: diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 336740c02..2419c9687 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -704,6 +704,43 @@ class MockEppLib(TestCase): ex_date=datetime.date(2023, 5, 25), ) + mockDataInfoDomainSubdomainAndIPAddress = fakedEppObject( + "fakePw", + cr_date=make_aware(datetime.datetime(2023, 5, 25, 19, 45, 35)), + contacts=[common.DomainContact(contact="123", type=PublicContact.ContactTypeChoices.SECURITY)], + hosts=["fake.meow.gov"], + statuses=[ + common.Status(state="serverTransferProhibited", description="", lang="en"), + common.Status(state="inactive", description="", lang="en"), + ], + ex_date=datetime.date(2023, 5, 25), + addrs=[common.Ip(addr="2.0.0.8")], + ) + + mockDataInfoDomainNotSubdomainNoIP = fakedEppObject( + "fakePw", + cr_date=make_aware(datetime.datetime(2023, 5, 25, 19, 45, 35)), + contacts=[common.DomainContact(contact="123", type=PublicContact.ContactTypeChoices.SECURITY)], + hosts=["fake.meow.com"], + statuses=[ + common.Status(state="serverTransferProhibited", description="", lang="en"), + common.Status(state="inactive", description="", lang="en"), + ], + ex_date=datetime.date(2023, 5, 25), + ) + + mockDataInfoDomainSubdomainNoIP = fakedEppObject( + "fakePw", + cr_date=make_aware(datetime.datetime(2023, 5, 25, 19, 45, 35)), + contacts=[common.DomainContact(contact="123", type=PublicContact.ContactTypeChoices.SECURITY)], + hosts=["fake.subdomainwoip.gov"], + statuses=[ + common.Status(state="serverTransferProhibited", description="", lang="en"), + common.Status(state="inactive", description="", lang="en"), + ], + ex_date=datetime.date(2023, 5, 25), + ) + mockDataExtensionDomain = fakedEppObject( "fakePw", cr_date=make_aware(datetime.datetime(2023, 5, 25, 19, 45, 35)), @@ -841,6 +878,24 @@ class MockEppLib(TestCase): addrs=[common.Ip(addr="1.2.3.4"), common.Ip(addr="2.3.4.5")], ) + mockDataInfoHosts1IP = fakedEppObject( + "lastPw", + cr_date=make_aware(datetime.datetime(2023, 8, 25, 19, 45, 35)), + addrs=[common.Ip(addr="2.0.0.8")], + ) + + mockDataInfoHostsNotSubdomainNoIP = fakedEppObject( + "lastPw", + cr_date=make_aware(datetime.datetime(2023, 8, 26, 19, 45, 35)), + addrs=[], + ) + + mockDataInfoHostsSubdomainNoIP = fakedEppObject( + "lastPw", + cr_date=make_aware(datetime.datetime(2023, 8, 27, 19, 45, 35)), + addrs=[], + ) + mockDataHostChange = fakedEppObject("lastPw", cr_date=make_aware(datetime.datetime(2023, 8, 25, 19, 45, 35))) addDsData1 = { "keyTag": 1234, @@ -1002,6 +1057,8 @@ class MockEppLib(TestCase): return self.mockDeleteDomainCommands(_request, cleaned) case commands.RenewDomain: return self.mockRenewDomainCommand(_request, cleaned) + case commands.InfoHost: + return self.mockInfoHostCommmands(_request, cleaned) case _: return MagicMock(res_data=[self.mockDataInfoHosts]) @@ -1016,6 +1073,25 @@ class MockEppLib(TestCase): code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, ) + # default - mock send + def mockInfoHostCommmands(self, _request, cleaned): + request_name = getattr(_request, "name", None) + + # Define a dictionary to map request names to data and extension values + request_mappings = { + "fake.meow.gov": (self.mockDataInfoHosts1IP, None), # is subdomain and has ip + "fake.meow.com": (self.mockDataInfoHostsNotSubdomainNoIP, None), # not subdomain w no ip + "fake.subdomainwoip.gov": (self.mockDataInfoHostsSubdomainNoIP, None), # subdomain w no ip + } + + # Retrieve the corresponding values from the dictionary + res_data, extensions = request_mappings.get(request_name, (self.mockDataInfoHosts, None)) # default + + return MagicMock( + res_data=[res_data], + extensions=[extensions] if extensions is not None else [], + ) + def mockUpdateHostCommands(self, _request, cleaned): test_ws_ip = common.Ip(addr="1.1. 1.1") addrs_submitted = getattr(_request, "addrs", []) @@ -1097,6 +1173,9 @@ class MockEppLib(TestCase): "defaulttechnical.gov": (self.InfoDomainWithDefaultTechnicalContact, None), "justnameserver.com": (self.justNameserver, None), "meoward.gov": (self.mockDataInfoDomainSubdomain, None), + "meow.gov": (self.mockDataInfoDomainSubdomainAndIPAddress, None), + "fakemeow.gov": (self.mockDataInfoDomainNotSubdomainNoIP, None), + "subdomainwoip.gov": (self.mockDataInfoDomainSubdomainNoIP, None), } # Retrieve the corresponding values from the dictionary diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index f4dea616b..adde4b410 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -1625,7 +1625,65 @@ class TestRegistrantNameservers(MockEppLib): self.assertEqual(nameservers[0][1], ["1.1.1.1"]) patcher.stop() - def test_nameservers_stored_on_fetch_cache(self): + # 1 - is it a subdomain and it has an ip address -- COVERED? + # 2 - is it a subdomain and it doesn't have an ip address + # 3 - no subdomain, it has an ip address -- COVERED + # 4 - no subomdina, doens't have ip address + + def test_nameservers_stored_on_fetch_cache_a_subdomain_with_ip(self): + """ + #1: It is a subdomain, and has an IP address -- referenced by mockDataInfoDomainSubdomainAndIPAddress + fake.meow.com is not a subdomain of fake.gov + """ + with less_console_noise(): + # make the domain + domain, _ = Domain.objects.get_or_create(name="meow.gov", state=Domain.State.READY) + + # mock the get_or_create methods for Host and HostIP + with patch.object(Host.objects, "get_or_create") as mock_host_get_or_create, patch.object( + HostIP.objects, "get_or_create" + ) as mock_host_ip_get_or_create: + mock_host_get_or_create.return_value = (Host(domain=domain), True) + mock_host_ip_get_or_create.return_value = (HostIP(), True) + + # force fetch_cache to be called, which will return above documented mocked hosts + domain.nameservers + + # This is never called? + mock_host_get_or_create.assert_called_once_with(domain=domain, name="fake.meow.gov") + # Retrieve the mocked_host from the return value of the mock + actual_mocked_host, _ = mock_host_get_or_create.return_value + mock_host_ip_get_or_create.assert_called_with(address="2.0.0.8", host=actual_mocked_host) + self.assertEqual(mock_host_ip_get_or_create.call_count, 1) + + def test_nameservers_stored_on_fetch_cache_a_subdomain_without_ip(self): + """ + #2: It is a subdomain, but doesn't has an IP address + """ + with less_console_noise(): + # make the domain + domain, _ = Domain.objects.get_or_create(name="subdomainwoip.gov", state=Domain.State.READY) + + # mock the get_or_create methods for Host and HostIP + # below should do it for mock_host_get_or_create and mock_host_ip_get_or_create right? + with patch.object(Host.objects, "get_or_create") as mock_host_get_or_create, patch.object( + HostIP.objects, "get_or_create" + ) as mock_host_ip_get_or_create: + mock_host_get_or_create.return_value = (Host(domain=domain), True) + mock_host_ip_get_or_create.return_value = (HostIP(), True) + + # force fetch_cache to be called, which will return above documented mocked hosts + domain.nameservers + + # This is never called? + mock_host_get_or_create.assert_called_once_with(domain=domain, name="fake.subdomainwoip.gov") + # Retrieve the mocked_host from the return value of the mock + actual_mocked_host, _ = mock_host_get_or_create.return_value + mock_host_ip_get_or_create.assert_called_with(address="", host=actual_mocked_host) + + self.assertEqual(mock_host_ip_get_or_create.call_count, 1) + + def test_nameservers_stored_on_fetch_cache_not_subdomain_with_ip(self): """ Scenario: Nameservers are stored in db when they are retrieved from fetch_cache. Verify the success of this by asserting get_or_create calls to db. @@ -1633,23 +1691,49 @@ class TestRegistrantNameservers(MockEppLib): of 'fake.host.com' from InfoDomain and an array of 2 IPs: 1.2.3.4 and 2.3.4.5 from InfoHost """ + + """ + #3: Not a subdomain, but it has an IP address returned due to how we return our defaults + fake.host.com is not a subdomain of fake.gov + """ with less_console_noise(): domain, _ = Domain.objects.get_or_create(name="fake.gov", state=Domain.State.READY) - # mock the get_or_create methods for Host and HostIP + with patch.object(Host.objects, "get_or_create") as mock_host_get_or_create, patch.object( HostIP.objects, "get_or_create" ) as mock_host_ip_get_or_create: - # Set the return value for the mocks - mock_host_get_or_create.return_value = (Host(), True) + mock_host_get_or_create.return_value = (Host(domain=domain), True) mock_host_ip_get_or_create.return_value = (HostIP(), True) + # force fetch_cache to be called, which will return above documented mocked hosts domain.nameservers - # assert that the mocks are called + # # assert that the mocks are called + mock_host_get_or_create.assert_called_once_with(domain=domain, name="fake.host.com") # Retrieve the mocked_host from the return value of the mock - actual_mocked_host, _ = mock_host_get_or_create.return_value - mock_host_ip_get_or_create.assert_called_with(address="2.3.4.5", host=actual_mocked_host) - self.assertEqual(mock_host_ip_get_or_create.call_count, 2) + mock_host_ip_get_or_create.assert_not_called() + self.assertEqual(mock_host_ip_get_or_create.call_count, 0) + + def test_nameservers_stored_on_fetch_cache_not_subdomain_without_ip(self): + """ + #4: Not a subdomain and doesn't has an IP address (not pointing to default) + referenced by self.mockDataInfoDomainNotSubdomainNoIP + """ + with less_console_noise(): + domain, _ = Domain.objects.get_or_create(name="fakemeow.gov", state=Domain.State.READY) + + with patch.object(Host.objects, "get_or_create") as mock_host_get_or_create, patch.object( + HostIP.objects, "get_or_create" + ) as mock_host_ip_get_or_create: + mock_host_get_or_create.return_value = (Host(domain=domain), True) + mock_host_ip_get_or_create.return_value = (HostIP(), True) + + # force fetch_cache to be called, which will return above documented mocked hosts + domain.nameservers + # # assert that the mocks are called + mock_host_get_or_create.assert_called_once_with(domain=domain, name="fake.meow.com") + mock_host_ip_get_or_create.assert_not_called() + self.assertEqual(mock_host_ip_get_or_create.call_count, 0) @skip("not implemented yet") def test_update_is_unsuccessful(self): From 51411f4e3ee2ad34c7bdb29b949729438bc88d7c Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 21 Feb 2024 08:57:26 -0800 Subject: [PATCH 10/23] Fix tests for diff scenarios --- src/registrar/models/domain.py | 2 -- src/registrar/tests/test_models_domain.py | 33 +++++++---------------- 2 files changed, 9 insertions(+), 26 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index f86990a5e..dca0693c2 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1679,8 +1679,6 @@ class Domain(TimeStampedModel, DomainHelper): try: data_response = self._get_or_create_domain() cache = self._extract_data_from_response(data_response) - # print("!!! cache is") - # print(cache) cleaned = self._clean_cache(cache, data_response) self._update_hosts_and_contacts(cleaned, fetch_hosts, fetch_contacts) if fetch_hosts: diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index adde4b410..f4047556e 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -1625,15 +1625,10 @@ class TestRegistrantNameservers(MockEppLib): self.assertEqual(nameservers[0][1], ["1.1.1.1"]) patcher.stop() - # 1 - is it a subdomain and it has an ip address -- COVERED? - # 2 - is it a subdomain and it doesn't have an ip address - # 3 - no subdomain, it has an ip address -- COVERED - # 4 - no subomdina, doens't have ip address - def test_nameservers_stored_on_fetch_cache_a_subdomain_with_ip(self): """ - #1: It is a subdomain, and has an IP address -- referenced by mockDataInfoDomainSubdomainAndIPAddress - fake.meow.com is not a subdomain of fake.gov + #1: Nameserver is a subdomain, and has an IP address + referenced by mockDataInfoDomainSubdomainAndIPAddress """ with less_console_noise(): # make the domain @@ -1649,7 +1644,6 @@ class TestRegistrantNameservers(MockEppLib): # force fetch_cache to be called, which will return above documented mocked hosts domain.nameservers - # This is never called? mock_host_get_or_create.assert_called_once_with(domain=domain, name="fake.meow.gov") # Retrieve the mocked_host from the return value of the mock actual_mocked_host, _ = mock_host_get_or_create.return_value @@ -1658,14 +1652,14 @@ class TestRegistrantNameservers(MockEppLib): def test_nameservers_stored_on_fetch_cache_a_subdomain_without_ip(self): """ - #2: It is a subdomain, but doesn't has an IP address + #2: Nameserver is a subdomain, but doesn't have an IP address associated + referenced by mockDataInfoDomainSubdomainNoIP """ with less_console_noise(): # make the domain domain, _ = Domain.objects.get_or_create(name="subdomainwoip.gov", state=Domain.State.READY) # mock the get_or_create methods for Host and HostIP - # below should do it for mock_host_get_or_create and mock_host_ip_get_or_create right? with patch.object(Host.objects, "get_or_create") as mock_host_get_or_create, patch.object( HostIP.objects, "get_or_create" ) as mock_host_ip_get_or_create: @@ -1675,13 +1669,9 @@ class TestRegistrantNameservers(MockEppLib): # force fetch_cache to be called, which will return above documented mocked hosts domain.nameservers - # This is never called? mock_host_get_or_create.assert_called_once_with(domain=domain, name="fake.subdomainwoip.gov") - # Retrieve the mocked_host from the return value of the mock - actual_mocked_host, _ = mock_host_get_or_create.return_value - mock_host_ip_get_or_create.assert_called_with(address="", host=actual_mocked_host) - - self.assertEqual(mock_host_ip_get_or_create.call_count, 1) + mock_host_ip_get_or_create.assert_not_called() + self.assertEqual(mock_host_ip_get_or_create.call_count, 0) def test_nameservers_stored_on_fetch_cache_not_subdomain_with_ip(self): """ @@ -1690,11 +1680,9 @@ class TestRegistrantNameservers(MockEppLib): The mocked data for the EPP calls returns a host name of 'fake.host.com' from InfoDomain and an array of 2 IPs: 1.2.3.4 and 2.3.4.5 from InfoHost - """ - """ - #3: Not a subdomain, but it has an IP address returned due to how we return our defaults - fake.host.com is not a subdomain of fake.gov + #3: Nameserver is not a subdomain, but it does have an IP address returned + due to how we set up our defaults """ with less_console_noise(): domain, _ = Domain.objects.get_or_create(name="fake.gov", state=Domain.State.READY) @@ -1707,16 +1695,14 @@ class TestRegistrantNameservers(MockEppLib): # force fetch_cache to be called, which will return above documented mocked hosts domain.nameservers - # # assert that the mocks are called mock_host_get_or_create.assert_called_once_with(domain=domain, name="fake.host.com") - # Retrieve the mocked_host from the return value of the mock mock_host_ip_get_or_create.assert_not_called() self.assertEqual(mock_host_ip_get_or_create.call_count, 0) def test_nameservers_stored_on_fetch_cache_not_subdomain_without_ip(self): """ - #4: Not a subdomain and doesn't has an IP address (not pointing to default) + #4: Nameserver is not a subdomain and doesn't have an associated IP address referenced by self.mockDataInfoDomainNotSubdomainNoIP """ with less_console_noise(): @@ -1730,7 +1716,6 @@ class TestRegistrantNameservers(MockEppLib): # force fetch_cache to be called, which will return above documented mocked hosts domain.nameservers - # # assert that the mocks are called mock_host_get_or_create.assert_called_once_with(domain=domain, name="fake.meow.com") mock_host_ip_get_or_create.assert_not_called() self.assertEqual(mock_host_ip_get_or_create.call_count, 0) From 56c253f4788352ba1aaed0ae63d4cb603fa5370b Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 21 Feb 2024 09:56:13 -0800 Subject: [PATCH 11/23] Update test for domain views --- src/registrar/tests/test_views_domain.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 2c8e796ac..bd0fe11a9 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -821,6 +821,7 @@ class TestDomainNameservers(TestDomainOverview): nameserver1 = "ns1.igorville.gov" nameserver2 = "ns2.igorville.gov" valid_ip = "1.1. 1.1" + valid_ip_2 = "2.2. 2.2" # initial nameservers page has one server with two ips # have to throw an error in order to test that the whitespace has been stripped from ip nameservers_page = self.app.get(reverse("domain-dns-nameservers", kwargs={"pk": self.domain.id})) @@ -828,7 +829,8 @@ class TestDomainNameservers(TestDomainOverview): self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) # attempt to submit the form without one host and an ip with whitespace nameservers_page.form["form-0-server"] = nameserver1 - nameservers_page.form["form-1-ip"] = valid_ip + nameservers_page.form["form-0-ip"] = valid_ip + nameservers_page.form["form-1-ip"] = valid_ip_2 nameservers_page.form["form-1-server"] = nameserver2 with less_console_noise(): # swallow log warning message result = nameservers_page.form.submit() @@ -937,15 +939,15 @@ class TestDomainNameservers(TestDomainOverview): nameserver1 = "ns1.igorville.gov" nameserver2 = "ns2.igorville.gov" valid_ip = "127.0.0.1" + valid_ip_2 = "128.0.0.2" # initial nameservers page has one server with two ips nameservers_page = self.app.get(reverse("domain-dns-nameservers", kwargs={"pk": self.domain.id})) session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - # attempt to submit the form without two hosts, both subdomains, - # only one has ips nameservers_page.form["form-0-server"] = nameserver1 + nameservers_page.form["form-0-ip"] = valid_ip nameservers_page.form["form-1-server"] = nameserver2 - nameservers_page.form["form-1-ip"] = valid_ip + nameservers_page.form["form-1-ip"] = valid_ip_2 with less_console_noise(): # swallow log warning message result = nameservers_page.form.submit() # form submission was a successful post, response should be a 302 From c914c36096ca7558e0b756e6a7d825749d4eb791 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 21 Feb 2024 13:01:36 -0800 Subject: [PATCH 12/23] Testing if sandbox is updating --- src/registrar/templates/home.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index a79065f50..918a0defd 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -13,7 +13,7 @@ {% block messages %} {% include "includes/form_messages.html" %} {% endblock %} -

Manage your domains

+

Manage your domains -- TEST TEST 123

Date: Wed, 21 Feb 2024 13:55:52 -0800 Subject: [PATCH 13/23] Remove extraneous code for deployment testing --- src/registrar/templates/home.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index 918a0defd..a79065f50 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -13,7 +13,7 @@ {% block messages %} {% include "includes/form_messages.html" %} {% endblock %} -

Manage your domains -- TEST TEST 123

+

Manage your domains

Date: Wed, 21 Feb 2024 14:45:26 -0800 Subject: [PATCH 14/23] Adjust comments --- src/registrar/tests/test_views_domain.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index bd0fe11a9..59b5faaa9 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -822,7 +822,6 @@ class TestDomainNameservers(TestDomainOverview): nameserver2 = "ns2.igorville.gov" valid_ip = "1.1. 1.1" valid_ip_2 = "2.2. 2.2" - # initial nameservers page has one server with two ips # have to throw an error in order to test that the whitespace has been stripped from ip nameservers_page = self.app.get(reverse("domain-dns-nameservers", kwargs={"pk": self.domain.id})) session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] @@ -940,7 +939,6 @@ class TestDomainNameservers(TestDomainOverview): nameserver2 = "ns2.igorville.gov" valid_ip = "127.0.0.1" valid_ip_2 = "128.0.0.2" - # initial nameservers page has one server with two ips nameservers_page = self.app.get(reverse("domain-dns-nameservers", kwargs={"pk": self.domain.id})) session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) From 1447156c0c03619c6b814d0c737e138fb947df98 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 21 Feb 2024 15:19:11 -0800 Subject: [PATCH 15/23] Just going to use this as a correct deployment holder --- src/registrar/templates/home.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index a79065f50..945f939a4 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -13,7 +13,7 @@ {% block messages %} {% include "includes/form_messages.html" %} {% endblock %} -

Manage your domains

+

Manage your domains - TESTING TESTING 123

Date: Wed, 21 Feb 2024 15:50:55 -0800 Subject: [PATCH 16/23] Remove extra comment --- src/registrar/tests/common.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 2419c9687..9ce65626a 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -1073,7 +1073,6 @@ class MockEppLib(TestCase): code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, ) - # default - mock send def mockInfoHostCommmands(self, _request, cleaned): request_name = getattr(_request, "name", None) From 65813f448e5eeb923cb9c7ae6b06532b6759bd9d Mon Sep 17 00:00:00 2001 From: Alysia Broddrick Date: Wed, 21 Feb 2024 16:51:57 -0800 Subject: [PATCH 17/23] handle 500 error with empty string --- src/registrar/models/domain.py | 13 ++++++++++-- src/registrar/models/utility/domain_helper.py | 11 +++++----- src/registrar/tests/test_models_domain.py | 20 ++++++++++++++----- 3 files changed, 32 insertions(+), 12 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 7ebe3dc34..d7e4a9471 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -13,6 +13,7 @@ from typing import Any from registrar.models.host import Host from registrar.models.host_ip import HostIP from registrar.utility.enums import DefaultEmail +from registrar.utility import errors from registrar.utility.errors import ( ActionNotAllowed, @@ -192,9 +193,17 @@ class Domain(TimeStampedModel, DomainHelper): @classmethod def available(cls, domain: str) -> bool: - """Check if a domain is available.""" + """Check if a domain is available. + This is called by the availablility api and + is called in the validate function on the request/domain page + + throws- RegistryError or InvalidDomainError""" if not cls.string_could_be_domain(domain): - raise ValueError("Not a valid domain: %s" % str(domain)) + logger.warning("Not a valid domain: %s" % str(domain)) + # throw invalid domain error so that it can be caught in + # validate_and_handle_errors in domain_helper + raise errors.InvalidDomainError() + domain_name = domain.lower() req = commands.CheckDomain([domain_name]) return registry.send(req, cleaned=True).res_data[0].avail diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index 9e3559676..8b9391add 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -33,11 +33,12 @@ class DomainHelper: # Split into pieces for the linter domain = cls._validate_domain_string(domain, blank_ok) - try: - if not check_domain_available(domain): - raise errors.DomainUnavailableError() - except RegistryError as err: - raise errors.RegistrySystemError() from err + if domain != "": + try: + if not check_domain_available(domain): + raise errors.DomainUnavailableError() + except RegistryError as err: + raise errors.RegistrySystemError() from err return domain @staticmethod diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 1c4d2521e..fbf47a745 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -20,7 +20,7 @@ from registrar.models.user import User from registrar.utility.errors import ActionNotAllowed, NameserverError from registrar.models.utility.contact_error import ContactError, ContactErrorCodes - +from registrar.utility import errors from django_fsm import TransitionNotAllowed # type: ignore from epplibwrapper import ( @@ -502,16 +502,26 @@ class TestDomainAvailable(MockEppLib): self.assertFalse(available) patcher.stop() - def test_domain_available_with_value_error(self): + def test_domain_available_with_invalid_error(self): """ Scenario: Testing whether an invalid domain is available - Should throw ValueError + Should throw InvalidDomainError - Validate ValueError is raised + Validate InvalidDomainError is raised """ - with self.assertRaises(ValueError): + with self.assertRaises(errors.InvalidDomainError()): Domain.available("invalid-string") + def test_domain_available_with_empty_string(self): + """ + Scenario: Testing whether an empty string domain name is available + Should throw InvalidDomainError + + Validate InvalidDomainError is raised + """ + with self.assertRaises(errors.InvalidDomainError()): + Domain.available("") + def test_domain_available_unsuccessful(self): """ Scenario: Testing behavior when registry raises a RegistryError From 68d321143f71d3ff78061a28b210bc85fb340f20 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 21 Feb 2024 18:04:57 -0800 Subject: [PATCH 18/23] Testing redeployment --- src/registrar/templates/home.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index 945f939a4..1f4cfc13b 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -13,7 +13,7 @@ {% block messages %} {% include "includes/form_messages.html" %} {% endblock %} -

Manage your domains - TESTING TESTING 123

+

Manage your domains - TESTING TESTING 1234

Date: Wed, 21 Feb 2024 19:03:59 -0800 Subject: [PATCH 19/23] removed () on Exception type --- src/registrar/tests/test_models_domain.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index fbf47a745..2bd581734 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -509,7 +509,7 @@ class TestDomainAvailable(MockEppLib): Validate InvalidDomainError is raised """ - with self.assertRaises(errors.InvalidDomainError()): + with self.assertRaises(errors.InvalidDomainError): Domain.available("invalid-string") def test_domain_available_with_empty_string(self): @@ -519,7 +519,7 @@ class TestDomainAvailable(MockEppLib): Validate InvalidDomainError is raised """ - with self.assertRaises(errors.InvalidDomainError()): + with self.assertRaises(errors.InvalidDomainError): Domain.available("") def test_domain_available_unsuccessful(self): From 0f12a535fb5e8148d5efbc4901470347012b748f Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Thu, 22 Feb 2024 11:44:30 -0800 Subject: [PATCH 20/23] Address feedback and remove head for deployment check --- src/registrar/models/domain.py | 4 ++-- src/registrar/templates/home.html | 2 +- src/registrar/tests/common.py | 3 ++- src/registrar/tests/test_models_domain.py | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index dca0693c2..500bf63be 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1779,8 +1779,8 @@ class Domain(TimeStampedModel, DomainHelper): host_in_db, host_created = Host.objects.get_or_create(domain=self, name=cleaned_host["name"]) # Check if the nameserver is a subdomain of the current domain # If it is NOT a subdomain, we remove the IP address - if not Domain.isSubdomain(self.name, cleaned_host["name"]): - cleaned_host["addrs"] = [] # or None + if not Domain.isSubdomain(self.name, host_in_db.name): + cleaned_host["addrs"] = [] # Get cleaned list of ips for update cleaned_ips = cleaned_host["addrs"] if not host_created: diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index 1f4cfc13b..a79065f50 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -13,7 +13,7 @@ {% block messages %} {% include "includes/form_messages.html" %} {% endblock %} -

Manage your domains - TESTING TESTING 1234

+

Manage your domains

Date: Thu, 22 Feb 2024 11:53:56 -0800 Subject: [PATCH 21/23] Point back to cleaned host --- src/registrar/models/domain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 500bf63be..7f684e327 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1779,7 +1779,7 @@ class Domain(TimeStampedModel, DomainHelper): host_in_db, host_created = Host.objects.get_or_create(domain=self, name=cleaned_host["name"]) # Check if the nameserver is a subdomain of the current domain # If it is NOT a subdomain, we remove the IP address - if not Domain.isSubdomain(self.name, host_in_db.name): + if not Domain.isSubdomain(self.name, cleaned_host["name"]): cleaned_host["addrs"] = [] # Get cleaned list of ips for update cleaned_ips = cleaned_host["addrs"] From 8af76b78f0a398940f5ecee38b4c33118713ffa2 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 22 Feb 2024 18:47:10 -0500 Subject: [PATCH 22/23] BCC help on submit --- src/registrar/config/settings.py | 1 + src/registrar/models/domain_application.py | 10 ++- src/registrar/tests/test_admin.py | 79 ++++++++++++++++++- .../test_environment_variables_effects.py | 31 -------- src/registrar/tests/test_views.py | 30 ++++++- src/registrar/utility/email.py | 8 +- src/registrar/views/domain.py | 3 +- 7 files changed, 122 insertions(+), 40 deletions(-) delete mode 100644 src/registrar/tests/test_environment_variables_effects.py diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 009baa1c6..bb8e22ad7 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -286,6 +286,7 @@ AWS_MAX_ATTEMPTS = 3 BOTO_CONFIG = Config(retries={"mode": AWS_RETRY_MODE, "max_attempts": AWS_MAX_ATTEMPTS}) # email address to use for various automated correspondence +# also used as a default to and bcc email DEFAULT_FROM_EMAIL = "help@get.gov " # connect to an (external) SMTP server for sending email diff --git a/src/registrar/models/domain_application.py b/src/registrar/models/domain_application.py index 17bc71cbe..1a0095ccd 100644 --- a/src/registrar/models/domain_application.py +++ b/src/registrar/models/domain_application.py @@ -4,6 +4,7 @@ from typing import Union import logging from django.apps import apps +from django.conf import settings from django.db import models from django_fsm import FSMField, transition # type: ignore from django.utils import timezone @@ -588,7 +589,7 @@ class DomainApplication(TimeStampedModel): logger.error(err) logger.error(f"Can't query an approved domain while attempting {called_from}") - def _send_status_update_email(self, new_status, email_template, email_template_subject, send_email=True): + def _send_status_update_email(self, new_status, email_template, email_template_subject, send_email=True, bcc_address=''): """Send a status update email to the submitter. The email goes to the email address that the submitter gave as their @@ -613,6 +614,7 @@ class DomainApplication(TimeStampedModel): email_template_subject, self.submitter.email, context={"application": self}, + bcc_address=bcc_address, ) logger.info(f"The {new_status} email sent to: {self.submitter.email}") except EmailSendingError: @@ -654,11 +656,17 @@ class DomainApplication(TimeStampedModel): # Limit email notifications to transitions from Started and Withdrawn limited_statuses = [self.ApplicationStatus.STARTED, self.ApplicationStatus.WITHDRAWN] + bcc_address = '' + if settings.IS_PRODUCTION: + bcc_address = settings.DEFAULT_FROM_EMAIL + if self.status in limited_statuses: self._send_status_update_email( "submission confirmation", "emails/submission_confirmation.txt", "emails/submission_confirmation_subject.txt", + True, + bcc_address, ) @transition( diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index f90b18584..c44c54ea9 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1,4 +1,4 @@ -from django.test import TestCase, RequestFactory, Client +from django.test import TestCase, RequestFactory, Client, override_settings from django.contrib.admin.sites import AdminSite from contextlib import ExitStack from django.contrib import messages @@ -426,7 +426,7 @@ class TestDomainApplicationAdmin(MockEppLib): # Use the model admin's save_model method self.admin.save_model(request, application, form=None, change=True) - def assert_email_is_accurate(self, expected_string, email_index, email_address): + def assert_email_is_accurate(self, expected_string, email_index, email_address, test_that_no_bcc=False, bcc_email_address=''): """Helper method for the email test cases. email_index is the index of the email in mock_client.""" @@ -445,12 +445,26 @@ class TestDomainApplicationAdmin(MockEppLib): self.assertEqual(to_email, email_address) self.assertIn(expected_string, email_body) + if test_that_no_bcc: + _ = '' + with self.assertRaises(KeyError): + with less_console_noise(): + _ = kwargs["Destination"]["BccAddresses"][0] + self.assertEqual(_, '') + + if bcc_email_address: + bcc_email = kwargs["Destination"]["BccAddresses"][0] + self.assertEqual(bcc_email, bcc_email_address) + def test_save_model_sends_submitted_email(self): """When transitioning to submitted from started or withdrawn on a domain request, an email is sent out. When transitioning to submitted from dns needed or in review on a domain request, - no email is sent out.""" + no email is sent out. + + Also test that the default email set in settings is NOT BCCd on non-prod whenever + an email does go out.""" # Ensure there is no user with this email EMAIL = "mayor@igorville.gov" @@ -461,7 +475,63 @@ class TestDomainApplicationAdmin(MockEppLib): # Test Submitted Status from started self.transition_state_and_send_email(application, DomainApplication.ApplicationStatus.SUBMITTED) - self.assert_email_is_accurate("We received your .gov domain request.", 0, EMAIL) + self.assert_email_is_accurate("We received your .gov domain request.", 0, EMAIL, True) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 1) + + # Test Withdrawn Status + self.transition_state_and_send_email(application, DomainApplication.ApplicationStatus.WITHDRAWN) + self.assert_email_is_accurate( + "Your .gov domain request has been withdrawn and will not be reviewed by our team.", 1, EMAIL, True + ) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 2) + + # Test Submitted Status Again (from withdrawn) + self.transition_state_and_send_email(application, DomainApplication.ApplicationStatus.SUBMITTED) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 3) + + # Move it to IN_REVIEW + self.transition_state_and_send_email(application, DomainApplication.ApplicationStatus.IN_REVIEW) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 3) + + # Test Submitted Status Again from in IN_REVIEW, no new email should be sent + self.transition_state_and_send_email(application, DomainApplication.ApplicationStatus.SUBMITTED) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 3) + + # Move it to IN_REVIEW + self.transition_state_and_send_email(application, DomainApplication.ApplicationStatus.IN_REVIEW) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 3) + + # Move it to ACTION_NEEDED + self.transition_state_and_send_email(application, DomainApplication.ApplicationStatus.ACTION_NEEDED) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 3) + + # Test Submitted Status Again from in ACTION_NEEDED, no new email should be sent + self.transition_state_and_send_email(application, DomainApplication.ApplicationStatus.SUBMITTED) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 3) + + @override_settings(IS_PRODUCTION=True) + def test_save_model_sends_submitted_email_with_bcc_on_prod(self): + """When transitioning to submitted from started or withdrawn on a domain request, + an email is sent out. + + When transitioning to submitted from dns needed or in review on a domain request, + no email is sent out. + + Also test that the default email set in settings IS BCCd on prod whenever + an email does go out.""" + + # Ensure there is no user with this email + EMAIL = "mayor@igorville.gov" + User.objects.filter(email=EMAIL).delete() + + BCC_EMAIL = settings.DEFAULT_FROM_EMAIL + + # Create a sample application + application = completed_application() + + # Test Submitted Status from started + self.transition_state_and_send_email(application, DomainApplication.ApplicationStatus.SUBMITTED) + self.assert_email_is_accurate("We received your .gov domain request.", 0, EMAIL, False, BCC_EMAIL) self.assertEqual(len(self.mock_client.EMAILS_SENT), 1) # Test Withdrawn Status @@ -473,6 +543,7 @@ class TestDomainApplicationAdmin(MockEppLib): # Test Submitted Status Again (from withdrawn) self.transition_state_and_send_email(application, DomainApplication.ApplicationStatus.SUBMITTED) + self.assert_email_is_accurate("We received your .gov domain request.", 0, EMAIL, False, BCC_EMAIL) self.assertEqual(len(self.mock_client.EMAILS_SENT), 3) # Move it to IN_REVIEW diff --git a/src/registrar/tests/test_environment_variables_effects.py b/src/registrar/tests/test_environment_variables_effects.py deleted file mode 100644 index 3a838c2a2..000000000 --- a/src/registrar/tests/test_environment_variables_effects.py +++ /dev/null @@ -1,31 +0,0 @@ -from django.test import Client, TestCase, override_settings -from django.contrib.auth import get_user_model - - -class MyTestCase(TestCase): - def setUp(self): - self.client = Client() - username = "test_user" - first_name = "First" - last_name = "Last" - email = "info@example.com" - self.user = get_user_model().objects.create( - username=username, first_name=first_name, last_name=last_name, email=email - ) - self.client.force_login(self.user) - - def tearDown(self): - super().tearDown() - self.user.delete() - - @override_settings(IS_PRODUCTION=True) - def test_production_environment(self): - """No banner on prod.""" - home_page = self.client.get("/") - self.assertNotContains(home_page, "You are on a test site.") - - @override_settings(IS_PRODUCTION=False) - def test_non_production_environment(self): - """Banner on non-prod.""" - home_page = self.client.get("/") - self.assertContains(home_page, "You are on a test site.") diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 891c254c5..b60c94b76 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1,4 +1,4 @@ -from django.test import Client, TestCase +from django.test import Client, TestCase, override_settings from django.contrib.auth import get_user_model from .common import MockEppLib # type: ignore @@ -50,3 +50,31 @@ class TestWithUser(MockEppLib): DomainApplication.objects.all().delete() DomainInformation.objects.all().delete() self.user.delete() + +class TestEnvironmentVariablesEffects(TestCase): + def setUp(self): + self.client = Client() + username = "test_user" + first_name = "First" + last_name = "Last" + email = "info@example.com" + self.user = get_user_model().objects.create( + username=username, first_name=first_name, last_name=last_name, email=email + ) + self.client.force_login(self.user) + + def tearDown(self): + super().tearDown() + self.user.delete() + + @override_settings(IS_PRODUCTION=True) + def test_production_environment(self): + """No banner on prod.""" + home_page = self.client.get("/") + self.assertNotContains(home_page, "You are on a test site.") + + @override_settings(IS_PRODUCTION=False) + def test_non_production_environment(self): + """Banner on non-prod.""" + home_page = self.client.get("/") + self.assertContains(home_page, "You are on a test site.") \ No newline at end of file diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index 461637f23..d6758969d 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -15,7 +15,7 @@ class EmailSendingError(RuntimeError): pass -def send_templated_email(template_name: str, subject_template_name: str, to_address: str, context={}): +def send_templated_email(template_name: str, subject_template_name: str, to_address: str, bcc_address='', context={}): """Send an email built from a template to one email address. template_name and subject_template_name are relative to the same template @@ -39,11 +39,15 @@ def send_templated_email(template_name: str, subject_template_name: str, to_addr ) except Exception as exc: raise EmailSendingError("Could not access the SES client.") from exc + + destination = {"ToAddresses": [to_address]} + if bcc_address: + destination["BccAddresses"] = [bcc_address] try: ses_client.send_email( FromEmailAddress=settings.DEFAULT_FROM_EMAIL, - Destination={"ToAddresses": [to_address]}, + Destination=destination, Content={ "Simple": { "Subject": {"Data": subject}, diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index f5517da25..72eb65f1e 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -14,6 +14,7 @@ from django.http import HttpResponseRedirect from django.shortcuts import redirect from django.urls import reverse from django.views.generic.edit import FormMixin +from django.conf import settings from registrar.models import ( Domain, @@ -707,7 +708,7 @@ class DomainAddUserView(DomainFormBaseView): adding a success message to the view if the email sending succeeds""" # Set a default email address to send to for staff - requestor_email = "help@get.gov" + requestor_email = settings.DEFAULT_FROM_EMAIL # Check if the email requestor has a valid email address if not requestor.is_staff and requestor.email is not None and requestor.email.strip() != "": From ec44f8fe58e32390d1f580723abd2dc1a1cb083b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 23 Feb 2024 10:45:54 -0700 Subject: [PATCH 23/23] Update admin.py --- src/registrar/admin.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index cdd8611f0..0ab6c3022 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1159,7 +1159,14 @@ class DomainAdmin(ListHeaderAdmin): if object_id is not None: domain = Domain.objects.get(pk=object_id) years_to_extend_by = self._get_calculated_years_for_exp_date(domain) - curr_exp_date = domain.registry_expiration_date + + try: + curr_exp_date = domain.registry_expiration_date + except KeyError: + # No expiration date was found. Return none. + extra_context["extended_expiration_date"] = None + return super().changeform_view(request, object_id, form_url, extra_context) + if curr_exp_date < date.today(): extra_context["extended_expiration_date"] = date.today() + relativedelta(years=years_to_extend_by) else: