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.

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

Fixes #294 #295

Merged
merged 5 commits into from
Jun 17, 2017
Merged

Fixes #294 #295

merged 5 commits into from
Jun 17, 2017

Conversation

peterfarrell
Copy link
Contributor

@peterfarrell peterfarrell commented Jun 14, 2017

Fixes form validation crash for USZipCodeField when field is null=True, blank=True. See #294

Do not call strip on a None.

All Changes

  • Add an entry to the docs/changelog.rst describing the change.

  • Add an entry for your name in the docs/authors.rst file if it's not
    already there.

  • Adjust your imports to a standard form by running this command:

    `isort --recursive --line-width 120 localflavor tests`
    

@codecov-io
Copy link

codecov-io commented Jun 14, 2017

Codecov Report

Merging #295 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #295      +/-   ##
==========================================
+ Coverage   96.01%   96.01%   +<.01%     
==========================================
  Files         150      150              
  Lines        4165     4167       +2     
  Branches      566      567       +1     
==========================================
+ Hits         3999     4001       +2     
  Misses        107      107              
  Partials       59       59
Impacted Files Coverage Δ
localflavor/us/forms.py 91.3% <100%> (+0.25%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 34c234d...d4c3cd9. Read the comment docs.

@benkonrath
Copy link
Member

Thanks for the fix. Can you add a failing test that's fixed by this change? It should be enough to test the form field with empty_value set to None instead of the default empty string.
https://docs.djangoproject.com/en/1.11/ref/forms/fields/#charfield

@peterfarrell
Copy link
Contributor Author

@benkonrath Added a test. It's a little hackish because Django < 1.11 don't support passing in empty_value to the field itself as it throws an unexpected argument error so instead of banging my head against the wall I went with the route that is in the test. Eventually it can be removed after support for Django < 1.11 happens.

Is there anything else you need to get this pulled in?

@benkonrath benkonrath self-requested a review June 17, 2017 15:19
@benkonrath benkonrath merged commit 7b92156 into django:master Jun 17, 2017
@benkonrath
Copy link
Member

Seems like a good solution. I'll make a 1.5.1 release with this fix soon.

benkonrath pushed a commit that referenced this pull request Jun 17, 2017
USZipCodeField with null=True and Blank=Ture caused a crash on Django 1.11.

Fixes #294
@peterfarrell peterfarrell deleted the fix_294 branch June 19, 2017 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants