spatie / laravel-multitenancy

Make your Laravel app usable by multiple tenants
https://spatie.be/docs/laravel-multitenancy
MIT License
1.11k stars 153 forks source link

NeedsTenant with route model binding #550

Closed jetapps-team closed 1 month ago

jetapps-team commented 3 months ago

I would expect that checking the existence of the tenant would be before trying to pull data from the DB Unfortunately, the check is performed after binding the route model, and if I'm in the main application I'll get a SQL error instead of a NoCurrentTenant error

masterix21 commented 3 months ago

Please explain better your issue with an example. Thanks

michaelklopf commented 1 month ago

I just came across the - I think - same problem. You have a route like this

        Route::get(
            '{tenant}/documents/{document}',
            DocumentPage::class
        );

And a Livewire component with a mount like this

    public function mount(Document $document)
    {
        $this->document = $document;
    }

which results in the error

SQLSTATE[3D000]: Invalid catalog name: 1046 No database selected (Connection: tenant, SQL: select * from `documents` where `id` = 16 limit 1)

This should be the same for a controller function, but I didn't test that case. Is it possible to get route model binding while using this package?

masterix21 commented 1 month ago

Do you use our tenant middleware in your route?

michaelklopf commented 1 month ago

Yes I do

    Route::middleware(['tenant'])->group(function () {
        Route::get(
            '{tenant}/documents/{document}',
            DocumentPage::class
        );
    });

edit Well, we solved it with moving the middleware that sets the tenant before the SubstituteBinding middleware, see https://laravel.com/docs/11.x/middleware#sorting-middleware