From cf47b1a578d1d1de0d41feaea061c0fa3c4fdaa5 Mon Sep 17 00:00:00 2001 From: Noam Kushinsky Date: Wed, 1 Jan 2025 17:50:09 +0200 Subject: [PATCH 1/2] Fixed reason field being shared between models. --- AUTHORS.rst | 1 + CHANGES.rst | 1 + simple_history/models.py | 2 +- simple_history/tests/models.py | 14 +++++++++++++ simple_history/tests/tests/test_models.py | 25 +++++++++++++++++++++++ 5 files changed, 42 insertions(+), 1 deletion(-) diff --git a/AUTHORS.rst b/AUTHORS.rst index 16602a25..b89a90ae 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -111,6 +111,7 @@ Authors - Nathan Villagaray-Carski (`ncvc `_) - Nianpeng Li - Nick Träger +- Noam Kushinsky (`noamkush `_) - Noel James (`NoelJames `_) - Ofek Lev (`ofek `_) - Phillip Marshall diff --git a/CHANGES.rst b/CHANGES.rst index a03ffd47..0a842c86 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -15,6 +15,7 @@ Unreleased - Added pagination to ``SimpleHistoryAdmin`` (gh-1277) - Fixed issue with history button not working when viewing historical entries in the admin (gh-527) +- Fixed history_change_reason_field being shared across inherited models (gh-1433) 3.7.0 (2024-05-29) ------------------ diff --git a/simple_history/models.py b/simple_history/models.py index b007efbf..67f3a82b 100644 --- a/simple_history/models.py +++ b/simple_history/models.py @@ -411,7 +411,7 @@ def copy_fields(self, model): def _get_history_change_reason_field(self): if self.history_change_reason_field: # User specific field from init - history_change_reason_field = self.history_change_reason_field + history_change_reason_field = self.history_change_reason_field.clone() elif getattr( settings, "SIMPLE_HISTORY_HISTORY_CHANGE_REASON_USE_TEXT_FIELD", False ): diff --git a/simple_history/tests/models.py b/simple_history/tests/models.py index c6771302..63a08930 100644 --- a/simple_history/tests/models.py +++ b/simple_history/tests/models.py @@ -650,6 +650,12 @@ class Meta: abstract = True +class TrackedAbstractBaseWithReasonField(models.Model): + history = HistoricalRecords( + inherit=True, history_change_reason_field=models.TextField() + ) + + class UntrackedAbstractBase(models.Model): class Meta: abstract = True @@ -709,6 +715,14 @@ class InheritTracking4(TrackedAbstractBaseA): pass +class InheritReasonField1(TrackedAbstractBaseWithReasonField): + pass + + +class InheritReasonField2(TrackedAbstractBaseWithReasonField): + pass + + class BasePlace(models.Model): name = models.CharField(max_length=50) history = HistoricalRecords(inherit=True, table_name="base_places_history") diff --git a/simple_history/tests/tests/test_models.py b/simple_history/tests/tests/test_models.py index 566e2635..16678fa4 100644 --- a/simple_history/tests/tests/test_models.py +++ b/simple_history/tests/tests/test_models.py @@ -71,6 +71,8 @@ HistoricalPollWithManyToMany_places, HistoricalState, InheritedRestaurant, + InheritReasonField1, + InheritReasonField2, Library, ManyToManyModelOther, ModelWithCustomAttrOneToOneField, @@ -686,6 +688,29 @@ def test_user_textfield_history_change_reason(self): self.assertTrue(isinstance(field, models.TextField)) self.assertEqual(history.history_change_reason, reason) + def test_inherited_history_change_reason_update(self): + inherited1 = InheritReasonField1.history.create( + history_change_reason="reason1", id=1, history_date=datetime.now() + ) + inherited2 = InheritReasonField2.history.create( + history_change_reason="reason2", id=1, history_date=datetime.now() + ) + + InheritReasonField1.history.update(history_change_reason="new_reason1") + InheritReasonField2.history.update(history_change_reason="new_reason2") + + inherited1.refresh_from_db() + inherited2.refresh_from_db() + + self.assertEqual(inherited1.history_change_reason, "new_reason1") + self.assertEqual(inherited2.history_change_reason, "new_reason2") + + def test_history_change_reason_field_not_shared(self): + self.assertIsNot( + InheritReasonField1.history.model._meta.get_field("history_change_reason"), + InheritReasonField2.history.model._meta.get_field("history_change_reason"), + ) + def test_history_diff_includes_changed_fields(self): p = Poll.objects.create(question="what's up?", pub_date=today) p.question = "what's up, man?" From 10027146e74f7fa4503b4e0744724db158813981 Mon Sep 17 00:00:00 2001 From: Noam Date: Mon, 27 Jan 2025 00:25:35 +0200 Subject: [PATCH 2/2] Update simple_history/models.py Co-authored-by: Tim Schilling --- simple_history/models.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/simple_history/models.py b/simple_history/models.py index 67f3a82b..3ed7ed9d 100644 --- a/simple_history/models.py +++ b/simple_history/models.py @@ -410,7 +410,8 @@ def copy_fields(self, model): def _get_history_change_reason_field(self): if self.history_change_reason_field: - # User specific field from init + # User specific field from init. + # It's cloned so a reason from an inherited model isn't shared. history_change_reason_field = self.history_change_reason_field.clone() elif getattr( settings, "SIMPLE_HISTORY_HISTORY_CHANGE_REASON_USE_TEXT_FIELD", False