Add Model Observer to tenant belonging models

[obrunsmann]

If I add a model observer to a model which belongs to a tenant (like https://laravel.com/docs/5.3/eloquent#observers) the tenant filter is getting ignored.

Listening to events works fine (https://laravel.com/docs/5.3/eloquent#events).

[obrunsmann]

So to reproduce:

Add Landlord Trait to a model (e.g. User), add Observer to the model User::observe(UserObserver::class); and try search on it. It will ignore the tenant assignment.

[hipsterjazzbo]

This works fine for me, if I understand correctly. What exactly are you trying to do that isn't getting scoped?

[lasseeee]

Maybe related to #45, have you tried with other models than User? Also see: #46 (comment)

[benkingcode]

Just come across this issue myself. Adding an observer to a model stops Landlord from working(!)

[pimski]

It seems that the order is of essence.

If you run Landlord::addTenant() first everything works fine (the tenant conditions are applied). However if you start with MyModel::observe() the conditions aren't applied.

In the HasEvents::observe() function a new instance of the model is created (MyModel in the example above). This fires off a sequence of boot events, including the boot event used by the trait BelongsToTenants. However, the $tenants collection it needs (from TenantManager) isn't filled yet, so it can't apply the tenant conditions to the global scope. The collection is filled later, once you call Landlord::addTenant()

@hipsterjazzbo any ideas how to make this more robust? maybe adding an exception or something?

[benkingcode]

That makes sense. I set addTenant in a route middleware, and the observers are registered in AppServiceProvider as recommended by the Laravel docs.

[aluferraz]

Also happening here... Any ideas ? I would like to keep my tenants in the route middleware and the observers in the appserviceprovider as recommended ...

[hipsterjazzbo]

.@pimski is totally right about what's happening there. I agree that this should be better handled, but I'm not aware of anything I can do from within a package context to force the order like that. The only solutions I've come up with (involving various ways to set the tenants at boot time) remove the ability to add tenant dynamically, which is important functionality for me.

Ideas?

[pimski]

Hi @hipsterjazzbo

An idea:
In the TenantManager add a variabele (collection/array) for models that couldn't get scope'd directly:

protected $tenants;
protected $deferredModels;

In applyTentantScopes() push a model to the deferredModels collection if the tenants haven't been set yet:

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

	if ($this->tenants->isEmpty()) {
		// No tenants yet, defer scoping to a later stage
		$this->deferredModels->push($model);
		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);
		});
	});
}

Finally, add a method to apply the global scope to the 'deferred models':

public function applyTenantScopesToDeferredModels()
{
	$this->deferredModels->each(function ($model) {
		$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);
			});
		});
	});

	$this->deferredModels = collect();
}

Bit of code duplication and a lengthy name... :)

[renanwilliam]

This solution works for me!

[OwenMelbz]

Just spent the last few hours trying to figure out why my users were not scoped - eventually commented out my User::observe(UserObserver::class); and behold - scoping worked once more!

What is the chance of the fix/workaround for this being merged soon?

Thanks :)

[hipsterjazzbo]

Okay I've tagged a beta release with the contents of #66. Could everyone please have a go, and if there are no issues after a while I'll tidy it up a bit and push it stable.

[gausam]

Hey team 👋
I see mention of deferred scoping in another conversation, and it solved scoping failure for me. Works as advertised, but what's the proper use case for it (i.e ideal scenario under which I'd call (Landlord::applyTenantScopesToDeferredModels(); right after Landlord::addTenant() - can't see anything about it in the README.
Thanks