Skip to content

Commit

Permalink
Fix FieldTracker's has_changed
Browse files Browse the repository at this point in the history 
The FieldTracker has_changed method no longer returns True for any input
when the instance is unsaved and no longer raises a FieldError for
fields after the first save.  The original ModelTracker behavior is
maintained.
  • Loading branch information
@treyhunner
treyhunner committed May 25, 2013
1 parent d28f386 commit 66f8358
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 22 deletions.
100 changes: 81 additions & 19 deletions model_utils/tests/tests.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from django.core.exceptions import ImproperlyConfigured, FieldError
from django.test import TestCase

from model_utils import Choices, FieldTracker, ModelTracker
from model_utils import Choices, FieldTracker
from model_utils.fields import get_excerpt, MonitorField, StatusField
from model_utils.managers import QueryManager
from model_utils.models import StatusModel, TimeFramedModel
Expand Down Expand Up @@ -686,11 +686,6 @@ def update_instance(self, **kwargs):

class FieldTrackerCommonTests(object):

def test_pre_save_has_changed(self):
self.assertHasChanged(name=True, number=True)
self.instance.name = 'new age'
self.assertHasChanged(name=True, number=True)

def test_pre_save_previous(self):
self.assertPrevious(name=None, number=None)
self.instance.name = 'new age'
Expand All @@ -710,32 +705,39 @@ def test_descriptor(self):
self.assertTrue(isinstance(self.tracked_class.tracker, FieldTracker))

def test_pre_save_changed(self):
self.assertChanged(name=None, number=None, id=None)
self.assertChanged(name=None) # XXX Is this odd?
self.instance.name = 'new age'
self.assertChanged(name=None, number=None, id=None)
self.assertChanged(name=None)
self.instance.number = 8
self.assertChanged(name=None, number=None, id=None)
self.assertChanged(name=None, number=None)
self.instance.name = ''
self.assertChanged(name=None, number=None, id=None)
self.assertChanged(name=None, number=None)

def test_first_save(self):
def test_pre_save_has_changed(self):
self.assertHasChanged(name=True, number=False)
self.instance.name = 'new age'
self.assertHasChanged(name=True, number=False)
self.instance.number = 7
self.assertHasChanged(name=True, number=True)

def test_first_save(self):
self.assertHasChanged(name=True, number=False)
self.assertPrevious(name=None, number=None)
self.assertCurrent(name='', number=None, id=None)
self.assertChanged(name=None, number=None, id=None)
self.assertChanged(name=None)
self.instance.name = 'retro'
self.instance.number = 4
self.assertHasChanged(name=True, number=True)
self.assertPrevious(name=None, number=None)
self.assertCurrent(name='retro', number=4, id=None)
self.assertChanged(name=None, number=None, id=None)
self.assertChanged(name=None, number=None)
# Django 1.4 doesn't have update_fields
if django.VERSION >= (1, 5, 0):
self.instance.save(update_fields=[])
self.assertHasChanged(name=True, number=True)
self.assertPrevious(name=None, number=None)
self.assertCurrent(name='retro', number=4, id=None)
self.assertChanged(name=None, number=None, id=None)
self.assertChanged(name=None, number=None)
self.assertRaises(ValueError, self.instance.save,
update_fields=['number'])

Expand Down Expand Up @@ -813,6 +815,25 @@ def test_pre_save_changed(self):
self.instance.name = ''
self.assertChanged(name=None)

def test_first_save(self):
self.assertHasChanged(name=True, number=None)
self.assertPrevious(name=None, number=None)
self.assertCurrent(name='')
self.assertChanged(name=None)
self.instance.name = 'retro'
self.instance.number = 4
self.assertHasChanged(name=True, number=None)
self.assertPrevious(name=None, number=None)
self.assertCurrent(name='retro')
self.assertChanged(name=None)

def test_pre_save_has_changed(self):
self.assertHasChanged(name=True, number=None)
self.instance.name = 'new age'
self.assertHasChanged(name=True, number=None)
self.instance.number = 7
self.assertHasChanged(name=True, number=None)

