Add m2m tracking support to django-simple-history

Working prototype with tests

- Custom through models may not work correctly
- Currently storing IDs for FKs, probably needs replacing

Now using model instances instead of IDs when possible

flake8 fixes

Delete dead code, comments, and change to bulk create

Clean up, add m2m support to diff_against

Add basic tests for several m2m fields on one model

Formatting fixes from Black

Housekeeping, fixing Python 2.7, removing Python 3.4

- Py3.4 removed as coverage no longer supports it, could be pinned instead, but I went with this.

remove python2 thing

Author: bmedx

AUTHORS.rst

@@ -20,8 +20,9 @@ Authors
 - Benjamin Mampaey (`bmampaey <https://github.com/bmampaey>`_)
 - `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>`_)

CHANGES.rst

@@ -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)
 
 3.1.1 (2022-04-23)
 ------------------

simple_history/models.py

@@ -2,6 +2,7 @@
 import importlib
 import uuid
 import warnings
+from functools import partial
 
 from django.apps import apps
 from django.conf import settings
@@ -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
@@ -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,
@@ -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
@@ -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]
@@ -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__)
@@ -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}"
@@ -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
@@ -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())
@@ -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,
@@ -800,6 +901,15 @@ def diff_against(self, old_history, excluded_fields=None, included_fields=None):
                 changes.append(ModelChange(field, old_value, current_value))
                 changed_fields.append(field)
 
+        for field in old_history._history_m2m_fields:
+            old_rows = list(getattr(old_history, field.name).values_list())
+            new_rows = list(getattr(self, field.name).values_list())
+
+            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)
 
 

simple_history/tests/models.py

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