Conversation
| $relation = $this->belongsToMany( | ||
| $relatedClass, | ||
| $definition['table'] ?? null, | ||
| $definition['table'] ?? $definition['pivotModel'] ?? null, |
There was a problem hiding this comment.
Arguably, a pivot model definition should be considered more authoritative than a table definition, on the off-chance that someone specifies both in the config.
| $definition['table'] ?? $definition['pivotModel'] ?? null, | |
| $definition['pivotModel'] ?? $definition['table'] ?? null, |
There was a problem hiding this comment.
There's nothing that ensures at this point that the pivot model defines the table...
There was a problem hiding this comment.
@mjauvin not sure what you mean? If you specify a pivot model, Laravel will determine the table of that model in the standard way (either the $table property of the model, or conventionally using the model's name).
There was a problem hiding this comment.
Yes, but there's no way to know at that point if the pivot actually defines a table name.
If it doesn't, laravel's way of guessing the table name is not compatible with ours
There was a problem hiding this comment.
What's an example of this not working?
We already have a call to the using method underneath where you've made your edit, which should switch the relation to use the pivot's table.
There was a problem hiding this comment.
@bennothommo I changed the order as you suggested, and added a [failing] test to demonstrate my point.
There was a problem hiding this comment.
@mjauvin sorry, but I'm still not understanding how this is an issue?
Your failing test is creating a BTM relation called pivot_without_table and using the CustomPivotWithoutTable class as a custom pivot without defining a table, and then expecting the table name to be custom_pivot_table, when by the normal automatic table name logic, it should be custom_pivot_without_table which is correctly being returned by the getTable() method.
It should be said that when defining a BTM relation in Laravel, you define either the table OR the pivot, you do not define both. If your pivot needs to use a table name that doesn't match the convention, then you specify the table name in the pivot.
EDIT: Never mind, it was buried deep in Laravel but evidently I'm wrong. The relation can still set the table name for a custom pivot.
There was a problem hiding this comment.
I've updated the test to show that it passes with the expected table name.
|
This pull request will be closed and archived in 3 days, as there has been no activity in the last 60 days. |
|
This pull request will be closed and archived in 3 days, as there has been no activity in the last 60 days. |
If using a custom pivot model, the table name may be omitted in the relation definition if it is set in the custom pivot model itself.
Laravel will resolve the table name to the Pivot model table.
(ref. https://github.com/laravel/framework/blob/12.x/src/Illuminate/Database/Eloquent/Relations/BelongsToMany.php#L184-L201)