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

CHANGELOG.md

@@ -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))
 

auditlog/registry.py

@@ -202,7 +202,7 @@ def _connect_signals(self, model):
                 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,14 +218,18 @@ def _disconnect_signals(self, model):
             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]
 
     def _dispatch_uid(self, signal, receiver) -> DispatchUID:
         """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:

auditlog_tests/models.py

@@ -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 @@ class AutoManyRelatedModel(models.Model):
 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)

auditlog_tests/tests.py

@@ -46,12 +46,14 @@
     JSONModel,
     ManyRelatedModel,
     ManyRelatedOtherModel,
+    ModelForReusableThroughModel,
     ModelPrimaryKeyModel,
     NoDeleteHistoryModel,
     NullableJSONModel,
     PostgresArrayFieldModel,
     ProxyModel,
     RelatedModel,
+    ReusableThroughRelatedModel,
     SerializeNaturalKeyRelatedModel,
     SerializeOnlySomeOfThisModel,
     SerializePrimaryKeyRelatedModel,
@@ -390,6 +392,8 @@ def setUp(self):
         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 @@ def test_object_repr_related_deleted(self):
             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 @@ def test_register_models_register_app(self):
 
         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(