Moved has_changed logic from widget to form field

Refs #16612. Thanks Aymeric Augustin for the suggestion.
django · Jan 25, 2013 · ebb504d · ebb504d
1 parent ce27fb1
commit ebb504d
Show file tree

Hide file tree

Showing 14 changed files with 230 additions and 251 deletions.
diff --git a/django/contrib/admin/widgets.py b/django/contrib/admin/widgets.py
@@ -213,17 +213,6 @@ def value_from_datadict(self, data, files, name):
         if value:
             return value.split(',')
 
-    def _has_changed(self, initial, data):
-        if initial is None:
-            initial = []
-        if data is None:
-            data = []
-        if len(initial) != len(data):
-            return True
-        for pk1, pk2 in zip(initial, data):
-            if force_text(pk1) != force_text(pk2):
-                return True
-        return False
 
 class RelatedFieldWidgetWrapper(forms.Widget):
     """
@@ -279,9 +268,6 @@ def build_attrs(self, extra_attrs=None, **kwargs):
     def value_from_datadict(self, data, files, name):
         return self.widget.value_from_datadict(data, files, name)
 
-    def _has_changed(self, initial, data):
-        return self.widget._has_changed(initial, data)
-
     def id_for_label(self, id_):
         return self.widget.id_for_label(id_)
 

diff --git a/django/contrib/gis/admin/widgets.py b/django/contrib/gis/admin/widgets.py
@@ -7,7 +7,7 @@
 from django.utils import translation
 
 from django.contrib.gis.gdal import OGRException
-from django.contrib.gis.geos import GEOSGeometry, GEOSException, fromstr
+from django.contrib.gis.geos import GEOSGeometry, GEOSException
 
 # Creating a template context that contains Django settings
 # values needed by admin map templates.
@@ -117,25 +117,3 @@ def ol_projection(srid):
                     raise TypeError
                 map_options[js_name] = value
         return map_options
-
-    def _has_changed(self, initial, data):
-        """ Compare geographic value of data with its initial value. """
-
-        # Ensure we are dealing with a geographic object
-        if isinstance(initial, six.string_types):
-            try:
-                initial = GEOSGeometry(initial)
-            except (GEOSException, ValueError):
-                initial = None
-
-        # Only do a geographic comparison if both values are available
-        if initial and data:
-            data = fromstr(data)
-            data.transform(initial.srid)
-            # If the initial value was not added by the browser, the geometry
-            # provided may be slightly different, the first time it is saved.
-            # The comparison is done with a very low tolerance.
-            return not initial.equals_exact(data, tolerance=0.000001)
-        else:
-            # Check for change of state of existence
-            return bool(initial) != bool(data)
diff --git a/django/contrib/gis/forms/fields.py b/django/contrib/gis/forms/fields.py
@@ -1,11 +1,13 @@
 from __future__ import unicode_literals
 
 from django import forms
+from django.utils import six
 from django.utils.translation import ugettext_lazy as _
 
 # While this couples the geographic forms to the GEOS library,
 # it decouples from database (by not importing SpatialBackend).
-from django.contrib.gis.geos import GEOSException, GEOSGeometry
+from django.contrib.gis.geos import GEOSException, GEOSGeometry, fromstr
+
 
 class GeometryField(forms.Field):
     """
@@ -73,3 +75,25 @@ def clean(self, value):
                     raise forms.ValidationError(self.error_messages['transform_error'])
 
         return geom
+
+    def _has_changed(self, initial, data):
+        """ Compare geographic value of data with its initial value. """
+
+        # Ensure we are dealing with a geographic object
+        if isinstance(initial, six.string_types):
+            try:
+                initial = GEOSGeometry(initial)
+            except (GEOSException, ValueError):
+                initial = None
+
+        # Only do a geographic comparison if both values are available
+        if initial and data:
+            data = fromstr(data)
+            data.transform(initial.srid)
+            # If the initial value was not added by the browser, the geometry
+            # provided may be slightly different, the first time it is saved.
+            # The comparison is done with a very low tolerance.
+            return not initial.equals_exact(data, tolerance=0.000001)
+        else:
+            # Check for change of state of existence
+            return bool(initial) != bool(data)
diff --git a/django/contrib/gis/tests/geoadmin/tests.py b/django/contrib/gis/tests/geoadmin/tests.py
@@ -38,7 +38,7 @@ def test_olwidget_has_changed(self):
         """ Check that changes are accurately noticed by OpenLayersWidget. """
         geoadmin = admin.site._registry[City]
         form = geoadmin.get_changelist_form(None)()
