From 58885814b1622fc8c05faac3c913ea4a08651226 Mon Sep 17 00:00:00 2001 From: Christopher Charbonneau Wells <10456740+cdubz@users.noreply.github.com> Date: Sun, 10 Nov 2024 16:53:52 +0000 Subject: [PATCH 1/6] 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 --- auditlog/registry.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/auditlog/registry.py b/auditlog/registry.py index 0c9067bb..40d7bd16 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,13 +218,17 @@ 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: From 35b431cc970d48fe6c0d187955fbbcbe0ad912a6 Mon Sep 17 00:00:00 2001 From: Christopher Charbonneau Wells <10456740+cdubz@users.noreply.github.com> Date: Sun, 10 Nov 2024 17:00:35 +0000 Subject: [PATCH 2/6] fix(format): apply black formatting --- auditlog/registry.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/auditlog/registry.py b/auditlog/registry.py index 40d7bd16..aa8b689e 100644 --- a/auditlog/registry.py +++ b/auditlog/registry.py @@ -225,7 +225,7 @@ def _disconnect_signals(self, 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) From 96267d5b7d9c2de9057e2fb87efc98810b05c7d8 Mon Sep 17 00:00:00 2001 From: Chris Wells Date: Mon, 11 Nov 2024 10:42:14 -0800 Subject: [PATCH 3/6] add test and changelog entry --- CHANGELOG.md | 1 + auditlog_tests/models.py | 57 ++++++++++++++++++++++++++++++++++++++++ auditlog_tests/tests.py | 12 ++++++++- 3 files changed, 69 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ebd95d76..108dee06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - feat: Added `LogEntry.remote_port` field. ([#671](https://github.com/jazzband/django-auditlog/pull/671)) - feat: Added `truncate` option to `auditlogflush` management command. ([#681](https://github.com/jazzband/django-auditlog/pull/681)) - feat: Added `AUDITLOG_CHANGE_DISPLAY_TRUNCATE_LENGTH` settings to keep or truncate strings of `changes_display_dict` property at variable length. ([#684](https://github.com/jazzband/django-auditlog/pull/684)) +- 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)) - Drop Python 3.8 support. ([#678](https://github.com/jazzband/django-auditlog/pull/678)) - Confirm Django 5.1 support and drop Django 3.2 support. ([#677](https://github.com/jazzband/django-auditlog/pull/677)) 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..ae6d5bcf 100644 --- a/auditlog_tests/tests.py +++ b/auditlog_tests/tests.py @@ -46,6 +46,9 @@ JSONModel, ManyRelatedModel, ManyRelatedOtherModel, + ModelForReusableThroughModel, + OtherModelForReusableThroughModel, + ReusableThroughRelatedModel, ModelPrimaryKeyModel, NoDeleteHistoryModel, NullableJSONModel, @@ -390,6 +393,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 +490,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 +1265,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( From 9c5899cf99af8e41da5d3c2b456d81f7a570f8f0 Mon Sep 17 00:00:00 2001 From: Chris Wells Date: Mon, 11 Nov 2024 10:46:07 -0800 Subject: [PATCH 4/6] remove unused import --- auditlog_tests/tests.py | 1 - 1 file changed, 1 deletion(-) diff --git a/auditlog_tests/tests.py b/auditlog_tests/tests.py index ae6d5bcf..3271456f 100644 --- a/auditlog_tests/tests.py +++ b/auditlog_tests/tests.py @@ -47,7 +47,6 @@ ManyRelatedModel, ManyRelatedOtherModel, ModelForReusableThroughModel, - OtherModelForReusableThroughModel, ReusableThroughRelatedModel, ModelPrimaryKeyModel, NoDeleteHistoryModel, From 031eae79ff3158671e11d65e4709203a9e03c334 Mon Sep 17 00:00:00 2001 From: Chris Wells Date: Mon, 11 Nov 2024 10:47:34 -0800 Subject: [PATCH 5/6] correct import sorting --- auditlog_tests/tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/auditlog_tests/tests.py b/auditlog_tests/tests.py index 3271456f..f043de01 100644 --- a/auditlog_tests/tests.py +++ b/auditlog_tests/tests.py @@ -47,13 +47,13 @@ ManyRelatedModel, ManyRelatedOtherModel, ModelForReusableThroughModel, - ReusableThroughRelatedModel, ModelPrimaryKeyModel, NoDeleteHistoryModel, NullableJSONModel, PostgresArrayFieldModel, ProxyModel, RelatedModel, + ReusableThroughRelatedModel, SerializeNaturalKeyRelatedModel, SerializeOnlySomeOfThisModel, SerializePrimaryKeyRelatedModel, From b8a36e33d48a5f71b3839cc777dec6d9d5b488a4 Mon Sep 17 00:00:00 2001 From: Christopher Charbonneau Wells <10456740+cdubz@users.noreply.github.com> Date: Tue, 12 Nov 2024 05:07:07 -0800 Subject: [PATCH 6/6] move change log message to correct section --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 108dee06..af7827eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,12 +7,12 @@ - feat: Added `LogEntry.remote_port` field. ([#671](https://github.com/jazzband/django-auditlog/pull/671)) - feat: Added `truncate` option to `auditlogflush` management command. ([#681](https://github.com/jazzband/django-auditlog/pull/681)) - feat: Added `AUDITLOG_CHANGE_DISPLAY_TRUNCATE_LENGTH` settings to keep or truncate strings of `changes_display_dict` property at variable length. ([#684](https://github.com/jazzband/django-auditlog/pull/684)) -- 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)) - Drop Python 3.8 support. ([#678](https://github.com/jazzband/django-auditlog/pull/678)) - Confirm Django 5.1 support and drop Django 3.2 support. ([#677](https://github.com/jazzband/django-auditlog/pull/677)) #### 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))