Skip to content

Commit

Permalink
Merge pull request #3340 from cisagov/nl/3275-slowness-admin-tables
Browse files Browse the repository at this point in the history
#3275: Slowness on Domain, Domain Info and Domain Requests - [NL]
  • Loading branch information
CocoByte authored Jan 28, 2025
2 parents 182c26e + 0881cf2 commit 093203e
Show file tree
Hide file tree
Showing 5 changed files with 168 additions and 87 deletions.
190 changes: 119 additions & 71 deletions src/registrar/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -1678,22 +1678,25 @@ class GenericOrgFilter(admin.SimpleListFilter):
parameter_name = "converted_generic_orgs"

def lookups(self, request, model_admin):
converted_generic_orgs = set()

# Populate the set with tuples of (value, display value)
for domain_info in DomainInformation.objects.all():
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
# 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()
)

# Sort the set by display value
return sorted(converted_generic_orgs, key=lambda x: x[1]) # x[1] is the display value
# Filter out empty results and return sorted list of unique values
return sorted([(org, DomainRequest.OrganizationChoices.get_org_label(org)) for org in queryset if org])

# Filter 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(
Q(portfolio__organization_type=self.value())
| Q(portfolio__isnull=True, generic_org_type=self.value())
Expand Down Expand Up @@ -2031,22 +2034,25 @@ class GenericOrgFilter(admin.SimpleListFilter):
parameter_name = "converted_generic_orgs"

def lookups(self, request, model_admin):
converted_generic_orgs = set()

# Populate the set with tuples of (value, display value)
for domain_request in DomainRequest.objects.all():
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
# 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()
)

# Sort the set by display value
return sorted(converted_generic_orgs, key=lambda x: x[1]) # x[1] is the display value
# Filter out empty results and return sorted list of unique values
return sorted([(org, DomainRequest.OrganizationChoices.get_org_label(org)) for org in queryset if org])

# Filter 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(
Q(portfolio__organization_type=self.value())
| Q(portfolio__isnull=True, generic_org_type=self.value())
Expand All @@ -2062,24 +2068,39 @@ class FederalTypeFilter(admin.SimpleListFilter):
parameter_name = "converted_federal_types"

def lookups(self, request, model_admin):
converted_federal_types = set()

# Populate the set with tuples of (value, display value)
for domain_request in DomainRequest.objects.all():
converted_federal_type = domain_request.converted_federal_type # Actual value
converted_federal_type_display = domain_request.converted_federal_type_display # Display value

