#28918 closed Bug (fixed)
Model.refresh_from_db() should use _base_manager instead of the default_manager
Reported by: | Sjoerd Job Postmus | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 2.0 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
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')
Change History (5)
comment:1 by , 7 years ago
Has patch: | set |
---|---|
Summary: | refresh_from_db should use _base_manager → Model.refresh_from_db() should use _base_manager instead of the default_manager |
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → Bug |
comment:2 by , 7 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
PR from the patch in the ticket description.
comment:4 by , 7 years ago
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?
Will you provide a pull request?