fix: use sender for m2m signal dispatch connection (#686)

* fix: use sender for m2m signal dispatch connection

This fix adds support for a use case where a single m2m through model is
used on multiple models. When the reciever is used for the dispatch uid
in this use case it cause duplicated logs because the through model
singal connection happens multiple times.

By changing the m2m signal connection to use the sender for the dispatch
uid this duplication is prevented because the signal connection only
happens once for the through model.

Refs: #685

* fix(format): apply black formatting

* add test and changelog entry

* remove unused import

* correct import sorting

* move change log message to correct section
This commit is contained in:
Christopher Charbonneau Wells 2024-11-12 07:35:06 -08:00 committed by GitHub
parent d4f99c2729
commit 5621777622
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 74 additions and 3 deletions

View file

@ -12,6 +12,7 @@
#### Fixes
- fix: Use sender instead of receiver for `m2m_changed` signal ID to prevent duplicate entries for models that share a related model. ([#686](https://github.com/jazzband/django-auditlog/pull/686))
- Fixed a problem when setting `Value(None)` in `JSONField` ([#646](https://github.com/jazzband/django-auditlog/pull/646))
- Fixed a problem when setting `django.db.models.functions.Now()` in `DateTimeField` ([#635](https://github.com/jazzband/django-auditlog/pull/635))

View file

@ -202,7 +202,7 @@ class AuditlogModelRegistry:
m2m_changed.connect(
receiver,
sender=m2m_model,
dispatch_uid=self._dispatch_uid(m2m_changed, receiver),
dispatch_uid=self._m2m_dispatch_uid(m2m_changed, m2m_model),
)
def _disconnect_signals(self, model):
@ -218,7 +218,7 @@ class AuditlogModelRegistry:
m2m_model = getattr(field, "through")
m2m_changed.disconnect(
sender=m2m_model,
dispatch_uid=self._dispatch_uid(m2m_changed, receiver),
dispatch_uid=self._m2m_dispatch_uid(m2m_changed, m2m_model),
)
del self._m2m_signals[model]
@ -226,6 +226,10 @@ class AuditlogModelRegistry:
"""Generate a dispatch_uid which is unique for a combination of self, signal, and receiver."""
return id(self), id(signal), id(receiver)
def _m2m_dispatch_uid(self, signal, sender) -> DispatchUID:
"""Generate a dispatch_uid which is unique for a combination of self, signal, and sender."""
return id(self), id(signal), id(sender)
def _get_model_classes(self, app_model: str) -> list[ModelBase]:
try:
try:

View file

@ -133,6 +133,61 @@ class ManyRelatedOtherModel(models.Model):
history = AuditlogHistoryField(delete_related=True)
class ReusableThroughRelatedModel(models.Model):
"""
A model related to multiple other models through a model.
"""
label = models.CharField(max_length=100)
class ReusableThroughModel(models.Model):
"""
A through model that can be associated multiple different models.
"""
label = models.ForeignKey(
ReusableThroughRelatedModel,
on_delete=models.CASCADE,
related_name="%(app_label)s_%(class)s_items",
)
one = models.ForeignKey(
"ModelForReusableThroughModel", on_delete=models.CASCADE, null=True, blank=True
)
two = models.ForeignKey(
"OtherModelForReusableThroughModel",
on_delete=models.CASCADE,
null=True,
blank=True,
)
class ModelForReusableThroughModel(models.Model):
"""
A model with many-to-many relations through a shared model.
"""
name = models.CharField(max_length=200)
related = models.ManyToManyField(
ReusableThroughRelatedModel, through=ReusableThroughModel
)
history = AuditlogHistoryField(delete_related=True)
class OtherModelForReusableThroughModel(models.Model):
"""
Another model with many-to-many relations through a shared model.
"""
name = models.CharField(max_length=200)
related = models.ManyToManyField(
ReusableThroughRelatedModel, through=ReusableThroughModel
)
history = AuditlogHistoryField(delete_related=True)
@auditlog.register(include_fields=["label"])
class SimpleIncludeModel(models.Model):
"""
@ -364,6 +419,8 @@ auditlog.register(RelatedModel)
auditlog.register(ManyRelatedModel)
auditlog.register(ManyRelatedModel.recursive.through)
m2m_only_auditlog.register(ManyRelatedModel, m2m_fields={"related"})
m2m_only_auditlog.register(ModelForReusableThroughModel, m2m_fields={"related"})
m2m_only_auditlog.register(OtherModelForReusableThroughModel, m2m_fields={"related"})
auditlog.register(SimpleExcludeModel, exclude_fields=["text"])
auditlog.register(SimpleMappingModel, mapping_fields={"sku": "Product No."})
auditlog.register(AdditionalDataIncludedModel)

View file

@ -46,12 +46,14 @@ from auditlog_tests.models import (
JSONModel,
ManyRelatedModel,
ManyRelatedOtherModel,
ModelForReusableThroughModel,
ModelPrimaryKeyModel,
NoDeleteHistoryModel,
NullableJSONModel,
PostgresArrayFieldModel,
ProxyModel,
RelatedModel,
ReusableThroughRelatedModel,
SerializeNaturalKeyRelatedModel,
SerializeOnlySomeOfThisModel,
SerializePrimaryKeyRelatedModel,
@ -390,6 +392,8 @@ class ManyRelatedModelTest(TestCase):
self.obj = ManyRelatedModel.objects.create()
self.recursive = ManyRelatedModel.objects.create()
self.related = ManyRelatedOtherModel.objects.create()
self.obj_reusable = ModelForReusableThroughModel.objects.create()
self.obj_reusable_related = ReusableThroughRelatedModel.objects.create()
self.base_log_entry_count = (
LogEntry.objects.count()
) # created by the create() calls above
@ -485,6 +489,11 @@ class ManyRelatedModelTest(TestCase):
log_entry = self.obj.history.first()
self.assertEqual(log_entry.object_repr, DEFAULT_OBJECT_REPR)
def test_changes_not_duplicated_with_reusable_through_model(self):
self.obj_reusable.related.add(self.obj_reusable_related)
entries = self.obj_reusable.history.all()
self.assertEqual(len(entries), 1)
class MiddlewareTest(TestCase):
"""
@ -1255,7 +1264,7 @@ class RegisterModelSettingsTest(TestCase):
self.assertTrue(self.test_auditlog.contains(SimpleExcludeModel))
self.assertTrue(self.test_auditlog.contains(ChoicesFieldModel))
self.assertEqual(len(self.test_auditlog.get_models()), 27)
self.assertEqual(len(self.test_auditlog.get_models()), 31)
def test_register_models_register_model_with_attrs(self):
self.test_auditlog._register_models(