Send history_instance in the pre_create_historical_record signal

This allows for better customization of historical records without
incurring extra database queries resulting from modifying the instance
after save in `post_create_historical_record`.

Author: kseever

docs/advanced.rst

@@ -76,7 +76,7 @@ folder of the package you register the model in, you could do:
 You can pass attributes of ``HistoricalRecords`` directly to ``register``:
 
 .. code-block:: python
-    
+
     register(User, excluded_fields=['last_login']))
 
 For a complete list of the attributes you can pass to ``register`` we refer
@@ -361,6 +361,55 @@ And you don't want to store the changes for the field ``pub_date``, it is necess
 
 By default, django-simple-history stores the changes for all fields in the model.
 
+Adding additional fields to historical models
+---------------------------------------------
+
+Sometimes it is useful to be able to add additional fields to historical models that do not exist on the
+source model. This is possible by combining the ``bases`` functionality with the ``pre_create_historical_record`` signal.
+
+.. code-block:: python
+
+    # in models.py
+    class IPAddressHistoricalModel(models.Model):
+        """
+        Abstract model for history models tracking the IP address.
+        """
+        ip_address = models.GenericIPAddressField(_('IP address'))
+
+        class Meta:
+            abstract = True
+
+
+    class PollWithExtraFields(models.Model):
+        question = models.CharField(max_length=200)
+        pub_date = models.DateTimeField('date published')
+
+        history = HistoricalRecords(bases=[IPAddressHistoricalModel,]
+
+
+.. code-block:: python
+
+    # define your signal handler/callback anywhere outside of models.py
+    def add_history_ip_address(sender, **kwargs):
+        instance = kwargs['instance']
+        history_instance = kwargs['history_instance']
+        history_instance.ip_address = instance.request.META['REMOTE_ADDR']
+
+
+.. code-block:: python
+
+    # in apps.py
+    class TestsConfig(AppConfig):
+        def ready(self):
+            from simple_history.tests.models \
+                import HistoricalPollWithExtraFields
+
+            pre_create_historical_record.connect(
+                add_history_ip_address,
+                sender=HistoricalPollWithExtraFields
+            )
+
+
 Change Reason
 -------------
 
@@ -462,7 +511,20 @@ This may be useful when you want to construct timelines and need to get only the
 
 Using signals
 ------------------------------------
-django-simple-history includes signals that helps you provide custom behaviour when saving a historical record. If you want to connect the signals you can do so using the following code:
+django-simple-history includes signals that helps you provide custom behaviour when saving a historical record. Arguments passed to the signals include the following:
+
+instance
+    The source model instance being saved
+history_instance
+    The corresponding history record
+history_date
+    Datetime of the history record's creation
+history_change_reason
+    Freetext description of the reason for the change
+history_user
+    The user that instigated the change
+
+To connect the signals to your callbacks, you can use the @receiver decorator:
 
 .. code-block:: python
 
@@ -473,9 +535,9 @@ django-simple-history includes signals that helps you provide custom behaviour w
     )
 
     @receiver(pre_create_historical_record)
-    def pre_create_historical_record(sender, instance, **kwargs):
+    def pre_create_historical_record_callback(sender, **kwargs):
         print("Sent before saving historical record")
 
-    @receiver(pre_create_historical_record)
-        def post_create_historical_record(sender, instance, history_instance, **kwargs):
-            print("Sent after saving historical record")
+    @receiver(post_create_historical_record)
+    def post_create_historical_record_callback(sender, **kwargs):
+        print("Sent after saving historical record")

simple_history/models.py

@@ -356,21 +356,26 @@ def create_historical_record(self, instance, history_type):
         history_change_reason = getattr(instance, 'changeReason', None)
         manager = getattr(instance, self.manager_name)
 
-        pre_create_historical_record.send(
-            sender=manager.model, instance=instance,
-            history_date=history_date, history_user=history_user,
-            history_change_reason=history_change_reason,
-        )
-
         attrs = {}
         for field in self.fields_included(instance):
             attrs[field.attname] = getattr(instance, field.attname)
