From a47919879946b95daba00b478d4ecb6204a2dffd Mon Sep 17 00:00:00 2001 From: Venelin Stoykov Date: Thu, 16 Jun 2016 12:16:42 +0300 Subject: [PATCH] Use separate URLs per model in administration to do the sorting - Correctly check if current user has permissions to made the change Fixes #142 --- adminsortable/admin.py | 71 ++++++---- .../edit_inline/stacked-1.5.x.html | 4 +- .../adminsortable/edit_inline/stacked.html | 2 +- .../edit_inline/tabular-1.5.x.html | 4 +- .../adminsortable/edit_inline/tabular.html | 2 +- .../adminsortable/shared/object_rep.html | 4 +- sample_project/app/tests.py | 128 +++++++++++++++++- 7 files changed, 181 insertions(+), 34 deletions(-) diff --git a/adminsortable/admin.py b/adminsortable/admin.py index bb53926..8708f5a 100755 --- a/adminsortable/admin.py +++ b/adminsortable/admin.py @@ -13,18 +13,21 @@ except ImportError: from django.contrib.admin import ModelAdmin, TabularInline, StackedInline from django.contrib.admin.options import InlineModelAdmin -if VERSION >= (1, 8): - from django.contrib.auth import get_permission_codename +try: from django.contrib.contenttypes.admin import (GenericStackedInline, - GenericTabularInline) -else: + GenericTabularInline) +except: + # Django < 1.7 from django.contrib.contenttypes.generic import (GenericStackedInline, - GenericTabularInline) + GenericTabularInline) from django.contrib.contenttypes.models import ContentType -from django.http import HttpResponse -from django.shortcuts import render +from django.core.exceptions import PermissionDenied +from django.http import HttpResponse, Http404 +from django.shortcuts import render, get_object_or_404 from django.template.defaultfilters import capfirst +from django.utils.decorators import method_decorator +from django.views.decorators.http import require_POST from adminsortable.fields import SortableForeignKey from adminsortable.models import SortableMixin @@ -82,18 +85,33 @@ class SortableAdmin(SortableAdminBase, ModelAdmin): def get_urls(self): urls = super(SortableAdmin, self).get_urls() + opts = self.model._meta + try: + info = opts.app_label, opts.model_name + except AttributeError: + # Django < 1.7 + info = opts.app_label, opts.modul_name - # this ajax view changes the order - admin_do_sorting_url = url(r'^sorting/do-sorting/(?P\d+)/$', + # this ajax view changes the order of instances of self.model + admin_do_sorting_url = url( + r'^sort/do-sorting/$', self.admin_site.admin_view(self.do_sorting_view), - name='admin_do_sorting') + name='%s_%s_do_sorting' % info) + + # this ajax view changes the order of instances of inline models + admin_do_inline_sorting_url = url( + r'^sort/do-sorting/(?P\d+)/$', + self.admin_site.admin_view(self.do_sorting_view), + name='%s_%s_do_sorting' % info) # this view displays the sortable objects - admin_sort_url = url(r'^sort/$', + admin_sort_url = url( + r'^sort/$', self.admin_site.admin_view(self.sort_view), - name='admin_sort') + name='%s_%s_sort' % info) urls = [ + admin_do_inline_sorting_url, admin_do_sorting_url, admin_sort_url ] + urls @@ -104,15 +122,10 @@ class SortableAdmin(SortableAdminBase, ModelAdmin): Custom admin view that displays the objects as a list whose sort order can be changed via drag-and-drop. """ + if not self.has_change_permission(request): + raise PermissionDenied opts = self.model._meta - if VERSION >= (1, 8): - codename = get_permission_codename('change', opts) - has_perm = request.user.has_perm('{0}.{1}'.format(opts.app_label, - codename)) - else: - has_perm = request.user.has_perm('{0}.{1}'.format(opts.app_label, - opts.get_change_permission())) jquery_lib_path = 'admin/js/jquery.js' if VERSION < (1, 9) \ else 'admin/js/vendor/jquery/jquery.js' @@ -202,7 +215,7 @@ class SortableAdmin(SortableAdminBase, ModelAdmin): capfirst(verbose_name_plural)), 'opts': opts, 'app_label': opts.app_label, - 'has_perm': has_perm, + 'has_perm': True, 'objects': objects, 'group_expression': sortable_by_expression, 'sortable_by_class': sortable_by_class, @@ -258,19 +271,31 @@ class SortableAdmin(SortableAdminBase, ModelAdmin): return super(SortableAdmin, self).change_view(request, object_id, form_url='', extra_context=extra_context) + @method_decorator(require_POST) def do_sorting_view(self, request, model_type_id=None): """ This view sets the ordering of the objects for the model type and primary keys passed in. It must be an Ajax POST. """ + if not self.has_change_permission(request): + raise PermissionDenied + response = {'objects_sorted': False} - if request.is_ajax() and request.method == 'POST': + if request.is_ajax(): try: + if model_type_id is None: + klass = self.model + else: + klass = get_object_or_404(ContentType, + id=model_type_id).model_class() + if klass not in (inline.model for inline in self.inlines): + raise Http404( + 'There is no inline model with type id: {0}'.format( + model_type_id)) + indexes = list(map(str, request.POST.get('indexes', []).split(','))) - klass = ContentType.objects.get( - id=model_type_id).model_class() objects_dict = dict([(str(obj.pk), obj) for obj in klass.objects.filter(pk__in=indexes)]) diff --git a/adminsortable/templates/adminsortable/edit_inline/stacked-1.5.x.html b/adminsortable/templates/adminsortable/edit_inline/stacked-1.5.x.html index c133fe6..776ae35 100644 --- a/adminsortable/templates/adminsortable/edit_inline/stacked-1.5.x.html +++ b/adminsortable/templates/adminsortable/edit_inline/stacked-1.5.x.html @@ -1,4 +1,4 @@ -{% load i18n admin_modify adminsortable_tags %} +{% load i18n admin_modify adminsortable_tags admin_urls %} {% load static from staticfiles %}

{{ inline_admin_formset.opts.verbose_name_plural|title }} {% if inline_admin_formset.formset.initial_form_count > 1 %} - {% trans "drag and drop to change order" %}{% endif %}

@@ -17,7 +17,7 @@ {% if inline_admin_form.has_auto_field %}{{ inline_admin_form.pk_field.field }}{% endif %} {{ inline_admin_form.fk_field.field }} {% if inline_admin_form.original %} - + {% endif %}
{% endfor %} diff --git a/adminsortable/templates/adminsortable/edit_inline/stacked.html b/adminsortable/templates/adminsortable/edit_inline/stacked.html index a6e96f5..682b391 100644 --- a/adminsortable/templates/adminsortable/edit_inline/stacked.html +++ b/adminsortable/templates/adminsortable/edit_inline/stacked.html @@ -16,7 +16,7 @@ {% include "adminsortable/shared/fieldset.html" with inline_admin_form_forloop=forloop.parentloop initial_forms_count=inline_admin_formset.formset.management_form.initial.INITIAL_FORMS %} {% endfor %} {% if inline_admin_form.original %} - + {% endif %} {% if inline_admin_form.needs_explicit_pk_field %}{{ inline_admin_form.pk_field.field }}{% endif %} {{ inline_admin_form.fk_field.field }} diff --git a/adminsortable/templates/adminsortable/edit_inline/tabular-1.5.x.html b/adminsortable/templates/adminsortable/edit_inline/tabular-1.5.x.html index 30f1772..66e89bc 100644 --- a/adminsortable/templates/adminsortable/edit_inline/tabular-1.5.x.html +++ b/adminsortable/templates/adminsortable/edit_inline/tabular-1.5.x.html @@ -1,4 +1,4 @@ -{% load i18n admin_modify adminsortable_tags %} +{% load i18n admin_modify adminsortable_tags admin_urls %} {% load static from staticfiles %}
@@ -41,7 +41,7 @@ {% endfor %} {% endspaceless %} {% if inline_admin_form.original %} - + {% endif %} {% for fieldset in inline_admin_form %} diff --git a/adminsortable/templates/adminsortable/edit_inline/tabular.html b/adminsortable/templates/adminsortable/edit_inline/tabular.html index fc69ace..f77dae0 100644 --- a/adminsortable/templates/adminsortable/edit_inline/tabular.html +++ b/adminsortable/templates/adminsortable/edit_inline/tabular.html @@ -50,7 +50,7 @@ {% endfor %} {% endspaceless %} {% if inline_admin_form.original %} - + {% endif %} {% for fieldset in inline_admin_form %} diff --git a/adminsortable/templates/adminsortable/shared/object_rep.html b/adminsortable/templates/adminsortable/shared/object_rep.html index 15654c0..3cbaf0a 100644 --- a/adminsortable/templates/adminsortable/shared/object_rep.html +++ b/adminsortable/templates/adminsortable/shared/object_rep.html @@ -1,6 +1,6 @@ -{% load adminsortable_tags %} +{% load adminsortable_tags admin_urls %}
- {{ object }} + {{ object }}
diff --git a/sample_project/app/tests.py b/sample_project/app/tests.py index 9d4501a..7877f7c 100644 --- a/sample_project/app/tests.py +++ b/sample_project/app/tests.py @@ -32,10 +32,16 @@ class SortableTestCase(TestCase): self.client = Client() self.user_raw_password = 'admin' self.user = User.objects.create_user('admin', 'admin@admin.com', - self.user_raw_password) + self.user_raw_password) self.user.is_staff = True self.user.is_superuser = True self.user.save() + self.staff_raw_password = 'staff' + self.staff = User.objects.create_user('staff', 'staff@staff.com', + self.staff_raw_password) + self.staff.is_staff = True + self.user.is_superuser = False + self.staff.save() # create people Person.objects.create(first_name='Bob', last_name='Smith', @@ -64,6 +70,9 @@ class SortableTestCase(TestCase): def test_new_user_is_staff(self): self.assertEqual(self.user.is_staff, True, 'User is not staff') + def test_new_staff_is_staff(self): + self.assertEqual(self.staff.is_staff, True, 'Staff User is not staff') + def test_is_not_sortable(self): """ A model should only become sortable if it has more than @@ -100,8 +109,11 @@ class SortableTestCase(TestCase): return category1, category2, category3 def get_sorting_url(self): - return '/admin/app/category/sorting/do-sorting/{0}/'.format( - Category.model_type_id()) + return '/admin/app/category/sort/do-sorting/' + + def get_inline_sorting_url(self, model): + return '/admin/app/project/sort/do-sorting/{0}/'.format( + model.model_type_id()) def get_category_indexes(self, *categories): return {'indexes': ','.join([str(c.id) for c in categories])} @@ -120,6 +132,38 @@ class SortableTestCase(TestCase): self.assertTrue('adminsortable/change_list.html' in template_names, 'adminsortable/change_list.html was not rendered') + def test_adminsortable_change_list_sorting_fails_if_not_post(self): + logged_in = self.client.login(username=self.user.username, + password=self.user_raw_password) + self.assertTrue(logged_in, 'User is not logged in') + + category1, category2, category3 = self.make_test_categories() + # make a normal GET + response = self.client.get( + self.get_sorting_url(), + data=self.get_category_indexes(category1, category2, category3)) + + self.assertEqual( + response.status_code, + httplib.METHOD_NOT_ALLOWED, + 'Objects should not have been sorted. A POST method is required.') + + def test_adminsortable_change_list_sorting_fails_permission_denied(self): + logged_in = self.client.login(username=self.staff.username, + password=self.staff_raw_password) + self.assertTrue(logged_in, 'User is not logged in') + + category1, category2, category3 = self.make_test_categories() + # make a normal POST + response = self.client.post( + self.get_sorting_url(), + data=self.get_category_indexes(category1, category2, category3)) + + self.assertEqual( + response.status_code, + httplib.FORBIDDEN, + 'Objects should not have been sorted. User is not allowed.') + def test_adminsortable_change_list_sorting_fails_if_not_ajax(self): logged_in = self.client.login(username=self.user.username, password=self.user_raw_password) @@ -203,3 +247,81 @@ class SortableTestCase(TestCase): response = self.client.get('/admin/app/project/sort/') self.assertEquals(response.status_code, httplib.OK, 'Unable to reach sort view.') + + def test_adminsortable_change_list_view_permission_denied(self): + category1 = self.create_category(title='Category 3') + Project.objects.create(category=category1, description="foo") + + self.client.login(username=self.staff.username, + password=self.staff_raw_password) + response = self.client.get('/admin/app/project/sort/') + self.assertEquals(response.status_code, httplib.FORBIDDEN, + 'Sort view must be forbidden.') + + def test_adminsortable_inline_changelist_not_found(self): + self.client.login(username=self.user.username, + password=self.user_raw_password) + + project = Project.objects.create( + category=self.create_category(), + description='Test project' + ) + note1 = project.note_set.create(text='note 1') + note2 = project.note_set.create(text='note 2') + + response = self.client.post( + self.get_inline_sorting_url(Category), + data=self.get_category_indexes(note2, note1), + HTTP_X_REQUESTED_WITH='XMLHttpRequest') + + self.assertEqual( + response.status_code, + httplib.NOT_FOUND, + 'Categories must not be sortable trough ProjectAdmin') + + def test_adminsortable_inline_changelist_success(self): + self.client.login(username=self.user.username, + password=self.user_raw_password) + + project = Project.objects.create( + category=self.create_category(), + description='Test project' + ) + note1 = project.note_set.create(text='note 1') + note2 = project.note_set.create(text='note 2') + note3 = project.note_set.create(text='note 3') + + response = self.client.post( + self.get_inline_sorting_url(project.note_set.model), + data=self.get_category_indexes(note3, note2, note1), + HTTP_X_REQUESTED_WITH='XMLHttpRequest') + + self.assertEqual( + response.status_code, + httplib.OK, + 'Note inline must be sortable in ProjectAdmin') + content = json.loads(response.content.decode(encoding='UTF-8'), + 'latin-1') + self.assertTrue(content.get('objects_sorted'), + 'Objects should have been sorted.') + + notes = list(project.note_set.all().values('id', 'order', 'text')) + expected_notes = [ + { + 'id': note3.pk, + 'order': 1, + 'text': note3.text, + }, + { + 'id': note2.pk, + 'order': 2, + 'text': note2.text, + }, + { + 'id': note1.pk, + 'order': 3, + 'text': note1.text, + } + ] + self.assertEqual(notes, expected_notes) +