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

Update to support Django 1.9 #59

Merged
merged 1 commit into from
Aug 13, 2015
Merged

Conversation

appden
Copy link
Contributor

@appden appden commented Aug 6, 2015

Django 1.9 removes ReverseManyRelatedObjectsDescriptor in favor of using ManyRelatedObjectsDescriptor with reverse argument passed into it (though the meaning of reverse has been reversed, hah).

It also adds a set() method to RelatedManager that handles setting all the new related objects. It tries to be more efficient by only adding and removing the ones necessary instead of clearing them all and adding them again. The old behavior is maintained by passing the clear keyword argument, which is what we now do.

Django 1.9 removes ReverseManyRelatedObjectsDescriptor in favor of using ManyRelatedObjectsDescriptor with `reverse` argument passed into it (though the meaning of reverse has been reversed, hah).

It also adds a set() method to RelatedManager that handles setting all the new related objects. It tries to be more efficient by only adding and removing the ones necessary instead of clearing them all and adding them again. The old behavior is maintained by passing the `clear` keyword argument, which is what we now do.
return create_sorted_many_related_manager(
self.rel.model._default_manager.__class__,
self.rel,
False # This is the new `reverse` argument (which ironically should be False)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we use keyword arguments here, so that it's clear (even without the comment) what the arguments stand for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Considered it, but thought it would be a bit dirtier because passing that up to Django's create_many_related_manager would require some more manipulation inside our create_sorted_many_related_manager function since it's a new argument they added in 1.9. They didn't make it a keyword argument, so I figured since our function really just wraps theirs, that we should treat it the same.

@gregmuellegger
Copy link
Collaborator

Awesome PR, thanks! I'll get this merged after you get back on the inline comment 👍 good work!

@gregmuellegger gregmuellegger merged commit 3328131 into jazzband:master Aug 13, 2015
gregmuellegger added a commit that referenced this pull request Aug 13, 2015
@gregmuellegger
Copy link
Collaborator

Meeeeerged :) A big cheers for the good work! 🍻

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