[4.x] Add --skip-tenants option to HasTenantOptions#1436
[4.x] Add --skip-tenants option to HasTenantOptions#1436jimish-gamit wants to merge 6 commits intoarchtechx:masterfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1436 +/- ##
============================================
- Coverage 86.08% 85.89% -0.19%
- Complexity 1148 1168 +20
============================================
Files 184 185 +1
Lines 3363 3410 +47
============================================
+ Hits 2895 2929 +34
- Misses 468 481 +13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hi @stancl Just checking in on this PR. Let me know if there’s any feedback or changes needed from my side, happy to update it. Thanks for your time! |
stancl
changed the title
📝 WalkthroughWalkthroughThe pull request adds a ChangesTenant Skipping Functionality
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/CommandsTest.php`:
- Around line 519-540: The pest()->artisan(...) invocation in the test closure
(the migrate commands test that creates tenant1/2/3) lacks an exit-code
assertion; modify the call to chain an assertion (for example
->assertExitCode(0) or ->assertSuccessful()) onto the pest()->artisan(...)
expression so the test fails if the artisan command crashes before the
subsequent tenancy/schema expectations run.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4729a91a-4d8b-45e5-8b47-efbd6a742e1e
📒 Files selected for processing (3)
src/Commands/Run.phpsrc/Concerns/HasTenantOptions.phptests/CommandsTest.php
| test('migrate commands can skip specified tenants', function (string $command) { | ||
| $tenant1 = Tenant::create(); | ||
| $tenant2 = Tenant::create(); | ||
| $tenant3 = Tenant::create(); | ||
|
|
||
| pest()->artisan("{$command} --skip-tenants={$tenant1->getTenantKey()} --skip-tenants={$tenant2->getTenantKey()}"); | ||
|
|
||
| tenancy()->initialize($tenant1); | ||
|
|
||
| expect(Schema::hasTable('users'))->toBeFalse(); | ||
|
|
||
| tenancy()->initialize($tenant2); | ||
|
|
||
| expect(Schema::hasTable('users'))->toBeFalse(); | ||
|
|
||
| tenancy()->initialize($tenant3); | ||
|
|
||
| expect(Schema::hasTable('users'))->toBeTrue(); | ||
| })->with([ | ||
| 'tenants:migrate', | ||
| 'tenants:migrate-fresh', | ||
| ]); |
There was a problem hiding this comment.
Missing exit-code assertion on the artisan() call.
Unlike the other two new tests (lines 551, 564), this pest()->artisan(...) call has no ->assertExitCode(0) (or ->assertSuccessful()). A command crash would go undetected if the schema assertions still happen to pass.
✅ Proposed fix
- pest()->artisan("{$command} --skip-tenants={$tenant1->getTenantKey()} --skip-tenants={$tenant2->getTenantKey()}");
+ pest()->artisan("{$command} --skip-tenants={$tenant1->getTenantKey()} --skip-tenants={$tenant2->getTenantKey()}")
+ ->assertExitCode(0);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/CommandsTest.php` around lines 519 - 540, The pest()->artisan(...)
invocation in the test closure (the migrate commands test that creates
tenant1/2/3) lacks an exit-code assertion; modify the call to chain an assertion
(for example ->assertExitCode(0) or ->assertSuccessful()) onto the
pest()->artisan(...) expression so the test fails if the artisan command crashes
before the subsequent tenancy/schema expectations run.
4 participants
Summary
Adds a --skip-tenants option to all tenant artisan commands (
tenants:run,tenants:migrate,tenants:rollback,tenants:seed,tenants:up,tenants:down).The option is the complement of the existing
--tenantsoption instead of specifying which tenants to include, you specify which to exclude.Changes
src/Concerns/HasTenantOptions.phpAdded--skip-tenants=*option togetOptions()and awhereNotInclause ingetTenantsQuery()to exclude the specified tenants from the query. Since all tenant commands share this trait, the option is available across all of them.src/Commands/Run.phpAdded--skip-tenants=*to the command signature so it appears intenants:run --help.Usage
# Works on all tenant commands php artisan tenants:migrate --skip-tenants=tenant3Summary by CodeRabbit
Release Notes
--skip-tenantsoption to tenant commands, enabling you to exclude specific tenants from execution--skip-tenantscan be combined with--tenantsfor precise control over which tenants are affected by console operations