Skip to content

Commit

Permalink
Added a disable_history() context manager
Browse files Browse the repository at this point in the history
This replaces setting the `skip_history_when_saving` attribute on
a model instance, which has been deprecated (and replaced with
`disable_history()` or the utils mentioned below where possible),
as well as having been removed from the docs and tests.
(`HistoricalRecords.post_delete()` does not generate deprecation
warnings on it, since support for `skip_history_when_saving` while
deleting objects is part of the next, unreleased version.)

The context manager was inspired by
#642 (comment)

Also added a couple useful utils related to `disable_history()`:
`is_history_disabled()` and a `DisableHistoryInfo` dataclass - see their
docstrings in `utils.py`.
  • Loading branch information
ddabble committed Sep 10, 2024
1 parent 78286f6 commit 8ea956c
Show file tree
Hide file tree
Showing 11 changed files with 815 additions and 105 deletions.
6 changes: 6 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ Unreleased
updating an object (gh-1262)
- Added ``delete_without_historical_record()`` to all history-tracked model objects,
which complements ``save_without_historical_record()`` (gh-1387)
- Added a ``disable_history()`` context manager, which disables history record creation
while it's active; see usage in the docs under "Disable Creating Historical Records"
(gh-1387)

**Breaking changes:**

Expand All @@ -23,6 +26,9 @@ Unreleased
- Deprecated the undocumented ``HistoricalRecords.thread`` - use
``HistoricalRecords.context`` instead. The former attribute will be removed in
version 3.10 (gh-1387)
- Deprecated ``skip_history_when_saving`` in favor of the newly added
``disable_history()`` context manager. The former attribute will be removed in
version 4.0 (gh-1387)

**Fixes and improvements:**

Expand Down
48 changes: 27 additions & 21 deletions docs/disabling_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,34 +8,40 @@ These methods are automatically added to a model when registering it for history
(i.e. defining a ``HistoricalRecords`` manager on the model),
and can be called instead of ``save()`` and ``delete()``, respectively.

Setting the ``skip_history_when_saving`` attribute
--------------------------------------------------
Using the ``disable_history()`` context manager
-----------------------------------------------

If you want to save or delete model objects without triggering the creation of any
historical records, you can do the following:
``disable_history()`` has three ways of being called:

.. code-block:: python
poll.skip_history_when_saving = True
# It applies both when saving...
poll.save()
# ...and when deleting
poll.delete()
# We recommend deleting the attribute afterward
del poll.skip_history_when_saving
#. With no arguments: This will disable all historical record creation
(as if the ``SIMPLE_HISTORY_ENABLED`` setting was set to ``False``; see below)
within the context manager's ``with`` block.
#. With ``only_for_model``: Only disable history creation for the provided model type.
#. With ``instance_predicate``: Only disable history creation for model instances passing
this predicate.

This also works when creating an object, but only when calling ``save()``:
See some examples below:

