Builder::getQuery() ignores scopes

rentalhost commented 5 years ago

Laravel Version: 5.8.16
PHP Version: 7.3.5 x64
Database Driver & Version: MariaDB 10.3.13

Description:

When you have a Model with SoftDeletes trait, for instance, that will apply the SoftDeletingScope to the model, it will be ignored if you use the getQuery() method on an instance of this model.

Steps To Reproduce:

Setup your .env (database, basically).

composer create-project --prefer-dist laravel/laravel=dev-master laravel-test
cd ./laravel-test
php artisan migrate:install

Patch with this gist: https://gist.github.com/rentalhost/a4b2446e3c772a3c14ababd990e5c7a9

php artisan migrate
phpunit

It will fail on second test of tests/Unit/ExampleTest.php file.

Real case

In my case, I have two tables: companies and company_groups. Each company could have one company group, but a group could belongs to many companies (N-to-1 relationship). The user can search by a company group "category", and it should returns all companies compatible with this search. Some groups was soft deleted. So I do basically that:

$companiesQuery = Company::query();
$companiesQuery->whereIn('id', 
    CompanyGroup::query()
        ->where('company_category', $categoryId)
        ->getQuery()
        ->select('company_id')
);

My expectation is that is generates that:

SELECT *
FROM companies

WHERE
    companies.id IN (
        SELECT company_groups.company_id
        FROM company_groups

        WHERE
            company_groups.company_category = ? AND
            company_groups.deleted_at IS NULL
    ) AND

    companies.deleted_at IS NULL

But currently it will skip all scopes when called from getQuery(), so we will get that:

SELECT *
FROM companies

WHERE
    companies.id IN (
        SELECT company_groups.company_id
        FROM company_groups

        WHERE
            company_groups.company_category = ?
            ### Plin! ###
    ) AND

    companies.deleted_at IS NULL

Which will list companies that belongs to some group category that was deleted previously.

Current workaround

Currently I need to force withoutTrashed() for every time that I need do that, and for every scope that I need to use that is global for this model (that is ignored).

Suggestion

To avoid BC, I think that is better create a new method like getQueryWithScopes(), so getQuery() will keeping return without scopes. But I still thinks that it is a bug -- or at least is very hard to know the behavior before you run it.

staudenmeir commented 5 years ago

This is the intended behavior. You can use toBase() to get the underlying query builder with all global scopes applied.

Why are you using getQuery()? In your example, it wouldn't be necessary.

rentalhost commented 5 years ago

Oh! You are right. For some reason I am using like this a long time, I don't remember why I started using that. Now I need update all my code to remove my hacks. haha

Edit: actually I can't... I will reopen it when I be able to create a test case better.

Edit 2: ok, seems that it will not works for HasMany relation and similar, but I could keeps the getQuery() in this case, once that the scopes are already applied on this time. (eg. $this->hasManySomething()->getQuery()->select('id'))

laravel / framework