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

[6.x] Fix belongsTo relation when using multiple database connections #30185

Closed
wants to merge 1 commit into from
Closed

[6.x] Fix belongsTo relation when using multiple database connections #30185

wants to merge 1 commit into from

Conversation

bertheyman
Copy link

@bertheyman bertheyman commented Oct 4, 2019

The problem
Could previously cause errors where the owner belonged to another database and the child to the default one.

Fix
This PR tries to fix that by using the correct relational connection.
Feel free to provide feedback or tweak whenever needed.
Possibly related to #29935.

This pull request was built together with @jordyvanderhaegen.

Update
I see the tests fail on the use of config.
Two possible options:

  1. Removing the setConnection, as getConnection will fall back to the right one when a model has no custom connection. Might cause elsewhere though
  2. Get the config another way.

Anyone who could point me in the right direction would be a great help!

Could previously cause errors where the owner belonged to another database and the child to the default one.
@driesvints driesvints changed the title Fix belongsTo relation when using multiple database connections [6.x] Fix belongsTo relation when using multiple database connections Oct 4, 2019
@taylorotwell
Copy link
Member

We can't call a global config helper in the middle of an Eloquent method.

@bertheyman
Copy link
Author

The issue still persists, so I've tried different solutions, but got stuck with testing.

If anyone would like to give me a hand fixing this issue, feel free!

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