diff --git a/.gitignore b/.gitignore index 8ade561..8313d6e 100644 --- a/.gitignore +++ b/.gitignore @@ -11,5 +11,8 @@ atlassian-* .codeintel __pycache__ .venv/ +.coverage +.tox/ +htmlcov/ build .vscode/* diff --git a/.travis.yml b/.travis.yml index cf3c863..fd4e063 100644 --- a/.travis.yml +++ b/.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 diff --git a/adminsortable/admin.py b/adminsortable/admin.py index a3c3362..15e3eab 100644 --- a/adminsortable/admin.py +++ b/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, - request.POST.get('indexes', []).split(','))) - objects_dict = dict([(str(obj.pk), obj) for obj in - klass.objects.filter(pk__in=indexes)]) + indexes = [str(idx) for idx in request.POST.get('indexes', []).split(',')] + # 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(), - key=lambda x: getattr(x, order_field_name)) + start_object = objects_dict[objects_list[-1]] + else: step = 1 - start_object = min(objects_dict.values(), - key=lambda x: getattr(x, order_field_name)) + start_object = objects_dict[objects_list[0]] 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 - obj.save(update_fields=(order_field_name,)) + objects_to_update.append(obj) 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() diff --git a/adminsortable/models.py b/adminsortable/models.py index 67240c3..94e15f7 100644 --- a/adminsortable/models.py +++ b/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 diff --git a/adminsortable/static/adminsortable/css/admin.sortable.css b/adminsortable/static/adminsortable/css/admin.sortable.css index e1c857c..4311972 100644 --- a/adminsortable/static/adminsortable/css/admin.sortable.css +++ b/adminsortable/static/adminsortable/css/admin.sortable.css @@ -10,8 +10,8 @@ margin-left: 1em; } -#sortable ul li, -#sortable ul li a +#sortable ul.sortable li, +#sortable ul.sortable li a { cursor: move; } diff --git a/adminsortable/templates/adminsortable/shared/object_rep.html b/adminsortable/templates/adminsortable/shared/object_rep.html index 8b5383b..960ba9c 100644 --- a/adminsortable/templates/adminsortable/shared/object_rep.html +++ b/adminsortable/templates/adminsortable/shared/object_rep.html @@ -2,6 +2,6 @@
- {{ object }} + {{ object }} {% csrf_token %}
diff --git a/sample_project/samples/tests.py b/sample_project/samples/tests.py index 4fb1442..708343c 100644 --- a/sample_project/samples/tests.py +++ b/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.') diff --git a/tox.ini b/tox.ini index b67add1..efd978f 100644 --- a/tox.ini +++ b/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 - django1.9: Django>=1.9,<1.10 - django1.10: Django>=1.10,<1.11 - django1.11: Django>=1.11a1,<1.12 - django2.0: Django>=2.0 + django2.2: Django>=2.2 + django3.0: Django>=3.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