From 8c6f3431112f3a2a7c9868cfa70603b5c29877f0 Mon Sep 17 00:00:00 2001 From: Mikhail Silonov Date: Thu, 8 Aug 2013 18:02:12 +0400 Subject: [PATCH 01/23] Added JSON Fields support --- AUTHORS.rst | 1 + CHANGES.rst | 2 + model_utils/tests/fields.py | 30 +++++++ model_utils/tests/models.py | 10 +++ model_utils/tests/tests.py | 171 +++++++++++++++++++++++++++++++++++- model_utils/tracker.py | 12 ++- 6 files changed, 222 insertions(+), 4 deletions(-) create mode 100644 model_utils/tests/fields.py diff --git a/AUTHORS.rst b/AUTHORS.rst index 3a33840..5ea4eba 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -19,3 +19,4 @@ Simon Meers sayane Trey Hunner zyegfryed +Mikhail Silonov diff --git a/CHANGES.rst b/CHANGES.rst index 9b3bb75..6f9defa 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -7,6 +7,8 @@ master (unreleased) * `Choices` now `__contains__` its Python identifier values. Thanks Keryn Knight. (Merge of GH-69). +* Added JSON Fields support. + 1.4.0 (2013.06.03) ------------------ diff --git a/model_utils/tests/fields.py b/model_utils/tests/fields.py new file mode 100644 index 0000000..b0cdcad --- /dev/null +++ b/model_utils/tests/fields.py @@ -0,0 +1,30 @@ +import json + +from django.db import models +from django.core.serializers.json import DjangoJSONEncoder + + +class SimpleJSONField(models.TextField): + + __metaclass__ = models.SubfieldBase + + def to_python(self, value): + if value == "": + return None + + try: + if isinstance(value, basestring): + return json.loads(value) + except ValueError: + pass + + return value + + def get_db_prep_save(self, value, connection): + if value == "": + return None + + if isinstance(value, dict): + value = json.dumps(value, cls=DjangoJSONEncoder) + + return super(SimpleJSONField, self).get_db_prep_save(value, connection) diff --git a/model_utils/tests/models.py b/model_utils/tests/models.py index 99d33ad..af8ff95 100644 --- a/model_utils/tests/models.py +++ b/model_utils/tests/models.py @@ -5,6 +5,7 @@ from model_utils.models import TimeStampedModel, StatusModel, TimeFramedModel from model_utils.tracker import FieldTracker, ModelTracker from model_utils.managers import QueryManager, InheritanceManager, PassThroughManager from model_utils.fields import SplitField, MonitorField, StatusField +from model_utils.tests.fields import SimpleJSONField from model_utils import Choices @@ -271,6 +272,15 @@ class TrackedMultiple(models.Model): number_tracker = FieldTracker(fields=['number']) +class TrackedWithJsonField(models.Model): + name = models.CharField(max_length=20) + number = models.IntegerField() + + props = SimpleJSONField() + + tracker = FieldTracker() + + class ModelTracked(models.Model): name = models.CharField(max_length=20) number = models.IntegerField() diff --git a/model_utils/tests/tests.py b/model_utils/tests/tests.py index ab60a62..779209c 100644 --- a/model_utils/tests/tests.py +++ b/model_utils/tests/tests.py @@ -26,9 +26,9 @@ from model_utils.tests.models import ( StatusPlainTuple, TimeFrame, Monitored, StatusManagerAdded, TimeFrameManagerAdded, Dude, SplitFieldAbstractParent, Car, Spot, ModelTracked, ModelTrackedFK, ModelTrackedNotDefault, ModelTrackedMultiple, - Tracked, TrackedFK, TrackedNotDefault, TrackedNonFieldAttr, - TrackedMultiple, StatusFieldDefaultFilled, StatusFieldDefaultNotFilled) - + Tracked, TrackedFK, TrackedNotDefault, TrackedWithJsonField, + TrackedNonFieldAttr, TrackedMultiple, StatusFieldDefaultFilled, + StatusFieldDefaultNotFilled) class GetExcerptTests(TestCase): @@ -924,6 +924,171 @@ class FieldTrackedModelCustomTests(FieldTrackerTestCase, self.assertCurrent(name='new age') +class JSONFieldTrackedModelTests(FieldTrackerTestCase): + + tracked_class = TrackedWithJsonField + + def setUp(self): + self.instance = self.tracked_class() + self.tracker = self.instance.tracker + + def test_pre_save_changed(self): + 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.props = {'attr': 1} + self.assertChanged(name=None, number=None, props=None) + + def test_first_save(self): + self.assertHasChanged(name=True) + self.assertPrevious(name=None, number=None, props=None) + self.assertCurrent(name='', number=None, props=None, id=None) + self.assertChanged(name=None) + self.instance.name = 'retro' + self.instance.number = 4 + self.instance.props = {'vodka': True} + self.assertHasChanged(name=True, number=True, props=True) + self.assertPrevious(name=None, number=None, props=None) + self.assertCurrent(name='retro', number=4, + props={'vodka': True}, id=None) + self.assertChanged(name=None, number=None, props=None) + + def test_pre_save_has_changed(self): + self.assertHasChanged(name=True) + self.instance.name = 'new age' + self.assertHasChanged(name=True) + self.instance.number = 7 + self.assertHasChanged(name=True, number=True) + self.instance.props = {'bears_on_red_square': False} + self.assertChanged(name=None, number=None, props=None) + + def test_post_save_has_changed(self): + self.update_instance( + name='retro', number=4, + props={ + 'goodies': { + 'balalaika': True, + 'Topol-M': True + } + } + ) + self.assertHasChanged(name=False, number=False, props=False) + self.instance.name = 'new age' + self.assertHasChanged(name=True, number=False, props=False) + self.instance.number = 8 + self.assertHasChanged(name=True, number=True) + self.instance.name = 'retro' + self.assertHasChanged(name=False, number=True) + self.instance.props = { + 'goodies': { + 'balalaika': False, + 'Topol-M': True + } + } + self.assertHasChanged(name=False, number=True, props=True) + + def test_post_save_previous(self): + self.update_instance( + name='retro', number=4, + props={ + 'goodies': { + 'balalaika': True, + 'Topol-M': True + } + } + ) + self.instance.name = 'new age' + self.instance.props = { + 'goodies': { + 'balalaika': False, + 'Topol-M': True + } + } + self.assertPrevious( + name='retro', + number=4, + props={ + 'goodies': { + 'balalaika': True, + 'Topol-M': True + } + } + ) + + def test_post_save_changed(self): + self.update_instance( + name='retro', number=4, + props={ + 'goodies': { + 'balalaika': True, + 'Topol-M': True + } + } + ) + self.assertChanged() + self.instance.name = 'new age' + self.assertChanged(name='retro') + self.instance.number = 8 + self.assertChanged(name='retro', number=4) + self.instance.name = 'retro' + self.assertChanged(number=4) + self.instance.props = { + 'goodies': { + 'balalaika': False, + 'Topol-M': True + } + } + self.assertChanged( + number=4, + props={ + 'goodies': { + 'balalaika': True, + 'Topol-M': True + } + } + ) + + def test_current(self): + self.assertCurrent(name='', number=None, props=None, id=None) + self.instance.name = 'new age' + self.assertCurrent(name='new age', number=None, props=None, id=None) + self.instance.number = 8 + self.assertCurrent(name='new age', number=8, props=None, id=None) + self.instance.props = { + 'goodies': { + 'balalaika': False, + 'Topol-M': True + } + } + self.assertCurrent( + name='new age', + number=8, + props={ + 'goodies': { + 'balalaika': False, + 'Topol-M': True + } + }, + id=None + ) + self.instance.save() + self.assertCurrent( + name='new age', + number=8, + props={ + 'goodies': { + 'balalaika': False, + 'Topol-M': True + } + }, + id=self.instance.id + ) + + class FieldTrackedModelAttributeTests(FieldTrackerTestCase): tracked_class = TrackedNonFieldAttr diff --git a/model_utils/tracker.py b/model_utils/tracker.py index 31fd0f2..a302c99 100644 --- a/model_utils/tracker.py +++ b/model_utils/tracker.py @@ -1,4 +1,7 @@ from __future__ import unicode_literals + +from copy import deepcopy + from django.db import models from django.core.exceptions import FieldError @@ -19,6 +22,7 @@ class FieldInstanceTracker(object): self.saved_data = self.current() else: self.saved_data.update(**self.current(fields=fields)) + return self.saved_data def current(self, fields=None): """Return dict of current values for all tracked fields""" @@ -76,7 +80,8 @@ class FieldTracker(object): def initialize_tracker(self, sender, instance, **kwargs): tracker = self.tracker_class(instance, self.fields, self.field_map) setattr(instance, self.attname, tracker) - tracker.set_saved_fields() + saved_data = tracker.set_saved_fields() + self.prevent_side_effects(saved_data) self.patch_save(instance) def patch_save(self, instance): @@ -88,6 +93,11 @@ class FieldTracker(object): return ret instance.save = save + def prevent_side_effects(self, saved_data): + for field, field_value in saved_data.items(): + if isinstance(field_value, dict): + saved_data[field] = deepcopy(field_value) + def __get__(self, instance, owner): if instance is None: return self From f20e723952fd9c2aefb0f6ca69c6528d08681a1d Mon Sep 17 00:00:00 2001 From: silonov Date: Sun, 11 Aug 2013 19:32:45 +0400 Subject: [PATCH 02/23] Added more general mutable fields support instead of json-specific --- CHANGES.rst | 2 +- model_utils/tests/fields.py | 32 +++-- model_utils/tests/models.py | 13 +- model_utils/tests/tests.py | 267 +++++++++--------------------------- model_utils/tracker.py | 19 +-- 5 files changed, 90 insertions(+), 243 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index c961a88..5dbc02a 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -12,7 +12,7 @@ master (unreleased) ``update_fields`` in which there are untracked fields. Thanks Mikhail Silonov. (Merge of GH-70, fixes GH-71). -* Added JSON Fields support. +* Added support for mutable fields (Merge of GH-73, fixes GH-74) 1.4.0 (2013.06.03) diff --git a/model_utils/tests/fields.py b/model_utils/tests/fields.py index b0cdcad..0c95815 100644 --- a/model_utils/tests/fields.py +++ b/model_utils/tests/fields.py @@ -1,30 +1,34 @@ -import json - from django.db import models -from django.core.serializers.json import DjangoJSONEncoder + +try: + unicode() + str_class = basestring +except NameError: + str_class = str + +def with_metaclass(meta, base=object): + return meta("NewBase", (base,), {}) -class SimpleJSONField(models.TextField): - - __metaclass__ = models.SubfieldBase +class MutableField(with_metaclass(models.SubfieldBase, models.TextField)): def to_python(self, value): - if value == "": + if value == '': return None try: - if isinstance(value, basestring): - return json.loads(value) + if isinstance(value, str_class): + return [int(i) for i in value.split(',')] except ValueError: pass return value def get_db_prep_save(self, value, connection): - if value == "": - return None + if value is None: + return '' - if isinstance(value, dict): - value = json.dumps(value, cls=DjangoJSONEncoder) + if isinstance(value, list): + value = ','.join((str(i) for i in value)) - return super(SimpleJSONField, self).get_db_prep_save(value, connection) + return super(MutableField, self).get_db_prep_save(value, connection) diff --git a/model_utils/tests/models.py b/model_utils/tests/models.py index af8ff95..a41cce7 100644 --- a/model_utils/tests/models.py +++ b/model_utils/tests/models.py @@ -5,7 +5,7 @@ from model_utils.models import TimeStampedModel, StatusModel, TimeFramedModel from model_utils.tracker import FieldTracker, ModelTracker from model_utils.managers import QueryManager, InheritanceManager, PassThroughManager from model_utils.fields import SplitField, MonitorField, StatusField -from model_utils.tests.fields import SimpleJSONField +from model_utils.tests.fields import MutableField from model_utils import Choices @@ -235,6 +235,7 @@ class Spot(models.Model): class Tracked(models.Model): name = models.CharField(max_length=20) number = models.IntegerField() + mutable = MutableField() tracker = FieldTracker() @@ -272,18 +273,10 @@ class TrackedMultiple(models.Model): number_tracker = FieldTracker(fields=['number']) -class TrackedWithJsonField(models.Model): - name = models.CharField(max_length=20) - number = models.IntegerField() - - props = SimpleJSONField() - - tracker = FieldTracker() - - class ModelTracked(models.Model): name = models.CharField(max_length=20) number = models.IntegerField() + mutable = MutableField() tracker = ModelTracker() diff --git a/model_utils/tests/tests.py b/model_utils/tests/tests.py index daeaa5d..2c1c538 100644 --- a/model_utils/tests/tests.py +++ b/model_utils/tests/tests.py @@ -26,9 +26,9 @@ from model_utils.tests.models import ( StatusPlainTuple, TimeFrame, Monitored, StatusManagerAdded, TimeFrameManagerAdded, Dude, SplitFieldAbstractParent, Car, Spot, ModelTracked, ModelTrackedFK, ModelTrackedNotDefault, ModelTrackedMultiple, - Tracked, TrackedFK, TrackedNotDefault, TrackedWithJsonField, - TrackedNonFieldAttr, TrackedMultiple, StatusFieldDefaultFilled, - StatusFieldDefaultNotFilled) + Tracked, TrackedFK, TrackedNotDefault,TrackedNonFieldAttr, TrackedMultiple, + StatusFieldDefaultFilled,StatusFieldDefaultNotFilled + ) class GetExcerptTests(TestCase): @@ -762,52 +762,61 @@ class FieldTrackerTests(FieldTrackerTestCase, FieldTrackerCommonTests): 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) def test_pre_save_has_changed(self): - self.assertHasChanged(name=True, number=False) + self.assertHasChanged(name=True, number=False, mutable=False) self.instance.name = 'new age' - self.assertHasChanged(name=True, number=False) + self.assertHasChanged(name=True, number=False, mutable=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) 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.assertHasChanged(name=True, number=False, mutable=False) + self.assertPrevious(name=None, number=None, mutable=None) + self.assertCurrent(name='', number=None, id=None, mutable=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) + 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) # 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) + 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) with self.assertRaises(ValueError): self.instance.save(update_fields=['number']) def test_post_save_has_changed(self): - self.update_instance(name='retro', number=4) - self.assertHasChanged(name=False, number=False) + self.update_instance(name='retro', number=4, mutable=[1,2,3]) + self.assertHasChanged(name=False, number=False, mutable=False) self.instance.name = 'new age' self.assertHasChanged(name=True, number=False) self.instance.number = 8 self.assertHasChanged(name=True, number=True) + self.instance.mutable[1] = 4 + self.assertHasChanged(name=True, number=True, mutable=True) self.instance.name = 'retro' - self.assertHasChanged(name=False, number=True) + self.assertHasChanged(name=False, number=True, mutable=True) def test_post_save_previous(self): - self.update_instance(name='retro', number=4) + self.update_instance(name='retro', number=4, mutable=[1,2,3]) self.instance.name = 'new age' - self.assertPrevious(name='retro', number=4) + self.assertPrevious(name='retro', number=4, mutable=[1,2,3]) + self.instance.mutable[1] = 4 + self.assertPrevious(name='retro', number=4, mutable=[1,2,3]) def test_post_save_changed(self): - self.update_instance(name='retro', number=4) + self.update_instance(name='retro', number=4, mutable=[1,2,3]) self.assertChanged() self.instance.name = 'new age' self.assertChanged(name='retro') @@ -815,36 +824,48 @@ class FieldTrackerTests(FieldTrackerTestCase, FieldTrackerCommonTests): self.assertChanged(name='retro', number=4) self.instance.name = 'retro' 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) def test_current(self): - self.assertCurrent(id=None, name='', number=None) + self.assertCurrent(id=None, name='', number=None, mutable=None) self.instance.name = 'new age' - self.assertCurrent(id=None, name='new age', number=None) + self.assertCurrent(id=None, name='new age', number=None, mutable=None) self.instance.number = 8 - self.assertCurrent(id=None, name='new age', number=8) + self.assertCurrent(id=None, name='new age', number=8, mutable=None) + self.instance.mutable = [1,2,3] + self.assertCurrent(id=None, name='new age', number=8, mutable=[1,2,3]) + self.instance.mutable[1] = 4 + self.assertCurrent(id=None, name='new age', number=8, mutable=[1,4,3]) self.instance.save() - self.assertCurrent(id=self.instance.id, name='new age', number=8) + self.assertCurrent(id=self.instance.id, name='new age', number=8, mutable=[1,4,3]) @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) + self.update_instance(name='retro', number=4, mutable=[1,2,3]) self.assertChanged() self.instance.name = 'new age' self.instance.number = 8 - self.assertChanged(name='retro', number=4) + self.instance.mutable = [4,5,6] + self.assertChanged(name='retro', number=4, mutable=[1,2,3]) self.instance.save(update_fields=[]) - self.assertChanged(name='retro', number=4) + self.assertChanged(name='retro', number=4, mutable=[1,2,3]) 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) + self.assertChanged(number=4, mutable=[1,2,3]) self.instance.save(update_fields=['number']) + self.assertChanged(mutable=[1,2,3]) + self.instance.save(update_fields=['mutable']) 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) class FieldTrackedModelCustomTests(FieldTrackerTestCase, @@ -929,171 +950,6 @@ class FieldTrackedModelCustomTests(FieldTrackerTestCase, self.assertChanged() -class JSONFieldTrackedModelTests(FieldTrackerTestCase): - - tracked_class = TrackedWithJsonField - - def setUp(self): - self.instance = self.tracked_class() - self.tracker = self.instance.tracker - - def test_pre_save_changed(self): - 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.props = {'attr': 1} - self.assertChanged(name=None, number=None, props=None) - - def test_first_save(self): - self.assertHasChanged(name=True) - self.assertPrevious(name=None, number=None, props=None) - self.assertCurrent(name='', number=None, props=None, id=None) - self.assertChanged(name=None) - self.instance.name = 'retro' - self.instance.number = 4 - self.instance.props = {'vodka': True} - self.assertHasChanged(name=True, number=True, props=True) - self.assertPrevious(name=None, number=None, props=None) - self.assertCurrent(name='retro', number=4, - props={'vodka': True}, id=None) - self.assertChanged(name=None, number=None, props=None) - - def test_pre_save_has_changed(self): - self.assertHasChanged(name=True) - self.instance.name = 'new age' - self.assertHasChanged(name=True) - self.instance.number = 7 - self.assertHasChanged(name=True, number=True) - self.instance.props = {'bears_on_red_square': False} - self.assertChanged(name=None, number=None, props=None) - - def test_post_save_has_changed(self): - self.update_instance( - name='retro', number=4, - props={ - 'goodies': { - 'balalaika': True, - 'Topol-M': True - } - } - ) - self.assertHasChanged(name=False, number=False, props=False) - self.instance.name = 'new age' - self.assertHasChanged(name=True, number=False, props=False) - self.instance.number = 8 - self.assertHasChanged(name=True, number=True) - self.instance.name = 'retro' - self.assertHasChanged(name=False, number=True) - self.instance.props = { - 'goodies': { - 'balalaika': False, - 'Topol-M': True - } - } - self.assertHasChanged(name=False, number=True, props=True) - - def test_post_save_previous(self): - self.update_instance( - name='retro', number=4, - props={ - 'goodies': { - 'balalaika': True, - 'Topol-M': True - } - } - ) - self.instance.name = 'new age' - self.instance.props = { - 'goodies': { - 'balalaika': False, - 'Topol-M': True - } - } - self.assertPrevious( - name='retro', - number=4, - props={ - 'goodies': { - 'balalaika': True, - 'Topol-M': True - } - } - ) - - def test_post_save_changed(self): - self.update_instance( - name='retro', number=4, - props={ - 'goodies': { - 'balalaika': True, - 'Topol-M': True - } - } - ) - self.assertChanged() - self.instance.name = 'new age' - self.assertChanged(name='retro') - self.instance.number = 8 - self.assertChanged(name='retro', number=4) - self.instance.name = 'retro' - self.assertChanged(number=4) - self.instance.props = { - 'goodies': { - 'balalaika': False, - 'Topol-M': True - } - } - self.assertChanged( - number=4, - props={ - 'goodies': { - 'balalaika': True, - 'Topol-M': True - } - } - ) - - def test_current(self): - self.assertCurrent(name='', number=None, props=None, id=None) - self.instance.name = 'new age' - self.assertCurrent(name='new age', number=None, props=None, id=None) - self.instance.number = 8 - self.assertCurrent(name='new age', number=8, props=None, id=None) - self.instance.props = { - 'goodies': { - 'balalaika': False, - 'Topol-M': True - } - } - self.assertCurrent( - name='new age', - number=8, - props={ - 'goodies': { - 'balalaika': False, - 'Topol-M': True - } - }, - id=None - ) - self.instance.save() - self.assertCurrent( - name='new age', - number=8, - props={ - 'goodies': { - 'balalaika': False, - 'Topol-M': True - } - }, - id=self.instance.id - ) - - class FieldTrackedModelAttributeTests(FieldTrackerTestCase): tracked_class = TrackedNonFieldAttr @@ -1291,24 +1147,27 @@ class ModelTrackerTests(FieldTrackerTests): self.assertChanged() self.instance.name = '' self.assertChanged() + self.instance.mutable = [1,2,3] + self.assertChanged() def test_first_save(self): - self.assertHasChanged(name=True, number=True) - self.assertPrevious(name=None, number=None) - self.assertCurrent(name='', number=None, id=None) + 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) 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', number=4, id=None) + 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() # 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.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() with self.assertRaises(ValueError): self.instance.save(update_fields=['number']) diff --git a/model_utils/tracker.py b/model_utils/tracker.py index a08a958..fa43c57 100644 --- a/model_utils/tracker.py +++ b/model_utils/tracker.py @@ -1,7 +1,6 @@ from __future__ import unicode_literals from copy import deepcopy -from json import JSONEncoder from django.db import models from django.core.exceptions import FieldError @@ -23,7 +22,10 @@ class FieldInstanceTracker(object): self.saved_data = self.current() else: self.saved_data.update(**self.current(fields=fields)) - return self.saved_data + + # preventing mutable fields side effects + for field, field_value in self.saved_data.items(): + self.saved_data[field] = deepcopy(field_value) def current(self, fields=None): """Return dict of current values for all tracked fields""" @@ -82,8 +84,7 @@ class FieldTracker(object): def initialize_tracker(self, sender, instance, **kwargs): tracker = self.tracker_class(instance, self.fields, self.field_map) setattr(instance, self.attname, tracker) - saved_data = tracker.set_saved_fields() - self.prevent_json_fields_side_effects(saved_data) + tracker.set_saved_fields() self.patch_save(instance) def patch_save(self, instance): @@ -106,16 +107,6 @@ class FieldTracker(object): return ret instance.save = save - def prevent_json_fields_side_effects(self, saved_data): - for field, field_value in saved_data.items(): - if isinstance(field_value, (dict, list, tuple)): - try: - JSONEncoder().encode(field_value) - except TypeError: - pass - else: - saved_data[field] = deepcopy(field_value) - def __get__(self, instance, owner): if instance is None: return self From ef510d53dd2a5b5369a2314155457c34b8b47b00 Mon Sep 17 00:00:00 2001 From: Mikhail Silonov Date: Mon, 12 Aug 2013 13:22:34 +0400 Subject: [PATCH 03/23] Removed reinvented wheel --- CHANGES.rst | 1 - model_utils/tests/fields.py | 12 ++---------- model_utils/tracker.py | 6 +++--- 3 files changed, 5 insertions(+), 14 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 5dbc02a..0289eb0 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -7,7 +7,6 @@ master (unreleased) * `Choices` now `__contains__` its Python identifier values. Thanks Keryn Knight. (Merge of GH-69). -======= * Fixed a bug causing ``KeyError`` when saving with the parameter ``update_fields`` in which there are untracked fields. Thanks Mikhail Silonov. (Merge of GH-70, fixes GH-71). diff --git a/model_utils/tests/fields.py b/model_utils/tests/fields.py index 0c95815..3f1503a 100644 --- a/model_utils/tests/fields.py +++ b/model_utils/tests/fields.py @@ -1,13 +1,5 @@ from django.db import models - -try: - unicode() - str_class = basestring -except NameError: - str_class = str - -def with_metaclass(meta, base=object): - return meta("NewBase", (base,), {}) +from django.utils.six import with_metaclass, string_types class MutableField(with_metaclass(models.SubfieldBase, models.TextField)): @@ -17,7 +9,7 @@ class MutableField(with_metaclass(models.SubfieldBase, models.TextField)): return None try: - if isinstance(value, str_class): + if isinstance(value, string_types): return [int(i) for i in value.split(',')] except ValueError: pass diff --git a/model_utils/tracker.py b/model_utils/tracker.py index fa43c57..ea95acf 100644 --- a/model_utils/tracker.py +++ b/model_utils/tracker.py @@ -28,7 +28,7 @@ class FieldInstanceTracker(object): self.saved_data[field] = deepcopy(field_value) def current(self, fields=None): - """Return dict of current values for all tracked fields""" + """Returns dict of current values for all tracked fields""" if fields is None: fields = self.fields return dict((f, self.get_field_value(f)) for f in fields) @@ -41,7 +41,7 @@ class FieldInstanceTracker(object): raise FieldError('field "%s" not tracked' % field) def previous(self, field): - """Return currently saved value of given field""" + """Returns currently saved value of given field""" return self.saved_data.get(field) def changed(self): @@ -61,7 +61,7 @@ class FieldTracker(object): self.fields = fields def get_field_map(self, cls): - """Return dict mapping fields names to model attribute names""" + """Returns dict mapping fields names to model attribute names""" field_map = dict((field, field) for field in self.fields) all_fields = dict((f.name, f.attname) for f in cls._meta.local_fields) field_map.update(**dict((k, v) for (k, v) in all_fields.items() From 09278083e857de6a8c4e2e776bd10a6dcfd0ccc7 Mon Sep 17 00:00:00 2001 From: Trey Hunner Date: Sat, 17 Aug 2013 11:31:03 -0700 Subject: [PATCH 04/23] Fix typo noted by @silonov --- CHANGES.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 397457e..2fac81a 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -13,7 +13,7 @@ master (unreleased) * Fixed ``FieldTracker`` usage on inherited models. Fixes GH-57. -* Add mutable field support to ``FieldTracker`` (Merge of GH-73, fixes GH-74) +* Added mutable field support to ``FieldTracker`` (Merge of GH-73, fixes GH-74) 1.4.0 (2013.06.03) From 003ad708051615afe229cbce32edce37542aafb1 Mon Sep 17 00:00:00 2001 From: Tony Aldridge Date: Fri, 23 Aug 2013 16:58:10 +0100 Subject: [PATCH 05/23] Added equality methods to Choices objects, and overrode + for Choices for easy concatenation with other Choices and choice-like iterables. Also wrote tests for them, and extended the readme to reflect this --- README.rst | 14 ++++++++ model_utils/choices.py | 22 ++++++++++++ model_utils/tests/tests.py | 73 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+) diff --git a/README.rst b/README.rst index 4d3fc08..c4304b0 100644 --- a/README.rst +++ b/README.rst @@ -99,6 +99,20 @@ the third is the human-readable version: # ... status = models.IntegerField(choices=STATUS, default=STATUS.draft) +Choices can be concatenated with the ``+`` operator, both to other Choices +instances and other iterable objects that could be converted into Choices: + +.. code-block:: python + + from model_utils import Choices + + GENERIC_CHOICES = Choices((0, 'draft', _('draft')), (1, 'published', _('published'))) + + class Article(models.Model): + STATUS = GENERIC_CHOICES + [(2, 'featured', _('featured))] + # ... + status = models.IntegerField(choices=STATUS, default=STATUS.draft) + StatusField =========== diff --git a/model_utils/choices.py b/model_utils/choices.py index b5177cd..0ce6faa 100644 --- a/model_utils/choices.py +++ b/model_utils/choices.py @@ -73,6 +73,28 @@ class Choices(object): def __getitem__(self, index): return self._choices[index] + def __add__(self, other): + if isinstance(other, self.__class__): + other = other._full + else: + other = list(other) + return Choices(*(self._full + other)) + + def __radd__(self, other): + if isinstance(other, self.__class__): + other = other._full + else: + other = list(other) + return Choices(*(other + self._full)) + + def __eq__(self, other): + if isinstance(other, self.__class__): + return self._full == other._full + return False + + def __ne__(self, other): + return not self.__eq__(other) + def __repr__(self): return '%s(%s)' % (self.__class__.__name__, ', '.join(("%s" % repr(i) for i in self._full))) diff --git a/model_utils/tests/tests.py b/model_utils/tests/tests.py index 78874d3..7b5ce06 100644 --- a/model_utils/tests/tests.py +++ b/model_utils/tests/tests.py @@ -220,6 +220,15 @@ class ChoicesTests(TestCase): self.assertRaises(ValueError, Choices, ('a',)) + def test_equality(self): + self.assertEqual(self.STATUS, Choices('DRAFT', 'PUBLISHED')) + + + def test_composability(self): + self.assertEqual(Choices('DRAFT') + Choices('PUBLISHED'), self.STATUS) + self.assertEqual(Choices('DRAFT') + ('PUBLISHED',), self.STATUS) + self.assertEqual(('DRAFT',) + Choices('PUBLISHED'), self.STATUS) + class LabelChoicesTests(ChoicesTests): def setUp(self): @@ -254,6 +263,14 @@ class LabelChoicesTests(ChoicesTests): self.assertEqual(len(self.STATUS), 3) + def test_equality(self): + self.assertEqual(self.STATUS, Choices( + ('DRAFT', 'is draft'), + ('PUBLISHED', 'is published'), + 'DELETED', + )) + + def test_repr(self): self.assertEqual(repr(self.STATUS), "Choices" + repr(( ('DRAFT', 'DRAFT', 'is draft'), @@ -262,6 +279,22 @@ class LabelChoicesTests(ChoicesTests): ))) + def test_composability(self): + self.assertEqual( + Choices(('DRAFT', 'is draft',)) + Choices(('PUBLISHED', 'is published'), 'DELETED'), + self.STATUS + ) + + self.assertEqual( + (('DRAFT', 'is draft',),) + Choices(('PUBLISHED', 'is published'), 'DELETED'), + self.STATUS + ) + + self.assertEqual( + Choices(('DRAFT', 'is draft',)) + (('PUBLISHED', 'is published'), 'DELETED'), + self.STATUS + ) + class IdentifierChoicesTests(ChoicesTests): def setUp(self): @@ -298,6 +331,46 @@ class IdentifierChoicesTests(ChoicesTests): ))) + def test_equality(self): + self.assertEqual(self.STATUS, Choices( + (0, 'DRAFT', 'is draft'), + (1, 'PUBLISHED', 'is published'), + (2, 'DELETED', 'is deleted')) + ) + + + def test_composability(self): + self.assertEqual( + Choices( + (0, 'DRAFT', 'is draft'), + (1, 'PUBLISHED', 'is published') + ) + Choices( + (2, 'DELETED', 'is deleted'), + ), + self.STATUS + ) + + self.assertEqual( + Choices( + (0, 'DRAFT', 'is draft'), + (1, 'PUBLISHED', 'is published') + ) + ( + (2, 'DELETED', 'is deleted'), + ), + self.STATUS + ) + + self.assertEqual( + ( + (0, 'DRAFT', 'is draft'), + (1, 'PUBLISHED', 'is published') + ) + Choices( + (2, 'DELETED', 'is deleted'), + ), + self.STATUS + ) + + class InheritanceManagerTests(TestCase): def setUp(self): self.child1 = InheritanceManagerTestChild1.objects.create() From 19560d90264b0363ec31db1b215c580b92ba413e Mon Sep 17 00:00:00 2001 From: Tony Aldridge Date: Fri, 23 Aug 2013 17:02:31 +0100 Subject: [PATCH 06/23] Added self to Authors file --- AUTHORS.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS.rst b/AUTHORS.rst index 33495d6..7faa28b 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -18,3 +18,4 @@ Simon Meers sayane Trey Hunner zyegfryed +Tony Aldridge From 814df377bd14a5da8330da9d9e99974a4a95a85d Mon Sep 17 00:00:00 2001 From: Tony Aldridge Date: Fri, 23 Aug 2013 17:04:18 +0100 Subject: [PATCH 07/23] Corrected typo --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index c4304b0..0c13706 100644 --- a/README.rst +++ b/README.rst @@ -109,7 +109,7 @@ instances and other iterable objects that could be converted into Choices: GENERIC_CHOICES = Choices((0, 'draft', _('draft')), (1, 'published', _('published'))) class Article(models.Model): - STATUS = GENERIC_CHOICES + [(2, 'featured', _('featured))] + STATUS = GENERIC_CHOICES + [(2, 'featured', _('featured'))] # ... status = models.IntegerField(choices=STATUS, default=STATUS.draft) From cac7416cda10236c3d0b2996443885939d00cf0a Mon Sep 17 00:00:00 2001 From: Tony Aldridge Date: Sat, 24 Aug 2013 10:40:53 +0100 Subject: [PATCH 08/23] Moved documentation for Choices field to the right place --- docs/utilities.rst | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/docs/utilities.rst b/docs/utilities.rst index 0c3ca61..1eab98b 100644 --- a/docs/utilities.rst +++ b/docs/utilities.rst @@ -55,6 +55,20 @@ the third is the human-readable version: # ... status = models.IntegerField(choices=STATUS, default=STATUS.draft) +Choices can be concatenated with the ``+`` operator, both to other Choices +instances and other iterable objects that could be converted into Choices: + +.. code-block:: python + + from model_utils import Choices + + GENERIC_CHOICES = Choices((0, 'draft', _('draft')), (1, 'published', _('published'))) + + class Article(models.Model): + STATUS = GENERIC_CHOICES + [(2, 'featured', _('featured'))] + # ... + status = models.IntegerField(choices=STATUS, default=STATUS.draft) + Field Tracker ============= From 1a6d9a193ed2c92298e8b2b6e4ed7f6ca417fbdf Mon Sep 17 00:00:00 2001 From: Tony Aldridge Date: Sat, 24 Aug 2013 10:51:25 +0100 Subject: [PATCH 09/23] Removed redundant inequality method on Choices --- model_utils/choices.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/model_utils/choices.py b/model_utils/choices.py index f46580c..e611084 100644 --- a/model_utils/choices.py +++ b/model_utils/choices.py @@ -92,9 +92,6 @@ class Choices(object): return self._full == other._full return False - def __ne__(self, other): - return not self.__eq__(other) - def __repr__(self): return '%s(%s)' % (self.__class__.__name__, ', '.join(("%s" % repr(i) for i in self._full))) From 2d6b8b460368f8740fd1dde260c87bf49d52d3ad Mon Sep 17 00:00:00 2001 From: Tony Aldridge Date: Sun, 25 Aug 2013 08:22:20 +0100 Subject: [PATCH 10/23] Alphabetised authors --- AUTHORS.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AUTHORS.rst b/AUTHORS.rst index d3ee8cc..928e32d 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -18,6 +18,6 @@ Rinat Shigapov Ryan Kaskel Simon Meers sayane +Tony Aldridge Trey Hunner zyegfryed -Tony Aldridge From b706aee4a9cd545c647fef3fde2816ed911f8b80 Mon Sep 17 00:00:00 2001 From: Tony Aldridge Date: Sun, 25 Aug 2013 08:29:10 +0100 Subject: [PATCH 11/23] Added tests to improve coverage --- model_utils/choices.py | 6 ++---- model_utils/tests/tests.py | 27 +++++++++++++++++++++++++-- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/model_utils/choices.py b/model_utils/choices.py index e611084..b657e59 100644 --- a/model_utils/choices.py +++ b/model_utils/choices.py @@ -81,10 +81,8 @@ class Choices(object): return Choices(*(self._full + other)) def __radd__(self, other): - if isinstance(other, self.__class__): - other = other._full - else: - other = list(other) + # radd is never called for matching types, so we don't check here + other = list(other) return Choices(*(other + self._full)) def __eq__(self, other): diff --git a/model_utils/tests/tests.py b/model_utils/tests/tests.py index 5444f3f..4d99bb5 100644 --- a/model_utils/tests/tests.py +++ b/model_utils/tests/tests.py @@ -229,10 +229,16 @@ class ChoicesTests(TestCase): def test_doesnt_contain_value(self): self.assertFalse('UNPUBLISHED' in self.STATUS) + def test_equality(self): self.assertEqual(self.STATUS, Choices('DRAFT', 'PUBLISHED')) + def test_inequality(self): + self.assertNotEqual(self.STATUS, ['DRAFT', 'PUBLISHED']) + self.assertNotEqual(self.STATUS, Choices('DRAFT')) + + def test_composability(self): self.assertEqual(Choices('DRAFT') + Choices('PUBLISHED'), self.STATUS) self.assertEqual(Choices('DRAFT') + ('PUBLISHED',), self.STATUS) @@ -280,6 +286,15 @@ class LabelChoicesTests(ChoicesTests): )) + def test_inequality(self): + self.assertNotEqual(self.STATUS, [ + ('DRAFT', 'is draft'), + ('PUBLISHED', 'is published'), + 'DELETED' + ]) + self.assertNotEqual(self.STATUS, Choices('DRAFT')) + + def test_repr(self): self.assertEqual(repr(self.STATUS), "Choices" + repr(( ('DRAFT', 'DRAFT', 'is draft'), @@ -370,8 +385,16 @@ class IdentifierChoicesTests(ChoicesTests): self.assertEqual(self.STATUS, Choices( (0, 'DRAFT', 'is draft'), (1, 'PUBLISHED', 'is published'), - (2, 'DELETED', 'is deleted')) - ) + (2, 'DELETED', 'is deleted') + )) + + def test_inequality(self): + self.assertNotEqual(self.STATUS, [ + (0, 'DRAFT', 'is draft'), + (1, 'PUBLISHED', 'is published'), + (2, 'DELETED', 'is deleted') + ]) + self.assertNotEqual(self.STATUS, Choices('DRAFT')) def test_composability(self): From 32f839b577fa1385e21256cb2fa960ad97903413 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Sun, 25 Aug 2013 09:04:27 -0600 Subject: [PATCH 12/23] Add Changelog note about Choices equality/addition. --- CHANGES.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index 2fac81a..50697fd 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -4,6 +4,9 @@ CHANGES master (unreleased) ------------------- +* `Choices` can now be added to other `Choices` or to any iterable, and can be + compared for equality with itself. Thanks Tony Aldridge. (Merge of GH-76.) + * `Choices` now `__contains__` its Python identifier values. Thanks Keryn Knight. (Merge of GH-69). From 2e44f1c2c0fcd07bd617d7c648640f7312b59b5d Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Thu, 29 Aug 2013 22:00:53 -0600 Subject: [PATCH 13/23] Add option-groups capability to Choices. --- CHANGES.rst | 2 + docs/utilities.rst | 16 +++++-- model_utils/choices.py | 95 ++++++++++++++++++++++++++++---------- model_utils/tests/tests.py | 51 ++++++++++++++++++++ 4 files changed, 135 insertions(+), 29 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 50697fd..3c20df0 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -4,6 +4,8 @@ CHANGES master (unreleased) ------------------- +* `Choices` now accepts option-groupings. Fixes GH-14. + * `Choices` can now be added to other `Choices` or to any iterable, and can be compared for equality with itself. Thanks Tony Aldridge. (Merge of GH-76.) diff --git a/docs/utilities.rst b/docs/utilities.rst index 1eab98b..838b87f 100644 --- a/docs/utilities.rst +++ b/docs/utilities.rst @@ -15,7 +15,6 @@ Choices class Article(models.Model): STATUS = Choices('draft', 'published') - # ... status = models.CharField(choices=STATUS, default=STATUS.draft, max_length=20) A ``Choices`` object is initialized with any number of choices. In the @@ -34,7 +33,6 @@ representation. In this case you can provide choices as two-tuples: class Article(models.Model): STATUS = Choices(('draft', _('draft')), ('published', _('published'))) - # ... status = models.CharField(choices=STATUS, default=STATUS.draft, max_length=20) But what if your database representation of choices is constrained in @@ -52,9 +50,20 @@ the third is the human-readable version: class Article(models.Model): STATUS = Choices((0, 'draft', _('draft')), (1, 'published', _('published'))) - # ... status = models.IntegerField(choices=STATUS, default=STATUS.draft) +Option groups can also be used with ``Choices``; in that case each +argument is a tuple consisting of the option group name and a list of +options, where each option in the list is either a string, a two-tuple, +or a triple as outlined above. For example:: + +.. code-block:: python + + from model_utils import Choices + + class Article(models.Model): + STATUS = Choices(('Visible', ['new', 'archived']), ('Invisible', ['draft', 'deleted'])) + Choices can be concatenated with the ``+`` operator, both to other Choices instances and other iterable objects that could be converted into Choices: @@ -66,7 +75,6 @@ instances and other iterable objects that could be converted into Choices: class Article(models.Model): STATUS = GENERIC_CHOICES + [(2, 'featured', _('featured'))] - # ... status = models.IntegerField(choices=STATUS, default=STATUS.draft) diff --git a/model_utils/choices.py b/model_utils/choices.py index b657e59..f69fc9b 100644 --- a/model_utils/choices.py +++ b/model_utils/choices.py @@ -34,66 +34,111 @@ class Choices(object): identifier, the database representation itself is available as an attribute on the ``Choices`` object, returning itself.) + Option groups can also be used with ``Choices``; in that case each + argument is a tuple consisting of the option group name and a list + of options, where each option in the list is either a string, a + two-tuple, or a triple as outlined above. + """ def __init__(self, *choices): - self._full = [] - self._choices = [] - self._choice_dict = {} - for choice in self.equalize(choices): - self._full.append(choice) - self._choices.append((choice[0], choice[2])) - self._choice_dict[choice[1]] = choice[0] + # list of choices expanded to triples - can include optgroups + self._triples = [] + # list of choices as (db, human-readable) - can include optgroups + self._doubles = [] + # dictionary mapping Python identifier to db representation + self._mapping = {} + # set of db representations + self._db_values = set() + + self._process(choices) + + + def _store(self, triple, triple_collector, double_collector): + self._mapping[triple[1]] = triple[0] + self._db_values.add(triple[0]) + triple_collector.append(triple) + double_collector.append((triple[0], triple[2])) + + + def _process(self, choices, triple_collector=None, double_collector=None): + if triple_collector is None: + triple_collector = self._triples + if double_collector is None: + double_collector = self._doubles + + store = lambda c: self._store(c, triple_collector, double_collector) - def equalize(self, choices): for choice in choices: if isinstance(choice, (list, tuple)): if len(choice) == 3: - yield choice + store(choice) elif len(choice) == 2: - yield (choice[0], choice[0], choice[1]) + if isinstance(choice[1], (list, tuple)): + # option group + group_name = choice[0] + subchoices = choice[1] + tc = [] + triple_collector.append((group_name, tc)) + dc = [] + double_collector.append((group_name, dc)) + self._process(subchoices, tc, dc) + else: + store((choice[0], choice[0], choice[1])) else: - raise ValueError("Choices can't handle a list/tuple of length %s, only 2 or 3" - % len(choice)) + raise ValueError( + "Choices can't take a list of length %s, only 2 or 3" + % len(choice) + ) else: - yield (choice, choice, choice) + store((choice, choice, choice)) + def __len__(self): - return len(self._choices) + return len(self._doubles) + def __iter__(self): - return iter(self._choices) + return iter(self._doubles) + def __getattr__(self, attname): try: - return self._choice_dict[attname] + return self._mapping[attname] except KeyError: raise AttributeError(attname) + def __getitem__(self, index): - return self._choices[index] + return self._doubles[index] + def __add__(self, other): if isinstance(other, self.__class__): - other = other._full + other = other._triples else: other = list(other) - return Choices(*(self._full + other)) + return Choices(*(self._triples + other)) + def __radd__(self, other): # radd is never called for matching types, so we don't check here other = list(other) - return Choices(*(other + self._full)) + return Choices(*(other + self._triples)) + def __eq__(self, other): if isinstance(other, self.__class__): - return self._full == other._full + return self._triples == other._triples return False + def __repr__(self): - return '%s(%s)' % (self.__class__.__name__, - ', '.join(("%s" % repr(i) for i in self._full))) + return '%s(%s)' % ( + self.__class__.__name__, + ', '.join(("%s" % repr(i) for i in self._triples)) + ) + def __contains__(self, item): - if item in self._choice_dict.values(): - return True + return item in self._db_values diff --git a/model_utils/tests/tests.py b/model_utils/tests/tests.py index 4d99bb5..e873944 100644 --- a/model_utils/tests/tests.py +++ b/model_utils/tests/tests.py @@ -222,10 +222,12 @@ class ChoicesTests(TestCase): with self.assertRaises(ValueError): Choices(('a',)) + def test_contains_value(self): self.assertTrue('PUBLISHED' in self.STATUS) self.assertTrue('DRAFT' in self.STATUS) + def test_doesnt_contain_value(self): self.assertFalse('UNPUBLISHED' in self.STATUS) @@ -245,6 +247,17 @@ class ChoicesTests(TestCase): self.assertEqual(('DRAFT',) + Choices('PUBLISHED'), self.STATUS) + def test_option_groups(self): + c = Choices(('group a', ['one', 'two']), ['group b', ('three',)]) + self.assertEqual( + list(c), + [ + ('group a', [('one', 'one'), ('two', 'two')]), + ('group b', [('three', 'three')]), + ], + ) + + class LabelChoicesTests(ChoicesTests): def setUp(self): self.STATUS = Choices( @@ -302,6 +315,7 @@ class LabelChoicesTests(ChoicesTests): ('DELETED', 'DELETED', 'DELETED'), ))) + def test_contains_value(self): self.assertTrue('PUBLISHED' in self.STATUS) self.assertTrue('DRAFT' in self.STATUS) @@ -309,9 +323,11 @@ class LabelChoicesTests(ChoicesTests): # and the internal representation are both DELETED. self.assertTrue('DELETED' in self.STATUS) + def test_doesnt_contain_value(self): self.assertFalse('UNPUBLISHED' in self.STATUS) + def test_doesnt_contain_display_value(self): self.assertFalse('is draft' in self.STATUS) @@ -333,6 +349,21 @@ class LabelChoicesTests(ChoicesTests): ) + def test_option_groups(self): + c = Choices( + ('group a', [(1, 'one'), (2, 'two')]), + ['group b', ((3, 'three'),)] + ) + self.assertEqual( + list(c), + [ + ('group a', [(1, 'one'), (2, 'two')]), + ('group b', [(3, 'three')]), + ], + ) + + + class IdentifierChoicesTests(ChoicesTests): def setUp(self): self.STATUS = Choices( @@ -367,20 +398,25 @@ class IdentifierChoicesTests(ChoicesTests): (2, 'DELETED', 'is deleted'), ))) + def test_contains_value(self): self.assertTrue(0 in self.STATUS) self.assertTrue(1 in self.STATUS) self.assertTrue(2 in self.STATUS) + def test_doesnt_contain_value(self): self.assertFalse(3 in self.STATUS) + def test_doesnt_contain_display_value(self): self.assertFalse('is draft' in self.STATUS) + def test_doesnt_contain_python_attr(self): self.assertFalse('PUBLISHED' in self.STATUS) + def test_equality(self): self.assertEqual(self.STATUS, Choices( (0, 'DRAFT', 'is draft'), @@ -388,6 +424,7 @@ class IdentifierChoicesTests(ChoicesTests): (2, 'DELETED', 'is deleted') )) + def test_inequality(self): self.assertNotEqual(self.STATUS, [ (0, 'DRAFT', 'is draft'), @@ -429,6 +466,20 @@ class IdentifierChoicesTests(ChoicesTests): ) + def test_option_groups(self): + c = Choices( + ('group a', [(1, 'ONE', 'one'), (2, 'TWO', 'two')]), + ['group b', ((3, 'THREE', 'three'),)] + ) + self.assertEqual( + list(c), + [ + ('group a', [(1, 'one'), (2, 'two')]), + ('group b', [(3, 'three')]), + ], + ) + + class InheritanceManagerTests(TestCase): def setUp(self): self.child1 = InheritanceManagerTestChild1.objects.create() From edbef99ce24d59b9dc4bef5d93e3f9ad1749c70d Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Thu, 29 Aug 2013 22:06:44 -0600 Subject: [PATCH 14/23] Bump version for 1.5.0 release. --- CHANGES.rst | 4 ++-- README.rst | 1 + model_utils/__init__.py | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 3c20df0..6307ade 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,8 +1,8 @@ CHANGES ======= -master (unreleased) -------------------- +1.5.0 (2013.08.29) +------------------ * `Choices` now accepts option-groupings. Fixes GH-14. diff --git a/README.rst b/README.rst index ccc554c..34aa4b3 100644 --- a/README.rst +++ b/README.rst @@ -42,3 +42,4 @@ pull requests tracked in it are closed, but all new issues should be filed at GitHub.) .. _BitBucket: https://bitbucket.org/carljm/django-model-utils/overview + diff --git a/model_utils/__init__.py b/model_utils/__init__.py index 36586ec..ec7658a 100644 --- a/model_utils/__init__.py +++ b/model_utils/__init__.py @@ -1,4 +1,4 @@ from .choices import Choices from .tracker import FieldTracker, ModelTracker -__version__ = '1.4.0.post1' +__version__ = '1.5.0' From 4e7ddfc221b9b6ef96246914854af999c1a176f4 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Thu, 29 Aug 2013 22:35:32 -0600 Subject: [PATCH 15/23] Bump version for dev. --- CHANGES.rst | 4 ++++ model_utils/__init__.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 6307ade..178dbe8 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,6 +1,10 @@ CHANGES ======= +master (unreleased) +------------------- + + 1.5.0 (2013.08.29) ------------------ diff --git a/model_utils/__init__.py b/model_utils/__init__.py index ec7658a..a3da699 100644 --- a/model_utils/__init__.py +++ b/model_utils/__init__.py @@ -1,4 +1,4 @@ from .choices import Choices from .tracker import FieldTracker, ModelTracker -__version__ = '1.5.0' +__version__ = '1.5.0.post1' From ff64b06c9deac30d5654a72cc2335c2c78c0c747 Mon Sep 17 00:00:00 2001 From: Tavistock Date: Sun, 8 Sep 2013 13:59:54 -0400 Subject: [PATCH 16/23] fixed code block the code block was displaying incorrectly --- docs/utilities.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/utilities.rst b/docs/utilities.rst index 838b87f..788dd17 100644 --- a/docs/utilities.rst +++ b/docs/utilities.rst @@ -55,7 +55,7 @@ the third is the human-readable version: Option groups can also be used with ``Choices``; in that case each argument is a tuple consisting of the option group name and a list of options, where each option in the list is either a string, a two-tuple, -or a triple as outlined above. For example:: +or a triple as outlined above. For example: .. code-block:: python From 4b6a8000503ce2cf754ec2b5974f9b4e5e0c0ebf Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Fri, 20 Sep 2013 10:04:10 -0600 Subject: [PATCH 17/23] Fix bug with child/grandchild select_subclasses in Django 1.6+; thanks Keryn Knight. --- CHANGES.rst | 4 ++++ model_utils/managers.py | 5 ++++- model_utils/tests/models.py | 12 +++++++++++- model_utils/tests/tests.py | 22 ++++++++++++++++++++-- 4 files changed, 39 insertions(+), 4 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 178dbe8..449608e 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -4,6 +4,10 @@ CHANGES master (unreleased) ------------------- +* Fix bug in `InheritanceManager` with grandchild classes on Django 1.6+; + `select_subclasses('child', 'child__grandchild')` would only ever get to the + child class. Thanks Keryn Knight for report and proposed fix. + 1.5.0 (2013.08.29) ------------------ diff --git a/model_utils/managers.py b/model_utils/managers.py index 231bffa..abb18b4 100644 --- a/model_utils/managers.py +++ b/model_utils/managers.py @@ -46,9 +46,12 @@ class InheritanceQuerySet(QuerySet): def iterator(self): iter = super(InheritanceQuerySet, self).iterator() if getattr(self, 'subclasses', False): + # sort the subclass names longest first, + # so with 'a' and 'a__b' it goes as deep as possible + subclasses = sorted(self.subclasses, key=len, reverse=True) for obj in iter: sub_obj = None - for s in self.subclasses: + for s in subclasses: sub_obj = self._get_sub_obj_recurse(obj, s) if sub_obj: break diff --git a/model_utils/tests/models.py b/model_utils/tests/models.py index 2a70a4d..f9e587d 100644 --- a/model_utils/tests/models.py +++ b/model_utils/tests/models.py @@ -1,4 +1,7 @@ +from __future__ import unicode_literals + from django.db import models +from django.utils.encoding import python_2_unicode_compatible from django.utils.translation import ugettext_lazy as _ from model_utils.models import TimeStampedModel, StatusModel, TimeFramedModel @@ -15,6 +18,7 @@ class InheritanceManagerTestRelated(models.Model): +@python_2_unicode_compatible class InheritanceManagerTestParent(models.Model): # FileField is just a handy descriptor-using field. Refs #6. non_related_field_using_descriptor = models.FileField(upload_to="test") @@ -24,11 +28,17 @@ class InheritanceManagerTestParent(models.Model): objects = InheritanceManager() + def __str__(self): + return "%s(%s)" % ( + self.__class__.__name__[len('InheritanceManagerTest'):], + self.pk, + ) + + class InheritanceManagerTestChild1(InheritanceManagerTestParent): non_related_field_using_descriptor_2 = models.FileField(upload_to="test") normal_field_2 = models.TextField() - pass class InheritanceManagerTestGrandChild1(InheritanceManagerTestChild1): diff --git a/model_utils/tests/tests.py b/model_utils/tests/tests.py index e873944..e0ee5e3 100644 --- a/model_utils/tests/tests.py +++ b/model_utils/tests/tests.py @@ -542,8 +542,26 @@ class InheritanceManagerTests(TestCase): self.assertEqual( set( self.get_manager().select_subclasses( - "inheritancemanagertestchild1__" - "inheritancemanagertestgrandchild1" + "inheritancemanagertestchild1__inheritancemanagertestgrandchild1" + ) + ), + children, + ) + + + @skipUnless(django.VERSION >= (1, 6, 0), "test only applies to Django 1.6+") + def test_children_and_grandchildren(self): + children = set([ + self.child1, + InheritanceManagerTestParent(pk=self.child2.pk), + self.grandchild1, + InheritanceManagerTestChild1(pk=self.grandchild1_2.pk), + ]) + self.assertEqual( + set( + self.get_manager().select_subclasses( + "inheritancemanagertestchild1", + "inheritancemanagertestchild1__inheritancemanagertestgrandchild1" ) ), children, From 5a33ff760a35e289ea35cb17a340afd7ca38cb35 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Tue, 24 Sep 2013 15:13:27 -0600 Subject: [PATCH 18/23] Fixed indexing into Choices so its useful. --- CHANGES.rst | 4 ++++ docs/utilities.rst | 7 +++++++ model_utils/choices.py | 13 ++++++++----- model_utils/tests/tests.py | 6 +++--- 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 449608e..159b4b1 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -4,6 +4,10 @@ CHANGES master (unreleased) ------------------- +* Indexing into a ``Choices`` instance now translates database representations + to human-readable choice names, rather than simply indexing into an array of + choice tuples. (Indexing into ``Choices`` was previously not documented.) + * Fix bug in `InheritanceManager` with grandchild classes on Django 1.6+; `select_subclasses('child', 'child__grandchild')` would only ever get to the child class. Thanks Keryn Knight for report and proposed fix. diff --git a/docs/utilities.rst b/docs/utilities.rst index 788dd17..9f9bb93 100644 --- a/docs/utilities.rst +++ b/docs/utilities.rst @@ -52,6 +52,13 @@ the third is the human-readable version: STATUS = Choices((0, 'draft', _('draft')), (1, 'published', _('published'))) status = models.IntegerField(choices=STATUS, default=STATUS.draft) +You can index into a ``Choices`` instance to translate a database +representation to its display name: + +.. code-block:: python + + status_display = Article.STATUS[article.status] + Option groups can also be used with ``Choices``; in that case each argument is a tuple consisting of the option group name and a list of options, where each option in the list is either a string, a two-tuple, diff --git a/model_utils/choices.py b/model_utils/choices.py index f69fc9b..bc25485 100644 --- a/model_utils/choices.py +++ b/model_utils/choices.py @@ -46,8 +46,10 @@ class Choices(object): self._triples = [] # list of choices as (db, human-readable) - can include optgroups self._doubles = [] + # dictionary mapping db representation to human-readable + self._display_map = {} # dictionary mapping Python identifier to db representation - self._mapping = {} + self._identifier_map = {} # set of db representations self._db_values = set() @@ -55,7 +57,8 @@ class Choices(object): def _store(self, triple, triple_collector, double_collector): - self._mapping[triple[1]] = triple[0] + self._identifier_map[triple[1]] = triple[0] + self._display_map[triple[0]] = triple[2] self._db_values.add(triple[0]) triple_collector.append(triple) double_collector.append((triple[0], triple[2])) @@ -104,13 +107,13 @@ class Choices(object): def __getattr__(self, attname): try: - return self._mapping[attname] + return self._identifier_map[attname] except KeyError: raise AttributeError(attname) - def __getitem__(self, index): - return self._doubles[index] + def __getitem__(self, key): + return self._display_map[key] def __add__(self, other): diff --git a/model_utils/tests/tests.py b/model_utils/tests/tests.py index e0ee5e3..8381815 100644 --- a/model_utils/tests/tests.py +++ b/model_utils/tests/tests.py @@ -200,7 +200,7 @@ class ChoicesTests(TestCase): def test_indexing(self): - self.assertEqual(self.STATUS[1], ('PUBLISHED', 'PUBLISHED')) + self.assertEqual(self.STATUS['PUBLISHED'], 'PUBLISHED') def test_iteration(self): @@ -276,7 +276,7 @@ class LabelChoicesTests(ChoicesTests): def test_indexing(self): - self.assertEqual(self.STATUS[1], ('PUBLISHED', 'is published')) + self.assertEqual(self.STATUS['PUBLISHED'], 'is published') def test_default(self): @@ -380,7 +380,7 @@ class IdentifierChoicesTests(ChoicesTests): def test_indexing(self): - self.assertEqual(self.STATUS[1], (1, 'is published')) + self.assertEqual(self.STATUS[1], 'is published') def test_getattr(self): From 911184375250d3d8e6d1fb0afd3ec6333d608469 Mon Sep 17 00:00:00 2001 From: Travis Swicegood Date: Mon, 7 Oct 2013 18:15:41 -0500 Subject: [PATCH 19/23] Refactor to make sure get_subclass() is on QuerySet There's an edge case where you might want to call get_subclass() on a QuerySet the same way you can call get() after you have already called various filter/exclude methods. --- model_utils/managers.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/model_utils/managers.py b/model_utils/managers.py index abb18b4..3c72585 100644 --- a/model_utils/managers.py +++ b/model_utils/managers.py @@ -96,6 +96,8 @@ class InheritanceQuerySet(QuerySet): else: return node + def get_subclass(self, *args, **kwargs): + return self.select_subclasses().get(*args, **kwargs) class InheritanceManager(models.Manager): @@ -108,8 +110,7 @@ class InheritanceManager(models.Manager): return self.get_query_set().select_subclasses(*subclasses) def get_subclass(self, *args, **kwargs): - return self.get_query_set().select_subclasses().get(*args, **kwargs) - + return self.get_query_set().get_subclass(*args, **kwargs) class QueryManager(models.Manager): From 03c8a1929b8c33bf690c4ead13f658805420a837 Mon Sep 17 00:00:00 2001 From: Travis Swicegood Date: Tue, 8 Oct 2013 10:38:52 -0500 Subject: [PATCH 20/23] Add test to verify get_subclass() on QuerySet --- model_utils/tests/tests.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/model_utils/tests/tests.py b/model_utils/tests/tests.py index 8381815..e8df6a2 100644 --- a/model_utils/tests/tests.py +++ b/model_utils/tests/tests.py @@ -574,6 +574,12 @@ class InheritanceManagerTests(TestCase): self.child1) + def test_get_subclass_on_queryset(self): + self.assertEqual( + self.get_manager().all().get_subclass(pk=self.child1.pk), + self.child1) + + def test_prior_select_related(self): with self.assertNumQueries(1): obj = self.get_manager().select_related( From d7f7a54e77faa2d3ce84452af383723be9af2dc0 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Tue, 8 Oct 2013 11:15:01 -0600 Subject: [PATCH 21/23] Update AUTHORS and changelog. --- AUTHORS.rst | 1 + CHANGES.rst | 3 +++ 2 files changed, 4 insertions(+) diff --git a/AUTHORS.rst b/AUTHORS.rst index 928e32d..e050fa5 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -19,5 +19,6 @@ Ryan Kaskel Simon Meers sayane Tony Aldridge +Travis Swicegood Trey Hunner zyegfryed diff --git a/CHANGES.rst b/CHANGES.rst index 159b4b1..7f0d134 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -4,6 +4,9 @@ CHANGES master (unreleased) ------------------- +* `get_subclass()` method is now available on both managers and + querysets. Thanks Travis Swicegood. Merge of GH-82. + * Indexing into a ``Choices`` instance now translates database representations to human-readable choice names, rather than simply indexing into an array of choice tuples. (Indexing into ``Choices`` was previously not documented.) From e3450977bdbfb397c071d976f39786a30cbd8524 Mon Sep 17 00:00:00 2001 From: Filipe Ximenes Date: Fri, 11 Oct 2013 00:08:38 -0300 Subject: [PATCH 22/23] adding 'when' parameter to MonitorField --- AUTHORS.rst | 1 + CHANGES.rst | 3 ++ docs/fields.rst | 13 ++++++ model_utils/fields.py | 9 ++++- model_utils/tests/models.py | 12 ++++++ model_utils/tests/tests.py | 79 ++++++++++++++++++++++++++++++++++++- 6 files changed, 114 insertions(+), 3 deletions(-) diff --git a/AUTHORS.rst b/AUTHORS.rst index e050fa5..93d1003 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -22,3 +22,4 @@ Tony Aldridge Travis Swicegood Trey Hunner zyegfryed +Filipe Ximenes diff --git a/CHANGES.rst b/CHANGES.rst index 7f0d134..ae10b14 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -15,6 +15,9 @@ master (unreleased) `select_subclasses('child', 'child__grandchild')` would only ever get to the child class. Thanks Keryn Knight for report and proposed fix. +* MonitorField now accepts a 'when' parameter. It will update only when the field + changes to one of the values specified. + 1.5.0 (2013.08.29) ------------------ diff --git a/docs/fields.rst b/docs/fields.rst index 9a0dbaf..db15f07 100644 --- a/docs/fields.rst +++ b/docs/fields.rst @@ -53,6 +53,19 @@ field changes: (A ``MonitorField`` can monitor any type of field for changes, not only a ``StatusField``.) +In case something is passed to the ``when`` parameter, the field will only +update when it matches one of the specified values: + +.. code-block:: python + + from model_utils.fields import MonitorField, StatusField + + class Article(models.Model): + STATUS = Choices('draft', 'published') + + status = StatusField() + published_at = MonitorField(monitor='status', when=['published']) + SplitField ---------- diff --git a/model_utils/fields.py b/model_utils/fields.py index 5aebb3c..2f12277 100644 --- a/model_utils/fields.py +++ b/model_utils/fields.py @@ -82,6 +82,10 @@ class MonitorField(models.DateTimeField): raise TypeError( '%s requires a "monitor" argument' % self.__class__.__name__) self.monitor = monitor + when = kwargs.pop('when', None) + if when and not isinstance(when, list): + when = [when] + self.when = when super(MonitorField, self).__init__(*args, **kwargs) def contribute_to_class(self, cls, name): @@ -101,8 +105,9 @@ class MonitorField(models.DateTimeField): previous = getattr(model_instance, self.monitor_attname, None) current = self.get_monitored_value(model_instance) if previous != current: - setattr(model_instance, self.attname, value) - self._save_initial(model_instance.__class__, model_instance) + if not self.when or current in self.when: + setattr(model_instance, self.attname, value) + self._save_initial(model_instance.__class__, model_instance) return super(MonitorField, self).pre_save(model_instance, add) diff --git a/model_utils/tests/models.py b/model_utils/tests/models.py index f9e587d..7d82aef 100644 --- a/model_utils/tests/models.py +++ b/model_utils/tests/models.py @@ -77,6 +77,18 @@ class Monitored(models.Model): +class MonitoredSingleWhen(models.Model): + name = models.CharField(max_length=25) + name_changed = MonitorField(monitor="name", when="Jose") + + + +class MonitoredMultipleWhen(models.Model): + name = models.CharField(max_length=25) + name_changed = MonitorField(monitor="name", when=["Jose", "Maria"]) + + + class Status(StatusModel): STATUS = Choices( ("active", _("active")), diff --git a/model_utils/tests/tests.py b/model_utils/tests/tests.py index e8df6a2..9ac0633 100644 --- a/model_utils/tests/tests.py +++ b/model_utils/tests/tests.py @@ -23,7 +23,7 @@ from model_utils.tests.models import ( InheritanceManagerTestGrandChild1_2, InheritanceManagerTestParent, InheritanceManagerTestChild1, InheritanceManagerTestChild2, TimeStamp, Post, Article, Status, - StatusPlainTuple, TimeFrame, Monitored, StatusManagerAdded, + StatusPlainTuple, TimeFrame, Monitored, MonitoredSingleWhen, MonitoredMultipleWhen, StatusManagerAdded, TimeFrameManagerAdded, Dude, SplitFieldAbstractParent, Car, Spot, ModelTracked, ModelTrackedFK, ModelTrackedNotDefault, ModelTrackedMultiple, InheritedModelTracked, Tracked, TrackedFK, TrackedNotDefault, TrackedNonFieldAttr, TrackedMultiple, @@ -170,6 +170,83 @@ class MonitorFieldTests(TestCase): MonitorField() + +class MonitorSingleWhenFieldTests(TestCase): + """ + Will record change only when name is 'Jose' + """ + def setUp(self): + self.instance = MonitoredSingleWhen(name='Charlie') + self.created = self.instance.name_changed + + + def test_save_no_change(self): + self.instance.save() + self.assertEqual(self.instance.name_changed, self.created) + + + def test_save_changed_to_Jose(self): + self.instance.name = 'Jose' + self.instance.save() + self.assertTrue(self.instance.name_changed > self.created) + + + def test_save_changed_to_Maria(self): + self.instance.name = 'Maria' + self.instance.save() + self.assertEqual(self.instance.name_changed, self.created) + + + def test_double_save(self): + self.instance.name = 'Jose' + self.instance.save() + changed = self.instance.name_changed + self.instance.save() + self.assertEqual(self.instance.name_changed, changed) + + + +class MonitorMultipleWhenFieldTests(TestCase): + """ + Will record change only when name is 'Jose' or 'Maria' + """ + def setUp(self): + self.instance = MonitoredMultipleWhen(name='Charlie') + self.created = self.instance.name_changed + + + def test_save_no_change(self): + self.instance.save() + self.assertEqual(self.instance.name_changed, self.created) + + + def test_save_changed_to_Jose(self): + self.instance.name = 'Jose' + self.instance.save() + self.assertTrue(self.instance.name_changed > self.created) + + + def test_save_changed_to_Maria(self): + self.instance.name = 'Maria' + self.instance.save() + self.assertTrue(self.instance.name_changed > self.created) + + + def test_save_changed_to_Pedro(self): + self.instance.name = 'Pedro' + self.instance.save() + self.assertEqual(self.instance.name_changed, self.created) + + + def test_double_save(self): + self.instance.name = 'Jose' + self.instance.save() + changed = self.instance.name_changed + self.instance.save() + self.assertEqual(self.instance.name_changed, changed) + + + class StatusFieldTests(TestCase): def test_status_with_default_filled(self): From 3d82ec89b439adf83903ccc22e10074649706a05 Mon Sep 17 00:00:00 2001 From: Filipe Ximenes Date: Fri, 11 Oct 2013 08:24:00 -0300 Subject: [PATCH 23/23] only accepting iterables to the when field --- docs/fields.rst | 2 +- model_utils/fields.py | 6 ++-- model_utils/tests/models.py | 14 ++++---- model_utils/tests/tests.py | 70 ++++++++++++++++--------------------- 4 files changed, 42 insertions(+), 50 deletions(-) diff --git a/docs/fields.rst b/docs/fields.rst index db15f07..5da4ab4 100644 --- a/docs/fields.rst +++ b/docs/fields.rst @@ -53,7 +53,7 @@ field changes: (A ``MonitorField`` can monitor any type of field for changes, not only a ``StatusField``.) -In case something is passed to the ``when`` parameter, the field will only +If a list is passed to the ``when`` parameter, the field will only update when it matches one of the specified values: .. code-block:: python diff --git a/model_utils/fields.py b/model_utils/fields.py index 2f12277..83e9e9a 100644 --- a/model_utils/fields.py +++ b/model_utils/fields.py @@ -83,8 +83,8 @@ class MonitorField(models.DateTimeField): '%s requires a "monitor" argument' % self.__class__.__name__) self.monitor = monitor when = kwargs.pop('when', None) - if when and not isinstance(when, list): - when = [when] + if when is not None: + when = set(when) self.when = when super(MonitorField, self).__init__(*args, **kwargs) @@ -105,7 +105,7 @@ class MonitorField(models.DateTimeField): previous = getattr(model_instance, self.monitor_attname, None) current = self.get_monitored_value(model_instance) if previous != current: - if not self.when or current in self.when: + if self.when is None or current in self.when: setattr(model_instance, self.attname, value) self._save_initial(model_instance.__class__, model_instance) return super(MonitorField, self).pre_save(model_instance, add) diff --git a/model_utils/tests/models.py b/model_utils/tests/models.py index 7d82aef..80537ed 100644 --- a/model_utils/tests/models.py +++ b/model_utils/tests/models.py @@ -77,18 +77,18 @@ class Monitored(models.Model): -class MonitoredSingleWhen(models.Model): - name = models.CharField(max_length=25) - name_changed = MonitorField(monitor="name", when="Jose") - - - -class MonitoredMultipleWhen(models.Model): +class MonitorWhen(models.Model): name = models.CharField(max_length=25) name_changed = MonitorField(monitor="name", when=["Jose", "Maria"]) +class MonitorWhenEmpty(models.Model): + name = models.CharField(max_length=25) + name_changed = MonitorField(monitor="name", when=[]) + + + class Status(StatusModel): STATUS = Choices( ("active", _("active")), diff --git a/model_utils/tests/tests.py b/model_utils/tests/tests.py index 9ac0633..c424af8 100644 --- a/model_utils/tests/tests.py +++ b/model_utils/tests/tests.py @@ -23,7 +23,7 @@ from model_utils.tests.models import ( InheritanceManagerTestGrandChild1_2, InheritanceManagerTestParent, InheritanceManagerTestChild1, InheritanceManagerTestChild2, TimeStamp, Post, Article, Status, - StatusPlainTuple, TimeFrame, Monitored, MonitoredSingleWhen, MonitoredMultipleWhen, StatusManagerAdded, + StatusPlainTuple, TimeFrame, Monitored, MonitorWhen, MonitorWhenEmpty, StatusManagerAdded, TimeFrameManagerAdded, Dude, SplitFieldAbstractParent, Car, Spot, ModelTracked, ModelTrackedFK, ModelTrackedNotDefault, ModelTrackedMultiple, InheritedModelTracked, Tracked, TrackedFK, TrackedNotDefault, TrackedNonFieldAttr, TrackedMultiple, @@ -171,47 +171,12 @@ class MonitorFieldTests(TestCase): -class MonitorSingleWhenFieldTests(TestCase): +class MonitorWhenFieldTests(TestCase): """ - Will record change only when name is 'Jose' + Will record changes only when name is 'Jose' or 'Maria' """ def setUp(self): - self.instance = MonitoredSingleWhen(name='Charlie') - self.created = self.instance.name_changed - - - def test_save_no_change(self): - self.instance.save() - self.assertEqual(self.instance.name_changed, self.created) - - - def test_save_changed_to_Jose(self): - self.instance.name = 'Jose' - self.instance.save() - self.assertTrue(self.instance.name_changed > self.created) - - - def test_save_changed_to_Maria(self): - self.instance.name = 'Maria' - self.instance.save() - self.assertEqual(self.instance.name_changed, self.created) - - - def test_double_save(self): - self.instance.name = 'Jose' - self.instance.save() - changed = self.instance.name_changed - self.instance.save() - self.assertEqual(self.instance.name_changed, changed) - - - -class MonitorMultipleWhenFieldTests(TestCase): - """ - Will record change only when name is 'Jose' or 'Maria' - """ - def setUp(self): - self.instance = MonitoredMultipleWhen(name='Charlie') + self.instance = MonitorWhen(name='Charlie') self.created = self.instance.name_changed @@ -247,6 +212,33 @@ class MonitorMultipleWhenFieldTests(TestCase): +class MonitorWhenEmptyFieldTests(TestCase): + """ + Monitor should never be updated id when is an empty list. + """ + def setUp(self): + self.instance = MonitorWhenEmpty(name='Charlie') + self.created = self.instance.name_changed + + + def test_save_no_change(self): + self.instance.save() + self.assertEqual(self.instance.name_changed, self.created) + + + def test_save_changed_to_Jose(self): + self.instance.name = 'Jose' + self.instance.save() + self.assertEqual(self.instance.name_changed, self.created) + + + def test_save_changed_to_Maria(self): + self.instance.name = 'Maria' + self.instance.save() + self.assertEqual(self.instance.name_changed, self.created) + + + class StatusFieldTests(TestCase): def test_status_with_default_filled(self):