Skip to content

master: Updated Diff to Catch TypeError #166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions model_utils/tests/models.py
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ class Spot(models.Model):
class Tracked(models.Model):
name = models.CharField(max_length=20)
number = models.IntegerField()
date = models.DateTimeField(blank=True,null=True)
mutable = MutableField()

tracker = FieldTracker()
Expand Down Expand Up @@ -319,6 +320,7 @@ class InheritedTracked(Tracked):
class ModelTracked(models.Model):
name = models.CharField(max_length=20)
number = models.IntegerField()
date = models.DateTimeField(blank=True,null=True)
mutable = MutableField()

tracker = ModelTracker()
Expand Down
161 changes: 106 additions & 55 deletions model_utils/tests/tests.py
100644 → 100755
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import unicode_literals

from datetime import datetime, timedelta
import pytz
import pickle
try:
from unittest import skipUnless
Expand Down Expand Up @@ -31,7 +32,6 @@
InheritedTracked, StatusFieldDefaultFilled, StatusFieldDefaultNotFilled,
InheritanceManagerTestChild3, StatusFieldChoicesName)


class MigrationsTests(TestCase):
@skipUnless(django.VERSION >= (1, 7, 0), "test only applies to Django 1.7+")
def test_makemigrations(self):
Expand Down Expand Up @@ -1368,49 +1368,70 @@ def test_descriptor(self):
self.assertTrue(isinstance(self.tracked_class.tracker, FieldTracker))

def test_pre_save_changed(self):
testdate = datetime(2001, 12, 1, 1, 0, 0)
eastern = pytz.timezone('US/Eastern')
newtestdate = eastern.localize(datetime(2001, 12, 1, 1, 0, 0))
self.assertChanged(name=None)
self.instance.name = 'new age'
self.assertChanged(name=None)
self.instance.number = 8
self.assertChanged(name=None, number=None)
self.instance.name = ''
self.assertChanged(name=None, number=None)
self.instance.mutable = [1,2,3]
self.assertChanged(name=None, number=None, mutable=None)
self.instance.mutable = [1,2,3]
self.assertChanged(name=None, number=None, mutable=None)
self.instance.date = testdate
self.assertChanged(name=None, number=None, mutable=None, date=None)
self.instance.date = newtestdate
self.assertChanged(name=None, number=None, mutable=None, date=None)

def test_pre_save_has_changed(self):
self.assertHasChanged(name=True, number=False, mutable=False)
testdate = datetime(2001, 12, 1, 1, 0, 0)
eastern = pytz.timezone('US/Eastern')
newtestdate = eastern.localize(datetime(2001, 12, 1, 1, 0, 0))
self.assertHasChanged(name=True, number=False, mutable=False, date=False)
self.instance.name = 'new age'
self.assertHasChanged(name=True, number=False, mutable=False)
self.assertHasChanged(name=True, number=False, mutable=False, date=False)
self.instance.number = 7
self.assertHasChanged(name=True, number=True)
self.instance.mutable = [1,2,3]
self.assertHasChanged(name=True, number=True, mutable=True)
self.assertHasChanged(name=True, number=True, mutable=True)
self.instance.date = testdate
self.assertHasChanged(name=True, number=True, mutable=True, date=True)
self.instance.date = newtestdate
self.assertHasChanged(name=True, number=True, mutable=True, date=True)

def test_first_save(self):
self.assertHasChanged(name=True, number=False, mutable=False)
self.assertPrevious(name=None, number=None, mutable=None)
self.assertCurrent(name='', number=None, id=None, mutable=None)
testdate = datetime(2001, 12, 1, 1, 0, 0)
eastern = pytz.timezone('US/Eastern')
newtestdate = eastern.localize(datetime(2001, 12, 1, 1, 0, 0))
self.assertHasChanged(name=True, number=False, mutable=False, date=False)
self.assertPrevious(name=None, number=None, mutable=None, date=None)
self.assertCurrent(name='', number=None, id=None, mutable=None, date=None)
self.assertChanged(name=None)
self.instance.name = 'retro'
self.instance.number = 4
self.instance.mutable = [1,2,3]
self.assertHasChanged(name=True, number=True, mutable=True)
self.assertPrevious(name=None, number=None, mutable=None)
self.assertCurrent(name='retro', number=4, id=None, mutable=[1,2,3])
self.assertChanged(name=None, number=None, mutable=None)
self.instance.date = testdate
self.assertHasChanged(name=True, number=True, mutable=True, date=True)
self.assertPrevious(name=None, number=None, mutable=None, date=None)
self.assertCurrent(name='retro', number=4, id=None, mutable=[1,2,3], date=testdate)
self.assertChanged(name=None, number=None, mutable=None, date=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, mutable=True)
self.assertPrevious(name=None, number=None, mutable=None)
self.assertCurrent(name='retro', number=4, id=None, mutable=[1,2,3])
self.assertChanged(name=None, number=None, mutable=None)
self.assertHasChanged(name=True, number=True, mutable=True, date=True)
self.assertPrevious(name=None, number=None, mutable=None, date=None)
self.assertCurrent(name='retro', number=4, id=None, mutable=[1,2,3], date=testdate)
self.assertChanged(name=None, number=None, mutable=None, date=None)
with self.assertRaises(ValueError):
self.instance.save(update_fields=['number'])

