laravel / ideas

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

[Proposal] Self activate Builder macro entry, #macro, #builder #200

Closed kocoten1992 closed 6 years ago

kocoten1992 commented 8 years ago

At here https://github.com/laravel/framework/blob/5.3/src/Illuminate/Database/Eloquent/Builder.php#L1417

That mean:

Builder::macro('custommacro', functionn () { //register to builder
    dd('I can only be call **manually**');
})

//Have to manually call it
User::with('profile')->custommacro();

My suggest is: it will open a whole new world if there is an entry it, like:

public function __call($method, $parameters)
{
    //Check if any global macro / entry point macro and run it before anything

    if (isset($this->macros[$method])) { //
    ....
}

//Then we could
User::with('profile') //automatically apply the global macro without the hassle

Request more detail or example if you think this interesting.

sebastiandedeyne commented 8 years ago

If this is specifically for queries, don't global scopes handle this already?

https://laravel.com/docs/5.3/eloquent#global-scopes

kocoten1992 commented 8 years ago

@sebastiandedeyne , That true but macro let you have a superior control, you could do many more thing if there is a method like that, consider this:

User::with('profile')->first()

Look simple, but this is I actually want:

//User model
public function profile()
{
    return $this->hasOne(StudentProfile::class);
    return $this->hasOne(TeacherProfile::class); //not working
    return $this->hasOne(ParentProfile::class); //not working
}

//Normally you would do this on User
public function studentProfile()
{
    return $this->hasOne(StudentProfile::class);
}

public function teacherProfile()
{
    return $this->hasOne(TeacherProfile::class);
}

public function parentProfile()
{
    return $this->hasOne(ParentProfile::class);
}

It much cumbersome if you have to do this:

User::with('studentProfile', 'teacherProfile', 'parentProfile')->first() //even more if there is nested relationship

Since teacher profile are not parent profile and of course are not student profile, it not beautiful if combine all the field on 3 table and let some of it null, would rather have 2 addition query for it.

Pretend it already been done, I could use accessor to get data, like:

public function getProfileAttribute()
{
    if ($this->student_profile) { //one can play only one role at a time, this is just a simple ex
        return $this->student_profile;
    }

    if ($this->teacher_profile) {
        return $this->teacher_profile;
    }

    if ($this->student_profile) {
        return $this->parent_profile;
    }

}

How can it be done?

We already have this: https://github.com/laravel/framework/blob/5.3/src/Illuminate/Database/Eloquent/Builder.php#L1355, so I could hijack it and rewrote the condition eagerload, on the surface, my team is just use User::with('profile')->first(), but behind the scene, it happen as follow:

          /  StudentProfile \
User      -  TeacherProfile -      profile
          \  ParentProfile /

//When use just $user->profile, it will magically load right for you.

It tends to work with complexing query as well, eg:

School::with('user.profile.abc.xyz') //you hijacking and put your logic in it, so it possible

The only remain is, the hijack method not exist yet :D, there is no way it could be done

P/s: forgot this: for simple User::with('profile'), accessor alone will do, but hasOne and belongsTo are different (don't know why), at first I thought a simple hijack "with" will do:

public function with($relation)
{
    if (strpos($relation, 'profile') !== false) { //it stink, and not working, belongsTo don't even call "with" behind the scend
        ...
    }

    return parent::with($new_relation);
}
benswinburne commented 8 years ago

Your above use case can be achieved using polymorphic relationships in Eloquent.

Adam Wathan gives a good example on his blog.

https://adamwathan.me/2015/09/03/pushing-polymorphism-to-the-database/

kocoten1992 commented 8 years ago

@benswinburne , that true, but I don't specific target this task, what I aim to is a self trigger point in builder so anyone could invoke and do magic thing while maintain a very beautiful api interface.

(I avoid polymorphic all together since encounter this bug https://github.com/laravel/framework/issues/5429, this approach just use simple relation, I think it not gonna have that problem).