.. code-block:: python
# Note that `Poll.objects.create()` is not called
poll = Poll(question="Why?")
poll.skip_history_when_saving = True
poll.save()
del poll.skip_history_when_saving
from simple_history.utils import disable_history
# No historical records are created
with disable_history():
User.objects.create(...)
Poll.objects.create(...)
# A historical record is only created for the poll
with disable_history(only_for_model=User):
User.objects.create(...)
Poll.objects.create(...)
.. note::
Historical records will always be created when calling the ``create()`` manager method.
# A historical record is created for the second poll, but not for the first poll
# (remember to check the instance type in the passed function if you expect
# historical records of more than one model to be created inside the `with` block)
with disable_history(instance_predicate=lambda poll: "ignore" in poll.question):
Poll.objects.create(question="ignore this")
Poll.objects.create(question="what's up?")
The ``SIMPLE_HISTORY_ENABLED`` setting
--------------------------------------
Expand Down
9 changes: 6 additions & 3 deletions simple_history/manager.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
from django.conf import settings
from django.db import models
from django.db.models import Exists, OuterRef, Q, QuerySet
from django.utils import timezone
Expand Down Expand Up @@ -216,15 +215,19 @@ def bulk_history_create(
If called by bulk_update_with_history, use the update boolean and
save the history_type accordingly.
"""
if not getattr(settings, "SIMPLE_HISTORY_ENABLED", True):
return
info = utils.DisableHistoryInfo.get()
if info.disabled_globally:
return []

history_type = "+"
if update:
history_type = "~"

historical_instances = []
for instance in objs:
if info.disabled_for(instance):
continue

history_user = getattr(
instance,
"_history_user",
Expand Down
51 changes: 31 additions & 20 deletions simple_history/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from typing import (
TYPE_CHECKING,
Any,
Callable,
ClassVar,
Dict,
Iterable,
Expand Down Expand Up @@ -195,31 +196,31 @@ def contribute_to_class(self, cls, name):
warnings.warn(msg, UserWarning)

def add_extra_methods(self, cls):
def get_instance_predicate(
self: models.Model,
) -> Callable[[models.Model], bool]:
def predicate(instance: models.Model) -> bool:
return instance == self

return predicate

def save_without_historical_record(self: models.Model, *args, **kwargs):
"""
Save the model instance without creating a historical record.
Make sure you know what you're doing before using this method.
"""
self.skip_history_when_saving = True
try:
ret = self.save(*args, **kwargs)
finally:
del self.skip_history_when_saving
return ret
with utils.disable_history(instance_predicate=get_instance_predicate(self)):
return self.save(*args, **kwargs)

def delete_without_historical_record(self: models.Model, *args, **kwargs):
"""
Delete the model instance without creating a historical record.
Make sure you know what you're doing before using this method.
"""
self.skip_history_when_saving = True
try:
ret = self.delete(*args, **kwargs)
finally:
del self.skip_history_when_saving
return ret
with utils.disable_history(instance_predicate=get_instance_predicate(self)):
return self.delete(*args, **kwargs)

cls.save_without_historical_record = save_without_historical_record
cls.delete_without_historical_record = delete_without_historical_record
Expand Down Expand Up @@ -682,18 +683,22 @@ def get_meta_options(self, model):
def post_save(
self, instance: models.Model, created: bool, using: str = None, **kwargs
):
if not getattr(settings, "SIMPLE_HISTORY_ENABLED", True):
return
if hasattr(instance, "skip_history_when_saving"):
warnings.warn(
"Setting 'skip_history_when_saving' has been deprecated in favor of the"
" 'disable_history()' context manager."
" Support for the former attribute will be removed in version 4.0.",
DeprecationWarning,
)
return
if utils.is_history_disabled(instance):
return

if not kwargs.get("raw", False):
self.create_historical_record(instance, created and "+" or "~", using=using)

def post_delete(self, instance: models.Model, using: str = None, **kwargs):
if not getattr(settings, "SIMPLE_HISTORY_ENABLED", True):
return
if hasattr(instance, "skip_history_when_saving"):
if utils.is_history_disabled(instance):
return

if self.cascade_delete_history:
Expand All @@ -709,10 +714,16 @@ def get_change_reason_for_object(self, instance, history_type, using):
"""
return utils.get_change_reason_from_object(instance)

def m2m_changed(self, instance, action, attr, pk_set, reverse, **_):
if not getattr(settings, "SIMPLE_HISTORY_ENABLED", True):
return
def m2m_changed(self, instance: models.Model, action: str, **kwargs):
if hasattr(instance, "skip_history_when_saving"):
warnings.warn(
"Setting 'skip_history_when_saving' has been deprecated in favor of the"
" 'disable_history()' context manager."
" Support for the former attribute will be removed in version 4.0.",
DeprecationWarning,
)
return
if utils.is_history_disabled(instance):
return

if action in ("post_add", "post_remove", "post_clear"):
Expand Down
5 changes: 2 additions & 3 deletions simple_history/tests/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@
from django.conf import settings
from django.db import models
from django.db.models.deletion import CASCADE
from django.db.models.fields.related import ForeignKey
from django.urls import reverse

from simple_history import register
from simple_history import register, utils
from simple_history.manager import HistoricalQuerySet, HistoryManager
from simple_history.models import HistoricalRecords, HistoricForeignKey

Expand Down Expand Up @@ -355,7 +354,7 @@ class Person(models.Model):
history = HistoricalRecords()

def save(self, *args, **kwargs):
if hasattr(self, "skip_history_when_saving"):
if utils.DisableHistoryInfo.get().disabled_for(self):
raise RuntimeError("error while saving")
else:
super().save(*args, **kwargs)
Expand Down
27 changes: 25 additions & 2 deletions simple_history/tests/tests/test_deprecation.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import unittest
from django.test import TestCase
from django.utils import timezone

from simple_history import __version__
from simple_history.models import HistoricalRecords
from simple_history.templatetags.simple_history_admin_list import display_list

from ..models import Place, PollWithManyToMany

class DeprecationWarningTest(unittest.TestCase):

class DeprecationWarningTest(TestCase):
def test__display_list__warns_deprecation(self):
with self.assertWarns(DeprecationWarning):
display_list({})
Expand All @@ -30,3 +33,23 @@ def test__HistoricalRecords_thread__warns_deprecation(self):
# DEV: `_DeprecatedThreadDescriptor` and the `thread` attribute of
# `HistoricalRecords` should be removed when 3.10 is released
self.assertLess(__version__, "3.10")

def test__skip_history_when_saving__warns_deprecation(self):
place = Place.objects.create(name="Here")

poll = PollWithManyToMany(question="why?", pub_date=timezone.now())
poll.skip_history_when_saving = True
with self.assertWarns(DeprecationWarning):
poll.save()
poll.question = "how?"
with self.assertWarns(DeprecationWarning):
poll.save()
with self.assertWarns(DeprecationWarning):
poll.places.add(place)
self.assertEqual(PollWithManyToMany.history.count(), 0)
self.assertEqual(poll.history.count(), 0)

# DEV: The `if` statements checking for `skip_history_when_saving` (in the
# `post_save()` and `m2m_changed()` methods of `HistoricalRecords`)
# should be removed when 4.0 is released
self.assertLess(__version__, "4.0")
43 changes: 43 additions & 0 deletions simple_history/tests/tests/test_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from django.test import TestCase, override_settings, skipUnlessDBFeature

from simple_history.manager import SIMPLE_HISTORY_REVERSE_ATTR_NAME
from simple_history.utils import disable_history

from ..models import Choice, Document, Poll, RankedDocument
from .utils import HistoricalTestCase
Expand Down Expand Up @@ -289,6 +290,23 @@ def test_simple_bulk_history_create_without_history_enabled(self):
Poll.history.bulk_history_create(self.data)
self.assertEqual(Poll.history.count(), 0)

def test_simple_bulk_history_create_with__disable_history(self):
with disable_history():
Poll.history.bulk_history_create(self.data)
self.assertEqual(Poll.history.count(), 0)

def test_simple_bulk_history_create_with__disable_history__only_for_model(self):
with disable_history(only_for_model=Poll):
Poll.history.bulk_history_create(self.data)
self.assertEqual(Poll.history.count(), 0)

def test_simple_bulk_history_create_with__disable_history__instance_predicate(self):
with disable_history(instance_predicate=lambda poll: poll.id == 2):
Poll.history.bulk_history_create(self.data)
self.assertEqual(Poll.history.count(), 3)
historical_poll_ids = sorted(record.id for record in Poll.history.all())
self.assertListEqual(historical_poll_ids, [1, 3, 4])

def test_bulk_history_create_with_change_reason(self):
for poll in self.data:
poll._change_reason = "reason"
Expand Down Expand Up @@ -412,6 +430,31 @@ def test_simple_bulk_history_create(self):
self.assertEqual(created, [])
self.assertEqual(Poll.history.count(), 4)

@override_settings(SIMPLE_HISTORY_ENABLED=False)
def test_simple_bulk_history_update_without_history_enabled(self):
Poll.history.bulk_history_create(self.data, update=True)
self.assertEqual(Poll.history.count(), 0)

def test_simple_bulk_history_update_with__disable_history(self):
with disable_history():
Poll.history.bulk_history_create(self.data, update=True)
self.assertEqual(Poll.history.count(), 0)

def test_simple_bulk_history_update_with__disable_history__only_for_model(self):
with disable_history(only_for_model=Poll):
Poll.history.bulk_history_create(self.data, update=True)
self.assertEqual(Poll.history.count(), 0)

def test_simple_bulk_history_update_with__disable_history__instance_predicate(self):
with disable_history(instance_predicate=lambda poll: poll.id == 2):
Poll.history.bulk_history_create(self.data, update=True)
self.assertEqual(Poll.history.count(), 3)
historical_poll_ids = sorted(record.id for record in Poll.history.all())
self.assertListEqual(historical_poll_ids, [1, 3, 4])
self.assertTrue(
all(record.history_type == "~" for record in Poll.history.all())
)

def test_bulk_history_create_with_change_reason(self):
for poll in self.data:
poll._change_reason = "reason"
Expand Down
43 changes: 3 additions & 40 deletions simple_history/tests/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ def test__delete_without_historical_record__creates_no_records(self):
)