def test_post_save_has_changed(self):
self.update_instance(name='retro', number=4)
self.assertHasChanged(name=False, number=None)
Expand Down Expand Up @@ -858,11 +879,6 @@ def setUp(self):
self.trackers = [self.instance.name_tracker,
self.instance.number_tracker]

def test_pre_save_has_changed(self):
for tracker in self.trackers:
self.tracker = tracker
super(FieldTrackedModelMultiTests, self).test_pre_save_has_changed()

def test_pre_save_changed(self):
self.tracker = self.instance.name_tracker
self.assertChanged(name=None)
Expand All @@ -879,6 +895,16 @@ def test_pre_save_changed(self):
self.instance.number = 8
self.assertChanged(number=None)

def test_pre_save_has_changed(self):
self.tracker = self.instance.name_tracker
self.assertHasChanged(name=True, number=None)
self.instance.name = 'new age'
self.assertHasChanged(name=True, number=None)
self.tracker = self.instance.number_tracker
self.assertHasChanged(name=None, number=False)
self.instance.name = 'new age'
self.assertHasChanged(name=None, number=False)

def test_pre_save_previous(self):
for tracker in self.trackers:
self.tracker = tracker
Expand Down Expand Up @@ -1011,11 +1037,37 @@ def test_first_save(self):
self.assertRaises(ValueError, self.instance.save,
update_fields=['number'])

def test_pre_save_has_changed(self):
self.assertHasChanged(name=True, number=True)
self.instance.name = 'new age'
self.assertHasChanged(name=True, number=True)
self.instance.number = 7
self.assertHasChanged(name=True, number=True)


class ModelTrackedModelCustomTests(FieldTrackedModelCustomTests):

tracked_class = ModelTrackedNotDefault

def test_first_save(self):
self.assertHasChanged(name=True, number=True)
self.assertPrevious(name=None, number=None)
self.assertCurrent(name='')
self.assertChanged()
self.instance.name = 'retro'
self.instance.number = 4
self.assertHasChanged(name=True, number=True)
self.assertPrevious(name=None, number=None)
self.assertCurrent(name='retro')
self.assertChanged()

def test_pre_save_has_changed(self):
self.assertHasChanged(name=True, number=True)
self.instance.name = 'new age'
self.assertHasChanged(name=True, number=True)
self.instance.number = 7
self.assertHasChanged(name=True, number=True)

def test_pre_save_changed(self):
self.assertChanged()
self.instance.name = 'new age'
Expand All @@ -1030,6 +1082,16 @@ class ModelTrackedModelMultiTests(FieldTrackedModelMultiTests):

tracked_class = ModelTrackedMultiple

def test_pre_save_has_changed(self):
self.tracker = self.instance.name_tracker
self.assertHasChanged(name=True, number=True)
self.instance.name = 'new age'
self.assertHasChanged(name=True, number=True)
self.tracker = self.instance.number_tracker
self.assertHasChanged(name=True, number=True)
self.instance.name = 'new age'
self.assertHasChanged(name=True, number=True)

def test_pre_save_changed(self):
self.tracker = self.instance.name_tracker
self.assertChanged()
Expand Down
13 changes: 10 additions & 3 deletions model_utils/tracker.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@ def current(self, fields=None):

def has_changed(self, field):
"""Returns ``True`` if field has changed from currently saved value"""
if not self.instance.pk:
return True
elif field in self.saved_data:
if field in self.fields:
return self.previous(field) != self.get_field_value(field)
else:
raise FieldError('field "%s" not tracked' % field)
Expand Down Expand Up @@ -98,6 +96,15 @@ def __get__(self, instance, owner):

class ModelInstanceTracker(FieldInstanceTracker):

def has_changed(self, field):
"""Returns ``True`` if field has changed from currently saved value"""
if not self.instance.pk:
return True
elif field in self.saved_data:
return self.previous(field) != self.get_field_value(field)
else:
raise FieldError('field "%s" not tracked' % field)

def changed(self):
"""Returns dict of fields that changed since save (with old values)"""
if not self.instance.pk:
Expand Down

0 comments on commit 66f8358

Please sign in to comment.