Skip to content

Commit 78286f6

Browse files
committed
Made some helper utils support models and instances
Their code already works with passing an instance instead of a model, so might as well make it official.
1 parent 23c37dd commit 78286f6

File tree

3 files changed

+75
-26
lines changed

3 files changed

+75
-26
lines changed

CHANGES.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ Unreleased
2929
- Improved performance of the ``latest_of_each()`` history manager method (gh-1360)
3030
- Moved the "Save without creating historical records" subsection of "Querying History"
3131
in the docs to a new section: "Disable Creating Historical Records" (gh-1387)
32+
- The ``utils`` functions ``get_history_manager_for_model()`` and
33+
``get_history_model_for_model()`` now explicitly support being passed model instances
34+
instead of just model types (gh-1387)
3235

3336
3.7.0 (2024-05-29)
3437
------------------

simple_history/tests/tests/test_utils.py

Lines changed: 57 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@
7777
Restaurant,
7878
Street,
7979
TestHistoricParticipanToHistoricOrganization,
80+
TestOrganizationWithHistory,
8081
TestParticipantToHistoricOrganization,
8182
TrackedAbstractBaseA,
8283
TrackedConcreteBase,
@@ -103,24 +104,37 @@ def test_update_change_reason_with_excluded_fields(self):
103104
class HistoryTrackedModelTestInfo:
104105
model: Type[Model]
105106
history_manager_name: Optional[str]
107+
init_kwargs: dict
106108

107109
def __init__(
108110
self,
109111
model: Type[Model],
110112
history_manager_name: Optional[str] = "history",
113+
**init_kwargs,
111114
):
112115
self.model = model
113116
self.history_manager_name = history_manager_name
117+
self.init_kwargs = init_kwargs
114118

115119

116120
class GetHistoryManagerAndModelHelpersTestCase(TestCase):
117121
@classmethod
118122
def setUpClass(cls):
119123
super().setUpClass()
120124

125+
user = User.objects.create(username="user")
126+
poll_kwargs = {"pub_date": timezone.now()}
127+
poll = Poll.objects.create(**poll_kwargs)
128+
choice_kwargs = {"poll": poll, "votes": 0}
129+
choice = Choice.objects.create(**choice_kwargs)
130+
place = Place.objects.create()
131+
org_kwarg = {
132+
"organization": TestOrganizationWithHistory.objects.create(),
133+
}
134+
121135
H = HistoryTrackedModelTestInfo
122136
cls.history_tracked_models = [
123-
H(Choice),
137+
H(Choice, **choice_kwargs),
124138
H(ConcreteAttr),
125139
H(ConcreteExternal),
126140
H(ConcreteUtil),
@@ -135,23 +149,23 @@ def setUpClass(cls):
135149
H(OverrideModelNameAsCallable),
136150
H(OverrideModelNameRegisterMethod1),
137151
H(OverrideModelNameUsingBaseModel1),
138-
H(Poll),
139-
H(PollChildBookWithManyToMany),
140-
H(PollWithAlternativeManager),
141-
H(PollWithCustomManager),
142-
H(PollWithExcludedFKField),
152+
H(Poll, **poll_kwargs),
153+
H(PollChildBookWithManyToMany, **poll_kwargs),
154+
H(PollWithAlternativeManager, **poll_kwargs),
155+
H(PollWithCustomManager, **poll_kwargs),
156+
H(PollWithExcludedFKField, place=place, **poll_kwargs),
143157
H(PollWithHistoricalSessionAttr),
144-
H(PollWithManyToMany),
145-
H(PollWithManyToManyCustomHistoryID),
146-
H(PollWithManyToManyWithIPAddress),
147-
H(PollWithQuerySetCustomizations),
158+
H(PollWithManyToMany, **poll_kwargs),
159+
H(PollWithManyToManyCustomHistoryID, **poll_kwargs),
160+
H(PollWithManyToManyWithIPAddress, **poll_kwargs),
161+
H(PollWithQuerySetCustomizations, **poll_kwargs),
148162
H(PollWithSelfManyToMany),
149-
H(Restaurant, "updates"),
150-
H(TestHistoricParticipanToHistoricOrganization),
163+
H(Restaurant, "updates", rating=0),
164+
H(TestHistoricParticipanToHistoricOrganization, **org_kwarg),
151165
H(TrackedConcreteBase),
152166
H(TrackedWithAbstractBase),
153167
H(TrackedWithConcreteBase),
154-
H(Voter),
168+
H(Voter, user=user, choice=choice),
155169
H(external.ExternalModel),
156170
H(external.ExternalModelRegistered, "histories"),
157171
H(external.Poll),
@@ -161,11 +175,11 @@ def setUpClass(cls):
161175
H(AbstractModelCallable1, None),
162176
H(BaseModel, None),
163177
H(FirstLevelInheritedModel, None),
164-
H(HardbackBook, None),
178+
H(HardbackBook, None, isbn="123", price=0),
165179
H(Place, None),
166-
H(PollParentWithManyToMany, None),
167-
H(Profile, None),
168-
H(TestParticipantToHistoricOrganization, None),
180+
H(PollParentWithManyToMany, None, **poll_kwargs),
181+
H(Profile, None, date_of_birth=timezone.now().date()),
182+
H(TestParticipantToHistoricOrganization, None, **org_kwarg),
169183
H(TrackedAbstractBaseA, None),
170184
]
171185

