[Django] #28918: refresh_from_db should use _base_manager

[Django]

#28918: refresh_from_db should use _base_manager
-------------------------------------+-------------------------------------
Reporter: Sjoerd | Owner: nobody
Job Postmus |
Type: | Status: new
Uncategorized |
Component: Database | Version: 2.0
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
When using `refresh_from_db`, currently the `_default_manager` is used.

Based upon the documentation at
https://docs.djangoproject.com/en/2.0/topics/db/managers/#default-managers
, it seems that there's a requirement for `_base_manager` to not filter
out results, while for `_default_manager` there's just a preference for
this to be the case.

I think the stronger "guarantee" provided by `_base_manager` is more
appropriate in this case.

The following patch both adds a test and fixes the issue.

{{{
diff --git a/django/db/models/base.py b/django/db/models/base.py
index 27ca63fd22..0813bedcb0 100644
--- a/django/db/models/base.py
+++ b/django/db/models/base.py
@@ -591,7 +591,7 @@ class Model(metaclass=ModelBase):
'are not allowed in fields.' % LOOKUP_SEP)

db = using if using is not None else self._state.db
- db_instance_qs =
self.__class__._default_manager.using(db).filter(pk=self.pk)
+ db_instance_qs =
self.__class__._base_manager.using(db).filter(pk=self.pk)

# Use provided fields, if not set then reload all non-deferred
fields.
deferred_fields = self.get_deferred_fields()
diff --git a/tests/custom_managers/tests.py
b/tests/custom_managers/tests.py
index ee2ac1d552..61b1a07933 100644
--- a/tests/custom_managers/tests.py
+++ b/tests/custom_managers/tests.py
@@ -643,3 +643,13 @@ class CustomManagersRegressTestCase(TestCase):
qs_custom = Person.custom_init_queryset_manager.all()
qs_default = Person.objects.all()
self.assertQuerysetEqual(qs_custom, qs_default)
+
+ def test_refresh_from_db_when_default_manager_filters(self):
+ """
+ obj.refresh_from_db() should also fetch object if default manager
+ hides it.
+ """
+ book = Book._base_manager.create(is_published=False)
+ Book._base_manager.filter(pk=book.pk).update(title='Hi')
+ book.refresh_from_db()
+ self.assertEqual(book.title, 'Hi')
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/28918>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#28918: Model.refresh_from_db() should use _base_manager instead of the
default_manager
-------------------------------------+-------------------------------------
Reporter: Sjoerd Job Postmus | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Tim Graham):

* has_patch: 0 => 1
* stage: Unreviewed => Accepted
* type: Uncategorized => Bug

Comment:

Will you provide a pull request?

--
Ticket URL: <https://code.djangoproject.com/ticket/28918#comment:1>

[Django]

#28918: Model.refresh_from_db() should use _base_manager instead of the
default_manager
-------------------------------------+-------------------------------------
Reporter: Sjoerd Job Postmus | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Tim Graham):

* stage: Accepted => Ready for checkin

Comment:

[https://github.com/django/django/pull/9512 PR] from the patch in the
ticket description.

--
Ticket URL: <https://code.djangoproject.com/ticket/28918#comment:2>

[Django]

#28918: Model.refresh_from_db() should use _base_manager instead of the
default_manager
-------------------------------------+-------------------------------------
Reporter: Sjoerd Job Postmus | Owner: nobody

Type: Bug | Status: closed

Component: Database layer | Version: 2.0
(models, ORM) |

Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by GitHub <noreply@…>):

* status: new => closed
* resolution: => fixed

Comment:

In [changeset:"d065c92678f5d11a70b88e195c6357576eb3a2ef" d065c926]:
{{{
#!CommitTicketReference repository=""
revision="d065c92678f5d11a70b88e195c6357576eb3a2ef"
Fixed #28918 -- Fixed Model.refresh_from_db() for instances hidden by the
default manager.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/28918#comment:3>

[Django]

#28918: Model.refresh_from_db() should use _base_manager instead of the
default_manager
-------------------------------------+-------------------------------------
Reporter: Sjoerd Job Postmus | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Sjoerd Job Postmus):

Sorry for not providing a PR myself. I was on a 2-week holiday. Thank you
for handling it.

Should I create a PR directly next time when I find an issue instead of
attaching a patch?

--
Ticket URL: <https://code.djangoproject.com/ticket/28918#comment:4>

[Django]

#28918: Model.refresh_from_db() should use _base_manager instead of the
default_manager
-------------------------------------+-------------------------------------
Reporter: Sjoerd Job Postmus | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham):

Yes, that's fine.

--
Ticket URL: <https://code.djangoproject.com/ticket/28918#comment:5>