Merge pull request #3340 from cisagov/nl/3275-slowness-admin-tables

#3275: Slowness on Domain, Domain Info and Domain Requests - [NL]
This commit is contained in:
CuriousX 2025-01-28 12:25:04 -07:00 committed by GitHub
commit 093203e848
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 168 additions and 87 deletions

View file

@ -1678,22 +1678,25 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin):
parameter_name = "converted_generic_orgs" parameter_name = "converted_generic_orgs"
def lookups(self, request, model_admin): def lookups(self, request, model_admin):
converted_generic_orgs = set() # Annotate the queryset to avoid Python-side iteration
queryset = (
DomainInformation.objects.annotate(
converted_generic_org=Case(
When(portfolio__organization_type__isnull=False, then="portfolio__organization_type"),
When(portfolio__isnull=True, generic_org_type__isnull=False, then="generic_org_type"),
default=Value(""),
output_field=CharField(),
)
)
.values_list("converted_generic_org", flat=True)
.distinct()
)
# Populate the set with tuples of (value, display value) # Filter out empty results and return sorted list of unique values
for domain_info in DomainInformation.objects.all(): return sorted([(org, DomainRequest.OrganizationChoices.get_org_label(org)) for org in queryset if org])
converted_generic_org = domain_info.converted_generic_org_type # Actual value
converted_generic_org_display = domain_info.converted_generic_org_type_display # Display value
if converted_generic_org:
converted_generic_orgs.add((converted_generic_org, converted_generic_org_display)) # Value, Display
# Sort the set by display value
return sorted(converted_generic_orgs, key=lambda x: x[1]) # x[1] is the display value
# Filter queryset
def queryset(self, request, queryset): def queryset(self, request, queryset):
if self.value(): # Check if a generic org is selected in the filter if self.value():
return queryset.filter( return queryset.filter(
Q(portfolio__organization_type=self.value()) Q(portfolio__organization_type=self.value())
| Q(portfolio__isnull=True, generic_org_type=self.value()) | Q(portfolio__isnull=True, generic_org_type=self.value())
@ -2031,22 +2034,25 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin):
parameter_name = "converted_generic_orgs" parameter_name = "converted_generic_orgs"
def lookups(self, request, model_admin): def lookups(self, request, model_admin):
converted_generic_orgs = set() # Annotate the queryset to avoid Python-side iteration
queryset = (
DomainRequest.objects.annotate(
converted_generic_org=Case(
When(portfolio__organization_type__isnull=False, then="portfolio__organization_type"),
When(portfolio__isnull=True, generic_org_type__isnull=False, then="generic_org_type"),
default=Value(""),
output_field=CharField(),
)
)
.values_list("converted_generic_org", flat=True)
.distinct()
)
# Populate the set with tuples of (value, display value) # Filter out empty results and return sorted list of unique values
for domain_request in DomainRequest.objects.all(): return sorted([(org, DomainRequest.OrganizationChoices.get_org_label(org)) for org in queryset if org])
converted_generic_org = domain_request.converted_generic_org_type # Actual value
converted_generic_org_display = domain_request.converted_generic_org_type_display # Display value
if converted_generic_org:
converted_generic_orgs.add((converted_generic_org, converted_generic_org_display)) # Value, Display
# Sort the set by display value
return sorted(converted_generic_orgs, key=lambda x: x[1]) # x[1] is the display value
# Filter queryset
def queryset(self, request, queryset): def queryset(self, request, queryset):
if self.value(): # Check if a generic org is selected in the filter if self.value():
return queryset.filter( return queryset.filter(
Q(portfolio__organization_type=self.value()) Q(portfolio__organization_type=self.value())
| Q(portfolio__isnull=True, generic_org_type=self.value()) | Q(portfolio__isnull=True, generic_org_type=self.value())
@ -2062,24 +2068,39 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin):
parameter_name = "converted_federal_types" parameter_name = "converted_federal_types"
def lookups(self, request, model_admin): def lookups(self, request, model_admin):
converted_federal_types = set() # Annotate the queryset for efficient filtering
queryset = (
# Populate the set with tuples of (value, display value) DomainRequest.objects.annotate(
for domain_request in DomainRequest.objects.all(): converted_federal_type=Case(
converted_federal_type = domain_request.converted_federal_type # Actual value When(
converted_federal_type_display = domain_request.converted_federal_type_display # Display value portfolio__isnull=False,
portfolio__federal_agency__federal_type__isnull=False,
if converted_federal_type: then="portfolio__federal_agency__federal_type",
converted_federal_types.add( ),
(converted_federal_type, converted_federal_type_display) # Value, Display When(
portfolio__isnull=True,
federal_agency__federal_type__isnull=False,
then="federal_agency__federal_type",
),
default=Value(""),
output_field=CharField(),
)
)
.values_list("converted_federal_type", flat=True)
.distinct()
) )
# Sort the set by display value # Filter out empty values and return sorted unique entries
return sorted(converted_federal_types, key=lambda x: x[1]) # x[1] is the display value return sorted(
[
(federal_type, BranchChoices.get_branch_label(federal_type))
for federal_type in queryset
if federal_type
]
)
# Filter queryset
def queryset(self, request, queryset): def queryset(self, request, queryset):
if self.value(): # Check if a federal type is selected in the filter if self.value():
return queryset.filter( return queryset.filter(
Q(portfolio__federal_agency__federal_type=self.value()) Q(portfolio__federal_agency__federal_type=self.value())
| Q(portfolio__isnull=True, federal_type=self.value()) | Q(portfolio__isnull=True, federal_type=self.value())
@ -3226,59 +3247,86 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin):
parameter_name = "converted_generic_orgs" parameter_name = "converted_generic_orgs"
def lookups(self, request, model_admin): def lookups(self, request, model_admin):
converted_generic_orgs = set() # Annotate the queryset to avoid Python-side iteration
queryset = (
Domain.objects.annotate(
converted_generic_org=Case(
When(
domain_info__isnull=False,
domain_info__portfolio__organization_type__isnull=False,
then="domain_info__portfolio__organization_type",
),
When(
domain_info__isnull=False,
domain_info__portfolio__isnull=True,
domain_info__generic_org_type__isnull=False,
then="domain_info__generic_org_type",
),
default=Value(""),
output_field=CharField(),
)
)
.values_list("converted_generic_org", flat=True)
.distinct()
)
# Populate the set with tuples of (value, display value) # Filter out empty results and return sorted list of unique values
for domain_info in DomainInformation.objects.all(): return sorted([(org, DomainRequest.OrganizationChoices.get_org_label(org)) for org in queryset if org])
converted_generic_org = domain_info.converted_generic_org_type # Actual value
converted_generic_org_display = domain_info.converted_generic_org_type_display # Display value
if converted_generic_org:
converted_generic_orgs.add((converted_generic_org, converted_generic_org_display)) # Value, Display
# Sort the set by display value
return sorted(converted_generic_orgs, key=lambda x: x[1]) # x[1] is the display value
# Filter queryset
def queryset(self, request, queryset): def queryset(self, request, queryset):
if self.value(): # Check if a generic org is selected in the filter if self.value():
return queryset.filter( return queryset.filter(
Q(domain_info__portfolio__organization_type=self.value()) Q(domain_info__portfolio__organization_type=self.value())
| Q(domain_info__portfolio__isnull=True, domain_info__generic_org_type=self.value()) | Q(domain_info__portfolio__isnull=True, domain_info__generic_org_type=self.value())
) )
return queryset return queryset
class FederalTypeFilter(admin.SimpleListFilter): class FederalTypeFilter(admin.SimpleListFilter):
"""Custom Federal Type filter that accomodates portfolio feature. """Custom Federal Type filter that accomodates portfolio feature.
If we have a portfolio, use the portfolio's federal type. If not, use the If we have a portfolio, use the portfolio's federal type. If not, use the
federal type in the Domain Information object.""" organization in the Domain Request object."""
title = "federal type" title = "federal type"
parameter_name = "converted_federal_types" parameter_name = "converted_federal_types"
def lookups(self, request, model_admin): def lookups(self, request, model_admin):
converted_federal_types = set() # Annotate the queryset for efficient filtering
queryset = (
# Populate the set with tuples of (value, display value) Domain.objects.annotate(
for domain_info in DomainInformation.objects.all(): converted_federal_type=Case(
converted_federal_type = domain_info.converted_federal_type # Actual value When(
converted_federal_type_display = domain_info.converted_federal_type_display # Display value domain_info__isnull=False,
domain_info__portfolio__isnull=False,
if converted_federal_type: then=F("domain_info__portfolio__federal_agency__federal_type"),
converted_federal_types.add( ),
(converted_federal_type, converted_federal_type_display) # Value, Display When(
domain_info__isnull=False,
domain_info__portfolio__isnull=True,
domain_info__federal_type__isnull=False,
then="domain_info__federal_agency__federal_type",
),
default=Value(""),
output_field=CharField(),
)
)
.values_list("converted_federal_type", flat=True)
.distinct()
) )
# Sort the set by display value # Filter out empty values and return sorted unique entries
return sorted(converted_federal_types, key=lambda x: x[1]) # x[1] is the display value return sorted(
[
(federal_type, BranchChoices.get_branch_label(federal_type))
for federal_type in queryset
if federal_type
]
)
# Filter queryset
def queryset(self, request, queryset): def queryset(self, request, queryset):
if self.value(): # Check if a federal type is selected in the filter if self.value():
return queryset.filter( return queryset.filter(
Q(domain_info__portfolio__federal_agency__federal_type=self.value()) Q(domain_info__portfolio__federal_type=self.value())
| Q(domain_info__portfolio__isnull=True, domain_info__federal_agency__federal_type=self.value()) | Q(domain_info__portfolio__isnull=True, domain_info__federal_type=self.value())
) )
return queryset return queryset

