Skip to content

Commit

Permalink
[TODO: PR number] Made some helper utils support models and instances
Browse files Browse the repository at this point in the history
Their code already works with passing an instance instead of a model,
so might as well make it official.
  • Loading branch information
ddabble committed Sep 8, 2024
1 parent d150848 commit 62b03c0
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 26 deletions.
3 changes: 3 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ Unreleased
- Improved performance of the ``latest_of_each()`` history manager method (gh-1360)
- Moved the "Save without creating historical records" subsection of "Querying History"
in the docs to a new section: "Disable Creating Historical Records" (gh-13xx)
- The ``utils`` functions ``get_history_manager_for_model()`` and
``get_history_model_for_model()`` now explicitly support being passed model instances
instead of just model types (gh-13xx)

3.7.0 (2024-05-29)
------------------
Expand Down
74 changes: 57 additions & 17 deletions simple_history/tests/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
Restaurant,
Street,
TestHistoricParticipanToHistoricOrganization,
TestOrganizationWithHistory,
TestParticipantToHistoricOrganization,
TrackedAbstractBaseA,
TrackedConcreteBase,
Expand All @@ -103,24 +104,37 @@ def test_update_change_reason_with_excluded_fields(self):
class HistoryTrackedModelTestInfo:
model: Type[Model]
history_manager_name: Optional[str]
init_kwargs: dict

def __init__(
self,
model: Type[Model],
history_manager_name: Optional[str] = "history",
**init_kwargs,
):
self.model = model
self.history_manager_name = history_manager_name
self.init_kwargs = init_kwargs


class GetHistoryManagerAndModelHelpersTestCase(TestCase):
@classmethod
def setUpClass(cls):
super().setUpClass()

user = User.objects.create(username="user")
poll_kwargs = {"pub_date": timezone.now()}
poll = Poll.objects.create(**poll_kwargs)
choice_kwargs = {"poll": poll, "votes": 0}
choice = Choice.objects.create(**choice_kwargs)
place = Place.objects.create()
org_kwarg = {
"organization": TestOrganizationWithHistory.objects.create(),
}

H = HistoryTrackedModelTestInfo
cls.history_tracked_models = [
H(Choice),
H(Choice, **choice_kwargs),
H(ConcreteAttr),
H(ConcreteExternal),
H(ConcreteUtil),
Expand All @@ -135,23 +149,23 @@ def setUpClass(cls):
H(OverrideModelNameAsCallable),
H(OverrideModelNameRegisterMethod1),
H(OverrideModelNameUsingBaseModel1),
H(Poll),
H(PollChildBookWithManyToMany),
H(PollWithAlternativeManager),
H(PollWithCustomManager),
H(PollWithExcludedFKField),
H(Poll, **poll_kwargs),
H(PollChildBookWithManyToMany, **poll_kwargs),
H(PollWithAlternativeManager, **poll_kwargs),
H(PollWithCustomManager, **poll_kwargs),
H(PollWithExcludedFKField, place=place, **poll_kwargs),
H(PollWithHistoricalSessionAttr),
H(PollWithManyToMany),
H(PollWithManyToManyCustomHistoryID),
H(PollWithManyToManyWithIPAddress),
H(PollWithQuerySetCustomizations),
H(PollWithManyToMany, **poll_kwargs),
H(PollWithManyToManyCustomHistoryID, **poll_kwargs),
H(PollWithManyToManyWithIPAddress, **poll_kwargs),
H(PollWithQuerySetCustomizations, **poll_kwargs),
H(PollWithSelfManyToMany),
H(Restaurant, "updates"),
H(TestHistoricParticipanToHistoricOrganization),
H(Restaurant, "updates", rating=0),
H(TestHistoricParticipanToHistoricOrganization, **org_kwarg),
H(TrackedConcreteBase),
H(TrackedWithAbstractBase),
H(TrackedWithConcreteBase),
H(Voter),
H(Voter, user=user, choice=choice),
H(external.ExternalModel),
H(external.ExternalModelRegistered, "histories"),
H(external.Poll),
Expand All @@ -161,11 +175,11 @@ def setUpClass(cls):
H(AbstractModelCallable1, None),
H(BaseModel, None),
H(FirstLevelInheritedModel, None),
H(HardbackBook, None),
H(HardbackBook, None, isbn="123", price=0),
H(Place, None),
H(PollParentWithManyToMany, None),
H(Profile, None),
H(TestParticipantToHistoricOrganization, None),
H(PollParentWithManyToMany, None, **poll_kwargs),
H(Profile, None, date_of_birth=timezone.now().date()),
H(TestParticipantToHistoricOrganization, None, **org_kwarg),
H(TrackedAbstractBaseA, None),
]

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

