archtechx
Add `tenant_path_route`
This PR adds the helper function, which automatically passes the tenant parameter.
Before
route('my-route-name', ['tenant' => tenant(), 'my-app-param' => 'foo'])
After
tenant_path_route('my-route-name', ['my-app-param' => 'foo'])
@stancl As you suggested, incorporating this change in the already existing helper method tenant_route which will be breaking change plus method signature issues.
Anyway, I came up with the implementation, and if you think the implementation is fine, we can update the PR.
tenant_route helper. Based on the identification, it will use the other helpers.
if (! function_exists('tenant_route')) {
function tenant_route($route, $parameters = [], string $domain, $absolute = true)
{
$url = route($route, $parameters, $absolute);
$route = app('router')->getRoutes()->match(request()->create($url));
$middlewares = Route::gatherRouteMiddleware($route);
if (in_array(\Stancl\Tenancy\Middleware\InitializeTenancyByDomain::class, $middlewares)
|| in_array(\Stancl\Tenancy\Middleware\InitializeTenancyByDomainOrSubdomain::class, $middlewares)) {
return tenant_domain_route($route, $parameters, $domain, $absolute);
}
if (in_array(\Stancl\Tenancy\Middleware\InitializeTenancyByPath::class, $middlewares)) {
return tenant_path_route($route, $parameters, $absolute);
}
}
}
if (! function_exists('tenant_path_route')) {
function tenant_path_route($route, $parameters = [], $absolute = true)
{
if (! array_key_exists(\Stancl\Tenancy\Resolvers\PathTenantResolver::$tenantParameterName, $parameters)) {
$parameters[\Stancl\Tenancy\Resolvers\PathTenantResolver::$tenantParameterName] = optional(tenant())->getTenantKey();
}
return route($route, $parameters, $absolute);
}
}
if (! function_exists('tenant_domain_route')) {
function tenant_domain_route($route, $parameters = [], string $domain, $absolute = true)
{
// replace first occurrence of hostname fragment with $domain
$url = route($route, $parameters, $absolute);
$hostname = parse_url($url, PHP_URL_HOST);
$position = strpos($url, $hostname);
return substr_replace($url, $domain, $position, strlen($hostname));
}
}
Usage
When using path identification It will be simple.
tenant_route('my-route-name', ['my-app-param' => 'foo'])
When using domain identification
tenant_route('my-route-name', ['my-app-param' => 'foo'], 'somedomain')
Want to make absolute parameter false when using path identification
tenant_route('my-route-name', ['my-app-param' => 'foo'], null, false) // using domain parameter is the issue here while we are on path identification, we need to pass domain = null.
Maybe the developer can use the named argument.
tenant_route('my-route-name', ['my-app-param' => 'foo'], absolute: false)
The main issue I see with using a single helper for both is the signature — right now we have $domain first, which makes sense.
I think we should first come up with a good signature and then deal with the implementation.
For the implementation, we could for example keep track of which identification middleware/resolver was used, and then just fetch that. Rather than having to deal with the router properties.
Also, an alternative solution to this entire tenant_path_route() thing could be figuring out if there's any way to specify a default value for the routes. I think there should be. So if we can do that at runtime, after routes are already defined, we could just tell Laravel to use the current tenant for all of those routes by default.
Then, the routes would be generated for the current tenant by default, or any other tenant when it's explicitly specified in the route() call.
Also, an alternative solution to this entire tenant_path_route() thing could be figuring out if there's any way to specify a default value for the routes. I think there should be. So if we can do that at runtime, after routes are already defined, we could just tell Laravel to use the current tenant for all of those routes by default.
Yes, we can use the URL::default feature. We will expose the middleware, let's say SetDefaultTenantForUrls which will simply set the tenant in middleware, and the developer can apply middleware on the route group. I think this is a cleaner way, plus we can keep the helper too if the developer doesn't want to use the middleware.
I think we should first come up with a good signature and then deal with the implementation.
Yes, the main issue is signature and avoiding big BC. But if we go with the URL::default approach, we can skip this helper. What do you suggest?
Can you attempt a separate PR with the implementation that sets the default tenant model?
Also, can you expand on the separate middleware? Would URL::default() set this for all routes with {tenant} or only routes where we apply some middleware?
If it's applied to all routes, then I don't see the point of the middleware? We could just add it to the existing logic.
Would URL::default() set this for all routes with {tenant} or only routes where we apply some middleware?
It's possible both ways. Don't think an app can have a route group with path identification and one group with domain identification. If this is possible, then a separate middleware makes sense. Plus, I think we shouldn't set tenant parameters on all routes out of the box by default.
Plus, I think we shouldn't set tenant parameters on all routes out of the box by default.
Then how would the middleware work? That was my question.
Then how would the middleware work?
We can just simply apply on the route group.
Route::group([
'prefix' => '/app/{tenant}',
'middleware' => [InitializeTenancyByPath::class, SetDefaultTenantForUrls::class],
], function () {
Middleware will be like this
class SetDefaultTenantForUrls
{
public function handle($request, Closure $next)
{
URL::defaults(['tenant' => tenant()->id]);
return $next($request);
}
}
Okay, but I don't see what the middleware does differently from simply executing URL::defaults() anywhere else? It's not like it will be limited to those routes. Re-read what I asked in the comments above.
If it's applied to all routes, then I don't see the point of the middleware? We could just add it to the existing logic.
I don't think this will be applied to all routes. It will be applied to the routes only implementing this middleware.
It will be applied to the routes only implementing this middleware.
Why? It will be applied to all routes, the only difference is that it's executed on a route with the middleware.
And when that's the case, I ask why do we even need a separate middleware? Since the presence of the middleware only controls if the URL::defaults() gets executed, we may as well put it into the existing path identification middleware.
Why? It will be applied to all routes, the only difference is that it's executed on a route with the middleware.
And when that's the case, I ask why do we even need a separate middleware? Since the presence of the middleware only controls if the
URL::defaults()gets executed, we may as well put it into the existing path identification middleware.