-        has_changed = form.fields['point'].widget._has_changed
+        has_changed = form.fields['point']._has_changed
 
         initial = Point(13.4197458572965953, 52.5194108501149799, srid=4326)
         data_same = "SRID=3857;POINT(1493879.2754093995 6894592.019687599)"

diff --git a/django/forms/extras/widgets.py b/django/forms/extras/widgets.py
@@ -135,11 +135,3 @@ def create_select(self, name, field, value, val, choices):
         s = Select(choices=choices)
         select_html = s.render(field % name, val, local_attrs)
         return select_html
-
-    def _has_changed(self, initial, data):
-        try:
-            input_format = get_format('DATE_INPUT_FORMATS')[0]
-            data = datetime_safe.datetime.strptime(data, input_format).date()
-        except (TypeError, ValueError):
-            pass
-        return super(SelectDateWidget, self)._has_changed(initial, data)
diff --git a/django/forms/fields.py b/django/forms/fields.py
@@ -175,6 +175,25 @@ def widget_attrs(self, widget):
         """
         return {}
 
+    def _has_changed(self, initial, data):
+        """
+        Return True if data differs from initial.
+        """
+        # For purposes of seeing whether something has changed, None is
+        # the same as an empty string, if the data or inital value we get
+        # is None, replace it w/ ''.
+        if data is None:
+            data_value = ''
+        else:
+            data_value = data
+        if initial is None:
+            initial_value = ''
+        else:
+            initial_value = initial
+        if force_text(initial_value) != force_text(data_value):
+            return True
+        return False
+
     def __deepcopy__(self, memo):
         result = copy.copy(self)
         memo[id(self)] = result
@@ -348,6 +367,13 @@ def to_python(self, value):
     def strptime(self, value, format):
         raise NotImplementedError('Subclasses must define this method.')
 
+    def _has_changed(self, initial, data):
+        try:
+            data = self.to_python(data)
+        except ValidationError:
+            return True
+        return self.to_python(initial) != data
+
 class DateField(BaseTemporalField):
     widget = DateInput
     input_formats = formats.get_format_lazy('DATE_INPUT_FORMATS')
@@ -371,6 +397,7 @@ def to_python(self, value):
     def strptime(self, value, format):
         return datetime.datetime.strptime(value, format).date()
 
+
 class TimeField(BaseTemporalField):
     widget = TimeInput
     input_formats = formats.get_format_lazy('TIME_INPUT_FORMATS')
@@ -529,6 +556,12 @@ def bound_data(self, data, initial):
             return initial
         return data
 
+    def _has_changed(self, initial, data):
+        if data is None:
+            return False
+        return True
+
+
 class ImageField(FileField):
     default_error_messages = {
         'invalid_image': _("Upload a valid image. The file you uploaded was either not an image or a corrupted image."),
@@ -618,6 +651,7 @@ def split_url(url):
             value = urlunsplit(url_fields)
         return value
 
+
 class BooleanField(Field):
     widget = CheckboxInput
 
@@ -636,6 +670,15 @@ def to_python(self, value):
             raise ValidationError(self.error_messages['required'])
         return value
 
+    def _has_changed(self, initial, data):
+        # Sometimes data or initial could be None or '' which should be the
+        # same thing as False.
+        if initial == 'False':
+            # show_hidden_initial may have transformed False to 'False'
+            initial = False
+        return bool(initial) != bool(data)
+
+
 class NullBooleanField(BooleanField):
     """
     A field whose valid values are None, True and False. Invalid values are
