Fixed #4136 -- Made ModelForm save empty values for nullable CharFields as NULL.

Previously, empty values were saved as strings.
This commit is contained in:
Jon Dufresne 2016-05-18 07:30:42 -07:00 committed by Tim Graham
parent f2c0eb19e9
commit 267dc4addd
9 changed files with 66 additions and 18 deletions

View File

@ -13,8 +13,8 @@ from django.core.exceptions import (
ObjectDoesNotExist, ValidationError, ObjectDoesNotExist, ValidationError,
) )
from django.db import ( from django.db import (
DEFAULT_DB_ALIAS, DJANGO_VERSION_PICKLE_KEY, DatabaseError, connections, DEFAULT_DB_ALIAS, DJANGO_VERSION_PICKLE_KEY, DatabaseError, connection,
router, transaction, connections, router, transaction,
) )
from django.db.models import signals from django.db.models import signals
from django.db.models.constants import LOOKUP_SEP from django.db.models.constants import LOOKUP_SEP
@ -1087,7 +1087,9 @@ class Model(six.with_metaclass(ModelBase)):
for field_name in unique_check: for field_name in unique_check:
f = self._meta.get_field(field_name) f = self._meta.get_field(field_name)
lookup_value = getattr(self, f.attname) lookup_value = getattr(self, f.attname)
if lookup_value is None: # TODO: Handle multiple backends with different feature flags.
if (lookup_value is None or
(lookup_value == '' and connection.features.interprets_empty_strings_as_nulls)):
# no value, skip the lookup # no value, skip the lookup
continue continue
if f.primary_key and not self._state.adding: if f.primary_key and not self._state.adding:

View File

@ -1086,6 +1086,9 @@ class CharField(Field):
# will be validated twice. This is considered acceptable since we want # will be validated twice. This is considered acceptable since we want
# the value in the form field (to pass into widget for example). # the value in the form field (to pass into widget for example).
defaults = {'max_length': self.max_length} defaults = {'max_length': self.max_length}
# TODO: Handle multiple backends with different feature flags.
if self.null and not connection.features.interprets_empty_strings_as_nulls:
defaults['empty_value'] = None
defaults.update(kwargs) defaults.update(kwargs)
return super(CharField, self).formfield(**defaults) return super(CharField, self).formfield(**defaults)

View File

@ -214,10 +214,11 @@ class Field(object):
class CharField(Field): class CharField(Field):
def __init__(self, max_length=None, min_length=None, strip=True, *args, **kwargs): def __init__(self, max_length=None, min_length=None, strip=True, empty_value='', *args, **kwargs):
self.max_length = max_length self.max_length = max_length
self.min_length = min_length self.min_length = min_length
self.strip = strip self.strip = strip
self.empty_value = empty_value
super(CharField, self).__init__(*args, **kwargs) super(CharField, self).__init__(*args, **kwargs)
if min_length is not None: if min_length is not None:
self.validators.append(validators.MinLengthValidator(int(min_length))) self.validators.append(validators.MinLengthValidator(int(min_length)))
@ -227,7 +228,7 @@ class CharField(Field):
def to_python(self, value): def to_python(self, value):
"Returns a Unicode object." "Returns a Unicode object."
if value in self.empty_values: if value in self.empty_values:
return '' return self.empty_value
value = force_text(value) value = force_text(value)
if self.strip: if self.strip:
value = value.strip() value = value.strip()

View File

@ -361,7 +361,7 @@ For each field, we describe the default widget used if you don't specify
.. class:: CharField(**kwargs) .. class:: CharField(**kwargs)
* Default widget: :class:`TextInput` * Default widget: :class:`TextInput`
* Empty value: ``''`` (an empty string) * Empty value: Whatever you've given as :attr:`empty_value`.
* Normalizes to: A Unicode object. * Normalizes to: A Unicode object.
* Validates ``max_length`` or ``min_length``, if they are provided. * Validates ``max_length`` or ``min_length``, if they are provided.
Otherwise, all inputs are valid. Otherwise, all inputs are valid.
@ -380,6 +380,12 @@ For each field, we describe the default widget used if you don't specify
If ``True`` (default), the value will be stripped of leading and If ``True`` (default), the value will be stripped of leading and
trailing whitespace. trailing whitespace.
.. attribute:: empty_value
.. versionadded:: 1.11
The value to use to represent "empty". Defaults to an empty string.
``ChoiceField`` ``ChoiceField``
--------------- ---------------

View File

@ -43,11 +43,13 @@ If ``True``, Django will store empty values as ``NULL`` in the database. Default
is ``False``. is ``False``.
Avoid using :attr:`~Field.null` on string-based fields such as Avoid using :attr:`~Field.null` on string-based fields such as
:class:`CharField` and :class:`TextField` because empty string values will :class:`CharField` and :class:`TextField`. If a string-based field has
always be stored as empty strings, not as ``NULL``. If a string-based field has
``null=True``, that means it has two possible values for "no data": ``NULL``, ``null=True``, that means it has two possible values for "no data": ``NULL``,
and the empty string. In most cases, it's redundant to have two possible values and the empty string. In most cases, it's redundant to have two possible values
for "no data;" the Django convention is to use the empty string, not ``NULL``. for "no data;" the Django convention is to use the empty string, not
``NULL``. One exception is when a :class:`CharField` has both ``unique=True``
and ``blank=True`` set. In this situation, ``null=True`` is required to avoid
unique constraint violations when saving multiple objects with blank values.
For both string-based and non-string-based fields, you will also need to For both string-based and non-string-based fields, you will also need to
set ``blank=True`` if you wish to permit empty values in forms, as the set ``blank=True`` if you wish to permit empty values in forms, as the

