diff --git a/docs/developer/registry-access.md b/docs/developer/registry-access.md index c7737d5bc..75f965609 100644 --- a/docs/developer/registry-access.md +++ b/docs/developer/registry-access.md @@ -50,11 +50,11 @@ request = commands.InfoContact(id='sh8013') ``` DF = common.DiscloseField di = common.Disclose(flag=False, fields={DF.FAX, DF.VOICE, DF.ADDR}, types={DF.ADDR: "loc"}) -addr = common.ContactAddr(street=['123 Example Dr.'], city='Dulles', pc='20166-6503', cc='US', sp='VA') +addr = common.ContactAddr(street=['123 Example Dr.',None ,None], city='Dulles', pc='20166-6503', cc='US', sp='VA') pi = common.PostalInfo(name='John Doe', addr=addr, org="Example Inc.", type="loc") ai = common.ContactAuthInfo(pw='feedabee') -request = commands.CreateContact(id='sh8013', postal_info=pi, email='jdoe@example.com', voice='+1.7035555555', fax='+1.7035555556', auth_info=ai, disclose=di, vat=None, ident=None, notify_email=None) +request = commands.CreateContact(id='1234ab', postal_info=pi, email='jdoe@example.com', voice='+1.7035555555', fax='+1.7035555556', auth_info=ai, disclose=di, vat=None, ident=None, notify_email=None) ``` ### Create a new domain diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 3d322ccae..074bf8d80 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -175,7 +175,7 @@ class DomainAdmin(ListHeaderAdmin): change_form_template = "django/admin/domain_change_form.html" readonly_fields = ["state"] - def response_change(self, request, obj): + def response_change(self, request, obj): # noqa GET_SECURITY_EMAIL = "_get_security_email" SET_SECURITY_CONTACT = "_set_security_contact" MAKE_DOMAIN = "_make_domain_in_registry" @@ -246,7 +246,7 @@ class DomainAdmin(ListHeaderAdmin): else: self.message_user( request, - ("The security email is %" ". Thanks!") % fake_email, + "The security email is %s. Thanks!" % fake_email, ) elif MAKE_DOMAIN in request.POST: diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index a8cea0177..f710b809e 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -108,7 +108,8 @@ class Domain(TimeStampedModel, DomainHelper): # the state is indeterminate UNKNOWN = "unknown" - # The domain object exists in the registry but nameservers don't exist for it yet + # The domain object exists in the registry + # but nameservers don't exist for it yet DNS_NEEDED = "dns needed" # Domain has had nameservers set, may or may not be active @@ -229,8 +230,8 @@ class Domain(TimeStampedModel, DomainHelper): hosts = self._get_property("hosts") except Exception as err: # Don't throw error as this is normal for a new domain - logger.info("Domain is missing nameservers") - return None + logger.info("Domain is missing nameservers %s" % err) + return [] hostList = [] for host in hosts: @@ -250,7 +251,8 @@ class Domain(TimeStampedModel, DomainHelper): return response.res_data[0].avail except RegistryError as err: logger.warning( - "Couldn't check hosts %. Errorcode was %s, error was %s" % (hostnames), + "Couldn't check hosts %s. Errorcode was %s, error was %s", + hostnames, err.code, err, ) @@ -262,7 +264,7 @@ class Domain(TimeStampedModel, DomainHelper): doesn't add the created host to the domain returns ErrorCode (int)""" logger.info("Creating host") - if not addrs is None: + if addrs is not None: addresses = [epp.Ip(addr=addr) for addr in addrs] request = commands.CreateHost(name=host, addrs=addresses) else: @@ -318,11 +320,13 @@ class Domain(TimeStampedModel, DomainHelper): try: self.ready() self.save() - except: + except Exception as err: logger.info( - "nameserver setter checked for create state and it did not succeed" + "nameserver setter checked for create state " + "and it did not succeed. Error: %s" % err ) - ##TODO - handle removed nameservers here will need to change the state go back to DNS_NEEDED + # TODO - handle removed nameservers here will need to change the state + # then go back to DNS_NEEDED @Cache def statuses(self) -> list[str]: @@ -333,9 +337,9 @@ class Domain(TimeStampedModel, DomainHelper): """ # implementation note: the Status object from EPP stores the string in # a dataclass property `state`, not to be confused with the `state` field here - if not "statuses" in self._cache: + if "statuses" not in self._cache: self._fetch_cache() - if not "statuses" in self._cache: + if "statuses" not in self._cache: raise Exception("Can't retreive status from domain info") else: return self._cache["statuses"] @@ -354,7 +358,8 @@ class Domain(TimeStampedModel, DomainHelper): @registrant_contact.setter # type: ignore def registrant_contact(self, contact: PublicContact): - """Registrant is set when a domain is created, so follow on additions will update the current registrant""" + """Registrant is set when a domain is created, + so follow on additions will update the current registrant""" logger.info("making registrant contact") self._set_singleton_contact( @@ -383,16 +388,19 @@ class Domain(TimeStampedModel, DomainHelper): return contact def _update_epp_contact(self, contact: PublicContact): - """Sends UpdateContact to update the actual contact object, domain object remains unaffected - should be used when changing email address or other contact infor on an existing domain + """Sends UpdateContact to update the actual contact object, + domain object remains unaffected + should be used when changing email address + or other contact info on an existing domain """ updateContact = commands.UpdateContact( id=contact.registry_id, + # type: ignore postal_info=self._make_epp_contact_postal_info(contact=contact), email=contact.email, voice=contact.voice, fax=contact.fax, - ) + ) # type: ignore try: registry.send(updateContact, cleaned=True) @@ -403,9 +411,7 @@ class Domain(TimeStampedModel, DomainHelper): # TODO - ticket 433 human readable error handling here def _update_domain_with_contact(self, contact: PublicContact, rem=False): - # TODO - consider making this use both add and rem at the same time, separating it out may not be needed - # good addition for ticket 850 - + """adds or removes a contact from a domain""" logger.info( "_update_domain_with_contact() received type %s " % contact.contact_type ) @@ -448,12 +454,12 @@ class Domain(TimeStampedModel, DomainHelper): return contact except Exception as err: # use better error handling - logger.info("Couldn't get contact") - ## send public contact to the thingy + logger.info("Couldn't get contact %s" % err) - ##TODO - remove this! ideally it should return None, but error handling needs to be + # TODO - remove this! ideally it should return None, + # but error handling needs to be # added on the security email page so that it can handle it being none - return self.get_default_security_contact() + return self.get_default_security_contact() def _add_registrant_to_existing_domain(self, contact: PublicContact): self._update_epp_contact(contact=contact) @@ -470,17 +476,18 @@ class Domain(TimeStampedModel, DomainHelper): ) # TODO-error handling better here? - def _set_singleton_contact(self, contact: PublicContact, expectedType: str): + def _set_singleton_contact(self, contact: PublicContact, expectedType: str): # noqa """Sets the contacts by adding them to the registry as new contacts, updates the contact if it is already in epp, deletes any additional contacts of the matching type for this domain does not create the PublicContact object, this should be made beforehand - (call save() on a public contact to trigger the contact setters which call this function) + (call save() on a public contact to trigger the contact setters + which inturn call this function) Raises ValueError if expected type doesn't match the contact type""" if expectedType != contact.contact_type: raise ValueError( - "Cannot set a contact with a different contact type, expected type was %s" - % expectedType + "Cannot set a contact with a different contact type," + " expected type was %s" % expectedType ) isRegistrant = contact.contact_type == contact.ContactTypeChoices.REGISTRANT @@ -489,7 +496,8 @@ class Domain(TimeStampedModel, DomainHelper): and contact.email == "" ) - # get publicContact objects that have the matching domain and type but a different id + # get publicContact objects that have the matching + # domain and type but a different id # like in highlander we there can only be one hasOtherContact = ( PublicContact.objects.exclude(registry_id=contact.registry_id) @@ -497,7 +505,7 @@ class Domain(TimeStampedModel, DomainHelper): .exists() ) - ##if no record exists with this contact type + # if no record exists with this contact type # make contact in registry, duplicate and errors handled there errorCode = self._make_contact_in_registry(contact) @@ -517,7 +525,7 @@ class Domain(TimeStampedModel, DomainHelper): # if has conflicting contacts in our db remove them if hasOtherContact: logger.info( - "_set_singleton_contact()-> updating domain by removing old contact and adding new one" + "_set_singleton_contact()-> updating domain, removing old contact" ) if isEmptySecurity: existing_contact = PublicContact.objects.filter( @@ -547,7 +555,8 @@ class Domain(TimeStampedModel, DomainHelper): ) raise (err) - # TODO- This could switch to just creating a list of ones to remove and a list of ones to add + # TODO- This could switch to just creating a + # list of ones to remove and a list of ones to add # or Change it to add contacts before deleting the old ones # update domain with contact or update the contact itself @@ -603,7 +612,8 @@ class Domain(TimeStampedModel, DomainHelper): ) def is_active(self) -> bool: - """Currently just returns if the state is created, because then it should be live, theoretically. + """Currently just returns if the state is created, + because then it should be live, theoretically. Post mvp this should indicate Is the domain live on the inter webs? could be replaced with request to see if ok status is set @@ -736,7 +746,7 @@ class Domain(TimeStampedModel, DomainHelper): def addRegistrant(self): registrant = PublicContact.get_default_registrant() registrant.domain = self - registrant.save() ##calls the registrant_contact.setter + registrant.save() # calls the registrant_contact.setter return registrant.registry_id @transition(field="state", source=State.UNKNOWN, target=State.DNS_NEEDED) @@ -754,7 +764,7 @@ class Domain(TimeStampedModel, DomainHelper): ) try: - response = registry.send(req, cleaned=True) + registry.send(req, cleaned=True) except RegistryError as err: if err.code != ErrorCode.OBJECT_EXISTS: @@ -777,7 +787,7 @@ class Domain(TimeStampedModel, DomainHelper): @transition(field="state", source=State.DNS_NEEDED, target=State.ON_HOLD) def clientHold(self): """place a clienthold on a domain (no longer should resolve)""" - ##TODO - check to see if client hold is allowed should happen outside of this function + # TODO - ensure all requirements for client hold are made here # (check prohibited statuses) logger.info("clientHold()-> inside clientHold") self._place_client_hold() @@ -786,8 +796,7 @@ class Domain(TimeStampedModel, DomainHelper): @transition(field="state", source=State.ON_HOLD, target=State.DNS_NEEDED) def revertClientHold(self): """undo a clienthold placed on a domain""" - ##TODO - check to see if client hold is allowed should happen outside of this function - # (check prohibited statuses) + logger.info("clientHold()-> inside clientHold") self._remove_client_hold() # TODO -on the client hold ticket any additional error handling here @@ -795,6 +804,10 @@ class Domain(TimeStampedModel, DomainHelper): @transition(field="state", source=State.ON_HOLD, target=State.DELETED) def deleted(self): """domain is deleted in epp but is saved in our database""" + # TODO Domains may not be deleted if: + # a child host is being used by + # another .gov domains. The host must be first removed + # and/or renamed before the parent domain may be deleted. logger.info("pendingCreate()-> inside pending create") self._delete_domain() # TODO - delete ticket any additional error handling here @@ -806,9 +819,11 @@ class Domain(TimeStampedModel, DomainHelper): ) def ready(self): """Transition to the ready state - domain should have nameservers and all contacts and now should be considered live on a domain + domain should have nameservers and all contacts + and now should be considered live on a domain """ - # TODO - in nameservers tickets 848 and 562 check here if updates need to be made + # TODO - in nameservers tickets 848 and 562 + # check here if updates need to be made nameserverList = self.nameservers logger.info("Changing to ready state") if len(nameserverList) < 2 or len(nameserverList) > 13: @@ -833,7 +848,7 @@ class Domain(TimeStampedModel, DomainHelper): types={DF.ADDR: "loc"}, ) - def _make_epp_contact_postal_info(self, contact: PublicContact): + def _make_epp_contact_postal_info(self, contact: PublicContact): # type: ignore return epp.PostalInfo( # type: ignore name=contact.name, addr=epp.ContactAddr( @@ -841,7 +856,7 @@ class Domain(TimeStampedModel, DomainHelper): getattr(contact, street) for street in ["street1", "street2", "street3"] if hasattr(contact, street) - ], + ], # type: ignore city=contact.city, pc=contact.pc, cc=contact.cc, @@ -862,7 +877,7 @@ class Domain(TimeStampedModel, DomainHelper): voice=contact.voice, fax=contact.fax, auth_info=epp.ContactAuthInfo(pw="2fooBAR123fooBaz"), - ) + ) # type: ignore # security contacts should only show email addresses, for now create.disclose = self._disclose_fields(contact=contact) try: @@ -872,7 +887,10 @@ class Domain(TimeStampedModel, DomainHelper): # don't throw an error if it is just saying this is a duplicate contact if err.code != ErrorCode.OBJECT_EXISTS: logger.error( - "Registry threw error for contact id %s contact type is %s, error code is\n %s full error is %s", + "Registry threw error for contact id %s" + " contact type is %s," + " error code is\n %s" + " full error is %s", contact.registry_id, contact.contact_type, err.code, @@ -907,7 +925,10 @@ class Domain(TimeStampedModel, DomainHelper): return self._request_contact_info(contact=contact) else: logger.error( - "Registry threw error for contact id %s contact type is %s, error code is\n %s full error is %s", + "Registry threw error for contact id %s" + " contact type is %s," + " error code is\n %s" + " full error is %s", contact.registry_id, contact.contact_type, e.code, @@ -994,7 +1015,8 @@ class Domain(TimeStampedModel, DomainHelper): and isinstance(cleaned["_hosts"], list) and len(cleaned["_hosts"]) ): - ##TODO- add elif in cache set it to be the old cache value, no point in removing + # TODO- add elif in cache set it to be the old cache value + # no point in removing cleaned["hosts"] = [] for name in cleaned["_hosts"]: # we do not use _get_or_create_* because we expect the object we diff --git a/src/registrar/models/public_contact.py b/src/registrar/models/public_contact.py index 8edcd2fc1..c620c7f89 100644 --- a/src/registrar/models/public_contact.py +++ b/src/registrar/models/public_contact.py @@ -150,4 +150,8 @@ class PublicContact(TimeStampedModel): ) def __str__(self): - return f"{self.name} <{self.email}> id: {self.registry_id} type: {self.contact_type}" + return ( + f"{self.name} <{self.email}>" + "id: {self.registry_id} " + "type: {self.contact_type}" + ) diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html index dd176c862..0eb88d71e 100644 --- a/src/registrar/templates/domain_detail.html +++ b/src/registrar/templates/domain_detail.html @@ -6,7 +6,7 @@
{% url 'domain-nameservers' pk=domain.id as url %} - {% if domain.nameservers %} + {% if domain.nameservers!==[] %} {% include "includes/summary_item.html" with title='DNS name servers' value=domain.nameservers list='true' edit_link=url %} {% else %}

