[MINOR BC] Create pending tenants with pending_since, improve --with-pending (#1458)

> Minor breaking change: Pending tenants would previously go through the
creation pipeline as *not* pending and would only be marked as pending
after full creation. Now, pending tenants go through the creation
process with pending_since set from the start.

Pending tenants aren't getting their `pending_since` set until they're
created completely (e.g. their DB was created, migrated and seeded --
first, the tenant is created fully, and only after that, the tenant is
updated to have `pending_since`). This is a problem if someone wants to
e.g. add a job to the `DatabaseCreated` job pipeline that would check
`$this->tenant->pending()`. Since at the point of `DatabaseCreated`, the
tenant's `pending_since` isn't set yet, `$this->tenant->pending()`
returns `false`, even for tenants created using `createPending()`. So
instead of letting the pending tenant get fully created, and only after
that, setting its `pending_since` (using `update()`), we now set
`pending_since` in `create()`. `CreatingPendingTenant` is now dispatched
from the `static::creating` hook, and `PendingTenantCreated` is
dispatched from `static::created` for consistency.

Setting `pending_since` right in `create()` made the `MigrateDatabase`
and `SeedDatabase` jobs exclude the pending tenants during their
creation if the `tenancy.pending.include_in_queries` config was set to
`false` -- in that case, these jobs would never migrate or seed the
databases of pending tenants. So these jobs now pass `--with-pending` to
their underlying commands, with the value set in their `$includePending`
static property (`true` by default). This overrides the
`tenancy.pending.include_in_queries` config -- unless the
`$includePending` properties are set to `false`, these jobs will always
include pending tenants.

The `--with-pending` tenant command option originally worked just to
opt-in for including pending tenants in the command. Now,
`--with-pending` can accept values (`true`/`1` or `false`/`0`), so e.g.
- `tenants:run foo` with
`--with-pending`/`--with-pending=true`/`--with-pending=1` includes
pending tenants
- `tenants:run foo` with `--with-pending=false`/`--with-pending=0`
**excludes** pending tenants (also `--with-pending=foobar` -- invalid
input, considered `false`)

Passing `--with-pending` makes the command bypass the
`tenancy.pending.include_in_queries` config (so e.g. if
`tenancy.pending.include_in_queries` is set to `true`, and
`--with-pending=false` is passed to a command, the command will exclude
pending tenants). When `--with-pending` is not passed, the command will
include or exclude pending tenants based on the
`tenancy.pending.include_in_queries` config.

---------

Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Samuel Štancl <samuel@archte.ch>

Author: 3 people

src/Concerns/HasTenantOptions.php

@@ -18,7 +18,7 @@ protected function getOptions()
     {
         return array_merge([
            new InputOption('tenants', null, InputOption::VALUE_IS_ARRAY|InputOption::VALUE_OPTIONAL, 'The tenants to run this command for. Leave empty for all tenants', null),
-           new InputOption('with-pending', null, InputOption::VALUE_NONE, 'Include pending tenants in query'), // todo@pending should we also offer without-pending? if we add this, mention in docs
+           new InputOption('with-pending', null, InputOption::VALUE_OPTIONAL, 'Include pending tenants in query if true/1, exclude if false/0. Defaults to the tenancy.pending.include_in_queries config value.'),
         ], parent::getOptions());
     }
 
@@ -43,7 +43,11 @@ protected function getTenantsQuery(?array $tenantKeys = null): Builder
                 $query->whereIn(tenancy()->model()->getTenantKeyName(), $this->option('tenants'));
             })
             ->when(tenancy()->model()::hasGlobalScope(PendingScope::class), function ($query) {
-                $query->withPending(config('tenancy.pending.include_in_queries') ?: $this->option('with-pending'));
+                $includePending = $this->input->hasParameterOption('--with-pending')
+                    ? filter_var($this->option('with-pending') ?? true, FILTER_VALIDATE_BOOLEAN)
+                    : config('tenancy.pending.include_in_queries');
+
+                $query->withPending($includePending);
             });
     }
 

src/Database/Concerns/HasPending.php

@@ -28,6 +28,18 @@ trait HasPending
     public static function bootHasPending(): void
     {
         static::addGlobalScope(new PendingScope());
+
+        static::creating(function (self $tenant): void {
+            if ($tenant->pending()) {
+                event(new CreatingPendingTenant($tenant));
+            }
+        });
+
+        static::created(function (self $tenant): void {
+            if ($tenant->pending()) {
+                event(new PendingTenantCreated($tenant));
+            }
+        });
     }
 
     /** Initialize the trait. */
