[4.x] URL generation, request data identification improvements (#1357)

* UrlGenerator: set defaults based on config; request data: move config to config file+resolver * Claude code adjustments * improve request data tests, simplify complex test in UrlGeneratorBootstrapperTest * url generator test: test changing tenant parameter name * request data identification: add tenant_model_column configuration * defaultParameterNames -> passQueryParameter * move comment * minor refactor in PathIdentificationTest, expand CLAUDE.md to include early identification section * Fix COLOR_FLAG * improve test name Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * TenancyUrlGenerator: add a check for queryParameterName being null Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Fix code style (php-cs-fixer) --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
2025-12-12 14:34:04 +00:00 · 2025-06-02 03:43:47 +02:00 · 2025-06-02 03:43:47 +02:00 · 5f7fd38e5a
commit 5f7fd38e5a
parent f4cc99b317
13 changed files with 440 additions and 126 deletions
--- a/tests/Bootstrappers/UrlGeneratorBootstrapperTest.php
+++ b/tests/Bootstrappers/UrlGeneratorBootstrapperTest.php
@ -1,5 +1,6 @@
 <?php

+use Illuminate\Database\Schema\Blueprint;
 use Illuminate\Routing\UrlGenerator;
 use Stancl\Tenancy\Tests\Etc\Tenant;
 use Illuminate\Support\Facades\Event;
@ -12,8 +13,13 @@ use Stancl\Tenancy\Overrides\TenancyUrlGenerator;
 use Stancl\Tenancy\Listeners\RevertToCentralContext;
 use Stancl\Tenancy\Middleware\InitializeTenancyByPath;
 use Illuminate\Routing\Exceptions\UrlGenerationException;
+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 () {
    Event::listen(TenancyInitialized::class, BootstrapTenancy::class);
@ -44,82 +50,224 @@ test('url generator bootstrapper swaps the url generator instance correctly', fu
 });

 test('tenancy url generator can prefix route names passed to the route helper', function() {
-    Route::get('/central/home', fn () => route('home'))->name('home');
-    // Tenant route name prefix is 'tenant.' by default
-    Route::get('/tenant/home', fn () => route('tenant.home'))->name('tenant.home');
+    config([
+        'tenancy.identification.resolvers.' . PathTenantResolver::class . '.tenant_route_name_prefix' => 'custom_prefix.',
+    ]);
+
+    Route::get('/central/home', fn () => '')->name('home');
+    Route::get('/tenant/home', fn () => '')->name('custom_prefix.home');

    $tenant = Tenant::create();
-    $centralRouteUrl = route('home');
-    $tenantRouteUrl = route('tenant.home');

    config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]);

    tenancy()->initialize($tenant);

    // Route names don't get prefixed when TenancyUrlGenerator::$prefixRouteNames is false (default)
-    expect(route('home'))->toBe($centralRouteUrl);
+    expect(route('home'))->toBe('http://localhost/central/home');

-    // When $prefixRouteNames is true, the route name passed to the route() helper ('home') gets prefixed with 'tenant.' automatically.
+    // When $prefixRouteNames is true, the route name passed to the route() helper ('home') gets prefixed automatically.
    TenancyUrlGenerator::$prefixRouteNames = true;

-    expect(route('home'))->toBe($tenantRouteUrl);
+    expect(route('home'))->toBe('http://localhost/tenant/home');

-    // The 'tenant.home' route name doesn't get prefixed -- it is already prefixed with 'tenant.'
-    expect(route('tenant.home'))->toBe($tenantRouteUrl);
+    // The 'custom_prefix.home' route name doesn't get prefixed -- it is already prefixed with 'custom_prefix.'
+    expect(route('custom_prefix.home'))->toBe('http://localhost/tenant/home');

    // Ending tenancy reverts route() behavior changes
    tenancy()->end();

-    expect(route('home'))->toBe($centralRouteUrl);
+    expect(route('home'))->toBe('http://localhost/central/home');
 });

