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

Enable diffing m2m fields #932

Merged
merged 6 commits into from
Sep 28, 2022
Merged
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
4 changes: 3 additions & 1 deletion AUTHORS.rst
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ Authors
- Bheesham Persaud (`bheesham <https://github.com/bheesham>`_)
- `bradford281 <https://github.com/bradford281>`_
- Brian Armstrong (`barm <https://github.com/barm>`_)
- Buddy Lindsey, Jr.
- Brian Dixon
- Brian Mesick (`bmedx <https://github.com/bmedx>`_)
- Buddy Lindsey, Jr.
- Carlos San Emeterio (`Carlos-San-Emeterio <https://github.com/Carlos-San-Emeterio>`_)
- Christopher Broderick (`uhurusurfa <https://github.com/uhurusurfa>`_)
- Christopher Johns (`tyrantwave <https://github.com/tyrantwave>`_)
Expand Down Expand Up @@ -114,6 +115,7 @@ Authors
- Stefan Borer (`sbor23 <https://github.com/sbor23>`_)
- Steven Buss (`sbuss <https://github.com/sbuss>`_)
- Steven Klass
- Thijs Kramer (`thijskramer <https://github.com/thijskramer>`_)
- Tim Schilling (`tim-schilling <https://github.com/tim-schilling>`_)
- Todd Wolfson (`twolfson <https://github.com/twolfson>`_)
- Tommy Beadle (`tbeadle <https://github.com/tbeadle>`_)
Expand Down
1 change: 1 addition & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Unreleased
- Fixed typos in the docs
- Removed n+1 query from ``bulk_create_with_history`` utility (gh-975)
- Started using ``exists`` query instead of ``count`` in ``populate_history`` command (gh-982)
- Add basic support for many-to-many fields (gh-399)
- Added support for Django 4.1 (gh-1021)

3.1.1 (2022-04-23)
Expand Down
39 changes: 39 additions & 0 deletions docs/historical_model.rst
Original file line number Diff line number Diff line change
Expand Up @@ -447,3 +447,42 @@ And you don't want to create database index for ``question``, it is necessary to

By default, django-simple-history keeps all indices. and even forces them on unique fields and relations.
WARNING: This will drop performance on historical lookups

Tracking many to many relationships
-----------------------------------
By default, many to many fields are ignored when tracking changes.
If you want to track many to many relationships, you need to define them explicitly:

.. code-block:: python

class Category(models.Model):
pass

class Poll(models.Model):
question = models.CharField(max_length=200)
categories = models.ManyToManyField(Category)
history = HistoricalRecords(many_to_many=[categories])

This will create a historical intermediate model that tracks each relational change
between `Poll` and `Category`.

You will see the many to many changes when diffing between two historical records:

.. code-block:: python

informal = Category(name="informal questions")
official = Category(name="official questions")
p = Poll.objects.create(question="what's up?")
p.save()
p.categories.add(informal, official)
p.categories.remove(informal)

last_record = p.history.latest()
previous_record = last_record.prev_record()
delta = last_record.diff_against(previous_record)

for change in delta.changes:
print("{} changed from {} to {}")

# Output:
# categories changed from [{'poll': 1, 'category': 1}, { 'poll': 1, 'category': 2}] to [{'poll': 1, 'category': 2}]
130 changes: 130 additions & 0 deletions simple_history/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import importlib
import uuid
import warnings
from functools import partial

from django.apps import apps
from django.conf import settings
Expand All @@ -18,6 +19,7 @@
create_reverse_many_to_one_manager,
)
from django.db.models.query import QuerySet
from django.db.models.signals import m2m_changed
from django.forms.models import model_to_dict
from django.urls import reverse
from django.utils import timezone
Expand Down Expand Up @@ -65,6 +67,7 @@ def _history_user_setter(historical_instance, user):

class HistoricalRecords:
thread = context = LocalContext() # retain thread for backwards compatibility
m2m_models = {}

def __init__(
self,
Expand All @@ -90,6 +93,7 @@ def __init__(
user_db_constraint=True,
no_db_index=list(),
excluded_field_kwargs=None,
m2m_fields=(),
):
self.user_set_verbose_name = verbose_name
self.user_set_verbose_name_plural = verbose_name_plural
Expand All @@ -109,6 +113,7 @@ def __init__(
self.user_setter = history_user_setter
self.related_name = related_name
self.use_base_model_db = use_base_model_db
self.m2m_fields = m2m_fields

if isinstance(no_db_index, str):
no_db_index = [no_db_index]
Expand Down Expand Up @@ -172,6 +177,7 @@ def finalize(self, sender, **kwargs):
)
)
history_model = self.create_history_model(sender, inherited)

if inherited:
# Make sure history model is in same module as concrete model
module = importlib.import_module(history_model.__module__)
Expand All @@ -183,11 +189,29 @@ def finalize(self, sender, **kwargs):
# so the signal handlers can't use weak references.
models.signals.post_save.connect(self.post_save, sender=sender, weak=False)
models.signals.post_delete.connect(self.post_delete, sender=sender, weak=False)
for field in self.m2m_fields:
m2m_changed.connect(
partial(self.m2m_changed, attr=field.name),
sender=field.remote_field.through,
weak=False,
)

descriptor = HistoryDescriptor(history_model)
setattr(sender, self.manager_name, descriptor)
sender._meta.simple_history_manager_attribute = self.manager_name

for field in self.m2m_fields:
m2m_model = self.create_history_m2m_model(
history_model, field.remote_field.through
)
self.m2m_models[field] = m2m_model

module = importlib.import_module(self.module)
setattr(module, m2m_model.__name__, m2m_model)

