Skip to content

Fix for resolving source when accessing sub-attr eg b.c #375

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

Merged
merged 1 commit into from
Nov 15, 2017

Conversation

cosmosgenius
Copy link
Contributor

@cosmosgenius cosmosgenius commented Oct 10, 2017

plan = PlanSerializer(source='subscription.plan')

plan is a field of another serializer which needs to be resolved using obj.subscription.plan property. It works for standard DRF JSON response but fails in drf-json-api.

This PR fixes that issue by replacing getattr with operator.attrgetter which is able to resolve the sub-attr access.

@cosmosgenius
Copy link
Contributor Author

ping!. Is there anything else which needs to be done to merge this PR?

@mblayman
Copy link
Collaborator

Hi, @cosmosgenius, thanks for your patience. Anyone working on this project is purely a volunteer so I guess we've all been fairly busy lately. Through no fault of your own, the tests are all failing. I believe this is because of a dependency problem that was resolved recently. Could you merge the latest develop into this branch? The project policy is to merge PRs only when they pass all tests. Once this branch is passing, I'm happy to merge as the fix content looks good. Thanks! 👍

@codecov-io
Copy link

Codecov Report

Merging #375 into develop will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #375      +/-   ##
===========================================
+ Coverage    93.05%   93.06%   +<.01%     
===========================================
  Files           51       51              
  Lines         2665     2666       +1     
===========================================
+ Hits          2480     2481       +1     
  Misses         185      185
Impacted Files Coverage Δ
rest_framework_json_api/utils.py 90.8% <100%> (+0.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 20d98cf...72d4588. Read the comment docs.

@cosmosgenius
Copy link
Contributor Author

@mblayman no issues. Just checking up :). I have rebased the branch with the latest develop and fixed an isort issue. Let me know if anything else is required.

@mblayman
Copy link
Collaborator

Awesome. Thanks, @cosmosgenius!

@mblayman mblayman merged commit a953a03 into django-json-api:develop Nov 15, 2017
@cosmosgenius cosmosgenius deleted the revolve-sub-attr branch November 15, 2017 21:24
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.

3 participants