Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

disable_history() context manager #1387

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,40 @@ Changes
Unreleased
----------

**What's new:**

- Made ``skip_history_when_saving`` work when creating an object - not just when
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:**

- Removed ``HistoryManager.get_super_queryset()`` (gh-1387)
- Renamed the ``utils`` functions ``get_history_manager_from_history()``
to ``get_historical_records_of_instance()`` and ``get_app_model_primary_key_name()``
to ``get_pk_name()`` (gh-1387)

**Deprecations:**

- 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:**

- 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-1387)
- 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-1387)

3.7.0 (2024-05-29)
------------------
Expand Down
77 changes: 77 additions & 0 deletions docs/disabling_history.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
Disable Creating Historical Records
===================================

``save_without_historical_record()`` and ``delete_without_historical_record()``
-------------------------------------------------------------------------------

These methods are automatically added to a model when registering it for history-tracking
(i.e. defining a ``HistoricalRecords`` manager on the model),
and can be called instead of ``save()`` and ``delete()``, respectively.

Using the ``disable_history()`` context manager
-----------------------------------------------

``disable_history()`` has three ways of being called:

#. 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this is better as models to match the signature of isinstance? This would allow people to disable multiple models in a single context manager and it shouldn't be too much additional complexity on our end.

#. With ``instance_predicate``: Only disable history creation for model instances passing
this predicate.
Comment on lines +20 to +21
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This argument name doesn't seem very beginner friendly. The best thing I can come up with is should_disable_callback. Or maybe should_disable?

We should also document what arguments this function must take.


See some examples below:

.. code-block:: python

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(...)

# 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?")

Overriding ``create_historical_record()``
-----------------------------------------

For even more fine-grained control, you can subclass ``HistoricalRecords`` and override
its ``create_historical_record()`` method, for example like this:

.. code-block:: python

class CustomHistoricalRecords(HistoricalRecords):
def create_historical_record(
self, instance: models.Model, history_type: str, *args, **kwargs
) -> None:
# Don't create records for "ignore" polls that are being deleted
if "ignore" in poll.question and history_type == "-":
return

super().create_historical_record(instance, history_type, *args, **kwargs)


class Poll(models.Model):
# ...
history = CustomHistoricalRecords()

The ``SIMPLE_HISTORY_ENABLED`` setting
--------------------------------------

Disable the creation of historical records for *all* models
by adding the following line to your settings:
Comment on lines +72 to +73
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to indicate this defaults to True?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, this can be ignored. I realized that it's just moving.


.. code-block:: python

SIMPLE_HISTORY_ENABLED = False
1 change: 1 addition & 0 deletions docs/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ Documentation
admin
historical_model
user_tracking
disabling_history
signals
history_diffing
multiple_dbs
Expand Down
50 changes: 0 additions & 50 deletions docs/querying_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -175,56 +175,6 @@ model history.
<Poll: Poll object as of 2010-10-25 18:04:13.814128>


Save without creating historical records
----------------------------------------

If you want to save model objects without triggering the creation of any historical
records, you can do the following:

.. code-block:: python

poll.skip_history_when_saving = True
poll.save()
# We recommend deleting the attribute afterward
del poll.skip_history_when_saving

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

.. 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

.. note::
Historical records will always be created when calling the ``create()`` manager method.

Alternatively, call the ``save_without_historical_record()`` method on each object
instead of ``save()``.
This method is automatically added to a model when registering it for history-tracking
(i.e. defining a ``HistoricalRecords`` manager field on the model),
and it looks like this:

.. code-block:: python

def save_without_historical_record(self, *args, **kwargs):
self.skip_history_when_saving = True
try:
ret = self.save(*args, **kwargs)
finally:
del self.skip_history_when_saving
return ret

Or disable the creation of historical records for *all* models
by adding the following line to your settings:

.. code-block:: python

SIMPLE_HISTORY_ENABLED = False


Filtering data using a relationship to the model
------------------------------------------------

Expand Down
25 changes: 11 additions & 14 deletions simple_history/manager.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
from django.conf import settings
from django.db import models
from django.db.models import Exists, OuterRef, Q, QuerySet
from django.utils import timezone

from simple_history.utils import (
get_app_model_primary_key_name,
get_change_reason_from_object,
)
from . import utils

# when converting a historical record to an instance, this attribute is added
# to the instance so that code can reverse the instance to its historical record
Expand Down Expand Up @@ -118,16 +114,13 @@ def __init__(self, model, instance=None):
self.model = model
self.instance = instance

def get_super_queryset(self):
return super().get_queryset()

def get_queryset(self):
qs = self.get_super_queryset()
qs = super().get_queryset()
if self.instance is None:
return qs

key_name = get_app_model_primary_key_name(self.instance)
return self.get_super_queryset().filter(**{key_name: self.instance.pk})
pk_name = utils.get_pk_name(self.instance._meta.model)
return qs.filter(**{pk_name: self.instance.pk})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pk_name = utils.get_pk_name(self.instance._meta.model)
pk_name = utils.get_pk_name(self.model)

Does that work just as well?


def most_recent(self):
"""
Expand Down Expand Up @@ -222,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 []

Comment on lines +218 to +220
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking API change and should be documented in the release notes.

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 All @@ -241,7 +238,7 @@ def bulk_history_create(
instance, "_history_date", default_date or timezone.now()
),
history_user=history_user,
history_change_reason=get_change_reason_from_object(instance)
history_change_reason=utils.get_change_reason_from_object(instance)
or default_change_reason,
history_type=history_type,
**{
Expand Down
Loading
Loading