@@ -49,22 +61,11 @@ public function pending(): bool
      */
     public static function createPending(array $attributes = []): Model&Tenant
     {
-        $tenant = null;
-
-        try {
-            $tenant = static::create(array_merge(static::getPendingAttributes($attributes), $attributes));
-            event(new CreatingPendingTenant($tenant));
-        } finally {
-            // Update the pending_since value only after the tenant is created so it's
-            // not marked as pending until after migrations, seeders, etc are run.
-            $tenant?->update([
-                'pending_since' => now()->timestamp,
-            ]);
-        }
-
-        event(new PendingTenantCreated($tenant));
-
-        return $tenant;
+        return static::create(array_merge(
+            static::getPendingAttributes($attributes),
+            $attributes,
+            ['pending_since' => now()->timestamp],
+        ));
     }
 
     /**

src/Jobs/MigrateDatabase.php

@@ -17,6 +17,16 @@ class MigrateDatabase implements ShouldQueue
 {
     use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;
 
+    /**
+     * Should pending tenants be included while migrating,
+     * regardless of the tenancy.pending.include_in_queries config value.
+     *
+     * If false, pending tenants will be specifically excluded.
+     *
+     * If null, default to tenancy.pending.include_in_queries config.
+     */
+    public static ?bool $includePending = true;
+
     public function __construct(
         protected TenantWithDatabase&Model $tenant,
     ) {}
@@ -25,6 +35,7 @@ public function handle(): void
     {
         Artisan::call('tenants:migrate', [
             '--tenants' => [$this->tenant->getTenantKey()],
+            '--with-pending' => static::$includePending ?? config('tenancy.pending.include_in_queries'),
         ]);
     }
 }

src/Jobs/SeedDatabase.php

@@ -17,6 +17,16 @@ class SeedDatabase implements ShouldQueue
 {
     use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;
 
+    /**
+     * Should pending tenants be included while seeding,
+     * regardless of the tenancy.pending.include_in_queries config value.
+     *
+     * If false, pending tenants will be specifically excluded.
+     *
+     * If null, default to tenancy.pending.include_in_queries config.
+     */
+    public static ?bool $includePending = true;
+
     public function __construct(
         protected TenantWithDatabase&Model $tenant,
     ) {}
@@ -25,6 +35,7 @@ public function handle(): void
     {
         Artisan::call('tenants:seed', [
             '--tenants' => [$this->tenant->getTenantKey()],
+            '--with-pending' => static::$includePending ?? config('tenancy.pending.include_in_queries'),
         ]);
     }
 }

tests/PendingTenantsTest.php

@@ -16,10 +16,25 @@
 use Stancl\Tenancy\Events\PullingPendingTenant;
 use Stancl\Tenancy\Tests\Etc\Tenant;
 use function Stancl\Tenancy\Tests\pest;
+use Stancl\Tenancy\Events\TenantCreated;
+use Stancl\JobPipeline\JobPipeline;
+use Stancl\Tenancy\Jobs\CreateDatabase;
+use Stancl\Tenancy\Jobs\MigrateDatabase;
+use Stancl\Tenancy\Jobs\SeedDatabase;
+use Stancl\Tenancy\Tests\Etc\User;
+use Stancl\Tenancy\Tests\Etc\TestSeeder;
+use Stancl\Tenancy\Bootstrappers\DatabaseTenancyBootstrapper;
+use Stancl\Tenancy\Events\TenancyInitialized;
+use Stancl\Tenancy\Listeners\BootstrapTenancy;
+use Stancl\Tenancy\Events\TenancyEnded;
+use Stancl\Tenancy\Listeners\RevertToCentralContext;
 
 beforeEach($cleanup = function () {
     Tenant::$extraCustomColumns = [];
     Tenant::$getPendingAttributesUsing = null;
+
+    MigrateDatabase::$includePending = true;
+    SeedDatabase::$includePending = true;
 });
 
 afterEach($cleanup);
@@ -154,8 +169,8 @@
     Event::assertDispatched(PendingTenantPulled::class);
 });
 
