Merge pull request #168 from treyhunner/fix-missing-onetoone-in-admin

Fix one-to-one relations in admin and restoring to historical instances

Author: macro1

CHANGES.rst

@@ -4,6 +4,8 @@ Changes
 tip (unreleased)
 ----------------
 - Fix OneToOneField transformation for historical models (gh-166)
+- Disable cascading deletes from related models to historical models
+- Fix restoring historical instances with missing one-to-one relations (gh-162)
 
 1.6.0 (2015-04-16)
 ------------------

simple_history/models.py

@@ -147,6 +147,7 @@ def copy_fields(self, model):
                     db_index=True,
                     serialize=True,
                     unique=False,
+                    on_delete=models.DO_NOTHING,
                     **field_arguments
                 )
                 field.name = old_field.name
@@ -173,7 +174,8 @@ def revert_url(self):
                     [getattr(self, opts.pk.attname), self.history_id])
 
         def get_instance(self):
-            return model(**dict([(k, getattr(self, k)) for k in fields]))
+            return model(**dict([(field.attname, getattr(self, field.attname))
+                                for field in fields.values()]))
 
         return {
             'history_id': models.AutoField(primary_key=True),

simple_history/tests/admin.py

@@ -3,7 +3,7 @@
 from django.contrib import admin
 
 from simple_history.admin import SimpleHistoryAdmin
-from .models import Poll, Choice, Person, Book, Document, Paper
+from .models import Poll, Choice, Person, Book, Document, Paper, Employee
 
 
 class PersonAdmin(SimpleHistoryAdmin):
@@ -17,3 +17,4 @@ def has_change_permission(self, request, obj=None):
 admin.site.register(Book, SimpleHistoryAdmin)
 admin.site.register(Document, SimpleHistoryAdmin)
 admin.site.register(Paper, SimpleHistoryAdmin)
+admin.site.register(Employee, SimpleHistoryAdmin)

simple_history/tests/models.py

@@ -245,3 +245,8 @@ class UserAccessorDefault(models.Model):
 
 class UserAccessorOverride(models.Model):
     pass
+
+
+class Employee(models.Model):
+    manager = models.OneToOneField('Employee', null=True)
+    history = HistoricalRecords()

simple_history/tests/tests/test_admin.py

@@ -12,7 +12,7 @@
 from django.conf import settings
 from simple_history.models import HistoricalRecords
 
-from ..models import Book, Person, Poll, State
+from ..models import Book, Person, Poll, State, Employee
 
 
 today = datetime(2021, 1, 1, 10, 0)
@@ -234,3 +234,14 @@ def test_deleteting_user(self):
 
         historical_poll = poll.history.all()[0]
         self.assertEqual(historical_poll.history_user, None)
+
+    def test_missing_one_to_one(self):
+        """A relation to a missing one-to-one model should still show history"""
+        self.login()
+        manager = Employee.objects.create()
+        employee = Employee.objects.create(manager=manager)
+        employee.manager = None
+        employee.save()
+        manager.delete()
+        response = self.app.get(get_history_url(employee, 0))
+        self.assertEqual(response.status_code, 200)

simple_history/tests/tests/test_models.py

@@ -18,7 +18,7 @@
     AbstractBase, ConcreteAttr, ConcreteUtil, SelfFK, Temperature, WaterLevel,
     ExternalModel1, ExternalModel3, UnicodeVerboseName, HistoricalChoice,
     HistoricalState, HistoricalCustomFKError, Series, SeriesWork, PollInfo,
-    UserAccessorDefault, UserAccessorOverride
+    UserAccessorDefault, UserAccessorOverride, Employee
 )
 from ..external.models import ExternalModel2, ExternalModel4
 
@@ -706,3 +706,26 @@ def test_accessor_default(self):
     def test_accessor_override(self):
         register(UserAccessorOverride, user_related_name='my_history_model_accessor')
         assert hasattr(User, 'my_history_model_accessor')
+
+
+class TestMissingOneToOne(TestCase):
+
+    def setUp(self):
+        self.manager1 = Employee.objects.create()
+        self.manager2 = Employee.objects.create()
+        self.employee = Employee.objects.create(manager=self.manager1)
+        self.employee.manager = self.manager2
+        self.employee.save()
+        self.manager1.delete()
+
+    def test_history_is_complete(self):
+        historical_manager_ids = list(self.employee.history.order_by('pk')
+                                      .values_list('manager_id', flat=True))
+        self.assertEqual(historical_manager_ids, [1, 2])
+
+    def test_restore_employee(self):
+        historical = self.employee.history.order_by('pk')[0]
+        original = historical.instance
+        self.assertEqual(original.manager_id, 1)
+        with self.assertRaises(Employee.DoesNotExist):
+            original.manager