View file

@ -323,9 +323,18 @@ class DomainRequestFixture:
cls._create_domain_requests(users) cls._create_domain_requests(users)
@classmethod @classmethod
def _create_domain_requests(cls, users): def _create_domain_requests(cls, users): # noqa: C901
"""Creates DomainRequests given a list of users.""" """Creates DomainRequests given a list of users."""
total_domain_requests_to_make = len(users) # 100000
# Check if the database is already populated with the desired
# number of entries.
# (Prevents re-adding more entries to an already populated database,
# which happens when restarting Docker src)
domain_requests_already_made = DomainRequest.objects.count()
domain_requests_to_create = [] domain_requests_to_create = []
if domain_requests_already_made < total_domain_requests_to_make:
for user in users: for user in users:
for request_data in cls.DOMAINREQUESTS: for request_data in cls.DOMAINREQUESTS:
# Prepare DomainRequest objects # Prepare DomainRequest objects
@ -340,6 +349,25 @@ class DomainRequestFixture:
except Exception as e: except Exception as e:
logger.warning(e) logger.warning(e)
num_additional_requests_to_make = (
total_domain_requests_to_make - domain_requests_already_made - len(domain_requests_to_create)
)
if num_additional_requests_to_make > 0:
for _ in range(num_additional_requests_to_make):
random_user = random.choice(users) # nosec
try:
random_request_type = random.choice(cls.DOMAINREQUESTS) # nosec
# Prepare DomainRequest objects
domain_request = DomainRequest(
creator=random_user,
organization_name=random_request_type["organization_name"],
)
cls._set_non_foreign_key_fields(domain_request, random_request_type)
cls._set_foreign_key_fields(domain_request, random_request_type, random_user)
domain_requests_to_create.append(domain_request)
except Exception as e:
logger.warning(f"Error creating random domain request: {e}")
# Bulk create domain requests # Bulk create domain requests
cls._bulk_create_requests(domain_requests_to_create) cls._bulk_create_requests(domain_requests_to_create)

