Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#3275: Slowness on Domain, Domain Info and Domain Requests - [NL] #3340

Merged
merged 33 commits into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
8432435
Adjusted fixtures to allow specification of number of domain requests…
CocoByte Jan 13, 2025
0a05039
Fixed slowness in DomainRequest, Domain, and DomainInformation admin …
CocoByte Jan 13, 2025
1831702
reducing domain requests fixtures to load only 1000 domain requests (…
CocoByte Jan 13, 2025
f9e62f9
Merge remote-tracking branch 'origin/main' into nl/3275-slowness-admi…
CocoByte Jan 14, 2025
3571f55
Merge branch 'main' into nl/3275-slowness-admin-tables
CocoByte Jan 14, 2025
352e190
fixes
CocoByte Jan 14, 2025
f858571
uni test updates to accommodate lack of _display values
CocoByte Jan 15, 2025
e276f45
cleanup in fixtures
CocoByte Jan 15, 2025
604004d
linted
CocoByte Jan 15, 2025
8700f35
linter errors resolved
CocoByte Jan 15, 2025
f2e284c
Merge remote-tracking branch 'origin/main' into nl/3275-slowness-admi…
CocoByte Jan 15, 2025
68a933f
Merge branch 'main' into nl/3275-slowness-admin-tables
CocoByte Jan 16, 2025
43a1344
temporarily commit fixture for loading 100000 entries
CocoByte Jan 17, 2025
f9fca1b
adjust fixtures for domain request objects to load only 10,000 entrie…
CocoByte Jan 17, 2025
0cee2ec
Corrected comments
CocoByte Jan 17, 2025
90b08f2
Merge branch 'main' into nl/3275-slowness-admin-tables
CocoByte Jan 21, 2025
5995a5c
Merge branch 'main' into nl/3275-slowness-admin-tables
CocoByte Jan 21, 2025
b8dc433
Merge branch 'main' into nl/3275-slowness-admin-tables
CocoByte Jan 22, 2025
f4670fe
restored labels
CocoByte Jan 24, 2025
137399a
Merge remote-tracking branch 'origin/main' into nl/3275-slowness-admi…
CocoByte Jan 24, 2025
ec351b0
updated unit tests
CocoByte Jan 24, 2025
74182b9
use users.Count() for domain Request fixtures
CocoByte Jan 24, 2025
c33ce78
linted
CocoByte Jan 24, 2025
ba2e82c
Merge branch 'main' into nl/3275-slowness-admin-tables
CocoByte Jan 27, 2025
cd0fbe4
PR feedback
CocoByte Jan 27, 2025
28623fb
Merge remote-tracking branch 'origin/main' into nl/3275-slowness-admi…
CocoByte Jan 27, 2025
4f217e4
Sneaking in fix for portfolio unit test
CocoByte Jan 27, 2025
9b91c93
Merge branch 'main' into nl/3275-slowness-admin-tables
CocoByte Jan 28, 2025
c496d01
Update src/registrar/admin.py
CocoByte Jan 28, 2025
b124c1b
Update src/registrar/admin.py
CocoByte Jan 28, 2025
bc2ffb3
linted
CocoByte Jan 28, 2025
ab63451
fix
CocoByte Jan 28, 2025
0881cf2
Merge branch 'main' into nl/3275-slowness-admin-tables
CocoByte Jan 28, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
181 changes: 108 additions & 73 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()
)
CocoByte marked this conversation as resolved.
Show resolved Hide resolved

# 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,27 +2068,35 @@ 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, DomainRequest.BranchChoices.get_branch_label(federal_type)) for federal_type in queryset if federal_type])
CocoByte marked this conversation as resolved.
Show resolved Hide resolved

# 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())
Q(portfolio__federal_agency__federal_type=self.value()) | Q(portfolio__isnull=True, federal_type=self.value())
)
return queryset

Expand Down Expand Up @@ -3226,59 +3240,80 @@ 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__organization_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, DomainRequest.BranchChoices.get_branch_label(federal_type)) for federal_type in queryset if federal_type])
CocoByte marked this conversation as resolved.
Show resolved Hide resolved

# 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}")
CocoByte marked this conversation as resolved.
Show resolved Hide resolved

# 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
Loading