Skip to content

Conversation

@EK1771
Copy link

@EK1771 EK1771 commented Apr 19, 2020

Laravel 7.x introduced the following change laravel/framework#30934 allowing for the feature https://laravel.com/docs/7.x/queries#subquery-where-clauses

This currently breaks parameter grouping when using eloquence due to the where() function passing = as the default operator even if a closure is passed.

This MR aims to fix the issue by detecting if a closure is passed and if so, not applying the default = operator.

To replicate the bug using sample Code on a fresh Laravel 7 install with sofa/eloquence-base:

app/User.php

<?php

namespace App;

use Illuminate\Contracts\Auth\MustVerifyEmail;
use Illuminate\Foundation\Auth\User as Authenticatable;
use Illuminate\Notifications\Notifiable;
use Sofa\Eloquence\Eloquence;

class User extends Authenticatable
{
    use Notifiable, Eloquence;

    protected $fillable = [
        'name', 'email', 'password',
    ];

    protected $hidden = [
        'password', 'remember_token',
    ];

    protected $casts = [
        'email_verified_at' => 'datetime',
    ];
}

routes/web.php

Route::get('/', function () {

    App\User::where(function ($query) {
        $query->whereNull('remember_token')
              ->orWhere('email','!=','test@test.com');
    })->get();

});

Results in the error

Illuminate\Database\QueryException
SQLSTATE[HY000]: General error: 1096 No tables used (SQL: select * from `users` where (select * where `remember_token` is null or `email` != test@test.com) is null)

@bytebrain
Copy link

Is it possible to merge this request? It would solve a problem with advanced where statements when using Laravel 7.x and your eloquence package. I've tried this fix and it works.

@gabomasi
Copy link

gabomasi commented Jan 5, 2021

Hi! This is an important fix, can you merge it, please?

@gabomasi
Copy link

gabomasi commented Jan 5, 2021

@jarektkaczyk

@rubenlagatie
Copy link

rubenlagatie commented Mar 11, 2021

I also really need this fix. My entire application is broken after upgrading to L7. Could this please be merged?
It's been almost a year...
I've also tested this in combination with Mappable and it works well.

@glennjacobs
Copy link

I too hope this can be merged ASAP. An application I'm having to support needs this to work in L7.

@gfernandez-me
Copy link

@jarektkaczyk plz help!

@roelVerdonschot
Copy link

Any update on this?

@paulosscruz
Copy link

@jarektkaczyk

jitendrajp added a commit to jitendrajp/hookable that referenced this pull request Jun 3, 2022
Remo added a commit to ortic/hookable that referenced this pull request Jun 3, 2022
jarektkaczyk#27 Fix Parameter Grouping with Laravel 7.x
@maqduni
Copy link

maqduni commented Apr 3, 2023

I used composer patches to apply this fix to my project,

  1. An example of using composer patches - link
  2. Install composer-patches - link to apply the patches.

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

Successfully merging this pull request may close these issues.

10 participants