From 22c6497f990fd12359b759a71abfcbf3f52b2d52 Mon Sep 17 00:00:00 2001 From: Alasdair Nicol Date: Sun, 11 Aug 2013 21:19:09 +0100 Subject: [PATCH] Fixed #20895 -- Made check management command warn if a BooleanField does not have a default value Thanks to Collin Anderson for the suggestion and Tim Graham for reviewing the patch. --- AUTHORS | 1 + django/contrib/gis/tests/geoapp/models.py | 2 +- .../core/checks/compatibility/django_1_6_0.py | 29 ++++++++++++++++++- tests/admin_views/models.py | 8 ++--- tests/aggregation_regress/models.py | 2 +- tests/check/models.py | 10 ++++++- tests/check/tests.py | 20 +++++++++++++ tests/comment_tests/models.py | 2 +- tests/custom_managers/models.py | 4 +-- tests/generic_relations/models.py | 2 +- tests/inspectdb/models.py | 2 +- tests/model_fields/models.py | 4 +-- tests/model_fields/tests.py | 23 +++++++++++---- tests/model_formsets/models.py | 6 ++-- tests/model_inheritance/models.py | 8 ++--- tests/model_inheritance_regress/models.py | 8 ++--- .../models.py | 4 +-- tests/modeladmin/models.py | 2 +- tests/one_to_one/models.py | 4 +-- tests/one_to_one_regress/models.py | 4 +-- tests/raw_query/models.py | 2 +- tests/reverse_single_related/models.py | 2 +- tests/serializers_regress/models.py | 4 +-- 23 files changed, 111 insertions(+), 42 deletions(-) diff --git a/AUTHORS b/AUTHORS index 215a06235a..059310d5c6 100644 --- a/AUTHORS +++ b/AUTHORS @@ -442,6 +442,7 @@ answer newbie questions, and generally made Django that much better: Gopal Narayanan Fraser Nevett Sam Newman + Alasdair Nicol Ryan Niemeyer Filip Noetzel Afonso Fernández Nogueira diff --git a/django/contrib/gis/tests/geoapp/models.py b/django/contrib/gis/tests/geoapp/models.py index abde509c8b..fa83859063 100644 --- a/django/contrib/gis/tests/geoapp/models.py +++ b/django/contrib/gis/tests/geoapp/models.py @@ -40,7 +40,7 @@ class Track(models.Model): def __str__(self): return self.name class Truth(models.Model): - val = models.BooleanField() + val = models.BooleanField(default=False) objects = models.GeoManager() if not spatialite: diff --git a/django/core/checks/compatibility/django_1_6_0.py b/django/core/checks/compatibility/django_1_6_0.py index 1998c5ba77..e38b2d32ec 100644 --- a/django/core/checks/compatibility/django_1_6_0.py +++ b/django/core/checks/compatibility/django_1_6_0.py @@ -1,5 +1,6 @@ from __future__ import unicode_literals +from django.db import models def check_test_runner(): """ @@ -24,6 +25,31 @@ def check_test_runner(): ] return ' '.join(message) +def check_boolean_field_default_value(): + """ + Checks if there are any BooleanFields without a default value, & + warns the user that the default has changed from False to Null. + """ + fields = [] + for cls in models.get_models(): + opts = cls._meta + for f in opts.local_fields: + if isinstance(f, models.BooleanField) and not f.has_default(): + fields.append( + '%s.%s: "%s"' % (opts.app_label, opts.object_name, f.name) + ) + if fields: + fieldnames = ", ".join(fields) + message = [ + "You have not set a default value for one or more BooleanFields:", + "%s." % fieldnames, + "In Django 1.6 the default value of BooleanField was changed from", + "False to Null when Field.default isn't defined. See", + "https://docs.djangoproject.com/en/1.6/ref/models/fields/#booleanfield" + "for more information." + ] + return ' '.join(message) + def run_checks(): """ @@ -31,7 +57,8 @@ def run_checks(): messages from all the relevant check functions for this version of Django. """ checks = [ - check_test_runner() + check_test_runner(), + check_boolean_field_default_value(), ] # Filter out the ``None`` or empty strings. return [output for output in checks if output] diff --git a/tests/admin_views/models.py b/tests/admin_views/models.py index a1131210d7..c63ac3b927 100644 --- a/tests/admin_views/models.py +++ b/tests/admin_views/models.py @@ -115,7 +115,7 @@ class ModelWithStringPrimaryKey(models.Model): @python_2_unicode_compatible class Color(models.Model): value = models.CharField(max_length=10) - warm = models.BooleanField() + warm = models.BooleanField(default=False) def __str__(self): return self.value @@ -144,7 +144,7 @@ class Actor(models.Model): @python_2_unicode_compatible class Inquisition(models.Model): - expected = models.BooleanField() + expected = models.BooleanField(default=False) leader = models.ForeignKey(Actor) country = models.CharField(max_length=20) @@ -376,7 +376,7 @@ class Link(models.Model): class PrePopulatedPost(models.Model): title = models.CharField(max_length=100) - published = models.BooleanField() + published = models.BooleanField(default=False) slug = models.SlugField() @@ -607,7 +607,7 @@ class PrePopulatedPostLargeSlug(models.Model): the javascript (ie, using THOUSAND_SEPARATOR ends up with maxLength=1,000) """ title = models.CharField(max_length=100) - published = models.BooleanField() + published = models.BooleanField(default=False) slug = models.SlugField(max_length=1000) class AdminOrderedField(models.Model): diff --git a/tests/aggregation_regress/models.py b/tests/aggregation_regress/models.py index 047c871c39..dfef691882 100644 --- a/tests/aggregation_regress/models.py +++ b/tests/aggregation_regress/models.py @@ -64,7 +64,7 @@ class Store(models.Model): class Entries(models.Model): EntryID = models.AutoField(primary_key=True, db_column='Entry ID') Entry = models.CharField(unique=True, max_length=50) - Exclude = models.BooleanField() + Exclude = models.BooleanField(default=False) class Clues(models.Model): diff --git a/tests/check/models.py b/tests/check/models.py index 78a10abba6..212b01bdd2 100644 --- a/tests/check/models.py +++ b/tests/check/models.py @@ -1 +1,9 @@ -# Stubby. +from django.db import models + +class Book(models.Model): + title = models.CharField(max_length=250) + is_published = models.BooleanField(default=False) + +class BlogPost(models.Model): + title = models.CharField(max_length=250) + is_published = models.BooleanField(default=False) diff --git a/tests/check/tests.py b/tests/check/tests.py index 98495e38ae..3faf086557 100644 --- a/tests/check/tests.py +++ b/tests/check/tests.py @@ -2,8 +2,10 @@ from django.core.checks.compatibility import base from django.core.checks.compatibility import django_1_6_0 from django.core.management.commands import check from django.core.management import call_command +from django.db.models.fields import NOT_PROVIDED from django.test import TestCase +from .models import Book class StubCheckModule(object): # Has no ``run_checks`` attribute & will trigger a warning. @@ -53,6 +55,24 @@ class CompatChecksTestCase(TestCase): with self.settings(TEST_RUNNER='myapp.test.CustomRunnner'): self.assertEqual(len(django_1_6_0.run_checks()), 0) + def test_boolean_field_default_value(self): + with self.settings(TEST_RUNNER='myapp.test.CustomRunnner'): + # We patch the field's default value to trigger the warning + boolean_field = Book._meta.get_field('is_published') + old_default = boolean_field.default + try: + boolean_field.default = NOT_PROVIDED + result = django_1_6_0.run_checks() + self.assertEqual(len(result), 1) + self.assertTrue("You have not set a default value for one or more BooleanFields" in result[0]) + self.assertTrue('check.Book: "is_published"' in result[0]) + # We did not patch the BlogPost.is_published field so + # there should not be a warning about it + self.assertFalse('check.BlogPost' in result[0]) + finally: + # Restore the ``default`` + boolean_field.default = old_default + def test_check_compatibility(self): with self.settings(TEST_RUNNER='django.test.runner.DiscoverRunner'): result = base.check_compatibility() diff --git a/tests/comment_tests/models.py b/tests/comment_tests/models.py index 472b66decd..a2b27e7e87 100644 --- a/tests/comment_tests/models.py +++ b/tests/comment_tests/models.py @@ -30,7 +30,7 @@ class Entry(models.Model): title = models.CharField(max_length=250) body = models.TextField() pub_date = models.DateField() - enable_comments = models.BooleanField() + enable_comments = models.BooleanField(default=False) def __str__(self): return self.title diff --git a/tests/custom_managers/models.py b/tests/custom_managers/models.py index 44d5eb70da..cba375f4d7 100644 --- a/tests/custom_managers/models.py +++ b/tests/custom_managers/models.py @@ -67,7 +67,7 @@ CustomManager = BaseCustomManager.from_queryset(CustomQuerySet) class Person(models.Model): first_name = models.CharField(max_length=30) last_name = models.CharField(max_length=30) - fun = models.BooleanField() + fun = models.BooleanField(default=False) objects = PersonManager() custom_queryset_default_manager = CustomQuerySet.as_manager() @@ -80,7 +80,7 @@ class Person(models.Model): class Book(models.Model): title = models.CharField(max_length=50) author = models.CharField(max_length=30) - is_published = models.BooleanField() + is_published = models.BooleanField(default=False) published_objects = PublishedBookManager() authors = models.ManyToManyField(Person, related_name='books') diff --git a/tests/generic_relations/models.py b/tests/generic_relations/models.py index 211df8aa3d..d819a53c0e 100644 --- a/tests/generic_relations/models.py +++ b/tests/generic_relations/models.py @@ -92,7 +92,7 @@ class GeckoManager(models.Manager): return super(GeckoManager, self).get_queryset().filter(has_tail=True) class Gecko(models.Model): - has_tail = models.BooleanField() + has_tail = models.BooleanField(default=False) objects = GeckoManager() # To test fix for #11263 diff --git a/tests/inspectdb/models.py b/tests/inspectdb/models.py index 2862f7f3dc..c25202f248 100644 --- a/tests/inspectdb/models.py +++ b/tests/inspectdb/models.py @@ -37,7 +37,7 @@ class SpecialColumnName(models.Model): class ColumnTypes(models.Model): id = models.AutoField(primary_key=True) big_int_field = models.BigIntegerField() - bool_field = models.BooleanField() + bool_field = models.BooleanField(default=False) null_bool_field = models.NullBooleanField() char_field = models.CharField(max_length=10) comma_separated_int_field = models.CommaSeparatedIntegerField(max_length=99) diff --git a/tests/model_fields/models.py b/tests/model_fields/models.py index 2d602d6412..8c304cc726 100644 --- a/tests/model_fields/models.py +++ b/tests/model_fields/models.py @@ -58,7 +58,7 @@ class NullBooleanModel(models.Model): nbfield = models.NullBooleanField() class BooleanModel(models.Model): - bfield = models.BooleanField() + bfield = models.BooleanField(default=None) string = models.CharField(max_length=10, default='abc') class FksToBooleans(models.Model): @@ -72,7 +72,7 @@ class RenamedField(models.Model): class VerboseNameField(models.Model): id = models.AutoField("verbose pk", primary_key=True) field1 = models.BigIntegerField("verbose field1") - field2 = models.BooleanField("verbose field2") + field2 = models.BooleanField("verbose field2", default=False) field3 = models.CharField("verbose field3", max_length=10) field4 = models.CommaSeparatedIntegerField("verbose field4", max_length=99) field5 = models.DateField("verbose field5") diff --git a/tests/model_fields/tests.py b/tests/model_fields/tests.py index 378e6280cc..429337f31c 100644 --- a/tests/model_fields/tests.py +++ b/tests/model_fields/tests.py @@ -12,7 +12,7 @@ from django.db.models.fields import ( AutoField, BigIntegerField, BinaryField, BooleanField, CharField, CommaSeparatedIntegerField, DateField, DateTimeField, DecimalField, EmailField, FilePathField, FloatField, IntegerField, IPAddressField, - GenericIPAddressField, NullBooleanField, PositiveIntegerField, + GenericIPAddressField, NOT_PROVIDED, NullBooleanField, PositiveIntegerField, PositiveSmallIntegerField, SlugField, SmallIntegerField, TextField, TimeField, URLField) from django.db.models.fields.files import FileField, ImageField @@ -275,10 +275,23 @@ class BooleanFieldTests(unittest.TestCase): Check that a BooleanField defaults to None -- which isn't a valid value (#15124). """ - b = BooleanModel() - self.assertIsNone(b.bfield) - with self.assertRaises(IntegrityError): - b.save() + # Patch the boolean field's default value. We give it a default + # value when defining the model to satisfy the check tests + # #20895. + boolean_field = BooleanModel._meta.get_field('bfield') + self.assertTrue(boolean_field.has_default()) + old_default = boolean_field.default + try: + boolean_field.default = NOT_PROVIDED + # check patch was succcessful + self.assertFalse(boolean_field.has_default()) + b = BooleanModel() + self.assertIsNone(b.bfield) + with self.assertRaises(IntegrityError): + b.save() + finally: + boolean_field.default = old_default + nb = NullBooleanModel() self.assertIsNone(nb.nbfield) nb.save() # no error diff --git a/tests/model_formsets/models.py b/tests/model_formsets/models.py index ae152448ab..adeb3455a4 100644 --- a/tests/model_formsets/models.py +++ b/tests/model_formsets/models.py @@ -117,7 +117,7 @@ class OwnerProfile(models.Model): @python_2_unicode_compatible class Restaurant(Place): - serves_pizza = models.BooleanField() + serves_pizza = models.BooleanField(default=False) def __str__(self): return self.name @@ -141,11 +141,11 @@ class Price(models.Model): unique_together = (('price', 'quantity'),) class MexicanRestaurant(Restaurant): - serves_tacos = models.BooleanField() + serves_tacos = models.BooleanField(default=False) class ClassyMexicanRestaurant(MexicanRestaurant): restaurant = models.OneToOneField(MexicanRestaurant, parent_link=True, primary_key=True) - tacos_are_yummy = models.BooleanField() + tacos_are_yummy = models.BooleanField(default=False) # models for testing unique_together validation when a fk is involved and # using inlineformset_factory. diff --git a/tests/model_inheritance/models.py b/tests/model_inheritance/models.py index 2101f394f7..106645d23c 100644 --- a/tests/model_inheritance/models.py +++ b/tests/model_inheritance/models.py @@ -63,7 +63,7 @@ class Attachment(models.Model): return self.content class Comment(Attachment): - is_spam = models.BooleanField() + is_spam = models.BooleanField(default=False) class Link(Attachment): url = models.URLField() @@ -96,8 +96,8 @@ class Rating(models.Model): @python_2_unicode_compatible class Restaurant(Place, Rating): - serves_hot_dogs = models.BooleanField() - serves_pizza = models.BooleanField() + serves_hot_dogs = models.BooleanField(default=False) + serves_pizza = models.BooleanField(default=False) chef = models.ForeignKey(Chef, null=True, blank=True) class Meta(Rating.Meta): @@ -108,7 +108,7 @@ class Restaurant(Place, Rating): @python_2_unicode_compatible class ItalianRestaurant(Restaurant): - serves_gnocchi = models.BooleanField() + serves_gnocchi = models.BooleanField(default=False) def __str__(self): return "%s the italian restaurant" % self.name diff --git a/tests/model_inheritance_regress/models.py b/tests/model_inheritance_regress/models.py index 0f45a2cb3f..bf076a47c6 100644 --- a/tests/model_inheritance_regress/models.py +++ b/tests/model_inheritance_regress/models.py @@ -18,15 +18,15 @@ class Place(models.Model): @python_2_unicode_compatible class Restaurant(Place): - serves_hot_dogs = models.BooleanField() - serves_pizza = models.BooleanField() + serves_hot_dogs = models.BooleanField(default=False) + serves_pizza = models.BooleanField(default=False) def __str__(self): return "%s the restaurant" % self.name @python_2_unicode_compatible class ItalianRestaurant(Restaurant): - serves_gnocchi = models.BooleanField() + serves_gnocchi = models.BooleanField(default=False) def __str__(self): return "%s the italian restaurant" % self.name @@ -184,7 +184,7 @@ class Station(SearchableLocation): class BusStation(Station): bus_routes = models.CommaSeparatedIntegerField(max_length=128) - inbound = models.BooleanField() + inbound = models.BooleanField(default=False) class TrainStation(Station): zone = models.IntegerField() diff --git a/tests/model_inheritance_select_related/models.py b/tests/model_inheritance_select_related/models.py index 6b28772620..46c67cf07d 100644 --- a/tests/model_inheritance_select_related/models.py +++ b/tests/model_inheritance_select_related/models.py @@ -20,8 +20,8 @@ class Place(models.Model): @python_2_unicode_compatible class Restaurant(Place): - serves_sushi = models.BooleanField() - serves_steak = models.BooleanField() + serves_sushi = models.BooleanField(default=False) + serves_steak = models.BooleanField(default=False) def __str__(self): return "%s the restaurant" % self.name diff --git a/tests/modeladmin/models.py b/tests/modeladmin/models.py index fdbcabd187..4789e35a43 100644 --- a/tests/modeladmin/models.py +++ b/tests/modeladmin/models.py @@ -32,7 +32,7 @@ class ValidationTestModel(models.Model): slug = models.SlugField() users = models.ManyToManyField(User) state = models.CharField(max_length=2, choices=(("CO", "Colorado"), ("WA", "Washington"))) - is_active = models.BooleanField() + is_active = models.BooleanField(default=False) pub_date = models.DateTimeField() band = models.ForeignKey(Band) no = models.IntegerField(verbose_name="Number", blank=True, null=True) # This field is intentionally 2 characters long. See #16080. diff --git a/tests/one_to_one/models.py b/tests/one_to_one/models.py index 9599496cb7..ff809be22d 100644 --- a/tests/one_to_one/models.py +++ b/tests/one_to_one/models.py @@ -22,8 +22,8 @@ class Place(models.Model): @python_2_unicode_compatible class Restaurant(models.Model): place = models.OneToOneField(Place, primary_key=True) - serves_hot_dogs = models.BooleanField() - serves_pizza = models.BooleanField() + serves_hot_dogs = models.BooleanField(default=False) + serves_pizza = models.BooleanField(default=False) def __str__(self): return "%s the restaurant" % self.place.name diff --git a/tests/one_to_one_regress/models.py b/tests/one_to_one_regress/models.py index 058baafc82..9b65edf2ab 100644 --- a/tests/one_to_one_regress/models.py +++ b/tests/one_to_one_regress/models.py @@ -15,8 +15,8 @@ class Place(models.Model): @python_2_unicode_compatible class Restaurant(models.Model): place = models.OneToOneField(Place) - serves_hot_dogs = models.BooleanField() - serves_pizza = models.BooleanField() + serves_hot_dogs = models.BooleanField(default=False) + serves_pizza = models.BooleanField(default=False) def __str__(self): return "%s the restaurant" % self.place.name diff --git a/tests/raw_query/models.py b/tests/raw_query/models.py index e7e221dc6e..33f754958e 100644 --- a/tests/raw_query/models.py +++ b/tests/raw_query/models.py @@ -18,7 +18,7 @@ class Author(models.Model): class Book(models.Model): title = models.CharField(max_length=255) author = models.ForeignKey(Author) - paperback = models.BooleanField() + paperback = models.BooleanField(default=False) opening_line = models.TextField() class Coffee(models.Model): diff --git a/tests/reverse_single_related/models.py b/tests/reverse_single_related/models.py index 30ba345120..5d53e04772 100644 --- a/tests/reverse_single_related/models.py +++ b/tests/reverse_single_related/models.py @@ -6,7 +6,7 @@ class SourceManager(models.Manager): return super(SourceManager, self).get_queryset().filter(is_public=True) class Source(models.Model): - is_public = models.BooleanField() + is_public = models.BooleanField(default=False) objects = SourceManager() class Item(models.Model): diff --git a/tests/serializers_regress/models.py b/tests/serializers_regress/models.py index 21a3448a8e..ab3d3063a4 100644 --- a/tests/serializers_regress/models.py +++ b/tests/serializers_regress/models.py @@ -16,7 +16,7 @@ class BinaryData(models.Model): data = models.BinaryField(null=True) class BooleanData(models.Model): - data = models.BooleanField() + data = models.BooleanField(default=False) class CharData(models.Model): data = models.CharField(max_length=30, null=True) @@ -166,7 +166,7 @@ class Intermediate(models.Model): # or all database backends. class BooleanPKData(models.Model): - data = models.BooleanField(primary_key=True) + data = models.BooleanField(primary_key=True, default=False) class CharPKData(models.Model): data = models.CharField(max_length=30, primary_key=True)