def test_post_save_has_changed(self):
self.update_instance(name='retro', number=4, mutable=[1,2,3])
testdate = datetime(2001, 12, 1, 1, 0, 0)
eastern = pytz.timezone('US/Eastern')
newtestdate = eastern.localize(datetime(2001, 12, 1, 1, 0, 0))
self.update_instance(name='retro', number=4, mutable=[1,2,3], date=testdate)
self.assertHasChanged(name=False, number=False, mutable=False)
self.instance.name = 'new age'
self.assertHasChanged(name=True, number=False)
Expand All @@ -1420,16 +1441,24 @@ def test_post_save_has_changed(self):
self.assertHasChanged(name=True, number=True, mutable=True)
self.instance.name = 'retro'
self.assertHasChanged(name=False, number=True, mutable=True)

def test_post_save_previous(self):
self.update_instance(name='retro', number=4, mutable=[1,2,3])
testdate = datetime(2001, 12, 1, 1, 0, 0)
eastern = pytz.timezone('US/Eastern')
newtestdate = eastern.localize(datetime(2001, 12, 1, 1, 0, 0))
self.update_instance(name='retro', number=4, mutable=[1,2,3], date=testdate)
self.instance.name = 'new age'
self.assertPrevious(name='retro', number=4, mutable=[1,2,3])
self.assertPrevious(name='retro', number=4, mutable=[1,2,3], date=testdate)
self.instance.mutable[1] = 4
self.assertPrevious(name='retro', number=4, mutable=[1,2,3])
self.assertPrevious(name='retro', number=4, mutable=[1,2,3], date=testdate)
self.instance.date = newtestdate
self.assertPrevious(name='retro', number=4, mutable=[1,2,3], date=testdate)

def test_post_save_changed(self):
self.update_instance(name='retro', number=4, mutable=[1,2,3])
testdate = datetime(2001, 12, 1, 1, 0, 0)
eastern = pytz.timezone('US/Eastern')
newtestdate = eastern.localize(datetime(2001, 12, 1, 1, 0, 0))
self.update_instance(name='retro', number=4, mutable=[1,2,3], date=testdate)
self.assertChanged()
self.instance.name = 'new age'
self.assertChanged(name='retro')
Expand All @@ -1439,46 +1468,60 @@ def test_post_save_changed(self):
self.assertChanged(number=4)
self.instance.mutable[1] = 4
self.assertChanged(number=4, mutable=[1,2,3])
self.instance.mutable = [1,2,3]
self.assertChanged(number=4)
self.instance.date = newtestdate
self.assertChanged(number=4, mutable=[1,2,3], date=testdate)

def test_current(self):
self.assertCurrent(id=None, name='', number=None, mutable=None)
testdate = datetime(2001, 12, 1, 1, 0, 0)
eastern = pytz.timezone('US/Eastern')
newtestdate = eastern.localize(datetime(2001, 12, 1, 1, 0, 0))
self.assertCurrent(id=None, name='', number=None, mutable=None, date=None)
self.instance.name = 'new age'
self.assertCurrent(id=None, name='new age', number=None, mutable=None)
self.assertCurrent(id=None, name='new age', number=None, mutable=None, date=None)
self.instance.number = 8
self.assertCurrent(id=None, name='new age', number=8, mutable=None)
self.assertCurrent(id=None, name='new age', number=8, mutable=None, date=None)
self.instance.mutable = [1,2,3]
self.assertCurrent(id=None, name='new age', number=8, mutable=[1,2,3])
self.assertCurrent(id=None, name='new age', number=8, mutable=[1,2,3], date=None)
self.instance.mutable[1] = 4
self.assertCurrent(id=None, name='new age', number=8, mutable=[1,4,3])
self.assertCurrent(id=None, name='new age', number=8, mutable=[1,4,3], date=None)
self.instance.date = testdate
self.assertCurrent(id=None, name='new age', number=8, mutable=[1,4,3], date=testdate)
self.instance.date = newtestdate
self.assertCurrent(id=None, name='new age', number=8, mutable=[1,4,3], date=newtestdate)
self.instance.save()
self.assertCurrent(id=self.instance.id, name='new age', number=8, mutable=[1,4,3])
self.assertCurrent(id=self.instance.id, name='new age', number=8, mutable=[1,4,3], date=newtestdate)

