Skip to content

Commit

Permalink
Use NoPkDescOrderedChangeList in comment's admin
Browse files Browse the repository at this point in the history
This will make sure that we don't use extra ordering fields that django normally enforces and would make the query much slower. Also moved LargeTablePaginator to the new admin_helpers file
  • Loading branch information
ffont committed Sep 14, 2023
1 parent e262b6f commit 6ba54ca
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 21 deletions.
22 changes: 1 addition & 21 deletions accounts/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,17 @@
from django.contrib.auth.admin import UserAdmin
from django.contrib.auth.forms import UserChangeForm
from django.contrib.auth.models import User
from django.core.paginator import Paginator
from django.db import connection
from django.forms import ValidationError
from django.http import HttpResponseRedirect
from django.shortcuts import render
from django.urls import reverse
from django.utils.functional import cached_property
from django.utils.safestring import mark_safe
from django_object_actions import DjangoObjectActions

from accounts.forms import username_taken_by_other_user
from accounts.models import Profile, UserFlag, EmailPreferenceType, OldUsername, DeletedUser, UserDeletionRequest, EmailBounce, GdprAcceptance
from general import tasks
from utils.admin_helpers import LargeTablePaginator

web_logger = logging.getLogger("web")

Expand Down Expand Up @@ -103,24 +101,6 @@ def get_num_sounds(self, obj):
return f'{obj.profile.num_sounds}'


class LargeTablePaginator(Paginator):
""" We use the information on postgres table 'reltuples' to avoid using count(*) for performance. """
@cached_property
def count(self):
try:
if not self.object_list.query.where:
cursor = connection.cursor()
cursor.execute("SELECT reltuples FROM pg_class WHERE relname = %s",
[self.object_list.query.model._meta.db_table])
ret = int(cursor.fetchone()[0])
return ret
else :
return self.object_list.count()
except :
# AttributeError if object_list has no count() method.
return len(self.object_list)


class AdminUserForm(UserChangeForm):

def clean_username(self):
Expand Down
7 changes: 7 additions & 0 deletions comments/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from django.contrib import admin

from comments.models import Comment
from utils.admin_helpers import NoPkDescOrderedChangeList


@admin.register(Comment)
Expand All @@ -31,6 +32,7 @@ class CommentAdmin(admin.ModelAdmin):
list_select_related = ('user', 'sound')
list_filter = ('contains_hyperlink',)
search_fields = ('comment', '=user__username', '=sound__id')
ordering = ('-created',)

@admin.display(description='Comment')
def get_comment_summary(self, obj):
Expand All @@ -42,3 +44,8 @@ def has_add_permission(self, request):

def has_change_permission(self, request, obj=None):
return False

def get_changelist(self, request):
# Use custom change list class to avoid ordering by '-pk' in addition to '-created'
# That would cause a slow query as we don't have a combined db index on both fields
return NoPkDescOrderedChangeList
51 changes: 51 additions & 0 deletions utils/admin_helpers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
#
# Freesound is (c) MUSIC TECHNOLOGY GROUP, UNIVERSITAT POMPEU FABRA
#
# Freesound is free software: you can redistribute it and/or modify
# it under the terms of the GNU Affero General Public License as
# published by the Free Software Foundation, either version 3 of the
# License, or (at your option) any later version.
#
# Freesound is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU Affero General Public License for more details.
#
# You should have received a copy of the GNU Affero General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#
# Authors:
# See AUTHORS file.
#

from functools import cached_property

from django.core.paginator import Paginator
from django.contrib.admin.views.main import ChangeList
from django.db import connection


class NoPkDescOrderedChangeList(ChangeList):
def get_ordering(self, request, queryset):
rv = super().get_ordering(request, queryset)
rv = list(rv)
rv.remove('-pk') if '-pk' in rv else None
return tuple(rv)


class LargeTablePaginator(Paginator):
""" We use the information on postgres table 'reltuples' to avoid using count(*) for performance. """
@cached_property
def count(self):
try:
if not self.object_list.query.where:
cursor = connection.cursor()
cursor.execute("SELECT reltuples FROM pg_class WHERE relname = %s",
[self.object_list.query.model._meta.db_table])
ret = int(cursor.fetchone()[0])
return ret
else :
return self.object_list.count()
except :
# AttributeError if object_list has no count() method.
return len(self.object_list)

0 comments on commit 6ba54ca

Please sign in to comment.