Skip to content
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

[5.7] Allow MorphTo relations to specify select columns #26483

Closed
wants to merge 2 commits into from
Closed

[5.7] Allow MorphTo relations to specify select columns #26483

wants to merge 2 commits into from

Conversation

fschalkwijk
Copy link

When eager loading relations with the with function, one can alter the query used to retrieve the related models with a closure. With normal relations you can select the columns which will be retrieved from the database, but with MorphTo relationships you can not.

My small change only allows column selection in eager loading MorphTo relations.

@jmarcher
Copy link
Contributor

The tests are failing. Can you add tests?
I think this should target master and not the current release.

@driesvints driesvints changed the title Allow MorphTo relations to specify select columns [5.7] Allow MorphTo relations to specify select columns Nov 12, 2018
@driesvints
Copy link
Member

@jmarcher tests are currently failing on multiple branches for a different reason. Not sure what's going on.

Copy link
Member

@driesvints driesvints left a comment

Choose a reason for hiding this comment

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

Can you fix the StyleCI failures?

@driesvints driesvints dismissed their stale review November 12, 2018 10:38

Nvm, did it myself quickly.

@fschalkwijk
Copy link
Author

Thanks for fixing my style issue. I thought StyleCI automaticly fixed these issues.

I'm sorry but I do not know how to write a test for this.

@staudenmeir
Copy link
Contributor

#25662 should have already fixed that. Can you provide an example that didn't work before this PR?

@fschalkwijk
Copy link
Author

This does work indeed. I forgot to update. I'm sorry for wasting your time.

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.

None yet

4 participants