Improve performance of sort view for moods and elements.

* Use `{% include  "..." with ... %}` instead of template tags that do
  nothing but pass through or rename context variables and render a
  template. This appears to yield a 2x increase in performance.

  As a side effect, this change also appears to fix some glitches with
  the rendering of `fa-sort`, `fa-sort-asc` and `fa-sort-desc` icons.

* Move queryset filtering from `sort_view()` to new `get_sort_view_queryset()`
  method, so subclasses can override to apply different or additional
  filtering (based on `request` and `sortable_by_expression`) to reduce
  the number of objects being reordered.

  `django-admin-sortable` already provides a mechanism to reorder a
  subset of objects via `sorting_filters`, but this is restricted to a
  small number of hard coded filters, and we found it not very useful.

  We have tens of thousands of nested objects grouped under hundreds or
  thousands of parent objects, and we needed a way to reorder child
  objects just within their own group.

  We also needed a way to reorder a subset of flat (not grouped by
  parent) sortable objects with much more flexibility.

Here's an example of additional filtering that allows us to reorder a
contiguous sequence of objects (nested or flat) that bounded by the min
and max (by ordering) selected objects:

```python
class MyBaseSortableAdmin(SortableAdmin):

    def get_sort_view_queryset(self, request, sortable_by_expression):
        """
        Filter the sort view queryset to include only a contiguous sequence of
        objects between the first and last of given parent objects, according
        to the current ordering.

        This should avoid inconsistent or ambiguous behaviour that might occur
        when re-ordering a non-contiguous sequence.
        """
        sortable_by_expression = sortable_by_expression or 'pk'
        queryset = super(MyBaseSortableAdmin, self) \
            .get_sort_view_queryset(request, sortable_by_expression)
        pks = [
            int(pk) for pk in request.GET.get('pks', '').split(',') if pk
        ]
        if pks:
            queryset = queryset.filter(**{
                '%s__in' % sortable_by_expression: pks,
            })
        return queryset

    def reorder_children(self, qs, child):
        # Get the min and max order field value for the selected objects, then
        # get contiguous PKs for objects between the min and max and pass to
        # the sort view, to avoid inconsistent or ambiguous behaviour.
        field = self.opts.ordering[0].replace('-', '')
        qs = qs.model.objects.filter(**qs.aggregate(**{
            '%s__gte' % field: Min(field),
            '%s__lte' % field: Max(field),
        }))
        ct = ContentType.objects.get_for_model(child)
        url = '%ssort/?pks=%s' % (
            reverse('admin:%s_%s_changelist' % (ct.app_label, ct.model)),
            ','.join([str(pk) for pk in qs.values_list('pk', flat=True)]),
        )
        return http.HttpResponseRedirect(url)

class MyModelAdmin(MyBaseSortableAdmin):
    actions = (
        "reorder_mymodel",
        "reorder_childmodel",
    )

    def reorder_mymodel(self, request, qs):
        return self.reorder_children(qs, MyModel)
    reorder_chapters.short_description = 'Reorder selected MyModels'

    def reorder_childmodel(self, request, qs):
        return self.reorder_children(qs, ChildModel)
    reorder_elements.short_description = 'Reorder ChildModels for the selected MyModels'
```

This could be made generic enough for inclusion by default with a few
tweaks, so that `Reorder selected {{ parent.verbose_name_plural }}` and
`Reorder {{ child.verbose_name_plural }} for selected {{ parent.verbose_name_plural }}`
admin actions could be included in sortable change lists.
master
Tai Lee 2018-03-13 23:05:35 +11:00
parent dcee65eab5
commit 108ef6dd18
7 changed files with 29 additions and 57 deletions

View File

@ -102,6 +102,24 @@ class SortableAdmin(SortableAdminBase, ModelAdmin):
] + urls ] + urls
return urls return urls
def get_sort_view_queryset(self, request, sortable_by_expression):
"""
Return a queryset, optionally filtered based on request and
`sortable_by_expression` to be used in the sort view.
"""
# get sort group index from querystring if present
sort_filter_index = request.GET.get('sort_filter')
filters = {}
if sort_filter_index:
try:
filters = self.model.sorting_filters[int(sort_filter_index)][1]
except (IndexError, ValueError):
pass
# Apply any sort filters to create a subset of sortable objects
return self.get_queryset(request).filter(**filters)
def sort_view(self, request): def sort_view(self, request):
""" """
Custom admin view that displays the objects as a list whose sort Custom admin view that displays the objects as a list whose sort
@ -115,19 +133,6 @@ class SortableAdmin(SortableAdminBase, ModelAdmin):
jquery_lib_path = 'admin/js/jquery.js' if VERSION < (1, 9) \ jquery_lib_path = 'admin/js/jquery.js' if VERSION < (1, 9) \
else 'admin/js/vendor/jquery/jquery.js' else 'admin/js/vendor/jquery/jquery.js'
# get sort group index from querystring if present
sort_filter_index = request.GET.get('sort_filter')
filters = {}
if sort_filter_index:
try:
filters = self.model.sorting_filters[int(sort_filter_index)][1]
except (IndexError, ValueError):
pass
# Apply any sort filters to create a subset of sortable objects
objects = self.get_queryset(request).filter(**filters)
# Determine if we need to regroup objects relative to a # Determine if we need to regroup objects relative to a
# foreign key specified on the model class that is extending Sortable. # foreign key specified on the model class that is extending Sortable.
# Legacy support for 'sortable_by' defined as a model property # Legacy support for 'sortable_by' defined as a model property
@ -175,6 +180,8 @@ class SortableAdmin(SortableAdminBase, ModelAdmin):
sortable_by_class_display_name = \ sortable_by_class_display_name = \
sortable_by_class_is_sortable = None sortable_by_class_is_sortable = None
objects = self.get_sort_view_queryset(request, sortable_by_expression)
if sortable_by_property or sortable_by_fk: if sortable_by_property or sortable_by_fk:
# Order the objects by the property they are sortable by, # Order the objects by the property they are sortable by,
# then by the order, otherwise the regroup # then by the order, otherwise the regroup

