[5.4] add a "second local key" to HasManyThrough

[phroggyy]

Let's take the example in the docs, with countries, users, and posts. With the current implementation of HasManyThrough, we can specify the local key to use on countries (id), the foreign key on users (country_id), and the foreign key on the target model (user_id on posts). However, we can't specify the local key on the intermediary table. Consider the following scenario, where we keep a separate table for stripe accounts:

users:
id | email | password

billing_accounts:
id | user_id | stripe_customer_id | name | address_line_1 | address_line_2 | city | zip | country [`user_id` references `id` on `users`]

transactions:
id | customer_id | charge_id | amount | currency | status [`customer_id` references `stripe_customer_id` on `billing_accounts`]

In this case, if we wish to retrieve a user's transactions, we can't, because we can't specify that the key to join billing_accounts and transactions on should be billing_accounts.stripe_customer_id = transactions.customer_id. Instead, Laravel assumes that we wish to use the primary key of our intermediary, something that can limit users in queries like this.

[taylorotwell]

Let's get rid off those over-mocked tests and just write a simple integration test for this relationship type. You can create a new test file for it.

[phroggyy]

@taylorotwell done! I've recreated every test that was in the old unit test class too

[ludo237]

This change could create more flexible relationships and database schemas👍

[phroggyy]

@taylorotwell any update on this?

[taylorotwell]

I wonder if there is a better variable name than secondLocalKey. Maybe something more descriptive of what it is.

[phroggyy]

@taylorotwell my other suggestion would be intermediateLocalKey. That was my first thought, but the reasoning behind secondLocalKey is that $localKey matches with $firstKey and $secondLocalKey matches with $secondKey (in terms of what you join on). I thought that might be less mentally straining, but I'm not sure.

Another option would be $throughLocalKey

[phroggyy]

Once this is merged, I'll also PR to docs to clarify regarding the keys, I think they could use it anyway.

[taylorotwell]

@phroggyy i think this looks good but I need you to send it to master branch since it changes a method signature.