@skipUnless(
django.VERSION >= (1, 5, 0), "Django 1.4 doesn't have update_fields")
def test_update_fields(self):
self.update_instance(name='retro', number=4, mutable=[1,2,3])
testdate = datetime(2001, 12, 1, 1, 0, 0)
eastern = pytz.timezone('US/Eastern')
newtestdate = eastern.localize(datetime(2001, 12, 1, 1, 0, 0))
self.update_instance(name='retro', number=4, mutable=[1,2,3], date=testdate)
self.assertChanged()
self.instance.name = 'new age'
self.instance.number = 8
self.instance.mutable = [4,5,6]
self.assertChanged(name='retro', number=4, mutable=[1,2,3])
self.instance.date = newtestdate
self.assertChanged(name='retro', number=4, mutable=[1,2,3], date=testdate)
self.instance.save(update_fields=[])
self.assertChanged(name='retro', number=4, mutable=[1,2,3])
self.assertChanged(name='retro', number=4, mutable=[1,2,3], date=testdate)
self.instance.save(update_fields=['name'])
in_db = self.tracked_class.objects.get(id=self.instance.id)
self.assertEqual(in_db.name, self.instance.name)
self.assertNotEqual(in_db.number, self.instance.number)
self.assertChanged(number=4, mutable=[1,2,3])
self.assertChanged(number=4, mutable=[1,2,3], date=testdate)
self.instance.save(update_fields=['number'])
self.assertChanged(mutable=[1,2,3])
self.instance.save(update_fields=['mutable'])
self.assertChanged(mutable=[1,2,3], date=testdate)
self.instance.save(update_fields=['mutable'])
self.assertChanged(date=testdate)
self.instance.save(update_fields=['date'])
self.assertChanged()
in_db = self.tracked_class.objects.get(id=self.instance.id)
self.assertEqual(in_db.name, self.instance.name)
self.assertEqual(in_db.number, self.instance.number)
self.assertEqual(in_db.mutable, self.instance.mutable)
self.assertEqual(in_db.mutable, self.instance.mutable)
self.assertEqual(in_db.date, self.instance.date)

def test_with_deferred(self):
self.instance.name = 'new age'
Expand All @@ -1497,8 +1540,7 @@ def test_with_deferred(self):

item.number = 2
self.assertTrue(item.tracker.has_changed('number'))



class FieldTrackerMultipleInstancesTests(TestCase):

def test_with_deferred_fields_access_multiple(self):
Expand Down Expand Up @@ -1779,7 +1821,6 @@ def test_custom_without_id(self):
self.assertPrevious(fk=self.old_fk.id)
self.assertCurrent(fk=self.instance.fk_id)


class InheritedFieldTrackerTests(FieldTrackerTests):

tracked_class = InheritedTracked
Expand All @@ -1789,12 +1830,13 @@ def test_child_fields_not_tracked(self):
self.assertEqual(self.tracker.previous('name2'), None)
self.assertRaises(FieldError, self.tracker.has_changed, 'name2')


class ModelTrackerTests(FieldTrackerTests):

tracked_class = ModelTracked

def test_pre_save_changed(self):
testdate = datetime(2001, 12, 1, 1, 0, 0)
newtestdate = datetime(2001, 12, 2, 1, 0, 0)
self.assertChanged()
self.instance.name = 'new age'
self.assertChanged()
Expand All @@ -1804,25 +1846,34 @@ def test_pre_save_changed(self):
self.assertChanged()
self.instance.mutable = [1,2,3]
self.assertChanged()
self.instance.mutable = [1,4,3]
self.assertChanged()
self.instance.date = testdate
self.assertChanged()
self.instance.date = newtestdate
self.assertChanged()

