-
Notifications
You must be signed in to change notification settings - Fork 190
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Fixes issue #175 (possible exception with custom managers hiding som… #176
base: master
Are you sure you want to change the base?
Conversation
@@ -68,7 +68,7 @@ def pre_save(sender, instance, raw, using, update_fields, **kwargs): | |||
|
|||
# created or updated? | |||
if not created: | |||
old_model = sender.objects.get(pk=instance.pk) | |||
old_model = sender._default_manager.get(pk=instance.pk) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_base_manager i think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://docs.djangoproject.com/en/4.0/topics/db/managers/#default-managers-1, https://docs.djangoproject.com/en/4.0/topics/db/managers/#base-managers
can someone verify the most correct solution?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be _base_manager which will always return all records, the _default_manager may be filtering out records and can still raise an DoesNotExist exception.
@@ -68,7 +68,7 @@ def pre_save(sender, instance, raw, using, update_fields, **kwargs): | |||
|
|||
# created or updated? | |||
if not created: | |||
old_model = sender.objects.get(pk=instance.pk) | |||
old_model = sender._default_manager.get(pk=instance.pk) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://docs.djangoproject.com/en/4.0/topics/db/managers/#default-managers-1, https://docs.djangoproject.com/en/4.0/topics/db/managers/#base-managers
can someone verify the most correct solution?
|
Is there a chance that the pull request will be accepted? |
Hi I also encountered this - could that be fixed? |
As promised - use Django
_default_manager
to prevent exceptions when custom manager excludes certain model instances.