Skip to content

Fix tenants:seed command name - #1478

Open
lucasmichot wants to merge 4 commits into
archtechx:masterfrom
lucasmichot:fix/1474-tenants-seed-command-name
Open

Fix tenants:seed command name#1478
lucasmichot wants to merge 4 commits into
archtechx:masterfrom
lucasmichot:fix/1474-tenants-seed-command-name

Conversation

@lucasmichot

@lucasmichot lucasmichot commented Aug 5, 2026

Copy link
Copy Markdown

Fixes #1474.

Replaces the version_compare check in Seed::__construct() with a configure() override that calls setName('tenants:seed'). configure() always runs after the command name is set by Symfony's Command constructor, regardless of whether the installed Laravel version's SeedCommand sets its name via $name (pre-13.24) or $signature (13.24+), so this works across versions without checking app()->version().

configure() is also the general place to add, remove, or tweak a parent command's arguments/options ($this->addArgument(), $this->addOption(), or $this->getDefinition()->remove*()), since it's guaranteed to run after the parent's own definition is built and before the command executes. Worth keeping in mind for other commands here that extend Laravel's built-in ones and need to adjust their inherited signature.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed tenant database seeding command registration so it no longer replaces Laravel’s built-in database seeding command.
    • Preserved separate access to both tenant seeding and standard database seeding commands.
  • Tests

    • Added coverage confirming each seeding command resolves and operates independently.

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 305861a1-9993-49e5-9cac-b6651b2b6491

📥 Commits

Reviewing files that changed from the base of the PR and between 5a5ab2b and dd34ebd.

📒 Files selected for processing (1)
  • src/Commands/Seed.php

📝 Walkthrough

Walkthrough

Seed preserves Laravel seed command configuration, adds tenant options when needed, and registers the command as tenants:seed. Tests verify that Laravel retains db:seed and tenancy registers tenants:seed.

Changes

Tenant seed command

Layer / File(s) Summary
Command initialization and naming
src/Commands/Seed.php
Seed initializes SeedCommand, conditionally adds tenant options, preserves inherited configuration, and sets the command name to tenants:seed.
Command registration validation
tests/CommandsTest.php
Tests verify that db:seed uses Laravel’s SeedCommand and tenants:seed uses the tenancy Seed command.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: stancl

Poem

A rabbit checks the seed command’s name,
Laravel’s db:seed stays the same.
Tenant options join the queue,
tenants:seed comes through.
Two commands hop without a fight. 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change to the tenants:seed command name.
Linked Issues check ✅ Passed The changes preserve tenants:seed registration and Laravel's db:seed command across supported Laravel versions, matching issue #1474.
Out of Scope Changes check ✅ Passed The changes are limited to command naming compatibility and focused tests for tenants:seed and db:seed registration.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.64%. Comparing base (e0990a4) to head (dd34ebd).

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1478      +/-   ##
============================================
- Coverage     86.65%   86.64%   -0.01%     
- Complexity     1220     1221       +1     
============================================
  Files           186      186              
  Lines          3589     3588       -1     
============================================
- Hits           3110     3109       -1     
  Misses          479      479              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Replaces the version_compare hack with a configure() override that
sets the command name, which works regardless of whether the
installed Laravel version's SeedCommand uses $name or $signature.
@lucasmichot
lucasmichot force-pushed the fix/1474-tenants-seed-command-name branch from 54f9c54 to d2c96bb Compare August 5, 2026 10:04
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@stancl

stancl commented Aug 8, 2026

Copy link
Copy Markdown
Member

Thanks for the PR.

The linked issue is already fixed by the code that's being replaced here.

I can see configure() being more appropriate, however the if (! $this->getDefinition()->hasOption('tenants')) { check still appears pretty magical, making the code perhaps more confusing than it is now.

@lucasmichot

lucasmichot commented Aug 8, 2026

Copy link
Copy Markdown
Author

Re-added the comments explaining the hasOption('tenants') check and the setName() placement - they got stripped in a later commit before I opened the PR. Should make the reasoning clear without needing an app()->version() check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tenants:seed command registers as db:seed on Laravel 13.24 (SeedCommand switched from $name to $signature)

2 participants