Skip to content

Add support for to_field_name to SortedMultipleChoiceField#76

Merged
gregmuellegger merged 1 commit intojazzband:masterfrom
DeskConnect:master
May 24, 2016
Merged

Add support for to_field_name to SortedMultipleChoiceField#76
gregmuellegger merged 1 commit intojazzband:masterfrom
DeskConnect:master

Conversation

@conradev
Copy link
Contributor

The current implementation depends on in_bulk, which requires value to be the pk, when it can actually be any unique field thanks to the to_field_name attribute.

The super implementation handles filtering the queryset, so clean only needs to order the objects. This implementation is partially inspired by how the ModelMultipleChoiceField implements it.

@gregmuellegger
Copy link
Collaborator

Cool that makes sense. Thanks for taking the effort to contribute this.

I see two things missing here before I can merge this:

  • The tests are failing for Python 2.6, could you change the dict comprehension to use the old style dict((key, value) ...) one?
  • There are unittests missing for this feature. It would be nice to have some tests here so that can be sure enough that we don't break the support for to_field_name in the future.

@conradev
Copy link
Contributor Author

I went ahead and made those two changes, let me know if I need to make any more! 👍

@gregmuellegger gregmuellegger merged commit 930548d into jazzband:master May 24, 2016
@gregmuellegger
Copy link
Collaborator

👍 great! Thanks for the contribution, it's merged.

gregmuellegger added a commit that referenced this pull request May 24, 2016
@gregmuellegger
Copy link
Collaborator

I just released 1.3.0, containing your changes: https://pypi.python.org/pypi/django-sortedm2m/1.3.0

@conradev
Copy link
Contributor Author

Awesome, thanks! 🎉

@vqw
Copy link

vqw commented May 29, 2016

@conradev @gregmuellegger
I have the same issue as @czpython in DjangoCMS which drives me crazy. Reverting to previous version.

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.

4 participants