View file

@ -779,7 +779,7 @@ class TestDomainAdminWithClient(TestCase):
response = self.client.get("/admin/registrar/domain/") response = self.client.get("/admin/registrar/domain/")
# There are 4 template references to Federal (4) plus four references in the table # There are 4 template references to Federal (4) plus four references in the table
# for our actual domain_request # for our actual domain_request
self.assertContains(response, "Federal", count=57) self.assertContains(response, "Federal", count=56)
# This may be a bit more robust # This may be a bit more robust
self.assertContains(response, '<td class="field-converted_generic_org_type">Federal</td>', count=1) self.assertContains(response, '<td class="field-converted_generic_org_type">Federal</td>', count=1)
# Now let's make sure the long description does not exist # Now let's make sure the long description does not exist

View file

@ -662,7 +662,7 @@ class TestDomainRequestAdmin(MockEppLib):
response = self.client.get("/admin/registrar/domainrequest/?generic_org_type__exact=federal") response = self.client.get("/admin/registrar/domainrequest/?generic_org_type__exact=federal")
# There are 2 template references to Federal (4) and two in the results data # There are 2 template references to Federal (4) and two in the results data
# of the request # of the request
self.assertContains(response, "Federal", count=55) self.assertContains(response, "Federal", count=54)
# This may be a bit more robust # This may be a bit more robust
self.assertContains(response, '<td class="field-converted_generic_org_type">Federal</td>', count=1) self.assertContains(response, '<td class="field-converted_generic_org_type">Federal</td>', count=1)
# Now let's make sure the long description does not exist # Now let's make sure the long description does not exist

View file

@ -2217,6 +2217,11 @@ class TestRemovePortfolios(TestCase):
def tearDown(self): def tearDown(self):
self.logger_patcher.stop() self.logger_patcher.stop()
DomainInformation.objects.all().delete()
DomainRequest.objects.all().delete()
Suborganization.objects.all().delete()
Portfolio.objects.all().delete()
User.objects.all().delete()
@patch("registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no") @patch("registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no")
def test_delete_unlisted_portfolios(self, mock_query_yes_no): def test_delete_unlisted_portfolios(self, mock_query_yes_no):