connect signals in tests instead; import sorting; doc tweaks

Author: kseever

docs/advanced.rst

@@ -391,10 +391,9 @@ source model. This is possible by combining the ``bases`` functionality with the
 
     # define your signal handler/callback anywhere outside of models.py
     def add_history_ip_address(sender, **kwargs):
-        thread = threading.local()
         history_instance = kwargs['history_instance']
-        # (thread.request for use only in conjunction with the simple_history middleware)
-        history_instance.ip_address = thread.request.META['REMOTE_ADDR']
+        # thread.request for use only when the simple_history middleware is enabled
+        history_instance.ip_address = HistoricalRecords.thread.request.META['REMOTE_ADDR']
 
 
 .. code-block:: python
@@ -512,7 +511,7 @@ 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. Arguments passed to the signals include the following:
+`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

simple_history/tests/apps.py

@@ -4,8 +4,8 @@
 
 from django.apps import apps
 from django.conf import settings
-from django.dispatch import receiver
 from django.db import models
+from django.dispatch import receiver
 from django.urls import reverse
 
 from simple_history import register

simple_history/tests/models.py

@@ -10,13 +10,14 @@
 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, override_settings
+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
@@ -45,16 +46,17 @@
     HistoricalChoice,
     HistoricalCustomFKError,
     HistoricalPoll,
+    HistoricalPollWithHistoricalIPAddress,
     HistoricalState,
     Library,
     MultiOneToOne,
     Person,
     Place,
     Poll,
     PollInfo,
-    PollWithHistoricalIPAddress,
     PollWithExcludeFields,
     PollWithExcludedFKField,
+    PollWithHistoricalIPAddress,
     Province,
     Restaurant,
     SelfFK,
@@ -1052,14 +1054,32 @@ def test_changed_value_lost(self):
         instance = historical.instance
         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'
+        )
 
-class ExtraFieldsTestCase(TestCase):
     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.order_by('history_date')[0]
+        poll_history = poll.history.first()
 
         self.assertEquals('192.168.0.1', poll_history.ip_address)
 
@@ -1072,13 +1092,31 @@ def test_extra_ip_address_field_not_present_on_poll(self):
             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 ExtraFieldsIPAddressTestCase(TestCase):
+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 = {
-            # workaround for bad interplay with signals and mocking; see
-            # simple_history.tests.tests.utils.add_history_ip_address
-            'question': 'read IP from request',
+            'question': 'Will it blend?',
             'pub_date': '2018-10-30'
         }
 

simple_history/tests/tests/test_models.py

@@ -15,13 +15,3 @@
             settings.MIDDLEWARE_CLASSES + [request_middleware]
         )
     }
-
-
-def add_history_ip_address(sender, **kwargs):
-    history_instance = kwargs['history_instance']
-    if history_instance.question == 'read IP from request':
-        from simple_history.models import HistoricalRecords
-        history_instance.ip_address = \
-            HistoricalRecords.thread.request.META['REMOTE_ADDR']
-    else:
-        history_instance.ip_address = '192.168.0.1'