From 5debbdfcc84266703191e084914998e38f5f52eb Mon Sep 17 00:00:00 2001 From: Srinivas Reddy Thatiparthy Date: Thu, 13 Jul 2017 20:25:32 +0530 Subject: [PATCH] Fixed #28387 -- Fixed has_changed() for disabled form fields that subclass it. --- django/forms/fields.py | 8 ++++++++ django/forms/models.py | 4 ++++ docs/releases/1.11.4.txt | 4 ++++ tests/forms_tests/field_tests/test_booleanfield.py | 4 ++++ tests/forms_tests/field_tests/test_filefield.py | 4 ++++ tests/forms_tests/field_tests/test_multiplechoicefield.py | 4 ++++ tests/forms_tests/field_tests/test_multivaluefield.py | 4 ++++ tests/model_forms/tests.py | 8 ++++++++ 8 files changed, 40 insertions(+) diff --git a/django/forms/fields.py b/django/forms/fields.py index 3a0609b915..ce4261cbf5 100644 --- a/django/forms/fields.py +++ b/django/forms/fields.py @@ -586,6 +586,8 @@ class FileField(Field): return data def has_changed(self, initial, data): + if self.disabled: + return False if data is None: return False return True @@ -706,6 +708,8 @@ class BooleanField(Field): raise ValidationError(self.error_messages['required'], code='required') def has_changed(self, initial, data): + if self.disabled: + return False # Sometimes data or initial may be a string equivalent of a boolean # so we should run it through to_python first to get a boolean value return self.to_python(initial) != self.to_python(data) @@ -864,6 +868,8 @@ class MultipleChoiceField(ChoiceField): ) def has_changed(self, initial, data): + if self.disabled: + return False if initial is None: initial = [] if data is None: @@ -1042,6 +1048,8 @@ class MultiValueField(Field): raise NotImplementedError('Subclasses must implement this method.') def has_changed(self, initial, data): + if self.disabled: + return False if initial is None: initial = ['' for x in range(0, len(data))] else: diff --git a/django/forms/models.py b/django/forms/models.py index b426623bce..37ef43b767 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -1250,6 +1250,8 @@ class ModelChoiceField(ChoiceField): return Field.validate(self, value) def has_changed(self, initial, data): + if self.disabled: + return False initial_value = initial if initial is not None else '' data_value = data if data is not None else '' return str(self.prepare_value(initial_value)) != str(data_value) @@ -1334,6 +1336,8 @@ class ModelMultipleChoiceField(ModelChoiceField): return super().prepare_value(value) def has_changed(self, initial, data): + if self.disabled: + return False if initial is None: initial = [] if data is None: diff --git a/docs/releases/1.11.4.txt b/docs/releases/1.11.4.txt index 9448611ecc..c64f5c9c6f 100644 --- a/docs/releases/1.11.4.txt +++ b/docs/releases/1.11.4.txt @@ -21,3 +21,7 @@ Bugfixes * Fixed crash in ``runserver``'s ``autoreload`` with Python 2 on Windows with non-``str`` environment variables (:ticket:`28174`). + +* Corrected ``Field.has_changed()`` to return ``False`` for disabled form + fields: ``BooleanField``, ``MultipleChoiceField``, ``MultiValueField``, + ``FileField``, ``ModelChoiceField``, and ``ModelMultipleChoiceField``. diff --git a/tests/forms_tests/field_tests/test_booleanfield.py b/tests/forms_tests/field_tests/test_booleanfield.py index d2ed9d7955..7ea117a071 100644 --- a/tests/forms_tests/field_tests/test_booleanfield.py +++ b/tests/forms_tests/field_tests/test_booleanfield.py @@ -57,3 +57,7 @@ class BooleanFieldTest(SimpleTestCase): self.assertFalse(f.has_changed(True, 'True')) self.assertTrue(f.has_changed(False, 'True')) self.assertTrue(f.has_changed(True, 'False')) + + def test_disabled_has_changed(self): + f = BooleanField(disabled=True) + self.assertIs(f.has_changed('True', 'False'), False) diff --git a/tests/forms_tests/field_tests/test_filefield.py b/tests/forms_tests/field_tests/test_filefield.py index 5961e16f67..fc5c4b5c1e 100644 --- a/tests/forms_tests/field_tests/test_filefield.py +++ b/tests/forms_tests/field_tests/test_filefield.py @@ -74,5 +74,9 @@ class FileFieldTest(SimpleTestCase): # with here) self.assertTrue(f.has_changed('resume.txt', {'filename': 'resume.txt', 'content': 'My resume'})) + def test_disabled_has_changed(self): + f = FileField(disabled=True) + self.assertIs(f.has_changed('x', 'y'), False) + def test_file_picklable(self): self.assertIsInstance(pickle.loads(pickle.dumps(FileField())), FileField) diff --git a/tests/forms_tests/field_tests/test_multiplechoicefield.py b/tests/forms_tests/field_tests/test_multiplechoicefield.py index dee916bd8e..9ffe461687 100644 --- a/tests/forms_tests/field_tests/test_multiplechoicefield.py +++ b/tests/forms_tests/field_tests/test_multiplechoicefield.py @@ -68,3 +68,7 @@ class MultipleChoiceFieldTest(SimpleTestCase): self.assertFalse(f.has_changed([2, 1], ['1', '2'])) self.assertTrue(f.has_changed([1, 2], ['1'])) self.assertTrue(f.has_changed([1, 2], ['1', '3'])) + + def test_disabled_has_changed(self): + f = MultipleChoiceField(choices=[('1', 'One'), ('2', 'Two')], disabled=True) + self.assertIs(f.has_changed('x', 'y'), False) diff --git a/tests/forms_tests/field_tests/test_multivaluefield.py b/tests/forms_tests/field_tests/test_multivaluefield.py index 82d51ac657..e77bb0df26 100644 --- a/tests/forms_tests/field_tests/test_multivaluefield.py +++ b/tests/forms_tests/field_tests/test_multivaluefield.py @@ -103,6 +103,10 @@ class MultiValueFieldTest(SimpleTestCase): ['some text', ['J', 'P'], ['2009-04-25', '11:44:00']], )) + def test_disabled_has_changed(self): + f = MultiValueField(fields=(CharField(), CharField()), disabled=True) + self.assertIs(f.has_changed(['x', 'x'], ['y', 'y']), False) + def test_form_as_table(self): form = ComplexFieldForm() self.assertHTMLEqual( diff --git a/tests/model_forms/tests.py b/tests/model_forms/tests.py index 9a4ca57786..578ef00696 100644 --- a/tests/model_forms/tests.py +++ b/tests/model_forms/tests.py @@ -1702,6 +1702,10 @@ class ModelChoiceFieldTests(TestCase): ['Select a valid choice. That choice is not one of the available choices.'] ) + def test_disabled_modelchoicefield_has_changed(self): + field = forms.ModelChoiceField(Author.objects.all(), disabled=True) + self.assertIs(field.has_changed('x', 'y'), False) + def test_disabled_multiplemodelchoicefield(self): class ArticleForm(forms.ModelForm): categories = forms.ModelMultipleChoiceField(Category.objects.all(), required=False) @@ -1727,6 +1731,10 @@ class ModelChoiceFieldTests(TestCase): self.assertEqual(form.errors, {}) self.assertEqual([x.pk for x in form.cleaned_data['categories']], [category1.pk]) + def test_disabled_modelmultiplechoicefield_has_changed(self): + field = forms.ModelMultipleChoiceField(Author.objects.all(), disabled=True) + self.assertIs(field.has_changed('x', 'y'), False) + def test_modelchoicefield_iterator(self): """ Iterator defaults to ModelChoiceIterator and can be overridden with