From c66ac66018655023631fc03f98740eb092c6cefb Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 13 Dec 2023 09:02:23 -0700 Subject: [PATCH 01/85] Alphabetical ordering and filter --- src/registrar/admin.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 518c67869..7ca44a1a7 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -11,6 +11,8 @@ from django.http.response import HttpResponseRedirect from django.urls import reverse from epplibwrapper.errors import ErrorCode, RegistryError from registrar.models.domain import Domain +from registrar.models.user import User +from registrar.models.user_domain_role import UserDomainRole from registrar.models.utility.admin_sort_fields import AdminSortFields from registrar.utility import csv_export from . import models @@ -546,6 +548,18 @@ class DomainApplicationAdminForm(forms.ModelForm): class DomainApplicationAdmin(ListHeaderAdmin): """Custom domain applications admin class.""" + class InvestigatorFilter(admin.SimpleListFilter): + title = 'investigator' + parameter_name = 'investigator' + + def lookups(self, request, model_admin): + valid_user_ids = UserDomainRole.objects.filter(role=UserDomainRole.Roles.MANAGER).values_list('user__id', flat=True) + privileged_users = User.objects.filter(id__in=valid_user_ids) + return [(user.id, user) for user in privileged_users] + + def queryset(self, request, queryset): + print(f"look here: {self.value()}") + return queryset.filter(investigator=self.value()) # Columns list_display = [ @@ -558,7 +572,7 @@ class DomainApplicationAdmin(ListHeaderAdmin): ] # Filters - list_filter = ("status", "organization_type", "investigator") + list_filter = ("status", "organization_type", InvestigatorFilter) # Search search_fields = [ @@ -641,6 +655,10 @@ class DomainApplicationAdmin(ListHeaderAdmin): kwargs["queryset"] = models.Website.objects.all().order_by("website") # Sort websites return super().formfield_for_manytomany(db_field, request, **kwargs) + def get_queryset(self, request): + query_set = super().get_queryset(request) + return query_set.order_by("requested_domain__name") + # Trigger action when a fieldset is changed def save_model(self, request, obj, form, change): if obj and obj.creator.status != models.User.RESTRICTED: From 528c77a8b361c99408057f3480f270ae64cab9d1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 13 Dec 2023 09:15:54 -0700 Subject: [PATCH 02/85] Update admin.py --- src/registrar/admin.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 7ca44a1a7..91bf58512 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -548,17 +548,24 @@ class DomainApplicationAdminForm(forms.ModelForm): class DomainApplicationAdmin(ListHeaderAdmin): """Custom domain applications admin class.""" + class InvestigatorFilter(admin.SimpleListFilter): - title = 'investigator' - parameter_name = 'investigator' + """Custom investigator filter that only displays users with the manager role""" + title = "investigator" + parameter_name = "investigator" def lookups(self, request, model_admin): - valid_user_ids = UserDomainRole.objects.filter(role=UserDomainRole.Roles.MANAGER).values_list('user__id', flat=True) + """Lookup reimplementation, gets users by the MANAGER role. + Returns a list of tuples consisting of (user.id, user) + """ + valid_user_ids = UserDomainRole.objects.filter(role=UserDomainRole.Roles.MANAGER).values_list( + "user__id", flat=True + ) privileged_users = User.objects.filter(id__in=valid_user_ids) return [(user.id, user) for user in privileged_users] def queryset(self, request, queryset): - print(f"look here: {self.value()}") + """Custom queryset implementation, filters by investigator""" return queryset.filter(investigator=self.value()) # Columns From 6de09d5bb83b4ba38529386d9dd95e23bf9460b2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 13 Dec 2023 14:27:06 -0700 Subject: [PATCH 03/85] Test cases --- src/registrar/tests/test_admin.py | 45 +++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 8a67fc191..a9181f97e 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -322,6 +322,7 @@ class TestDomainApplicationAdmin(MockEppLib): self.admin = DomainApplicationAdmin(model=DomainApplication, admin_site=self.site) self.superuser = create_superuser() self.staffuser = create_user() + self.client = Client(HTTP_HOST="localhost:8080") def test_short_org_name_in_applications_list(self): """ @@ -842,6 +843,50 @@ class TestDomainApplicationAdmin(MockEppLib): with self.assertRaises(DomainInformation.DoesNotExist): domain_information.refresh_from_db() + def test_has_correct_filters(self): + """Tests if DomainApplicationAdmin has the correct filters""" + request = self.factory.get("/") + request.user = self.superuser + + readonly_fields = self.admin.get_list_filter(request) + expected_fields = ("status", "organization_type", DomainApplicationAdmin.InvestigatorFilter) + + self.assertEqual(readonly_fields, expected_fields) + + def test_displays_investigator_filter(self): + """Tests if DomainApplicationAdmin displays the investigator filter""" + p = "userpass" + self.client.login(username="staffuser", password=p) + response = self.client.get( + "/admin/registrar/domainapplication/", + {}, + follow=True, + ) + + expected_sort_column = "sortable column-investigator" + self.assertContains(response, expected_sort_column, count=1) + + def test_investigator_filter(self): + """Tests the custom investigator filter""" + # Creates multiple domain applications + multiple_unalphabetical_domain_objects("application") + p = "userpass" + self.client.login(username="staffuser", password=p) + + response = self.client.get( + "/admin/registrar/domainapplication/", + { + "investigator": 1, + }, + follow=True, + ) + + # The "multiple_unalphabetical_domain_objects" function will create + # user objects with a first name that contains "first_name:investigator". + # We can simply count how many times this appears in the table to determine + # how many records exist. + self.assertContains(response, "first_name:investigator", count=1) + def tearDown(self): super().tearDown() Domain.objects.all().delete() From 3d4972773c31c079d91b4cbea0b9b0c3cfe752fb Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Dec 2023 09:15:59 -0700 Subject: [PATCH 04/85] Remove old code --- src/registrar/admin.py | 8 +++----- src/registrar/tests/test_admin.py | 29 +++++++++++++++++++++++++++-- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 91bf58512..af2cb3104 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -555,13 +555,10 @@ class DomainApplicationAdmin(ListHeaderAdmin): parameter_name = "investigator" def lookups(self, request, model_admin): - """Lookup reimplementation, gets users by the MANAGER role. + """Lookup reimplementation, gets users of is_staff. Returns a list of tuples consisting of (user.id, user) """ - valid_user_ids = UserDomainRole.objects.filter(role=UserDomainRole.Roles.MANAGER).values_list( - "user__id", flat=True - ) - privileged_users = User.objects.filter(id__in=valid_user_ids) + privileged_users = User.objects.filter(is_staff=True) return [(user.id, user) for user in privileged_users] def queryset(self, request, queryset): @@ -663,6 +660,7 @@ class DomainApplicationAdmin(ListHeaderAdmin): return super().formfield_for_manytomany(db_field, request, **kwargs) def get_queryset(self, request): + """Queryset reimplementation to order the table alphabetically""" query_set = super().get_queryset(request) return query_set.order_by("requested_domain__name") diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index a9181f97e..a3969d386 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -848,6 +848,7 @@ class TestDomainApplicationAdmin(MockEppLib): request = self.factory.get("/") request.user = self.superuser + # Grab the current list of table filters readonly_fields = self.admin.get_list_filter(request) expected_fields = ("status", "organization_type", DomainApplicationAdmin.InvestigatorFilter) @@ -859,13 +860,37 @@ class TestDomainApplicationAdmin(MockEppLib): self.client.login(username="staffuser", password=p) response = self.client.get( "/admin/registrar/domainapplication/", - {}, - follow=True, ) + + # Create a mock DomainApplication object, with a fake investigator + application: DomainApplication = generic_domain_object("application", "SomeGuy") + # Add the role manager to the investigator + UserDomainRole.objects.get_or_create( + user=application.investigator, + role=UserDomainRole.Roles.MANAGER, + domain=Domain.objects.create(name="SomeGuy.gov") + ) + + # First, make sure that there is still an investigator field to begin with expected_sort_column = "sortable column-investigator" self.assertContains(response, expected_sort_column, count=1) + # Then, test if the filter actually exists + self.assertIn("filters", response.context) + # Assert the content of filters and search_query + filters = response.context["filters"] + self.assertEqual( + filters, + [ + {"parameter_name": "status", "parameter_value": "started"}, + { + "parameter_name": "investigator", + "parameter_value": "test", + }, + ], + ) + def test_investigator_filter(self): """Tests the custom investigator filter""" # Creates multiple domain applications From daf640ded33ce92b0368c02246bd8ec25cbf0bcc Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Dec 2023 09:29:18 -0700 Subject: [PATCH 05/85] Fix test cases --- src/registrar/admin.py | 1 + src/registrar/tests/test_admin.py | 52 +++++++++---------------------- 2 files changed, 15 insertions(+), 38 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index af2cb3104..85bdf9c1b 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -551,6 +551,7 @@ class DomainApplicationAdmin(ListHeaderAdmin): class InvestigatorFilter(admin.SimpleListFilter): """Custom investigator filter that only displays users with the manager role""" + title = "investigator" parameter_name = "investigator" diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index a3969d386..d95d52203 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -853,65 +853,41 @@ class TestDomainApplicationAdmin(MockEppLib): expected_fields = ("status", "organization_type", DomainApplicationAdmin.InvestigatorFilter) self.assertEqual(readonly_fields, expected_fields) - + def test_displays_investigator_filter(self): """Tests if DomainApplicationAdmin displays the investigator filter""" + + # Create a mock DomainApplication object, with a fake investigator + application: DomainApplication = generic_domain_object("application", "SomeGuy") + investigator_user = User.objects.filter(username=application.investigator.username).get() + investigator_user.is_staff = True + investigator_user.save() + p = "userpass" self.client.login(username="staffuser", password=p) response = self.client.get( "/admin/registrar/domainapplication/", + { + "investigator": investigator_user.id, + }, + follow=True, ) - - # Create a mock DomainApplication object, with a fake investigator - application: DomainApplication = generic_domain_object("application", "SomeGuy") - - # Add the role manager to the investigator - UserDomainRole.objects.get_or_create( - user=application.investigator, - role=UserDomainRole.Roles.MANAGER, - domain=Domain.objects.create(name="SomeGuy.gov") - ) - - # First, make sure that there is still an investigator field to begin with - expected_sort_column = "sortable column-investigator" - self.assertContains(response, expected_sort_column, count=1) # Then, test if the filter actually exists self.assertIn("filters", response.context) # Assert the content of filters and search_query filters = response.context["filters"] + print(response.context.__dict__) self.assertEqual( filters, [ - {"parameter_name": "status", "parameter_value": "started"}, { "parameter_name": "investigator", - "parameter_value": "test", + "parameter_value": "4", }, ], ) - def test_investigator_filter(self): - """Tests the custom investigator filter""" - # Creates multiple domain applications - multiple_unalphabetical_domain_objects("application") - p = "userpass" - self.client.login(username="staffuser", password=p) - - response = self.client.get( - "/admin/registrar/domainapplication/", - { - "investigator": 1, - }, - follow=True, - ) - - # The "multiple_unalphabetical_domain_objects" function will create - # user objects with a first name that contains "first_name:investigator". - # We can simply count how many times this appears in the table to determine - # how many records exist. - self.assertContains(response, "first_name:investigator", count=1) - def tearDown(self): super().tearDown() Domain.objects.all().delete() From 7ea5503c91d84c357e7e355f18bba8a376752a15 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Dec 2023 09:58:33 -0700 Subject: [PATCH 06/85] Add formfield --- src/registrar/admin.py | 7 +++++++ src/registrar/models/domain_application.py | 1 + 2 files changed, 8 insertions(+) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 85bdf9c1b..e783ff1a6 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -660,6 +660,13 @@ class DomainApplicationAdmin(ListHeaderAdmin): kwargs["queryset"] = models.Website.objects.all().order_by("website") # Sort websites return super().formfield_for_manytomany(db_field, request, **kwargs) + def formfield_for_foreignkey(self, db_field, request, **kwargs): + # Removes invalid investigator options from the investigator dropdown + if db_field.name == "investigator": + kwargs["queryset"] = User.objects.filter(is_staff=True) + return db_field.formfield(**kwargs) + return super().formfield_for_foreignkey(db_field, request, **kwargs) + def get_queryset(self, request): """Queryset reimplementation to order the table alphabetically""" query_set = super().get_queryset(request) diff --git a/src/registrar/models/domain_application.py b/src/registrar/models/domain_application.py index 12eda4caf..1ca749c84 100644 --- a/src/registrar/models/domain_application.py +++ b/src/registrar/models/domain_application.py @@ -5,6 +5,7 @@ import logging from django.apps import apps from django.db import models +from django.forms import ValidationError from django_fsm import FSMField, transition # type: ignore from registrar.models.domain import Domain From ec43865f702dd8d44672ef82feac0de9a0d9b36e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Dec 2023 10:36:46 -0700 Subject: [PATCH 07/85] Finish test cases --- src/registrar/admin.py | 5 ++- src/registrar/tests/test_admin.py | 61 ++++++++++++++++++++++++++++++- 2 files changed, 62 insertions(+), 4 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index e783ff1a6..fb1f49e0b 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -553,7 +553,8 @@ class DomainApplicationAdmin(ListHeaderAdmin): """Custom investigator filter that only displays users with the manager role""" title = "investigator" - parameter_name = "investigator" + # Match the old param name to avoid unnecessary refactoring + parameter_name = "investigator__id__exact" def lookups(self, request, model_admin): """Lookup reimplementation, gets users of is_staff. @@ -564,7 +565,7 @@ class DomainApplicationAdmin(ListHeaderAdmin): def queryset(self, request, queryset): """Custom queryset implementation, filters by investigator""" - return queryset.filter(investigator=self.value()) + return queryset.filter(investigator__id__exact=self.value()) # Columns list_display = [ diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index d95d52203..8aa1c1a31 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -877,17 +877,74 @@ class TestDomainApplicationAdmin(MockEppLib): self.assertIn("filters", response.context) # Assert the content of filters and search_query filters = response.context["filters"] - print(response.context.__dict__) + + # Ensure that the format is correct. We will test the value later in the test. self.assertEqual( filters, [ { "parameter_name": "investigator", - "parameter_value": "4", + "parameter_value": str(investigator_user.id), }, ], ) + # Manually test the returned values + request = self.factory.get("/admin/registrar/domainapplication/") + # Set the GET parameters for testing + request.GET = { + "investigator__id__exact": investigator_user.id, + } + # Call the get_filters method + filters = self.admin.get_filters(request) + + # Assert the filters extracted from the request GET + self.assertEqual( + filters, + [ + { + "parameter_name": "investigator", + # We intentionally test a weird value, to see what happens + "parameter_value": "SomeGuy first_name:investigator SomeGuy last_name:investigator", + }, + ], + ) + + def test_investigator_filter_filters_correctly(self): + """Tests the investigator filter""" + + # Create a mock DomainApplication object, with a fake investigator + application: DomainApplication = generic_domain_object("application", "SomeGuy") + investigator_user = User.objects.filter(username=application.investigator.username).get() + investigator_user.is_staff = True + investigator_user.save() + + # Create a second mock DomainApplication object, to test filtering + application: DomainApplication = generic_domain_object("application", "BadGuy") + another_user = User.objects.filter(username=application.investigator.username).get() + another_user.is_staff = True + another_user.save() + + p = "userpass" + self.client.login(username="staffuser", password=p) + response = self.client.get( + "/admin/registrar/domainapplication/", + { + "investigator__id__exact": investigator_user.id, + }, + follow=True, + ) + + expected_name = "SomeGuy first_name:investigator SomeGuy last_name:investigator" + # We expect to see this four times, two of them are from the html for the filter, + # the other two are the html from the list entry in the table. + self.assertContains(response, expected_name, count=4) + + # Check that we don't also get the thing we aren't filtering for. + # We expect to see this two times, two of them are from the html for the filter. + unexpected_name = "BadGuy first_name:investigator BadGuy last_name:investigator" + self.assertContains(response, unexpected_name, count=2) + def tearDown(self): super().tearDown() Domain.objects.all().delete() From a9e252bced9f28f883b5eaf3a8f639c15d0f25b9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Dec 2023 10:42:08 -0700 Subject: [PATCH 08/85] Remove unused imports --- src/registrar/admin.py | 1 - src/registrar/models/domain_application.py | 1 - 2 files changed, 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index fb1f49e0b..be84bcbcf 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -12,7 +12,6 @@ from django.urls import reverse from epplibwrapper.errors import ErrorCode, RegistryError from registrar.models.domain import Domain from registrar.models.user import User -from registrar.models.user_domain_role import UserDomainRole from registrar.models.utility.admin_sort_fields import AdminSortFields from registrar.utility import csv_export from . import models diff --git a/src/registrar/models/domain_application.py b/src/registrar/models/domain_application.py index 1ca749c84..12eda4caf 100644 --- a/src/registrar/models/domain_application.py +++ b/src/registrar/models/domain_application.py @@ -5,7 +5,6 @@ import logging from django.apps import apps from django.db import models -from django.forms import ValidationError from django_fsm import FSMField, transition # type: ignore from registrar.models.domain import Domain From 7b2badac05c247ee4c7fbd7974613b12ba90dcb8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Dec 2023 11:49:23 -0700 Subject: [PATCH 09/85] Add requester --- src/registrar/views/domain.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 5ec4433f7..761018433 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -644,7 +644,7 @@ class DomainAddUserView(DomainFormBaseView): """Get an absolute URL for this domain.""" return self.request.build_absolute_uri(reverse("domain", kwargs={"pk": self.object.id})) - def _send_domain_invitation_email(self, email: str, add_success=True): + def _send_domain_invitation_email(self, email: str, requester: User, add_success=True): """Performs the sending of the domain invitation email, does not make a domain information object email: string- email to send to @@ -655,7 +655,10 @@ class DomainAddUserView(DomainFormBaseView): domainInfo = domainInfoResults.first() first = "" last = "" - if domainInfo is not None: + if requester is not None: + first = requester.first_name + last = requester.last_name + elif domainInfo is not None: first = domainInfo.creator.first_name last = domainInfo.creator.last_name full_name = f"{first} {last}" @@ -683,7 +686,7 @@ class DomainAddUserView(DomainFormBaseView): if add_success: messages.success(self.request, f"Invited {email} to this domain.") - def _make_invitation(self, email_address: str): + def _make_invitation(self, email_address: str, requester: User): """Make a Domain invitation for this email and redirect with a message.""" invitation, created = DomainInvitation.objects.get_or_create(email=email_address, domain=self.object) if not created: @@ -693,21 +696,22 @@ class DomainAddUserView(DomainFormBaseView): f"{email_address} has already been invited to this domain.", ) else: - self._send_domain_invitation_email(email=email_address) + self._send_domain_invitation_email(email=email_address, requester=requester) return redirect(self.get_success_url()) def form_valid(self, form): """Add the specified user on this domain.""" requested_email = form.cleaned_data["email"] + requester = None # look up a user with that email try: requested_user = User.objects.get(email=requested_email) except User.DoesNotExist: # no matching user, go make an invitation - return self._make_invitation(requested_email) + return self._make_invitation(requested_email, requester) else: # if user already exists then just send an email - self._send_domain_invitation_email(requested_email, add_success=False) + self._send_domain_invitation_email(requested_email, requester, add_success=False) try: UserDomainRole.objects.create( From 66a5af08eb55abf0efcd2231ed00d8af0ed80f73 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Dec 2023 12:02:15 -0700 Subject: [PATCH 10/85] Add requester --- src/registrar/views/domain.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 761018433..c4e0336f4 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -653,15 +653,25 @@ class DomainAddUserView(DomainFormBaseView): # created a new invitation in the database, so send an email domainInfoResults = DomainInformation.objects.filter(domain=self.object) domainInfo = domainInfoResults.first() - first = "" - last = "" + first: str = "" + last: str = "" if requester is not None: first = requester.first_name last = requester.last_name elif domainInfo is not None: first = domainInfo.creator.first_name last = domainInfo.creator.last_name - full_name = f"{first} {last}" + else: + # This edgecase would be unusual if encountered. We don't want to handle this here. Rather, we would + # want to fix this upstream where it is happening. + raise ValueError("Can't send email. The given DomainInformation object has no requestor or creator.") + + if first and last: + full_name = f"{first} {last}" + else: + # This edgecase would be unusual if encountered. We don't want to handle this here. Rather, we would + # want to fix this upstream where it is happening. + raise ValueError("Can't send email. First and last names are none.") try: send_templated_email( @@ -702,7 +712,7 @@ class DomainAddUserView(DomainFormBaseView): def form_valid(self, form): """Add the specified user on this domain.""" requested_email = form.cleaned_data["email"] - requester = None + requester = self.request.user # look up a user with that email try: requested_user = User.objects.get(email=requested_email) From de82da6e78b576abb14d4d4f35cf4eaa5c672aaf Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Dec 2023 12:25:14 -0700 Subject: [PATCH 11/85] Add last resort --- src/registrar/views/domain.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index c4e0336f4..5f21a7150 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -666,12 +666,17 @@ class DomainAddUserView(DomainFormBaseView): # want to fix this upstream where it is happening. raise ValueError("Can't send email. The given DomainInformation object has no requestor or creator.") + # Attempt to grab the first and last names. As a last resort, just use the email. if first and last: full_name = f"{first} {last}" + elif requester.email is not None and requester.email.strip() != "": + full_name = requester.email + elif domainInfo.creator.email is not None and domainInfo.creator.email.strip() != "": + full_name = domainInfo.creator.email else: # This edgecase would be unusual if encountered. We don't want to handle this here. Rather, we would # want to fix this upstream where it is happening. - raise ValueError("Can't send email. First and last names are none.") + raise ValueError("Can't send email. First and last names, as well as the email, are none.") try: send_templated_email( From fbad8eef6b8beed3ffd3ab18adf479ffed46d4f6 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Dec 2023 12:28:24 -0700 Subject: [PATCH 12/85] Update domain.py --- src/registrar/views/domain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 5f21a7150..632530e71 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -666,7 +666,7 @@ class DomainAddUserView(DomainFormBaseView): # want to fix this upstream where it is happening. raise ValueError("Can't send email. The given DomainInformation object has no requestor or creator.") - # Attempt to grab the first and last names. As a last resort, just use the email. + # Attempt to grab the first and last names. As a last resort, just use the email. if first and last: full_name = f"{first} {last}" elif requester.email is not None and requester.email.strip() != "": From 74161a86ee54ac625c8456450650694111eb39a4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Dec 2023 12:38:31 -0700 Subject: [PATCH 13/85] Modify unit test --- src/registrar/tests/test_views.py | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 17641636e..ed47f3278 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1415,11 +1415,36 @@ class TestDomainManagers(TestDomainOverview): add_page.form["email"] = email_address self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) add_page.form.submit() + + expected_email_content = { + 'Simple': { + 'Subject': { + 'Data': 'You’ve been added to a .gov domain' + }, + 'Body': { + 'Text': { + 'Data': '\nHi.\n\nFirst Last has added you as a manager on igorville.gov.\n\n' + 'YOU NEED A LOGIN.GOV ACCOUNT\nYou’ll need a Login.gov account to manage your .gov domain. ' + 'Login.gov provides\na simple and secure process for signing into many government services with ' + 'one\naccount. If you don’t already have one, follow these steps to create your\nLogin.gov ' + 'account .\n\nDOMAIN MANAGEMENT\nAs a ' + '.gov domain manager you can add or update information about your domain.\nYou’ll also serve as ' + 'a contact for your .gov domain. Please keep your contact\ninformation updated. ' + 'Learn more about domain management .\n\nSOMETHING WRONG?\nIf you’re not ' + 'affiliated with igorville.gov or think you received this\nmessage in error, contact the ' + '.gov team .\n\n\nTHANK YOU\n\n.Gov helps the public identify ' + 'official, trusted information. ' + 'Thank you for\nusing a .gov domain.\n\n-------------------------------------------------------' + '---------\n\nThe .gov team\nContact us: \nVisit \n' + } + } + } + } # check the mock instance to see if `send_email` was called right mock_client_instance.send_email.assert_called_once_with( FromEmailAddress=settings.DEFAULT_FROM_EMAIL, Destination={"ToAddresses": [email_address]}, - Content=ANY, + Content=expected_email_content, ) def test_domain_invitation_cancel(self): From ceec0a1efdddf16752fb2dbce8e1a543be268e80 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Dec 2023 12:39:11 -0700 Subject: [PATCH 14/85] Linting --- src/registrar/tests/test_views.py | 40 +++++++++++++++---------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index ed47f3278..351b223d6 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1415,29 +1415,27 @@ class TestDomainManagers(TestDomainOverview): add_page.form["email"] = email_address self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) add_page.form.submit() - + expected_email_content = { - 'Simple': { - 'Subject': { - 'Data': 'You’ve been added to a .gov domain' - }, - 'Body': { - 'Text': { - 'Data': '\nHi.\n\nFirst Last has added you as a manager on igorville.gov.\n\n' - 'YOU NEED A LOGIN.GOV ACCOUNT\nYou’ll need a Login.gov account to manage your .gov domain. ' - 'Login.gov provides\na simple and secure process for signing into many government services with ' - 'one\naccount. If you don’t already have one, follow these steps to create your\nLogin.gov ' - 'account .\n\nDOMAIN MANAGEMENT\nAs a ' - '.gov domain manager you can add or update information about your domain.\nYou’ll also serve as ' - 'a contact for your .gov domain. Please keep your contact\ninformation updated. ' - 'Learn more about domain management .\n\nSOMETHING WRONG?\nIf you’re not ' - 'affiliated with igorville.gov or think you received this\nmessage in error, contact the ' - '.gov team .\n\n\nTHANK YOU\n\n.Gov helps the public identify ' - 'official, trusted information. ' - 'Thank you for\nusing a .gov domain.\n\n-------------------------------------------------------' - '---------\n\nThe .gov team\nContact us: \nVisit \n' + "Simple": { + "Subject": {"Data": "You’ve been added to a .gov domain"}, + "Body": { + "Text": { + "Data": "\nHi.\n\nFirst Last has added you as a manager on igorville.gov.\n\n" + "YOU NEED A LOGIN.GOV ACCOUNT\nYou’ll need a Login.gov account to manage your .gov domain. " + "Login.gov provides\na simple and secure process for signing into many government services with " + "one\naccount. If you don’t already have one, follow these steps to create your\nLogin.gov " + "account .\n\nDOMAIN MANAGEMENT\nAs a " + ".gov domain manager you can add or update information about your domain.\nYou’ll also serve as " + "a contact for your .gov domain. Please keep your contact\ninformation updated. " + "Learn more about domain management .\n\nSOMETHING WRONG?\nIf you’re not " + "affiliated with igorville.gov or think you received this\nmessage in error, contact the " + ".gov team .\n\n\nTHANK YOU\n\n.Gov helps the public identify " + "official, trusted information. " + "Thank you for\nusing a .gov domain.\n\n-------------------------------------------------------" + "---------\n\nThe .gov team\nContact us: \nVisit \n" } - } + }, } } # check the mock instance to see if `send_email` was called right From b04e3f679b679300035b3ce9c4b14a326af7eb3d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Dec 2023 12:49:43 -0700 Subject: [PATCH 15/85] Fix linter --- src/registrar/tests/test_views.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 351b223d6..250f8ac07 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1,5 +1,5 @@ from unittest import skip -from unittest.mock import MagicMock, ANY, patch +from unittest.mock import MagicMock, patch from django.conf import settings from django.test import Client, TestCase @@ -1423,15 +1423,15 @@ class TestDomainManagers(TestDomainOverview): "Text": { "Data": "\nHi.\n\nFirst Last has added you as a manager on igorville.gov.\n\n" "YOU NEED A LOGIN.GOV ACCOUNT\nYou’ll need a Login.gov account to manage your .gov domain. " - "Login.gov provides\na simple and secure process for signing into many government services with " - "one\naccount. If you don’t already have one, follow these steps to create your\nLogin.gov " + "Login.gov provides\na simple and secure process for signing into many government services with" + " one\naccount. If you don’t already have one, follow these steps to create your\nLogin.gov " "account .\n\nDOMAIN MANAGEMENT\nAs a " - ".gov domain manager you can add or update information about your domain.\nYou’ll also serve as " - "a contact for your .gov domain. Please keep your contact\ninformation updated. " - "Learn more about domain management .\n\nSOMETHING WRONG?\nIf you’re not " + ".gov domain manager you can add or update information about your domain.\nYou’ll also serve as" + " a contact for your .gov domain. Please keep your contact\ninformation updated. " + " Learn more about domain management .\n\nSOMETHING WRONG?\nIf you’re not" "affiliated with igorville.gov or think you received this\nmessage in error, contact the " - ".gov team .\n\n\nTHANK YOU\n\n.Gov helps the public identify " - "official, trusted information. " + ".gov team .\n\n\nTHANK YOU\n\n.Gov helps the public identify" + " official, trusted information. " "Thank you for\nusing a .gov domain.\n\n-------------------------------------------------------" "---------\n\nThe .gov team\nContact us: \nVisit \n" } From d6fc1dabb6686ba5e32fe0fa75cf8325398f3db1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Dec 2023 12:53:52 -0700 Subject: [PATCH 16/85] Fix lint and test --- src/registrar/tests/test_views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 250f8ac07..d34f1e2a5 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1428,8 +1428,8 @@ class TestDomainManagers(TestDomainOverview): "account .\n\nDOMAIN MANAGEMENT\nAs a " ".gov domain manager you can add or update information about your domain.\nYou’ll also serve as" " a contact for your .gov domain. Please keep your contact\ninformation updated. " - " Learn more about domain management .\n\nSOMETHING WRONG?\nIf you’re not" - "affiliated with igorville.gov or think you received this\nmessage in error, contact the " + "Learn more about domain management .\n\nSOMETHING WRONG?\nIf you’re" + "not affiliated with igorville.gov or think you received this\nmessage in error, contact the " ".gov team .\n\n\nTHANK YOU\n\n.Gov helps the public identify" " official, trusted information. " "Thank you for\nusing a .gov domain.\n\n-------------------------------------------------------" From 8638ee7dc9446e557b724d353519f136b64a69ee Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Dec 2023 14:02:46 -0700 Subject: [PATCH 17/85] Better test cases, align with AC --- .../templates/emails/domain_invitation.txt | 2 +- src/registrar/tests/test_views.py | 149 +++++++++++++++--- src/registrar/views/domain.py | 25 +-- 3 files changed, 131 insertions(+), 45 deletions(-) diff --git a/src/registrar/templates/emails/domain_invitation.txt b/src/registrar/templates/emails/domain_invitation.txt index ed9c297f4..f0612a67f 100644 --- a/src/registrar/templates/emails/domain_invitation.txt +++ b/src/registrar/templates/emails/domain_invitation.txt @@ -1,7 +1,7 @@ {% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} Hi. -{{ full_name }} has added you as a manager on {{ domain.name }}. +{{ requester_email }} has added you as a manager on {{ domain.name }}. YOU NEED A LOGIN.GOV ACCOUNT You’ll need a Login.gov account to manage your .gov domain. Login.gov provides diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index d34f1e2a5..2e1d29296 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1,5 +1,5 @@ from unittest import skip -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, ANY, call, patch from django.conf import settings from django.test import Client, TestCase @@ -1416,35 +1416,136 @@ class TestDomainManagers(TestDomainOverview): self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) add_page.form.submit() - expected_email_content = { - "Simple": { - "Subject": {"Data": "You’ve been added to a .gov domain"}, - "Body": { - "Text": { - "Data": "\nHi.\n\nFirst Last has added you as a manager on igorville.gov.\n\n" - "YOU NEED A LOGIN.GOV ACCOUNT\nYou’ll need a Login.gov account to manage your .gov domain. " - "Login.gov provides\na simple and secure process for signing into many government services with" - " one\naccount. If you don’t already have one, follow these steps to create your\nLogin.gov " - "account .\n\nDOMAIN MANAGEMENT\nAs a " - ".gov domain manager you can add or update information about your domain.\nYou’ll also serve as" - " a contact for your .gov domain. Please keep your contact\ninformation updated. " - "Learn more about domain management .\n\nSOMETHING WRONG?\nIf you’re" - "not affiliated with igorville.gov or think you received this\nmessage in error, contact the " - ".gov team .\n\n\nTHANK YOU\n\n.Gov helps the public identify" - " official, trusted information. " - "Thank you for\nusing a .gov domain.\n\n-------------------------------------------------------" - "---------\n\nThe .gov team\nContact us: \nVisit \n" - } - }, - } - } # check the mock instance to see if `send_email` was called right mock_client_instance.send_email.assert_called_once_with( FromEmailAddress=settings.DEFAULT_FROM_EMAIL, Destination={"ToAddresses": [email_address]}, - Content=expected_email_content, + Content=ANY, ) + @boto3_mocking.patching + def test_domain_invitation_email_has_email_as_requester_non_existent(self): + """Inviting a non existent user sends them an email, with email as the name.""" + # make sure there is no user with this email + email_address = "mayor@igorville.gov" + User.objects.filter(email=email_address).delete() + + self.domain_information, _ = DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain) + + mock_client = MagicMock() + mock_client_instance = mock_client.return_value + + with boto3_mocking.clients.handler_for("sesv2", mock_client): + add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id})) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + add_page.form["email"] = email_address + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + add_page.form.submit() + + # check the mock instance to see if `send_email` was called right + mock_client_instance.send_email.assert_called_once_with( + FromEmailAddress=settings.DEFAULT_FROM_EMAIL, + Destination={"ToAddresses": [email_address]}, + Content=ANY, + ) + + # Check the arguments passed to send_email method + _, kwargs = mock_client_instance.send_email.call_args + + # Extract the email content, and check that the message is as we expect + email_content = kwargs["Content"]["Simple"]["Body"]["Text"]["Data"] + self.assertIn("info@example.com", email_content) + + # Check that the requesters first/last name do not exist + self.assertNotIn("First", email_content) + self.assertNotIn("Last", email_content) + + @boto3_mocking.patching + def test_domain_invitation_email_has_email_as_requester(self): + """Inviting a user sends them an email, with email as the name.""" + # Create a fake user object + email_address = "mayor@igorville.gov" + User.objects.get_or_create(email=email_address, username="fakeuser@fakeymail.com") + + self.domain_information, _ = DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain) + + mock_client = MagicMock() + mock_client_instance = mock_client.return_value + + with boto3_mocking.clients.handler_for("sesv2", mock_client): + add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id})) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + add_page.form["email"] = email_address + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + add_page.form.submit() + + # check the mock instance to see if `send_email` was called right + mock_client_instance.send_email.assert_called_once_with( + FromEmailAddress=settings.DEFAULT_FROM_EMAIL, + Destination={"ToAddresses": [email_address]}, + Content=ANY, + ) + + # Check the arguments passed to send_email method + _, kwargs = mock_client_instance.send_email.call_args + + # Extract the email content, and check that the message is as we expect + email_content = kwargs["Content"]["Simple"]["Body"]["Text"]["Data"] + self.assertIn("info@example.com", email_content) + + # Check that the requesters first/last name do not exist + self.assertNotIn("First", email_content) + self.assertNotIn("Last", email_content) + + @boto3_mocking.patching + def test_domain_invitation_email_throws_exception_non_existent(self): + """Inviting a non existent user sends them an email, with email as the name.""" + # make sure there is no user with this email + email_address = "mayor@igorville.gov" + User.objects.filter(email=email_address).delete() + + # Give the user who is sending the email an invalid email address + requester = User.objects.filter(email="info@example.com").get() + requester.email = "" + requester.save() + + self.domain_information, _ = DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain) + + mock_client = MagicMock() + + with boto3_mocking.clients.handler_for("sesv2", mock_client): + with self.assertRaisesMessage(ValueError, "Can't send email. No email exists for the requester"): + add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id})) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + add_page.form["email"] = email_address + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + add_page.form.submit() + + @boto3_mocking.patching + def test_domain_invitation_email_throws_exception(self): + """Inviting a user sends them an email, with email as the name.""" + # make sure there is no user with this email + # Create a fake user object + email_address = "mayor@igorville.gov" + User.objects.get_or_create(email=email_address, username="fakeuser@fakeymail.com") + + # Give the user who is sending the email an invalid email address + requester = User.objects.filter(email="info@example.com").get() + requester.email = "" + requester.save() + + self.domain_information, _ = DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain) + + mock_client = MagicMock() + + with boto3_mocking.clients.handler_for("sesv2", mock_client): + with self.assertRaisesMessage(ValueError, "Can't send email. No email exists for the requester"): + add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id})) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + add_page.form["email"] = email_address + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + add_page.form.submit() + def test_domain_invitation_cancel(self): """Posting to the delete view deletes an invitation.""" email_address = "mayor@igorville.gov" diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 632530e71..7c3f175d3 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -651,32 +651,17 @@ class DomainAddUserView(DomainFormBaseView): add_success: bool- default True indicates: adding a success message to the view if the email sending succeeds""" # created a new invitation in the database, so send an email - domainInfoResults = DomainInformation.objects.filter(domain=self.object) - domainInfo = domainInfoResults.first() - first: str = "" - last: str = "" - if requester is not None: - first = requester.first_name - last = requester.last_name - elif domainInfo is not None: - first = domainInfo.creator.first_name - last = domainInfo.creator.last_name - else: + if requester is None: # This edgecase would be unusual if encountered. We don't want to handle this here. Rather, we would # want to fix this upstream where it is happening. raise ValueError("Can't send email. The given DomainInformation object has no requestor or creator.") - # Attempt to grab the first and last names. As a last resort, just use the email. - if first and last: - full_name = f"{first} {last}" - elif requester.email is not None and requester.email.strip() != "": - full_name = requester.email - elif domainInfo.creator.email is not None and domainInfo.creator.email.strip() != "": - full_name = domainInfo.creator.email + if requester.email is not None and requester.email.strip() != "": + requester_email = requester.email else: # This edgecase would be unusual if encountered. We don't want to handle this here. Rather, we would # want to fix this upstream where it is happening. - raise ValueError("Can't send email. First and last names, as well as the email, are none.") + raise ValueError("Can't send email. No email exists for the requester.") try: send_templated_email( @@ -686,7 +671,7 @@ class DomainAddUserView(DomainFormBaseView): context={ "domain_url": self._domain_abs_url(), "domain": self.object, - "full_name": full_name, + "requester_email": requester_email, }, ) except EmailSendingError: From 9e16b0b65cc237be3c2708721e9e60283e7aeeef Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Dec 2023 14:06:37 -0700 Subject: [PATCH 18/85] Remove unused imports --- src/registrar/tests/test_views.py | 2 +- src/registrar/views/domain.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 2e1d29296..87eb80cd3 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1,5 +1,5 @@ from unittest import skip -from unittest.mock import MagicMock, ANY, call, patch +from unittest.mock import MagicMock, ANY, patch from django.conf import settings from django.test import Client, TestCase diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 7c3f175d3..3bd9e15a8 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -17,7 +17,6 @@ from django.views.generic.edit import FormMixin from registrar.models import ( Domain, - DomainInformation, DomainInvitation, User, UserDomainRole, From 9f7f1764729a8c88aa158c16923a821a886fa21b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 14 Dec 2023 14:18:01 -0700 Subject: [PATCH 19/85] Update domain.py --- src/registrar/views/domain.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 3bd9e15a8..52e4e27d8 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -649,12 +649,8 @@ class DomainAddUserView(DomainFormBaseView): email: string- email to send to add_success: bool- default True indicates: adding a success message to the view if the email sending succeeds""" - # created a new invitation in the database, so send an email - if requester is None: - # This edgecase would be unusual if encountered. We don't want to handle this here. Rather, we would - # want to fix this upstream where it is happening. - raise ValueError("Can't send email. The given DomainInformation object has no requestor or creator.") + # Check if the email requester has a valid email address if requester.email is not None and requester.email.strip() != "": requester_email = requester.email else: From d72e704ba43e4ba01e0fae944c73e429dd328c6a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 15 Dec 2023 08:49:57 -0700 Subject: [PATCH 20/85] Add test alphabetical sorting test --- src/registrar/tests/test_admin.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 8aa1c1a31..72c639e4d 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -843,6 +843,26 @@ class TestDomainApplicationAdmin(MockEppLib): with self.assertRaises(DomainInformation.DoesNotExist): domain_information.refresh_from_db() + def test_table_sorted_alphabetically(self): + """Tests if DomainApplicationAdmin table is sorted alphabetically""" + self.max_diff = None + # Creates a list of DomainApplications in scrambled order + multiple_unalphabetical_domain_objects("application") + request = self.factory.get("/") + request.user = self.superuser + + # Get the expected list of alphabetically sorted DomainApplications + expected_order = DomainApplication.objects.order_by('requested_domain__name') + + # Get the returned queryset + queryset = self.admin.get_queryset(request) + + # Check the order + self.assertEqual( + list(queryset), + list(expected_order), + ) + def test_has_correct_filters(self): """Tests if DomainApplicationAdmin has the correct filters""" request = self.factory.get("/") From 1be36839da753d3cfaed90457f0220edebb50468 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 15 Dec 2023 08:52:44 -0700 Subject: [PATCH 21/85] Remove max_diff --- src/registrar/tests/test_admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 72c639e4d..dff40b8cf 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -845,9 +845,9 @@ class TestDomainApplicationAdmin(MockEppLib): def test_table_sorted_alphabetically(self): """Tests if DomainApplicationAdmin table is sorted alphabetically""" - self.max_diff = None # Creates a list of DomainApplications in scrambled order multiple_unalphabetical_domain_objects("application") + request = self.factory.get("/") request.user = self.superuser From 72be62a750ca993f6e590064bff0c88289a52def Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 15 Dec 2023 08:53:15 -0700 Subject: [PATCH 22/85] Black formatting --- src/registrar/tests/test_admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index dff40b8cf..604383975 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -852,7 +852,7 @@ class TestDomainApplicationAdmin(MockEppLib): request.user = self.superuser # Get the expected list of alphabetically sorted DomainApplications - expected_order = DomainApplication.objects.order_by('requested_domain__name') + expected_order = DomainApplication.objects.order_by("requested_domain__name") # Get the returned queryset queryset = self.admin.get_queryset(request) From eec7c577d71ba7ac10671014946f49f02d5ba894 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 15 Dec 2023 10:34:02 -0700 Subject: [PATCH 23/85] Raise message --- src/registrar/tests/test_views.py | 53 ++----------------------------- src/registrar/views/domain.py | 10 ++++-- 2 files changed, 10 insertions(+), 53 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 87eb80cd3..1f418da26 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -6,7 +6,7 @@ from django.test import Client, TestCase from django.urls import reverse from django.contrib.auth import get_user_model from .common import MockEppLib, completed_application, create_user # type: ignore - +from django.contrib.messages import get_messages from django_webtest import WebTest # type: ignore import boto3_mocking # type: ignore @@ -1459,6 +1459,7 @@ class TestDomainManagers(TestDomainOverview): # Check that the requesters first/last name do not exist self.assertNotIn("First", email_content) self.assertNotIn("Last", email_content) + self.assertNotIn("First Last", email_content) @boto3_mocking.patching def test_domain_invitation_email_has_email_as_requester(self): @@ -1496,55 +1497,7 @@ class TestDomainManagers(TestDomainOverview): # Check that the requesters first/last name do not exist self.assertNotIn("First", email_content) self.assertNotIn("Last", email_content) - - @boto3_mocking.patching - def test_domain_invitation_email_throws_exception_non_existent(self): - """Inviting a non existent user sends them an email, with email as the name.""" - # make sure there is no user with this email - email_address = "mayor@igorville.gov" - User.objects.filter(email=email_address).delete() - - # Give the user who is sending the email an invalid email address - requester = User.objects.filter(email="info@example.com").get() - requester.email = "" - requester.save() - - self.domain_information, _ = DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain) - - mock_client = MagicMock() - - with boto3_mocking.clients.handler_for("sesv2", mock_client): - with self.assertRaisesMessage(ValueError, "Can't send email. No email exists for the requester"): - add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id})) - session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] - add_page.form["email"] = email_address - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - add_page.form.submit() - - @boto3_mocking.patching - def test_domain_invitation_email_throws_exception(self): - """Inviting a user sends them an email, with email as the name.""" - # make sure there is no user with this email - # Create a fake user object - email_address = "mayor@igorville.gov" - User.objects.get_or_create(email=email_address, username="fakeuser@fakeymail.com") - - # Give the user who is sending the email an invalid email address - requester = User.objects.filter(email="info@example.com").get() - requester.email = "" - requester.save() - - self.domain_information, _ = DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain) - - mock_client = MagicMock() - - with boto3_mocking.clients.handler_for("sesv2", mock_client): - with self.assertRaisesMessage(ValueError, "Can't send email. No email exists for the requester"): - add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id})) - session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] - add_page.form["email"] = email_address - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - add_page.form.submit() + self.assertNotIn("First Last", email_content) def test_domain_invitation_cancel(self): """Posting to the delete view deletes an invitation.""" diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 52e4e27d8..c0f4cc5b1 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -654,9 +654,13 @@ class DomainAddUserView(DomainFormBaseView): if requester.email is not None and requester.email.strip() != "": requester_email = requester.email else: - # This edgecase would be unusual if encountered. We don't want to handle this here. Rather, we would - # want to fix this upstream where it is happening. - raise ValueError("Can't send email. No email exists for the requester.") + messages.error(self.request, "Can't send invitation email. No email is associated with your account.") + logger.error( + f"Can't send email to '{email}' on domain '{self.object}'." + f"No email exists for the requester '{requester.username}'.", + exc_info=True, + ) + return None try: send_templated_email( From b64fa2410c5f84cff04e1f6697bf7409a394b1de Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 15 Dec 2023 11:06:18 -0700 Subject: [PATCH 24/85] Fix none bug --- src/registrar/admin.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index be84bcbcf..86c564e05 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -564,7 +564,10 @@ class DomainApplicationAdmin(ListHeaderAdmin): def queryset(self, request, queryset): """Custom queryset implementation, filters by investigator""" - return queryset.filter(investigator__id__exact=self.value()) + if self.value() is None: + return queryset + else: + return queryset.filter(investigator__id__exact=self.value()) # Columns list_display = [ From 3e811f80ce3a6b9be342cd77290d73e6d938cf19 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Fri, 15 Dec 2023 11:07:31 -0800 Subject: [PATCH 25/85] Remove middle name and phone number --- src/registrar/forms/domain.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index ac96393b4..c08b43ba0 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -227,8 +227,6 @@ class AuthorizingOfficialContactForm(ContactForm): self.fields["email"].error_messages = { "required": "Enter an email address in the required format, like name@example.com." } - self.fields["phone"].error_messages["required"] = "Enter a phone number for your authorizing official." - class DomainSecurityEmailForm(forms.Form): """Form for adding or editing a security email to a domain.""" From 66899da2837c847b7e5f36fe76bd78bd0bbb07c7 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Fri, 15 Dec 2023 11:19:53 -0800 Subject: [PATCH 26/85] Remove from form --- src/registrar/forms/domain.py | 1 + src/registrar/templates/domain_authorizing_official.html | 6 ------ 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index c08b43ba0..64a653ff5 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -228,6 +228,7 @@ class AuthorizingOfficialContactForm(ContactForm): "required": "Enter an email address in the required format, like name@example.com." } + class DomainSecurityEmailForm(forms.Form): """Form for adding or editing a security email to a domain.""" diff --git a/src/registrar/templates/domain_authorizing_official.html b/src/registrar/templates/domain_authorizing_official.html index 57a315e86..e7fc12a5e 100644 --- a/src/registrar/templates/domain_authorizing_official.html +++ b/src/registrar/templates/domain_authorizing_official.html @@ -19,18 +19,12 @@ {% input_with_errors form.first_name %} - {% input_with_errors form.middle_name %} - {% input_with_errors form.last_name %} {% input_with_errors form.title %} {% input_with_errors form.email %} - {% input_with_errors form.phone %} - - -