Merge pull request #230 from alsoicode/develop

Develop

.gitignore

@@ -11,5 +11,8 @@ atlassian-*
 .codeintel
 __pycache__
 .venv/
+.coverage
+.tox/
+htmlcov/
 build
 .vscode/*

.travis.yml

@@ -1,12 +1,14 @@
 language: python
 
 python:
-    - "3.4"
     - "3.5"
     - "3.6"
+    - "3.7"
+    - "3.8"
 
 env:
-    - DJANGO_VERSION=3.0.0 SAMPLE_PROJECT=sample_project
+    - DJANGO_VERSION=2.2 SAMPLE_PROJECT=sample_project
+    - DJANGO_VERSION=3.0 SAMPLE_PROJECT=sample_project
 
 branches:
     only:
@@ -14,13 +16,9 @@ branches:
 
 matrix:
     exclude:
-        -
-            python: "3.4"
-            env: DJANGO_VERSION=3.0.0 SAMPLE_PROJECT=sample_project
-
         -
             python: "3.5"
-            env: DJANGO_VERSION=3.0.0 SAMPLE_PROJECT=sample_project
+            env: DJANGO_VERSION=3.0 SAMPLE_PROJECT=sample_project
 
 install:
     - pip install django==$DJANGO_VERSION

adminsortable/admin.py

@@ -1,4 +1,5 @@
 import json
+from urllib.parse import urlencode
 
 from django import VERSION
 
@@ -11,10 +12,12 @@ from django.contrib.contenttypes.admin import (GenericStackedInline,
                                                GenericTabularInline)
 from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import PermissionDenied
-from django.http import HttpResponse
+from django.db import transaction
+from django.http import HttpResponse, HttpResponseBadRequest
 from django.shortcuts import render
 from django.template.defaultfilters import capfirst
 from django.utils.decorators import method_decorator
+from django.utils.translation import gettext as _
 from django.views.decorators.http import require_POST
 
 from adminsortable.fields import SortableForeignKey
@@ -182,7 +185,9 @@ class SortableAdmin(SortableAdminBase, ModelAdmin):
                     # Django < 1.9
                     sortable_by_fk = field.rel.to
                 sortable_by_field_name = field.name.lower()
-                sortable_by_class_is_sortable = sortable_by_fk.objects.count() >= 2
+                sortable_by_class_is_sortable = \
+                    isinstance(sortable_by_fk, SortableMixin) and \
+                    sortable_by_fk.objects.count() >= 2
 
         if sortable_by_property:
             # backwards compatibility for < 1.1.1, where sortable_by was a
@@ -234,6 +239,8 @@ class SortableAdmin(SortableAdminBase, ModelAdmin):
         else:
             context = self.admin_site.each_context(request)
 
+        filters = urlencode(self.get_querystring_filters(request))
+
         context.update({
             'title': u'Drag and drop {0} to change display order'.format(
                 capfirst(verbose_name_plural)),
@@ -244,6 +251,7 @@ class SortableAdmin(SortableAdminBase, ModelAdmin):
             'sortable_by_class': sortable_by_class,
             'sortable_by_class_is_sortable': sortable_by_class_is_sortable,
             'sortable_by_class_display_name': sortable_by_class_display_name,
+            'filters': filters,
             'jquery_lib_path': jquery_lib_path,
             'csrf_cookie_name': getattr(settings, 'CSRF_COOKIE_NAME', 'csrftoken'),
             'csrf_header_name': getattr(settings, 'CSRF_HEADER_NAME', 'X-CSRFToken'),
@@ -290,41 +298,62 @@ class SortableAdmin(SortableAdminBase, ModelAdmin):
         response = {'objects_sorted': False}
 
         if request.is_ajax():
-            try:
+            klass = ContentType.objects.get(id=model_type_id).model_class()
-                klass = ContentType.objects.get(id=model_type_id).model_class()
 
-                indexes = list(map(str,
+            indexes = [str(idx) for idx in request.POST.get('indexes', []).split(',')]
-                    request.POST.get('indexes', []).split(',')))
-                objects_dict = dict([(str(obj.pk), obj) for obj in
-                    klass.objects.filter(pk__in=indexes)])
 
+            # apply any filters via the querystring
+            filters = self.get_querystring_filters(request)
+
+            filters['pk__in'] = indexes
+
+            # Lock rows that we might update
+            qs = klass.objects.select_for_update().filter(**filters)
+
+            with transaction.atomic():
+
+                # Python 3.6+ only
+                # objects_dict = {str(obj.pk): obj for obj in qs}
+                # objects_list = list(objects_dict.keys())
+                objects_dict = {}
+                objects_list = []
+                for obj in qs:
+                    key = str(obj.pk)
+                    objects_dict[key] = obj
+                    objects_list.append(key)
+                if len(indexes) != len(objects_dict):
+                    return HttpResponseBadRequest(
+                        json.dumps({
+                            'objects_sorted': False,
+                            'reason': _("An object has been added or removed "
+                                        "since the last load. Please refresh "
+                                        "the page and try reordering again."),
+                        }, ensure_ascii=False),
+                        content_type='application/json')
                 order_field_name = klass._meta.ordering[0]
 
                 if order_field_name.startswith('-'):
                     order_field_name = order_field_name[1:]
                     step = -1
-                    start_object = max(objects_dict.values(),
+                    start_object = objects_dict[objects_list[-1]]
-                        key=lambda x: getattr(x, order_field_name))
+
                 else:
                     step = 1
-                    start_object = min(objects_dict.values(),
+                    start_object = objects_dict[objects_list[0]]
-                        key=lambda x: getattr(x, order_field_name))
 
                 start_index = getattr(start_object, order_field_name,
                     len(indexes))
-
+                objects_to_update = []
                 for index in indexes:
                     obj = objects_dict.get(index)
                     # perform the update only if the order field has changed
                     if getattr(obj, order_field_name) != start_index:
                         setattr(obj, order_field_name, start_index)
-                        # only update the object's order field
+                        objects_to_update.append(obj)
-                        obj.save(update_fields=(order_field_name,))
                     start_index += step
+
+                qs.bulk_update(objects_to_update, [order_field_name])
                 response = {'objects_sorted': True}
-            except (KeyError, IndexError, klass.DoesNotExist,
-                    AttributeError, ValueError):
-                pass
 
         self.after_sorting()
 

adminsortable/models.py

@@ -89,7 +89,7 @@ class SortableMixin(models.Model):
 
     def save(self, *args, **kwargs):
         needs_default = (self._state.adding if VERSION >= (1, 8) else not self.pk)
-        if needs_default:
+        if not getattr(self, self.order_field_name) and needs_default:
             try:
                 current_max = self.__class__.objects.aggregate(
                     models.Max(self.order_field_name))[self.order_field_name + '__max'] or 0

adminsortable/static/adminsortable/css/admin.sortable.css

@@ -10,8 +10,8 @@
 	margin-left: 1em;
 }
 
-#sortable ul li,
+#sortable ul.sortable li,
-#sortable ul li a
+#sortable ul.sortable li a
 {
     cursor: move;
 }

adminsortable/templates/adminsortable/shared/object_rep.html

@@ -2,6 +2,6 @@
 
 <form>
     <input name="pk" type="hidden" value="{{ object.pk|unlocalize }}" />
-    <a href="{% url opts|admin_urlname:'do_sorting' object.model_type_id|unlocalize %}" class="admin_sorting_url"><i class="fa fa-{% if forloop.first %}sort-desc{% elif forloop.last %}sort-asc{% else %}sort{% endif %}"></i> {{ object }}</a>
+    <a href="{% url opts|admin_urlname:'do_sorting' object.model_type_id|unlocalize %}{% if filters %}?{{ filters }}{% endif %}" class="admin_sorting_url"><i class="fa fa-{% if forloop.first %}sort-desc{% elif forloop.last %}sort-asc{% else %}sort{% endif %}"></i> {{ object }}</a>
     {% csrf_token %}
 </form>

sample_project/samples/tests.py

@@ -82,6 +82,11 @@ class SortableTestCase(TestCase):
         self.assertTrue(get_is_sortable(Category.objects.all()),
             'Category has more than one record. It should be sortable.')
 
+    def test_doesnt_overwrite_preexisting_order_field_value(self):
+        self.create_category()
+        category = Category.objects.create(title='Category 2', order=5)
+        self.assertEqual(category.order, 5)
+
     def test_save_order_incremented(self):
         category1 = self.create_category()
         self.assertEqual(category1.order, 1, 'Category 1 order should be 1.')

tox.ini

@@ -1,21 +1,18 @@
 [tox]
-envlist = django{1.8,1.9,1.10,1.11,2}-{py27,py34,py35,py36},coverage
+envlist = django{2.2,3.0}-{py36,py37,py38},coverage
 
 [testenv]
 deps =
     coverage
-    django1.8: Django>=1.8,<1.9
+    django2.2: Django>=2.2
-    django1.9: Django>=1.9,<1.10
+    django3.0: Django>=3.0
-    django1.10: Django>=1.10,<1.11
-    django1.11: Django>=1.11a1,<1.12
-    django2.0: Django>=2.0
 whitelist_externals = cd
 setenv =
     PYTHONPATH = {toxinidir}/sample_project
     PYTHONWARNINGS = module
     PYTHONDONTWRITEBYTECODE = 1
 commands =
-    coverage run -p sample_project/manage.py test app
+    coverage run -p sample_project/manage.py test samples
 
 [testenv:coverage]
 deps = coverage