-
Notifications
You must be signed in to change notification settings - Fork 3
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
Upgrade to django-filter v2 #148
Conversation
54156f7
to
934631d
Compare
- Remove Backports Package - Use tempfile lib - Update tests - Update models, serializers and viewsets - Remove tox and travis python2 environments - Remove Future Package - Remove Future Module Dependency - Modify setup.cfg
a8e75ef
to
6a6f738
Compare
14d24f9
to
8a8e50b
Compare
@@ -1,10 +1,8 @@ | |||
# -*- coding: utf-8 -*- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you comment on what necessitated the changes in this module?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new django-filter
doesn't support passing lists to lookup_expr
like we used to do before. Needed to create a separate filter backend TaskOccurrenceFilter
to handle the filtering of task occurrences from the tasks endpoint.
@@ -25,7 +21,7 @@ def test_submission_model_str(self): | |||
submission = mommy.make( | |||
'tasking.Submission', | |||
task=cattle, | |||
_fill_optional=['user', 'comment', 'submission_time']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the effects of changing from comment
to comments
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment
field doesn't exist for the Submission model.
expected = 'Cow prices - {}'.format(cow_price.pk) | ||
self.assertEqual(expected, six.text_type(cow_price)) | ||
expected = f'Cow prices - {cow_price.pk}' | ||
self.assertEqual(expected, str(cow_price)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the str
necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test was testing the string representation of the object. I can replace this with a call to __str__()
though.
- Update Pipfile Lock - Update tests - Update TaskFilterSet to utilize LookUpChoiceFilter
8a8e50b
to
44f3481
Compare
Fix #136
Previous PR #138