View File

@ -154,7 +154,8 @@ File Uploads
Forms Forms
~~~~~ ~~~~~
* ... * The new :attr:`CharField.empty_value <django.forms.CharField.empty_value>`
attribute allows specifying the Python value to use to represent "empty".
Generic Views Generic Views
~~~~~~~~~~~~~ ~~~~~~~~~~~~~
@ -258,6 +259,13 @@ Miscellaneous
displays the related object's ID. Remove the ``_id`` suffix if you want the displays the related object's ID. Remove the ``_id`` suffix if you want the
old behavior of the string representation of the object. old behavior of the string representation of the object.
* In model forms, :class:`~django.db.models.CharField` with ``null=True`` now
saves ``NULL`` for blank values instead of empty strings.
* On Oracle, :meth:`Model.validate_unique()
<django.db.models.Model.validate_unique>` no longer checks empty strings for
uniqueness as the database interprets the value as ``NULL``.
.. _deprecated-features-1.11: .. _deprecated-features-1.11:
Features deprecated in 1.11 Features deprecated in 1.11

View File

@ -66,7 +66,9 @@ Model field Form field
:class:`CharField` :class:`~django.forms.CharField` with :class:`CharField` :class:`~django.forms.CharField` with
``max_length`` set to the model field's ``max_length`` set to the model field's
``max_length`` ``max_length`` and
:attr:`~django.forms.CharField.empty_value`
set to ``None`` if ``null=True``.
:class:`CommaSeparatedIntegerField` :class:`~django.forms.CharField` :class:`CommaSeparatedIntegerField` :class:`~django.forms.CharField`

View File

@ -483,3 +483,7 @@ class StrictAssignmentAll(models.Model):
class Award(models.Model): class Award(models.Model):
name = models.CharField(max_length=30) name = models.CharField(max_length=30)
character = models.ForeignKey(Character, models.SET_NULL, blank=False, null=True) character = models.ForeignKey(Character, models.SET_NULL, blank=False, null=True)
class NullableUniqueCharFieldModel(models.Model):
codename = models.CharField(max_length=50, blank=True, null=True, unique=True)

View File

@ -28,9 +28,10 @@ from .models import (
CustomErrorMessage, CustomFF, CustomFieldForExclusionModel, DateTimePost, CustomErrorMessage, CustomFF, CustomFieldForExclusionModel, DateTimePost,
DerivedBook, DerivedPost, Document, ExplicitPK, FilePathModel, DerivedBook, DerivedPost, Document, ExplicitPK, FilePathModel,
FlexibleDatePost, Homepage, ImprovedArticle, ImprovedArticleWithParentLink, FlexibleDatePost, Homepage, ImprovedArticle, ImprovedArticleWithParentLink,
Inventory, Person, Photo, Post, Price, Product, Publication, Inventory, NullableUniqueCharFieldModel, Person, Photo, Post, Price,
PublicationDefaults, StrictAssignmentAll, StrictAssignmentFieldSpecific, Product, Publication, PublicationDefaults, StrictAssignmentAll,
Student, StumpJoke, TextFile, Triple, Writer, WriterProfile, test_images, StrictAssignmentFieldSpecific, Student, StumpJoke, TextFile, Triple,
Writer, WriterProfile, test_images,
) )
if test_images: if test_images:
@ -270,6 +271,21 @@ class ModelFormBaseTest(TestCase):
obj = form.save() obj = form.save()
self.assertEqual(obj.name, '') self.assertEqual(obj.name, '')
def test_save_blank_null_unique_charfield_saves_null(self):
form_class = modelform_factory(model=NullableUniqueCharFieldModel, fields=['codename'])
empty_value = '' if connection.features.interprets_empty_strings_as_nulls else None
form = form_class(data={'codename': ''})
self.assertTrue(form.is_valid())
form.save()
self.assertEqual(form.instance.codename, empty_value)
# Save a second form to verify there isn't a unique constraint violation.
form = form_class(data={'codename': ''})
self.assertTrue(form.is_valid())
form.save()
self.assertEqual(form.instance.codename, empty_value)
def test_missing_fields_attribute(self): def test_missing_fields_attribute(self):
message = ( message = (
"Creating a ModelForm without either the 'fields' attribute " "Creating a ModelForm without either the 'fields' attribute "
@ -800,6 +816,10 @@ class UniqueTest(TestCase):
form.save() form.save()
form = ExplicitPKForm({'key': 'key1', 'desc': ''}) form = ExplicitPKForm({'key': 'key1', 'desc': ''})
self.assertFalse(form.is_valid()) self.assertFalse(form.is_valid())
if connection.features.interprets_empty_strings_as_nulls:
self.assertEqual(len(form.errors), 1)
self.assertEqual(form.errors['key'], ['Explicit pk with this Key already exists.'])
else:
self.assertEqual(len(form.errors), 3) self.assertEqual(len(form.errors), 3)
self.assertEqual(form.errors['__all__'], ['Explicit pk with this Key and Desc already exists.']) self.assertEqual(form.errors['__all__'], ['Explicit pk with this Key and Desc already exists.'])
self.assertEqual(form.errors['desc'], ['Explicit pk with this Desc already exists.']) self.assertEqual(form.errors['desc'], ['Explicit pk with this Desc already exists.'])