Issue on tests

[mariomka]

I'm trying to test features that use Landlord but the scope is not applied unless I call $landlord->applyTenantScopes(new Model); manually before query.

Simple failed test example:

/** @test **/
function scopeTest()
{
    $user = User::create([
        'name' => 'User name',
        'email' =>'test@domain',
        'password' => bcrypt('secret'),
        'remember_token' => str_random(10),
    ]);

    Landlord::addTenant($user);

    Account::create([
        'name' => 'Account name',
        'user_id' => $user->id,
    ]);

    Account::create([
        'name' => 'Account name',
    ]);

    $count = Account::count();

    $this->assertEquals(1, $count);
}

I can fix easy the test calling applyTenantScopes() but I can't fix real tests that are using controllers, repositories...

[fgilio]

Hi @mariomka! I'm having this same issue. Did you manage to solve it? Also, how are you using $landlord->applyTenantScopes(new Model); ? Could you provide an example with the code you posted above?

Thanks!

[mariomka]

@fgilio I didn't find a solution. Finally I wrote a simple multi-tenancy system for my project.

I don't remember exactly. I think that you can call Landlord::applyTenantScopes($account); before get the count to fix this simple test.

Good luck!

[fgilio]

Hi @mariomka, thanks for the reply!

I was actually editing the comment with my temporary solution:

I added this method to the model and I call it when I need to re apply the scope with a different ID. But of course, I was already doing Landlord::addTenant($tenant); on start and Landlord::removeTenant($tenant); on finish (which I think is not necessary, as it resets when you add a new tenant of the same type) of the loop.

public static function reApplyTenantScopes()
{
    static::$landlord->applyTenantScopes(new static());
}

So in the DB seeder I call it like this before executing a query:

App\MyTenantScopedModel::reApplyTenantScopes();

I just feels too hacky for my like, but seems to be working ok. I'll keep testing it and update here if I find that this brings some kind of problem. I hope it helps other having similar issues, I can't believe the amount of time I spent debugging this.

[mariomka]

Seems to be a bit ugly solution but if works for you is enough. For sure it will help other users.

I'm remembering that the problem is that the library applies a global scope on boot by each tenant. Maybe It can be fixed if applies a global scope for all tenants.

Something like (I didn't test it):

public function applyTenantScopes(Model $model)
    {
        if (!$this->enabled) {
            return;
        }

        $model->addGlobalScope('landlord-scope', function (Builder $builder) use ($model) {
            $this->modelTenants($model)->each(function ($id, $tenant) use ($builder, $model) {
                $builder->where($model->getQualifiedTenant($tenant), '=', $id);
            });
        });
    }

When current code is ( https://github.com/HipsterJazzbo/Landlord/blob/master/src/TenantManager.php#L118 ):

public function applyTenantScopes(Model $model)
    {
        if (!$this->enabled) {
            return;
        }
        $this->modelTenants($model)->each(function ($id, $tenant) use ($model) {
            $model->addGlobalScope($tenant, function (Builder $builder) use ($tenant, $id, $model) {
                $builder->where($model->getQualifiedTenant($tenant), '=', $id);
            });
        });
    }

[fgilio]

A much cleaner solution, and seems to be working fine! Thanks!

[mariomka]

Nice! I didn't have time to try. Maybe I'll make a pull request when I have time, Right now I don't know if the change breaks anything.

PD: Feel free to make a pull request if you want.

[hipsterjazzbo]

There are a few things I'm aware of (just generally) that don't play nice between Eloquent boot functionality and tests, because the framework doesn't re-boot between each test. If someone finds a good solution to this issue I'll consider it.

[fgilio]

Didn't do the pr because I didn't have time to do formal tests. But, I'm using @mariomka solution since then and It's working flawlessly.

[hipsterjazzbo]

Unfortunately, that breaks some pretty important functionality for the library, namely that you can no longer run queries without a single tenant scope while leaving the others on:

// Will not scope by 'tenant_id', but will continue to scope by any other tenants that have been set
ExampleModel::withoutGlobalScope('tenant_id')->get();

[fgilio]

What if we make this optional? It could be like having a master switch that dictates how the library will behave, and the default would still be the current behaviour.

Honestly, I'd like to continue using the official library and not a fork.

[EmadAdly]

Why is there no solution in order to work together globally?

as brainstorming,

If requested this method

ExampleModel::withoutGlobalScope('tenant_id')->get();

Stop work for this method

applyTenantScopes(Model $model)

[hipsterjazzbo]

Sorry guys, I haven't had much time to dig into this, but I won't be putting this into the package. Being able to deal with the individual tenancies separately is a core function of Landlord.

At some point when I have time to really dig into it, I'll have to figure out why that solution even works — why would it fail when applying 2 scopes, but not when applying 1? That's the real issue as far as I can see.

[ashleyhood]

I managed to get it to work by creating the following middleware and adding it to the web middleware group in Kernel.php:

public function handle($request, Closure $next)
{
    if (Auth::check()) {
        foreach (Auth::user()->getTenantColumns() as $tenant_scope) {
            Landlord::addTenant($tenant_scope, Auth::user()->{$tenant_scope});
            Landlord::applyTenantScopes(Auth::user());
        }
    }

    return $next($request);
}

I think it has something to do with the user model being booted before the global scope is being applied.

If I change the above code to use the authenticated user directly in the addTenant method, it doesn't work:

public function handle($request, Closure $next)
{
    if (Auth::check()) {
        foreach (Auth::user()->getTenantColumns() as $tenant_scope) {
            Landlord::addTenant(Auth::user());
            Landlord::applyTenantScopes(Auth::user());
        }
    }

    return $next($request);
}

[hipsterjazzbo]

This is absolutely to do with the way laravel runs tests. Every mode is booted once for the whole test run. This means that things can get out of whack on any test after the first one — this is the case for any global scope on an eloquent model, not just Landlord.

Unfortunately, I don't think there's much that can be done about it from this package. I've opened PRs in the past to fix it in the framework, but Taylor has declined them. Perhaps that'd be the better strategy?

[awebartisan]

public function handle($request, Closure $next)
{
    if(auth()->check())
    {
        $tenantId = auth()->user()->tenant_id;
        Landlord::addTenant('tenant_id', $tenantId);
        Landlord::applyTenantScopes(auth()->user());
    }
    return $next($request);
}

I just added this as my middleware and it worked somehow. I am not sure 👯‍♂️

[jsphpl]

Hey guys, does anyone have a solution on how to test queries being scoped (testing through http, not unit tests) without explicitly calling applyTenantScopes() for each scoped model in the middleware?