From 7d3066b76b75a6d1a61575bf1150cd102cf3b11f Mon Sep 17 00:00:00 2001 From: John Whitlock Date: Sun, 16 Nov 2014 21:32:36 -0600 Subject: [PATCH] Fix #140 - Convert OrderWrt to IntegerField An OrderWrt field named _order is added when the Meta.order_with_respect_to option is used on the model. This code: - Switches it to an IntegerField in the historical model - Restores the old value when loading from a history_object, which may result in non-deterministic ordering until set_RELATED_order() is called again. --- simple_history/models.py | 4 + simple_history/tests/models.py | 15 +++ simple_history/tests/tests/test_models.py | 122 +++++++++++++++++++++- 3 files changed, 140 insertions(+), 1 deletion(-) diff --git a/simple_history/models.py b/simple_history/models.py index a3120d99d..e1dd5f108 100644 --- a/simple_history/models.py +++ b/simple_history/models.py @@ -8,6 +8,7 @@ apps = None from django.db import models, router from django.db.models import loading +from django.db.models.fields.proxy import OrderWrt from django.db.models.fields.related import RelatedField from django.db.models.related import RelatedObject from django.conf import settings @@ -135,6 +136,9 @@ def copy_fields(self, model): field.rel.related_name = '+' field.null = True field.blank = True + if isinstance(field, OrderWrt): + # OrderWrt is a proxy field, switch to a plain IntegerField + field.__class__ = models.IntegerField transform_field(field) fields[field.name] = field return fields diff --git a/simple_history/tests/models.py b/simple_history/tests/models.py index a720222ac..b0bb65bc8 100644 --- a/simple_history/tests/models.py +++ b/simple_history/tests/models.py @@ -217,3 +217,18 @@ class Meta: class CustomFKError(models.Model): fk = models.ForeignKey(SecondLevelInheritedModel) history = HistoricalRecords() + + +class Series(models.Model): + """A series of works, like a trilogy of books.""" + name = models.CharField(max_length=100) + author = models.CharField(max_length=100) + + +class SeriesWork(models.Model): + series = models.ForeignKey('Series', related_name='works') + title = models.CharField(max_length=100) + history = HistoricalRecords() + + class Meta: + order_with_respect_to = 'series' diff --git a/simple_history/tests/tests/test_models.py b/simple_history/tests/tests/test_models.py index ccb8caf5e..a1e51bb89 100644 --- a/simple_history/tests/tests/test_models.py +++ b/simple_history/tests/tests/test_models.py @@ -14,6 +14,7 @@ from django.contrib.auth.models import User from django.db import models from django.db.models.loading import get_model +from django.db.models.fields.proxy import OrderWrt from django.test import TestCase from django.core.files.base import ContentFile @@ -24,7 +25,7 @@ Person, FileModel, Document, Book, HistoricalPoll, Library, State, AbstractBase, ConcreteAttr, ConcreteUtil, SelfFK, Temperature, WaterLevel, ExternalModel1, ExternalModel3, UnicodeVerboseName, HistoricalChoice, - HistoricalState, HistoricalCustomFKError + HistoricalState, HistoricalCustomFKError, Series, SeriesWork ) from ..external.models import ExternalModel2, ExternalModel4 @@ -519,3 +520,122 @@ def test_non_relational(self): with self.settings(DATABASES={'default': { 'ENGINE': 'django_mongodb_engine'}}): assert convert_auto_field(self.field) == models.TextField + + +class TestOrderWrtField(TestCase): + """Check behaviour of _order field added by Meta.order_with_respect_to. + + The Meta.order_with_respect_to option adds an OrderWrt field named + "_order", where OrderWrt is a proxy class for an IntegerField that sets + some default options. + + The simple_history strategy is: + - Convert to a plain IntegerField in the historical record + - When restoring a historical instance, add the old value. This may + result in duplicate ordering values and non-deterministic ordering. + """ + + def setUp(self): + """Create works in published order.""" + s = self.series = Series.objects.create( + name="The Chronicles of Narnia", author="C.S. Lewis") + self.w_lion = s.works.create( + title="The Lion, the Witch and the Wardrobe") + self.w_caspian = s.works.create(title="Prince Caspian") + self.w_voyage = s.works.create(title="The Voyage of the Dawn Treader") + self.w_chair = s.works.create(title="The Silver Chair") + self.w_horse = s.works.create(title="The Horse and His Boy") + self.w_nephew = s.works.create(title="The Magician's Nephew") + self.w_battle = s.works.create(title="The Last Battle") + + def test_order(self): + """Confirm that works are ordered by creation.""" + order = self.series.get_serieswork_order() + expected = [ + self.w_lion.pk, + self.w_caspian.pk, + self.w_voyage.pk, + self.w_chair.pk, + self.w_horse.pk, + self.w_nephew.pk, + self.w_battle.pk] + self.assertEqual(order, expected) + self.assertEqual(0, self.w_lion._order) + self.assertEqual(1, self.w_caspian._order) + self.assertEqual(2, self.w_voyage._order) + self.assertEqual(3, self.w_chair._order) + self.assertEqual(4, self.w_horse._order) + self.assertEqual(5, self.w_nephew._order) + self.assertEqual(6, self.w_battle._order) + + def test_order_field_in_historical_model(self): + work_order_field = self.w_lion._meta.get_field_by_name('_order')[0] + self.assertEqual(type(work_order_field), OrderWrt) + + history = self.w_lion.history.all()[0] + history_order_field = history._meta.get_field_by_name('_order')[0] + self.assertEqual(type(history_order_field), models.IntegerField) + + def test_history_object_has_order(self): + history = self.w_lion.history.all()[0] + self.assertEqual(self.w_lion._order, history.history_object._order) + + def test_restore_object_with_changed_order(self): + # Change a title + self.w_caspian.title = 'Prince Caspian: The Return to Narnia' + self.w_caspian.save() + self.assertEqual(2, len(self.w_caspian.history.all())) + self.assertEqual(1, self.w_caspian._order) + + # Switch to internal chronological order + chronological = [ + self.w_nephew.pk, + self.w_lion.pk, + self.w_horse.pk, + self.w_caspian.pk, + self.w_voyage.pk, + self.w_chair.pk, + self.w_battle.pk] + self.series.set_serieswork_order(chronological) + self.assertEqual(self.series.get_serieswork_order(), chronological) + + # This uses an update, not a save, so no new history is created + w_caspian = SeriesWork.objects.get(id=self.w_caspian.id) + self.assertEqual(2, len(w_caspian.history.all())) + self.assertEqual(1, w_caspian.history.all()[0]._order) + self.assertEqual(1, w_caspian.history.all()[1]._order) + self.assertEqual(3, w_caspian._order) + + # Revert to first title, old order + old = w_caspian.history.all()[1].history_object + old.save() + w_caspian = SeriesWork.objects.get(id=self.w_caspian.id) + self.assertEqual(3, len(w_caspian.history.all())) + self.assertEqual(1, w_caspian.history.all()[0]._order) + self.assertEqual(1, w_caspian.history.all()[1]._order) + self.assertEqual(1, w_caspian.history.all()[2]._order) + self.assertEqual(1, w_caspian._order) # The order changed + w_lion = SeriesWork.objects.get(id=self.w_lion.id) + self.assertEqual(1, w_lion._order) # and is identical to another order + + # New order is non-deterministic around identical IDs + series = Series.objects.get(id=self.series.id) + order = series.get_serieswork_order() + self.assertEqual(order[0], self.w_nephew.pk) + self.assertTrue(order[1] in (self.w_lion.pk, self.w_caspian.pk)) + self.assertTrue(order[2] in (self.w_lion.pk, self.w_caspian.pk)) + self.assertEqual(order[3], self.w_horse.pk) + self.assertEqual(order[4], self.w_voyage.pk) + self.assertEqual(order[5], self.w_chair.pk) + self.assertEqual(order[6], self.w_battle.pk) + + @skipUnless(django.get_version() >= "1.7", "Requires 1.7 migrations") + def test_migrations_include_order(self): + from django.db.migrations import state + model_state = state.ModelState.from_model(SeriesWork.history.model) + found = False + for name, field in model_state.fields: + if name == '_order': + found = True + self.assertEqual(type(field), models.IntegerField) + assert found, '_order not in fields ' + repr(model_state.fields)