-test('the route helper can receive the tenant parameter automatically', function (
-    string $identification,
-    bool $addTenantParameterToDefaults,
-    bool $passTenantParameterToRoutes,
-) {
+test('path identification route helper behavior', function (bool $addTenantParameterToDefaults, bool $passTenantParameterToRoutes) {
    config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]);

-    $appUrl = config('app.url');
-
    UrlGeneratorBootstrapper::$addTenantParameterToDefaults = $addTenantParameterToDefaults;
-
-    // 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.
    TenancyUrlGenerator::$passTenantParameterToRoutes = $passTenantParameterToRoutes;

    $tenant = Tenant::create();
-    $tenantKey = $tenant->getTenantKey();

-    Route::get('/central/home', fn () => route('home'))->name('home');
-
-    $tenantRoute = $identification === InitializeTenancyByPath::class ? "/{tenant}/home" : "/tenant/home";
-
-    Route::get($tenantRoute, fn () => route('tenant.home'))
+    Route::get('/{tenant}/home', fn () => tenant('id'))
        ->name('tenant.home')
-        ->middleware(['tenant', $identification]);
+        ->middleware([InitializeTenancyByPath::class]);

    tenancy()->initialize($tenant);

-    $expectedUrl = match (true) {
-        $identification === InitializeTenancyByRequestData::class && $passTenantParameterToRoutes => "{$appUrl}/tenant/home?tenant={$tenantKey}",
-        $identification === InitializeTenancyByRequestData::class => "{$appUrl}/tenant/home", // $passTenantParameterToRoutes is false
-        $identification === InitializeTenancyByPath::class && ($addTenantParameterToDefaults || $passTenantParameterToRoutes) => "{$appUrl}/{$tenantKey}/home",
-        $identification === InitializeTenancyByPath::class => null, // Should throw an exception -- route() doesn't receive the tenant parameter in this case
-    };
-
-    if ($expectedUrl === null) {
+    if (! $addTenantParameterToDefaults && ! $passTenantParameterToRoutes) {
        expect(fn () => route('tenant.home'))->toThrow(UrlGenerationException::class, 'Missing parameter: tenant');
    } else {
-        expect(route('tenant.home'))->toBe($expectedUrl);
+        // If at least *one* of the approaches was used, the parameter will make its way to the route
+        expect(route('tenant.home'))->toBe("http://localhost/{$tenant->id}/home");
+        pest()->get(route('tenant.home'))->assertSee($tenant->id);
    }
-})->with([InitializeTenancyByPath::class, InitializeTenancyByRequestData::class])
-    ->with([true, false]) // UrlGeneratorBootstrapper::$addTenantParameterToDefaults
+})->with([true, false]) // UrlGeneratorBootstrapper::$addTenantParameterToDefaults
    ->with([true, false]); // TenancyUrlGenerator::$passTenantParameterToRoutes

