From 33106895192fca9cbbd82470982d175085ca5154 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 13 Sep 2023 16:20:14 -0400 Subject: [PATCH 01/12] Add DraftDomain to superusers and staff, Add filter to domains (state), Update user table definition to show first, last AND email by default, Use inline editors to expose DomainInformation, Make Domain Information fields available to CISA Analyst --- src/registrar/admin.py | 293 +++++++++++++++++++++++------------ src/registrar/fixtures.py | 10 ++ src/registrar/models/user.py | 2 +- 3 files changed, 204 insertions(+), 101 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 78b19191e..6bad9d9d0 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -189,102 +189,6 @@ class MyHostAdmin(AuditedAdmin): inlines = [HostIPInline] -class DomainAdmin(ListHeaderAdmin): - """Custom domain admin class to add extra buttons.""" - - # Columns - list_display = [ - "name", - "organization_type", - "state", - ] - - def organization_type(self, obj): - return obj.domain_info.organization_type - - organization_type.admin_order_field = ( # type: ignore - "domain_info__organization_type" - ) - - # Filters - list_filter = ["domain_info__organization_type"] - - search_fields = ["name"] - search_help_text = "Search by domain name." - change_form_template = "django/admin/domain_change_form.html" - readonly_fields = ["state"] - - def response_change(self, request, obj): - # Create dictionary of action functions - ACTION_FUNCTIONS = { - "_place_client_hold": self.do_place_client_hold, - "_remove_client_hold": self.do_remove_client_hold, - "_edit_domain": self.do_edit_domain, - } - - # Check which action button was pressed and call the corresponding function - for action, function in ACTION_FUNCTIONS.items(): - if action in request.POST: - return function(request, obj) - - # If no matching action button is found, return the super method - return super().response_change(request, obj) - - def do_place_client_hold(self, request, obj): - try: - obj.place_client_hold() - obj.save() - except Exception as err: - self.message_user(request, err, messages.ERROR) - else: - self.message_user( - request, - ( - "%s is in client hold. This domain is no longer accessible on" - " the public internet." - ) - % obj.name, - ) - return HttpResponseRedirect(".") - - def do_remove_client_hold(self, request, obj): - try: - obj.remove_client_hold() - obj.save() - except Exception as err: - self.message_user(request, err, messages.ERROR) - else: - self.message_user( - request, - ("%s is ready. This domain is accessible on the public internet.") - % obj.name, - ) - return HttpResponseRedirect(".") - - def do_edit_domain(self, request, obj): - # We want to know, globally, when an edit action occurs - request.session["analyst_action"] = "edit" - # Restricts this action to this domain (pk) only - request.session["analyst_action_location"] = obj.id - return HttpResponseRedirect(reverse("domain", args=(obj.id,))) - - def change_view(self, request, object_id): - # If the analyst was recently editing a domain page, - # delete any associated session values - if "analyst_action" in request.session: - del request.session["analyst_action"] - del request.session["analyst_action_location"] - return super().change_view(request, object_id) - - def has_change_permission(self, request, obj=None): - # Fixes a bug wherein users which are only is_staff - # can access 'change' when GET, - # but cannot access this page when it is a request of type POST. - if request.user.is_staff: - return True - return super().has_change_permission(request, obj) - - class ContactAdmin(ListHeaderAdmin): """Custom contact admin class to add search.""" @@ -380,6 +284,84 @@ class DomainInformationAdmin(ListHeaderAdmin): ] search_help_text = "Search by domain." + # TODO: on merging with 446, replace type_of_work with + # about_your_organization and remove more_organization_information + fieldsets = [ + (None, {"fields": ["creator", "domain_application"]}), + ( + "Type of organization", + { + "fields": [ + "organization_type", + "federally_recognized_tribe", + "state_recognized_tribe", + "tribe_name", + "federal_agency", + "federal_type", + "is_election_board", + "type_of_work", + "more_organization_information", + ] + }, + ), + ( + "Organization name and mailing address", + { + "fields": [ + "organization_name", + "address_line1", + "address_line2", + "city", + "state_territory", + "zipcode", + "urbanization", + ] + }, + ), + ("Authorizing official", {"fields": ["authorizing_official"]}), + (".gov domain", {"fields": ["domain"]}), + ("Your contact information", {"fields": ["submitter"]}), + ("Other employees from your organization?", {"fields": ["other_contacts"]}), + ( + "No other employees from your organization?", + {"fields": ["no_other_contacts_rationale"]}, + ), + ("Anything else we should know?", {"fields": ["anything_else"]}), + ( + "Requirements for operating .gov domains", + {"fields": ["is_policy_acknowledged"]}, + ), + ] + + # Read only that we'll leverage for CISA Analysts + analyst_readonly_fields = [ + "creator", + "type_of_work", + "more_organization_information", + "address_line1", + "address_line2", + "zipcode", + "domain", + "submitter", + "no_other_contacts_rationale", + "anything_else", + "is_policy_acknowledged", + ] + + def get_readonly_fields(self, request, obj=None): + """Set the read-only state on form elements. + We have 1 conditions that determine which fields are read-only: + admin user permissions. + """ + + readonly_fields = list(self.readonly_fields) + + if request.user.is_superuser: + return readonly_fields + else: + readonly_fields.extend([field for field in self.analyst_readonly_fields]) + return readonly_fields + class DomainApplicationAdminForm(forms.ModelForm): """Custom form to limit transitions to available transitions""" @@ -416,10 +398,6 @@ class DomainApplicationAdmin(ListHeaderAdmin): """Custom domain applications admin class.""" - # Set multi-selects 'read-only' (hide selects and show data) - # based on user perms and application creator's status - # form = DomainApplicationForm - # Columns list_display = [ "requested_domain", @@ -589,6 +567,120 @@ class DomainApplicationAdmin(ListHeaderAdmin): return super().change_view(request, object_id, form_url, extra_context) +class DomainInformationInline(admin.StackedInline): + """Edit a domain information on the domain page. + We had issues inheriting from both StackedInline + and the source DomainInformationAdmin since these + classes conflict, so we'll just pull what we need + from DomainInformationAdmin""" + + model = models.DomainInformation + + fieldsets = DomainInformationAdmin.fieldsets + analyst_readonly_fields = DomainInformationAdmin.analyst_readonly_fields + + def get_readonly_fields(self, request, obj=None): + return DomainInformationAdmin.get_readonly_fields(self, request, obj=None) + + +class DomainAdmin(ListHeaderAdmin): + """Custom domain admin class to add extra buttons.""" + + inlines = [DomainInformationInline] + + # Columns + list_display = [ + "name", + "organization_type", + "state", + ] + + def organization_type(self, obj): + return obj.domain_info.organization_type + + organization_type.admin_order_field = ( # type: ignore + "domain_info__organization_type" + ) + + # Filters + list_filter = ["domain_info__organization_type", "state"] + + search_fields = ["name"] + search_help_text = "Search by domain name." + change_form_template = "django/admin/domain_change_form.html" + readonly_fields = ["state"] + + def response_change(self, request, obj): + # Create dictionary of action functions + ACTION_FUNCTIONS = { + "_place_client_hold": self.do_place_client_hold, + "_remove_client_hold": self.do_remove_client_hold, + "_edit_domain": self.do_edit_domain, + } + + # Check which action button was pressed and call the corresponding function + for action, function in ACTION_FUNCTIONS.items(): + if action in request.POST: + return function(request, obj) + + # If no matching action button is found, return the super method + return super().response_change(request, obj) + + def do_place_client_hold(self, request, obj): + try: + obj.place_client_hold() + obj.save() + except Exception as err: + self.message_user(request, err, messages.ERROR) + else: + self.message_user( + request, + ( + "%s is in client hold. This domain is no longer accessible on" + " the public internet." + ) + % obj.name, + ) + return HttpResponseRedirect(".") + + def do_remove_client_hold(self, request, obj): + try: + obj.remove_client_hold() + obj.save() + except Exception as err: + self.message_user(request, err, messages.ERROR) + else: + self.message_user( + request, + ("%s is ready. This domain is accessible on the public internet.") + % obj.name, + ) + return HttpResponseRedirect(".") + + def do_edit_domain(self, request, obj): + # We want to know, globally, when an edit action occurs + request.session["analyst_action"] = "edit" + # Restricts this action to this domain (pk) only + request.session["analyst_action_location"] = obj.id + return HttpResponseRedirect(reverse("domain", args=(obj.id,))) + + def change_view(self, request, object_id): + # If the analyst was recently editing a domain page, + # delete any associated session values + if "analyst_action" in request.session: + del request.session["analyst_action"] + del request.session["analyst_action_location"] + return super().change_view(request, object_id) + + def has_change_permission(self, request, obj=None): + # Fixes a bug wherein users which are only is_staff + # can access 'change' when GET, + # but cannot access this page when it is a request of type POST. + if request.user.is_staff: + return True + return super().has_change_permission(request, obj) + + admin.site.unregister(LogEntry) # Unregister the default registration admin.site.register(LogEntry, CustomLogEntryAdmin) admin.site.register(models.User, MyUserAdmin) @@ -597,6 +689,7 @@ admin.site.register(models.Contact, ContactAdmin) admin.site.register(models.DomainInvitation, DomainInvitationAdmin) admin.site.register(models.DomainInformation, DomainInformationAdmin) admin.site.register(models.Domain, DomainAdmin) +admin.site.register(models.DraftDomain, ListHeaderAdmin) admin.site.register(models.Host, MyHostAdmin) admin.site.register(models.Nameserver, MyHostAdmin) admin.site.register(models.Website, WebsiteAdmin) diff --git a/src/registrar/fixtures.py b/src/registrar/fixtures.py index 30924b8bf..b5c25a170 100644 --- a/src/registrar/fixtures.py +++ b/src/registrar/fixtures.py @@ -143,12 +143,22 @@ class UserFixture: "permissions": ["view_logentry"], }, {"app_label": "registrar", "model": "contact", "permissions": ["view_contact"]}, + { + "app_label": "registrar", + "model": "domaininformation", + "permissions": ["change_domaininformation"], + }, { "app_label": "registrar", "model": "domainapplication", "permissions": ["change_domainapplication"], }, {"app_label": "registrar", "model": "domain", "permissions": ["view_domain"]}, + { + "app_label": "registrar", + "model": "draftdomain", + "permissions": ["change_draftdomain"], + }, {"app_label": "registrar", "model": "user", "permissions": ["view_user"]}, ] diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 5cf1dd71f..5b04c628d 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -45,7 +45,7 @@ class User(AbstractUser): def __str__(self): # this info is pulled from Login.gov if self.first_name or self.last_name: - return f"{self.first_name or ''} {self.last_name or ''}" + return f"{self.first_name or ''} {self.last_name or ''} {self.email or ''}" elif self.email: return self.email else: From 68012389a8c2bb9e936847272e54ac08a2c4f960 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 18 Sep 2023 15:21:57 -0400 Subject: [PATCH 02/12] fix typo in exception message --- src/registrar/models/domain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index b0bf00082..dda295a60 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -341,7 +341,7 @@ class Domain(TimeStampedModel, DomainHelper): if "statuses" not in self._cache: self._fetch_cache() if "statuses" not in self._cache: - raise Exception("Can't retreive status from domain info") + raise Exception("Can't retrieve status from domain info") else: return self._cache["statuses"] From 76a2a1a6fd2be1422b89973241910a439843e18a Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 19 Sep 2023 13:32:57 -0400 Subject: [PATCH 03/12] refactor status getter, add unit test --- src/registrar/models/domain.py | 11 +++---- src/registrar/tests/common.py | 4 ++- src/registrar/tests/test_models_domain.py | 40 ++++++++++++++++------- 3 files changed, 36 insertions(+), 19 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index dda295a60..c20e92b63 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -336,14 +336,11 @@ class Domain(TimeStampedModel, DomainHelper): A domain's status indicates various properties. See Domain.Status. """ - # 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 "statuses" not in self._cache: - self._fetch_cache() - if "statuses" not in self._cache: - raise Exception("Can't retrieve status from domain info") - else: - return self._cache["statuses"] + try: + return self._get_property("statuses") + except KeyError: + logger.error("Can't retrieve status from domain info") @statuses.setter # type: ignore def statuses(self, statuses: list[str]): diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index e21431321..d9fdc0b52 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -547,17 +547,19 @@ class MockEppLib(TestCase): class fakedEppObject(object): """""" - def __init__(self, auth_info=..., cr_date=..., contacts=..., hosts=...): + def __init__(self, auth_info=..., cr_date=..., contacts=..., hosts=..., statuses=...,): self.auth_info = auth_info self.cr_date = cr_date self.contacts = contacts self.hosts = hosts + self.statuses = statuses mockDataInfoDomain = fakedEppObject( "fakepw", cr_date=datetime.datetime(2023, 5, 25, 19, 45, 35), contacts=[common.DomainContact(contact="123", type="security")], hosts=["fake.host.com"], + statuses=[common.Status(state='serverTransferProhibited', description=None, lang='en'), common.Status(state='inactive', description=None, lang='en')], ) infoDomainNoContact = fakedEppObject( "security", diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 9aaac7321..02bb04e26 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -34,6 +34,8 @@ class TestDomainCache(MockEppLib): # (see InfoDomainResult) self.assertEquals(domain._cache["auth_info"], self.mockDataInfoDomain.auth_info) self.assertEquals(domain._cache["cr_date"], self.mockDataInfoDomain.cr_date) + status_list = [status.state for status in self.mockDataInfoDomain.statuses] + self.assertEquals(domain._cache["statuses"], status_list) self.assertFalse("avail" in domain._cache.keys()) # using a setter should clear the cache @@ -49,7 +51,8 @@ class TestDomainCache(MockEppLib): ), call(commands.InfoContact(id="123", auth_info=None), cleaned=True), call(commands.InfoHost(name="fake.host.com"), cleaned=True), - ] + ], + any_order=False # Ensure calls are in the specified order ) def test_cache_used_when_avail(self): @@ -168,22 +171,37 @@ class TestDomainCreation(TestCase): with self.assertRaisesRegex(IntegrityError, "name"): Domain.objects.create(name="igorville.gov") - @skip("cannot activate a domain without mock registry") - def test_get_status(self): - """Returns proper status based on `state`.""" - domain = Domain.objects.create(name="igorville.gov") - domain.save() - self.assertEqual(None, domain.status) - domain.activate() - domain.save() - self.assertIn("ok", domain.status) - def tearDown(self) -> None: DomainInformation.objects.all().delete() DomainApplication.objects.all().delete() Domain.objects.all().delete() +class TestDomainStatuses(MockEppLib): + + def test_get_status(self): + """Getter returns statuses""" + domain, _ = Domain.objects.get_or_create(name="igorville2.gov") + _ = domain.statuses + status_list = [status.state for status in self.mockDataInfoDomain.statuses] + self.assertEquals(domain._cache["statuses"], status_list) + + # print(self.mockedSendFunction.call_args_list) + + # Called in _fetch_cache + self.mockedSendFunction.assert_has_calls( + [ + call(commands.InfoDomain(name='igorville2.gov', auth_info=None), cleaned=True), + call(commands.InfoContact(id='123', auth_info=None), cleaned=True), + call(commands.InfoHost(name='fake.host.com'), cleaned=True) + ], + any_order=False # Ensure calls are in the specified order + ) + + def tearDown(self) -> None: + Domain.objects.all().delete() + + class TestRegistrantContacts(MockEppLib): """Rule: Registrants may modify their WHOIS data""" From d0effe6018f5d96c255e346f2b21d0b2b7b0427a Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 19 Sep 2023 14:47:43 -0400 Subject: [PATCH 04/12] lint --- 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 4df2401c3..cbbbe1077 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -702,7 +702,7 @@ class DomainAdmin(ListHeaderAdmin): # If no matching action button is found, return the super method return super().response_change(request, obj) - + def do_delete_domain(self, request, obj): try: obj.deleted() From 425045783af1d8bd3617bba950ee972a4ede2532 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 19 Sep 2023 14:50:58 -0400 Subject: [PATCH 05/12] replace type_of_work with # about_your_organization and remove more_organization_information --- src/registrar/admin.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index cbbbe1077..d0f5c0dbf 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -333,8 +333,6 @@ class DomainInformationAdmin(ListHeaderAdmin): ] search_help_text = "Search by domain." - # TODO: on merging with 446, replace type_of_work with - # about_your_organization and remove more_organization_information fieldsets = [ (None, {"fields": ["creator", "domain_application"]}), ( @@ -348,8 +346,7 @@ class DomainInformationAdmin(ListHeaderAdmin): "federal_agency", "federal_type", "is_election_board", - "type_of_work", - "more_organization_information", + "about_your_organization", ] }, ), From 021f71f2f5b2b4418499429fbe3ece114806d155 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 19 Sep 2023 17:46:47 -0400 Subject: [PATCH 06/12] clean up refactor, refactor an unclosed patch in a test that was causing subsequent tests to fail, lint --- src/registrar/models/domain.py | 12 +++--- src/registrar/tests/common.py | 16 +++++++- src/registrar/tests/test_models_domain.py | 49 ++++++++++++----------- 3 files changed, 46 insertions(+), 31 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index c20e92b63..440bed0aa 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -336,11 +336,11 @@ class Domain(TimeStampedModel, DomainHelper): A domain's status indicates various properties. See Domain.Status. """ - if "statuses" not in self._cache: - try: - return self._get_property("statuses") - except KeyError: - logger.error("Can't retrieve status from domain info") + try: + return self._get_property("statuses") + except KeyError: + logger.error("Can't retrieve status from domain info") + return [] @statuses.setter # type: ignore def statuses(self, statuses: list[str]): @@ -954,7 +954,7 @@ class Domain(TimeStampedModel, DomainHelper): # remove null properties (to distinguish between "a value of None" and null) cleaned = {k: v for k, v in cache.items() if v is not ...} - + print(f"cleaned {cleaned}") # statuses can just be a list no need to keep the epp object if "statuses" in cleaned.keys(): cleaned["statuses"] = [status.state for status in cleaned["statuses"]] diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index d9fdc0b52..5258a88f3 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -547,7 +547,14 @@ class MockEppLib(TestCase): class fakedEppObject(object): """""" - def __init__(self, auth_info=..., cr_date=..., contacts=..., hosts=..., statuses=...,): + def __init__( + self, + auth_info=..., + cr_date=..., + contacts=..., + hosts=..., + statuses=..., + ): self.auth_info = auth_info self.cr_date = cr_date self.contacts = contacts @@ -559,7 +566,10 @@ class MockEppLib(TestCase): cr_date=datetime.datetime(2023, 5, 25, 19, 45, 35), contacts=[common.DomainContact(contact="123", type="security")], hosts=["fake.host.com"], - statuses=[common.Status(state='serverTransferProhibited', description=None, lang='en'), common.Status(state='inactive', description=None, lang='en')], + statuses=[ + common.Status(state="serverTransferProhibited", description="", lang="en"), + common.Status(state="inactive", description="", lang="en"), + ], ) infoDomainNoContact = fakedEppObject( "security", @@ -597,6 +607,7 @@ class MockEppLib(TestCase): def setUp(self): """mock epp send function as this will fail locally""" + print("Set up EPP MOCK") self.mockSendPatch = patch("registrar.models.domain.registry.send") self.mockedSendFunction = self.mockSendPatch.start() self.mockedSendFunction.side_effect = self.mockSend @@ -660,4 +671,5 @@ class MockEppLib(TestCase): ) def tearDown(self): + print("tear down EPP MOCK") self.mockSendPatch.stop() diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 02bb04e26..f0c01d239 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -5,7 +5,7 @@ This file tests the various ways in which the registrar interacts with the regis """ from django.test import TestCase from django.db.utils import IntegrityError -from unittest.mock import patch, call +from unittest.mock import call import datetime from registrar.models import Domain @@ -52,7 +52,7 @@ class TestDomainCache(MockEppLib): call(commands.InfoContact(id="123", auth_info=None), cleaned=True), call(commands.InfoHost(name="fake.host.com"), cleaned=True), ], - any_order=False # Ensure calls are in the specified order + any_order=False, # Ensure calls are in the specified order ) def test_cache_used_when_avail(self): @@ -109,16 +109,14 @@ class TestDomainCache(MockEppLib): domain._get_property("hosts") self.assertEqual(domain._cache["hosts"], [expectedHostsDict]) + def tearDown(self) -> None: + Domain.objects.all().delete() + super().tearDown() -class TestDomainCreation(TestCase): + +class TestDomainCreation(MockEppLib): """Rule: An approved domain application must result in a domain""" - def setUp(self): - """ - Background: - Given that a valid domain application exists - """ - def test_approved_application_creates_domain_locally(self): """ Scenario: Analyst approves a domain application @@ -126,8 +124,6 @@ class TestDomainCreation(TestCase): Then a Domain exists in the database with the same `name` But a domain object does not exist in the registry """ - patcher = patch("registrar.models.domain.Domain._get_or_create_domain") - mocked_domain_creation = patcher.start() draft_domain, _ = DraftDomain.objects.get_or_create(name="igorville.gov") user, _ = User.objects.get_or_create() application = DomainApplication.objects.create( @@ -140,7 +136,7 @@ class TestDomainCreation(TestCase): # should hav information present for this domain domain = Domain.objects.get(name="igorville.gov") self.assertTrue(domain) - mocked_domain_creation.assert_not_called() + self.mockedSendFunction.assert_not_called() @skip("not implemented yet") def test_accessing_domain_properties_creates_domain_in_registry(self): @@ -175,32 +171,39 @@ class TestDomainCreation(TestCase): DomainInformation.objects.all().delete() DomainApplication.objects.all().delete() Domain.objects.all().delete() + super().tearDown() class TestDomainStatuses(MockEppLib): - def test_get_status(self): """Getter returns statuses""" - domain, _ = Domain.objects.get_or_create(name="igorville2.gov") + domain, _ = Domain.objects.get_or_create(name="chicken-liver.gov") + # trigger getter _ = domain.statuses + print("checkpoint 1") + print(domain._cache["statuses"]) + print(domain._cache) + status_list = [status.state for status in self.mockDataInfoDomain.statuses] self.assertEquals(domain._cache["statuses"], status_list) - - # print(self.mockedSendFunction.call_args_list) - + # Called in _fetch_cache self.mockedSendFunction.assert_has_calls( [ - call(commands.InfoDomain(name='igorville2.gov', auth_info=None), cleaned=True), - call(commands.InfoContact(id='123', auth_info=None), cleaned=True), - call(commands.InfoHost(name='fake.host.com'), cleaned=True) + call( + commands.InfoDomain(name="chicken-liver.gov", auth_info=None), + cleaned=True, + ), + call(commands.InfoContact(id="123", auth_info=None), cleaned=True), + call(commands.InfoHost(name="fake.host.com"), cleaned=True), ], - any_order=False # Ensure calls are in the specified order + any_order=False, # Ensure calls are in the specified order ) - + def tearDown(self) -> None: Domain.objects.all().delete() - + super().tearDown() + class TestRegistrantContacts(MockEppLib): """Rule: Registrants may modify their WHOIS data""" From ca6fa62ca43760cc96bcf628e9bef9485df22a3a Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 19 Sep 2023 17:48:21 -0400 Subject: [PATCH 07/12] clean up print statements --- src/registrar/models/domain.py | 2 +- src/registrar/tests/common.py | 2 -- src/registrar/tests/test_models_domain.py | 4 ---- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 440bed0aa..d7b012282 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -954,7 +954,7 @@ class Domain(TimeStampedModel, DomainHelper): # remove null properties (to distinguish between "a value of None" and null) cleaned = {k: v for k, v in cache.items() if v is not ...} - print(f"cleaned {cleaned}") + # statuses can just be a list no need to keep the epp object if "statuses" in cleaned.keys(): cleaned["statuses"] = [status.state for status in cleaned["statuses"]] diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 5258a88f3..66d9c2db1 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -607,7 +607,6 @@ class MockEppLib(TestCase): def setUp(self): """mock epp send function as this will fail locally""" - print("Set up EPP MOCK") self.mockSendPatch = patch("registrar.models.domain.registry.send") self.mockedSendFunction = self.mockSendPatch.start() self.mockedSendFunction.side_effect = self.mockSend @@ -671,5 +670,4 @@ class MockEppLib(TestCase): ) def tearDown(self): - print("tear down EPP MOCK") self.mockSendPatch.stop() diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index f0c01d239..0bed3845b 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -180,10 +180,6 @@ class TestDomainStatuses(MockEppLib): domain, _ = Domain.objects.get_or_create(name="chicken-liver.gov") # trigger getter _ = domain.statuses - print("checkpoint 1") - print(domain._cache["statuses"]) - print(domain._cache) - status_list = [status.state for status in self.mockDataInfoDomain.statuses] self.assertEquals(domain._cache["statuses"], status_list) From 1653784a759502a946e63188e8bd7a6431d6e8f6 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 19 Sep 2023 18:50:27 -0400 Subject: [PATCH 08/12] Add search on draft domain --- src/registrar/admin.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index d0f5c0dbf..d78947c85 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -780,6 +780,13 @@ class DomainAdmin(ListHeaderAdmin): return super().has_change_permission(request, obj) +class DraftDomainAdmin(ListHeaderAdmin): + """Custom draft domain admin class.""" + + search_fields = ["name"] + search_help_text = "Search by draft domain name." + + admin.site.unregister(LogEntry) # Unregister the default registration admin.site.register(LogEntry, CustomLogEntryAdmin) admin.site.register(models.User, MyUserAdmin) @@ -788,7 +795,7 @@ admin.site.register(models.Contact, ContactAdmin) admin.site.register(models.DomainInvitation, DomainInvitationAdmin) admin.site.register(models.DomainInformation, DomainInformationAdmin) admin.site.register(models.Domain, DomainAdmin) -admin.site.register(models.DraftDomain, ListHeaderAdmin) +admin.site.register(models.DraftDomain, DraftDomainAdmin) admin.site.register(models.Host, MyHostAdmin) admin.site.register(models.Nameserver, MyHostAdmin) admin.site.register(models.Website, WebsiteAdmin) From a0b0fe27763a574c34076cdc87c2311dd5e1e490 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 20 Sep 2023 13:32:09 -0400 Subject: [PATCH 09/12] test the exception, partial implementation of test_accessing_domain_properties_creates_domain_in_registry --- src/registrar/models/domain.py | 10 ++-- src/registrar/tests/test_models_domain.py | 70 +++++++++++++++++++++-- 2 files changed, 71 insertions(+), 9 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index d7b012282..13405d9bb 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -332,7 +332,7 @@ class Domain(TimeStampedModel, DomainHelper): @Cache def statuses(self) -> list[str]: """ - Get or set the domain `status` elements from the registry. + Get the domain `status` elements from the registry. A domain's status indicates various properties. See Domain.Status. """ @@ -344,9 +344,11 @@ class Domain(TimeStampedModel, DomainHelper): @statuses.setter # type: ignore def statuses(self, statuses: list[str]): - # TODO: there are a long list of rules in the RFC about which statuses - # can be combined; check that here and raise errors for invalid combinations - - # some statuses cannot be set by the client at all + """ + We will not implement this. Statuses are set by the registry + when we run delete and client hold, and these are the only statuses + we will be triggering. + """ raise NotImplementedError() @Cache diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 0bed3845b..d35b0ba96 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -5,7 +5,7 @@ This file tests the various ways in which the registrar interacts with the regis """ from django.test import TestCase from django.db.utils import IntegrityError -from unittest.mock import call +from unittest.mock import patch, call import datetime from registrar.models import Domain @@ -138,17 +138,44 @@ class TestDomainCreation(MockEppLib): self.assertTrue(domain) self.mockedSendFunction.assert_not_called() - @skip("not implemented yet") def test_accessing_domain_properties_creates_domain_in_registry(self): """ Scenario: A registrant checks the status of a newly approved domain Given that no domain object exists in the registry When a property is accessed Then Domain sends `commands.CreateDomain` to the registry - And `domain.state` is set to `CREATED` + And `domain.state` is set to `UNKNOWN` And `domain.is_active()` returns False """ - raise + domain = Domain.objects.create(name="beef-tongue.gov") + # trigger getter + _ = domain.statuses + + # contacts = PublicContact.objects.filter(domain=domain, + # type=PublicContact.ContactTypeChoices.REGISTRANT).get() + + # Called in _fetch_cache + self.mockedSendFunction.assert_has_calls( + [ + # TODO: due to complexity of the test, will return to it in + # a future ticket + # call( + # commands.CreateDomain(name="beef-tongue.gov", + # id=contact.registry_id, auth_info=None), + # cleaned=True, + # ), + call( + commands.InfoDomain(name="beef-tongue.gov", auth_info=None), + cleaned=True, + ), + call(commands.InfoContact(id="123", auth_info=None), cleaned=True), + call(commands.InfoHost(name="fake.host.com"), cleaned=True), + ], + any_order=False, # Ensure calls are in the specified order + ) + + self.assertEqual(domain.state, Domain.State.UNKNOWN) + self.assertEqual(domain.is_active(), False) @skip("assertion broken with mock addition") def test_empty_domain_creation(self): @@ -175,8 +202,10 @@ class TestDomainCreation(MockEppLib): class TestDomainStatuses(MockEppLib): + """Domain statuses are set by the registry""" + def test_get_status(self): - """Getter returns statuses""" + """Domain 'statuses' getter returns statuses by calling epp""" domain, _ = Domain.objects.get_or_create(name="chicken-liver.gov") # trigger getter _ = domain.statuses @@ -196,6 +225,37 @@ class TestDomainStatuses(MockEppLib): any_order=False, # Ensure calls are in the specified order ) + def test_get_status_returns_empty_list_when_value_error(self): + """Domain 'statuses' getter returns an empty list + when value error""" + domain, _ = Domain.objects.get_or_create(name="pig-knuckles.gov") + + def side_effect(self): + raise KeyError + + patcher = patch("registrar.models.domain.Domain._get_property") + mocked_get = patcher.start() + mocked_get.side_effect = side_effect + + # trigger getter + _ = domain.statuses + + with self.assertRaises(KeyError): + _ = domain._cache["statuses"] + self.assertEquals(_, []) + + patcher.stop() + + @skip("not implemented yet") + def test_place_client_hold_sets_status(self): + """Domain 'place_client_hold' method causes the registry to change statuses""" + raise + + @skip("not implemented yet") + def test_revert_client_hold_sets_status(self): + """Domain 'revert_client_hold' method causes the registry to change statuses""" + raise + def tearDown(self) -> None: Domain.objects.all().delete() super().tearDown() From df62bf12beec9c9cf1cc8b2b67f994ee513bf8a4 Mon Sep 17 00:00:00 2001 From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> Date: Wed, 20 Sep 2023 15:45:33 -0500 Subject: [PATCH 10/12] Name servers page: Remove link --- src/registrar/templates/domain_nameservers.html | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/registrar/templates/domain_nameservers.html b/src/registrar/templates/domain_nameservers.html index 9c261306a..6a6a0b729 100644 --- a/src/registrar/templates/domain_nameservers.html +++ b/src/registrar/templates/domain_nameservers.html @@ -14,8 +14,6 @@