@@ -191,12 +205,25 @@ def assert_history_manager(history_manager, info: HistoryTrackedModelTestInfo):
191205
manager = get_history_manager_for_model(model)
192206
assert_history_manager(manager, model_info)
193207

208+
# Passing a model instance should also work
209+
instance = model(**model_info.init_kwargs)
210+
instance.save()
211+
manager = get_history_manager_for_model(instance)
212+
assert_history_manager(manager, model_info)
213+
194214
for model_info in self.models_without_history_manager:
195215
with self.subTest(model_info=model_info):
196216
model = model_info.model
197217
with self.assertRaises(NotHistoricalModelError):
198218
get_history_manager_for_model(model)
199219

220+
# The same error should be raised if passing a model instance
221+
if not model._meta.abstract:
222+
instance = model(**model_info.init_kwargs)
223+
instance.save()
224+
with self.assertRaises(NotHistoricalModelError):
225+
get_history_manager_for_model(instance)
226+
200227
def test__get_history_model_for_model(self):
201228
"""Test that ``get_history_model_for_model()`` returns the expected value
202229
for various models."""
@@ -207,12 +234,25 @@ def test__get_history_model_for_model(self):
207234
self.assertTrue(issubclass(historical_model, HistoricalChanges))
208235
self.assertEqual(historical_model.instance_type, model)
209236

237+
# Passing a model instance should also work
238+
instance = model(**model_info.init_kwargs)
239+
instance.save()
240+
historical_model_from_instance = get_history_model_for_model(instance)
241+
self.assertEqual(historical_model_from_instance, historical_model)
242+
210243
for model_info in self.models_without_history_manager:
211244
with self.subTest(model_info=model_info):
212245
model = model_info.model
213246
with self.assertRaises(NotHistoricalModelError):
214247
get_history_model_for_model(model)
215248

249+
# The same error should be raised if passing a model instance
250+
if not model._meta.abstract:
251+
instance = model(**model_info.init_kwargs)
252+
instance.save()
253+
with self.assertRaises(NotHistoricalModelError):
254+
get_history_model_for_model(instance)
255+
216256
def test__get_pk_name(self):
217257
"""Test that ``get_pk_name()`` returns the expected value for various models."""
218258
self.assertEqual(get_pk_name(Poll), "id")

simple_history/utils.py

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
from typing import TYPE_CHECKING, Optional, Type
1+
from typing import TYPE_CHECKING, Optional, Type, Union
22

33
from django.db import transaction
44
from django.db.models import Case, ForeignKey, ManyToManyField, Model, Q, When
@@ -36,26 +36,32 @@ def update_change_reason(instance: Model, reason: Optional[str]) -> None:
3636
record.save()
3737

3838

39-
def get_history_manager_for_model(model: Type[Model]) -> "HistoryManager":
40-
"""Return the history manager for ``model``.
39+
def get_history_manager_for_model(
40+
model_or_instance: Union[Type[Model], Model]
41+
) -> "HistoryManager":
42+
"""Return the history manager for ``model_or_instance``.
4143
4244
:raise NotHistoricalModelError: If the model has not been registered to track
4345
history.
4446
"""
4547
try:
46-
manager_name = model._meta.simple_history_manager_attribute
48+
manager_name = model_or_instance._meta.simple_history_manager_attribute
4749
except AttributeError:
48-
raise NotHistoricalModelError(f"Cannot find a historical model for {model}.")
49-
return getattr(model, manager_name)
50+
raise NotHistoricalModelError(
51+
f"Cannot find a historical model for {model_or_instance}."
52+
)
53+
return getattr(model_or_instance, manager_name)
5054

5155

52-
def get_history_model_for_model(model: Type[Model]) -> Type["HistoricalChanges"]:
53-
"""Return the history model for ``model``.
56+
def get_history_model_for_model(
57+
model_or_instance: Union[Type[Model], Model]
58+
) -> Type["HistoricalChanges"]:
59+
"""Return the history model for ``model_or_instance``.
5460
5561
:raise NotHistoricalModelError: If the model has not been registered to track
5662
history.
5763
"""
58-
return get_history_manager_for_model(model).model
64+
return get_history_manager_for_model(model_or_instance).model
5965

6066

6167
def get_historical_records_of_instance(

0 commit comments

Comments
 (0)