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

Re-adding check for None to clean() (#80) #83

Merged
merged 3 commits into from
Jun 28, 2016

Conversation

NinjaDero
Copy link
Contributor

This is basically just the removed check for None and isinstance(queryset, QuerySet) back in place.
If value is None, it can not be iterated and throws an error when handling with pages, articles or any app using this module in Django CMS.

I also bumped the version, to make writing pip requirements easier, like:

django-sortedm2m!=1.3.0,>=1.2.2

to skip version 1.3.0.dev1.

@gregmuellegger
Copy link
Collaborator

Yes, that change makes sense. Sorry that the removal of this check slipped through the review. What do you think about testing for an iterable instead of a queryset? We actually don't require it to be a queryset.

@gregmuellegger gregmuellegger mentioned this pull request Jun 27, 2016
@NinjaDero
Copy link
Contributor Author

NinjaDero commented Jun 28, 2016

Sure, I just went with the approach of least damage and added the line close to what it was before.
I went with checking with hasattr() instead of iter(), because iter will fall back to __getitem__ if __iter__ is not present.
Is that alright? What are possible iterables that could fail?

@gregmuellegger gregmuellegger merged commit d2e9c7f into jazzband:master Jun 28, 2016
gregmuellegger added a commit that referenced this pull request Jun 28, 2016
@gregmuellegger
Copy link
Collaborator

Just released 1.3.2, containing your fix. Thanks for the effort!
https://pypi.python.org/pypi/django-sortedm2m

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.

2 participants