# Passing a model instance should also work
instance = model(**model_info.init_kwargs)
instance.save()
manager = get_history_manager_for_model(instance)
assert_history_manager(manager, model_info)

for model_info in self.models_without_history_manager:
with self.subTest(model_info=model_info):
model = model_info.model
with self.assertRaises(NotHistoricalModelError):
get_history_manager_for_model(model)

# The same error should be raised if passing a model instance
if not model._meta.abstract:
instance = model(**model_info.init_kwargs)
instance.save()
with self.assertRaises(NotHistoricalModelError):
get_history_manager_for_model(instance)

def test__get_history_model_for_model(self):
"""Test that ``get_history_model_for_model()`` returns the expected value
for various models."""
Expand All @@ -207,12 +234,25 @@ def test__get_history_model_for_model(self):
self.assertTrue(issubclass(historical_model, HistoricalChanges))
self.assertEqual(historical_model.instance_type, model)

# Passing a model instance should also work
instance = model(**model_info.init_kwargs)
instance.save()
historical_model_from_instance = get_history_model_for_model(instance)
self.assertEqual(historical_model_from_instance, historical_model)

for model_info in self.models_without_history_manager:
with self.subTest(model_info=model_info):
model = model_info.model
with self.assertRaises(NotHistoricalModelError):
get_history_model_for_model(model)

# The same error should be raised if passing a model instance
if not model._meta.abstract:
instance = model(**model_info.init_kwargs)
instance.save()
with self.assertRaises(NotHistoricalModelError):
get_history_model_for_model(instance)

def test__get_pk_name(self):
"""Test that ``get_pk_name()`` returns the expected value for various models."""
self.assertEqual(get_pk_name(Poll), "id")
Expand Down
24 changes: 15 additions & 9 deletions simple_history/utils.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import TYPE_CHECKING, Optional, Type
from typing import TYPE_CHECKING, Optional, Type, Union

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


def get_history_manager_for_model(model: Type[Model]) -> "HistoryManager":
"""Return the history manager for ``model``.
def get_history_manager_for_model(
model_or_instance: Union[Type[Model], Model]
) -> "HistoryManager":
"""Return the history manager for ``model_or_instance``.
:raise NotHistoricalModelError: If the model has not been registered to track
history.
"""
try:
manager_name = model._meta.simple_history_manager_attribute
manager_name = model_or_instance._meta.simple_history_manager_attribute
except AttributeError:
raise NotHistoricalModelError(f"Cannot find a historical model for {model}.")
return getattr(model, manager_name)
raise NotHistoricalModelError(
f"Cannot find a historical model for {model_or_instance}."
)
return getattr(model_or_instance, manager_name)


def get_history_model_for_model(model: Type[Model]) -> Type["HistoricalChanges"]:
"""Return the history model for ``model``.
def get_history_model_for_model(
model_or_instance: Union[Type[Model], Model]
) -> Type["HistoricalChanges"]:
"""Return the history model for ``model_or_instance``.
:raise NotHistoricalModelError: If the model has not been registered to track
history.
"""
return get_history_manager_for_model(model).model
return get_history_manager_for_model(model_or_instance).model


def get_historical_records_of_instance(
Expand Down

0 comments on commit 62b03c0

Please sign in to comment.