Skip to content

Commit a75ec47

Browse files
muneeb706ddabble
andauthored
Fix: Error when setting table name with inherit (#1196)
* fix(simple_histor.models): Custom history table name Error was thrown when table_name is set with inherit=True in base model. - add not inherited check #1195 * fix(docs): Custom history table name - authors and changes docs updated * merge(master): Merged latest changes from master * merge(master): Resolved conflicts * merge(master): Resolved conflicts * merge(master): Resolved conflicts * merge(master): Resolved conflicts * fix(simple_histor.models): Custom history table name - unit test added * Small improvement of the test for #1196 Now also compares the table name of the inherited (base) model, so that it's easier to see that the history table name is not inherited. * Added note on table_name inheritance to docs --------- Co-authored-by: Anders <[email protected]>
1 parent f2e028c commit a75ec47

File tree

5 files changed

+25
-3
lines changed

5 files changed

+25
-3
lines changed

CHANGES.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ Unreleased
1919
- Fixed ``HistoryRequestMiddleware`` not cleaning up after itself (i.e. deleting
2020
``HistoricalRecords.context.request``) under some circumstances (gh-1188)
2121
- Made ``HistoryRequestMiddleware`` async-capable (gh-1209)
22+
- Fixed error when setting ``table_name`` with ``inherit=True`` (gh-1195)
2223

2324
3.3.0 (2023-03-08)
2425
------------------

docs/historical_model.rst

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ Allow tracking to be inherited
361361

362362
By default history tracking is only added for the model that is passed
363363
to ``register()`` or has the ``HistoricalRecords`` descriptor. By
364-
passing ``inherit=True`` to either way of registering you can change
364+
passing ``inherit=True`` to either way of registering, you can change
365365
that behavior so that any child model inheriting from it will have
366366
historical tracking as well. Be careful though, in cases where a model
367367
can be tracked more than once, ``MultipleRegistrationsError`` will be
@@ -384,6 +384,9 @@ raised.
384384
Both ``User`` and ``Poll`` in the example above will cause any model
385385
inheriting from them to have historical tracking as well.
386386

387+
**Note:** For parent models having a ``HistoricalRecords`` field with ``inherit=True``
388+
*and* a ``table_name``, the latter option will not be inherited by child models.
389+
387390
History Model In Different App
388391
------------------------------
389392

simple_history/models.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ def create_history_model(self, model, inherited):
298298
attrs.update(self.get_extra_fields(model, fields))
299299
# type in python2 wants str as a first argument
300300
attrs.update(Meta=type("Meta", (), self.get_meta_options(model)))
301-
if self.table_name is not None:
301+
if not inherited and self.table_name is not None:
302302
attrs["Meta"].db_table = self.table_name
303303

304304
# Set as the default then check for overrides

simple_history/tests/models.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -670,7 +670,7 @@ class InheritTracking4(TrackedAbstractBaseA):
670670

671671
class BasePlace(models.Model):
672672
name = models.CharField(max_length=50)
673-
history = HistoricalRecords(inherit=True)
673+
history = HistoricalRecords(inherit=True, table_name="base_places_history")
674674

675675

676676
class InheritedRestaurant(BasePlace):

simple_history/tests/tests/test_models.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -724,6 +724,24 @@ def test_history_diff_includes_changed_fields_of_base_model(self):
724724
self.assertEqual(delta.new_record, new_record)
725725
self.assertEqual(expected_change.field, delta.changes[0].field)
726726

727+
def test_history_table_name_is_not_inherited(self):
728+
def assert_table_name(obj, expected_table_name):
729+
history_model = obj.history.model
730+
self.assertEqual(
731+
history_model.__name__, f"Historical{obj._meta.model.__name__}"
732+
)
733+
self.assertEqual(history_model._meta.db_table, expected_table_name)
734+
735+
place = BasePlace.objects.create(name="Place Name")
736+
# This is set in `BasePlace.history`
737+
assert_table_name(place, "base_places_history")
738+
739+
r = InheritedRestaurant.objects.create(name="KFC", serves_hot_dogs=True)
740+
self.assertTrue(isinstance(r, BasePlace))
741+
# The default table name of the history model,
742+
# instead of inheriting from `BasePlace`
743+
assert_table_name(r, f"tests_Historical{r._meta.model.__name__}".lower())
744+
727745
def test_history_diff_with_incorrect_type(self):
728746
p = Poll.objects.create(question="what's up?", pub_date=today)
729747
p.question = "what's up, man?"

0 commit comments

Comments
 (0)