@override_settings(SIMPLE_HISTORY_ENABLED=False)
def test_save_with_disabled_history(self):
def test_saving_without_history_enabled_creates_no_records(self):
anthony = Person.objects.create(name="Anthony Gillard")
anthony.name = "something else"
anthony.save()
Expand All @@ -330,7 +330,6 @@ def test_save_raises_exception(self):
anthony = Person(name="Anthony Gillard")
with self.assertRaises(RuntimeError):
anthony.save_without_historical_record()
self.assertFalse(hasattr(anthony, "skip_history_when_saving"))
self.assertEqual(Person.history.count(), 0)
anthony.save()
self.assertEqual(Person.history.count(), 1)
Expand Down Expand Up @@ -2430,45 +2429,9 @@ def test_m2m_relation(self):
self.assertEqual(self.poll.history.all()[0].places.count(), 0)
self.assertEqual(poll_2.history.all()[0].places.count(), 2)

def test_skip_history_when_updating_an_object(self):
skip_poll = PollWithManyToMany.objects.create(
question="skip history?", pub_date=today
)
self.assertEqual(skip_poll.history.all().count(), 1)
self.assertEqual(skip_poll.history.all()[0].places.count(), 0)

skip_poll.skip_history_when_saving = True

skip_poll.question = "huh?"
skip_poll.save()
skip_poll.places.add(self.place)

