Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/Concerns/HasTenantOptions.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

Expand All @@ -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);
Comment thread
stancl marked this conversation as resolved.
});
}

Expand Down
33 changes: 17 additions & 16 deletions src/Database/Concerns/HasPending.php
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand All @@ -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],
));
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

/**
Expand Down
11 changes: 11 additions & 0 deletions src/Jobs/MigrateDatabase.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Comment thread
stancl marked this conversation as resolved.
public static ?bool $includePending = true;

public function __construct(
protected TenantWithDatabase&Model $tenant,
) {}
Expand All @@ -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'),
]);
}
}
11 changes: 11 additions & 0 deletions src/Jobs/SeedDatabase.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Comment thread
stancl marked this conversation as resolved.
public static ?bool $includePending = true;

public function __construct(
protected TenantWithDatabase&Model $tenant,
) {}
Expand All @@ -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'),
]);
}
}
193 changes: 163 additions & 30 deletions tests/PendingTenantsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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(),
Expand All @@ -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(),
Expand All @@ -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(),
Expand All @@ -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) {
Expand All @@ -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) {
Comment on lines +349 to +352

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description talks about:

This is a problem if someone wants to e.g. add a job to the DatabaseCreated job pipeline

But the added test uses a TenantCreated pipeline. Those two might behave differently so it might be worth seeing if this regression test acts as intended.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying this test on master - I get false with both TenantCreated and DatabaseCreated.

On this branch it passes with both this code as well as

Event::listen(TenantCreated::class, JobPipeline::make([
    CreateDatabase::class,
])->send(function (TenantCreated $event) {
    return $event->tenant;
})->toListener());

Event::listen(DatabaseCreated::class, JobPipeline::make([
    PendingTenantJob::class,
])->send(function (DatabaseCreated $event) {
    return $event->tenant;
})->toListener());

So we can probably merge it like this since it's simpler. Just keeping this review here to note the difference in tests and PR desc.

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());
}
}
Loading