Fixes to page ordering logic in explorer:

- keep ordering and pagination as two distinct operations
- only perform the 'annotate' hack when ordering by latest_revision_created_at
- entries with a null latest_revision_created_at should appear at the top when
  ordering oldest first
- don't uselessly call order_by twice
- actually test that the 'ordering' parameter orders the results :-)
pull/2215/merge
Matt Westcott 2016-02-09 16:53:49 +00:00
rodzic df623ec1a5
commit 395cecc642
4 zmienionych plików z 91 dodań i 9 usunięć

Wyświetl plik

@ -25,6 +25,7 @@ Changelog
* Fix: Documents created by a user are no longer deleted when the user is deleted
* Fix: Fixed a crash in `RedirectMiddleware` when a middleware class before `SiteMiddleware` returns a response (Josh Schneier)
* Fix: Fixed error retrieving the moderator list on pages that are covered by multiple moderator permission records (Matt Fozard)
* Fix: Ordering pages in the explorer by reverse 'last updated' time now puts pages with no revisions at the top
1.3.1 (05.01.2016)
~~~~~~~~~~~~~~~~~~

Wyświetl plik

@ -54,6 +54,7 @@ Bug fixes
* Documents created by a user are no longer deleted when the user is deleted
* Fixed a crash in ``RedirectMiddleware`` when a middleware class before ``SiteMiddleware`` returns a response (Josh Schneier)
* Fixed error retrieving the moderator list on pages that are covered by multiple moderator permission records (Matt Fozard)
* Ordering pages in the explorer by reverse 'last updated' time now puts pages with no revisions at the top
Upgrade considerations

Wyświetl plik

@ -1,4 +1,4 @@
from datetime import timedelta
from datetime import datetime, timedelta
import mock
import django
@ -45,6 +45,21 @@ class TestPageExplorer(TestCase, WagtailTestUtils):
)
self.root_page.add_child(instance=self.child_page)
# more child pages to test ordering
self.old_page = StandardIndex(
title="Old page",
slug="old-page",
latest_revision_created_at=datetime(2010, 1, 1)
)
self.root_page.add_child(instance=self.old_page)
self.new_page = SimplePage(
title="New page",
slug="new-page",
latest_revision_created_at=datetime(2016, 1, 1)
)
self.root_page.add_child(instance=self.new_page)
# Login
self.login()
@ -53,7 +68,11 @@ class TestPageExplorer(TestCase, WagtailTestUtils):
self.assertEqual(response.status_code, 200)
self.assertTemplateUsed(response, 'wagtailadmin/pages/index.html')
self.assertEqual(self.root_page, response.context['parent_page'])
self.assertTrue(response.context['pages'].paginator.object_list.filter(id=self.child_page.id).exists())
# child pages should be most recent first
# (with null latest_revision_created_at at the end)
page_ids = [page.id for page in response.context['pages']]
self.assertEqual(page_ids, [self.new_page.id, self.old_page.id, self.child_page.id])
def test_explore_root(self):
response = self.client.get(reverse('wagtailadmin_explore_root'))
@ -63,23 +82,67 @@ class TestPageExplorer(TestCase, WagtailTestUtils):
self.assertTrue(response.context['pages'].paginator.object_list.filter(id=self.root_page.id).exists())
def test_ordering(self):
response = self.client.get(reverse('wagtailadmin_explore_root'), {'ordering': 'content_type'})
response = self.client.get(
reverse('wagtailadmin_explore', args=(self.root_page.id, )),
{'ordering': 'title'}
)
self.assertEqual(response.status_code, 200)
self.assertTemplateUsed(response, 'wagtailadmin/pages/index.html')
self.assertEqual(response.context['ordering'], 'content_type')
self.assertEqual(response.context['ordering'], 'title')
# child pages should be ordered by title
page_ids = [page.id for page in response.context['pages']]
self.assertEqual(page_ids, [self.child_page.id, self.new_page.id, self.old_page.id])
def test_reverse_ordering(self):
response = self.client.get(
reverse('wagtailadmin_explore', args=(self.root_page.id, )),
{'ordering': '-title'}
)
self.assertEqual(response.status_code, 200)
self.assertTemplateUsed(response, 'wagtailadmin/pages/index.html')
self.assertEqual(response.context['ordering'], '-title')
# child pages should be ordered by title
page_ids = [page.id for page in response.context['pages']]
self.assertEqual(page_ids, [self.old_page.id, self.new_page.id, self.child_page.id])
def test_ordering_by_last_revision_forward(self):
response = self.client.get(
reverse('wagtailadmin_explore', args=(self.root_page.id, )),
{'ordering': 'latest_revision_created_at'}
)
self.assertEqual(response.status_code, 200)
self.assertTemplateUsed(response, 'wagtailadmin/pages/index.html')
self.assertEqual(response.context['ordering'], 'latest_revision_created_at')
# child pages should be oldest revision first
# (with null latest_revision_created_at at the start)
page_ids = [page.id for page in response.context['pages']]
self.assertEqual(page_ids, [self.child_page.id, self.old_page.id, self.new_page.id])
def test_invalid_ordering(self):
response = self.client.get(reverse('wagtailadmin_explore_root'), {'ordering': 'invalid_order'})
response = self.client.get(
reverse('wagtailadmin_explore', args=(self.root_page.id, )),
{'ordering': 'invalid_order'}
)
self.assertEqual(response.status_code, 200)
self.assertTemplateUsed(response, 'wagtailadmin/pages/index.html')
self.assertEqual(response.context['ordering'], '-latest_revision_created_at')
def test_reordering(self):
response = self.client.get(reverse('wagtailadmin_explore_root'), {'ordering': 'ord'})
response = self.client.get(
reverse('wagtailadmin_explore', args=(self.root_page.id, )),
{'ordering': 'ord'}
)
self.assertEqual(response.status_code, 200)
self.assertTemplateUsed(response, 'wagtailadmin/pages/index.html')
self.assertEqual(response.context['ordering'], 'ord')
# child pages should be ordered by native tree order (i.e. by creation time)
page_ids = [page.id for page in response.context['pages']]
self.assertEqual(page_ids, [self.child_page.id, self.old_page.id, self.new_page.id])
# Pages must not be paginated
self.assertNotIsInstance(response.context['pages'], paginator.Page)

Wyświetl plik

@ -48,14 +48,31 @@ def index(request, parent_page_id=None):
]:
ordering = '-latest_revision_created_at'
if ordering == 'ord':
# preserve the native ordering from get_children()
pass
elif ordering == 'latest_revision_created_at':
# order by oldest revision first.
# Special case NULL entries - these should go at the top of the list.
# Do this by annotating with Count('latest_revision_created_at'),
# which returns 0 for these
pages = pages.annotate(
null_position=Count('latest_revision_created_at')
).order_by('null_position', 'latest_revision_created_at')
elif ordering == '-latest_revision_created_at':
# order by oldest revision first.
# Special case NULL entries - these should go at the end of the list.
pages = pages.annotate(
null_position=Count('latest_revision_created_at')
).order_by('-null_position', '-latest_revision_created_at')
else:
pages = pages.order_by(ordering)
# Pagination
# Don't paginate if sorting by page order - all pages must be shown to
# allow drag-and-drop reordering
do_paginate = ordering != 'ord'
if do_paginate:
ordering_no_minus = ordering.lstrip('-')
pages = pages.order_by(ordering).annotate(
null_position=Count(ordering_no_minus)).order_by('-null_position', ordering)
paginator, pages = paginate(request, pages, per_page=50)
return render(request, 'wagtailadmin/pages/index.html', {