-        history_instance = manager.create(
+
+        history_instance = manager.model(
             history_date=history_date, history_type=history_type,
             history_user=history_user,
-            history_change_reason=history_change_reason, **attrs
+            history_change_reason=history_change_reason,
+            **attrs
         )
 
+        pre_create_historical_record.send(
+            sender=manager.model, instance=instance,
+            history_date=history_date, history_user=history_user,
+            history_change_reason=history_change_reason,
+            history_instance=history_instance,
+        )
+
+        history_instance.save()
+
         post_create_historical_record.send(
             sender=manager.model, instance=instance,
             history_instance=history_instance,

simple_history/tests/__init__.py

@@ -0,0 +1 @@
+default_app_config = 'simple_history.tests.apps.TestsConfig'

simple_history/tests/apps.py

@@ -0,0 +1,21 @@
+from django.apps import AppConfig
+from simple_history.signals import pre_create_historical_record
+
+
+def add_history_ip_address(sender, **kwargs):
+    history_instance = kwargs['history_instance']
+    history_instance.ip_address = '127.0.0.1'
+
+
+class TestsConfig(AppConfig):
+    name = 'simple_history.tests'
+    verbose_name = 'Tests'
+
+    def ready(self):
+        from simple_history.tests.models \
+            import HistoricalPollWithHistoricalIPAddress
+
+        pre_create_historical_record.connect(
+            add_history_ip_address,
+            sender=HistoricalPollWithHistoricalIPAddress
+        )

simple_history/tests/models.py

@@ -4,11 +4,13 @@
 
 from django.apps import apps
 from django.conf import settings
+from django.dispatch import receiver
 from django.db import models
 from django.urls import reverse
 
 from simple_history import register
 from simple_history.models import HistoricalRecords
+from simple_history.signals import pre_create_historical_record
 from .custom_user.models import CustomUser as User
 from .external.models.model1 import AbstractExternal
 
@@ -40,6 +42,21 @@ class PollWithExcludedFKField(models.Model):
     history = HistoricalRecords(excluded_fields=['place'])
 
 
+class IPAddressHistoricalModel(models.Model):
+    ip_address = models.GenericIPAddressField()
+
+    class Meta:
+        abstract = True
+
+
+class PollWithHistoricalIPAddress(models.Model):
+    question = models.CharField(max_length=200)
+    pub_date = models.DateTimeField('date published')
+    place = models.ForeignKey('Place', on_delete=models.CASCADE)
+
+    history = HistoricalRecords(bases=[IPAddressHistoricalModel])
+
+
 class Temperature(models.Model):
     location = models.CharField(max_length=200)
     temperature = models.IntegerField()

simple_history/tests/tests/test_models.py

@@ -50,6 +50,7 @@
     Place,
     Poll,
     PollInfo,
+    PollWithHistoricalIPAddress,
     PollWithExcludeFields,
     PollWithExcludedFKField,
     Province,
@@ -1048,3 +1049,24 @@ def test_changed_value_lost(self):
         historical = self.get_first_historical()
         instance = historical.instance
         self.assertEqual(instance.place, new_place)
+
+
+class ExtraFieldsTestCase(TestCase):
+    def test_extra_ip_address_field_populated_on_save(self):
+        poll = PollWithHistoricalIPAddress.objects.create(
+            question="Will it blend?", pub_date=today,
+            place=Place.objects.create(name="Here")
+        )
+
+        poll_history = poll.history.order_by('history_date')[0]
+
+        self.assertEquals('127.0.0.1', poll_history.ip_address)
+
+    def test_extra_ip_address_field_not_present_on_poll(self):
+        poll = PollWithHistoricalIPAddress.objects.create(
+            question="Will it blend?", pub_date=today,
+            place=Place.objects.create(name="Here")
+        )
+
+        with self.assertRaises(AttributeError):
+            poll.ip_address

simple_history/tests/tests/test_signals.py

@@ -25,6 +25,7 @@ def test_pre_create_historical_record_signal(self):
         def handler(sender, instance, **kwargs):
             self.signal_was_called = True
             self.signal_instance = instance
+            self.signal_history_instance = kwargs['history_instance']
             self.signal_sender = sender
         pre_create_historical_record.connect(handler)
 
@@ -33,6 +34,7 @@ def handler(sender, instance, **kwargs):
 
         self.assertTrue(self.signal_was_called)
         self.assertEqual(self.signal_instance, p)
+        self.assertIsNotNone(self.signal_history_instance)
         self.assertEqual(self.signal_sender, p.history.first().__class__)
 
     def test_post_create_historical_record_signal(self):
@@ -48,5 +50,5 @@ def handler(sender, instance, history_instance, **kwargs):
 
         self.assertTrue(self.signal_was_called)
         self.assertEqual(self.signal_instance, p)
-        self.assertTrue(self.signal_history_instance is not None)
+        self.assertIsNotNone(self.signal_history_instance)
         self.assertEqual(self.signal_sender, p.history.first().__class__)