Skip to content

only include 'allow_blank' on supported fields #3012

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.

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

Merged
merged 2 commits into from
Jun 24, 2015

Conversation

jannon
Copy link

@jannon jannon commented Jun 5, 2015

When I actually looked at fixing it for myself, it seemed like a pretty quick fix so I just went ahead and made a PR. The issue is described in #3011

@@ -102,7 +102,7 @@ def get_field_kwargs(field_name, model_field):
if model_field.null and not isinstance(model_field, models.NullBooleanField):
kwargs['allow_null'] = True

if model_field.blank:
if model_field.blank and isinstance(model_field, models.CharField):
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest having a look through the history for this line first.
Have we ever had different behavior in the past?
Are there any other built-in fields that don't subclass CharField but that do take an allow_blank argument?

@jannon jannon changed the title only include 'allow_blank' on CharField only include 'allow_blank' on supported fields Jun 16, 2015
@tomchristie
Copy link
Member

This feels like the right fix to me.
Any objections to merging this and then closing the alternate #3044?

@tomchristie tomchristie added this to the 3.1.4 Release milestone Jun 23, 2015
tomchristie added a commit that referenced this pull request Jun 24, 2015
only include 'allow_blank' on supported fields
@tomchristie tomchristie merged commit 8d4c96e into encode:master Jun 24, 2015
@jannon jannon deleted the fix-allow-blank-mapping branch July 15, 2015 21:33
@tomchristie tomchristie modified the milestones: 3.1.4 Release, 3.2.0 Release Jul 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants