diff options
| -rw-r--r-- | django/contrib/admin/views/main.py | 35 | ||||
| -rw-r--r-- | tests/regressiontests/admin_changelist/models.py | 24 | ||||
| -rw-r--r-- | tests/regressiontests/admin_changelist/tests.py | 92 | ||||
| -rw-r--r-- | tests/regressiontests/admin_views/admin.py | 9 | ||||
| -rw-r--r-- | tests/regressiontests/admin_views/models.py | 11 | ||||
| -rwxr-xr-x | tests/regressiontests/admin_views/tests.py | 29 |
6 files changed, 184 insertions, 16 deletions
diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py index 32113f56eb..56f13f8099 100644 --- a/django/contrib/admin/views/main.py +++ b/django/contrib/admin/views/main.py @@ -66,7 +66,6 @@ class ChangeList(object): self.list_editable = () else: self.list_editable = list_editable - self.ordering = self.get_ordering(request) self.query = request.GET.get(SEARCH_VAR, '') self.query_set = self.get_query_set(request) self.get_results(request) @@ -218,13 +217,18 @@ class ChangeList(object): attr = getattr(self.model, field_name) return getattr(attr, 'admin_order_field', None) - def get_ordering(self, request): + def get_ordering(self, request, queryset): + """ + Returns the list of ordering fields for the change list. + First we check the get_ordering() method in model admin, then we check + the object's default ordering. Then, any manually-specified ordering + from the query string overrides anything. Finally, a deterministic + order is guaranteed by ensuring the primary key is used as the last + ordering field. + """ params = self.params - # For ordering, first check the if exists the "get_ordering" method - # in model admin, then check "ordering" parameter in the admin - # options, then check the object's default ordering. Finally, a - # manually-specified ordering from the query string overrides anything. - ordering = self.model_admin.get_ordering(request) or self._get_default_ordering() + ordering = list(self.model_admin.get_ordering(request) + or self._get_default_ordering()) if ORDER_VAR in params: # Clear ordering and used params ordering = [] @@ -239,6 +243,19 @@ class ChangeList(object): ordering.append(pfx + order_field) except (IndexError, ValueError): continue # Invalid ordering specified, skip it. + + # Add the given query's ordering fields, if any. + ordering.extend(queryset.query.order_by) + + # Ensure that the primary key is systematically present in the list of + # ordering fields so we can guarantee a deterministic order across all + # database backends. + pk_name = self.lookup_opts.pk.name + if not (set(ordering) & set(['pk', '-pk', pk_name, '-' + pk_name])): + # The two sets do not intersect, meaning the pk isn't present. So + # we add it. + ordering.append('pk') + return ordering def get_ordering_field_columns(self): @@ -322,8 +339,8 @@ class ChangeList(object): break # Set ordering. - if self.ordering: - qs = qs.order_by(*self.ordering) + ordering = self.get_ordering(request, qs) + qs = qs.order_by(*ordering) # Apply keyword searches. def construct_search(field_name): diff --git a/tests/regressiontests/admin_changelist/models.py b/tests/regressiontests/admin_changelist/models.py index 97080fb548..6b443bf536 100644 --- a/tests/regressiontests/admin_changelist/models.py +++ b/tests/regressiontests/admin_changelist/models.py @@ -57,3 +57,27 @@ class Swallow(models.Model): class Meta: ordering = ('speed', 'load') + + +class UnorderedObject(models.Model): + """ + Model without any defined `Meta.ordering`. + Refs #17198. + """ + bool = models.BooleanField(default=True) + + +class OrderedObjectManager(models.Manager): + def get_query_set(self): + return super(OrderedObjectManager, self).get_query_set().order_by('-number') + +class OrderedObject(models.Model): + """ + Model with Manager that defines a default order. + Refs #17198. + """ + name = models.CharField(max_length=255) + bool = models.BooleanField(default=True) + number = models.IntegerField(default=0) + + objects = OrderedObjectManager()
\ No newline at end of file diff --git a/tests/regressiontests/admin_changelist/tests.py b/tests/regressiontests/admin_changelist/tests.py index b422519bfb..d2b017b258 100644 --- a/tests/regressiontests/admin_changelist/tests.py +++ b/tests/regressiontests/admin_changelist/tests.py @@ -14,7 +14,8 @@ from .admin import (ChildAdmin, QuartetAdmin, BandAdmin, ChordsBandAdmin, FilteredChildAdmin, CustomPaginator, site as custom_site, SwallowAdmin) from .models import (Child, Parent, Genre, Band, Musician, Group, Quartet, - Membership, ChordsMusician, ChordsBand, Invitation, Swallow) + Membership, ChordsMusician, ChordsBand, Invitation, Swallow, + UnorderedObject, OrderedObject) class ChangeListTests(TestCase): @@ -430,3 +431,92 @@ class ChangeListTests(TestCase): self.assertContains(response, unicode(swallow.load)) self.assertContains(response, unicode(swallow.speed)) + def test_deterministic_order_for_unordered_model(self): + """ + Ensure that the primary key is systematically used in the ordering of + the changelist's results to guarantee a deterministic order, even + when the Model doesn't have any default ordering defined. + Refs #17198. + """ + superuser = self._create_superuser('superuser') + + for counter in range(1, 51): + UnorderedObject.objects.create(id=counter, bool=True) + + class UnorderedObjectAdmin(admin.ModelAdmin): + list_per_page = 10 + + def check_results_order(reverse=False): + admin.site.register(UnorderedObject, UnorderedObjectAdmin) + model_admin = UnorderedObjectAdmin(UnorderedObject, admin.site) + counter = 51 if reverse else 0 + for page in range (0, 5): + request = self._mocked_authenticated_request('/unorderedobject/?p=%s' % page, superuser) + response = model_admin.changelist_view(request) + for result in response.context_data['cl'].result_list: + counter += -1 if reverse else 1 + self.assertEqual(result.id, counter) + admin.site.unregister(UnorderedObject) + + # When no order is defined at all, everything is ordered by 'pk'. + check_results_order() + + # When an order field is defined but multiple records have the same + # value for that field, make sure everything gets ordered by pk as well. + UnorderedObjectAdmin.ordering = ['bool'] + check_results_order() + + # When order fields are defined, including the pk itself, use them. + UnorderedObjectAdmin.ordering = ['bool', '-pk'] + check_results_order(reverse=True) + UnorderedObjectAdmin.ordering = ['bool', 'pk'] + check_results_order() + UnorderedObjectAdmin.ordering = ['-id', 'bool'] + check_results_order(reverse=True) + UnorderedObjectAdmin.ordering = ['id', 'bool'] + check_results_order() + + def test_deterministic_order_for_model_ordered_by_its_manager(self): + """ + Ensure that the primary key is systematically used in the ordering of + the changelist's results to guarantee a deterministic order, even + when the Model has a manager that defines a default ordering. + Refs #17198. + """ + superuser = self._create_superuser('superuser') + + for counter in range(1, 51): + OrderedObject.objects.create(id=counter, bool=True, number=counter) + + class OrderedObjectAdmin(admin.ModelAdmin): + list_per_page = 10 + + def check_results_order(reverse=False): + admin.site.register(OrderedObject, OrderedObjectAdmin) + model_admin = OrderedObjectAdmin(OrderedObject, admin.site) + counter = 51 if reverse else 0 + for page in range (0, 5): + request = self._mocked_authenticated_request('/orderedobject/?p=%s' % page, superuser) + response = model_admin.changelist_view(request) + for result in response.context_data['cl'].result_list: + counter += -1 if reverse else 1 + self.assertEqual(result.id, counter) + admin.site.unregister(OrderedObject) + + # When no order is defined at all, use the model's default ordering (i.e. '-number') + check_results_order(reverse=True) + + # When an order field is defined but multiple records have the same + # value for that field, make sure everything gets ordered by pk as well. + OrderedObjectAdmin.ordering = ['bool'] + check_results_order() + + # When order fields are defined, including the pk itself, use them. + OrderedObjectAdmin.ordering = ['bool', '-pk'] + check_results_order(reverse=True) + OrderedObjectAdmin.ordering = ['bool', 'pk'] + check_results_order() + OrderedObjectAdmin.ordering = ['-id', 'bool'] + check_results_order(reverse=True) + OrderedObjectAdmin.ordering = ['id', 'bool'] + check_results_order()
\ No newline at end of file diff --git a/tests/regressiontests/admin_views/admin.py b/tests/regressiontests/admin_views/admin.py index b10d178640..d9607496c3 100644 --- a/tests/regressiontests/admin_views/admin.py +++ b/tests/regressiontests/admin_views/admin.py @@ -26,7 +26,8 @@ from .models import (Article, Chapter, Account, Media, Child, Parent, Picture, CoverLetter, Story, OtherStory, Book, Promo, ChapterXtra1, Pizza, Topping, Album, Question, Answer, ComplexSortedPerson, PrePopulatedPostLargeSlug, AdminOrderedField, AdminOrderedModelMethod, AdminOrderedAdminMethod, - AdminOrderedCallable, Report, Color2, MainPrepopulated, RelatedPrepopulated) + AdminOrderedCallable, Report, Color2, UnorderedObject, MainPrepopulated, + RelatedPrepopulated) def callable_year(dt_value): @@ -562,6 +563,11 @@ class MainPrepopulatedAdmin(admin.ModelAdmin): 'slug2': ['status', 'name']} +class UnorderedObjectAdmin(admin.ModelAdmin): + list_display = ['name'] + list_editable = ['name'] + list_per_page = 2 + site = admin.AdminSite(name="admin") @@ -609,6 +615,7 @@ site.register(Story, StoryAdmin) site.register(OtherStory, OtherStoryAdmin) site.register(Report, ReportAdmin) site.register(MainPrepopulated, MainPrepopulatedAdmin) +site.register(UnorderedObject, UnorderedObjectAdmin) # We intentionally register Promo and ChapterXtra1 but not Chapter nor ChapterXtra2. # That way we cover all four cases: diff --git a/tests/regressiontests/admin_views/models.py b/tests/regressiontests/admin_views/models.py index cf2896f60a..17533f9f80 100644 --- a/tests/regressiontests/admin_views/models.py +++ b/tests/regressiontests/admin_views/models.py @@ -596,4 +596,13 @@ class RelatedPrepopulated(models.Model): choices=(('option one', 'Option One'), ('option two', 'Option Two'))) slug1 = models.SlugField(max_length=50) - slug2 = models.SlugField(max_length=60)
\ No newline at end of file + slug2 = models.SlugField(max_length=60) + + +class UnorderedObject(models.Model): + """ + Model without any defined `Meta.ordering`. + Refs #16819. + """ + name = models.CharField(max_length=255) + bool = models.BooleanField(default=True) diff --git a/tests/regressiontests/admin_views/tests.py b/tests/regressiontests/admin_views/tests.py index 4069c99ef6..5241d263a7 100755 --- a/tests/regressiontests/admin_views/tests.py +++ b/tests/regressiontests/admin_views/tests.py @@ -12,6 +12,7 @@ from django.core.exceptions import SuspiciousOperation from django.core.files import temp as tempfile from django.core.urlresolvers import reverse # Register auth models with the admin. +from django.contrib import admin from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME from django.contrib.admin.models import LogEntry, DELETION from django.contrib.admin.sites import LOGIN_FORM_KEY @@ -41,7 +42,7 @@ from .models import (Article, BarAccount, CustomArticle, EmptyModel, FooAccount, FoodDelivery, RowLevelChangePermissionModel, Paper, CoverLetter, Story, OtherStory, ComplexSortedPerson, Parent, Child, AdminOrderedField, AdminOrderedModelMethod, AdminOrderedAdminMethod, AdminOrderedCallable, - Report, MainPrepopulated, RelatedPrepopulated) + Report, MainPrepopulated, RelatedPrepopulated, UnorderedObject) ERROR_MESSAGE = "Please enter the correct username and password \ @@ -272,9 +273,12 @@ class AdminViewBasicTest(TestCase): ) def testChangeListSortingPreserveQuerySetOrdering(self): - # If no ordering on ModelAdmin, or query string, the underlying order of - # the queryset should not be changed. - + """ + If no ordering is defined in `ModelAdmin.ordering` or in the query + string, then the underlying order of the queryset should not be + changed, even if it is defined in `Modeladmin.queryset()`. + Refs #11868, #7309. + """ p1 = Person.objects.create(name="Amy", gender=1, alive=True, age=80) p2 = Person.objects.create(name="Bob", gender=1, alive=True, age=70) p3 = Person.objects.create(name="Chris", gender=2, alive=False, age=60) @@ -1881,6 +1885,23 @@ class AdminViewListEditable(TestCase): self.assertEqual(Category.objects.get(id=3).order, 1) self.assertEqual(Category.objects.get(id=4).order, 0) + def test_list_editable_pagination(self): + """ + Ensure that pagination works for list_editable items. + Refs #16819. + """ + UnorderedObject.objects.create(id=1, name='Unordered object #1') + UnorderedObject.objects.create(id=2, name='Unordered object #2') + UnorderedObject.objects.create(id=3, name='Unordered object #3') + response = self.client.get('/test_admin/admin/admin_views/unorderedobject/') + self.assertContains(response, 'Unordered object #1') + self.assertContains(response, 'Unordered object #2') + self.assertNotContains(response, 'Unordered object #3') + response = self.client.get('/test_admin/admin/admin_views/unorderedobject/?p=1') + self.assertNotContains(response, 'Unordered object #1') + self.assertNotContains(response, 'Unordered object #2') + self.assertContains(response, 'Unordered object #3') + def test_list_editable_action_submit(self): # List editable changes should not be executed if the action "Go" button is # used to submit the form. |