From 7cf8b8a82ea0a0d3df4885eaae45b1c4d0082dfc Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Fri, 22 Nov 2024 10:51:13 -0600 Subject: [PATCH 01/97] Delete contacts and subdomains on delete domain --- src/registrar/models/domain.py | 44 ++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 7fdc56971..03a969471 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1026,6 +1026,26 @@ class Domain(TimeStampedModel, DomainHelper): # 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_contacts(self): + """Contacts associated with this domain will be deleted. + RegistryErrors will be logged and raised. Additional + error handling should be provided by the caller. + """ + contacts = self._cache.get("contacts") + for contact in contacts: + self._delete_contact(contact) + + def _delete_subdomains(self): + """Subdomains of this domain should be deleted from the registry. + Subdomains which are used by other domains (eg as a hostname) will + not be deleted. + + Supresses registry error, as registry can disallow delete for various reasons + """ + nameservers = [n[0] for n in self.nameservers] + hostsToDelete = self.createDeleteHostList(nameservers) + self._delete_hosts_if_not_used(hostsToDelete) def _delete_domain(self): """This domain should be deleted from the registry @@ -1431,6 +1451,8 @@ class Domain(TimeStampedModel, DomainHelper): @transition(field="state", source=[State.ON_HOLD, State.DNS_NEEDED], target=State.DELETED) def deletedInEpp(self): """Domain is deleted in epp but is saved in our database. + Subdomains will be deleted first if not in use by another domain. + Contacts for this domain will also be deleted. Error handling should be provided by the caller.""" # While we want to log errors, we want to preserve # that information when this function is called. @@ -1438,6 +1460,8 @@ class Domain(TimeStampedModel, DomainHelper): # as doing everything here would reduce reliablity. try: logger.info("deletedInEpp()-> inside _delete_domain") + self._delete_subdomains() + self._delete_contacts() self._delete_domain() self.deleted = timezone.now() except RegistryError as err: @@ -1639,6 +1663,26 @@ class Domain(TimeStampedModel, DomainHelper): ) raise e + + def _delete_contact(self, contact: PublicContact): + """Try to delete a contact. RegistryErrors will be logged. + + raises: + RegistryError: if the registry is unable to delete the contact + """ + logger.info("_delete_contact() -> Attempting to delete contact for %s from domain %s", contact.name, contact.domain) + try: + req = commands.DeletContact(id=contact.registry_id) + return registry.send(req, cleaned=True).res_data[0] + except RegistryError as error: + logger.error( + "Registry threw error when trying to delete contact id %s contact type is %s, error code is\n %s full error is %s", # noqa + contact.registry_id, + contact.contact_type, + error.code, + error, + ) + raise error def is_ipv6(self, ip: str): ip_addr = ipaddress.ip_address(ip) From 6891f5c8df34785be4452f81d0cf17a0f37bc754 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Tue, 26 Nov 2024 13:56:45 -0600 Subject: [PATCH 02/97] Rework delete from epp --- src/registrar/models/domain.py | 29 +++++++++---- src/registrar/tests/common.py | 30 +++++++++---- src/registrar/tests/test_models_domain.py | 51 ++++++++++++----------- 3 files changed, 70 insertions(+), 40 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 03a969471..37ce6c501 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -744,7 +744,12 @@ class Domain(TimeStampedModel, DomainHelper): successTotalNameservers = len(oldNameservers) - deleteCount + addToDomainCount - self._delete_hosts_if_not_used(hostsToDelete=deleted_values) + try: + self._delete_hosts_if_not_used(hostsToDelete=deleted_values) + except: + # in this case we don't care if there's an error, and it will be logged in the function. + pass + if successTotalNameservers < 2: try: self.dns_needed() @@ -1032,19 +1037,28 @@ class Domain(TimeStampedModel, DomainHelper): RegistryErrors will be logged and raised. Additional error handling should be provided by the caller. """ + logger.info("Deleting contacts for %s", self.name) contacts = self._cache.get("contacts") - for contact in contacts: - self._delete_contact(contact) + logger.debug("Contacts to delete for %s inside _delete_contacts -> %s", self.name, contacts) + if contacts: + for contact in contacts: + self._delete_contact(contact) + def _delete_subdomains(self): """Subdomains of this domain should be deleted from the registry. Subdomains which are used by other domains (eg as a hostname) will not be deleted. - Supresses registry error, as registry can disallow delete for various reasons + raises: + RegistryError: if any subdomain cannot be deleted """ + logger.info("Deleting nameservers for %s", self.name) nameservers = [n[0] for n in self.nameservers] - hostsToDelete = self.createDeleteHostList(nameservers) + logger.info("Nameservers found: %s", nameservers) + hostsToDelete, _ = self.createDeleteHostList(nameservers) + logger.debug("HostsToDelete from %s inside _delete_subdomains -> %s", self.name, hostsToDelete) + self._delete_hosts_if_not_used(hostsToDelete) def _delete_domain(self): @@ -1665,7 +1679,7 @@ class Domain(TimeStampedModel, DomainHelper): raise e def _delete_contact(self, contact: PublicContact): - """Try to delete a contact. RegistryErrors will be logged. + """Try to delete a contact from the registry. raises: RegistryError: if the registry is unable to delete the contact @@ -1790,7 +1804,6 @@ class Domain(TimeStampedModel, DomainHelper): """delete the host object in registry, will only delete the host object, if it's not being used by another domain Performs just the DeleteHost epp call - Supresses regstry error, as registry can disallow delete for various reasons Args: hostsToDelete (list[str])- list of nameserver/host names to remove Returns: @@ -1808,6 +1821,8 @@ class Domain(TimeStampedModel, DomainHelper): logger.info("Did not remove host %s because it is in use on another domain." % nameserver) else: logger.error("Error _delete_hosts_if_not_used, code was %s error was %s" % (e.code, e)) + + raise e def _fix_unknown_state(self, cleaned): """ diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 4edfbe680..3807534b2 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -1279,6 +1279,15 @@ class MockEppLib(TestCase): hosts=["fake.host.com"], ) + infoDomainSharedHost = fakedEppObject( + "sharedHost.gov", + cr_date=make_aware(datetime(2023, 5, 25, 19, 45, 35)), + contacts=[], + hosts=[ + "ns1.sharedhost.com", + ], + ) + infoDomainThreeHosts = fakedEppObject( "my-nameserver.gov", cr_date=make_aware(datetime(2023, 5, 25, 19, 45, 35)), @@ -1496,10 +1505,7 @@ class MockEppLib(TestCase): case commands.UpdateHost: return self.mockUpdateHostCommands(_request, cleaned) case commands.DeleteHost: - return MagicMock( - res_data=[self.mockDataHostChange], - code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, - ) + return self.mockDeletHostCommands(_request, cleaned) case commands.CheckDomain: return self.mockCheckDomainCommand(_request, cleaned) case commands.DeleteDomain: @@ -1551,6 +1557,16 @@ class MockEppLib(TestCase): res_data=[self.mockDataHostChange], code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, ) + + def mockDeletHostCommands(self, _request, cleaned): + hosts = getattr(_request, "name", None).hosts + for host in hosts: + if "sharedhost.com" in host: + raise RegistryError(code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) + return MagicMock( + res_data=[self.mockDataHostChange], + code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, + ) def mockUpdateDomainCommands(self, _request, cleaned): if getattr(_request, "name", None) == "dnssec-invalid.gov": @@ -1563,10 +1579,7 @@ class MockEppLib(TestCase): def mockDeleteDomainCommands(self, _request, cleaned): if getattr(_request, "name", None) == "failDelete.gov": - name = getattr(_request, "name", None) - fake_nameserver = "ns1.failDelete.gov" - if name in fake_nameserver: - raise RegistryError(code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) + raise RegistryError(code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) return None def mockRenewDomainCommand(self, _request, cleaned): @@ -1636,6 +1649,7 @@ class MockEppLib(TestCase): "subdomainwoip.gov": (self.mockDataInfoDomainSubdomainNoIP, None), "ddomain3.gov": (self.InfoDomainWithContacts, None), "igorville.gov": (self.InfoDomainWithContacts, None), + "sharingiscaring.gov": (self.infoDomainSharedHost, 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 bbd1e3f54..f39c485c7 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -2585,6 +2585,7 @@ class TestAnalystDelete(MockEppLib): self.domain_on_hold, _ = Domain.objects.get_or_create(name="fake-on-hold.gov", state=Domain.State.ON_HOLD) def tearDown(self): + Host.objects.all().delete() Domain.objects.all().delete() super().tearDown() @@ -2597,39 +2598,39 @@ class TestAnalystDelete(MockEppLib): The deleted date is set. """ - with less_console_noise(): - # Put the domain in client hold - self.domain.place_client_hold() - # Delete it... - self.domain.deletedInEpp() - self.domain.save() - self.mockedSendFunction.assert_has_calls( - [ - call( - commands.DeleteDomain(name="fake.gov"), - cleaned=True, - ) - ] - ) - # Domain itself should not be deleted - self.assertNotEqual(self.domain, None) - # Domain should have the right state - self.assertEqual(self.domain.state, Domain.State.DELETED) - # Domain should have a deleted - self.assertNotEqual(self.domain.deleted, None) - # Cache should be invalidated - self.assertEqual(self.domain._cache, {}) + # with less_console_noise(): + # Put the domain in client hold + self.domain.place_client_hold() + # Delete it... + self.domain.deletedInEpp() + self.domain.save() + self.mockedSendFunction.assert_has_calls( + [ + call( + commands.DeleteDomain(name="fake.gov"), + cleaned=True, + ) + ] + ) + # Domain itself should not be deleted + self.assertNotEqual(self.domain, None) + # Domain should have the right state + self.assertEqual(self.domain.state, Domain.State.DELETED) + # Domain should have a deleted + self.assertNotEqual(self.domain.deleted, None) + # Cache should be invalidated + self.assertEqual(self.domain._cache, {}) def test_deletion_is_unsuccessful(self): """ Scenario: Domain deletion is unsuccessful - When a subdomain exists + When a subdomain exists that is in use by another domain Then a client error is returned of code 2305 And `state` is not set to `DELETED` """ with less_console_noise(): # Desired domain - domain, _ = Domain.objects.get_or_create(name="failDelete.gov", state=Domain.State.ON_HOLD) + domain, _ = Domain.objects.get_or_create(name="sharingiscaring.gov", state=Domain.State.ON_HOLD) # Put the domain in client hold domain.place_client_hold() # Delete it @@ -2640,7 +2641,7 @@ class TestAnalystDelete(MockEppLib): self.mockedSendFunction.assert_has_calls( [ call( - commands.DeleteDomain(name="failDelete.gov"), + commands.DeleteHost(name=common.HostObjSet(hosts=['ns1.sharedhost.com'])), cleaned=True, ) ] From b5e4f8b40c0bf062f203d7e6dc05e305d3b05e8b Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Tue, 3 Dec 2024 15:04:25 -0600 Subject: [PATCH 03/97] update deletion process and tests --- src/registrar/models/domain.py | 39 ++++++++++++------- src/registrar/tests/common.py | 13 +++++++ src/registrar/tests/test_models_domain.py | 46 ++++++++++++++++++++++- 3 files changed, 84 insertions(+), 14 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 37ce6c501..2f5524ab4 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -254,7 +254,7 @@ class Domain(TimeStampedModel, DomainHelper): return not cls.available(domain) @Cache - def contacts(self) -> dict[str, str]: + def registry_contacts(self) -> dict[str, str]: """ Get a dictionary of registry IDs for the contacts for this domain. @@ -263,7 +263,10 @@ class Domain(TimeStampedModel, DomainHelper): { PublicContact.ContactTypeChoices.REGISTRANT: "jd1234", PublicContact.ContactTypeChoices.ADMINISTRATIVE: "sh8013",...} """ - raise NotImplementedError() + if self._cache.get("contacts"): + return self._cache.get("contacts") + else: + return self._get_property("contacts") @Cache def creation_date(self) -> date: @@ -1032,17 +1035,19 @@ class Domain(TimeStampedModel, DomainHelper): logger.error(f"registry error removing client hold: {err}") raise (err) - def _delete_contacts(self): - """Contacts associated with this domain will be deleted. - RegistryErrors will be logged and raised. Additional - error handling should be provided by the caller. + def _delete_nonregistrant_contacts(self): + """Non-registrant contacts associated with this domain will be deleted. + RegistryErrors will be logged and raised. Error + handling should be provided by the caller. """ logger.info("Deleting contacts for %s", self.name) - contacts = self._cache.get("contacts") + contacts = self.registry_contacts logger.debug("Contacts to delete for %s inside _delete_contacts -> %s", self.name, contacts) if contacts: - for contact in contacts: - self._delete_contact(contact) + for contact, id in contacts.items(): + # registrants have to be deleted after the domain + if contact != PublicContact.ContactTypeChoices.REGISTRANT: + self._delete_contact(contact, id) def _delete_subdomains(self): @@ -1067,6 +1072,13 @@ class Domain(TimeStampedModel, DomainHelper): request = commands.DeleteDomain(name=self.name) registry.send(request, cleaned=True) + def _delete_domain_registrant(self): + """This domain's registrant should be deleted from the registry + may raises RegistryError, should be caught or handled correctly by caller""" + registrantID = self.registrant_contact.registry_id + request = commands.DeleteContact(id=registrantID) + registry.send(request, cleaned=True) + def __str__(self) -> str: return self.name @@ -1475,8 +1487,9 @@ class Domain(TimeStampedModel, DomainHelper): try: logger.info("deletedInEpp()-> inside _delete_domain") self._delete_subdomains() - self._delete_contacts() + self._delete_nonregistrant_contacts() self._delete_domain() + self._delete_domain_registrant() self.deleted = timezone.now() except RegistryError as err: logger.error(f"Could not delete domain. Registry returned error: {err}") @@ -1678,15 +1691,15 @@ class Domain(TimeStampedModel, DomainHelper): raise e - def _delete_contact(self, contact: PublicContact): + def _delete_contact(self, contact_name: str, registry_id: str): """Try to delete a contact from the registry. raises: RegistryError: if the registry is unable to delete the contact """ - logger.info("_delete_contact() -> Attempting to delete contact for %s from domain %s", contact.name, contact.domain) + logger.info("_delete_contact() -> Attempting to delete contact for %s from domain %s", contact_name, self.name) try: - req = commands.DeletContact(id=contact.registry_id) + req = commands.DeleteContact(id=registry_id) return registry.send(req, cleaned=True).res_data[0] except RegistryError as error: logger.error( diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 5bfa63462..ac444c8aa 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -1229,6 +1229,7 @@ class MockEppLib(TestCase): common.Status(state="serverTransferProhibited", description="", lang="en"), common.Status(state="inactive", description="", lang="en"), ], + registrant="regContact", ex_date=date(2023, 5, 25), ) @@ -1610,6 +1611,8 @@ class MockEppLib(TestCase): return self.mockInfoContactCommands(_request, cleaned) case commands.CreateContact: return self.mockCreateContactCommands(_request, cleaned) + case commands.DeleteContact: + return self.mockDeleteContactCommands(_request, cleaned) case commands.UpdateDomain: return self.mockUpdateDomainCommands(_request, cleaned) case commands.CreateHost: @@ -1731,6 +1734,7 @@ class MockEppLib(TestCase): # Define a dictionary to map request names to data and extension values request_mappings = { + "fake.gov": (self.mockDataInfoDomain, None), "security.gov": (self.infoDomainNoContact, None), "dnssec-dsdata.gov": ( self.mockDataInfoDomain, @@ -1811,6 +1815,15 @@ class MockEppLib(TestCase): # mocks a contact error on creation raise ContactError(code=ContactErrorCodes.CONTACT_TYPE_NONE) return MagicMock(res_data=[self.mockDataInfoHosts]) + + def mockDeleteContactCommands(self, _request, cleaned): + if getattr(_request, "id", None) == "fail": + raise RegistryError(code=ErrorCode.OBJECT_EXISTS) + else: + return MagicMock( + res_data=[self.mockDataInfoContact], + code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, + ) def setUp(self): """mock epp send function as this will fail locally""" diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index f39c485c7..73691bb69 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -2586,6 +2586,7 @@ class TestAnalystDelete(MockEppLib): def tearDown(self): Host.objects.all().delete() + PublicContact.objects.all().delete() Domain.objects.all().delete() super().tearDown() @@ -2643,7 +2644,7 @@ class TestAnalystDelete(MockEppLib): call( commands.DeleteHost(name=common.HostObjSet(hosts=['ns1.sharedhost.com'])), cleaned=True, - ) + ), ] ) # Domain itself should not be deleted @@ -2651,6 +2652,49 @@ class TestAnalystDelete(MockEppLib): # State should not have changed self.assertEqual(domain.state, Domain.State.ON_HOLD) + def test_deletion_with_host_and_contacts(self): + """ + Scenario: Domain with related Host and Contacts is Deleted + When a contact and host exists that is tied to this domain + Then `commands.DeleteHost` is sent to the registry + Then `commands.DeleteContact` is sent to the registry + Then `commands.DeleteDomain` is sent to the registry + Then `commands.DeleteContact` is sent to the registry for the registrant contact + And `state` is set to `DELETED` + """ + # with less_console_noise(): + # Desired domain + domain, _ = Domain.objects.get_or_create(name="freeman.gov", state=Domain.State.ON_HOLD) + # Put the domain in client hold + domain.place_client_hold() + # Delete it + domain.deletedInEpp() + domain.save() + + # Check that the host and contacts are deleted, order doesn't matter + self.mockedSendFunction.assert_has_calls( + [ + call(commands.DeleteHost(name=common.HostObjSet(hosts=['fake.host.com'])), cleaned=True), + call(commands.DeleteContact(id="securityContact"), cleaned=True), + call(commands.DeleteContact(id="technicalContact"), cleaned=True), + call(commands.DeleteContact(id="adminContact"),cleaned=True,) + ], + any_order=True + ) + + # These calls need to be in order + self.mockedSendFunction.assert_has_calls( + [ + call(commands.DeleteDomain(name="freeman.gov"), cleaned=True), + call(commands.InfoContact(id="regContact"), cleaned=True), + call(commands.DeleteContact(id="regContact"), cleaned=True), + ], + ) + # Domain itself should not be deleted + self.assertNotEqual(domain, None) + # State should have changed + self.assertEqual(domain.state, Domain.State.DELETED) + def test_deletion_ready_fsm_failure(self): """ Scenario: Domain deletion is unsuccessful due to FSM rules From 27868a0aed8f1fe6fad6566788b281ca761dc163 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Dec 2024 11:24:49 -0600 Subject: [PATCH 04/97] minor fixes to tests --- src/registrar/models/domain.py | 10 ++-- src/registrar/tests/common.py | 11 ++-- src/registrar/tests/test_models_domain.py | 71 ++++++++++++----------- 3 files changed, 46 insertions(+), 46 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 2f5524ab4..9c954b073 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -161,12 +161,12 @@ class Domain(TimeStampedModel, DomainHelper): """Returns a help message for a desired state. If none is found, an empty string is returned""" help_texts = { # For now, unknown has the same message as DNS_NEEDED - cls.UNKNOWN: ("Before this domain can be used, " "you’ll need to add name server addresses."), - cls.DNS_NEEDED: ("Before this domain can be used, " "you’ll need to add name server addresses."), + cls.UNKNOWN: ("Before this domain can be used, " "you'll need to add name server addresses."), + cls.DNS_NEEDED: ("Before this domain can be used, " "you'll need to add name server addresses."), cls.READY: "This domain has name servers and is ready for use.", cls.ON_HOLD: ( "This domain is administratively paused, " - "so it can’t be edited and won’t resolve in DNS. " + "so it can't be edited and won't resolve in DNS. " "Contact help@get.gov for details." ), cls.DELETED: ("This domain has been removed and " "is no longer registered to your organization."), @@ -1060,11 +1060,11 @@ class Domain(TimeStampedModel, DomainHelper): """ logger.info("Deleting nameservers for %s", self.name) nameservers = [n[0] for n in self.nameservers] - logger.info("Nameservers found: %s", nameservers) hostsToDelete, _ = self.createDeleteHostList(nameservers) logger.debug("HostsToDelete from %s inside _delete_subdomains -> %s", self.name, hostsToDelete) - self._delete_hosts_if_not_used(hostsToDelete) + for objSet in hostsToDelete: + self._delete_hosts_if_not_used(objSet.hosts) def _delete_domain(self): """This domain should be deleted from the registry diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index ac444c8aa..72a315e9b 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -1620,7 +1620,7 @@ class MockEppLib(TestCase): case commands.UpdateHost: return self.mockUpdateHostCommands(_request, cleaned) case commands.DeleteHost: - return self.mockDeletHostCommands(_request, cleaned) + return self.mockDeleteHostCommands(_request, cleaned) case commands.CheckDomain: return self.mockCheckDomainCommand(_request, cleaned) case commands.DeleteDomain: @@ -1673,11 +1673,10 @@ class MockEppLib(TestCase): code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, ) - def mockDeletHostCommands(self, _request, cleaned): - hosts = getattr(_request, "name", None).hosts - for host in hosts: - if "sharedhost.com" in host: - raise RegistryError(code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) + def mockDeleteHostCommands(self, _request, cleaned): + host = getattr(_request, "name", None) + if "sharedhost.com" in host: + raise RegistryError(code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) return MagicMock( res_data=[self.mockDataHostChange], code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 73691bb69..b013c7811 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -1422,40 +1422,41 @@ class TestRegistrantNameservers(MockEppLib): And `domain.is_active` returns False """ - with less_console_noise(): - self.domainWithThreeNS.nameservers = [(self.nameserver1,)] - expectedCalls = [ - call( - commands.InfoDomain(name=self.domainWithThreeNS.name, auth_info=None), - cleaned=True, + # with less_console_noise(): + self.domainWithThreeNS.nameservers = [(self.nameserver1,)] + expectedCalls = [ + call( + commands.InfoDomain(name=self.domainWithThreeNS.name, auth_info=None), + cleaned=True, + ), + call(commands.InfoHost(name="ns1.my-nameserver-1.com"), cleaned=True), + call(commands.InfoHost(name="ns1.my-nameserver-2.com"), cleaned=True), + call(commands.InfoHost(name="ns1.cats-are-superior3.com"), cleaned=True), + call(commands.DeleteHost(name="ns1.my-nameserver-2.com"), cleaned=True), + call( + commands.UpdateDomain( + name=self.domainWithThreeNS.name, + add=[], + rem=[ + common.HostObjSet( + hosts=[ + "ns1.my-nameserver-2.com", + "ns1.cats-are-superior3.com", + ] + ), + ], + nsset=None, + keyset=None, + registrant=None, + auth_info=None, ), - call(commands.InfoHost(name="ns1.my-nameserver-1.com"), cleaned=True), - call(commands.InfoHost(name="ns1.my-nameserver-2.com"), cleaned=True), - call(commands.InfoHost(name="ns1.cats-are-superior3.com"), cleaned=True), - call(commands.DeleteHost(name="ns1.my-nameserver-2.com"), cleaned=True), - call( - commands.UpdateDomain( - name=self.domainWithThreeNS.name, - add=[], - rem=[ - common.HostObjSet( - hosts=[ - "ns1.my-nameserver-2.com", - "ns1.cats-are-superior3.com", - ] - ), - ], - nsset=None, - keyset=None, - registrant=None, - auth_info=None, - ), - cleaned=True, - ), - call(commands.DeleteHost(name="ns1.cats-are-superior3.com"), cleaned=True), - ] - self.mockedSendFunction.assert_has_calls(expectedCalls, any_order=True) - self.assertFalse(self.domainWithThreeNS.is_active()) + cleaned=True, + ), + call(commands.DeleteHost(name="ns1.cats-are-superior3.com"), cleaned=True), + ] + + self.mockedSendFunction.assert_has_calls(expectedCalls, any_order=True) + self.assertFalse(self.domainWithThreeNS.is_active()) def test_user_replaces_nameservers(self): """ @@ -2642,7 +2643,7 @@ class TestAnalystDelete(MockEppLib): self.mockedSendFunction.assert_has_calls( [ call( - commands.DeleteHost(name=common.HostObjSet(hosts=['ns1.sharedhost.com'])), + commands.DeleteHost(name='ns1.sharedhost.com'), cleaned=True, ), ] @@ -2674,7 +2675,7 @@ class TestAnalystDelete(MockEppLib): # Check that the host and contacts are deleted, order doesn't matter self.mockedSendFunction.assert_has_calls( [ - call(commands.DeleteHost(name=common.HostObjSet(hosts=['fake.host.com'])), cleaned=True), + call(commands.DeleteHost(name='fake.host.com'), cleaned=True), call(commands.DeleteContact(id="securityContact"), cleaned=True), call(commands.DeleteContact(id="technicalContact"), cleaned=True), call(commands.DeleteContact(id="adminContact"),cleaned=True,) From 9437b732c8a475d3b5216ca9c805942e3507b586 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Dec 2024 12:50:28 -0600 Subject: [PATCH 05/97] minor test fix --- src/registrar/tests/test_admin_domain.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py index f02b59a91..ee275741c 100644 --- a/src/registrar/tests/test_admin_domain.py +++ b/src/registrar/tests/test_admin_domain.py @@ -16,6 +16,7 @@ from registrar.models import ( Host, Portfolio, ) +from registrar.models.public_contact import PublicContact from registrar.models.user_domain_role import UserDomainRole from .common import ( MockSESClient, @@ -59,6 +60,7 @@ class TestDomainAdminAsStaff(MockEppLib): def tearDown(self): super().tearDown() Host.objects.all().delete() + PublicContact.objects.all().delete() Domain.objects.all().delete() DomainInformation.objects.all().delete() DomainRequest.objects.all().delete() From 89253a1626f9e4cf6a24f0d8ed4ef203822a8e30 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Dec 2024 13:37:23 -0600 Subject: [PATCH 06/97] linter fixes --- src/registrar/models/domain.py | 25 ++++++++--------------- src/registrar/tests/common.py | 10 ++++----- src/registrar/tests/test_models_domain.py | 13 +++++++----- 3 files changed, 22 insertions(+), 26 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 9c954b073..64d29a21a 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -747,11 +747,7 @@ class Domain(TimeStampedModel, DomainHelper): successTotalNameservers = len(oldNameservers) - deleteCount + addToDomainCount - try: - self._delete_hosts_if_not_used(hostsToDelete=deleted_values) - except: - # in this case we don't care if there's an error, and it will be logged in the function. - pass + self._delete_hosts_if_not_used(hostsToDelete=deleted_values) if successTotalNameservers < 2: try: @@ -1034,10 +1030,10 @@ class Domain(TimeStampedModel, DomainHelper): # 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_nonregistrant_contacts(self): """Non-registrant contacts associated with this domain will be deleted. - RegistryErrors will be logged and raised. Error + RegistryErrors will be logged and raised. Error handling should be provided by the caller. """ logger.info("Deleting contacts for %s", self.name) @@ -1048,8 +1044,7 @@ class Domain(TimeStampedModel, DomainHelper): # registrants have to be deleted after the domain if contact != PublicContact.ContactTypeChoices.REGISTRANT: self._delete_contact(contact, id) - - + def _delete_subdomains(self): """Subdomains of this domain should be deleted from the registry. Subdomains which are used by other domains (eg as a hostname) will @@ -1690,10 +1685,10 @@ class Domain(TimeStampedModel, DomainHelper): ) raise e - + def _delete_contact(self, contact_name: str, registry_id: str): """Try to delete a contact from the registry. - + raises: RegistryError: if the registry is unable to delete the contact """ @@ -1703,10 +1698,8 @@ class Domain(TimeStampedModel, DomainHelper): return registry.send(req, cleaned=True).res_data[0] except RegistryError as error: logger.error( - "Registry threw error when trying to delete contact id %s contact type is %s, error code is\n %s full error is %s", # noqa - contact.registry_id, - contact.contact_type, - error.code, + "Registry threw error when trying to delete contact %s, error: %s", # noqa + contact_name, error, ) raise error @@ -1834,7 +1827,7 @@ class Domain(TimeStampedModel, DomainHelper): logger.info("Did not remove host %s because it is in use on another domain." % nameserver) else: logger.error("Error _delete_hosts_if_not_used, code was %s error was %s" % (e.code, e)) - + raise e def _fix_unknown_state(self, cleaned): diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 72a315e9b..79c262cb9 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -1672,7 +1672,7 @@ class MockEppLib(TestCase): res_data=[self.mockDataHostChange], code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, ) - + def mockDeleteHostCommands(self, _request, cleaned): host = getattr(_request, "name", None) if "sharedhost.com" in host: @@ -1814,15 +1814,15 @@ class MockEppLib(TestCase): # mocks a contact error on creation raise ContactError(code=ContactErrorCodes.CONTACT_TYPE_NONE) return MagicMock(res_data=[self.mockDataInfoHosts]) - + def mockDeleteContactCommands(self, _request, cleaned): if getattr(_request, "id", None) == "fail": raise RegistryError(code=ErrorCode.OBJECT_EXISTS) else: return MagicMock( - res_data=[self.mockDataInfoContact], - code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, - ) + res_data=[self.mockDataInfoContact], + code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, + ) def setUp(self): """mock epp send function as this will fail locally""" diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index b013c7811..e381a06fe 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -2643,7 +2643,7 @@ class TestAnalystDelete(MockEppLib): self.mockedSendFunction.assert_has_calls( [ call( - commands.DeleteHost(name='ns1.sharedhost.com'), + commands.DeleteHost(name="ns1.sharedhost.com"), cleaned=True, ), ] @@ -2664,7 +2664,7 @@ class TestAnalystDelete(MockEppLib): And `state` is set to `DELETED` """ # with less_console_noise(): - # Desired domain + # Desired domain domain, _ = Domain.objects.get_or_create(name="freeman.gov", state=Domain.State.ON_HOLD) # Put the domain in client hold domain.place_client_hold() @@ -2675,12 +2675,15 @@ class TestAnalystDelete(MockEppLib): # Check that the host and contacts are deleted, order doesn't matter self.mockedSendFunction.assert_has_calls( [ - call(commands.DeleteHost(name='fake.host.com'), cleaned=True), + call(commands.DeleteHost(name="fake.host.com"), cleaned=True), call(commands.DeleteContact(id="securityContact"), cleaned=True), call(commands.DeleteContact(id="technicalContact"), cleaned=True), - call(commands.DeleteContact(id="adminContact"),cleaned=True,) + call( + commands.DeleteContact(id="adminContact"), + cleaned=True, + ), ], - any_order=True + any_order=True, ) # These calls need to be in order From f25bb9be055835866c004a827e7241ef0485c1cd Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Dec 2024 16:28:33 -0600 Subject: [PATCH 07/97] include hostname in error messages for shared hosts --- src/registrar/models/domain.py | 23 ++++++- src/registrar/tests/common.py | 1 + src/registrar/tests/test_models_domain.py | 79 +++++++++++------------ 3 files changed, 59 insertions(+), 44 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 64d29a21a..61cc539b0 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -231,6 +231,14 @@ class Domain(TimeStampedModel, DomainHelper): """Called during delete. Example: `del domain.registrant`.""" super().__delete__(obj) + def save( + self, force_insert=False, force_update=False, using=None, update_fields=None + ): + # If the domain is deleted we don't want the expiration date to be set + if self.state == self.State.DELETED and self.expiration_date: + self.expiration_date = None + super().save(force_insert, force_update, using, update_fields) + @classmethod def available(cls, domain: str) -> bool: """Check if a domain is available. @@ -1054,6 +1062,13 @@ class Domain(TimeStampedModel, DomainHelper): RegistryError: if any subdomain cannot be deleted """ logger.info("Deleting nameservers for %s", self.name) + # check if any nameservers are in use by another domain + hosts = Host.objects.filter(name__regex=r'.+{}'.format(self.name)) + for host in hosts: + if host.domain != self: + logger.error("Host %s in use by another domain: %s", host.name, host.domain) + raise RegistryError("Host in use by another domain: {}".format(host.domain), code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) + nameservers = [n[0] for n in self.nameservers] hostsToDelete, _ = self.createDeleteHostList(nameservers) logger.debug("HostsToDelete from %s inside _delete_subdomains -> %s", self.name, hostsToDelete) @@ -1070,9 +1085,10 @@ class Domain(TimeStampedModel, DomainHelper): def _delete_domain_registrant(self): """This domain's registrant should be deleted from the registry may raises RegistryError, should be caught or handled correctly by caller""" - registrantID = self.registrant_contact.registry_id - request = commands.DeleteContact(id=registrantID) - registry.send(request, cleaned=True) + if self.registrant_contact: + registrantID = self.registrant_contact.registry_id + request = commands.DeleteContact(id=registrantID) + registry.send(request, cleaned=True) def __str__(self) -> str: return self.name @@ -1486,6 +1502,7 @@ class Domain(TimeStampedModel, DomainHelper): self._delete_domain() self._delete_domain_registrant() self.deleted = timezone.now() + self.expiration_date = None except RegistryError as err: logger.error(f"Could not delete domain. Registry returned error: {err}") raise err diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 79c262cb9..16fa58104 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -1676,6 +1676,7 @@ class MockEppLib(TestCase): def mockDeleteHostCommands(self, _request, cleaned): host = getattr(_request, "name", None) if "sharedhost.com" in host: + print("raising registry error") raise RegistryError(code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) return MagicMock( res_data=[self.mockDataHostChange], diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index e381a06fe..8dfb764e3 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -2584,6 +2584,7 @@ class TestAnalystDelete(MockEppLib): super().setUp() self.domain, _ = Domain.objects.get_or_create(name="fake.gov", state=Domain.State.READY) self.domain_on_hold, _ = Domain.objects.get_or_create(name="fake-on-hold.gov", state=Domain.State.ON_HOLD) + Host.objects.create(name="ns1.sharingiscaring.gov", domain=self.domain_on_hold) def tearDown(self): Host.objects.all().delete() @@ -2639,15 +2640,9 @@ class TestAnalystDelete(MockEppLib): with self.assertRaises(RegistryError) as err: domain.deletedInEpp() domain.save() + self.assertTrue(err.is_client_error() and err.code == ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) - self.mockedSendFunction.assert_has_calls( - [ - call( - commands.DeleteHost(name="ns1.sharedhost.com"), - cleaned=True, - ), - ] - ) + self.assertEqual(err.msg, "Host in use by another domain: fake-on-hold.gov") # Domain itself should not be deleted self.assertNotEqual(domain, None) # State should not have changed @@ -2663,41 +2658,43 @@ class TestAnalystDelete(MockEppLib): Then `commands.DeleteContact` is sent to the registry for the registrant contact And `state` is set to `DELETED` """ - # with less_console_noise(): - # Desired domain - domain, _ = Domain.objects.get_or_create(name="freeman.gov", state=Domain.State.ON_HOLD) - # Put the domain in client hold - domain.place_client_hold() - # Delete it - domain.deletedInEpp() - domain.save() + with less_console_noise(): + # Desired domain + domain, _ = Domain.objects.get_or_create(name="freeman.gov", state=Domain.State.ON_HOLD) + # Put the domain in client hold + domain.place_client_hold() + # Delete it + domain.deletedInEpp() + domain.save() - # Check that the host and contacts are deleted, order doesn't matter - self.mockedSendFunction.assert_has_calls( - [ - call(commands.DeleteHost(name="fake.host.com"), cleaned=True), - call(commands.DeleteContact(id="securityContact"), cleaned=True), - call(commands.DeleteContact(id="technicalContact"), cleaned=True), - call( - commands.DeleteContact(id="adminContact"), - cleaned=True, - ), - ], - any_order=True, - ) + # Check that the host and contacts are deleted, order doesn't matter + self.mockedSendFunction.assert_has_calls( + [ + call(commands.DeleteHost(name="fake.host.com"), cleaned=True), + call(commands.DeleteContact(id="securityContact"), cleaned=True), + call(commands.DeleteContact(id="technicalContact"), cleaned=True), + call( + commands.DeleteContact(id="adminContact"), + cleaned=True, + ), + ], + any_order=True, + ) + actual_calls = self.mockedSendFunction.call_args_list + print("actual_calls", actual_calls) - # These calls need to be in order - self.mockedSendFunction.assert_has_calls( - [ - call(commands.DeleteDomain(name="freeman.gov"), cleaned=True), - call(commands.InfoContact(id="regContact"), cleaned=True), - call(commands.DeleteContact(id="regContact"), cleaned=True), - ], - ) - # Domain itself should not be deleted - self.assertNotEqual(domain, None) - # State should have changed - self.assertEqual(domain.state, Domain.State.DELETED) + # These calls need to be in order + self.mockedSendFunction.assert_has_calls( + [ + call(commands.DeleteDomain(name="freeman.gov"), cleaned=True), + call(commands.InfoContact(id="regContact"), cleaned=True), + call(commands.DeleteContact(id="regContact"), cleaned=True), + ], + ) + # Domain itself should not be deleted + self.assertNotEqual(domain, None) + # State should have changed + self.assertEqual(domain.state, Domain.State.DELETED) def test_deletion_ready_fsm_failure(self): """ From dad42264bf6293765c107dac17861bec078b7357 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Dec 2024 16:32:17 -0600 Subject: [PATCH 08/97] add back in less console noise decorator --- src/registrar/tests/test_models_domain.py | 110 +++++++++++----------- 1 file changed, 55 insertions(+), 55 deletions(-) diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 8dfb764e3..8fd2b5411 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -1422,41 +1422,41 @@ class TestRegistrantNameservers(MockEppLib): And `domain.is_active` returns False """ - # with less_console_noise(): - self.domainWithThreeNS.nameservers = [(self.nameserver1,)] - expectedCalls = [ - call( - commands.InfoDomain(name=self.domainWithThreeNS.name, auth_info=None), - cleaned=True, - ), - call(commands.InfoHost(name="ns1.my-nameserver-1.com"), cleaned=True), - call(commands.InfoHost(name="ns1.my-nameserver-2.com"), cleaned=True), - call(commands.InfoHost(name="ns1.cats-are-superior3.com"), cleaned=True), - call(commands.DeleteHost(name="ns1.my-nameserver-2.com"), cleaned=True), - call( - commands.UpdateDomain( - name=self.domainWithThreeNS.name, - add=[], - rem=[ - common.HostObjSet( - hosts=[ - "ns1.my-nameserver-2.com", - "ns1.cats-are-superior3.com", - ] - ), - ], - nsset=None, - keyset=None, - registrant=None, - auth_info=None, + with less_console_noise(): + self.domainWithThreeNS.nameservers = [(self.nameserver1,)] + expectedCalls = [ + call( + commands.InfoDomain(name=self.domainWithThreeNS.name, auth_info=None), + cleaned=True, ), - cleaned=True, - ), - call(commands.DeleteHost(name="ns1.cats-are-superior3.com"), cleaned=True), - ] + call(commands.InfoHost(name="ns1.my-nameserver-1.com"), cleaned=True), + call(commands.InfoHost(name="ns1.my-nameserver-2.com"), cleaned=True), + call(commands.InfoHost(name="ns1.cats-are-superior3.com"), cleaned=True), + call(commands.DeleteHost(name="ns1.my-nameserver-2.com"), cleaned=True), + call( + commands.UpdateDomain( + name=self.domainWithThreeNS.name, + add=[], + rem=[ + common.HostObjSet( + hosts=[ + "ns1.my-nameserver-2.com", + "ns1.cats-are-superior3.com", + ] + ), + ], + nsset=None, + keyset=None, + registrant=None, + auth_info=None, + ), + cleaned=True, + ), + call(commands.DeleteHost(name="ns1.cats-are-superior3.com"), cleaned=True), + ] - self.mockedSendFunction.assert_has_calls(expectedCalls, any_order=True) - self.assertFalse(self.domainWithThreeNS.is_active()) + self.mockedSendFunction.assert_has_calls(expectedCalls, any_order=True) + self.assertFalse(self.domainWithThreeNS.is_active()) def test_user_replaces_nameservers(self): """ @@ -2601,28 +2601,28 @@ class TestAnalystDelete(MockEppLib): The deleted date is set. """ - # with less_console_noise(): - # Put the domain in client hold - self.domain.place_client_hold() - # Delete it... - self.domain.deletedInEpp() - self.domain.save() - self.mockedSendFunction.assert_has_calls( - [ - call( - commands.DeleteDomain(name="fake.gov"), - cleaned=True, - ) - ] - ) - # Domain itself should not be deleted - self.assertNotEqual(self.domain, None) - # Domain should have the right state - self.assertEqual(self.domain.state, Domain.State.DELETED) - # Domain should have a deleted - self.assertNotEqual(self.domain.deleted, None) - # Cache should be invalidated - self.assertEqual(self.domain._cache, {}) + with less_console_noise(): + # Put the domain in client hold + self.domain.place_client_hold() + # Delete it... + self.domain.deletedInEpp() + self.domain.save() + self.mockedSendFunction.assert_has_calls( + [ + call( + commands.DeleteDomain(name="fake.gov"), + cleaned=True, + ) + ] + ) + # Domain itself should not be deleted + self.assertNotEqual(self.domain, None) + # Domain should have the right state + self.assertEqual(self.domain.state, Domain.State.DELETED) + # Domain should have a deleted + self.assertNotEqual(self.domain.deleted, None) + # Cache should be invalidated + self.assertEqual(self.domain._cache, {}) def test_deletion_is_unsuccessful(self): """ From 6fdb763c0249bdc46684a0d8d2e3928a442fae43 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Dec 2024 17:10:45 -0600 Subject: [PATCH 09/97] admin fix --- src/registrar/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 40d4befb5..042666619 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1570,7 +1570,7 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): "is_policy_acknowledged", ] - # For each filter_horizontal, init in admin js initFilterHorizontalWidget + # For each filter_horizontal, init in admin js extendFilterHorizontalWidgets # to activate the edit/delete/view buttons filter_horizontal = ("other_contacts",) From e8fdf0c5d376b2b94647ff782d59adfdf6d957f5 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 5 Dec 2024 10:16:40 -0600 Subject: [PATCH 10/97] revert accidental admin change --- src/registrar/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 042666619..40d4befb5 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1570,7 +1570,7 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): "is_policy_acknowledged", ] - # For each filter_horizontal, init in admin js extendFilterHorizontalWidgets + # For each filter_horizontal, init in admin js initFilterHorizontalWidget # to activate the edit/delete/view buttons filter_horizontal = ("other_contacts",) From 3f79b562bd9db55af9eb5aac5bf08c3aca61a962 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 5 Dec 2024 10:58:12 -0600 Subject: [PATCH 11/97] temp test changes --- src/registrar/models/domain.py | 6 +++--- src/registrar/tests/test_views.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 61cc539b0..6ca3676f7 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -161,12 +161,12 @@ class Domain(TimeStampedModel, DomainHelper): """Returns a help message for a desired state. If none is found, an empty string is returned""" help_texts = { # For now, unknown has the same message as DNS_NEEDED - cls.UNKNOWN: ("Before this domain can be used, " "you'll need to add name server addresses."), - cls.DNS_NEEDED: ("Before this domain can be used, " "you'll need to add name server addresses."), + cls.UNKNOWN: ("Before this domain can be used, " "you’ll need to add name server addresses."), + cls.DNS_NEEDED: ("Before this domain can be used, " "you’ll need to add name server addresses."), cls.READY: "This domain has name servers and is ready for use.", cls.ON_HOLD: ( "This domain is administratively paused, " - "so it can't be edited and won't resolve in DNS. " + "so it can’t be edited and won’t resolve in DNS. " "Contact help@get.gov for details." ), cls.DELETED: ("This domain has been removed and " "is no longer registered to your organization."), diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index f46e417be..3c1f1959e 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -169,7 +169,7 @@ class HomeTests(TestWithUser): self.assertContains(response, "You don't have any registered domains.") self.assertContains(response, "Why don't I see my domain when I sign in to the registrar?") - @less_console_noise_decorator + # @less_console_noise_decorator def test_state_help_text(self): """Tests if each domain state has help text""" From 2e841711e112cf0d1482dd42e19d839d86cfbbac Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 5 Dec 2024 11:30:16 -0600 Subject: [PATCH 12/97] fix a test --- src/registrar/models/domain.py | 2 +- src/registrar/tests/test_views.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 6ca3676f7..661e958e6 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1154,7 +1154,7 @@ class Domain(TimeStampedModel, DomainHelper): Returns True if expired, False otherwise. """ if self.expiration_date is None: - return True + return self.state != self.State.DELETED now = timezone.now().date() return self.expiration_date < now diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 3c1f1959e..f46e417be 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -169,7 +169,7 @@ class HomeTests(TestWithUser): self.assertContains(response, "You don't have any registered domains.") self.assertContains(response, "Why don't I see my domain when I sign in to the registrar?") - # @less_console_noise_decorator + @less_console_noise_decorator def test_state_help_text(self): """Tests if each domain state has help text""" From aaaa4f21d238e1e46b0010741cf7be55a7a41822 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 5 Dec 2024 12:50:25 -0600 Subject: [PATCH 13/97] fix broken test --- src/registrar/models/domain.py | 13 +++++++------ src/registrar/tests/test_models_domain.py | 2 +- src/registrar/tests/test_reports.py | 8 ++++---- src/registrar/utility/csv_export.py | 3 ++- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 661e958e6..348ccf3ad 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -231,9 +231,7 @@ class Domain(TimeStampedModel, DomainHelper): """Called during delete. Example: `del domain.registrant`.""" super().__delete__(obj) - def save( - self, force_insert=False, force_update=False, using=None, update_fields=None - ): + def save(self, force_insert=False, force_update=False, using=None, update_fields=None): # If the domain is deleted we don't want the expiration date to be set if self.state == self.State.DELETED and self.expiration_date: self.expiration_date = None @@ -1063,12 +1061,15 @@ class Domain(TimeStampedModel, DomainHelper): """ logger.info("Deleting nameservers for %s", self.name) # check if any nameservers are in use by another domain - hosts = Host.objects.filter(name__regex=r'.+{}'.format(self.name)) + hosts = Host.objects.filter(name__regex=r".+{}".format(self.name)) for host in hosts: if host.domain != self: logger.error("Host %s in use by another domain: %s", host.name, host.domain) - raise RegistryError("Host in use by another domain: {}".format(host.domain), code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) - + raise RegistryError( + "Host in use by another domain: {}".format(host.domain), + code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION, + ) + nameservers = [n[0] for n in self.nameservers] hostsToDelete, _ = self.createDeleteHostList(nameservers) logger.debug("HostsToDelete from %s inside _delete_subdomains -> %s", self.name, hostsToDelete) diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 8fd2b5411..e5df19d82 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -2640,7 +2640,7 @@ class TestAnalystDelete(MockEppLib): with self.assertRaises(RegistryError) as err: domain.deletedInEpp() domain.save() - + self.assertTrue(err.is_client_error() and err.code == ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION) self.assertEqual(err.msg, "Host in use by another domain: fake-on-hold.gov") # Domain itself should not be deleted diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 377216aa4..0c3fad51a 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -880,18 +880,18 @@ class MemberExportTest(MockDbForIndividualTests, MockEppLib): "Email,Organization admin,Invited by,Joined date,Last active,Domain requests," "Member management,Domain management,Number of domains,Domains\n" # Content + "big_lebowski@dude.co,False,help@get.gov,2022-04-01,Invalid date,None,Viewer,True,1,cdomain1.gov\n" + "cozy_staffuser@igorville.gov,True,help@get.gov,2022-04-01,2024-02-01,Viewer,Viewer,False,0,\n" + "icy_superuser@igorville.gov,True,help@get.gov,2022-04-01,2024-02-01,Viewer Requester,Manager,False,0,\n" "meoward@rocks.com,False,big_lebowski@dude.co,2022-04-01,Invalid date,None," 'Manager,True,2,"adomain2.gov,cdomain1.gov"\n' - "big_lebowski@dude.co,False,help@get.gov,2022-04-01,Invalid date,None,Viewer,True,1,cdomain1.gov\n" - "tired_sleepy@igorville.gov,False,System,2022-04-01,Invalid date,Viewer,None,False,0,\n" - "icy_superuser@igorville.gov,True,help@get.gov,2022-04-01,2024-02-01,Viewer Requester,Manager,False,0,\n" - "cozy_staffuser@igorville.gov,True,help@get.gov,2022-04-01,2024-02-01,Viewer,Viewer,False,0,\n" "nonexistentmember_1@igorville.gov,False,help@get.gov,Unretrieved,Invited,None,Manager,False,0,\n" "nonexistentmember_2@igorville.gov,False,help@get.gov,Unretrieved,Invited,None,Viewer,False,0,\n" "nonexistentmember_3@igorville.gov,False,help@get.gov,Unretrieved,Invited,Viewer,None,False,0,\n" "nonexistentmember_4@igorville.gov,True,help@get.gov,Unretrieved," "Invited,Viewer Requester,Manager,False,0,\n" "nonexistentmember_5@igorville.gov,True,help@get.gov,Unretrieved,Invited,Viewer,Viewer,False,0,\n" + "tired_sleepy@igorville.gov,False,System,2022-04-01,Invalid date,Viewer,None,False,0,\n" ) # Normalize line endings and remove commas, # spaces and leading/trailing whitespace diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index a03e51de5..48a5f9e2d 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -415,7 +415,8 @@ class MemberExport(BaseExport): .values(*shared_columns) ) - return convert_queryset_to_dict(permissions.union(invitations), is_model=False) + members = permissions.union(invitations).order_by("email_display") + return convert_queryset_to_dict(members, is_model=False) @classmethod def get_invited_by_query(cls, object_id_query): From 8b473d5e1846d80d4495ff5b375a2136c8b14f53 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 5 Dec 2024 13:55:32 -0600 Subject: [PATCH 14/97] add error message to registry errors --- src/epplibwrapper/errors.py | 3 ++- src/registrar/admin.py | 5 ++--- src/registrar/models/domain.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/epplibwrapper/errors.py b/src/epplibwrapper/errors.py index 2b7bdd255..4ded1e5a7 100644 --- a/src/epplibwrapper/errors.py +++ b/src/epplibwrapper/errors.py @@ -62,9 +62,10 @@ class RegistryError(Exception): - 2501 - 2502 Something malicious or abusive may have occurred """ - def __init__(self, *args, code=None, **kwargs): + def __init__(self, *args, code=None, msg=None,**kwargs): super().__init__(*args, **kwargs) self.code = code + self.msg = msg def should_retry(self): return self.code == ErrorCode.COMMAND_FAILED diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 40d4befb5..6bafbab08 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2916,18 +2916,17 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): except RegistryError as err: # Using variables to get past the linter message1 = f"Cannot delete Domain when in state {obj.state}" - message2 = "This subdomain is being used as a hostname on another domain" # Human-readable mappings of ErrorCodes. Can be expanded. error_messages = { # noqa on these items as black wants to reformat to an invalid length ErrorCode.OBJECT_STATUS_PROHIBITS_OPERATION: message1, - ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION: message2, + ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION: err.msg, } message = "Cannot connect to the registry" if not err.is_connection_error(): # If nothing is found, will default to returned err - message = error_messages.get(err.code, err) + message = error_messages[err.code] self.message_user(request, f"Error deleting this Domain: {message}", messages.ERROR) except TransitionNotAllowed: if obj.state == Domain.State.DELETED: diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 348ccf3ad..f4922bfdd 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1066,7 +1066,7 @@ class Domain(TimeStampedModel, DomainHelper): if host.domain != self: logger.error("Host %s in use by another domain: %s", host.name, host.domain) raise RegistryError( - "Host in use by another domain: {}".format(host.domain), + msg="Host in use by another domain: {}".format(host.domain), code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION, ) From 3dbafb52207d2c64af201a736f53e510b123b5c8 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 5 Dec 2024 14:21:02 -0600 Subject: [PATCH 15/97] up log level --- ops/manifests/manifest-ms.yaml | 2 +- src/epplibwrapper/errors.py | 4 ++-- src/registrar/admin.py | 1 + src/registrar/models/domain.py | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/ops/manifests/manifest-ms.yaml b/ops/manifests/manifest-ms.yaml index 153ee5f08..ac46f5d92 100644 --- a/ops/manifests/manifest-ms.yaml +++ b/ops/manifests/manifest-ms.yaml @@ -20,7 +20,7 @@ applications: # Tell Django where it is being hosted DJANGO_BASE_URL: https://getgov-ms.app.cloud.gov # Tell Django how much stuff to log - DJANGO_LOG_LEVEL: INFO + DJANGO_LOG_LEVEL: DEBUG # default public site location GETGOV_PUBLIC_SITE_URL: https://get.gov # Flag to disable/enable features in prod environments diff --git a/src/epplibwrapper/errors.py b/src/epplibwrapper/errors.py index 4ded1e5a7..d30ae93ea 100644 --- a/src/epplibwrapper/errors.py +++ b/src/epplibwrapper/errors.py @@ -62,10 +62,10 @@ class RegistryError(Exception): - 2501 - 2502 Something malicious or abusive may have occurred """ - def __init__(self, *args, code=None, msg=None,**kwargs): + def __init__(self, *args, code=None, note=None,**kwargs): super().__init__(*args, **kwargs) self.code = code - self.msg = msg + self.note = note def should_retry(self): return self.code == ErrorCode.COMMAND_FAILED diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 6bafbab08..81e4772e5 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2916,6 +2916,7 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): except RegistryError as err: # Using variables to get past the linter message1 = f"Cannot delete Domain when in state {obj.state}" + message2 = f"This subdomain is being used as a hostname on another domain: {err.note}" # Human-readable mappings of ErrorCodes. Can be expanded. error_messages = { # noqa on these items as black wants to reformat to an invalid length diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index f4922bfdd..e3a2c910a 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1066,8 +1066,8 @@ class Domain(TimeStampedModel, DomainHelper): if host.domain != self: logger.error("Host %s in use by another domain: %s", host.name, host.domain) raise RegistryError( - msg="Host in use by another domain: {}".format(host.domain), code=ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION, + note=host.domain, ) nameservers = [n[0] for n in self.nameservers] From ed4e30c9597a30ef956d6aedf389eb5090bf8d75 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 5 Dec 2024 15:32:27 -0500 Subject: [PATCH 16/97] infrastructure --- .../includes/member_domains_edit_table.html | 115 ++++++++++++++++++ .../includes/member_domains_table.html | 2 +- .../portfolio_member_domains_edit.html | 51 ++++++++ 3 files changed, 167 insertions(+), 1 deletion(-) create mode 100644 src/registrar/templates/includes/member_domains_edit_table.html create mode 100644 src/registrar/templates/portfolio_member_domains_edit.html diff --git a/src/registrar/templates/includes/member_domains_edit_table.html b/src/registrar/templates/includes/member_domains_edit_table.html new file mode 100644 index 000000000..ff87dd999 --- /dev/null +++ b/src/registrar/templates/includes/member_domains_edit_table.html @@ -0,0 +1,115 @@ +{% load static %} + +{% if member %} + +{% else %} + +{% endif %} + +{% comment %} Stores the json endpoint in a url for easier access {% endcomment %} +{% url 'get_member_domains_edit_json' as url %} + +
+ +

