Skip to content

Fixes error when using EagerLoad on a relationship #1000

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
May 20, 2021

Conversation

bart-degreed
Copy link
Contributor

Fixes #988.

I intentionally haven't added tests, because [EagerLoad] was never meant to be combined with relationship attributes. This PR does not make that a supported feature. Instead this fix is solely to unblock a user that ran into a problem while trying to do so. In that process, additional problems may surface and we'll see how to deal with them on a case-by-case basis.

A few tests exist in unmerged #990, which was used to reproduce the problem.

@bart-degreed bart-degreed requested a review from maurei May 18, 2021 13:55
@codecov
Copy link

codecov bot commented May 18, 2021

Codecov Report

Merging #1000 (024f1c7) into master (e34709d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 024f1c7 differs from pull request most recent head 70abcd5. Consider uploading reports for the commit 70abcd5 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1000   +/-   ##
=======================================
  Coverage   91.38%   91.39%           
=======================================
  Files         283      283           
  Lines        7537     7539    +2     
=======================================
+ Hits         6888     6890    +2     
  Misses        649      649           
Impacted Files Coverage Δ
.../Internal/QueryableBuilding/SelectClauseBuilder.cs 94.28% <100.00%> (+0.11%) ⬆️

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 e34709d...70abcd5. Read the comment docs.

@bart-degreed bart-degreed merged commit 487538b into master May 20, 2021
@bart-degreed bart-degreed deleted the fix-eagerloads branch May 20, 2021 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Cannot sort resource having a relationship with EagerLoadAttribute
2 participants