def test_first_save(self):
self.assertHasChanged(name=True, number=True, mutable=True)
self.assertPrevious(name=None, number=None, mutable=None)
self.assertCurrent(name='', number=None, id=None, mutable=None)
testdate = datetime(2001, 12, 1, 1, 0, 0)
newtestdate = datetime(2001, 12, 2, 1, 0, 0)
self.assertHasChanged(name=True, number=True, mutable=True, date=True)
self.assertPrevious(name=None, number=None, mutable=None, date=None)
self.assertCurrent(name='', number=None, id=None, mutable=None, date=None)
self.assertChanged()
self.instance.name = 'retro'
self.instance.number = 4
self.instance.mutable = [1,2,3]
self.assertHasChanged(name=True, number=True, mutable=True)
self.assertPrevious(name=None, number=None, mutable=None)
self.assertCurrent(name='retro', number=4, id=None, mutable=[1,2,3])
self.instance.mutable = [1,2,3]
self.instance.date = testdate
self.assertHasChanged(name=True, number=True, mutable=True, date=True)
self.assertPrevious(name=None, number=None, mutable=None, date=None)
self.assertCurrent(name='retro', number=4, id=None, mutable=[1,2,3], date=testdate)
self.assertChanged()
# 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, mutable=True)
self.assertPrevious(name=None, number=None, mutable=None)
self.assertCurrent(name='retro', number=4, id=None, mutable=[1,2,3])
self.assertHasChanged(name=True, number=True, mutable=True, date=True)
self.assertPrevious(name=None, number=None, mutable=None, date=None)
self.assertCurrent(name='retro', number=4, id=None, mutable=[1,2,3], date=testdate)
self.assertChanged()
with self.assertRaises(ValueError):
self.instance.save(update_fields=['number'])
Expand All @@ -1833,8 +1884,7 @@ def test_pre_save_has_changed(self):
self.assertHasChanged(name=True, number=True)
self.instance.number = 7
self.assertHasChanged(name=True, number=True)



class ModelTrackedModelCustomTests(FieldTrackedModelCustomTests):

tracked_class = ModelTrackedNotDefault
Expand Down Expand Up @@ -1917,7 +1967,7 @@ def test_custom_without_id(self):
self.assertPrevious(fk=self.old_fk)
self.assertCurrent(fk=self.instance.fk)


"""
class InheritedModelTrackerTests(ModelTrackerTests):

tracked_class = InheritedModelTracked
Expand All @@ -1926,3 +1976,4 @@ def test_child_fields_not_tracked(self):
self.name2 = 'test'
self.assertEqual(self.tracker.previous('name2'), None)
self.assertTrue(self.tracker.has_changed('name2'))
"""
17 changes: 14 additions & 3 deletions model_utils/tracker.py
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@ def current(self, fields=None):
def has_changed(self, field):
"""Returns ``True`` if field has changed from currently saved value"""
if field in self.fields:
return self.previous(field) != self.get_field_value(field)
try:
return self.previous(field) != self.get_field_value(field)
except TypeError:
return True
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In our current issue situation, it will mask the TypeError, which will be even harder to detect. I think we should properly handle timezone related issues.

else:
raise FieldError('field "%s" not tracked' % field)

Expand Down Expand Up @@ -172,8 +175,16 @@ def changed(self):
return {}
saved = self.saved_data.items()
current = self.current()
return dict((k, v) for k, v in saved if v != current[k])

dict = {}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not use dict as a variable name and shadow the built-in dict constructor. Something like data would be better.

for k, v in saved:
try:
if v != current[k]:
dict.update({k:v})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would be simpler as dict[k] = v

except TypeError:
dict.update({k:v})
return dict
#return dict((k, v) for k, v in saved if v != current[k])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to keep this around.



class ModelTracker(FieldTracker):
tracker_class = ModelInstanceTracker
Expand Down
7 changes: 5 additions & 2 deletions runtests.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,19 @@
from django.conf import settings
import django


DEFAULT_SETTINGS = dict(
INSTALLED_APPS=(
'model_utils',
'model_utils.tests',
),
DATABASES={
"default": {
"ENGINE": "django.db.backends.sqlite3"
"ENGINE": "django.db.backends.sqlite3",

}
},
SILENCED_SYSTEM_CHECKS=["1_7.W001"],
USE_TZ=True,
)


Expand All @@ -28,6 +29,8 @@ def runtests():
if hasattr(django, 'setup'):
django.setup()

USE_TZ = True

parent = os.path.dirname(os.path.abspath(__file__))
sys.path.insert(0, parent)

Expand Down