-test('commands do not run for pending tenants if tenancy.pending.include_in_queries is false and the with pending option does not get passed', function() {
-    config(['tenancy.pending.include_in_queries' => false]);
+test('commands include tenants based on the include_in_queries config when --with-pending is not passed', function (bool $includeInQueries) {
+    config(['tenancy.pending.include_in_queries' => $includeInQueries]);
 
     $tenants = collect([
         Tenant::create(),
@@ -164,21 +179,21 @@
         Tenant::createPending(),
     ]);
 
-    pest()->artisan('tenants:migrate --with-pending');
-
-    $artisan = pest()->artisan("tenants:run 'foo foo --b=bar --c=xyz'");
+    $command = pest()->artisan("tenants:run 'bar testing testing@test.test password foo'");
 
-    $pendingTenants = $tenants->filter->pending();
-    $readyTenants = $tenants->reject->pending();
-
-    $pendingTenants->each(fn ($tenant) => $artisan->doesntExpectOutputToContain("Tenant: {$tenant->getTenantKey()}"));
-    $readyTenants->each(fn ($tenant) => $artisan->expectsOutputToContain("Tenant: {$tenant->getTenantKey()}"));
+    $tenants->each(function ($tenant) use ($command, $includeInQueries) {
+        if ($tenant->pending() && ! $includeInQueries) {
+            $command->doesntExpectOutputToContain("Tenant: {$tenant->getTenantKey()}");
+        } else {
+            $command->expectsOutputToContain("Tenant: {$tenant->getTenantKey()}");
+        }
+    });
 
-    $artisan->assertExitCode(0);
-});
+    $command->assertSuccessful();
+})->with([true, false]);
 
-test('commands run for pending tenants too if tenancy.pending.include_in_queries is true', function() {
-    config(['tenancy.pending.include_in_queries' => true]);
+test('commands include pending tenants when truthy --with-pending is passed', function (bool $includeInQueries) {
+    config(['tenancy.pending.include_in_queries' => $includeInQueries]);
 
     $tenants = collect([
         Tenant::create(),
@@ -187,17 +202,22 @@
         Tenant::createPending(),
     ]);
 
-    pest()->artisan('tenants:migrate --with-pending');
+    foreach ([
+        '--with-pending',
+        '--with-pending=true',
+        '--with-pending=1'
+    ] as $option) {
+        $command = pest()->artisan("tenants:run 'bar testing testing@test.test password foo' {$option}");
 
-    $artisan = pest()->artisan("tenants:run 'foo foo --b=bar --c=xyz'");
+        // Pending tenants are included regardless of tenancy.pending.include_in_queries
+        $tenants->each(fn ($tenant) => $command->expectsOutputToContain("Tenant: {$tenant->getTenantKey()}"));
 
-    $tenants->each(fn ($tenant) => $artisan->expectsOutputToContain("Tenant: {$tenant->getTenantKey()}"));
-
-    $artisan->assertExitCode(0);
-});
+        $command->assertSuccessful();
+    }
+})->with([true, false]);
 
