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.8] Add Eloquent HasOneThrough relationship #26057

Merged
merged 5 commits into from
Oct 11, 2018
Merged

[5.8] Add Eloquent HasOneThrough relationship #26057

merged 5 commits into from
Oct 11, 2018

Conversation

driesvints
Copy link
Member

This commit adds the Eloquent HasOneThrough relationship. This will allow for easy linking to deeper nested models and easy eager loading of them.

Many thanks to @frdteknikelektro and his original PR which really was most of the work: #25083

This commit adds the Eloquent HasOneThrough relationship. This will allow for easy linking to deeper nested models and easy eager loading of them.

Many thanks to @frdteknikelektro and his original PR which really was most of the work: #25083
*
* @return void
*/
public function tearDown()
Copy link
Contributor

@staudenmeir staudenmeir Oct 10, 2018

Choose a reason for hiding this comment

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

That's not necessary: The in-memory database "destroys" itself.

Copy link
Member Author

Choose a reason for hiding this comment

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

Going to stick to this for now since the other tests have it as well. I see what you're saying but it's best to stick to the convention of having them if all the other tests do as well.

@staudenmeir
Copy link
Contributor

Should we create a HasOneOrManyThrough parent class (similar to HasOneOrMany and MorphOneOrMany)?

This rename now covers both use cases for one or many through relationships. If people were using this key, this might break some things but imo they shouldn't rely on it.
@driesvints
Copy link
Member Author

@staudenmeir not sure if it has that much benefit here tbh.

@freekmurze
Copy link
Contributor

This could already be added to L5.7 right?

@driesvints
Copy link
Member Author

@freekmurze all other prs for this were requested to be sent to master so sending this to master as well. Since this PR renames the through key, I think it's the best thing to do.

@taylorotwell taylorotwell merged commit 959b547 into laravel:master Oct 11, 2018
@driesvints driesvints deleted the has-one-through branch October 11, 2018 15:43
@frdteknikelektro
Copy link

frdteknikelektro commented Oct 12, 2018

@driesvints Thanks for the credit, i'm glad that it has been merged! awesome!

@JeffreyDavidson
Copy link

Can an example case be provided to understand when the right time to use it would be?

@driesvints
Copy link
Member Author

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.

6 participants