self.assertEqual(skip_poll.history.all().count(), 1)
self.assertEqual(skip_poll.history.all()[0].places.count(), 0)

del skip_poll.skip_history_when_saving
place_2 = Place.objects.create(name="Place 2")

skip_poll.places.add(place_2)

self.assertEqual(skip_poll.history.all().count(), 2)
self.assertEqual(skip_poll.history.all()[0].places.count(), 2)

def test_skip_history_when_creating_an_object(self):
initial_poll_count = PollWithManyToMany.objects.count()

skip_poll = PollWithManyToMany(question="skip history?", pub_date=today)
skip_poll.skip_history_when_saving = True
skip_poll.save()
skip_poll.places.add(self.place)

self.assertEqual(skip_poll.history.count(), 0)
self.assertEqual(PollWithManyToMany.objects.count(), initial_poll_count + 1)
self.assertEqual(skip_poll.places.count(), 1)

@override_settings(SIMPLE_HISTORY_ENABLED=False)
def test_saving_with_disabled_history_doesnt_create_records(self):
# 1 from `setUp()`
def test_saving_without_history_enabled_creates_no_records(self):
# 1 record from `setUp()`
self.assertEqual(PollWithManyToMany.history.count(), 1)

poll = PollWithManyToMany.objects.create(
Expand Down
Loading

0 comments on commit 8ea956c

Please sign in to comment.