Make Conditionable::when working with relation methods like wherePivot

[Gnorxan]

Hello,
today I tried to refactor some code where the pivot of a many-to-many relation was conditionally added.
I used the when statement but got an SQL Exception with the following code:

public function getRelations()
{
    $condition = true;
    
    return Model::find(1)
        ->relations()
        ->when($condition, function($query) {
            $query->wherePivot('status', Request::get('status'))
        })->get();
}

I investigated the frameworks code base. The 'Conditionable Trait' is used only in the 'Query' class and not the 'Relation' class.

Are there any reasons why it's not supporting the relations?

[juse-less]

I'm not sure why you get a SQL Exception.
But I use when() on relationships in Laravel 9.
The callback will get the relationship class.

I think I've only used it on BelongsTo relationships, though.

[Gnorxan]

@juse-less Where do you get the relationship class?

For clarification I created a new laravel project demonstrating the issue.

I added the logic to the routes file:
Which outputs:

"coursesQuery of type: Illuminate\Database\Eloquent\Relations\BelongsToMany"
"Query of type: Illuminate\Database\Eloquent\Builder"

Therefore the method wherePivot will not be available inside the when callback.

[juse-less]

Reposting in the right place (thread) instead of a separate message I accidentally posted as.

---

Hmm. That is interesting.
If it's not part of the Relation class itself, it's possible that not all relationships has support.

Like I said, I've only really done BelongsTo. So that's probably why I haven't gotten the error.
I think I started using it in Laravel ~9.16 (currently 9.20).

Looking at the routes file, it looks like you've understood it correctly.
This is the exact code I use (including type hints) in production, but with replaced variables for a simpler example.

$withActiveUsersOnly = true;

Post::query()
    ->first()
    ->user()->when($withActiveUsersOnly, function (BelongsTo $user, bool $withActiveUsersOnly): void {
        // $withActiveUsersOnly is only used to demonstrate that the value passed to `when()` is also supplied,
        //   just like any `Conditionable`. Database-related, `Stringable`, or otherwise.

        $user->where('active', true);
    });

I can see if I can quickly do some testing for BelongsToMany as well.
But you could be right in that it doesn't support BelongsToMany/pivot currently.

I'll try get back in a few hours, unless some else more knowledgable knows the answer. 🙂

[juse-less]

First off - I apologise for the confusion.

I realised I 'didn't get the memo', and wasn't thinking of our macros we have.
I dug into our code, as well as, the Laravel side of things.

1. We apparently have macros for it. When we register our macro, this part is executed as the 'forwarder' is effectively overridden.
2. But, when no macro is registered, this part is run.
   As seen, Laravel will forward when() to the actual Query Builder of the relationship - not execute anything on the relationship itself.
   This is why you're receiving the underlying Query Builder, instead of the relationship.

[erikn69]

We apparently have macros for it. When we register our macro, this part is executed as the 'forwarder' is effectively overridden.

why don't use the macro on the Query Builder instead of the Eloquent Builder
https://medium.com/@danielalvidrez/laravel-query-builder-macros-fe176d34135e
So, then it will work on relations builder instance

[juse-less]

In our case, we primarily do it to update relationships. So we don't use the Builders directly.
In the case of our BelongsTo, we often conditionally associate, or dissociate, which is why we need to do it on the relationship, as opposed to the Builder.

That aside - we have a base Eloquent Query Builder, which we also happen to use the Conditionable trait on already.
And all our Model's then extend from that for Model-specific Query methods.

So, we have it on both. But this is really about relationships and not the Query Builders.
Relationship-specific where's is usually defined in the relevant relationship class.

In @Gnorxan's case, this'd be here, for instance ->

framework/src/Illuminate/Database/Eloquent/Relations/BelongsToMany.php

Lines 362 to 367 in bea761c

	public function wherePivot($column, $operator = null, $value = null, $boolean = 'and')
	{
	$this->pivotWheres[] = func_get_args();
	return $this->where($this->qualifyPivotColumn($column), $operator, $value, $boolean);
	}

[juse-less]

I should also mention that both the base Query Builder and the Eloquent Query Builder in Laravel is already using the Conditionable trait, as part of the BuildsQueries trait, that they both use.
So we could technically remove ours.

[Gnorxan]

Thanks @juse-less for the investigation!

I thought about creating a pull request adding the Conditionable trait to the Relation class. This way the method should work fine.

I didn't investigate further on it, since I wanted to know if there are good reasons that this trait has not been added to the Relation class already.

Maybe someone tried this already by extending the class or something similar and had issues with it.

Also I'm new to the open source community so any thoughts and advises are welcome.

[juse-less]

@Gnorxan If I recall correctly, the Conditionable trait was added to the Builders fairly recently (after Laravel 9 was released).
So could be that no one has needed it on relations before, and therefore not thought about it.

I'm rather new to open source as well (other than browsing). Which is also why I usually create things in my projects (like the mentioned Conditionable macros), instead of creating PR's or even feature requests/discussions. 🙂

Thanks for bringing this up. It'd reduce some of our overhead, if it was sorted.
Hopefully someone more familiar with it all has some answers.

I haven't experienced any issues or conflicts in the BelongsTo Relation, at least.

[romainprevost]

I have had the same isssue with a BelongsToMany relationship that could use the when() method but the closure received the query builder and not the BelongsToMany relationship, so I could not use wherePivot* methods.

The error was different though, as the builder tried to find a column with the name pivot in it such as "column pivot_not_null does not exist).

It is a small issue, the code is just a bit less clean :)