Query builder when() method producing wrongly conditioned query-CodePudding

I posted what I think is a Laravel bug (https://github.com/laravel/framework/issues/39398) but it was closed because I'm not 100% that it's really a bug. Could someone else tell me whether this behaviour is erraneous?

I have the following piece of code:

$user=251;
$description='created';
Activity::when($user, function ($query, $user) {
    return $query->where('causer_id', $user)
    ->orWhere(function($query) use ($user) {
        $query->where('subject_id', $user)
        ->where('subject_type', "App\\User");
    });
})->when($description, function ($query, $description) {
    return $query->where('description', $description);
});

Logically I think it should produce a query that return all rows that has the correct user and the correct description but I get all rows with that user regardless of description.

The code above produces this sql query:

select * from activity_log where causer_id = '251' or (subject_id = '251' and subject_type = 'App\User') and description = 'created'

...whereas I think it should produce:

select * from activity_log where (causer_id = '251' or (subject_id = '251' and subject_type = 'App\User')) and (description = 'created')

To put it short: Shouldn't the when() function put parentheses around it's returned result not to affect other parts of the query? Is it a bug that it doesn't?

CodePudding user response：

I don't think this is a bug. This isn't intended behavior that is broken. It would be a new feature to be added to the framework. You can go to the discord server and attempt to make your case, or simply submit a PR with the change and argue for it, but this isn't "broken" existing functionality.

The when() method is just syntactic sugar so that you can conditionally modify the query in a method chain without having to break out into a separate if statement. The following code would be equivalent:

$query = Activity::query();

if ($user) {
    $query->where('causer_id', $user)
        ->orWhere(function($query) use ($user) {
            $query->where('subject_id', $user)
                ->where('subject_type', "App\\User");
        });
}

if ($description) {
    $query->where('description', $description);
}

As you can see in the above code, there would be no expectation that any clauses added conditionally would be wrapped in parentheses (except where explicitly specified in your orWhere() call).

One issue with what you're expecting is that the when() method can be used to modify the query in any way, not just adding extra conditions. So, if you added an orderBy() or a limit() inside a when() clause, what would you expect to happen?

So, the when() method itself isn't going to modify your query, but it will run exactly what you tell it to. Therefore, to do what you're looking for, you would need to wrap the query yourself inside your when() clause:

Activity::when($user, function ($query, $user) {
              return $query->where(function($query) use ($user) {
                  return $query->where('causer_id', $user)
                               ->orWhere(function($query) use ($user) {
                                   $query->where('subject_id', $user)
                                         ->where('subject_type', "App\\User");
                               });
              });
       })
       ->when($description, function ($query, $description) {
           return $query->where('description', $description);
       });