if converted_federal_type:
converted_federal_types.add(
(converted_federal_type, converted_federal_type_display) # Value, Display
# Annotate the queryset for efficient filtering
queryset = (
DomainRequest.objects.annotate(
converted_federal_type=Case(
When(
portfolio__isnull=False,
portfolio__federal_agency__federal_type__isnull=False,
then="portfolio__federal_agency__federal_type",
),
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
return sorted(converted_federal_types, key=lambda x: x[1]) # x[1] is the display value
# Filter out empty values and return sorted unique entries
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):
if self.value(): # Check if a federal type is selected in the filter
if self.value():
return queryset.filter(
Q(portfolio__federal_agency__federal_type=self.value())
| Q(portfolio__isnull=True, federal_type=self.value())
Expand Down Expand Up @@ -3226,59 +3247,86 @@ class GenericOrgFilter(admin.SimpleListFilter):
parameter_name = "converted_generic_orgs"

def lookups(self, request, model_admin):
converted_generic_orgs = set()

# Populate the set with tuples of (value, display value)
for domain_info in DomainInformation.objects.all():
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
# 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()
)

# Sort the set by display value
return sorted(converted_generic_orgs, key=lambda x: x[1]) # x[1] is the display value
# Filter out empty results and return sorted list of unique values
return sorted([(org, DomainRequest.OrganizationChoices.get_org_label(org)) for org in queryset if org])

# Filter 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(
Q(domain_info__portfolio__organization_type=self.value())
| Q(domain_info__portfolio__isnull=True, domain_info__generic_org_type=self.value())
)

return queryset

class FederalTypeFilter(admin.SimpleListFilter):
"""Custom Federal Type filter that accomodates portfolio feature.
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"
parameter_name = "converted_federal_types"

def lookups(self, request, model_admin):
converted_federal_types = set()

# Populate the set with tuples of (value, display value)
for domain_info in DomainInformation.objects.all():
converted_federal_type = domain_info.converted_federal_type # Actual value
converted_federal_type_display = domain_info.converted_federal_type_display # Display value

if converted_federal_type:
converted_federal_types.add(
(converted_federal_type, converted_federal_type_display) # Value, Display
# Annotate the queryset for efficient filtering
queryset = (
Domain.objects.annotate(
converted_federal_type=Case(
When(
domain_info__isnull=False,
domain_info__portfolio__isnull=False,
then=F("domain_info__portfolio__federal_agency__federal_type"),
),
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
return sorted(converted_federal_types, key=lambda x: x[1]) # x[1] is the display value
# Filter out empty values and return sorted unique entries
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):
if self.value(): # Check if a federal type is selected in the filter
if self.value():
return queryset.filter(
Q(domain_info__portfolio__federal_agency__federal_type=self.value())
| Q(domain_info__portfolio__isnull=True, domain_info__federal_agency__federal_type=self.value())
Q(domain_info__portfolio__federal_type=self.value())
| Q(domain_info__portfolio__isnull=True, domain_info__federal_type=self.value())
)
return queryset

Expand Down
56 changes: 42 additions & 14 deletions src/registrar/fixtures/fixtures_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,22 +323,50 @@ def load(cls):
cls._create_domain_requests(users)

@classmethod
def _create_domain_requests(cls, users):
def _create_domain_requests(cls, users): # noqa: C901
"""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 = []
for user in users:
for request_data in cls.DOMAINREQUESTS:
# Prepare DomainRequest objects
try:
domain_request = DomainRequest(
creator=user,
organization_name=request_data["organization_name"],
)
cls._set_non_foreign_key_fields(domain_request, request_data)
cls._set_foreign_key_fields(domain_request, request_data, user)
domain_requests_to_create.append(domain_request)
except Exception as e:
logger.warning(e)
if domain_requests_already_made < total_domain_requests_to_make:
for user in users:
for request_data in cls.DOMAINREQUESTS:
# Prepare DomainRequest objects
try:
domain_request = DomainRequest(
creator=user,
organization_name=request_data["organization_name"],
)
cls._set_non_foreign_key_fields(domain_request, request_data)
cls._set_foreign_key_fields(domain_request, request_data, user)
domain_requests_to_create.append(domain_request)
except Exception as 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
cls._bulk_create_requests(domain_requests_to_create)
Expand Down
2 changes: 1 addition & 1 deletion src/registrar/tests/test_admin_domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -779,7 +779,7 @@ def test_short_org_name_in_domains_list(self):
response = self.client.get("/admin/registrar/domain/")
# There are 4 template references to Federal (4) plus four references in the table
# for our actual domain_request
self.assertContains(response, "Federal", count=57)
self.assertContains(response, "Federal", count=56)
# This may be a bit more robust
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
Expand Down
2 changes: 1 addition & 1 deletion src/registrar/tests/test_admin_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ def test_short_org_name_in_domain_requests_list(self):
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
# of the request
self.assertContains(response, "Federal", count=55)
self.assertContains(response, "Federal", count=54)
# This may be a bit more robust
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
Expand Down
5 changes: 5 additions & 0 deletions src/registrar/tests/test_management_scripts.py
Original file line number Diff line number Diff line change
Expand Up @@ -2217,6 +2217,11 @@ def setUp(self):

def tearDown(self):
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")
def test_delete_unlisted_portfolios(self, mock_query_yes_no):
Expand Down

0 comments on commit 093203e

Please sign in to comment.