laravel / ideas

Issues board used for Laravel internals discussions.
938 stars 28 forks source link

Eloquent Relationship: Has many through intermediate primary key name. #418

Open wesdean opened 7 years ago

wesdean commented 7 years ago

I am trying to create a relationship through an intermediate table but the hasManyThrough function does not accommodate my table structure.

libraries = ['id', 'name'] librarians = ['id', 'library_id', 'user_id'] users = ['id', 'name']

This is the query currently generated. select * from "users" inner join "librarians" on "librarians"."id" = "users"."id" where "librarians"."library_id" = ? and "users"."deleted_at" is null

This is the query I need. select * from "users" inner join "librarians" on "librarians"."user_id" = "users"."id" where "librarians"."library_id" = ? and "users"."deleted_at" is null

I have made a small change to the HasRelationships trait that allows me to customize the intermediate model used for the relationship.

In public function hasManyThrough I changed $through = new $through to if (is_string($through)) { $through = new $through; }

This change allows me use a customized model for the relationship. $librarian = new Librarian(); $librarian->setKeyName('user_id'); return $this->hasManyThrough( User::class, $librarian, 'library_id', 'id', 'id' );

It would be great if this change or something similar could be made so that I don't have to hack the framework to get this relationship to work. Thanks.

asantibanez commented 6 years ago

Hi! I just stumbled with this issue and I believe it is a valid use case. Sometimes the JOIN needs to be using different column names instead of the defaults. I don't like the setKeyName() or is_string() hack, I think it would better to allow defining those column names as extra parameters in the hasManyThrough method when defining the relationship.

I have the following tables

user
- id

app_notification
- id

app_notification_recipients
- recipient_id (id on users)

I would like to define the following relationship:

In users

public function appNotifications()
{
    return $this->hasManyThrough(AppNotification::class, AppNotificationRecipient::class, 'recipient_id');
}

Which returns the following SQL

select * from "app_notifications" inner join "app_notification_recipients" on "app_notification_recipients"."id" = "app_notifications"."app_notification_recipient_id" where "app_notification_recipients"."recipient_id" = ?

instead of the desired on

select * from "app_notifications" inner join "app_notification_recipients" on "app_notification_recipients"."app_notification_id" = "app_notifications"."id" where "app_notification_recipients"."recipient_id" = ?

Messing around with the extra parameters of hasManyThrough does not do the trick either.

The only required change would be to allow modification of the JOIN keys.

Have you run into this @themsaid ? Thanks in advance.

staudenmeir commented 6 years ago

@wesdean In your case, a BelongsToMany relationship is the right choice:

public function users()
{
    return $this->belongsToMany(User::class, 'librarians');
}

@asantibanez The same goes for your relationship:

public function appNotifications()
{
    return $this->belongsToMany(AppNotification::class, 'app_notification_recipients', 'recipient_id');
}