View File

@ -1,5 +1,5 @@
{% extends "admin/base_site.html" %} {% extends "admin/base_site.html" %}
{% load i18n admin_urls static admin_list adminsortable_tags %} {% load i18n admin_urls static admin_list %}
{% block extrastyle %} {% block extrastyle %}
{{ block.super }} {{ block.super }}
@ -86,9 +86,9 @@
{% if objects %} {% if objects %}
<div id="sortable"> <div id="sortable">
{% if group_expression %} {% if group_expression %}
{% render_nested_sortable_objects objects group_expression %} {% include "adminsortable/shared/nested_objects.html" %}
{% else %} {% else %}
{% render_sortable_objects objects %} {% include "adminsortable/shared/objects.html" %}
{% endif %} {% endif %}
{% csrf_token %} {% csrf_token %}
</div> </div>

View File

@ -1,9 +1,8 @@
{% load adminsortable_tags %}
{% with list_objects_length=list_objects|length %} {% with list_objects_length=list_objects|length %}
{% for object in list_objects %} {% for object in list_objects %}
<li> <li>
{% if list_objects_length > 1 %} {% if list_objects_length > 1 %}
{% render_object_rep object forloop %} {% include "adminsortable/shared/object_rep.html" %}
{% else %} {% else %}
{{ object }} {{ object }}
{% endif %} {% endif %}

View File

@ -1,4 +1,4 @@
{% load django_template_additions adminsortable_tags %} {% load django_template_additions %}
{% dynamic_regroup objects by group_expression as regrouped_objects %} {% dynamic_regroup objects by group_expression as regrouped_objects %}
{% if regrouped_objects %} {% if regrouped_objects %}
<ul {% if sortable_by_class_is_sortable %}class="sortable"{% endif %}> <ul {% if sortable_by_class_is_sortable %}class="sortable"{% endif %}>
@ -6,7 +6,7 @@
{% with object=regrouped_object.grouper %} {% with object=regrouped_object.grouper %}
{% if object %} {% if object %}
<li class="parent">{% if sortable_by_class_is_sortable %} <li class="parent">{% if sortable_by_class_is_sortable %}
{% render_object_rep object forloop %} {% include "adminsortable/shared/object_rep.html" %}
{% else %} {% else %}
{{ object }} {{ object }}
{% endif %} {% endif %}
@ -14,7 +14,7 @@
{% if regrouped_object.list %} {% if regrouped_object.list %}
{% with regrouped_object_list_length=regrouped_object.list|length %} {% with regrouped_object_list_length=regrouped_object.list|length %}
<ul {% if regrouped_object_list_length > 1 %}class="sortable"{% endif %}> <ul {% if regrouped_object_list_length > 1 %}class="sortable"{% endif %}>
{% render_list_items regrouped_object.list %} {% include "adminsortable/shared/list_items.html" with list_objects=regrouped_object.list %}
</ul> </ul>
{% endwith %} {% endwith %}
{% endif %} {% endif %}

View File

@ -1,4 +1,4 @@
{% load adminsortable_tags admin_urls %} {% load admin_urls %}
<form> <form>
<input name="pk" type="hidden" value="{{ object.pk }}" /> <input name="pk" type="hidden" value="{{ object.pk }}" />

View File

@ -1,7 +1,5 @@
{% load adminsortable_tags %}
{% if objects %} {% if objects %}
<ul class="sortable single"> <ul class="sortable single">
{% render_list_items objects %} {% include "adminsortable/shared/list_items.html" with list_objects=objects %}
</ul> </ul>
{% endif %} {% endif %}

View File

@ -1,32 +0,0 @@
from django.template.loader import render_to_string
from django import template
register = template.Library()
@register.simple_tag(takes_context=True)
def render_sortable_objects(context, objects,
sortable_objects_template='adminsortable/shared/objects.html'):
context.update({'objects': objects})
return render_to_string(sortable_objects_template, context.flatten())
@register.simple_tag(takes_context=True)
def render_nested_sortable_objects(context, objects, group_expression,
sortable_nested_objects_template='adminsortable/shared/nested_objects.html'):
context.update({'objects': objects, 'group_expression': group_expression})
return render_to_string(sortable_nested_objects_template, context.flatten())
@register.simple_tag(takes_context=True)
def render_list_items(context, list_objects,
sortable_list_items_template='adminsortable/shared/list_items.html'):
context.update({'list_objects': list_objects})
return render_to_string(sortable_list_items_template, context.flatten())
@register.simple_tag(takes_context=True)
def render_object_rep(context, obj, forloop,
sortable_object_rep_template='adminsortable/shared/object_rep.html'):
context.update({'object': obj, 'forloop': forloop})
return render_to_string(sortable_object_rep_template, context.flatten())