-test('commands run for pending tenants too if the with pending option is passed', function() {
-    config(['tenancy.pending.include_in_queries' => false]);
+test('commands exclude pending tenants when falsy --with-pending is passed', function (bool $includeInQueries) {
+    config(['tenancy.pending.include_in_queries' => $includeInQueries]);
 
     $tenants = collect([
         Tenant::create(),
@@ -206,14 +226,25 @@
         Tenant::createPending(),
     ]);
 
-    pest()->artisan('tenants:migrate --with-pending');
-
-    $artisan = pest()->artisan("tenants:run 'foo foo --b=bar --c=xyz' --with-pending");
-
-    $tenants->each(fn ($tenant) => $artisan->expectsOutputToContain("Tenant: {$tenant->getTenantKey()}"));
-
-    $artisan->assertExitCode(0);
-});
+    foreach ([
+        '--with-pending=false',
+        '--with-pending=0',
+        '--with-pending=foo' // Invalid values are treated as false
+    ] as $option) {
+        $command = pest()->artisan("tenants:run 'bar testing testing@test.test password foo' {$option}");
+
+        $tenants->each(function ($tenant) use ($command) {
+            if ($tenant->pending()) {
+                // Pending tenants are excluded regardless of tenancy.pending.include_in_queries
+                $command->doesntExpectOutputToContain("Tenant: {$tenant->getTenantKey()}");
+            } else {
+                $command->expectsOutputToContain("Tenant: {$tenant->getTenantKey()}");
+            }
+        });
+
+        $command->assertSuccessful();
+    }
+})->with([true, false]);
 
 test('pending tenants can have default attributes for non-nullable columns', function (bool $withPendingAttributes) {
     Schema::table('tenants', function (Blueprint $table) {
@@ -236,3 +267,105 @@
     else
         expect($fn)->toThrow(QueryException::class);
 })->with([true, false]);
+
+test('pending tenant databases can be migrated using a job unless configured otherwise', function (bool $includeInQueries, ?bool $migrateWithPending) {
+    config([
+        'tenancy.bootstrappers' => [DatabaseTenancyBootstrapper::class],
+        'tenancy.pending.include_in_queries' => $includeInQueries,
+    ]);
+
+    MigrateDatabase::$includePending = $migrateWithPending;
+
+    Event::listen(TenancyInitialized::class, BootstrapTenancy::class);
+    Event::listen(TenancyEnded::class, RevertToCentralContext::class);
+    Event::listen(TenantCreated::class, JobPipeline::make([
+        CreateDatabase::class,
+        MigrateDatabase::class,
+    ])->send(function (TenantCreated $event) {
+        return $event->tenant;
+    })->toListener());
+
+    $pendingTenant = Tenant::createPending();
+
+    expect(Schema::hasTable('users'))->toBeFalse();
+
+    tenancy()->initialize($pendingTenant);
+
+    // MigrateDatabase includes/excludes pending tenants based on its $includePending property,
+    // regardless of the tenancy.pending.include_in_queries config.
+    expect(Schema::hasTable('users'))->toBe($migrateWithPending ?? $includeInQueries);
+})->with([
+    'include pending in queries' => [true],
+    'exclude pending from queries' => [false],
+])->with([
+    'migrate with pending' => [true],
+    'migrate without pending' => [false],
+    'default to config' => [null],
+]);
+
+test('pending tenant databases can be seeded using a job unless configured otherwise', function (bool $includeInQueries, ?bool $seedWithPending) {
+    config([
+        'tenancy.bootstrappers' => [DatabaseTenancyBootstrapper::class],
+        'tenancy.pending.include_in_queries' => $includeInQueries,
+        'tenancy.seeder_parameters.--class' => TestSeeder::class,
+    ]);
+
+    MigrateDatabase::$includePending = true;
+    SeedDatabase::$includePending = $seedWithPending;
+
+    Event::listen(TenancyInitialized::class, BootstrapTenancy::class);
+    Event::listen(TenancyEnded::class, RevertToCentralContext::class);
+    Event::listen(TenantCreated::class, JobPipeline::make([
+        CreateDatabase::class,
+        MigrateDatabase::class,
+        SeedDatabase::class,
+    ])->send(function (TenantCreated $event) {
+        return $event->tenant;
+    })->toListener());
+
+    $pendingTenant = Tenant::createPending();
+
+    tenancy()->initialize($pendingTenant);
+
+    // SeedDatabase includes/excludes pending tenants based on its $includePending property,
+    // regardless of the tenancy.pending.include_in_queries config.
+    expect(User::where('email', 'seeded@user')->exists())->toBe($seedWithPending ?? $includeInQueries);
+})->with([
+    'include pending in queries' => [true],
+    'exclude pending from queries' => [false],
+])->with([
+    'seed with pending' => [true],
+    'seed without pending' => [false],
+    'default to config' => [null],
+]);
+
+test('jobs that run before tenants get fully created recognize pending tenants', function () {
+    config([
+        'tenancy.bootstrappers' => [DatabaseTenancyBootstrapper::class],
+    ]);
+
+    Event::listen(TenancyInitialized::class, BootstrapTenancy::class);
+    Event::listen(TenancyEnded::class, RevertToCentralContext::class);
+    Event::listen(TenantCreated::class, JobPipeline::make([
+        CreateDatabase::class,
+        PendingTenantJob::class,
+    ])->send(function (TenantCreated $event) {
+        return $event->tenant;
+    })->toListener());
+
+    Tenant::createPending();
+
+    expect(app('tenant_is_pending'))->toBeTrue();
+});
+
+class PendingTenantJob
+{
+    public function __construct(
+        public Tenant $tenant,
+    ) {}
+
+    public function handle()
+    {
+        app()->instance('tenant_is_pending', $this->tenant->pending());
+    }
+}