Document that get_next/previous_by_FOO uses default manager

[Jani Tiainen]

Model methods get_next_by_FOO() and get_previous_by_FOO() doesn't honor original manager used but uses always default manager (first manager declared on a model).

This might potentially return incorrect data, for example in cases where softdelete is used. Default manager would return all objects but normally user uses manager that returns only non-deleted objects. In that case get_next|pervious_by_FOO would return incorrect data.

Possible workaround would be using proxy models.

[Jani Tiainen]

Testcase to demonstrate bug

[Carl Meyer]

Model instances don't have a memory of how they were queried for, and the only way to "fix" this would be to add such a memory. I think the downsides of adding that additional state to model instances far outweighs the benefit of fixing this behavior; it would require much stronger coupling between both managers and the querysets they return, and querysets and the instances they query for. get_next and get_previous operate with respect to the entire set of model instances of that type, not some arbitrarily narrowed set. If you need them to operate with respect to a narrowed set, you can pass additional filter arguments to them.

It would be worth documenting this more clearly, however; accepting and changing the component to documentation.

[Carl Meyer]

Also, thanks for the report and for providing a demonstration test case! I would have found it much harder to understand the report without the testcase. In future, though, it's easiest to work with a test case if provided as a patch file against Django's own test suite (master branch) rather than as a zipped project.

[nkryptic]

possible alternative that allows use of alternate manager

[nkryptic]

I added a patch which provides an alternative implementation for _get_next_or_previous_by_FIELD. It simply checks for a kwarg of using_manager, which the method will attempt to use instead, but defaults to using the _default_manager. The default behavior wouldn't change, though. So, using the example from the initial test case from jtiai, to get the behavior he wants would look like:

  man1 = Person.men.get(last_name='SquarePants')
  man2 = man1.get_previous_by_birthday(using_manager='men')

While the manager in the test case is fairly simple and you could repeat the filtering it performs when calling get_previous_by_birthday, given more complicated managers it starts to get ugly.

This is definitely just a proof of concept, at this point. If anyone wants this pursued further, I'd be happy to create a pull request for the full implementation with tests.

[nkryptic]

Another possibility I discussed with jtiai, which could be included in the updated documentation, is to use a Proxy model with it's default manager set to the expected manager. Then, if using the proxy model for the initial query, the instance calling get_previous_by_birthday would pass the test.

[Carl Meyer]

I don't think this is worth adding another keyword argument to these functions. It's a bit of a smell to have "special" kwarg names for a function that otherwise interprets all its kwargs as field filters. And the new argument doesn't fully solve the problem as phrased in the ticket; you'd still get the "wrong" filtering initially and have to go digging to figure out a) what's happening and b) the name of the keyword arg you need to use to work around it. If people are going to need to do that digging anyway, I think the manual-filtering and proxy-model workarounds are good enough, and both work today.

So I still think this should just be documented, with mention of both filtering manually and using a proxy model.

[Aymeric Augustin]

I agree with carljm that the only reasonable thing to do given the existing API is to document this behavior.

To solve the problem, we'd have to introduce a new API hooked on QuerySet and not Model. I'm thinking of something like:

QuerySet.get_next(instance, ('-date', 'name'))
QuerySet.get_previous(instance, ('-date', 'name'))

[Baptiste Mispelon]

Pull request (for the documentation) here: ​https://github.com/django/django/pull/1290.

[Tim Graham <timograham@…>]

In ba610cb319d8882a663effcaf0a4e53c04593f98:

Fixed #19881 -- Documented that get_next/previous_by_FOO uses default manager.

[Tim Graham <timograham@…>]

In 9b0604e5149e20fc98f07256af60eac87421a346:

[1.5.x] Fixed #19881 -- Documented that get_next/previous_by_FOO uses default manager.

Backport of ba610cb319 from master