diff --git a/AUTHORS.rst b/AUTHORS.rst index e79cd1cc3..767c79902 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -60,7 +60,8 @@ Authors - Nathan Villagaray-Carski - Mike Spainhower - Alexander Anikeev -- Kyle Seever +- Adnan Umer (@uadnan) +- Jonathan Zvesper (@zvesp) Background ========== diff --git a/CHANGES.rst b/CHANGES.rst index 74762bc22..c6454ecb0 100644 --- a/CHANGES.rst +++ b/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) ---------------- diff --git a/simple_history/management/commands/_populate_utils.py b/simple_history/management/commands/_populate_utils.py index 7cd1b4b05..b8ec4334d 100644 --- a/simple_history/management/commands/_populate_utils.py +++ b/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, diff --git a/simple_history/models.py b/simple_history/models.py index 27df2e948..679481733 100644 --- a/simple_history/models.py +++ b/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): """ diff --git a/simple_history/tests/models.py b/simple_history/tests/models.py index 5050a9082..39df3301a 100644 --- a/simple_history/tests/models.py +++ b/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() diff --git a/simple_history/tests/tests/test_commands.py b/simple_history/tests/tests/test_commands.py index 0047b8a79..514b76d4c 100644 --- a/simple_history/tests/tests/test_commands.py +++ b/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) diff --git a/simple_history/tests/tests/test_models.py b/simple_history/tests/tests/test_models.py index e99057299..020d6f3a6 100644 --- a/simple_history/tests/tests/test_models.py +++ b/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)