Add excluded_field_kwargs to support custom OneToOneFields.

Coercing OneToOneFields to be ForeignKeys on historical
models causes issues when the OneToOneField is a custom
subclass that includes additional arguments that do not
exist on the ForeignKey.__init__  method.

These additional arguments can be specified via excluded_field_kwargs
to allow these custom OneToOneFields be coerced into ForeignKeys
on the historical model.

Fixes jazzband#807

AUTHORS.rst

@@ -105,6 +105,7 @@ Authors
 - Stefan Borer (`sbor23 <https://github.com/sbor23>`_)
 - Steven Buss (`sbuss <https://github.com/sbuss>`_)
 - Steven Klass
+- Tim Schilling (`tim-schilling <https://github.com/tim-schilling>`_)
 - Tommy Beadle (`tbeadle <https://github.com/tbeadle>`_)
 - Trey Hunner (`treyhunner <https://github.com/treyhunner>`_)
 - Ulysses Vilela

CHANGES.rst

@@ -5,6 +5,8 @@ Unreleased
 ----------
 
  - Fixed bug where serializer of djangorestframework crashed if used with ``OrderingFilter`` (gh-821)
+ - Added ``excluded_field_kwargs`` to support custom ``OneToOneField`` that have
+   additional arguments that don't exist on ``ForeignKey``. (gh-870)
 
 3.0.0 (2021-04-16)
 ------------------

docs/common_issues.rst

@@ -254,3 +254,25 @@ Working with BitBucket Pipelines
 
 When using BitBucket Pipelines to test your Django project with the
 django-simple-history middleware, you will run into an error relating to missing migrations relating to the historic User model from the auth app. This is because the migration file is not held within either your project or django-simple-history.  In order to pypass the error you need to add a ```python manage.py makemigrations auth``` step into your YML file prior to running the tests.
+
+
+Using custom OneToOneFields
+---------------------------
+
+If you are using a custom OneToOneField that has additional arguments and receiving
+the the following ``TypeError``::
+
+..
+    TypeError: __init__() got an unexpected keyword argument
+
+This is because Django Simple History coerces ``OneToOneField`` into ``ForeignKey``
+on the historical model. You can work around this by excluded those additional
+arguments using ``excluded_field_kwargs`` as follows:
+
+.. code-block:: python
+
+    class Poll(models.Model):
+        organizer = CustomOneToOneField(Organizer, ..., custom_argument="some_value")
+        history = HistoricalRecords(
+            excluded_field_kwargs={"organizer": set(["custom_argument"])}
+        )

simple_history/models.py

@@ -79,6 +79,7 @@ def __init__(
         related_name=None,
         use_base_model_db=False,
         user_db_constraint=True,
+        excluded_field_kwargs=None,
     ):
         self.user_set_verbose_name = verbose_name
         self.user_related_name = user_related_name
