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

Expand valid values for BooleanWidget #1071

Merged
merged 5 commits into from May 28, 2020

Conversation

MinchinWeb
Copy link
Contributor

Deals better with NULL/None values, which are now (Django 2.2) permitted in the regular fields.BooleanField (fields.NullBooleanField has been deprecated). Importing a field with TRUE/FALSE/NULL was otherwise failing (via turning all the NULL's into FALSE's).

Problem

The Boolean widget (which is used to diff an re-import) wasn't dealing with Null/None/Empty values, and instead casting them as "0" (i.e. False)

Solution

Explicitly add "" (an empty string) and None as "None values".

Acceptance Criteria

Have you written tests? Have you included screenshots of your changes if applicable?
Did you document your changes?

No additional tests. No screenshot. I've added it to the Changelog (under v2.0.2).

Deals better with NULL/None values, which are now (Django 2.2) permitted in the regular fields.BooleanField (fields.NullBooleanField has been deprecated). Importing a field with TRUE/FALSE/NULL was otherwise failing (and turning all the NULL's into FALSE's).
@coveralls
Copy link

coveralls commented Jan 24, 2020

Coverage Status

Coverage increased (+0.02%) to 96.348% when pulling 4437761 on MinchinWeb:boolean-widget into 3d7c943 on django-import-export:master.

Copy link
Member

@andrewgy8 andrewgy8 left a comment

Choose a reason for hiding this comment

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

Thanks @MinchinWeb for the PR! Do you mind adding some docs and tests for this change?

@MinchinWeb
Copy link
Contributor Author

@andrewgy8 : docs and tests added!

Copy link
Contributor

@manelclos manelclos left a comment

Choose a reason for hiding this comment

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

LGTM!

@MinchinWeb
Copy link
Contributor Author

Is anything holding this up?

@andrewgy8
Copy link
Member

@MinchinWeb There are some conflicts that need to be resolved.

@andrewgy8 andrewgy8 mentioned this pull request May 28, 2020
Copy link
Member

@andrewgy8 andrewgy8 left a comment

Choose a reason for hiding this comment

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

Fixes #1135

@andrewgy8 andrewgy8 changed the title Improvements to BooleanWidget Exapnd valid values for BooleanWidget May 28, 2020
@andrewgy8 andrewgy8 merged commit 981b9cc into django-import-export:master May 28, 2020
@andrewgy8
Copy link
Member

Thanks @MinchinWeb !

@andrewgy8
Copy link
Member

Ill try to have this in the next minor release asap.

@MinchinWeb MinchinWeb deleted the boolean-widget branch May 28, 2020 18:48
@manelclos manelclos changed the title Exapnd valid values for BooleanWidget Expand valid values for BooleanWidget May 29, 2020
ZuluPro pushed a commit to ZuluPro/django-import-export that referenced this pull request Dec 23, 2020
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.

None yet

4 participants