@@ -660,6 +703,15 @@ def to_python(self, value):
     def validate(self, value):
         pass
 
+    def _has_changed(self, initial, data):
+        # None (unknown) and False (No) are not the same
+        if initial is not None:
+            initial = bool(initial)
+        if data is not None:
+            data = bool(data)
+        return initial != data
+
+
 class ChoiceField(Field):
     widget = Select
     default_error_messages = {
@@ -739,6 +791,7 @@ def to_python(self, value):
     def validate(self, value):
         pass
 
+
 class MultipleChoiceField(ChoiceField):
     hidden_widget = MultipleHiddenInput
     widget = SelectMultiple
@@ -765,6 +818,18 @@ def validate(self, value):
             if not self.valid_value(val):
                 raise ValidationError(self.error_messages['invalid_choice'] % {'value': val})
 
+    def _has_changed(self, initial, data):
+        if initial is None:
+            initial = []
+        if data is None:
+            data = []
+        if len(initial) != len(data):
+            return True
+        initial_set = set([force_text(value) for value in initial])
+        data_set = set([force_text(value) for value in data])
+        return data_set != initial_set
+
+
 class TypedMultipleChoiceField(MultipleChoiceField):
     def __init__(self, *args, **kwargs):
         self.coerce = kwargs.pop('coerce', lambda val: val)
@@ -899,6 +964,18 @@ def compress(self, data_list):
         """
         raise NotImplementedError('Subclasses must implement this method.')
 
+    def _has_changed(self, initial, data):
+        if initial is None:
+            initial = ['' for x in range(0, len(data))]
+        else:
+            if not isinstance(initial, list):
+                initial = self.widget.decompress(initial)
+        for field, initial, data in zip(self.fields, initial, data):
+            if field._has_changed(initial, data):
+                return True
+        return False
+
+
 class FilePathField(ChoiceField):
     def __init__(self, path, match=None, recursive=False, allow_files=True,
                  allow_folders=False, required=True, widget=None, label=None,

diff --git a/django/forms/forms.py b/django/forms/forms.py
@@ -341,7 +341,13 @@ def _get_changed_data(self):
                     hidden_widget = field.hidden_widget()
                     initial_value = hidden_widget.value_from_datadict(
                         self.data, self.files, initial_prefixed_name)
-                if field.widget._has_changed(initial_value, data_value):
+                if hasattr(field.widget, '_has_changed'):
+                    warnings.warn("The _has_changed method on widgets is deprecated,"
+                        " define it at field level instead.",
+                        PendingDeprecationWarning, stacklevel=2)
+                    if field.widget._has_changed(initial_value, data_value):
+                        self._changed_data.append(name)
+                elif field._has_changed(initial_value, data_value):
                     self._changed_data.append(name)
         return self._changed_data
     changed_data = property(_get_changed_data)

diff --git a/django/forms/models.py b/django/forms/models.py
@@ -858,15 +858,12 @@ def inlineformset_factory(parent_model, model, form=ModelForm,
 
 # Fields #####################################################################
 
-class InlineForeignKeyHiddenInput(HiddenInput):
-    def _has_changed(self, initial, data):
-        return False
-
 class InlineForeignKeyField(Field):
     """
     A basic integer field that deals with validating the given value to a
     given parent instance in an inline.
     """
+    widget = HiddenInput
     default_error_messages = {
         'invalid_choice': _('The inline foreign key did not match the parent instance primary key.'),
     }
@@ -881,7 +878,6 @@ def __init__(self, parent_instance, *args, **kwargs):
             else:
                 kwargs["initial"] = self.parent_instance.pk
         kwargs["required"] = False
-        kwargs["widget"] = InlineForeignKeyHiddenInput
         super(InlineForeignKeyField, self).__init__(*args, **kwargs)
 
     def clean(self, value):
@@ -899,6 +895,9 @@ def clean(self, value):
             raise ValidationError(self.error_messages['invalid_choice'])
         return self.parent_instance
 
+    def _has_changed(self, initial, data):
+        return False
+
 class ModelChoiceIterator(object):
     def __init__(self, field):
         self.field = field