@@ -101,6 +102,10 @@ def __init__(
         if excluded_fields is None:
             excluded_fields = []
         self.excluded_fields = excluded_fields
+
+        if excluded_field_kwargs is None:
+            excluded_field_kwargs = {}
+        self.excluded_field_kwargs = excluded_field_kwargs
         try:
             if isinstance(bases, str):
                 raise TypeError
@@ -235,6 +240,12 @@ def fields_included(self, model):
                 fields.append(field)
         return fields
 
+    def field_excluded_kwargs(self, field):
+        """
+        Find the excluded kwargs for a given field.
+        """
+        return self.excluded_field_kwargs.get(field.name, set())
+
     def copy_fields(self, model):
         """
         Creates copies of the model's original fields, returning
@@ -262,6 +273,12 @@ def copy_fields(self, model):
                 else:
                     FieldType = type(old_field)
 
+                # Remove any excluded kwargs for the field.
+                # This is useful when a custom OneToOneField is being used that
+                # has a different set of arguments than ForeignKey
+                for exclude_arg in self.field_excluded_kwargs(old_field):
+                    field_args.pop(exclude_arg, None)
+
                 # If field_args['to'] is 'self' then we have a case where the object
                 # has a foreign key to itself. If we pass the historical record's
                 # field to = 'self', the foreign key will point to an historical

simple_history/registry_tests/migration_test_app/migrations/0003_historicalmodelwithcustomattronetoonefield_modelwithcustomattronetoonefield.py

@@ -0,0 +1,91 @@
+# Generated by Django 3.2.6 on 2021-08-24 10:36
+
+import django.db.models.deletion
+from django.conf import settings
+from django.db import migrations, models
+
+import simple_history.models
+import simple_history.registry_tests.migration_test_app.models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        (
+            "migration_test_app",
+            "0002_historicalmodelwithcustomattrforeignkey_modelwithcustomattrforeignkey",
+        ),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name="ModelWithCustomAttrOneToOneField",
+            fields=[
+                (
+                    "id",
+                    models.AutoField(
+                        auto_created=True,
+                        primary_key=True,
+                        serialize=False,
+                        verbose_name="ID",
+                    ),
+                ),
+                (
+                    "what_i_mean",
+                    simple_history.registry_tests.migration_test_app.models.CustomAttrNameOneToOneField(
+                        attr_name="custom_attr_name",
+                        on_delete=django.db.models.deletion.CASCADE,
+                        to="migration_test_app.whatimean",
+                    ),
+                ),
+            ],
+        ),
+        migrations.CreateModel(
+            name="HistoricalModelWithCustomAttrOneToOneField",
+            fields=[
+                (
+                    "id",
+                    models.IntegerField(
+                        auto_created=True, blank=True, db_index=True, verbose_name="ID"
+                    ),
+                ),
+                ("history_id", models.AutoField(primary_key=True, serialize=False)),
+                ("history_date", models.DateTimeField()),
+                ("history_change_reason", models.CharField(max_length=100, null=True)),
+                (
+                    "history_type",
+                    models.CharField(
+                        choices=[("+", "Created"), ("~", "Changed"), ("-", "Deleted")],
+                        max_length=1,
+                    ),
+                ),
+                (
+                    "history_user",
+                    models.ForeignKey(
+                        null=True,
+                        on_delete=django.db.models.deletion.SET_NULL,
+                        related_name="+",
+                        to=settings.AUTH_USER_MODEL,
+                    ),
+                ),
+                (
+                    "what_i_mean",
+                    models.ForeignKey(
+                        blank=True,
+                        db_constraint=False,
+                        null=True,
+                        on_delete=django.db.models.deletion.DO_NOTHING,
+                        related_name="+",
+                        to="migration_test_app.whatimean",
+                    ),
+                ),
+            ],
+            options={
+                "verbose_name": "historical model with custom attr one to one field",
+                "ordering": ("-history_date", "-history_id"),
+                "get_latest_by": "history_date",
+            },
+            bases=(simple_history.models.HistoricalChanges, models.Model),
+        ),
+    ]

simple_history/registry_tests/migration_test_app/models.py

@@ -36,3 +36,29 @@ class ModelWithCustomAttrForeignKey(models.Model):
         WhatIMean, models.CASCADE, attr_name="custom_attr_name"
     )
     history = HistoricalRecords()
+
+
+class CustomAttrNameOneToOneField(models.OneToOneField):
+    def __init__(self, *args, **kwargs):
+        self.attr_name = kwargs.pop("attr_name", None)
+        super(CustomAttrNameOneToOneField, self).__init__(*args, **kwargs)
+
+    def get_attname(self):
+        return self.attr_name or super(CustomAttrNameOneToOneField, self).get_attname()
+
+    def deconstruct(self):
+        name, path, args, kwargs = super(
+            CustomAttrNameOneToOneField, self
+        ).deconstruct()
+        if self.attr_name:
+            kwargs["attr_name"] = self.attr_name
+        return name, path, args, kwargs
+
+
+class ModelWithCustomAttrOneToOneField(models.Model):
+    what_i_mean = CustomAttrNameOneToOneField(
+        WhatIMean, models.CASCADE, attr_name="custom_attr_name"
+    )
+    history = HistoricalRecords(
+        excluded_field_kwargs={"what_i_mean": set(["attr_name"])}
+    )

simple_history/registry_tests/tests.py

@@ -17,6 +17,7 @@
     InheritTracking3,
     InheritTracking4,
     ModelWithCustomAttrForeignKey,
+    ModelWithCustomAttrOneToOneField,
     ModelWithHistoryInDifferentApp,
     Poll,
     Restaurant,
@@ -198,13 +199,21 @@ def test_registering_with_tracked_abstract_base(self):
 
 
 class TestCustomAttrForeignKey(TestCase):
-    """ https://github.com/jazzband/django-simple-history/issues/431 """
+    """https://github.com/jazzband/django-simple-history/issues/431"""
 
     def test_custom_attr(self):
         field = ModelWithCustomAttrForeignKey.history.model._meta.get_field("poll")
         self.assertEqual(field.attr_name, "custom_poll")
 
 
+class TestCustomAttrOneToOneField(TestCase):
+    """https://github.com/jazzband/django-simple-history/issues/870"""
+
+    def test_custom_attr(self):
+        field = ModelWithCustomAttrOneToOneField.history.model._meta.get_field("poll")
+        self.assertFalse(hasattr(field, "attr_name"))
+
+
 @override_settings(MIGRATION_MODULES={})
 class TestMigrate(TestCase):
     def test_makemigration_command(self):
@@ -219,7 +228,7 @@ def test_migrate_command(self):
 
 
 class TestModelWithHistoryInDifferentApp(TestCase):
-    """ https://github.com/jazzband/django-simple-history/issues/485 """
+    """https://github.com/jazzband/django-simple-history/issues/485"""
 
     def test__different_app(self):
         appLabel = ModelWithHistoryInDifferentApp.history.model._meta.app_label

simple_history/tests/models.py

@@ -119,6 +119,28 @@ class ModelWithCustomAttrForeignKey(models.Model):
     history = HistoricalRecords()
 
 
+class CustomAttrNameOneToOneField(models.OneToOneField):
+    def __init__(self, *args, **kwargs):
+        self.attr_name = kwargs.pop("attr_name", None)
+        super(CustomAttrNameOneToOneField, self).__init__(*args, **kwargs)
+
+    def get_attname(self):
+        return self.attr_name or super(CustomAttrNameOneToOneField, self).get_attname()
+
+    def deconstruct(self):
+        name, path, args, kwargs = super(
+            CustomAttrNameOneToOneField, self
+        ).deconstruct()
+        if self.attr_name:
+            kwargs["attr_name"] = self.attr_name
+        return name, path, args, kwargs
+
+
+class ModelWithCustomAttrOneToOneField(models.Model):
+    poll = CustomAttrNameOneToOneField(Poll, models.CASCADE, attr_name="custom_poll")
+    history = HistoricalRecords(excluded_field_kwargs={"poll": set(["attr_name"])})
+
+
 class Temperature(models.Model):
     location = models.CharField(max_length=200)
     temperature = models.IntegerField()

simple_history/tests/tests/test_models.py

@@ -1189,7 +1189,7 @@ def test_migrations_include_order(self):
 
 
 class TestLatest(TestCase):
-    """"Test behavior of `latest()` without any field parameters"""
+    """Test behavior of `latest()` without any field parameters"""
 
     def setUp(self):
         poll = Poll.objects.create(question="Does `latest()` work?", pub_date=yesterday)