archtechx / tenancy

Automatic multi-tenancy for Laravel. No code changes needed.
https://tenancyforlaravel.com
MIT License
3.55k stars 425 forks source link

Route helper in job doesn't use tenant context domain #1218

Closed giorgiopogliani closed 4 months ago

giorgiopogliani commented 4 months ago

Bug description

I am sending an email in a job and I am passing to the template an url using the route helper. The email is sent with the central domain instead of the tenant context domain.

Logging tenancy()->tenant shows that the tenant is correct so I guess the tenant is initialized?

My setup is using multi database tenancy

  Drivers
  Broadcasting ................................... log  
  Cache ........................................ redis  
  Database ..................................... mysql  
  Logs ................................ stack / single  
  Mail .......................................... smtp  
  Queue ........................................ redis  
  Session ...................................... redis

My bootstrappers:

'bootstrappers' => [
        Stancl\Tenancy\Bootstrappers\DatabaseTenancyBootstrapper::class,
        Stancl\Tenancy\Bootstrappers\CacheTenancyBootstrapper::class,
        Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper::class,
        Stancl\Tenancy\Bootstrappers\QueueTenancyBootstrapper::class,
        Stancl\Tenancy\Bootstrappers\RedisTenancyBootstrapper::class,
]

Steps to reproduce

Dispatch a job from a tenant context and log any route using the route helper.

Log::info('NOT IN JOB:' . tenancy()->tenenat->id);
Log::info('NOT IN JOB:' . route('dashboard'));
dispatch(new MyJob());

The job class handle method

// ...
public function handle() 
{
    Log::info('IN JOB:' . tenancy()->tenenat->id);
    Log::info('IN JOB:' . route('dashboard'));
}

Logs in the laravel.log file:

[2024-05-02 12:37:43] local.INFO: NOT IN JOB: 1  
[2024-05-02 12:37:43] local.INFO: NOT IN JOB: http://tenant1.myapp.test/dashboard
[2024-05-02 12:37:45] local.INFO: IN JOB: 1  
[2024-05-02 12:37:45] local.INFO: IN JOB: http://myapp.test/dashboard 

Expected behavior

As tenancy()->tenant has the correct tennat I would expect the route helper to use the tenant domain.

Laravel version

10.48.7

stancl/tenancy version

v3.8.1

stancl commented 4 months ago

This isn't done by the package automatically because there's no way for the package to know what the correct domain is:

You should manually update app.url or use app('url')->forceRoot() with the right hostname.

We've added a bootstrapper for this in v4 but it still requires configuration due to the reasons above.

giorgiopogliani commented 4 months ago

Could we add a primary domain config somewhere, maybe as extra column in the domains table?

stancl commented 4 months ago

Yes, you can do that. It's up to you how you approach this, the right solution here will be application-specific.

giorgiopogliani commented 4 months ago

Ok, might be a common use case, anyway I'll handle it. Thanks for the super fast reply!

stancl commented 4 months ago

In v4 we've added:

In the SaaS boilerplate we use this primary domain concept. iirc it's a hasOne() relation from the tenant.

The package won't ship with primary domains out of the box since they only make sense when you really do need multiple domains and have some UI for letting the tenant manage their domains. But the changes we've made should add a good default implementation for this use case.