From e9d57e60fe23f84c0fc5203d7dd5a0496cb2de15 Mon Sep 17 00:00:00 2001 From: romgar Date: Fri, 18 Nov 2016 00:24:57 +0000 Subject: [PATCH 1/5] Add test to demonstrate issue #241 --- model_utils/tests/models.py | 6 ++++++ model_utils/tests/tests.py | 15 ++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/model_utils/tests/models.py b/model_utils/tests/models.py index eee735b..7876828 100644 --- a/model_utils/tests/models.py +++ b/model_utils/tests/models.py @@ -105,6 +105,12 @@ class MonitorWhenEmpty(models.Model): name_changed = MonitorField(monitor="name", when=[]) +class DoubleMonitored(models.Model): + name = models.CharField(max_length=25) + name_changed = MonitorField(monitor="name") + name2 = models.CharField(max_length=25) + name_changed2 = MonitorField(monitor="name2") + class Status(StatusModel): STATUS = Choices( diff --git a/model_utils/tests/tests.py b/model_utils/tests/tests.py index 17fd67e..99869cc 100644 --- a/model_utils/tests/tests.py +++ b/model_utils/tests/tests.py @@ -31,7 +31,7 @@ from model_utils.tests.models import ( Tracked, TrackedFK, InheritedTrackedFK, TrackedNotDefault, TrackedNonFieldAttr, TrackedMultiple, InheritedTracked, TrackedFileField, StatusFieldDefaultFilled, StatusFieldDefaultNotFilled, InheritanceManagerTestChild3, StatusFieldChoicesName, - SoftDeletable) + SoftDeletable, DoubleMonitored) class MigrationsTests(TestCase): @@ -248,6 +248,19 @@ class MonitorWhenEmptyFieldTests(TestCase): self.assertEqual(self.instance.name_changed, self.created) +class MonitorDoubleFieldTests(TestCase): + + def setUp(self): + DoubleMonitored.objects.create(name='Charlie', name2='Charlie2') + + def test_recursion_error_with_only(self): + # Any field passed to only() is generating a recursion error + list(DoubleMonitored.objects.only('id')) + + def test_recursion_error_with_defer(self): + # Only monitored fields passed to defer() are failing + list(DoubleMonitored.objects.defer('name')) + class StatusFieldTests(TestCase): From 72158f182004df5113af18b8494ff9fbcacab937 Mon Sep 17 00:00:00 2001 From: romgar Date: Fri, 18 Nov 2016 23:11:23 +0000 Subject: [PATCH 2/5] Avoid to directly initialise a monitored field if defered to avoid recursion issue --- model_utils/fields.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/model_utils/fields.py b/model_utils/fields.py index 805c707..0a3f9cf 100644 --- a/model_utils/fields.py +++ b/model_utils/fields.py @@ -110,8 +110,11 @@ class MonitorField(models.DateTimeField): return getattr(instance, self.monitor) def _save_initial(self, sender, instance, **kwargs): + if self.monitor in instance.get_deferred_fields(): + # Fix related to issue #241 to avoid recursive error on double monitor fields + return setattr(instance, self.monitor_attname, - self.get_monitored_value(instance)) + self.get_monitored_value(instance)) def pre_save(self, model_instance, add): value = now() From 2455c983fc913f58eba7f91f7550ded326a2ecee Mon Sep 17 00:00:00 2001 From: romgar Date: Fri, 18 Nov 2016 23:12:39 +0000 Subject: [PATCH 3/5] Restore initial indentation --- model_utils/fields.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model_utils/fields.py b/model_utils/fields.py index 0a3f9cf..f54f0ee 100644 --- a/model_utils/fields.py +++ b/model_utils/fields.py @@ -114,7 +114,7 @@ class MonitorField(models.DateTimeField): # Fix related to issue #241 to avoid recursive error on double monitor fields return setattr(instance, self.monitor_attname, - self.get_monitored_value(instance)) + self.get_monitored_value(instance)) def pre_save(self, model_instance, add): value = now() From 93dd940a5ddefd6a8d811e4fde98c6d12f394026 Mon Sep 17 00:00:00 2001 From: romgar Date: Fri, 18 Nov 2016 23:31:45 +0000 Subject: [PATCH 4/5] Remove defered fields in _save_initial only for Django 1.10+ --- model_utils/fields.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model_utils/fields.py b/model_utils/fields.py index f54f0ee..fe9d4a8 100644 --- a/model_utils/fields.py +++ b/model_utils/fields.py @@ -110,7 +110,7 @@ class MonitorField(models.DateTimeField): return getattr(instance, self.monitor) def _save_initial(self, sender, instance, **kwargs): - if self.monitor in instance.get_deferred_fields(): + if django.VERSION >= (1, 10) and self.monitor in instance.get_deferred_fields(): # Fix related to issue #241 to avoid recursive error on double monitor fields return setattr(instance, self.monitor_attname, From e2440a6872dc195d2e7446bf9f68832477bbd68f Mon Sep 17 00:00:00 2001 From: romgar Date: Sat, 19 Nov 2016 12:08:48 +0000 Subject: [PATCH 5/5] Add tests to prevent regression in MonitorField behaviour if we filter out deferred fields in _save_initial --- model_utils/tests/tests.py | 9 +++++++++ tox.ini | 1 + 2 files changed, 10 insertions(+) diff --git a/model_utils/tests/tests.py b/model_utils/tests/tests.py index 99869cc..3d3f2f7 100644 --- a/model_utils/tests/tests.py +++ b/model_utils/tests/tests.py @@ -2,6 +2,8 @@ from __future__ import unicode_literals from datetime import datetime, timedelta +from freezegun import freeze_time + try: from unittest import skipUnless except ImportError: # Python 2.6 @@ -261,6 +263,13 @@ class MonitorDoubleFieldTests(TestCase): # Only monitored fields passed to defer() are failing list(DoubleMonitored.objects.defer('name')) + def test_monitor_still_works_with_deferred_fields_filtered_out_of_save_initial(self): + obj = DoubleMonitored.objects.defer('name').get(name='Charlie') + with freeze_time("2016-12-01"): + obj.name = 'Charlie2' + obj.save() + self.assertEqual(obj.name_changed, datetime(2016, 12, 1)) + class StatusFieldTests(TestCase): diff --git a/tox.ini b/tox.ini index b096f7a..6ce334e 100644 --- a/tox.ini +++ b/tox.ini @@ -25,5 +25,6 @@ deps = django110: Django>=1.10,<1.11 django_trunk: https://github.com/django/django/tarball/master django{14,15,16}: South==1.0.2 + freezegun == 0.3.8 commands = coverage run -a setup.py test