+ Domains assigned to + {% if member %} + {{ member.email }} + {% else %} + {{ portfolio_invitation.email }} + {% endif %} +

+ +
+ +
+
+ +
+
+
+ + + + + +
+ diff --git a/src/registrar/templates/includes/member_domains_table.html b/src/registrar/templates/includes/member_domains_table.html index 77d9b9891..c95196c51 100644 --- a/src/registrar/templates/includes/member_domains_table.html +++ b/src/registrar/templates/includes/member_domains_table.html @@ -37,7 +37,7 @@
-
+
- diff --git a/src/registrar/views/__init__.py b/src/registrar/views/__init__.py index 9a9cb7856..10d8acf92 100644 --- a/src/registrar/views/__init__.py +++ b/src/registrar/views/__init__.py @@ -20,4 +20,5 @@ from .index import * from .portfolios import * from .transfer_user import TransferUserView from .member_domains_json import PortfolioMemberDomainsJson +from .member_domains_edit_json import PortfolioMemberDomainsEditJson from .portfolio_members_json import PortfolioMembersJson diff --git a/src/registrar/views/member_domains_edit_json.py b/src/registrar/views/member_domains_edit_json.py new file mode 100644 index 000000000..3d06a9d85 --- /dev/null +++ b/src/registrar/views/member_domains_edit_json.py @@ -0,0 +1,121 @@ +import logging +from django.http import JsonResponse +from django.core.paginator import Paginator +from django.shortcuts import get_object_or_404 +from django.views import View +from registrar.models import UserDomainRole, Domain, DomainInformation, User +from django.urls import reverse +from django.db.models import Q + +from registrar.models.domain_invitation import DomainInvitation +from registrar.views.utility.mixins import PortfolioMemberDomainsEditPermission + +logger = logging.getLogger(__name__) + + +class PortfolioMemberDomainsEditJson(PortfolioMemberDomainsEditPermission, View): + + def get(self, request): + """Given the current request, + get all domains that are associated with the portfolio, or + associated with the member/invited member""" + + domain_ids = self.get_domain_ids_from_request(request) + + objects = Domain.objects.filter(id__in=domain_ids).select_related("domain_info__sub_organization") + unfiltered_total = objects.count() + + objects = self.apply_search(objects, request) + objects = self.apply_sorting(objects, request) + + paginator = Paginator(objects, 10) + page_number = request.GET.get("page") + page_obj = paginator.get_page(page_number) + + domains = [self.serialize_domain(domain, request.user) for domain in page_obj.object_list] + + return JsonResponse( + { + "domains": domains, + "page": page_obj.number, + "num_pages": paginator.num_pages, + "has_previous": page_obj.has_previous(), + "has_next": page_obj.has_next(), + "total": paginator.count, + "unfiltered_total": unfiltered_total, + } + ) + + def get_domain_ids_from_request(self, request): + """Get domain ids from request. + + request.get.email - email address of invited member + request.get.member_id - member id of member + request.get.portfolio - portfolio id of portfolio + request.get.member_only - whether to return only domains associated with member + or to return all domains in the portfolio + """ + portfolio = request.GET.get("portfolio") + email = request.GET.get("email") + member_id = request.GET.get("member_id") + member_only = request.GET.get("member_only", "false").lower() in ["true", "1"] + if member_only: + if member_id: + member = get_object_or_404(User, pk=member_id) + domain_info_ids = DomainInformation.objects.filter(portfolio=portfolio).values_list( + "domain_id", flat=True + ) + user_domain_roles = UserDomainRole.objects.filter(user=member).values_list("domain_id", flat=True) + return domain_info_ids.intersection(user_domain_roles) + elif email: + domain_info_ids = DomainInformation.objects.filter(portfolio=portfolio).values_list( + "domain_id", flat=True + ) + domain_invitations = DomainInvitation.objects.filter(email=email).values_list("domain_id", flat=True) + return domain_info_ids.intersection(domain_invitations) + else: + domain_infos = DomainInformation.objects.filter(portfolio=portfolio) + return domain_infos.values_list("domain_id", flat=True) + logger.warning("Invalid search criteria, returning empty results list") + return [] + + def apply_search(self, queryset, request): + search_term = request.GET.get("search_term") + if search_term: + queryset = queryset.filter(Q(name__icontains=search_term)) + return queryset + + def apply_sorting(self, queryset, request): + sort_by = request.GET.get("sort_by", "name") + order = request.GET.get("order", "asc") + if order == "desc": + sort_by = f"-{sort_by}" + return queryset.order_by(sort_by) + + def serialize_domain(self, domain, user): + suborganization_name = None + try: + domain_info = domain.domain_info + if domain_info: + suborganization = domain_info.sub_organization + if suborganization: + suborganization_name = suborganization.name + except Domain.domain_info.RelatedObjectDoesNotExist: + domain_info = None + logger.debug(f"Issue in domains_json: We could not find domain_info for {domain}") + + # Check if there is a UserDomainRole for this domain and user + user_domain_role_exists = UserDomainRole.objects.filter(domain_id=domain.id, user=user).exists() + view_only = not user_domain_role_exists or domain.state in [Domain.State.DELETED, Domain.State.ON_HOLD] + return { + "id": domain.id, + "name": domain.name, + "expiration_date": domain.expiration_date, + "state": domain.state, + "state_display": domain.state_display(), + "get_state_help_text": domain.get_state_help_text(), + "action_url": reverse("domain", kwargs={"pk": domain.id}), + "action_label": ("View" if view_only else "Manage"), + "svg_icon": ("visibility" if view_only else "settings"), + "domain_info__sub_organization": suborganization_name, + } From 5355b9d3dc7920d75037af117603a6e4cf522443 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 5 Dec 2024 18:26:05 -0500 Subject: [PATCH 21/97] change col name --- src/registrar/assets/src/js/getgov/table-edit-member-domains.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/assets/src/js/getgov/table-edit-member-domains.js b/src/registrar/assets/src/js/getgov/table-edit-member-domains.js index b0cd8ce9a..706f15186 100644 --- a/src/registrar/assets/src/js/getgov/table-edit-member-domains.js +++ b/src/registrar/assets/src/js/getgov/table-edit-member-domains.js @@ -17,7 +17,7 @@ export class EditMemberDomainsTable extends BaseTable { const domain = dataObject; const row = document.createElement('tr'); row.innerHTML = ` - +
Date: Fri, 6 Dec 2024 12:39:29 -0500 Subject: [PATCH 22/97] updated javascript --- .../js/getgov/table-edit-member-domains.js | 71 ++++++++++++++++++- .../includes/member_domains_edit_table.html | 6 +- 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/src/registrar/assets/src/js/getgov/table-edit-member-domains.js b/src/registrar/assets/src/js/getgov/table-edit-member-domains.js index 706f15186..e4d76b953 100644 --- a/src/registrar/assets/src/js/getgov/table-edit-member-domains.js +++ b/src/registrar/assets/src/js/getgov/table-edit-member-domains.js @@ -6,6 +6,11 @@ export class EditMemberDomainsTable extends BaseTable { constructor() { super('edit-member-domain'); this.currentSortBy = 'name'; + this.initialDomainAssignments = []; + this.initialDomainAssignmentsOnlyMember = []; + this.addedDomains = []; + this.removedDomains = []; + this.initializeDomainAssignments(); } getBaseUrl() { return document.getElementById("get_member_domains_edit_json_url"); @@ -13,9 +18,49 @@ export class EditMemberDomainsTable extends BaseTable { getDataObjects(data) { return data.domains; } + getDomainAssignmentSearchParams(portfolio) { + let searchParams = new URLSearchParams(); + let emailValue = this.portfolioElement ? this.portfolioElement.getAttribute('data-email') : null; + let memberIdValue = this.portfolioElement ? this.portfolioElement.getAttribute('data-member-id') : null; + let memberOnly = true; + if (portfolio) + searchParams.append("portfolio", portfolio); + if (emailValue) + searchParams.append("email", emailValue); + if (memberIdValue) + searchParams.append("member_id", memberIdValue); + if (memberOnly) + searchParams.append("member_only", memberOnly); + return searchParams; + } + getSearchParams(page, sortBy, order, searchTerm, status, portfolio) { + let searchParams = super.getSearchParams(page, sortBy, order, searchTerm, status, portfolio); + if (this.addedDomains) + searchParams.append("addedDomainIds", this.addedDomains); + if (this.removedDomains) + searchParams.append("removedDomainIds", this.removedDomains); + return searchParams; + } addRow(dataObject, tbody, customTableOptions) { const domain = dataObject; const row = document.createElement('tr'); + //console.log("initialDomainAssignments: " + this.initialDomainAssignments); + //console.log("testing domain: " + domain.id); + let checked = false; + let disabled = false; + if ( + (this.initialDomainAssignments.includes(domain.id) || + this.addedDomains.map(obj => obj.id).includes(domain.id)) && + !this.removedDomains.map(obj => obj.id).includes(domain.id) + ) { + console.log("checked domain: " + domain.id); + checked = true; + } + if (this.initialDomainAssignmentsOnlyMember.includes(domain.id)) { + console.log("disabled domain: " + domain.id); + disabled = true; + } + row.innerHTML = `
@@ -25,6 +70,8 @@ export class EditMemberDomainsTable extends BaseTable { type="checkbox" name="${domain.name}" value="${domain.id}" + ${checked ? 'checked' : ''} + ${disabled ? 'disabled' : ''} />
- diff --git a/src/registrar/templates/includes/modal.html b/src/registrar/templates/includes/modal.html index 00c51cee0..625044585 100644 --- a/src/registrar/templates/includes/modal.html +++ b/src/registrar/templates/includes/modal.html @@ -23,18 +23,24 @@
    - {% if not_form %}
  • + {% if not_form and modal_button %} {{ modal_button }} -
    • - {% else %} -
  • + {% elif modal_button_url and modal_button_text %} + + {{ modal_button_text }} + + {% else %}
    {% csrf_token %} {{ modal_button }}
    -
    • - {% endif %} + {% endif %} +
  • {% comment %} The cancel button the DS form actually triggers a context change in the view, in addition to being a close modal hook {% endcomment %} diff --git a/src/registrar/templates/portfolio_member_domains_edit.html b/src/registrar/templates/portfolio_member_domains_edit.html index 98720ae46..d430e7572 100644 --- a/src/registrar/templates/portfolio_member_domains_edit.html +++ b/src/registrar/templates/portfolio_member_domains_edit.html @@ -44,5 +44,26 @@ {% include "includes/member_domains_edit_table.html" %} +
      +
    • + + +
      • +
    • + +
      • +
    +
{% endblock %} From 01e6296ed44cf45676d7c6614a8f987db6935178 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 6 Dec 2024 16:25:53 -0500 Subject: [PATCH 28/97] pagination and file cleanup --- .../js/getgov/table-edit-member-domains.js | 2 +- src/registrar/config/urls.py | 1 - .../includes/member_domains_edit_table.html | 4 +- src/registrar/views/__init__.py | 1 - .../views/member_domains_edit_json.py | 161 ------------------ src/registrar/views/member_domains_json.py | 69 +++++++- 6 files changed, 66 insertions(+), 172 deletions(-) delete mode 100644 src/registrar/views/member_domains_edit_json.py diff --git a/src/registrar/assets/src/js/getgov/table-edit-member-domains.js b/src/registrar/assets/src/js/getgov/table-edit-member-domains.js index 44dab50d3..24c34685e 100644 --- a/src/registrar/assets/src/js/getgov/table-edit-member-domains.js +++ b/src/registrar/assets/src/js/getgov/table-edit-member-domains.js @@ -15,7 +15,7 @@ export class EditMemberDomainsTable extends BaseTable { this.initCancelEditDomainAssignmentButton(); } getBaseUrl() { - return document.getElementById("get_member_domains_edit_json_url"); + return document.getElementById("get_member_domains_json_url"); } getDataObjects(data) { return data.domains; diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 1a13a4556..d906920bf 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -383,7 +383,6 @@ urlpatterns = [ path("get-domain-requests-json/", get_domain_requests_json, name="get_domain_requests_json"), path("get-portfolio-members-json/", views.PortfolioMembersJson.as_view(), name="get_portfolio_members_json"), path("get-member-domains-json/", views.PortfolioMemberDomainsJson.as_view(), name="get_member_domains_json"), - path("get-member-domains-edit-json/", views.PortfolioMemberDomainsEditJson.as_view(), name="get_member_domains_edit_json"), ] # Djangooidc strips out context data from that context, so we define a custom error diff --git a/src/registrar/templates/includes/member_domains_edit_table.html b/src/registrar/templates/includes/member_domains_edit_table.html index 325b646cd..198b85093 100644 --- a/src/registrar/templates/includes/member_domains_edit_table.html +++ b/src/registrar/templates/includes/member_domains_edit_table.html @@ -21,8 +21,8 @@ {% endif %} {% comment %} Stores the json endpoint in a url for easier access {% endcomment %} -{% url 'get_member_domains_edit_json' as url %} - +{% url 'get_member_domains_json' as url %} +

diff --git a/src/registrar/views/__init__.py b/src/registrar/views/__init__.py index 10d8acf92..9a9cb7856 100644 --- a/src/registrar/views/__init__.py +++ b/src/registrar/views/__init__.py @@ -20,5 +20,4 @@ from .index import * from .portfolios import * from .transfer_user import TransferUserView from .member_domains_json import PortfolioMemberDomainsJson -from .member_domains_edit_json import PortfolioMemberDomainsEditJson from .portfolio_members_json import PortfolioMembersJson diff --git a/src/registrar/views/member_domains_edit_json.py b/src/registrar/views/member_domains_edit_json.py deleted file mode 100644 index ddaca563b..000000000 --- a/src/registrar/views/member_domains_edit_json.py +++ /dev/null @@ -1,161 +0,0 @@ -import logging -from django.db import models -from django.http import JsonResponse -from django.core.paginator import Paginator -from django.shortcuts import get_object_or_404 -from django.views import View -from registrar.models import UserDomainRole, Domain, DomainInformation, User -from django.urls import reverse -from django.db.models import Q - -from registrar.models.domain_invitation import DomainInvitation -from registrar.views.utility.mixins import PortfolioMemberDomainsEditPermission - -logger = logging.getLogger(__name__) - - -class PortfolioMemberDomainsEditJson(PortfolioMemberDomainsEditPermission, View): - - def get(self, request): - """Given the current request, - get all domains that are associated with the portfolio, or - associated with the member/invited member""" - - domain_ids = self.get_domain_ids_from_request(request) - - objects = Domain.objects.filter(id__in=domain_ids).select_related("domain_info__sub_organization") - unfiltered_total = objects.count() - - objects = self.apply_search(objects, request) - objects = self.apply_sorting(objects, request) - - paginator = Paginator(objects, 10) - page_number = request.GET.get("page") - page_obj = paginator.get_page(page_number) - - member_id = request.GET.get("member_id") - domains = [self.serialize_domain(domain, member_id, request.user) for domain in page_obj.object_list] - - return JsonResponse( - { - "domains": domains, - "page": page_obj.number, - "num_pages": paginator.num_pages, - "has_previous": page_obj.has_previous(), - "has_next": page_obj.has_next(), - "total": paginator.count, - "unfiltered_total": unfiltered_total, - } - ) - - def get_domain_ids_from_request(self, request): - """Get domain ids from request. - - request.get.email - email address of invited member - request.get.member_id - member id of member - request.get.portfolio - portfolio id of portfolio - request.get.member_only - whether to return only domains associated with member - or to return all domains in the portfolio - """ - portfolio = request.GET.get("portfolio") - email = request.GET.get("email") - member_id = request.GET.get("member_id") - member_only = request.GET.get("member_only", "false").lower() in ["true", "1"] - if member_only: - if member_id: - member = get_object_or_404(User, pk=member_id) - domain_info_ids = DomainInformation.objects.filter(portfolio=portfolio).values_list( - "domain_id", flat=True - ) - user_domain_roles = UserDomainRole.objects.filter(user=member).values_list("domain_id", flat=True) - return domain_info_ids.intersection(user_domain_roles) - elif email: - domain_info_ids = DomainInformation.objects.filter(portfolio=portfolio).values_list( - "domain_id", flat=True - ) - domain_invitations = DomainInvitation.objects.filter(email=email).values_list("domain_id", flat=True) - return domain_info_ids.intersection(domain_invitations) - else: - domain_infos = DomainInformation.objects.filter(portfolio=portfolio) - return domain_infos.values_list("domain_id", flat=True) - logger.warning("Invalid search criteria, returning empty results list") - return [] - - def apply_search(self, queryset, request): - search_term = request.GET.get("search_term") - if search_term: - queryset = queryset.filter(Q(name__icontains=search_term)) - return queryset - - def apply_sorting(self, queryset, request): - # Get the sorting parameters from the request - sort_by = request.GET.get("sort_by", "name") - order = request.GET.get("order", "asc") - # Sort by 'checked' if specified, otherwise by the given field - if sort_by == "checked": - # Get list of checked ids from the request - checked_ids = request.GET.get("checkedDomainIds") - if checked_ids: - # Split the comma-separated string into a list of integers - checked_ids = [int(id.strip()) for id in checked_ids.split(",") if id.strip().isdigit()] - else: - # If no value is passed, set checked_ids to an empty list - checked_ids = [] - # Annotate each object with a 'checked' value based on whether its ID is in checkedIds - queryset = queryset.annotate( - checked=models.Case( - models.When(id__in=checked_ids, then=models.Value(True)), - default=models.Value(False), - output_field=models.BooleanField(), - ) - ) - # Add ordering logic for 'checked' - if order == "desc": - queryset = queryset.order_by("-checked","name") - else: - queryset = queryset.order_by("checked","name") - else: - # Handle other fields as normal - if order == "desc": - sort_by = f"-{sort_by}" - queryset = queryset.order_by(sort_by) - - return queryset - - - def serialize_domain(self, domain, member_id, user): - suborganization_name = None - try: - domain_info = domain.domain_info - if domain_info: - suborganization = domain_info.sub_organization - if suborganization: - suborganization_name = suborganization.name - except Domain.domain_info.RelatedObjectDoesNotExist: - domain_info = None - logger.debug(f"Issue in domains_json: We could not find domain_info for {domain}") - - # Check if there is a UserDomainRole for this domain and user - user_domain_role_exists = UserDomainRole.objects.filter(domain_id=domain.id, user=user).exists() - view_only = not user_domain_role_exists or domain.state in [Domain.State.DELETED, Domain.State.ON_HOLD] - - # Check if the specified member is the only member assigned as manager of domain - only_member_assigned_to_domain = False - if member_id: - member_domain_role_count = UserDomainRole.objects.filter(domain_id=domain.id).count() - member_domain_role_exists = UserDomainRole.objects.filter(domain_id=domain.id, user_id=member_id).exists() - only_member_assigned_to_domain = member_domain_role_exists and member_domain_role_count == 1 - - return { - "id": domain.id, - "name": domain.name, - "member_is_only_manager": only_member_assigned_to_domain, - "expiration_date": domain.expiration_date, - "state": domain.state, - "state_display": domain.state_display(), - "get_state_help_text": domain.get_state_help_text(), - "action_url": reverse("domain", kwargs={"pk": domain.id}), - "action_label": ("View" if view_only else "Manage"), - "svg_icon": ("visibility" if view_only else "settings"), - "domain_info__sub_organization": suborganization_name, - } diff --git a/src/registrar/views/member_domains_json.py b/src/registrar/views/member_domains_json.py index f37afdff0..cfe82251e 100644 --- a/src/registrar/views/member_domains_json.py +++ b/src/registrar/views/member_domains_json.py @@ -1,4 +1,5 @@ import logging +from django.db import models from django.http import JsonResponse from django.core.paginator import Paginator from django.shortcuts import get_object_or_404 @@ -28,11 +29,12 @@ class PortfolioMemberDomainsJson(PortfolioMemberDomainsPermission, View): objects = self.apply_search(objects, request) objects = self.apply_sorting(objects, request) - paginator = Paginator(objects, 10) + paginator = Paginator(objects, self.get_page_size(request)) page_number = request.GET.get("page") page_obj = paginator.get_page(page_number) - domains = [self.serialize_domain(domain, request.user) for domain in page_obj.object_list] + member_id = request.GET.get("member_id") + domains = [self.serialize_domain(domain, member_id, request.user) for domain in page_obj.object_list] return JsonResponse( { @@ -46,6 +48,23 @@ class PortfolioMemberDomainsJson(PortfolioMemberDomainsPermission, View): } ) + def get_page_size(self, request): + """Gets the page size. + + If member_only, need to return the entire result set every time, so need + to set to a very large page size. If not member_only, this can be adjusted + to provide a smaller page size""" + + member_only = request.GET.get("member_only", "false").lower() in ["true", "1"] + if member_only: + # This number needs to remain very high as the entire result set + # must be returned when member_only + return 1000 + else: + # This number can be adjusted if we want to add pagination to the result page + # later + return 1000 + def get_domain_ids_from_request(self, request): """Get domain ids from request. @@ -86,13 +105,42 @@ class PortfolioMemberDomainsJson(PortfolioMemberDomainsPermission, View): return queryset def apply_sorting(self, queryset, request): + # Get the sorting parameters from the request sort_by = request.GET.get("sort_by", "name") order = request.GET.get("order", "asc") - if order == "desc": - sort_by = f"-{sort_by}" - return queryset.order_by(sort_by) + # Sort by 'checked' if specified, otherwise by the given field + if sort_by == "checked": + # Get list of checked ids from the request + checked_ids = request.GET.get("checkedDomainIds") + if checked_ids: + # Split the comma-separated string into a list of integers + checked_ids = [int(id.strip()) for id in checked_ids.split(",") if id.strip().isdigit()] + else: + # If no value is passed, set checked_ids to an empty list + checked_ids = [] + # Annotate each object with a 'checked' value based on whether its ID is in checkedIds + queryset = queryset.annotate( + checked=models.Case( + models.When(id__in=checked_ids, then=models.Value(True)), + default=models.Value(False), + output_field=models.BooleanField(), + ) + ) + # Add ordering logic for 'checked' + if order == "desc": + queryset = queryset.order_by("-checked","name") + else: + queryset = queryset.order_by("checked","name") + else: + # Handle other fields as normal + if order == "desc": + sort_by = f"-{sort_by}" + queryset = queryset.order_by(sort_by) + + return queryset - def serialize_domain(self, domain, user): + + def serialize_domain(self, domain, member_id, user): suborganization_name = None try: domain_info = domain.domain_info @@ -107,9 +155,18 @@ class PortfolioMemberDomainsJson(PortfolioMemberDomainsPermission, View): # Check if there is a UserDomainRole for this domain and user user_domain_role_exists = UserDomainRole.objects.filter(domain_id=domain.id, user=user).exists() view_only = not user_domain_role_exists or domain.state in [Domain.State.DELETED, Domain.State.ON_HOLD] + + # Check if the specified member is the only member assigned as manager of domain + only_member_assigned_to_domain = False + if member_id: + member_domain_role_count = UserDomainRole.objects.filter(domain_id=domain.id).count() + member_domain_role_exists = UserDomainRole.objects.filter(domain_id=domain.id, user_id=member_id).exists() + only_member_assigned_to_domain = member_domain_role_exists and member_domain_role_count == 1 + return { "id": domain.id, "name": domain.name, + "member_is_only_manager": only_member_assigned_to_domain, "expiration_date": domain.expiration_date, "state": domain.state, "state_display": domain.state_display(), From 20f0a492aa28af6a1be38fd1c3793c8d0d0b74b0 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 6 Dec 2024 16:36:22 -0500 Subject: [PATCH 29/97] accounting for annotations in figma --- .../includes/member_domains_edit_table.html | 4 ++-- .../templates/includes/member_domains_table.html | 14 +++++--------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/registrar/templates/includes/member_domains_edit_table.html b/src/registrar/templates/includes/member_domains_edit_table.html index 198b85093..4eade2511 100644 --- a/src/registrar/templates/includes/member_domains_edit_table.html +++ b/src/registrar/templates/includes/member_domains_edit_table.html @@ -26,7 +26,7 @@

- Domains assigned to + Edit domains assigned to {% if member %} {{ member.email }} {% else %} @@ -36,7 +36,7 @@
-
+

{% with group_classes="usa-form-editable usa-form-editable--no-border padding-top-0" %} - {% input_with_errors form.admin_org_domain_request_permissions %} + {% input_with_errors form.domain_request_permissions_admin %} {% endwith %}

Organization members

{% with group_classes="usa-form-editable usa-form-editable--no-border padding-top-0" %} - {% input_with_errors form.admin_org_members_permissions %} + {% input_with_errors form.member_permissions_admin %} {% endwith %}

@@ -112,7 +94,7 @@

Organization domain requests

{% with group_classes="usa-form-editable usa-form-editable--no-border padding-top-0" %} - {% input_with_errors form.basic_org_domain_request_permissions %} + {% input_with_errors form.domain_request_permissions_member %} {% endwith %}
@@ -123,17 +105,11 @@ href="{% url 'members' %}" class="usa-button usa-button--outline" name="btn-cancel-click" - aria-label="Cancel adding new member" - >Cancel - - - + aria-label="Cancel editing member" + > + Cancel + + diff --git a/src/registrar/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index e88830156..6140130c8 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -282,3 +282,11 @@ def display_requesting_entity(domain_request): ) return display + + +@register.filter +def get_dict_value(dictionary, key): + """Get a value from a dictionary. Returns a string on empty.""" + if isinstance(dictionary, dict): + return dictionary.get(key, "") + return "" From 2730047588e8f75a02ac825b470f9a3130474a0c Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 10 Dec 2024 13:41:48 -0500 Subject: [PATCH 39/97] domain information changes done --- src/registrar/admin.py | 151 +++++++++++++++++- .../getgov-admin/domain-information-form.js | 12 +- .../helpers-portfolio-dynamic-fields.js | 24 +-- 3 files changed, 161 insertions(+), 26 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 0e8e4847a..7adb7e3ed 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -220,6 +220,14 @@ class DomainInformationAdminForm(forms.ModelForm): fields = "__all__" widgets = { "other_contacts": NoAutocompleteFilteredSelectMultiple("other_contacts", False), + "portfolio": AutocompleteSelectWithPlaceholder( + DomainRequest._meta.get_field("portfolio"), admin.site, attrs={"data-placeholder": "---------"} + ), + "sub_organization": AutocompleteSelectWithPlaceholder( + DomainRequest._meta.get_field("sub_organization"), + admin.site, + attrs={"data-placeholder": "---------", "ajax-url": "get-suborganization-list-json"}, + ), } @@ -1523,6 +1531,71 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): orderable_fk_fields = [("domain", "name")] + # Define methods to display fields from the related portfolio + def portfolio_senior_official(self, obj) -> Optional[SeniorOfficial]: + return obj.portfolio.senior_official if obj.portfolio and obj.portfolio.senior_official else None + + portfolio_senior_official.short_description = "Senior official" # type: ignore + + def portfolio_organization_type(self, obj): + return ( + DomainRequest.OrganizationChoices.get_org_label(obj.portfolio.organization_type) + if obj.portfolio and obj.portfolio.organization_type + else "-" + ) + + portfolio_organization_type.short_description = "Organization type" # type: ignore + + def portfolio_federal_type(self, obj): + return ( + BranchChoices.get_branch_label(obj.portfolio.federal_type) + if obj.portfolio and obj.portfolio.federal_type + else "-" + ) + + portfolio_federal_type.short_description = "Federal type" # type: ignore + + def portfolio_organization_name(self, obj): + return obj.portfolio.organization_name if obj.portfolio else "" + + portfolio_organization_name.short_description = "Organization name" # type: ignore + + def portfolio_federal_agency(self, obj): + return obj.portfolio.federal_agency if obj.portfolio else "" + + portfolio_federal_agency.short_description = "Federal agency" # type: ignore + + def portfolio_state_territory(self, obj): + return obj.portfolio.state_territory if obj.portfolio else "" + + portfolio_state_territory.short_description = "State, territory, or military post" # type: ignore + + def portfolio_address_line1(self, obj): + return obj.portfolio.address_line1 if obj.portfolio else "" + + portfolio_address_line1.short_description = "Address line 1" # type: ignore + + def portfolio_address_line2(self, obj): + return obj.portfolio.address_line2 if obj.portfolio else "" + + portfolio_address_line2.short_description = "Address line 2" # type: ignore + + def portfolio_city(self, obj): + return obj.portfolio.city if obj.portfolio else "" + + portfolio_city.short_description = "City" # type: ignore + + def portfolio_zipcode(self, obj): + return obj.portfolio.zipcode if obj.portfolio else "" + + portfolio_zipcode.short_description = "Zip code" # type: ignore + + def portfolio_urbanization(self, obj): + return obj.portfolio.urbanization if obj.portfolio else "" + + portfolio_urbanization.short_description = "Urbanization" # type: ignore + + # Filters list_filter = [GenericOrgFilter] @@ -1537,16 +1610,36 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): None, { "fields": [ - "portfolio", - "sub_organization", - "creator", "domain_request", "notes", ] }, ), + ( + "Requested by", + { + "fields": [ + "portfolio", + "sub_organization", + "creator", + ] + }, + ), (".gov domain", {"fields": ["domain"]}), - ("Contacts", {"fields": ["senior_official", "other_contacts", "no_other_contacts_rationale"]}), + ( + "Contacts", + { + "fields": [ + "senior_official", + "portfolio_senior_official", + "other_contacts", + "no_other_contacts_rationale", + "cisa_representative_first_name", + "cisa_representative_last_name", + "cisa_representative_email", + ] + }, + ), ("Background info", {"fields": ["anything_else"]}), ( "Type of organization", @@ -1595,10 +1688,58 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): ], }, ), + # the below three sections are for portfolio fields + ( + "Type of organization", + { + "fields": [ + "portfolio_organization_type", + "portfolio_federal_type", + ] + }, + ), + ( + "Organization name and mailing address", + { + "fields": [ + "portfolio_organization_name", + "portfolio_federal_agency", + ] + }, + ), + ( + "Show details", + { + "classes": ["collapse--dgfieldset"], + "description": "Extends organization name and mailing address", + "fields": [ + "portfolio_state_territory", + "portfolio_address_line1", + "portfolio_address_line2", + "portfolio_city", + "portfolio_zipcode", + "portfolio_urbanization", + ], + }, + ), ] # Readonly fields for analysts and superusers - readonly_fields = ("other_contacts", "is_election_board") + readonly_fields = ( + "portfolio_senior_official", + "portfolio_organization_type", + "portfolio_federal_type", + "portfolio_organization_name", + "portfolio_federal_agency", + "portfolio_state_territory", + "portfolio_address_line1", + "portfolio_address_line2", + "portfolio_city", + "portfolio_zipcode", + "portfolio_urbanization", + "other_contacts", + "is_election_board" + ) # Read only that we'll leverage for CISA Analysts analyst_readonly_fields = [ diff --git a/src/registrar/assets/src/js/getgov-admin/domain-information-form.js b/src/registrar/assets/src/js/getgov-admin/domain-information-form.js index 8139c752f..072b73720 100644 --- a/src/registrar/assets/src/js/getgov-admin/domain-information-form.js +++ b/src/registrar/assets/src/js/getgov-admin/domain-information-form.js @@ -1,17 +1,11 @@ -import { handleSuborganizationFields } from './helpers-portfolio-dynamic-fields.js'; +import { handlePortfolioSelection } from './helpers-portfolio-dynamic-fields.js'; /** - * A function for dynamic DomainInformation fields + * A function for dynamic DomainRequest fields */ export function initDynamicDomainInformationFields(){ const domainInformationPage = document.getElementById("domaininformation_form"); if (domainInformationPage) { - handleSuborganizationFields(); - } - - // DomainInformation is embedded inside domain so this should fire there too - const domainPage = document.getElementById("domain_form"); - if (domainPage) { - handleSuborganizationFields(portfolioDropdownSelector="#id_domain_info-0-portfolio", suborgDropdownSelector="#id_domain_info-0-sub_organization"); + handlePortfolioSelection(); } } diff --git a/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js b/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js index 39f30b87f..ca4c4b44c 100644 --- a/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js +++ b/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js @@ -24,13 +24,13 @@ export function handleSuborganizationFields( function toggleSuborganizationFields() { if (portfolioDropdown.val() && !suborganizationDropdown.val()) { - showElement(requestedSuborgField); - showElement(suborgCity); - showElement(suborgStateTerritory); + if (requestedSuborgField) showElement(requestedSuborgField); + if (suborgCity) showElement(suborgCity); + if (suborgStateTerritory) showElement(suborgStateTerritory); }else { - hideElement(requestedSuborgField); - hideElement(suborgCity); - hideElement(suborgStateTerritory); + if (requestedSuborgField) hideElement(requestedSuborgField); + if (suborgCity) hideElement(suborgCity); + if (suborgStateTerritory) hideElement(suborgStateTerritory); } } @@ -504,14 +504,14 @@ export function handlePortfolioSelection() { if (portfolio_id && !suborganization_id) { // Show suborganization request fields - showElement(requestedSuborganizationField); - showElement(suborganizationCity); - showElement(suborganizationStateTerritory); + if (requestedSuborganizationField) showElement(requestedSuborganizationField); + if (suborganizationCity) showElement(suborganizationCity); + if (suborganizationStateTerritory) showElement(suborganizationStateTerritory); } else { // Hide suborganization request fields if suborganization is selected - hideElement(requestedSuborganizationField); - hideElement(suborganizationCity); - hideElement(suborganizationStateTerritory); + if (requestedSuborganizationField) hideElement(requestedSuborganizationField); + if (suborganizationCity) hideElement(suborganizationCity); + if (suborganizationStateTerritory) hideElement(suborganizationStateTerritory); } } From 787506989742024344ebbb526e8c61ccbae8b223 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 10 Dec 2024 15:37:18 -0500 Subject: [PATCH 40/97] fixed rest of domain information --- src/registrar/admin.py | 4 ++-- .../django/admin/domain_information_change_form.html | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 7adb7e3ed..30b49f17e 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -221,10 +221,10 @@ class DomainInformationAdminForm(forms.ModelForm): widgets = { "other_contacts": NoAutocompleteFilteredSelectMultiple("other_contacts", False), "portfolio": AutocompleteSelectWithPlaceholder( - DomainRequest._meta.get_field("portfolio"), admin.site, attrs={"data-placeholder": "---------"} + DomainInformation._meta.get_field("portfolio"), admin.site, attrs={"data-placeholder": "---------"} ), "sub_organization": AutocompleteSelectWithPlaceholder( - DomainRequest._meta.get_field("sub_organization"), + DomainInformation._meta.get_field("sub_organization"), admin.site, attrs={"data-placeholder": "---------", "ajax-url": "get-suborganization-list-json"}, ), diff --git a/src/registrar/templates/django/admin/domain_information_change_form.html b/src/registrar/templates/django/admin/domain_information_change_form.html index c5b0d54b8..487fd97e1 100644 --- a/src/registrar/templates/django/admin/domain_information_change_form.html +++ b/src/registrar/templates/django/admin/domain_information_change_form.html @@ -1,6 +1,13 @@ {% extends 'admin/change_form.html' %} {% load i18n static %} +{% block content %} + {% comment %} Stores the json endpoint in a url for easier access {% endcomment %} + {% url 'get-portfolio-json' as url %} + + {{ block.super }} +{% endblock content %} + {% block field_sets %} {% for fieldset in adminform %} {% comment %} From bc3a96aa87d1d6216a1fcb0105d61b6fcb445353 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 10 Dec 2024 13:50:30 -0700 Subject: [PATCH 41/97] fine comb --- src/registrar/admin.py | 1 + .../src/js/getgov/portfolio-member-page.js | 2 +- src/registrar/context_processors.py | 2 + src/registrar/forms/portfolio.py | 99 ++++++++++--------- .../django/forms/widgets/multiple_input.html | 2 +- .../portfolio_member_permissions.html | 15 +-- src/registrar/views/portfolios.py | 2 +- 7 files changed, 65 insertions(+), 58 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 0e8e4847a..144d1fcab 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -3791,6 +3791,7 @@ class WaffleFlagAdmin(FlagAdmin): if extra_context is None: extra_context = {} extra_context["dns_prototype_flag"] = flag_is_active_for_user(request.user, "dns_prototype_flag") + extra_context["organization_member"] = flag_is_active_for_user(request.user, "organization_member") return super().changelist_view(request, extra_context=extra_context) diff --git a/src/registrar/assets/src/js/getgov/portfolio-member-page.js b/src/registrar/assets/src/js/getgov/portfolio-member-page.js index ac0b7cffe..98bcf7d03 100644 --- a/src/registrar/assets/src/js/getgov/portfolio-member-page.js +++ b/src/registrar/assets/src/js/getgov/portfolio-member-page.js @@ -49,7 +49,7 @@ export function initPortfolioMemberPageToggle() { * on the Add New Member page. */ export function initAddNewMemberPageListeners() { - add_member_form = document.getElementById("add_member_form") + let add_member_form = document.getElementById("add_member_form") if (!add_member_form){ return; } diff --git a/src/registrar/context_processors.py b/src/registrar/context_processors.py index c1547ad88..5a526f86f 100644 --- a/src/registrar/context_processors.py +++ b/src/registrar/context_processors.py @@ -107,6 +107,8 @@ def is_widescreen_mode(request): "/no-organization-requests/", "/no-organization-domains/", "/domain-request/", + # "/members/", + # "/member/" ] is_widescreen = any(path in request.path for path in widescreen_paths) or request.path == "/" is_portfolio_widescreen = bool( diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py index 65911200b..92fd23906 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -4,7 +4,7 @@ import logging from django import forms from django.core.validators import RegexValidator from django.core.validators import MaxLengthValidator - +from django.utils.safestring import mark_safe from registrar.models import ( PortfolioInvitation, UserPortfolioPermission, @@ -109,13 +109,13 @@ class PortfolioSeniorOfficialForm(forms.ModelForm): cleaned_data.pop("full_name", None) return cleaned_data - -class BasePortfolioMemberForm(forms.ModelForm): +class BasePortfolioMemberForm(forms.Form): + required_star = '*' + role = forms.ChoiceField( - label="Select permission", choices=[ - (UserPortfolioRoleChoices.ORGANIZATION_ADMIN.value, "Admin Access"), - (UserPortfolioRoleChoices.ORGANIZATION_MEMBER.value, "Basic Access") + (UserPortfolioRoleChoices.ORGANIZATION_ADMIN.value, "Admin access"), + (UserPortfolioRoleChoices.ORGANIZATION_MEMBER.value, "Basic access") ], widget=forms.RadioSelect, required=True, @@ -123,12 +123,12 @@ class BasePortfolioMemberForm(forms.ModelForm): "required": "Member access level is required", }, ) - # Permissions for admins + domain_request_permissions_admin = forms.ChoiceField( - label="Select permission", + label=mark_safe(f"Select permission {required_star}"), choices=[ (UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, "View all requests"), - (UserPortfolioPermissionChoices.EDIT_REQUESTS.value, "Create and edit requests") + (UserPortfolioPermissionChoices.EDIT_REQUESTS.value, "View all requests plus create requests"), ], widget=forms.RadioSelect, required=False, @@ -136,11 +136,12 @@ class BasePortfolioMemberForm(forms.ModelForm): "required": "Admin domain request permission is required", }, ) + member_permissions_admin = forms.ChoiceField( - label="Select permission", + label=mark_safe(f"Select permission {required_star}"), choices=[ (UserPortfolioPermissionChoices.VIEW_MEMBERS.value, "View all members"), - (UserPortfolioPermissionChoices.EDIT_MEMBERS.value, "Create and edit members") + (UserPortfolioPermissionChoices.EDIT_MEMBERS.value, "View all members plus manage members"), ], widget=forms.RadioSelect, required=False, @@ -148,11 +149,13 @@ class BasePortfolioMemberForm(forms.ModelForm): "required": "Admin member permission is required", }, ) + domain_request_permissions_member = forms.ChoiceField( - label="Select permission", + label=mark_safe(f"Select permission {required_star}"), choices=[ - (UserPortfolioPermissionChoices.VIEW_MEMBERS.value, "View all members"), - (UserPortfolioPermissionChoices.EDIT_MEMBERS.value, "Create and edit members") + (UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, "View all requests"), + (UserPortfolioPermissionChoices.EDIT_REQUESTS.value, "View all requests plus create requests"), + ("no_access", "No access"), ], widget=forms.RadioSelect, required=False, @@ -161,8 +164,6 @@ class BasePortfolioMemberForm(forms.ModelForm): }, ) - # this form dynamically shows/hides some fields, depending on what - # was selected prior. This toggles which field is required or not. ROLE_REQUIRED_FIELDS = { UserPortfolioRoleChoices.ORGANIZATION_ADMIN: [ "domain_request_permissions_admin", @@ -173,10 +174,19 @@ class BasePortfolioMemberForm(forms.ModelForm): ], } + def __init__(self, *args, instance=None, **kwargs): + self.instance = instance + # If we have an instance, set initial + if instance: + kwargs['initial'] = self._map_instance_to_form(instance) + + super().__init__(*args, **kwargs) + def _map_instance_to_form(self, instance): """Maps model instance data to form fields""" if not instance: return {} + mapped_data = {} # Map roles with priority for admin if instance.roles: @@ -192,37 +202,16 @@ class BasePortfolioMemberForm(forms.ModelForm): mapped_data['domain_request_permissions_admin'] = UserPortfolioPermissionChoices.EDIT_REQUESTS.value elif UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value in perms: mapped_data['domain_request_permissions_admin'] = UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value + else: + mapped_data["member_permissions_admin"] = "no_access" if UserPortfolioPermissionChoices.EDIT_MEMBERS.value in perms: mapped_data['member_permissions_admin'] = UserPortfolioPermissionChoices.EDIT_MEMBERS.value elif UserPortfolioPermissionChoices.VIEW_MEMBERS.value in perms: mapped_data['member_permissions_admin'] = UserPortfolioPermissionChoices.VIEW_MEMBERS.value - + return mapped_data - def _map_form_to_instance(self, instance): - """Maps form data to model instance""" - if not self.is_valid(): - return - - role = self.cleaned_data.get("role") - domain_request_permissions_member = self.cleaned_data.get("domain_request_permissions_member") - domain_request_permissions_admin = self.cleaned_data.get('domain_request_permissions_admin') - member_permissions_admin = self.cleaned_data.get('member_permissions_admin') - - instance.roles = [role] - additional_permissions = [] - if domain_request_permissions_member: - additional_permissions.append(domain_request_permissions_member) - elif domain_request_permissions_admin: - additional_permissions.append(domain_request_permissions_admin) - - if member_permissions_admin: - additional_permissions.append(member_permissions_admin) - - instance.additional_permissions = additional_permissions - return instance - def clean(self): cleaned_data = super().clean() role = cleaned_data.get("role") @@ -239,6 +228,27 @@ class BasePortfolioMemberForm(forms.ModelForm): return cleaned_data + def save(self): + """Save the form data to the instance""" + if not self.instance: + raise ValueError("Cannot save form without instance") + + role = self.cleaned_data.get("role") + self.instance.roles = [self.cleaned_data["role"]] + + additional_permissions = [] + if self.cleaned_data.get("domain_request_permissions_member") and self.cleaned_data["domain_request_permissions_member"] != "no_access": + additional_permissions.append(self.cleaned_data["domain_request_permissions_member"]) + elif self.cleaned_data.get("domain_request_permissions_admin"): + additional_permissions.append(self.cleaned_data["domain_request_permissions_admin"]) + + if self.cleaned_data.get("member_permissions_admin"): + additional_permissions.append(self.cleaned_data["member_permissions_admin"]) + self.instance.additional_permissions = additional_permissions + + self.instance.save() + return self.instance + class PortfolioMemberForm(BasePortfolioMemberForm): """ @@ -250,6 +260,7 @@ class PortfolioMemberForm(BasePortfolioMemberForm): "roles", "additional_permissions", ] + def __init__(self, *args, instance=None, **kwargs): super().__init__(*args, **kwargs) self.fields['role'].descriptions = { @@ -258,14 +269,6 @@ class PortfolioMemberForm(BasePortfolioMemberForm): } self.instance = instance self.initial = self._map_instance_to_form(self.instance) - - def save(self): - """Save form data to instance""" - if not self.instance: - self.instance = self.Meta.model() - self._map_form_to_instance(self.instance) - self.instance.save() - return self.instance class PortfolioInvitedMemberForm(BasePortfolioMemberForm): diff --git a/src/registrar/templates/django/forms/widgets/multiple_input.html b/src/registrar/templates/django/forms/widgets/multiple_input.html index 76e19b169..cc0e11989 100644 --- a/src/registrar/templates/django/forms/widgets/multiple_input.html +++ b/src/registrar/templates/django/forms/widgets/multiple_input.html @@ -21,7 +21,7 @@ {% if field and field.field and field.field.descriptions %} {% with description=field.field.descriptions|get_dict_value:option.value %} {% if description %} -

{{ description }}

+

{{ description }}

{% endif %} {% endwith %} {% endif %} diff --git a/src/registrar/templates/portfolio_member_permissions.html b/src/registrar/templates/portfolio_member_permissions.html index a5f1731d0..f1db5941c 100644 --- a/src/registrar/templates/portfolio_member_permissions.html +++ b/src/registrar/templates/portfolio_member_permissions.html @@ -9,6 +9,7 @@ {% endblock %} {% block portfolio_content %} +{% include "includes/form_errors.html" with form=form %}