From 347da8edf3e840116510193cd65bad06ce5125cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Fri, 30 May 2025 05:09:39 +0200 Subject: [PATCH] defaultParameterNames -> passQueryParameter --- .../Integrations/FortifyRouteBootstrapper.php | 33 ++++++++++--- src/Overrides/TenancyUrlGenerator.php | 24 ++++++--- src/Resolvers/RequestDataTenantResolver.php | 8 ++- .../FortifyRouteBootstrapperTest.php | 2 +- .../UrlGeneratorBootstrapperTest.php | 49 ++++++++++++++----- 5 files changed, 87 insertions(+), 29 deletions(-) diff --git a/src/Bootstrappers/Integrations/FortifyRouteBootstrapper.php b/src/Bootstrappers/Integrations/FortifyRouteBootstrapper.php index 05f3fa11..fb371d6a 100644 --- a/src/Bootstrappers/Integrations/FortifyRouteBootstrapper.php +++ b/src/Bootstrappers/Integrations/FortifyRouteBootstrapper.php @@ -8,6 +8,7 @@ use Illuminate\Config\Repository; use Stancl\Tenancy\Contracts\TenancyBootstrapper; use Stancl\Tenancy\Contracts\Tenant; use Stancl\Tenancy\Resolvers\PathTenantResolver; +use Stancl\Tenancy\Resolvers\RequestDataTenantResolver; /** * Allows customizing Fortify action redirects so that they can also redirect @@ -38,7 +39,7 @@ class FortifyRouteBootstrapper implements TenancyBootstrapper * is handled in another way (TenancyUrlGenerator::$passTenantParameter for both, * UrlGeneratorBootstrapper:$addTenantParameterToDefaults for path identification). */ - public static bool $passTenantParameter = true; + public static bool $passTenantParameter = false; /** * Tenant route that serves as Fortify's home (e.g. a tenant dashboard route). @@ -47,12 +48,22 @@ class FortifyRouteBootstrapper implements TenancyBootstrapper public static string|null $fortifyHome = 'tenant.dashboard'; /** - * Use default parameter names ('tenant' name and tenant key value) instead of the parameter name - * and column name configured in the path resolver config. + * Follow the query_parameter config instead of the tenant_parameter_name (path identification) config. * - * You want to enable this when using query string identification while having customized that config. + * This only has an effect when: + * - $passTenantParameter is enabled, and + * - the tenant_parameter_name config for the path resolver differs from the query_parameter config for the request data resolver. + * + * In such a case, instead of adding ['tenant' => '...'] to the route parameters (or whatever your tenant_parameter_name is if not 'tenant'), + * the query_parameter will be passed instead, e.g. ['team' => '...'] if your query_parameter config is 'team'. + * + * This is enabled by default because typically you will not need $passTenantParameter with path identification. + * UrlGeneratorBootstrapper::$addTenantParameterToDefaults is recommended instead when using path identification. + * + * On the other hand, when using request data identification (specifically query string) you WILL need to + * pass the parameter therefore you would use $passTenantParameter. */ - public static bool $defaultParameterNames = false; + public static bool $passQueryParameter = true; protected array $originalFortifyConfig = []; @@ -74,8 +85,14 @@ class FortifyRouteBootstrapper implements TenancyBootstrapper protected function useTenantRoutesInFortify(Tenant $tenant): void { - $tenantParameterName = static::$defaultParameterNames ? 'tenant' : PathTenantResolver::tenantParameterName(); - $tenantParameterValue = static::$defaultParameterNames ? $tenant->getTenantKey() : PathTenantResolver::tenantParameterValue($tenant); + if (static::$passQueryParameter) { + // todo@tests + $tenantParameterName = RequestDataTenantResolver::queryParameterName(); + $tenantParameterValue = RequestDataTenantResolver::payloadValue($tenant); + } else { + $tenantParameterName = PathTenantResolver::tenantParameterName(); + $tenantParameterValue = PathTenantResolver::tenantParameterValue($tenant); + } $generateLink = function (string $redirect) use ($tenantParameterValue, $tenantParameterName) { return route($redirect, static::$passTenantParameter ? [$tenantParameterName => $tenantParameterValue] : []); @@ -89,7 +106,7 @@ class FortifyRouteBootstrapper implements TenancyBootstrapper if (static::$fortifyHome) { // Generate the home route URL with the tenant parameter and make it the Fortify home route - $this->config->set('fortify.home', route(static::$fortifyHome, static::$passTenantParameter ? [$tenantParameterName => $tenantParameterValue] : [])); + $this->config->set('fortify.home', $generateLink(static::$fortifyHome)); } $this->config->set('fortify.redirects', $redirects); diff --git a/src/Overrides/TenancyUrlGenerator.php b/src/Overrides/TenancyUrlGenerator.php index 06ef7673..ed940e2f 100644 --- a/src/Overrides/TenancyUrlGenerator.php +++ b/src/Overrides/TenancyUrlGenerator.php @@ -9,6 +9,7 @@ use Illuminate\Routing\UrlGenerator; use Illuminate\Support\Arr; use InvalidArgumentException; use Stancl\Tenancy\Resolvers\PathTenantResolver; +use Stancl\Tenancy\Resolvers\RequestDataTenantResolver; /** * This class is used in place of the default UrlGenerator when UrlGeneratorBootstrapper is enabled. @@ -86,12 +87,22 @@ class TenancyUrlGenerator extends UrlGenerator public static array $overrides = []; /** - * Use default parameter names ('tenant' name and tenant key value) instead of the parameter name - * and column name configured in the path resolver config. + * Follow the query_parameter config instead of the tenant_parameter_name (path identification) config. * - * You want to enable this when using query string identification while having customized that config. + * This only has an effect when: + * - $passTenantParameterToRoutes is enabled, and + * - the tenant_parameter_name config for the path resolver differs from the query_parameter config for the request data resolver. + * + * In such a case, instead of adding ['tenant' => '...'] to the route parameters (or whatever your tenant_parameter_name is if not 'tenant'), + * the query_parameter will be passed instead, e.g. ['team' => '...'] if your query_parameter config is 'team'. + * + * This is enabled by default because typically you will not need $passTenantParameterToRoutes with path identification. + * UrlGeneratorBootstrapper::$addTenantParameterToDefaults is recommended instead when using path identification. + * + * On the other hand, when using request data identification (specifically query string) you WILL need to pass the parameter + * directly to route() calls, therefore you would use $passTenantParameterToRoutes to avoid having to do that manually. */ - public static bool $defaultParameterNames = false; + public static bool $passQueryParameter = true; /** * Override the route() method so that the route name gets prefixed @@ -175,9 +186,8 @@ class TenancyUrlGenerator extends UrlGenerator protected function addTenantParameter(array $parameters): array { if (tenant() && static::$passTenantParameterToRoutes) { - if (static::$defaultParameterNames) { - // todo0 this should be changed to something like static::$queryParameter and it should respect the configured tenant model column - return array_merge($parameters, ['tenant' => tenant()->getTenantKey()]); + if (static::$passQueryParameter) { + return array_merge($parameters, [RequestDataTenantResolver::queryParameterName() => RequestDataTenantResolver::payloadValue(tenant())]); } else { return array_merge($parameters, [PathTenantResolver::tenantParameterName() => PathTenantResolver::tenantParameterValue(tenant())]); } diff --git a/src/Resolvers/RequestDataTenantResolver.php b/src/Resolvers/RequestDataTenantResolver.php index 760b61f0..4d8b3277 100644 --- a/src/Resolvers/RequestDataTenantResolver.php +++ b/src/Resolvers/RequestDataTenantResolver.php @@ -31,11 +31,17 @@ class RequestDataTenantResolver extends Contracts\CachedTenantResolver public function getPossibleCacheKeys(Tenant&Model $tenant): array { + // todo@tests return [ - $this->formatCacheKey($tenant->getTenantKey()), + $this->formatCacheKey(static::payloadValue($tenant)), ]; } + public static function payloadValue(Tenant $tenant): string + { + return $tenant->getAttribute(static::tenantModelColumn()); + } + public static function tenantModelColumn(): string { return config('tenancy.identification.resolvers.' . static::class . '.tenant_model_column') ?? tenancy()->model()->getTenantKeyName(); diff --git a/tests/Bootstrappers/FortifyRouteBootstrapperTest.php b/tests/Bootstrappers/FortifyRouteBootstrapperTest.php index 1942a1c5..63f0f2a0 100644 --- a/tests/Bootstrappers/FortifyRouteBootstrapperTest.php +++ b/tests/Bootstrappers/FortifyRouteBootstrapperTest.php @@ -20,7 +20,7 @@ afterEach(function () { FortifyRouteBootstrapper::$passTenantParameter = true; FortifyRouteBootstrapper::$fortifyRedirectMap = []; FortifyRouteBootstrapper::$fortifyHome = 'tenant.dashboard'; - FortifyRouteBootstrapper::$defaultParameterNames = false; + FortifyRouteBootstrapper::$passQueryParameter = false; }); test('fortify route tenancy bootstrapper updates fortify config correctly', function() { diff --git a/tests/Bootstrappers/UrlGeneratorBootstrapperTest.php b/tests/Bootstrappers/UrlGeneratorBootstrapperTest.php index d1a0d1e3..39fcc475 100644 --- a/tests/Bootstrappers/UrlGeneratorBootstrapperTest.php +++ b/tests/Bootstrappers/UrlGeneratorBootstrapperTest.php @@ -17,6 +17,8 @@ use Illuminate\Support\Facades\Schema; use Stancl\Tenancy\Bootstrappers\UrlGeneratorBootstrapper; use Stancl\Tenancy\Exceptions\TenantCouldNotBeIdentifiedByRequestDataException; use Stancl\Tenancy\Middleware\InitializeTenancyByRequestData; +use Stancl\Tenancy\Resolvers\RequestDataTenantResolver; + use function Stancl\Tenancy\Tests\pest; beforeEach(function () { @@ -81,16 +83,14 @@ test('tenancy url generator can prefix route names passed to the route helper', test('path identification route helper behavior', function (bool $addTenantParameterToDefaults, bool $passTenantParameterToRoutes) { config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]); - $appUrl = config('app.url'); UrlGeneratorBootstrapper::$addTenantParameterToDefaults = $addTenantParameterToDefaults; TenancyUrlGenerator::$passTenantParameterToRoutes = $passTenantParameterToRoutes; $tenant = Tenant::create(); - $tenantKey = $tenant->getTenantKey(); - Route::get('/{tenant}/home', fn () => '') + Route::get('/{tenant}/home', fn () => tenant('id')) ->name('tenant.home') - ->middleware(['tenant', InitializeTenancyByPath::class]); + ->middleware([InitializeTenancyByPath::class]); tenancy()->initialize($tenant); @@ -98,7 +98,8 @@ test('path identification route helper behavior', function (bool $addTenantParam expect(fn () => route('tenant.home'))->toThrow(UrlGenerationException::class, 'Missing parameter: tenant'); } else { // If at least *one* of the approaches was used, the parameter will make its way to the route - expect(route('tenant.home'))->toBe("{$appUrl}/{$tenantKey}/home"); + expect(route('tenant.home'))->toBe("http://localhost/{$tenant->id}/home"); + pest()->get(route('tenant.home'))->assertSee($tenant->id); } })->with([true, false]) // UrlGeneratorBootstrapper::$addTenantParameterToDefaults ->with([true, false]); // TenancyUrlGenerator::$passTenantParameterToRoutes @@ -106,31 +107,55 @@ test('path identification route helper behavior', function (bool $addTenantParam test('request data identification route helper behavior', function (bool $addTenantParameterToDefaults, bool $passTenantParameterToRoutes) { config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]); - $appUrl = config('app.url'); UrlGeneratorBootstrapper::$addTenantParameterToDefaults = $addTenantParameterToDefaults; TenancyUrlGenerator::$passTenantParameterToRoutes = $passTenantParameterToRoutes; $tenant = Tenant::create(); - $tenantKey = $tenant->getTenantKey(); Route::get('/tenant/home', fn () => tenant('id')) ->name('tenant.home') - ->middleware(['tenant', InitializeTenancyByRequestData::class]); + ->middleware([InitializeTenancyByRequestData::class]); tenancy()->initialize($tenant); - // todo0 test changing tenancy.identification.resolvers..query_parameter and tenant_model_column - if ($passTenantParameterToRoutes) { - expect(route('tenant.home'))->toBe("{$appUrl}/tenant/home?tenant={$tenantKey}"); + // Only $passTenantParameterToRoutes has an effect, defaults do not affect request data URL generation + expect(route('tenant.home'))->toBe("http://localhost/tenant/home?tenant={$tenant->id}"); pest()->get(route('tenant.home'))->assertSee($tenant->id); } else { - expect(route('tenant.home'))->toBe("{$appUrl}/tenant/home"); + expect(route('tenant.home'))->toBe("http://localhost/tenant/home"); expect(fn () => $this->withoutExceptionHandling()->get(route('tenant.home')))->toThrow(TenantCouldNotBeIdentifiedByRequestDataException::class); } })->with([true, false]) // UrlGeneratorBootstrapper::$addTenantParameterToDefaults ->with([true, false]); // TenancyUrlGenerator::$passTenantParameterToRoutes +test('changing request data query parameter and model column is respected by the url generator', function () { + config([ + 'tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class], + 'tenancy.identification.resolvers.' . RequestDataTenantResolver::class . '.query_parameter' => 'team', + 'tenancy.identification.resolvers.' . RequestDataTenantResolver::class . '.tenant_model_column' => 'slug', + ]); + + Tenant::$extraCustomColumns = ['slug']; + + Schema::table('tenants', function (Blueprint $table) { + $table->string('slug')->unique(); + }); + + TenancyUrlGenerator::$passTenantParameterToRoutes = true; + + $tenant = Tenant::create(['slug' => 'acme']); + + Route::get('/tenant/home', fn () => tenant('id')) + ->name('tenant.home') + ->middleware([InitializeTenancyByRequestData::class]); + + tenancy()->initialize($tenant); + + expect(route('tenant.home'))->toBe("http://localhost/tenant/home?team=acme"); + pest()->get(route('tenant.home'))->assertSee($tenant->id); +}); + test('setting extra model columns sets additional URL defaults', function () { Tenant::$extraCustomColumns = ['slug']; TenancyUrlGenerator::$passTenantParameterToRoutes = false;