hipsterjazzbo / Landlord

A simple, single database multi-tenancy solution for Laravel 5.2+
MIT License
614 stars 138 forks source link

Scope is not applied in middleware (TenantManager::tenants array is always empty) #57

Closed jamestierney closed 7 years ago

jamestierney commented 7 years ago

I'm not sure if I've made a mistake in the set up of my application, but it doesn't apply the scope when I call Landlord::addTenant() in middleware, whereas if I hardcode it in the routes file, it applies it.

It seems that this is because all the models using the BelongsToTenants trait have already been instantiated before the middleware is run, so that when TenantManager::applyTenantScopes() calls the modelTenants() method, the tenants array is always empty.

AFAIK this was not an issue in v1.

I'd be grateful of any help!

Edit: my current workaround is calling bootBelongsToTenants() on the model again after addTenant(): Landlord::addTenant('company_id', $company->id); \App\Models\Site::bootBelongsToTenants();

lasseeee commented 7 years ago

Where are you calling addTenant()from? Having you tested if scoping works for any other models?

shadoWalker89 commented 7 years ago

I'm upgrading my 5.3 app to 5.4 and my tests are failing because of this.

When running the app from the borwser everything works just fine, because the Landlord::addTenant() is called in global middleware.

But in my tests, where i'm doing setup before visiting the route and making assertions. The query is returning model that shouldn't be loaded.

hipsterjazzbo commented 7 years ago

@jamestierney I know this has been a while, but just in case: are you sure that the middleware is being called? That's where I'd look.

@shadoWalker89 addTenant() needs to be called before doing anything that requires scoping. If you're running tests in a way where the middleware isn't being used, then you'll have to call it yourself in your test setup. However, I don't see any real utility in testing scoped calls. This package is tested, so why are you testing it again?

shadoWalker89 commented 7 years ago

@HipsterJazzbo I'm not unit testing my models, i know that you already cover that. But, in my browser kit tests (acceptance test), i'm making sure that when a user opens a page, he should only see the records that concerns him.