-
Notifications
You must be signed in to change notification settings - Fork 299
Reverse the included relation name formatting #204
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
Conversation
c67445b to
29c5b3d
Compare
I have yet to figure out how the unit testing works...
As far as the tests go python runtests.py should get you up and running. Is there something specific you're wondering about? You can add a function to this file https://github.com/django-json-api/django-rest-framework-json-api/blob/develop/example/tests/integration/test_includes.py and it should just be ran with the tests. Name it similar to the ones in there and if you add single_entry as function parameters pytest will know to create mocks for you.
29c5b3d to
c51ecb8
Compare
I think for it to test the existing code I'd have to rename all the 'comments' and 'authors' to singular.. which has an effect on the "relationships" too.. not sure if it's OK to do that?
edit Ok I found a (hacky) way to at least test the singularize :)
@jesushernandez
jesushernandez
Feb 16, 2016
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.
pytest-django provides a good way of doing this: http://pytest-django.readthedocs.org/en/latest/helpers.html#settings
Quote from #257:
It seems that #204 is taking things further by also reversing pluralization and adding a setting to specify which format the keys should be reverted to. In my opinion, the effort of that PR should be expanded to the other areas that perform this behavior, namely parsing. This could then be considered a stopgap until that larger development is ready.
I suggest this PR be expanded to also apply the reversal to attribute/relationship parsing here.
Hi, @schtibe. Thanks for the contribution! This branch is now in conflict in the main branch. Are you interested in fixing this up or should we close out this PR? I'm stepping up to maintain the PR queue and get DRF JA changes flowing again, but I can't merge conflicted stuff.
If this is still valuable, resolving the merge conflicts will give me a chance to review it well. If this is no longer important to you, closing the PR is right next step.
Please let me know what you'd like to do. Thanks!
Hi @mblayman. I'd like to see if I can fix the the conflict, but I'm currently not even sure anymore if the issue adressed in this PR really complies with the JSON API standard. Please give me some time as I cannot check it right away.
Ok I don't see the possibility any time soon to investigate further.
will try to investigate this in near future
@sliverc do you see the approach of this PR right?
See #203 for more information