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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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 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 09/29] 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 10/29] 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 11/29] 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 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 12/29] 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 fa0beb1f8d8e038865195d08380672a7d68b5915 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 15 Dec 2023 15:16:39 -0700 Subject: [PATCH 13/29] Change test order --- src/registrar/tests/test_admin.py | 40 +++++++++++++++---------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 604383975..81b740818 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -843,26 +843,6 @@ 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""" - # 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("/") @@ -965,6 +945,26 @@ class TestDomainApplicationAdmin(MockEppLib): unexpected_name = "BadGuy first_name:investigator BadGuy last_name:investigator" self.assertContains(response, unexpected_name, count=2) + def test_table_sorted_alphabetically(self): + """Tests if DomainApplicationAdmin table is sorted alphabetically""" + # 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 tearDown(self): super().tearDown() Domain.objects.all().delete() From 5e19ad1b15cf82f7177cf7470aae741721b6049d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 15 Dec 2023 15:32:01 -0700 Subject: [PATCH 14/29] Update test_admin.py --- src/registrar/tests/test_admin.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 81b740818..9cd1d2bf7 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -21,6 +21,8 @@ from registrar.models import ( DomainInformation, User, DomainInvitation, + Contact, + Website ) from registrar.models.user_domain_role import UserDomainRole from .common import ( @@ -971,6 +973,8 @@ class TestDomainApplicationAdmin(MockEppLib): DomainInformation.objects.all().delete() DomainApplication.objects.all().delete() User.objects.all().delete() + Contact.objects.all().delete() + Website.objects.all().delete() class DomainInvitationAdminTest(TestCase): From 1b688b4abd220411669ad7243146dd2aa2f38559 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 18 Dec 2023 09:11:50 -0700 Subject: [PATCH 15/29] Update test_admin.py --- src/registrar/tests/test_admin.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 9cd1d2bf7..a37544c36 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -947,6 +947,24 @@ class TestDomainApplicationAdmin(MockEppLib): 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() + DomainInformation.objects.all().delete() + DomainApplication.objects.all().delete() + User.objects.all().delete() + Contact.objects.all().delete() + Website.objects.all().delete() + + +class TestDomainApplicationAdminTable(MockEppLib): + """Tests the table for DomainApplicationAdmin""" + def setUp(self): + """Enables epplib patching, and creates a fake admin object""" + super().setUp() + self.site = AdminSite() + self.admin = DomainApplicationAdmin(model=DomainApplication, admin_site=self.site) + def test_table_sorted_alphabetically(self): """Tests if DomainApplicationAdmin table is sorted alphabetically""" # Creates a list of DomainApplications in scrambled order @@ -968,6 +986,7 @@ class TestDomainApplicationAdmin(MockEppLib): ) def tearDown(self): + """Delete all associated domain objects""" super().tearDown() Domain.objects.all().delete() DomainInformation.objects.all().delete() From b6cb72aabbc538cefee1e4ae90dedbc4ada27762 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 18 Dec 2023 09:20:41 -0700 Subject: [PATCH 16/29] Update test_admin.py --- src/registrar/tests/test_admin.py | 80 ++++++++++++++++--------------- 1 file changed, 41 insertions(+), 39 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index a37544c36..2f453ccbd 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -316,6 +316,47 @@ class TestDomainApplicationAdminForm(TestCase): ) +class TestDomainApplicationAdminTable(MockEppLib): + """Tests the table for DomainApplicationAdmin""" + def setUp(self): + """Enables epplib patching, and creates a fake admin object""" + super().setUp() + self.site = AdminSite() + self.factory = RequestFactory() + self.superuser = create_superuser() + self.admin = DomainApplicationAdmin(model=DomainApplication, admin_site=self.site) + + def test_table_sorted_alphabetically(self): + """Tests if DomainApplicationAdmin table is sorted alphabetically""" + # 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 tearDown(self): + """Delete all associated domain objects""" + super().tearDown() + Domain.objects.all().delete() + DomainInformation.objects.all().delete() + DomainApplication.objects.all().delete() + User.objects.all().delete() + Contact.objects.all().delete() + Website.objects.all().delete() + + class TestDomainApplicationAdmin(MockEppLib): def setUp(self): super().setUp() @@ -957,45 +998,6 @@ class TestDomainApplicationAdmin(MockEppLib): Website.objects.all().delete() -class TestDomainApplicationAdminTable(MockEppLib): - """Tests the table for DomainApplicationAdmin""" - def setUp(self): - """Enables epplib patching, and creates a fake admin object""" - super().setUp() - self.site = AdminSite() - self.admin = DomainApplicationAdmin(model=DomainApplication, admin_site=self.site) - - def test_table_sorted_alphabetically(self): - """Tests if DomainApplicationAdmin table is sorted alphabetically""" - # 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 tearDown(self): - """Delete all associated domain objects""" - super().tearDown() - Domain.objects.all().delete() - DomainInformation.objects.all().delete() - DomainApplication.objects.all().delete() - User.objects.all().delete() - Contact.objects.all().delete() - Website.objects.all().delete() - - class DomainInvitationAdminTest(TestCase): """Tests for the DomainInvitation page""" From dace59411a94396a80508706ebd943e780638f5e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 18 Dec 2023 09:31:32 -0700 Subject: [PATCH 17/29] Change test approach --- src/registrar/tests/test_admin.py | 92 +++++++++++++++++-------------- 1 file changed, 50 insertions(+), 42 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 2f453ccbd..9d73fa18d 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -315,48 +315,6 @@ class TestDomainApplicationAdminForm(TestCase): DomainApplication._meta.get_field("status").choices, ) - -class TestDomainApplicationAdminTable(MockEppLib): - """Tests the table for DomainApplicationAdmin""" - def setUp(self): - """Enables epplib patching, and creates a fake admin object""" - super().setUp() - self.site = AdminSite() - self.factory = RequestFactory() - self.superuser = create_superuser() - self.admin = DomainApplicationAdmin(model=DomainApplication, admin_site=self.site) - - def test_table_sorted_alphabetically(self): - """Tests if DomainApplicationAdmin table is sorted alphabetically""" - # 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 tearDown(self): - """Delete all associated domain objects""" - super().tearDown() - Domain.objects.all().delete() - DomainInformation.objects.all().delete() - DomainApplication.objects.all().delete() - User.objects.all().delete() - Contact.objects.all().delete() - Website.objects.all().delete() - - class TestDomainApplicationAdmin(MockEppLib): def setUp(self): super().setUp() @@ -988,6 +946,56 @@ class TestDomainApplicationAdmin(MockEppLib): unexpected_name = "BadGuy first_name:investigator BadGuy last_name:investigator" self.assertContains(response, unexpected_name, count=2) + def test_table_sorted_alphabetically(self): + """Tests if DomainApplicationAdmin table is sorted alphabetically""" + + # Create a list of DomainApplications in scrambled order + application_1: DomainApplication = generic_domain_object("application", "CupOfTea") + another_user_1 = User.objects.filter(username=application_1.investigator.username).get() + another_user_1.is_staff = True + another_user_1.save() + + application_2: DomainApplication = generic_domain_object("application", "Xylophone") + another_user_2 = User.objects.filter(username=application_2.investigator.username).get() + another_user_2.is_staff = True + another_user_2.save() + + application_3: DomainApplication = generic_domain_object("application", "Blue") + another_user_3 = User.objects.filter(username=application_3.investigator.username).get() + another_user_3.is_staff = True + another_user_3.save() + + application_4: DomainApplication = generic_domain_object("application", "BadGuy") + another_user_4 = User.objects.filter(username=application_4.investigator.username).get() + another_user_4.is_staff = True + another_user_4.save() + + application_5: DomainApplication = generic_domain_object("application", "AppleMan") + application_5 = User.objects.filter(username=application_5.investigator.username).get() + application_5.is_staff = True + application_5.save() + + 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") + + # First, ensure that our test data is not alphabetical + self.assertEqual( + list(DomainApplication.objects.all()), + list(expected_order) + ) + + # Get the returned queryset + queryset = self.admin.get_queryset(request) + + # Then, Check the order of the queryset + self.assertEqual( + list(queryset), + list(expected_order), + ) + def tearDown(self): super().tearDown() Domain.objects.all().delete() From 04c9c8388fca2977b8e3295d93dd012894445c6f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 18 Dec 2023 09:40:11 -0700 Subject: [PATCH 18/29] Revert "Change test approach" This reverts commit dace59411a94396a80508706ebd943e780638f5e. --- src/registrar/tests/test_admin.py | 92 ++++++++++++++----------------- 1 file changed, 42 insertions(+), 50 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 9d73fa18d..2f453ccbd 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -315,6 +315,48 @@ class TestDomainApplicationAdminForm(TestCase): DomainApplication._meta.get_field("status").choices, ) + +class TestDomainApplicationAdminTable(MockEppLib): + """Tests the table for DomainApplicationAdmin""" + def setUp(self): + """Enables epplib patching, and creates a fake admin object""" + super().setUp() + self.site = AdminSite() + self.factory = RequestFactory() + self.superuser = create_superuser() + self.admin = DomainApplicationAdmin(model=DomainApplication, admin_site=self.site) + + def test_table_sorted_alphabetically(self): + """Tests if DomainApplicationAdmin table is sorted alphabetically""" + # 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 tearDown(self): + """Delete all associated domain objects""" + super().tearDown() + Domain.objects.all().delete() + DomainInformation.objects.all().delete() + DomainApplication.objects.all().delete() + User.objects.all().delete() + Contact.objects.all().delete() + Website.objects.all().delete() + + class TestDomainApplicationAdmin(MockEppLib): def setUp(self): super().setUp() @@ -946,56 +988,6 @@ class TestDomainApplicationAdmin(MockEppLib): unexpected_name = "BadGuy first_name:investigator BadGuy last_name:investigator" self.assertContains(response, unexpected_name, count=2) - def test_table_sorted_alphabetically(self): - """Tests if DomainApplicationAdmin table is sorted alphabetically""" - - # Create a list of DomainApplications in scrambled order - application_1: DomainApplication = generic_domain_object("application", "CupOfTea") - another_user_1 = User.objects.filter(username=application_1.investigator.username).get() - another_user_1.is_staff = True - another_user_1.save() - - application_2: DomainApplication = generic_domain_object("application", "Xylophone") - another_user_2 = User.objects.filter(username=application_2.investigator.username).get() - another_user_2.is_staff = True - another_user_2.save() - - application_3: DomainApplication = generic_domain_object("application", "Blue") - another_user_3 = User.objects.filter(username=application_3.investigator.username).get() - another_user_3.is_staff = True - another_user_3.save() - - application_4: DomainApplication = generic_domain_object("application", "BadGuy") - another_user_4 = User.objects.filter(username=application_4.investigator.username).get() - another_user_4.is_staff = True - another_user_4.save() - - application_5: DomainApplication = generic_domain_object("application", "AppleMan") - application_5 = User.objects.filter(username=application_5.investigator.username).get() - application_5.is_staff = True - application_5.save() - - 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") - - # First, ensure that our test data is not alphabetical - self.assertEqual( - list(DomainApplication.objects.all()), - list(expected_order) - ) - - # Get the returned queryset - queryset = self.admin.get_queryset(request) - - # Then, Check the order of the queryset - self.assertEqual( - list(queryset), - list(expected_order), - ) - def tearDown(self): super().tearDown() Domain.objects.all().delete() From fdb8147c1f66fe901aa72b1f107d8acec312a5fa Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 18 Dec 2023 09:56:04 -0700 Subject: [PATCH 19/29] Fix test cases --- src/registrar/tests/test_admin.py | 85 ++++++++----------------------- 1 file changed, 22 insertions(+), 63 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 2f453ccbd..2d4a70d46 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -316,47 +316,6 @@ class TestDomainApplicationAdminForm(TestCase): ) -class TestDomainApplicationAdminTable(MockEppLib): - """Tests the table for DomainApplicationAdmin""" - def setUp(self): - """Enables epplib patching, and creates a fake admin object""" - super().setUp() - self.site = AdminSite() - self.factory = RequestFactory() - self.superuser = create_superuser() - self.admin = DomainApplicationAdmin(model=DomainApplication, admin_site=self.site) - - def test_table_sorted_alphabetically(self): - """Tests if DomainApplicationAdmin table is sorted alphabetically""" - # 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 tearDown(self): - """Delete all associated domain objects""" - super().tearDown() - Domain.objects.all().delete() - DomainInformation.objects.all().delete() - DomainApplication.objects.all().delete() - User.objects.all().delete() - Contact.objects.all().delete() - Website.objects.all().delete() - - class TestDomainApplicationAdmin(MockEppLib): def setUp(self): super().setUp() @@ -897,6 +856,26 @@ class TestDomainApplicationAdmin(MockEppLib): self.assertEqual(readonly_fields, expected_fields) + def test_table_sorted_alphabetically(self): + """Tests if DomainApplicationAdmin table is sorted alphabetically""" + # 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_displays_investigator_filter(self): """Tests if DomainApplicationAdmin displays the investigator filter""" @@ -911,13 +890,14 @@ class TestDomainApplicationAdmin(MockEppLib): response = self.client.get( "/admin/registrar/domainapplication/", { - "investigator": investigator_user.id, + "investigator__id__exact": investigator_user.id, }, follow=True, ) # Then, test if the filter actually exists self.assertIn("filters", response.context) + # Assert the content of filters and search_query filters = response.context["filters"] @@ -927,27 +907,6 @@ class TestDomainApplicationAdmin(MockEppLib): [ { "parameter_name": "investigator", - "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", }, ], From eb075bfd4b6da1fc0356cf40583f2e379a7f0a0b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 18 Dec 2023 10:02:13 -0700 Subject: [PATCH 20/29] Remove alphabetical test --- src/registrar/tests/test_admin.py | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 2d4a70d46..b4634cacc 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -856,26 +856,6 @@ class TestDomainApplicationAdmin(MockEppLib): self.assertEqual(readonly_fields, expected_fields) - def test_table_sorted_alphabetically(self): - """Tests if DomainApplicationAdmin table is sorted alphabetically""" - # 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_displays_investigator_filter(self): """Tests if DomainApplicationAdmin displays the investigator filter""" From 234d9eb5d7354124f72158f48715182664c7c77a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 19 Dec 2023 08:49:23 -0700 Subject: [PATCH 21/29] Test separating out test file --- src/registrar/tests/test_admin.py | 82 ------------- src/registrar/tests/test_admin_tables.py | 146 +++++++++++++++++++++++ 2 files changed, 146 insertions(+), 82 deletions(-) create mode 100644 src/registrar/tests/test_admin_tables.py diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index b4634cacc..2c83cc545 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -845,88 +845,6 @@ 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 - - # Grab the current list of table filters - 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""" - - # 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__id__exact": investigator_user.id, - }, - follow=True, - ) - - # Then, test if the filter actually exists - self.assertIn("filters", response.context) - - # Assert the content of filters and search_query - filters = response.context["filters"] - - # Ensure that the format is correct. We will test the value later in the test. - self.assertEqual( - filters, - [ - { - "parameter_name": "investigator", - "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() diff --git a/src/registrar/tests/test_admin_tables.py b/src/registrar/tests/test_admin_tables.py new file mode 100644 index 000000000..57cc6fbd1 --- /dev/null +++ b/src/registrar/tests/test_admin_tables.py @@ -0,0 +1,146 @@ +from django.test import TestCase, RequestFactory, Client +from django.contrib.admin.sites import AdminSite + +from registrar.admin import ( + DomainApplicationAdmin, +) +from registrar.models import ( + Domain, + DomainApplication, + DomainInformation, + User, + Contact, + Website +) +from .common import ( + generic_domain_object, + create_superuser, + create_user, + multiple_unalphabetical_domain_objects, +) + +import logging + +logger = logging.getLogger(__name__) + + +class TestDomainApplicationAdmin(TestCase): + def setUp(self): + super().setUp() + self.site = AdminSite() + self.factory = RequestFactory() + 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_has_correct_filters(self): + """Tests if DomainApplicationAdmin has the correct filters""" + 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) + + self.assertEqual(readonly_fields, expected_fields) + + def test_table_sorted_alphabetically(self): + """Tests if DomainApplicationAdmin table is sorted alphabetically""" + # 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_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__id__exact": investigator_user.id, + }, + follow=True, + ) + + # Then, test if the filter actually exists + self.assertIn("filters", response.context) + + # Assert the content of filters and search_query + filters = response.context["filters"] + + # Ensure that the format is correct. We will test the value later in the test. + self.assertEqual( + filters, + [ + { + "parameter_name": "investigator", + "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() + DomainInformation.objects.all().delete() + DomainApplication.objects.all().delete() + User.objects.all().delete() + Contact.objects.all().delete() + Website.objects.all().delete() \ No newline at end of file From 889811b77675658f4664d90fa92a13f8d274c923 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 19 Dec 2023 09:01:21 -0700 Subject: [PATCH 22/29] Undo changes --- src/registrar/tests/test_admin.py | 103 ++++++++++++++++ src/registrar/tests/test_admin_tables.py | 146 ----------------------- 2 files changed, 103 insertions(+), 146 deletions(-) delete mode 100644 src/registrar/tests/test_admin_tables.py diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 2c83cc545..4195f6a79 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -844,6 +844,109 @@ class TestDomainApplicationAdmin(MockEppLib): # Assert that DomainInformation got Deleted 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 + + # Grab the current list of table filters + readonly_fields = self.admin.get_list_filter(request) + expected_fields = ("status", "organization_type", DomainApplicationAdmin.InvestigatorFilter) + + self.assertEqual(readonly_fields, expected_fields) + + def test_table_sorted_alphabetically(self): + """Tests if DomainApplicationAdmin table is sorted alphabetically""" + # 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_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__id__exact": investigator_user.id, + }, + follow=True, + ) + + # Then, test if the filter actually exists + self.assertIn("filters", response.context) + + # Assert the content of filters and search_query + filters = response.context["filters"] + + # Ensure that the format is correct. We will test the value later in the test. + self.assertEqual( + filters, + [ + { + "parameter_name": "investigator", + "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() diff --git a/src/registrar/tests/test_admin_tables.py b/src/registrar/tests/test_admin_tables.py deleted file mode 100644 index 57cc6fbd1..000000000 --- a/src/registrar/tests/test_admin_tables.py +++ /dev/null @@ -1,146 +0,0 @@ -from django.test import TestCase, RequestFactory, Client -from django.contrib.admin.sites import AdminSite - -from registrar.admin import ( - DomainApplicationAdmin, -) -from registrar.models import ( - Domain, - DomainApplication, - DomainInformation, - User, - Contact, - Website -) -from .common import ( - generic_domain_object, - create_superuser, - create_user, - multiple_unalphabetical_domain_objects, -) - -import logging - -logger = logging.getLogger(__name__) - - -class TestDomainApplicationAdmin(TestCase): - def setUp(self): - super().setUp() - self.site = AdminSite() - self.factory = RequestFactory() - 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_has_correct_filters(self): - """Tests if DomainApplicationAdmin has the correct filters""" - 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) - - self.assertEqual(readonly_fields, expected_fields) - - def test_table_sorted_alphabetically(self): - """Tests if DomainApplicationAdmin table is sorted alphabetically""" - # 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_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__id__exact": investigator_user.id, - }, - follow=True, - ) - - # Then, test if the filter actually exists - self.assertIn("filters", response.context) - - # Assert the content of filters and search_query - filters = response.context["filters"] - - # Ensure that the format is correct. We will test the value later in the test. - self.assertEqual( - filters, - [ - { - "parameter_name": "investigator", - "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() - DomainInformation.objects.all().delete() - DomainApplication.objects.all().delete() - User.objects.all().delete() - Contact.objects.all().delete() - Website.objects.all().delete() \ No newline at end of file From 724b549d70611cbe5a796f268bd2b7ba0c3c090d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 19 Dec 2023 09:12:32 -0700 Subject: [PATCH 23/29] Fix test cases broken by recent merge --- src/registrar/tests/test_admin.py | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 204721027..5ac6751ac 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -15,15 +15,7 @@ from registrar.admin import ( ContactAdmin, UserDomainRoleAdmin, ) -from registrar.models import ( - Domain, - DomainApplication, - DomainInformation, - User, - DomainInvitation, - Contact, - Website -) +from registrar.models import Domain, DomainApplication, DomainInformation, User, DomainInvitation, Contact, Website from registrar.models.user_domain_role import UserDomainRole from .common import ( completed_application, @@ -844,7 +836,6 @@ class TestDomainApplicationAdmin(MockEppLib): # Assert that DomainInformation got Deleted with self.assertRaises(DomainInformation.DoesNotExist): domain_information.refresh_from_db() - def test_has_correct_filters(self): """Tests if DomainApplicationAdmin has the correct filters""" @@ -908,7 +899,7 @@ class TestDomainApplicationAdmin(MockEppLib): [ { "parameter_name": "investigator", - "parameter_value": "SomeGuy first_name:investigator SomeGuy last_name:investigator", + "parameter_value": "SomeGuy first_name:creator SomeGuy last_name:creator", }, ], ) @@ -938,15 +929,14 @@ class TestDomainApplicationAdmin(MockEppLib): 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) + expected_name = "SomeGuy first_name:creator SomeGuy last_name:creator" + # We expect to see this six times, two of them are from the html for the filter, + # two are from the page content, and the other two are the html from the list entry in the table. + self.assertContains(response, expected_name, count=6) # 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) + unexpected_name = "BadGuy first_name:creator BadGuy last_name:creator" + self.assertContains(response, unexpected_name, count=0) def tearDown(self): super().tearDown() From ccce752138fc238997850a40d550b1977d03ec9f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 19 Dec 2023 10:57:19 -0700 Subject: [PATCH 24/29] Remove queryset reimplementation --- src/registrar/admin.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index da293f85f..cae13473d 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -489,6 +489,9 @@ class DomainInformationAdmin(ListHeaderAdmin): # For each filter_horizontal, init in admin js extendFilterHorizontalWidgets # to activate the edit/delete/view buttons filter_horizontal = ("other_contacts",) + + # Table ordering + ordering = ["domain__name"] # lists in filter_horizontal are not sorted properly, sort them # by first_name @@ -656,6 +659,9 @@ class DomainApplicationAdmin(ListHeaderAdmin): filter_horizontal = ("current_websites", "alternative_domains", "other_contacts") + # Table ordering + ordering = ["requested_domain__name"] + # lists in filter_horizontal are not sorted properly, sort them # by website def formfield_for_manytomany(self, db_field, request, **kwargs): @@ -670,11 +676,6 @@ class DomainApplicationAdmin(ListHeaderAdmin): 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) - 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: @@ -845,6 +846,9 @@ class DomainAdmin(ListHeaderAdmin): change_list_template = "django/admin/domain_change_list.html" readonly_fields = ["state", "expiration_date"] + # Table ordering + ordering = ["name"] + def export_data_type(self, request): # match the CSV example with all the fields response = HttpResponse(content_type="text/csv") From 2cf4508770d226e3b4de19269dacc036e099cb40 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 19 Dec 2023 12:04:23 -0700 Subject: [PATCH 25/29] Sort alphabetically, add tests --- src/registrar/admin.py | 4 +- src/registrar/tests/test_admin.py | 115 ++++++++++++++++++++++++++++-- 2 files changed, 113 insertions(+), 6 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index cae13473d..5aae76194 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -562,7 +562,9 @@ class DomainApplicationAdmin(ListHeaderAdmin): """Lookup reimplementation, gets users of is_staff. Returns a list of tuples consisting of (user.id, user) """ - privileged_users = User.objects.filter(is_staff=True) + privileged_users = User.objects.filter(is_staff=True).order_by( + "first_name", "last_name", "email" + ) return [(user.id, user) for user in privileged_users] def queryset(self, request, queryset): diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 5ac6751ac..eac2bfdc8 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -838,7 +838,12 @@ class TestDomainApplicationAdmin(MockEppLib): domain_information.refresh_from_db() def test_has_correct_filters(self): - """Tests if DomainApplicationAdmin has the correct filters""" + """ + This test verifies that DomainApplicationAdmin has the correct filters set up. + + It retrieves the current list of filters from DomainApplicationAdmin + and checks that it matches the expected list of filters. + """ request = self.factory.get("/") request.user = self.superuser @@ -849,7 +854,15 @@ class TestDomainApplicationAdmin(MockEppLib): self.assertEqual(readonly_fields, expected_fields) def test_table_sorted_alphabetically(self): - """Tests if DomainApplicationAdmin table is sorted alphabetically""" + """ + This test verifies that the DomainApplicationAdmin table is sorted alphabetically + by the 'requested_domain__name' field. + + It creates a list of DomainApplication instances in a non-alphabetical order, + then retrieves the queryset from the DomainApplicationAdmin and checks + that it matches the expected queryset, + which is sorted alphabetically by the 'requested_domain__name' field. + """ # Creates a list of DomainApplications in scrambled order multiple_unalphabetical_domain_objects("application") @@ -869,7 +882,18 @@ class TestDomainApplicationAdmin(MockEppLib): ) def test_displays_investigator_filter(self): - """Tests if DomainApplicationAdmin displays the investigator filter""" + """ + This test verifies that the investigator filter in the admin interface for + the DomainApplication model displays correctly. + + It creates two DomainApplication instances, each with a different investigator. + It then simulates a staff user logging in and applying the investigator filter + on the DomainApplication admin page. + + We then test if the page displays the filter we expect, but we do not test + if we get back the correct response in the table. This is to isolate if + the filter displays correctly, when the filter isn't filtering correctly. + """ # Create a mock DomainApplication object, with a fake investigator application: DomainApplication = generic_domain_object("application", "SomeGuy") @@ -893,7 +917,6 @@ class TestDomainApplicationAdmin(MockEppLib): # Assert the content of filters and search_query filters = response.context["filters"] - # Ensure that the format is correct. We will test the value later in the test. self.assertEqual( filters, [ @@ -905,7 +928,18 @@ class TestDomainApplicationAdmin(MockEppLib): ) def test_investigator_filter_filters_correctly(self): - """Tests the investigator filter""" + """ + This test verifies that the investigator filter in the admin interface for + the DomainApplication model works correctly. + + It creates two DomainApplication instances, each with a different investigator. + It then simulates a staff user logging in and applying the investigator filter + on the DomainApplication admin page. + + It then verifies that it was applied correctly. + The test checks that the response contains the expected DomainApplication pbjects + in the table. + """ # Create a mock DomainApplication object, with a fake investigator application: DomainApplication = generic_domain_object("application", "SomeGuy") @@ -938,6 +972,77 @@ class TestDomainApplicationAdmin(MockEppLib): unexpected_name = "BadGuy first_name:creator BadGuy last_name:creator" self.assertContains(response, unexpected_name, count=0) + def test_investigator_dropdown_displays_only_staff(self): + """ + This test verifies that the dropdown for the 'investigator' field in the DomainApplicationAdmin + interface only displays users who are marked as staff. + + It creates two DomainApplication instances, one with an investigator + who is a staff user and another with an investigator who is not a staff user. + + It then retrieves the queryset for the 'investigator' dropdown from DomainApplicationAdmin + and checks that it matches the expected queryset, which only includes staff users. + """ + # 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 mock DomainApplication object, with a user that is not staff + application_2: DomainApplication = generic_domain_object("application", "SomeOtherGuy") + investigator_user_2 = User.objects.filter(username=application_2.investigator.username).get() + investigator_user_2.is_staff = False + investigator_user_2.save() + + p = "userpass" + self.client.login(username="staffuser", password=p) + + request = self.factory.post("/admin/registrar/domainapplication/{}/change/".format(application.pk)) + + # Get the actual field from the model's meta information + investigator_field = DomainApplication._meta.get_field("investigator") + + # We should only be displaying staff users, in alphabetical order + expected_dropdown = list(User.objects.filter(is_staff=True)) + current_dropdown = list(self.admin.formfield_for_foreignkey(investigator_field, request).queryset) + + self.assertEqual(expected_dropdown, current_dropdown) + + # Non staff users should not be in the list + self.assertNotIn(application_2, current_dropdown) + + def test_investigator_list_is_alphabetically_sorted(self): + """ """ + # 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() + + application_2: DomainApplication = generic_domain_object("application", "AGuy") + investigator_user_2 = User.objects.filter(username=application_2.investigator.username).get() + investigator_user_2.first_name = "AGuy" + investigator_user_2.is_staff = True + investigator_user_2.save() + + application_3: DomainApplication = generic_domain_object("application", "FinalGuy") + investigator_user_3 = User.objects.filter(username=application_3.investigator.username).get() + investigator_user_3.first_name = "FinalGuy" + investigator_user_3.is_staff = True + investigator_user_3.save() + + p = "userpass" + self.client.login(username="staffuser", password=p) + request = RequestFactory().get("/") + + expected_list = list(User.objects.filter(is_staff=True).order_by("first_name", "last_name", "email")) + + # Get the actual sorted list of investigators from the lookups method + actual_list = [item for _, item in self.admin.InvestigatorFilter.lookups(self, request, self.admin)] + + self.assertEqual(expected_list, actual_list) + def tearDown(self): super().tearDown() Domain.objects.all().delete() From 5dcc5e4a13a0a35e8b463c6ef54510f947b2fdeb Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 19 Dec 2023 12:07:12 -0700 Subject: [PATCH 26/29] Update test_admin.py --- src/registrar/tests/test_admin.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index eac2bfdc8..c36cf649f 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1013,7 +1013,10 @@ class TestDomainApplicationAdmin(MockEppLib): self.assertNotIn(application_2, current_dropdown) def test_investigator_list_is_alphabetically_sorted(self): - """ """ + """ + This test verifies that filter list for the 'investigator' + is displayed alphabetically + """ # 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() From 3eb26e0865cfceff255916703d6a3be406d9e076 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 19 Dec 2023 12:07:54 -0700 Subject: [PATCH 27/29] Linting --- src/registrar/admin.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 5aae76194..5c5f4279c 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -489,7 +489,7 @@ class DomainInformationAdmin(ListHeaderAdmin): # For each filter_horizontal, init in admin js extendFilterHorizontalWidgets # to activate the edit/delete/view buttons filter_horizontal = ("other_contacts",) - + # Table ordering ordering = ["domain__name"] @@ -562,9 +562,7 @@ class DomainApplicationAdmin(ListHeaderAdmin): """Lookup reimplementation, gets users of is_staff. Returns a list of tuples consisting of (user.id, user) """ - privileged_users = User.objects.filter(is_staff=True).order_by( - "first_name", "last_name", "email" - ) + privileged_users = User.objects.filter(is_staff=True).order_by("first_name", "last_name", "email") return [(user.id, user) for user in privileged_users] def queryset(self, request, queryset): From 46dd9c1b386606933ae4a79fdca11b58801953ae Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 27 Dec 2023 08:07:31 -0700 Subject: [PATCH 28/29] Fix merge conflict --- src/registrar/admin.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index d997570ab..d91021225 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -13,7 +13,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.utility.admin_sort_fields import AdminSortFields from registrar.utility import csv_export from . import models from auditlog.models import LogEntry # type: ignore From b68ea1ec9f152b72814e67ea6bd334c7043cf9c5 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 27 Dec 2023 08:17:08 -0700 Subject: [PATCH 29/29] Fix test cases --- src/registrar/tests/test_admin.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 2178f1bdd..0518b4ceb 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -922,7 +922,7 @@ class TestDomainApplicationAdmin(MockEppLib): [ { "parameter_name": "investigator", - "parameter_value": "SomeGuy first_name:creator SomeGuy last_name:creator", + "parameter_value": "SomeGuy first_name:investigator SomeGuy last_name:investigator", }, ], ) @@ -963,14 +963,15 @@ class TestDomainApplicationAdmin(MockEppLib): follow=True, ) - expected_name = "SomeGuy first_name:creator SomeGuy last_name:creator" - # We expect to see this six times, two of them are from the html for the filter, - # two are from the page content, and the other two are the html from the list entry in the table. - self.assertContains(response, expected_name, count=6) + 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, + # and 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. - unexpected_name = "BadGuy first_name:creator BadGuy last_name:creator" - self.assertContains(response, unexpected_name, count=0) + # We expect to see this two times in the filter + unexpected_name = "BadGuy first_name:investigator BadGuy last_name:investigator" + self.assertContains(response, unexpected_name, count=2) def test_investigator_dropdown_displays_only_staff(self): """