Before your domain can be used we'll need information about your domain name servers.

-

Get help with domain servers.

- {% include "includes/required_fields.html" %}
From 46a9241635916c7f9a2c03f826f34b8c21aafd7b Mon Sep 17 00:00:00 2001 From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> Date: Wed, 20 Sep 2023 15:59:18 -0500 Subject: [PATCH 11/12] Domain request confirmation: add link --- src/registrar/templates/application_done.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/application_done.html b/src/registrar/templates/application_done.html index 5df03d698..a5b37d2ab 100644 --- a/src/registrar/templates/application_done.html +++ b/src/registrar/templates/application_done.html @@ -26,7 +26,7 @@
  • Domain meets our naming requirements
    • -

    You can check the status +

    You can check the status of your request at any time. We'll email you with any questions or when we complete our review.

    From 0818e982bcc461f990cbd0e8947319a7508190da Mon Sep 17 00:00:00 2001 From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> Date: Wed, 20 Sep 2023 17:52:05 -0500 Subject: [PATCH 12/12] Remove strikethru --- src/registrar/templates/application_done.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/application_done.html b/src/registrar/templates/application_done.html index a5b37d2ab..a9ee55b47 100644 --- a/src/registrar/templates/application_done.html +++ b/src/registrar/templates/application_done.html @@ -26,7 +26,7 @@
  • Domain meets our naming requirements
    • -

    You can check the status +

    You can check the status of your request at any time. We'll email you with any questions or when we complete our review.