[Django] #36108: The `update_field` when using `django.db.models.signals` does not work with a custom Model Manager

[Django]

#36108: The `update_field` when using `django.db.models.signals` does not work with
a custom Model Manager
-------------------------------------+-------------------------------------
Reporter: NicoJJohnson | Type: Bug
Status: new | Component: Database
| layer (models, ORM)
Version: 4.2 | Severity: Normal
Keywords: | Triage Stage:
signal,model,manager,filter | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
I have a Model that has some sensitive objects labeled with
`is_private=True`.
Furthermore, all private objects should be filtered out by default, unless
there is a User, in which we can check if they own the object.

{{{
class SensitiveObjectManager(models.Manager):
def __init__(self):
super().__init__()
self.user = None

def for_user(self, user):
"""Create a new manager instance with the user context"""
manager = SensitiveObjectManager()
manager.model = self.model
manager.user = user
if hasattr(self, "core_filters"):
manager.core_filters = self.core_filters
return manager

def get_queryset(self):
qs = super().get_queryset()

if hasattr(self, "core_filters"):
qs = qs.filter(**self.core_filters)

if self.user is None:
return qs.filter(is_private=False)
return qs.filter(Q(is_private=False) | Q(owner=self.user.id))


class SensitiveObject(models.Model):
objects = SensitiveObjectManager()
all_objects = models.Manager()

id = models.UUIDField(primary_key=True, default=uuid.uuid4)
is_private = models.BooleanField(default=True)
owner = models.ForeignKey(User)
is_leaked = models.BooleanField(default=False)

}}}


This is designed because {{{SensitiveObject.objects.all()}}} is commonly
used throughout our code, but with this Manager, we can always filter out
the private objects. To include objects that the user owns, we can use
`SensitiveObject.objects.for_user(User).all()`.

Everything works fine with it so far except for a very odd bug using
`django.db.models.signals.post_save`. We want to catch when `is_leaked` is
updated so that we can update a few other Models. So we have

{{{
#apps.py
def sensitive_object_updated(sender, instance, created, update_fields,
**kwargs):
# ( Would perform additional ORM logic, but for testing, the print
statements are all that's needed)
print("Instance:")
print(instance)
print("Instance.is_private:")
print(instance.is_private)
print("Instance.is_leaked:")
print(instance.is_leaked)
print("update_fields:")
print(update_fields)

from django.apps import AppConfig

class SensitiveObjectConfig(AppConfig):
default_auto_field = "django.db.models.BigAutoField"
name = "sensitive_object"

def ready(self):
from django.db.models.signals import post_save
from sensitive_object.models import SensitiveObject
post_save.connect(sensitive_object_updated,
sender=SensitiveObject)
}}}
If the `SensitiveObject.is_private=True`, then the `update_fields` will
NOT be included in `sensitive_object_updated` which is very annoying
(`update_fields` will equal `None`). But when
`SensitiveObject.is_private=False`, the `update_fields` work fine. I
tested this, and I know it is because of the `SensitiveObjectManager`.
Furthermore, if you test it, you will see the `instance` and
`instance.is_leaked` have the correct values inside
`sensitive_object_updated` always, (whether if `is_private` is True or
False). I’m pretty confident that this is a bug with Django.

For more information about trying to find a patch, this is also posted on
the django forum under the name "Signal does not pick up update_field if
the model’s Manager filters objects"

Example Test Case:
{{{
# tests.py
from django.test import TestCase
def SensitiveObjectTest(TestCase):
def example_test(self):
private_object = SensitiveObject.objects.create(
owner=self.user,
is_private=True,
is_leaked=False
)
self.assertTrue(SensitiveObject.all_objects.filter(id=private_dataset.id).exists())
# Thanks to the SensitiveObjectManager, private datasets are
filtered out of objects by default.
self.assertFalse(SensitiveObject.objects.filter(id=private_dataset.id).exists())
self.assertTrue(SensitiveObject.objects.for_user(self.user).filter(id=private_dataset.id).exists())

print("Observe print logs from sensitive_object_updated for private
object")
private_object.is_leaked=True
private_object.save()
# Should see the print statements from `senstive_object_updated`.
# update_fields will be None

public_object = SensitiveObject.objects.create(
owner=self.user,
is_private=False,
is_leaked=False
)
self.assertTrue(SensitiveObject.all_objects.filter(id=public_dataset.id).exists())
self.assertTrue(SensitiveObject.objects.filter(id=public_dataset.id).exists())

print("Observe print logs from sensitive_object_updated for public
object")
public_object.is_leaked=True
public_object.save()
# Should see the print statements from `senstive_object_updated`.
# update_fields will be correct
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36108>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36108: The `update_field` when using `django.db.models.signals` does not work with
a custom Model Manager
-------------------------------------+-------------------------------------

Reporter: NicoJJohnson | Owner: (none)
Type: Bug | Status: closed
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution: invalid

Keywords: | Triage Stage:
signal,model,manager,filter | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

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

Comment:

[https://forum.djangoproject.com/t/avoiding-duplicate-entries-when-one-or-
more-fields-are-none/37109 This the forum thread referred to in the
report] which I assume could not be included due to spam filter
configuration.

There's something that doesn't add up in this report with regards to
`update_fields` and I believe could be due to
[https://docs.djangoproject.com/en/5.1/ref/signals/#post-save a
misunderstanding of the feature]; the value provided to the save signal
receiver is nothing more than what's passed to `Model.save(update_fields)`
and not the the set of fields that were locally changed from their last
persisted value.

> `update_fields`
>
> The set of fields to update as passed to `Model.save()`, or `None` if
`update_fields` wasn’t passed to `save()`.

In other words Django doesn't keep track of model instance fields that
were assigned a different value since their last retrieval / persistence
to the database and will
[https://docs.djangoproject.com/en/5.1/ref/models/instances/#specifying-
which-fields-to-save default to saving all fields] if `update_fields` is
not specified which is denoted by `update_fields=None` in signal
receivers.

If you want to keep track of which field changed and [https://django-
model-utils.readthedocs.io/en/latest/utilities.html#field-tracker you
should use a third party application for that].
--
Ticket URL: <https://code.djangoproject.com/ticket/36108#comment:1>