Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Conditionally support NullBooleanField #250

Merged
merged 2 commits into from Oct 29, 2021
Merged

Conditionally support NullBooleanField #250

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2e47a15
Conditionally support NullBooleanField
ashiazed Oct 23, 2021
35dc478
Merge branch 'main' into null_boolean_fields
berinhard Oct 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

### Changed
- Fix a simple typo in `bulk_create` disclaimer in docs
- Conditionally support NullBooleanField, it's under deprecation and will be removed in Django 4.0 [PR #25](https://github.com/model-bakers/model_bakery/pull/250)

### Removed

Expand Down
11 changes: 9 additions & 2 deletions model_bakery/generators.py
View file
Open in desktop
Expand Up @@ -20,7 +20,6 @@
IntegerField,
IPAddressField,
ManyToManyField,
NullBooleanField,
OneToOneField,
PositiveIntegerField,
PositiveSmallIntegerField,
Expand Down Expand Up @@ -87,6 +86,13 @@
CIEmailField = None
CITextField = None

try:
# Deprecated since Django 3.1
from django.db.models import NullBooleanField
except ImportError:
NullBooleanField = None


try:
# PostgreSQL-specific fields (only available when psycopg2 is installed)
from django.contrib.postgres.fields.ranges import (
Expand Down Expand Up @@ -124,7 +130,6 @@ def gen_integer():
OneToOneField: random_gen.gen_related,
ManyToManyField: random_gen.gen_m2m,
BooleanField: random_gen.gen_boolean,
NullBooleanField: random_gen.gen_boolean,
IntegerField: _make_integer_gen_by_range(IntegerField),
BigIntegerField: _make_integer_gen_by_range(BigIntegerField),
SmallIntegerField: _make_integer_gen_by_range(SmallIntegerField),
Expand Down Expand Up @@ -185,6 +190,8 @@ def gen_integer():
default_mapping[DateRangeField] = random_gen.gen_date_range
if DateTimeRangeField:
default_mapping[DateTimeRangeField] = random_gen.gen_datetime_range
if NullBooleanField:
default_mapping[NullBooleanField] = random_gen.gen_boolean


# Add GIS fields
Expand Down
2 changes: 1 addition & 1 deletion tests/generic/models.py
View file
Open in desktop
Expand Up @@ -228,7 +228,7 @@ def save(self, *args, **kwargs):

class Classroom(models.Model):
students = models.ManyToManyField(Person, null=True)
active = models.NullBooleanField()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@berinhard Since we would still technically support NullBooleanField until model_bakery no longer supports Django < 4.0, would it be okay if there were no test models with this field type? Or should there be conditional test models to make sure everything still works?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to say yes but our test requirements use latest Django and as soon as we are on 4.0 we won't be able to import NullBooleanField. Should we add this as a custom field and include it that way?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need such type of safety checks. In the past, with other deprecated fields, we did the same strategy as @ashiazed did: use ImportError and don't worry about it until bakery also drops support for that field.

active = models.BooleanField(null=True)


class Store(models.Model):
Expand Down