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.

Sign up for GitHub

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

Get sources list for filter from content_provider table #5180

Merged
merged 1 commit into from
Nov 25, 2024
Merged

Get sources list for filter from content_provider table #5180

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 26 additions & 6 deletions api/api/admin/media_report.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
AudioDecision,
AudioDecisionThrough,
AudioReport,
ContentSource,
DeletedAudio,
DeletedImage,
Image,
Expand Down Expand Up @@ -170,6 +171,30 @@ def _get_default_ordering(self):
return []


def get_source_filter(media_type: str):
class SourceFilter(admin.SimpleListFilter):
"""
Custom filter for source field in admin list view to prevent Django
from performing expensive `DISTINCT` query on media tables.
"""

title = "source"
parameter_name = "source"

def lookups(self, request, model_admin):
return ContentSource.objects.filter(media_type=media_type).values_list(
"source_identifier", "source_name"
)

def queryset(self, request, queryset):
value = self.value()
if value is not None:
queryset = queryset.filter(source=value)
return queryset

return SourceFilter


def get_pending_record_filter(media_type: str):
class PendingRecordCountFilter(admin.SimpleListFilter):
title = "pending record count"
Expand Down Expand Up @@ -528,8 +553,7 @@ def has_sensitive_text(self, obj):

def get_list_filter(self, request):
return (
"source",
"provider",
get_source_filter(self.media_type),
get_pending_record_filter(self.media_type),
)

Expand Down Expand Up @@ -866,10 +890,6 @@ def is_pending(self, obj):
"is_pending",
"media_id", # used because ``media_obj`` does not render a link
)
list_filter = (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this filter removed, @krysal ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to reduce potential issues when rendering the page, and the filters declared this way will result in more DISTINCT queries, which are prohibitively expensive in the image table. After confirming we can access the image list, we can re-add these filters optimally (like done here with sources), although I don't see them listed in the admin currently.

"reason",
("decision", admin.EmptyFieldListFilter), # ~is_pending
)
search_fields = ("description", *_production_deferred("media_obj__identifier"))

@admin.display(description="Media obj")
Expand Down
Loading