+test('request data identification route helper behavior', function (bool $addTenantParameterToDefaults, bool $passTenantParameterToRoutes) {
+    config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]);
+
+    UrlGeneratorBootstrapper::$addTenantParameterToDefaults = $addTenantParameterToDefaults;
+    TenancyUrlGenerator::$passTenantParameterToRoutes = $passTenantParameterToRoutes;
+
+    $tenant = Tenant::create();
+
+    Route::get('/tenant/home', fn () => tenant('id'))
+        ->name('tenant.home')
+        ->middleware([InitializeTenancyByRequestData::class]);
+
+    tenancy()->initialize($tenant);
+
+    if ($passTenantParameterToRoutes) {
+        // 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("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;
+    UrlGeneratorBootstrapper::$addTenantParameterToDefaults = true;
+
+    config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]);
+    config(['tenancy.identification.resolvers.' . PathTenantResolver::class . '.allowed_extra_model_columns' => ['slug']]);
+
+    Schema::table('tenants', function (Blueprint $table) {
+        $table->string('slug')->unique();
+    });
+
+    Route::get('/{tenant}/foo/{user}', function (string $user) {
+        return tenant()->getTenantKey() . " $user";
+    })->middleware([InitializeTenancyByPath::class, 'web'])->name('foo');
+
+    Route::get('/{tenant:slug}/fooslug/{user}', function (string $user) {
+        return tenant()->getTenantKey() . " $user";
+    })->middleware([InitializeTenancyByPath::class, 'web'])->name('fooslug');
+
+    $tenant = Tenant::create(['slug' => 'acme']);
+
+    // In central context, no URL defaults are applied
+    expect(route('foo', [$tenant->getTenantKey(), 'bar']))->toBe("http://localhost/{$tenant->getTenantKey()}/foo/bar");
+    pest()->get(route('foo', [$tenant->getTenantKey(), 'bar']))->assertSee(tenant()->getTenantKey() . ' bar');
+    tenancy()->end();
+
+    expect(route('fooslug', ['acme', 'bar']))->toBe('http://localhost/acme/fooslug/bar');
+    pest()->get(route('fooslug', ['acme', 'bar']))->assertSee(tenant()->getTenantKey() . ' bar');
+    tenancy()->end();
+
+    // In tenant context, URL defaults are applied
+    tenancy()->initialize($tenant);
+    expect(route('foo', ['bar']))->toBe("http://localhost/{$tenant->getTenantKey()}/foo/bar");
+    pest()->get(route('foo', ['bar']))->assertSee(tenant()->getTenantKey() . ' bar');
+
+    expect(route('fooslug', ['bar']))->toBe('http://localhost/acme/fooslug/bar');
+    pest()->get(route('fooslug', ['bar']))->assertSee(tenant()->getTenantKey() . ' bar');
+});
+
+test('changing the tenant model column changes the default value for the tenant parameter', function () {
+    Tenant::$extraCustomColumns = ['slug'];
+    TenancyUrlGenerator::$passTenantParameterToRoutes = false;
+    UrlGeneratorBootstrapper::$addTenantParameterToDefaults = true;
+
+    config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]);
+    config(['tenancy.identification.resolvers.' . PathTenantResolver::class . '.tenant_model_column' => 'slug']);
+
+    Schema::table('tenants', function (Blueprint $table) {
+        $table->string('slug')->unique();
+    });
+
+    Route::get('/{tenant}/foo/{user}', function (string $user) {
+        return tenant()->getTenantKey() . " $user";
+    })->middleware([InitializeTenancyByPath::class, 'web'])->name('foo');
+
+    $tenant = Tenant::create(['slug' => 'acme']);
+
+    // In central context, no URL defaults are applied
+    expect(route('foo', ['acme', 'bar']))->toBe("http://localhost/acme/foo/bar");
+    pest()->get(route('foo', ['acme', 'bar']))->assertSee(tenant()->getTenantKey() . ' bar');
+    tenancy()->end();
+
+    // In tenant context, URL defaults are applied
+    tenancy()->initialize($tenant);
+    expect(route('foo', ['bar']))->toBe("http://localhost/acme/foo/bar");
+    pest()->get(route('foo', ['bar']))->assertSee(tenant()->getTenantKey() . ' bar');
+});
+
+test('changing the tenant parameter name is respected by the url generator', function () {
+    Tenant::$extraCustomColumns = ['slug', 'slug2'];
+    TenancyUrlGenerator::$passTenantParameterToRoutes = false;
+    UrlGeneratorBootstrapper::$addTenantParameterToDefaults = true;
+
+    config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]);
+    config(['tenancy.identification.resolvers.' . PathTenantResolver::class . '.tenant_parameter_name' => 'team']);
+    config(['tenancy.identification.resolvers.' . PathTenantResolver::class . '.tenant_model_column' => 'slug']);
+    config(['tenancy.identification.resolvers.' . PathTenantResolver::class . '.allowed_extra_model_columns' => ['slug2']]);
+
+    Schema::table('tenants', function (Blueprint $table) {
+        $table->string('slug')->unique();
+        $table->string('slug2')->unique();
+    });
+
+    Route::get('/{team}/foo/{user}', function (string $user) {
+        return tenant()->getTenantKey() . " $user";
+    })->middleware([InitializeTenancyByPath::class, 'web'])->name('foo');
+
+    Route::get('/{team:slug2}/fooslug2/{user}', function (string $user) {
+        return tenant()->getTenantKey() . " $user";
+    })->middleware([InitializeTenancyByPath::class, 'web'])->name('fooslug2');
+
+    $tenant = Tenant::create(['slug' => 'acme', 'slug2' => 'acme2']);
+
+    // In central context, no URL defaults are applied
+    expect(route('foo', ['acme', 'bar']))->toBe("http://localhost/acme/foo/bar");
+    pest()->get(route('foo', ['acme', 'bar']))->assertSee(tenant()->getTenantKey() . ' bar');
+    tenancy()->end();
+
+    expect(route('fooslug2', ['acme2', 'bar']))->toBe("http://localhost/acme2/fooslug2/bar");
+    pest()->get(route('fooslug2', ['acme2', 'bar']))->assertSee(tenant()->getTenantKey() . ' bar');
+    tenancy()->end();
+
+    // In tenant context, URL defaults are applied
+    tenancy()->initialize($tenant);
+    expect(route('foo', ['bar']))->toBe("http://localhost/acme/foo/bar");
+    pest()->get(route('foo', ['bar']))->assertSee(tenant()->getTenantKey() . ' bar');
+
+    expect(route('fooslug2', ['bar']))->toBe("http://localhost/acme2/fooslug2/bar");
+    pest()->get(route('fooslug2', ['bar']))->assertSee(tenant()->getTenantKey() . ' bar');
+});
+
 test('url generator can override specific route names', function() {
    config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]);