diff --git a/CHANGELOG.md b/CHANGELOG.md index ebd95d76..af7827eb 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/auditlog/registry.py b/auditlog/registry.py index 0c9067bb..aa8b689e 100644 --- a/auditlog/registry.py +++ b/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,7 +218,7 @@ 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] @@ -226,6 +226,10 @@ 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: diff --git a/auditlog_tests/models.py b/auditlog_tests/models.py index ffacc838..819556bc 100644 --- a/auditlog_tests/models.py +++ b/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) diff --git a/auditlog_tests/tests.py b/auditlog_tests/tests.py index d94db9fb..f043de01 100644 --- a/auditlog_tests/tests.py +++ b/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(