Merge pull request #471 from kseever/422-add-history-extra-fields

Send history_instance in the `pre_create_historical_record` signal

Author: kseever

CHANGES.rst

@@ -5,6 +5,7 @@ Unreleased
 ----------
 - Add `custom_model_name` parameter to the constructor of `HistoricalRecords` (gh-451)
 - Fix header on history pages when custom site_header is used (gh-448)
+- Modify `pre_create_historircal_record` to pass `history_instance` for ease of customization (gh-421)
 - Raise warning if HistoricalRecords(inherit=False) is in an abstract model (gh-341)
 
 2.5.1 (2018-10-19)

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):
+        history_instance = kwargs['history_instance']
+        # thread.request for use only when the simple_history middleware is on and enabled
+        history_instance.ip_address = HistoricalRecords.thread.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 help you provide custom behavior 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

@@ -362,21 +362,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/signals.py

@@ -2,7 +2,8 @@
 
 
 pre_create_historical_record = django.dispatch.Signal(providing_args=[
-    'instance', 'history_date', 'history_user', 'history_change_reason',
+    'instance', 'history_instance', 'history_date', 'history_user',
+    'history_change_reason',
 ])
 post_create_historical_record = django.dispatch.Signal(providing_args=[
     'instance', 'history_instance', 'history_date', 'history_user',

simple_history/tests/models.py

@@ -5,10 +5,12 @@
 from django.apps import apps
 from django.conf import settings
 from django.db import models
+from django.dispatch import receiver
 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,23 @@ 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')
+
+    history = HistoricalRecords(bases=[IPAddressHistoricalModel])
+
+    def get_absolute_url(self):
+        return reverse('poll-detail', kwargs={'pk': self.pk})
+
+
 class Temperature(models.Model):
     location = models.CharField(max_length=200)
     temperature = models.IntegerField()

simple_history/tests/tests/test_models.py

@@ -10,12 +10,15 @@
 from django.core.files.base import ContentFile
 from django.db import models
 from django.db.models.fields.proxy import OrderWrt
-from django.test import TestCase
+from django.test import override_settings, TestCase
+from django.urls import reverse
 
 from simple_history.models import (
     HistoricalRecords,
     ModelChange
 )
+from simple_history.signals import pre_create_historical_record
+from simple_history.tests.tests.utils import middleware_override_settings
 from simple_history.utils import update_change_reason
 from ..external.models import ExternalModel2, ExternalModel4
 from ..models import (
@@ -43,6 +46,7 @@
     HistoricalChoice,
     HistoricalCustomFKError,
     HistoricalPoll,
+    HistoricalPollWithHistoricalIPAddress,
     HistoricalState,
     Library,
     MultiOneToOne,
@@ -52,6 +56,7 @@
     PollInfo,
     PollWithExcludeFields,
     PollWithExcludedFKField,
+    PollWithHistoricalIPAddress,
     Province,
     Restaurant,
     SelfFK,
@@ -1050,6 +1055,81 @@ def test_changed_value_lost(self):
         self.assertEqual(instance.place, new_place)
 
 
+def add_static_history_ip_address(sender, **kwargs):
+    history_instance = kwargs['history_instance']
+    history_instance.ip_address = '192.168.0.1'
+
+
+class ExtraFieldsStaticIPAddressTestCase(TestCase):
+    def setUp(self):
+        pre_create_historical_record.connect(
+            add_static_history_ip_address,
+            sender=HistoricalPollWithHistoricalIPAddress,
+            dispatch_uid='add_static_history_ip_address'
+        )
+
+    def tearDown(self):
+        pre_create_historical_record.disconnect(
+            add_static_history_ip_address,
+            sender=HistoricalPollWithHistoricalIPAddress,
+            dispatch_uid='add_static_history_ip_address'
+        )
+
+    def test_extra_ip_address_field_populated_on_save(self):
+        poll = PollWithHistoricalIPAddress.objects.create(
+            question="Will it blend?", pub_date=today
+        )
+
+        poll_history = poll.history.first()
+
+        self.assertEquals('192.168.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
+        )
+
+        with self.assertRaises(AttributeError):
+            poll.ip_address
+
+
+def add_dynamic_history_ip_address(sender, **kwargs):
+    history_instance = kwargs['history_instance']
+    history_instance.ip_address = \
+        HistoricalRecords.thread.request.META['REMOTE_ADDR']
+
+
+@override_settings(**middleware_override_settings)
+class ExtraFieldsDynamicIPAddressTestCase(TestCase):
+    def setUp(self):
+        pre_create_historical_record.connect(
+            add_dynamic_history_ip_address,
+            sender=HistoricalPollWithHistoricalIPAddress,
+            dispatch_uid='add_dynamic_history_ip_address'
+        )
+
+    def tearDown(self):
+        pre_create_historical_record.disconnect(
+            add_dynamic_history_ip_address,
+            sender=HistoricalPollWithHistoricalIPAddress,
+            dispatch_uid='add_dynamic_history_ip_address'
+        )
+
+    def test_signal_is_able_to_retrieve_request_from_thread(self):
+        data = {
+            'question': 'Will it blend?',
+            'pub_date': '2018-10-30'
+        }
+
+        self.client.post(reverse('pollip-add'), data=data)
+
+        polls = PollWithHistoricalIPAddress.objects.all()
+        self.assertEqual(1, polls.count())
+
+        poll_history = polls[0].history.first()
+        self.assertEqual('127.0.0.1', poll_history.ip_address)
+
+
 class WarningOnAbstractModelWithInheritFalseTest(TestCase):
     def test_warning_on_abstract_model_with_inherit_false(self):
 

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

simple_history/tests/urls.py

@@ -10,7 +10,8 @@
     PollDelete,
     PollDetail,
     PollList,
-    PollUpdate
+    PollUpdate,
+    PollWithHistoricalIPAddressCreate,
 )
 from . import other_admin
 
@@ -26,6 +27,11 @@
         BucketDataRegisterRequestUserDetail.as_view(),
         name='bucket_data-detail'),
     url(r'^poll/add/$', PollCreate.as_view(), name='poll-add'),
+    url(
+        r'^pollwithhistoricalipaddress/add$',
+        PollWithHistoricalIPAddressCreate.as_view(),
+        name='pollip-add'
+    ),
     url(r'^poll/(?P<pk>[0-9]+)/$', PollUpdate.as_view(), name='poll-update'),
     url(r'^poll/(?P<pk>[0-9]+)/delete/$', PollDelete.as_view(),
         name='poll-delete'),

simple_history/tests/view.py

@@ -7,14 +7,23 @@
     UpdateView
 )
 
-from simple_history.tests.models import BucketDataRegisterRequestUser, Poll
+from simple_history.tests.models import (
+    BucketDataRegisterRequestUser,
+    Poll,
+    PollWithHistoricalIPAddress
+)
 
 
 class PollCreate(CreateView):
     model = Poll
     fields = ['question', 'pub_date']
 
 
+class PollWithHistoricalIPAddressCreate(CreateView):
+    model = PollWithHistoricalIPAddress
+    fields = ['question', 'pub_date']
+
+
 class PollUpdate(UpdateView):
     model = Poll
     fields = ['question', 'pub_date']