archtechx / tenancy

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

tenant_route domain or subdomain #1259

Closed mauritskorse closed 1 month ago

mauritskorse commented 1 month ago

Bug description

tenant_route only works for full domains and not when subdomains are passed.

I am trying to get the tenant_route() from central context to a tenant context. Some of my tenants use subdomain others have their own domain. Right now the tenant_route function assumes all passed domains are full domains and not subdomains. So when a subdomain is passed into tenant_route (for example using tenant()->domains->first()->domain ) the url that is created becomes something like https://sudomain/route/path instead of https://sudomain.central_domain/route/path.

possible solution would be something like adding

  if (!strstr($domain, '.')) {
      $domain = $domain.'.'.config('tenancy.central_domains')[0];
  }

Steps to reproduce

$domain = tenant()->domains->first()->domain;
$url = tenant_route($subdomain, 'app.dashboard');
// url becomes https://sudomain/route/path

Expected behavior

https://sudomain.central_domain/route/path

Laravel version

11

stancl/tenancy version

3.8.4

stancl commented 1 month ago

I don't understand how this is a bug.

stancl commented 1 month ago

The only real change we can make here is renaming the argument from $domain to $hostname to clarify the behavior. There's nothing else that can be done to the code.

$domain = $domain.'.'.config('tenancy.central_domains')[0];

What if there are multiple central domains, and a different one than [0] is meant to be used?

People have asked about this before, but it's really not a bug, at most you can submit an issue in the docs repo if you feel something should be clarified in the docs.

All other solutions would be flaky, for instance if we only used [0] if count($centralDomains) === 1, that would break apps if a new central domain is added.