Use Manager.from_queryset to construct PageManager, to avoid duplication of queryset methods.

This exposes an inconsistency between the old `PageManager.(not_)sibling_of` method (which was
exclusive by default) and `PageQuerySet.(not_)sibling_of)` (which is inclusive). We therefore
standardise on the inclusive behaviour (in line with treebeard's own sibling methods), and note
this change of behaviour in the release notes.
This commit is contained in:
Matt Westcott 2015-10-13 19:15:12 +01:00
parent 928bc0d25f
commit dafbdaf20f
4 changed files with 83 additions and 70 deletions

View file

@ -24,6 +24,7 @@ Changelog
* Fix: Wagtail userbar is shown on pages that do not pass a `page` variable to the template (e.g. because they override the `serve` method)
* Fix: request.site now set correctly on page preview when the page is not in the default site
* Fix: Project template no longer raises a deprecation warning (Maximilian Stauss)
* Fix: `PageManager.sibling_of(page)` and `PageManager.not_sibling_of(page)` now default to inclusive (i.e. `page` is considered a sibling of itself), for consistency with other sibling methods
1.1 (15.09.2015)

View file

@ -56,11 +56,53 @@ Bug fixes
* Wagtail userbar is shown on pages that do not pass a ``page`` variable to the template (e.g. because they override the ``serve`` method)
* ``request.site`` now set correctly on page preview when the page is not in the default site
* Project template no longer raises a deprecation warning (Maximilian Stauss)
* ``PageManager.sibling_of(page)`` and ``PageManager.not_sibling_of(page)`` now default to inclusive (i.e. ``page`` is considered a sibling of itself), for consistency with other sibling methods
Upgrade considerations
======================
``PageManager.sibling_of(page)`` and ``PageManager.not_sibling_of(page)`` have changed behaviour
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In previous versions of Wagtail, the ``sibling_of`` and ``not_sibling_of`` methods behaved inconsistently depending on whether they were called on a manager (e.g. ``Page.objects.sibling_of(some_page)`` or ``EventPage.objects.sibling_of(some_page)``) or a QuerySet (e.g. ``Page.objects.all().sibling_of(some_page)`` or ``EventPage.objects.live().sibling_of(some_page)``).
Previously, the manager methods behaved as *exclusive* by default; that is, they did not count the passed-in page object as a sibling of itself:
.. code-block:: python
>>> event_1 = EventPage.objects.get(title='Event 1')
>>> EventPage.objects.sibling_of(event_1)
[<EventPage: Event 2>] # OLD behaviour: Event 1 is not considered a sibling of itself
This has now been changed to be *inclusive* by default; that is, the page is counted as a sibling of itself:
.. code-block:: python
>>> event_1 = EventPage.objects.get(title='Event 1')
>>> EventPage.objects.sibling_of(event_1)
[<EventPage: Event 1>, <EventPage: Event 2>] # NEW behaviour: Event 1 is considered a sibling of itself
If the call to ``sibling_of`` or ``not_sibling_of`` is chained after another QuerySet method - such as ``all()``, ``filter()`` or ``live()`` - behaviour is unchanged; this behaves as *inclusive*, as it did in previous versions:
.. code-block:: python
>>> event_1 = EventPage.objects.get(title='Event 1')
>>> EventPage.objects.all().sibling_of(event_1)
[<EventPage: Event 1>, <EventPage: Event 2>] # OLD and NEW behaviour
If your project includes queries that rely on the old (exclusive) behaviour, this behaviour can be restored by adding the keyword argument ``inclusive=False``:
.. code-block:: python
>>> event_1 = EventPage.objects.get(title='Event 1')
>>> EventPage.objects.sibling_of(event_1, inclusive=False)
[<EventPage: Event 2>] # passing inclusive=False restores the OLD behaviour
``Image.search`` and ``Document.search`` methods are deprecated
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

View file

@ -171,77 +171,11 @@ def get_page_types():
return _PAGE_CONTENT_TYPES
class PageManager(models.Manager):
class BasePageManager(models.Manager):
def get_queryset(self):
return PageQuerySet(self.model).order_by('path')
def live(self):
return self.get_queryset().live()
def not_live(self):
return self.get_queryset().not_live()
def in_menu(self):
return self.get_queryset().in_menu()
def not_in_menu(self):
return self.get_queryset().not_in_menu()
def page(self, other):
return self.get_queryset().page(other)
def not_page(self, other):
return self.get_queryset().not_page(other)
def descendant_of(self, other, inclusive=False):
return self.get_queryset().descendant_of(other, inclusive)
def not_descendant_of(self, other, inclusive=False):
return self.get_queryset().not_descendant_of(other, inclusive)
def child_of(self, other):
return self.get_queryset().child_of(other)
def not_child_of(self, other):
return self.get_queryset().not_child_of(other)
def ancestor_of(self, other, inclusive=False):
return self.get_queryset().ancestor_of(other, inclusive)
def not_ancestor_of(self, other, inclusive=False):
return self.get_queryset().not_ancestor_of(other, inclusive)
def parent_of(self, other):
return self.get_queryset().parent_of(other)
def not_parent_of(self, other):
return self.get_queryset().not_parent_of(other)
def sibling_of(self, other, inclusive=False):
return self.get_queryset().sibling_of(other, inclusive)
def not_sibling_of(self, other, inclusive=False):
return self.get_queryset().not_sibling_of(other, inclusive)
def type(self, model):
return self.get_queryset().type(model)
def not_type(self, model):
return self.get_queryset().not_type(model)
def public(self):
return self.get_queryset().public()
def not_public(self):
return self.get_queryset().not_public()
def search(self, query_string, fields=None,
operator=None, order_by_relevance=True, backend='default'):
return self.get_queryset().search(query_string, fields=fields,
operator=operator, order_by_relevance=order_by_relevance, backend=backend)
def specific(self):
return self.get_queryset().specific()
PageManager = BasePageManager.from_queryset(PageQuerySet)
class PageBase(models.base.ModelBase):

View file

@ -192,11 +192,27 @@ class TestPageQuerySet(TestCase):
# Test that events index is in pages
self.assertTrue(pages.filter(id=events_index.id).exists())
def test_sibling_of(self):
def test_sibling_of_default(self):
"""
sibling_of should default to an inclusive definition of sibling
if 'inclusive' flag not passed
"""
events_index = Page.objects.get(url_path='/home/events/')
event = Page.objects.get(url_path='/home/events/christmas/')
pages = Page.objects.sibling_of(event)
# Check that all pages are children of events_index
for page in pages:
self.assertEqual(page.get_parent(), events_index)
# Check that the event is included
self.assertTrue(pages.filter(id=event.id).exists())
def test_sibling_of_exclusive(self):
events_index = Page.objects.get(url_path='/home/events/')
event = Page.objects.get(url_path='/home/events/christmas/')
pages = Page.objects.sibling_of(event, inclusive=False)
# Check that all pages are children of events_index
for page in pages:
self.assertEqual(page.get_parent(), events_index)
@ -216,11 +232,31 @@ class TestPageQuerySet(TestCase):
# Check that the event is included
self.assertTrue(pages.filter(id=event.id).exists())
def test_not_sibling_of(self):
def test_not_sibling_of_default(self):
"""
not_sibling_of should default to an inclusive definition of sibling -
i.e. eliminate self from the results as well -
if 'inclusive' flag not passed
"""
events_index = Page.objects.get(url_path='/home/events/')
event = Page.objects.get(url_path='/home/events/christmas/')
pages = Page.objects.not_sibling_of(event)
# Check that all pages are not children of events_index
for page in pages:
self.assertNotEqual(page.get_parent(), events_index)
# Check that the event is not included
self.assertFalse(pages.filter(id=event.id).exists())
# Test that events index is in pages
self.assertTrue(pages.filter(id=events_index.id).exists())
def test_not_sibling_of_exclusive(self):
events_index = Page.objects.get(url_path='/home/events/')
event = Page.objects.get(url_path='/home/events/christmas/')
pages = Page.objects.not_sibling_of(event, inclusive=False)
# Check that all pages are not children of events_index
for page in pages:
if page != event: