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.4] Custom Intermediate (Pivot) Models not instantiable #17770

Closed
JuanDMeGon opened this issue Feb 5, 2017 · 36 comments
Closed

[5.4] Custom Intermediate (Pivot) Models not instantiable #17770

JuanDMeGon opened this issue Feb 5, 2017 · 36 comments

Comments

@JuanDMeGon
Copy link
Contributor

  • Laravel Version: 5.4.9
  • PHP Version: 7.1
  • Database Driver & Version: MySQL 5.7.16

Description:

When trying to use a custom intermediate model (a model that extends from Illuminate\Database\Eloquent\Relations\Pivot) as a regular model (Pivot extends from Model), it fails, because is not able to create an instance.

Too few arguments to function Illuminate\Database\Eloquent\Relations\Pivot::__construct(), 0 passed

I checked the commit but did not found anything would help on this: 4439576

Steps To Reproduce:

Create a model that extends from Pivot instead of Model directly:

...
use Illuminate\Database\Eloquent\Relations\Pivot;
...
class CustomIntermediate extends Pivot
{
    ......
}

Then try to use the model for some specific functions like all(), with(), query(), etc.

Route::get('test', function() {
    return CustomIntermediate::all();
});
@GrahamCampbell
Copy link
Member

Please post your entire custom class code.

@JuanDMeGon
Copy link
Contributor Author

Hello @GrahamCampbell,
I just checked it again on a fresh Laravel project: composer create-project laravel/laravel testing 5.4

Then, created a Test model: php artisan make:model Test

Modified the Test to extends from pivot:

<?php

namespace App;

use Illuminate\Database\Eloquent\Relations\Pivot;

class Test extends Pivot
{
    //
}

Then on the web.php routes issued this:

Route::get('/', function () {
    return App\Test::all();
});

Then going to my browser obtained this:

Type error: Argument 1 passed to Illuminate\Database\Eloquent\Relations\Pivot::__construct() must be an instance of Illuminate\Database\Eloquent\Model, none given, called in {somePath}\vendor\laravel\framework\src\Illuminate\Database\Eloquent\Model.php on line 335

@JuanDMeGon JuanDMeGon changed the title [5.4] Custom Intermediate Models not instantiable [5.4] Custom Intermediate Pivot Models not instantiable Feb 5, 2017
@JuanDMeGon JuanDMeGon changed the title [5.4] Custom Intermediate Pivot Models not instantiable [5.4] Custom Intermediate (Pivot) Models not instantiable Feb 5, 2017
@arjasco
Copy link
Contributor

arjasco commented Feb 5, 2017

Reason for wanting to instantiate a custom pivot model on its own?

My understand is it's for use within a relationship with the ->using() method on your relationship definition. You may as well extend Model?

@JuanDMeGon
Copy link
Contributor Author

If you check the definition of Illuminate\Database\Eloquent\Relations\Pivot it extends from eloquent Model, so in my understanding it still being a regular model.
The custom pivot model acts, as as you said, like a pivot AND is a model too I should be able to use it directly or to resolve the many to many relationship through it.
Maybe I am misunderstanding something here but it does not seem logic for me to use a custom pivot model and not be able to use it as a regular model as well.
Possibly @pstephan1187 can give us some orientation for this?

@arjasco
Copy link
Contributor

arjasco commented Feb 5, 2017

I see your logic, it may extend Model but doesn't mean it completely reflects the same behaviour.

https://github.com/laravel/framework/blob/5.4/src/Illuminate/Database/Eloquent/Relations/Pivot.php#L47

Looks like the Pivot requires a parent model, and this is provided when the relationship builds that pivot here:

https://github.com/laravel/framework/blob/5.4/src/Illuminate/Database/Eloquent/Relations/Concerns/InteractsWithPivotTable.php#L378

I might be wrong so i would be interested too. I think the idea is so you can define some further logic on your custom pivot model. Eg when updating a property your might want to use a mutator or maybe define some helper methods on that pivot model which you couldn't do before.

@JuanDMeGon
Copy link
Contributor Author

Yeah, it is possible that it does not need to reflects the same behaviour but so it should not extends directly from model but from a modified model or similar.
Let see if someone can clarify this for us.
Thanks for commenting.

@dees040
Copy link
Contributor

dees040 commented Feb 28, 2017

Is there any progress? Getting this issue as well.

@JuanDMeGon
Copy link
Contributor Author

@dees040 Unfortunately not. It seems like a not important issue yet.
I decided to do not use it for now and use the conventional old approach.
Hopefully, later this will be treated.

@ghost
Copy link

ghost commented Mar 13, 2017

I have the same issue and error. The docs aren't really clear about the purpose of the $relation->using('model') functionality.

For example is it possible to do something like this with that functionality:

users

  • id
  • belongsToMany posts

posts

  • id
  • belongsToMany users

post_user

  • id
  • post_id
  • user_id
  • hasMany roles (post_user_role)

post_user_role

  • id
  • post_user_id
  • role

$user->load('posts.pivot.roles');

That would be awesome, but can't get it to function.

@pstephan1187
Copy link
Contributor

@RSDrsd you need to load in post_user_role field on the pivot table:

function posts(){
    return $this->belongsToMany(\App\Post::class)->withPivot('post_user_role');
}

@minhchu
Copy link

minhchu commented Mar 23, 2017

I think the document should show a practical example using intermediate table.

@JuanDMeGon
Copy link
Contributor Author

You are probably right. I would like to add an example to the documentation, but it seems like it does not work like I think it does, so may is a matter of time.

@cjaoude
Copy link

cjaoude commented Apr 2, 2017

Guys, agreed, some documentation would be nice, but after toying with it, here's how it works.

Let's say M-M for User and Books. And you're also adding a store_id attribute to the pivot table. How can you get that Store model based on the store_id?

// User.php

public function books()
{
    return $this->belongsToMany(Book::class)
                ->withPivot('store_id')
                ->using(BookUser::class);
}

So now when you hit $user->books->first()->pivot you're getting the BookUser model. Remember, $user->books returns a collection. You want a Book from the collection to make use of BookUser. (...so above i just used ->first())

// BookUser.php
class BookUser extends Pivot {
    public function store()
    {
        return $this->belongsTo(Store::class);
    }
}

Bingo. Now.... $user->books->first()->pivot->store yields the Store of store_id.

So now you might use it for something like this...

@foreach ($user->books as $book)
  Store name: {{ $book->pivot->store->name }}
@endforeach

So that's it. Each row in the pivot table is now coming back to you as a BookUser model. And you can do relations, saves, updates, whatever.

I hope that helps.

@JuanDMeGon
Copy link
Contributor Author

Yes, it is the expected behavior here.
In your example, you should be able to use the BookUser model as the pivot, that is good. In fact, when you use the pivot property it returns an instance of BookUser, but when you want to directly create an instance of BookUser instead of go trough the other models, it just fails.

@cjaoude
Copy link

cjaoude commented Apr 2, 2017

Yes, BookUser can't be instanced directly. [edit...] It extends Pivot class which extends Model but for whatever reason is not quite the behavior as a normal Model.

It's 'intermediary' and we just need to use it that way. It's very useful.

@JuanDMeGon
Copy link
Contributor Author

@cjaoude Yes, it is a Pivot class, wich extends from Model.
Check the definition of here. It extends directly from Model.
If you think about that is the reason why you can obtain an instance when using the pivot property.

I clearly understand that Pivot is like a "special model," but it is extending from Model and should provide the same behavior expected from a regular model. In another case, it should extend from another kind of "Model."

That is, in fact, the main reason for the discussion here.

@cjaoude
Copy link

cjaoude commented Apr 2, 2017

I understand what you mean. My posts are to answer how to use intermediary models, which some people have asked about above.

@JuanDMeGon
Copy link
Contributor Author

Yes, you are right. Surely that will be very helpful for all.

💯 :)

@rdpascua
Copy link

rdpascua commented Apr 27, 2017

Now how can we eagerload the intermediate table?

$foo->with('bar.pivot.baz'); doesn't work since pivot relation doesn't exists.

@Dreambox13
Copy link

I spend at least one week trying to find that question @rdpascua.
Anyways, after a long efford I tried a threeway pivot and works just fine. I don't know if it helps you.

@rdpascua
Copy link

rdpascua commented Apr 27, 2017 via email

@dherran
Copy link

dherran commented Jun 10, 2017

@Dreambox13 @rdpascua can you provide an example of how you solved the problem of eager loading the intermediate table?

@rdpascua
Copy link

rdpascua commented Jun 11, 2017 via email

@decadence
Copy link
Contributor

I face similar issue but it's expected behavior because Pivot constructor needs some values indeed. Would be nice if we can use it as normal Model.

@antonkomarev
Copy link
Contributor

Faced same issue recently.

@dherran
Copy link

dherran commented Jun 12, 2017

This proposal solved it for me @decadence @a-komarev :
laravel/ideas#175

Read the comments all the way through. It explains the changes required to eager load a pivot model relationships on 5.4.

@antonkomarev
Copy link
Contributor

@dherran Thanks for pointing to the solution.

@decadence
Copy link
Contributor

decadence commented Jun 12, 2017

@dherran thanks. But would be cool to have this out of box.

@themsaid themsaid reopened this Jun 14, 2017
@darkylmnx
Copy link

darkylmnx commented Jun 23, 2017

i'm having the same issue, i can't instantiate a pivot model, here's my structure and the use case

posts (belongsToMany tags)

  • id
  • name

tags (belongsToMany posts)

  • id
  • name

post_tag (pivot)

  • id
  • post_id
  • tag_id
  • votes_count

votable (polymorphic)

  • id
  • votable_id
  • votable_type

and as the tables explain by themselves, you can vote for a post_tag just like the likedin tag recommandation feature

the api would be something like : POST /post-tag/{post_tag}/likes
And inside this something like

$post_tag = PostTag::findOrFail($id);
$post_tag->votes()->save(new Vote)

But this doesn't work because you can't instantiate, the only thing i can do now is

POST /post/{post}/tag/{tag}/likes

$post = Post::findOrFail($id);
$post_tag = $post->tags()->findOrFail($tag)->pivot;
$post_tag->votes()->save(new Vote);

that's so much cumbersome and so less performant as you have so many queries here while the only info needed is the pivot table record to save the associated vote, and not the tag record nor the post record

@brunocascio
Copy link

Take a look at https://stackoverflow.com/a/44932979/2723301

I've implemented in a better way.

@darkylmnx
Copy link

darkylmnx commented Jul 5, 2017

@brunocascio the solution in your link doesn't make the pivot model instantiable since the pivots are gotten from the related query Banner

What we want here is BannerRegion to be in instantiable (based on your link)

@decadence
Copy link
Contributor

Merged in 5.5.

@JuanDMeGon
Copy link
Contributor Author

Awesome! Thank you @BertvanHoekelen

@caseydwyer
Copy link

@JuanDMeGon +1 for posting. I've gone hunting for this a couple of times; glad to see it make its way into the 5.5 release. Can anyone confirm if this addition to 5.5 will also allow for eager loading?

eg, @rdpascua's example: $foo->with('bar.pivot.baz');

@pnlinh-ivc
Copy link

@cjaoude How can I return data relate with pivot table to json without loop?

@mazyvan
Copy link

mazyvan commented Dec 23, 2018

I don't clearly understand why this issue gets to long in comments. I mean AFAIK following the Liskov substitution principle the behavior this had was is not correct. Now I'm looking a way to go workaround with this in 5.4. I have a MorphPivot class and I need it to relation with another class. But I can't. Any idea?
BTW I know this isn't the proper place to ask the question, but I bet some of you guys had facing the same problem

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

No branches or pull requests