m2m_descriptor = HistoryDescriptor(m2m_model)
setattr(history_model, field.name, m2m_descriptor)

def get_history_model_name(self, model):
if not self.custom_model_name:
return f"Historical{model._meta.object_name}"
Expand All @@ -210,13 +234,58 @@ def get_history_model_name(self, model):
)
)

def create_history_m2m_model(self, model, through_model):
attrs = {
"__module__": self.module,
"__str__": lambda self: "{} as of {}".format(
self._meta.verbose_name, self.history.history_date
),
}

app_module = "%s.models" % model._meta.app_label

if model.__module__ != self.module:
# registered under different app
attrs["__module__"] = self.module
elif app_module != self.module:
# Abuse an internal API because the app registry is loading.
app = apps.app_configs[model._meta.app_label]
models_module = app.name
attrs["__module__"] = models_module

# Get the primary key to the history model this model will look up to
attrs["m2m_history_id"] = self._get_history_id_field()
attrs["history"] = models.ForeignKey(
model,
db_constraint=False,
on_delete=models.DO_NOTHING,
)
attrs["instance_type"] = through_model

fields = self.copy_fields(through_model)
attrs.update(fields)

name = self.get_history_model_name(through_model)
registered_models[through_model._meta.db_table] = through_model
meta_fields = {"verbose_name": name}

if self.app:
meta_fields["app_label"] = self.app

attrs.update(Meta=type(str("Meta"), (), meta_fields))

m2m_history_model = type(str(name), (models.Model,), attrs)

return m2m_history_model

def create_history_model(self, model, inherited):
"""
Creates a historical model to associate with the model provided.
"""
attrs = {
"__module__": self.module,
"_history_excluded_fields": self.excluded_fields,
"_history_m2m_fields": self.m2m_fields,
}

app_module = "%s.models" % model._meta.app_label
Expand Down Expand Up @@ -559,6 +628,37 @@ def get_change_reason_for_object(self, instance, history_type, using):
"""
return get_change_reason_from_object(instance)

def m2m_changed(self, instance, action, attr, pk_set, reverse, **_):
if hasattr(instance, "skip_history_when_saving"):
return

if action in ("post_add", "post_remove", "post_clear"):
# It should be safe to ~ this since the row must exist to modify m2m on it
self.create_historical_record(instance, "~")

def create_historical_record_m2ms(self, history_instance, instance):
for field in self.m2m_fields:
m2m_history_model = self.m2m_models[field]
original_instance = history_instance.instance
through_model = getattr(original_instance, field.name).through

insert_rows = []

through_field_name = type(original_instance).__name__.lower()

rows = through_model.objects.filter(**{through_field_name: instance})

for row in rows:
insert_row = {"history": history_instance}

for through_model_field in through_model._meta.fields:
insert_row[through_model_field.name] = getattr(
row, through_model_field.name
)
insert_rows.append(m2m_history_model(**insert_row))

m2m_history_model.objects.bulk_create(insert_rows)

def create_historical_record(self, instance, history_type, using=None):
using = using if self.use_base_model_db else None
history_date = getattr(instance, "_history_date", timezone.now())
Expand Down Expand Up @@ -595,6 +695,7 @@ def create_historical_record(self, instance, history_type, using=None):
)

history_instance.save(using=using)
self.create_historical_record_m2ms(history_instance, instance)

post_create_historical_record.send(
sender=manager.model,
Expand Down Expand Up @@ -800,6 +901,35 @@ def diff_against(self, old_history, excluded_fields=None, included_fields=None):
changes.append(ModelChange(field, old_value, current_value))
changed_fields.append(field)

# Separately compare m2m fields:
for field in old_history._history_m2m_fields:
# First retrieve a single item to get the field names from:
reference_history_m2m_item = (
getattr(old_history, field.name).first()
or getattr(self, field.name).first()
)
history_field_names = []
if reference_history_m2m_item:
# Create a list of field names to compare against.
# The list is generated without the primary key of the intermediate
# table, the foreign key to the history record, and the actual 'history'
# field, to avoid false positives while diffing.
history_field_names = [
f.name
for f in reference_history_m2m_item._meta.fields
if f.editable and f.name not in ["id", "m2m_history_id", "history"]
]

old_rows = list(
getattr(old_history, field.name).values(*history_field_names)
)
new_rows = list(getattr(self, field.name).values(*history_field_names))

if old_rows != new_rows:
change = ModelChange(field.name, old_rows, new_rows)
changes.append(change)
changed_fields.append(field.name)

return ModelDelta(changes, changed_fields, old_history, self)


Expand Down
18 changes: 18 additions & 0 deletions simple_history/tests/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,24 @@ def get_absolute_url(self):
return reverse("poll-detail", kwargs={"pk": self.pk})


class PollWithManyToMany(models.Model):
question = models.CharField(max_length=200)
pub_date = models.DateTimeField("date published")
places = models.ManyToManyField("Place")

history = HistoricalRecords(m2m_fields=[places])


class PollWithSeveralManyToMany(models.Model):
question = models.CharField(max_length=200)
pub_date = models.DateTimeField("date published")
places = models.ManyToManyField("Place", related_name="places_poll")
restaurants = models.ManyToManyField("Restaurant", related_name="restaurants_poll")
books = models.ManyToManyField("Book", related_name="books_poll")

history = HistoricalRecords(m2m_fields=[places, restaurants, books])


class CustomAttrNameForeignKey(models.ForeignKey):
def __init__(self, *args, **kwargs):
self.attr_name = kwargs.pop("attr_name", None)
Expand Down
Loading