DNS name servers

diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index f28a32074..26d4ef10d 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -12,7 +12,6 @@ from registrar.models import Domain # add in DomainApplication, User, from unittest import skip from epplibwrapper import commands, common, RegistryError, ErrorCode from registrar.models.domain_application import DomainApplication -from registrar.models.domain_information import DomainInformation from registrar.models.draft_domain import DraftDomain from registrar.models.public_contact import PublicContact from registrar.models.user import User @@ -88,18 +87,19 @@ class MockEppLib(TestCase): fields=fields, types={DF.ADDR: "loc"}, ) + # check docs here looks like we may have more than one address field but addr = common.ContactAddr( - street=[ - contact.street1, - contact.street2, - contact.street3, - ], + [ + getattr(contact, street) + for street in ["street1", "street2", "street3"] + if hasattr(contact, street) + ], # type: ignore city=contact.city, pc=contact.pc, cc=contact.cc, sp=contact.sp, - ) + ) # type: ignore pi = common.PostalInfo( name=contact.name, @@ -107,11 +107,12 @@ class MockEppLib(TestCase): org=contact.org, type="loc", ) + ai = common.ContactAuthInfo(pw="2fooBAR123fooBaz") if createContact: return commands.CreateContact( id=contact.registry_id, - postal_info=pi, + postal_info=pi, # type: ignore email=contact.email, voice=contact.voice, fax=contact.fax, @@ -120,7 +121,7 @@ class MockEppLib(TestCase): vat=None, ident=None, notify_email=None, - ) + ) # type: ignore else: return commands.UpdateContact( id=contact.registry_id, @@ -315,14 +316,14 @@ class TestRegistrantContacts(MockEppLib): self.domain.pendingCreate() - assert self.mockedSendFunction.call_count == 8 - assert PublicContact.objects.filter(domain=self.domain).count() == 4 - assert ( + self.assertEqual(self.mockedSendFunction.call_count, 8) + self.assertEqual(PublicContact.objects.filter(domain=self.domain).count(), 4) + self.assertEqual( PublicContact.objects.get( domain=self.domain, contact_type=PublicContact.ContactTypeChoices.SECURITY, - ).email - == expectedSecContact.email + ).email, + expectedSecContact.email, ) id = PublicContact.objects.get( @@ -356,7 +357,7 @@ class TestRegistrantContacts(MockEppLib): created contact of type 'security' """ # make a security contact that is a PublicContact - self.domain.pendingCreate() ##make sure a security email already exists + self.domain.pendingCreate() # make sure a security email already exists expectedSecContact = PublicContact.get_default_security() expectedSecContact.domain = self.domain expectedSecContact.email = "newEmail@fake.com" @@ -386,7 +387,7 @@ class TestRegistrantContacts(MockEppLib): domain=self.domain, contact_type=PublicContact.ContactTypeChoices.SECURITY ) - assert receivedSecurityContact == expectedSecContact + self.assertEqual(receivedSecurityContact, expectedSecContact) self.mockedSendFunction.assert_any_call(expectedCreateCommand, cleaned=True) self.mockedSendFunction.assert_any_call(expectedUpdateDomain, cleaned=True) @@ -397,7 +398,7 @@ class TestRegistrantContacts(MockEppLib): to the registry twice with identical data Then no errors are raised in Domain """ - # self.domain.pendingCreate() ##make sure a security email already exists + security_contact = self.domain.get_default_security_contact() security_contact.registry_id = "fail" security_contact.save() @@ -422,7 +423,7 @@ class TestRegistrantContacts(MockEppLib): call(expectedUpdateDomain, cleaned=True), ] self.mockedSendFunction.assert_has_calls(expected_calls, any_order=True) - assert PublicContact.objects.filter(domain=self.domain).count() == 1 + self.assertEqual(PublicContact.objects.filter(domain=self.domain).count(), 1) def test_user_deletes_security_email(self): """ @@ -454,9 +455,9 @@ class TestRegistrantContacts(MockEppLib): common.DomainContact(contact=old_contact.registry_id, type="security") ], ) - assert ( - PublicContact.objects.filter(domain=self.domain).get().email - == PublicContact.get_default_security().email + self.assertEqual( + PublicContact.objects.filter(domain=self.domain).get().email, + PublicContact.get_default_security().email, ) # this one triggers the fail secondCreateContact = self._convertPublicContactToEpp( @@ -468,7 +469,6 @@ class TestRegistrantContacts(MockEppLib): common.DomainContact(contact=old_contact.registry_id, type="security") ], ) - args = self.mockedSendFunction.call_args_list defaultSecID = ( PublicContact.objects.filter(domain=self.domain).get().registry_id @@ -534,7 +534,7 @@ class TestRegistrantContacts(MockEppLib): call(updateContact, cleaned=True), ] self.mockedSendFunction.assert_has_calls(expected_calls, any_order=True) - assert PublicContact.objects.filter(domain=self.domain).count() == 1 + self.assertEqual(PublicContact.objects.filter(domain=self.domain).count(), 1) @skip("not implemented yet") def test_update_is_unsuccessful(self):