Don't populate excluded fields while populating existing models history

AUTHORS.rst

@@ -60,7 +60,8 @@ Authors
 - Nathan Villagaray-Carski
 - Mike Spainhower
 - Alexander Anikeev
-- Kyle Seever
+- Adnan Umer (@uadnan)
+- Jonathan Zvesper (@zvesp)
 
 Background
 ==========

CHANGES.rst

@@ -1,13 +1,16 @@
 Changes
 =======
 
+Unreleased
+----------
+- Fix TypeError on populate_history if excluded_fields are specified
+
 2.1.0 (2018-06-04)
 ------------------
 - Add ability to specify custom history_reason field (gh-379)
 - Add ability to specify custom history_id field (gh-368)
 - Add HistoricalRecord instance properties `prev_record` and `next_record` (gh-365)
 - Can set admin methods as attributes on object history change list template (gh-390)
-- Fixed compatibility of >= 2.0 versions with old-style middleware (gh-369)
 
 2.0 (2018-04-05)
 ----------------

simple_history/management/commands/_populate_utils.py

@@ -24,6 +24,7 @@ def bulk_history_create(model, history_model, batch_size):
             **{
                 field.attname: getattr(instance, field.attname)
                 for field in instance._meta.fields
+                if field.name not in history_model._history_excluded_fields
             }
         ) for instance in model.objects.all()]
     history_model.objects.bulk_create(historical_instances,

simple_history/models.py

@@ -113,7 +113,10 @@ def create_history_model(self, model, inherited):
         """
         Creates a historical model to associate with the model provided.
         """
-        attrs = {'__module__': self.module}
+        attrs = {
+            '__module__': self.module,
+            '_history_excluded_fields': self.excluded_fields
+        }
 
         app_module = '%s.models' % model._meta.app_label
 
@@ -220,10 +223,20 @@ def revert_url(self):
             )
 
         def get_instance(self):
-            return model(**{
+            attrs = {
                 field.attname: getattr(self, field.attname)
                 for field in fields.values()
-            })
+            }
+            if self._history_excluded_fields:
+                excluded_attnames = [
+                    model._meta.get_field(field).attname
+                    for field in self._history_excluded_fields
+                ]
+                values = model.objects.filter(
+                    pk=getattr(self, model._meta.pk.attname)
+                ).values(*excluded_attnames).get()
+                attrs.update(values)
+            return model(**attrs)
 
         def get_next_record(self):
             """

simple_history/tests/models.py

@@ -32,6 +32,14 @@ class PollWithExcludeFields(models.Model):
     history = HistoricalRecords(excluded_fields=['pub_date'])
 
 
+class PollWithExcludedFKField(models.Model):
+    question = models.CharField(max_length=200)
+    pub_date = models.DateTimeField('date published')
+    place = models.ForeignKey('Place', on_delete=models.CASCADE)
+
+    history = HistoricalRecords(excluded_fields=['place'])
+
+
 class Temperature(models.Model):
     location = models.CharField(max_length=200)
     temperature = models.IntegerField()

simple_history/tests/tests/test_commands.py

@@ -111,3 +111,12 @@ def test_no_historical(self):
                                     stdout=out)
         self.assertIn(populate_history.Command.NO_REGISTERED_MODELS,
                       out.getvalue())
+
+    def test_excluded_fields(self):
+        poll = models.PollWithExcludeFields.objects.create(
+            question="Will this work?", pub_date=datetime.now())
+        models.PollWithExcludeFields.history.all().delete()
+        management.call_command(self.command_name,
+                                'tests.pollwithexcludefields', auto=True)
+        initial_history_record = models.PollWithExcludeFields.history.all()[0]
+        self.assertEqual(initial_history_record.question, poll.question)

simple_history/tests/tests/test_models.py

@@ -40,9 +40,11 @@
     Library,
     MultiOneToOne,
     Person,
+    Place,
     Poll,
     PollInfo,
     PollWithExcludeFields,
+    PollWithExcludedFKField,
     Province,
     Restaurant,
     SelfFK,
@@ -946,3 +948,51 @@ def test_custom_table_name_from_register(self):
             self.get_table_name(ContactRegister.history),
             'contacts_register_history',
         )
+
+
+class ExcludeFieldsTest(TestCase):
+    def test_restore_pollwithexclude(self):
+        poll = PollWithExcludeFields.objects.create(question="what's up?",
+                                                    pub_date=today)
+        historical = poll.history.order_by('pk')[0]
+        with self.assertRaises(AttributeError):
+            historical.pub_date
+        original = historical.instance
+        self.assertEqual(original.pub_date, poll.pub_date)
+
+
+class ExcludeForeignKeyTest(TestCase):
+    def setUp(self):
+        self.poll = PollWithExcludedFKField.objects.create(
+            question="Is it?", pub_date=today,
+            place=Place.objects.create(name="Somewhere")
+        )
+
+    def get_first_historical(self):
+        """
+        Retrieve the idx'th HistoricalPoll, ordered by time.
+        """
+        return self.poll.history.order_by('history_date')[0]
+
+    def test_instance_fk_value(self):
+        historical = self.get_first_historical()
+        original = historical.instance
+        self.assertEqual(original.place, self.poll.place)
+
+    def test_history_lacks_fk(self):
+        historical = self.get_first_historical()
+        with self.assertRaises(AttributeError):
+            historical.place
+
+    def test_nb_queries(self):
+        with self.assertNumQueries(2):
+            historical = self.get_first_historical()
+            historical.instance
+
+    def test_changed_value_lost(self):
+        new_place = Place.objects.create(name="More precise")
+        self.poll.place = new_place
+        self.poll.save()
+        historical = self.get_first_historical()
+        instance = historical.instance
+        self.assertEqual(instance.place, new_place)