From 53ea46afe8f2e7a7a1c42f4567f2ac1e22ec0f14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Fri, 14 Feb 2025 13:47:06 +0100 Subject: [PATCH] cleanup --- .../UrlGeneratorBootstrapper.php | 11 ++- src/Overrides/TenancyUrlGenerator.php | 68 ++++++++++--------- .../UrlGeneratorBootstrapperTest.php | 20 +++--- 3 files changed, 48 insertions(+), 51 deletions(-) diff --git a/src/Bootstrappers/UrlGeneratorBootstrapper.php b/src/Bootstrappers/UrlGeneratorBootstrapper.php index f8e1a398..6158f22a 100644 --- a/src/Bootstrappers/UrlGeneratorBootstrapper.php +++ b/src/Bootstrappers/UrlGeneratorBootstrapper.php @@ -25,15 +25,14 @@ use Stancl\Tenancy\Resolvers\PathTenantResolver; class UrlGeneratorBootstrapper implements TenancyBootstrapper { /** - * Determine if the tenant route parameter should get added to the defaults of the TenancyUrlGenerator. + * Should the tenant route parameter get added to TenancyUrlGenerator::defaults(). * - * This is preferrable with path identification since the tenant parameter is passed to the tenant routes automatically, - * even with integrations like the Ziggy route() helper. + * This is recommended when using path identification since defaults() generally has better support in integrations, + * namely Ziggy, compared to TenancyUrlGenerator::$passTenantParameterToRoutes. * - * With query strig identification, this essentialy has no effect because URL::defaults() works only for route paramaters, - * not for query strings. + * With query string identification, this has no effect since URL::defaults() only works for route paramaters. */ - public static bool $addTenantParameterToDefaults = false; + public static bool $addTenantParameterToDefaults = true; public function __construct( protected Application $app, diff --git a/src/Overrides/TenancyUrlGenerator.php b/src/Overrides/TenancyUrlGenerator.php index 211c90e9..53798c4e 100644 --- a/src/Overrides/TenancyUrlGenerator.php +++ b/src/Overrides/TenancyUrlGenerator.php @@ -13,75 +13,77 @@ use Stancl\Tenancy\Resolvers\PathTenantResolver; /** * This class is used in place of the default UrlGenerator when UrlGeneratorBootstrapper is enabled. * - * TenancyUrlGenerator does two extra things: + * TenancyUrlGenerator does a few extra things: + * - Autofills the tenant parameter in the tenant context with the current tenant. + * This is done either by: + * - URL::defaults() -- if UrlGeneratorBootstrapper::$addTenantParameterToDefaults is enabled. + * This generally has the best support since tools like e.g. Ziggy read defaults(). + * - Automatically passing ['tenant' => ...] to each route() call -- if TenancyUrlGenerator::$passTenantParameterToRoutes is enabled + * This is a more universal solution since it supports both path identification and query parameter identification. * - * - Autofills the tenant parameter in the tenant context with the current tenant if $passTenantParameterToRoutes is enabled. - * With path identification, this is required to be done by URL::defaults() -- setting - * UrlGeneratorBootstrapper::$addTenantParameterToDefaults (which is false by default) to true handles that. - * Enabling $passTenantParameterToRoutes is preferable with query string identification. + * - Prepends route names passed to route() and URL::temporarySignedRoute() + * with `tenant.` (or the configured prefix) if $prefixRouteNames is enabled. + * This is primarily useful when using route cloning with path identification. * - * - Prepends the route name passed to route() and URL::temporarySignedRoute() - * with `tenant.` (or the configured prefix) if $prefixRouteNames is enabled (disabled by default). - * Primarily intended to be used with path identification. - * - * This behavior can be bypassed by passing the $bypassParameter to the mentioned methods (`['central' => true]` by default). + * To bypass this behavior on any single route() call, pass the $bypassParameter as true (['central' => true] by default). */ class TenancyUrlGenerator extends UrlGenerator { /** - * Parameter which bypasses the behavior modification of route() and temporarySignedRoute(). + * Parameter which works as a flag for bypassing the behavior modification of route() and temporarySignedRoute(). * * For example, in tenant context: * Route::get('/', ...)->name('home'); + * // query string identification * Route::get('/tenant', ...)->middleware(InitializeTenancyByRequestData::class)->name('tenant.home'); * - route('home') => app.test/tenant?tenant=tenantKey * - route('home', [$bypassParameter => true]) => app.test/ - * - route('tenant.home', [$bypassParameter => true]) => app.test/tenant -- query string identification (no query string passed) + * - route('tenant.home', [$bypassParameter => true]) => app.test/tenant -- no query string added * - * With path identification, it is recommended to pass the tenant parameter automatically by setting - * UrlGeneratorBootstrapper::$addTenantParameterToDefaults to true, - * and in that case, this affects only the automatic route name prefixing, - * and the forceful route name overrides set in the $override property. + * Note: UrlGeneratorBootstrapper::$addTenantParameterToDefaults is not affected by this, though + * it doesn't matter since it doesn't pass any extra parameters when not needed. * * @see UrlGeneratorBootstrapper */ public static string $bypassParameter = 'central'; /** - * Determine if the route names passed to `route()` or `temporarySignedRoute()` - * should get prefixed with the tenant route name prefix. + * Should route names passed to route() or temporarySignedRoute() + * get prefixed with the tenant route name prefix. * - * This is useful when using path identification with packages that generate URLs, - * like Jetstream, so that you don't have to manually prefix route names passed to each route() call. + * This is useful when using e.g. path identification with third-party packages + * where you don't have control over all route() calls or don't want to change + * too many files. Often this will be when using route cloning. */ public static bool $prefixRouteNames = false; /** - * Determine if the tenant parameter should get passed - * to the links generated by `route()` or `temporarySignedRoute()` whenever available. - * This is primarily intended to be used with query string identification. + * Should the tenant parameter be passed to route() or temporarySignedRoute() calls. * - * With path identification, the tenant parameter is passed by URL::defaults() - * when UrlGeneratorBootstrapper::$addTenantParameterToDefaults is true (which is the default). + * This is useful with path or query parameter identification. The former can be handled + * more elegantly using UrlGeneratorBootstrapper::$addTenantParameterToDefaults. * * @see UrlGeneratorBootstrapper */ public static bool $passTenantParameterToRoutes = false; /** - * Route names that should always be overridden. - * This behavior can still be bypassed by passing the bypass parameter. + * Route name overrides. * - * For example, Jetstream integration: + * Note: This behavior can be bypassed using $bypassParameter just like + * $prefixRouteNames and $passTenantParameterToRoutes. + * + * Example from a Jetstream integration: * [ * 'profile.show' => 'tenant.profile.show', * 'two-factor.login' => 'tenant.two-factor.login', * ] * - * `route('profile.show')` will return an URL as if you called `route('tenant.profile.show')`. - * `route('profile.show', ['central' => true])` will return an URL as if you called `route('profile.show')`. + * In the tenant context: + * - `route('profile.show')` will return a URL as if you called `route('tenant.profile.show')`. + * - `route('profile.show', ['central' => true])` will return a URL as if you called `route('profile.show')`. */ - public static array $override = []; + public static array $overrides = []; /** * Override the route() method so that the route name gets prefixed @@ -160,7 +162,7 @@ class TenancyUrlGenerator extends UrlGenerator } /** - * If `tenant()` isn't null, add tenant parameter to the passed parameters. + * If `tenant()` isn't null, add the tenant parameter to the passed parameters. */ protected function addTenantParameter(array $parameters): array { @@ -169,6 +171,6 @@ class TenancyUrlGenerator extends UrlGenerator protected function routeNameOverride(string $name): string|null { - return static::$override[$name] ?? null; + return static::$overrides[$name] ?? null; } } diff --git a/tests/Bootstrappers/UrlGeneratorBootstrapperTest.php b/tests/Bootstrappers/UrlGeneratorBootstrapperTest.php index 1e99e277..77d50073 100644 --- a/tests/Bootstrappers/UrlGeneratorBootstrapperTest.php +++ b/tests/Bootstrappers/UrlGeneratorBootstrapperTest.php @@ -62,10 +62,8 @@ test('tenancy url generator can prefix route names passed to the route helper', // When $prefixRouteNames is true, the route name passed to the route() helper ('home') gets prefixed with 'tenant.' automatically. TenancyUrlGenerator::$prefixRouteNames = true; - // Reinitialize tenancy to apply the URL generator changes - tenancy()->initialize($tenant); - expect(route('home'))->toBe($tenantRouteUrl); + // The 'tenant.home' route name doesn't get prefixed -- it is already prefixed with 'tenant.' expect(route('tenant.home'))->toBe($tenantRouteUrl); @@ -86,8 +84,8 @@ test('the route helper can receive the tenant parameter automatically', function UrlGeneratorBootstrapper::$addTenantParameterToDefaults = $addTenantParameterToDefaults; - // When the tenant parameter isn't added to the defaults, the tenant parameter has to be passed "manually" - // by setting $passTenantParameterToRoutes to true. This is only preferrable with query string identification. + // When the tenant parameter isn't added to defaults, the tenant parameter has to be passed "manually" + // by setting $passTenantParameterToRoutes to true. This is only preferable with query string identification. // With path identification, this ultimately doesn't have any effect // if UrlGeneratorBootstrapper::$addTenantParameterToDefaults is true, // but TenancyUrlGenerator::$passTenantParameterToRoutes can still be used instead. @@ -122,14 +120,14 @@ test('the route helper can receive the tenant parameter automatically', function ->with([true, false]) // UrlGeneratorBootstrapper::$addTenantParameterToDefaults ->with([true, false]); // TenancyUrlGenerator::$passTenantParameterToRoutes -test('url generator can override specific route names while all other functionality is disabled', function() { +test('url generator can override specific route names', function() { config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]); Route::get('/foo', fn () => 'foo')->name('foo'); Route::get('/bar', fn () => 'bar')->name('bar'); Route::get('/baz', fn () => 'baz')->name('baz'); // Not overridden - TenancyUrlGenerator::$override = ['foo' => 'bar']; + TenancyUrlGenerator::$overrides = ['foo' => 'bar']; expect(route('foo'))->toBe(url('/foo')); expect(route('bar'))->toBe(url('/bar')); @@ -138,13 +136,11 @@ test('url generator can override specific route names while all other functional tenancy()->initialize(Tenant::create()); expect(route('foo'))->toBe(url('/bar')); + expect(route('bar'))->toBe(url('/bar')); // not overridden + expect(route('baz'))->toBe(url('/baz')); // not overridden - // Pass the bypass parameter bypasses the override + // Bypass the override expect(route('foo', ['central' => true]))->toBe(url('/foo')); - - // Not overridden - expect(route('bar'))->toBe(url('/bar')); - expect(route('baz'))->toBe(url('/baz')); }); test('both the name prefixing and the tenant parameter logic gets skipped when bypass parameter is used', function () {