Skip to content

Conversation

santiavenda2
Copy link
Contributor

@santiavenda2 santiavenda2 commented Dec 28, 2018

Fixes #537

Description of the Change

Fix AutoPrefetchMixin when include have a one to one relation

Checklist

  • PR only contains one change (considered splitting up PR)
  • unit-test added
  • documentation updated
  • changelog entry added to CHANGELOG.md
  • author name in AUTHORS

@codecov
Copy link

codecov bot commented Dec 28, 2018

Codecov Report

Merging #536 into master will increase coverage by 0.01%.
The diff coverage is 96.15%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #536      +/-   ##
==========================================
+ Coverage   94.33%   94.34%   +0.01%     
==========================================
  Files          60       61       +1     
  Lines        3725     3750      +25     
==========================================
+ Hits         3514     3538      +24     
- Misses        211      212       +1
Impacted Files Coverage Δ
example/tests/integration/test_includes.py 100% <100%> (ø) ⬆️
rest_framework_json_api/views.py 94.34% <100%> (+0.04%) ⬆️
example/serializers.py 100% <100%> (ø) ⬆️
example/migrations/0006_auto_20181228_0752.py 100% <100%> (ø)
example/models.py 90.65% <87.5%> (-0.26%) ⬇️

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 cc64dad...d8ce701. Read the comment docs.

@codecov
Copy link

codecov bot commented Dec 28, 2018

Codecov Report

Merging #536 into master will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #536      +/-   ##
==========================================
+ Coverage   95.47%   95.51%   +0.04%     
==========================================
  Files          54       55       +1     
  Lines        2804     2829      +25     
==========================================
+ Hits         2677     2702      +25     
  Misses        127      127
Impacted Files Coverage Δ
example/tests/test_views.py 100% <ø> (ø) ⬆️
example/tests/conftest.py 100% <100%> (ø) ⬆️
example/models.py 100% <100%> (ø) ⬆️
example/tests/integration/test_includes.py 100% <100%> (ø) ⬆️
example/factories.py 98.76% <100%> (+0.09%) ⬆️
example/migrations/0006_auto_20181228_0752.py 100% <100%> (ø)

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 11e0edd...1409954. Read the comment docs.

Copy link
Member

@sliverc sliverc left a comment

Choose a reason for hiding this comment

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

Thanks for working on this. Fix looks good to me, there is just one question concerning test.

assert len(response.json()['data']) == len(multiple_entries), (
'Incorrect entry count'
)
expected_include_types = ['authorBios', 'authorBios', 'authors', 'authors']
Copy link
Member

Choose a reason for hiding this comment

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

when authors.bio.metadata is requested to be included shouldn't there be a authorsBioMetadata type be included as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added AuthorBiosMetadata on Serializarers. I change the tests to reflect that.

@sliverc sliverc changed the title Feature/fix autoprefetch mixin Avoid exception in AutoPrefetchMixin when including a reverse one to one relation Jan 4, 2019
@sliverc sliverc merged commit 55e8def into django-json-api:master Jan 4, 2019
@sliverc
Copy link
Member

sliverc commented Jan 4, 2019

@santiavenda2
Merged. Thanks for your contribution.

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