Issue/500 qt prefix class rename#501
Merged
armanist merged 6 commits intosoftberg:masterfrom May 4, 2026
Merged
Conversation
📝 WalkthroughWalkthroughThe PR renames four framework base classes throughout the codebase: ChangesBase Class Renaming & Migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #501 +/- ##
=========================================
Coverage 90.87% 90.87%
Complexity 2926 2926
=========================================
Files 255 255
Lines 7703 7703
=========================================
Hits 7000 7000
Misses 703 703 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
armanist
added
enhancement
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Module/Templates/DemoWeb/src/Middlewares/Auth.php.tpl (1)
17-35:⚠️ Potential issue | 🔴 CriticalAdd a constructor to match
MiddlewareManager's instantiation pattern.
MiddlewareManager::getMiddleware()instantiates middleware withnew $middlewareClass($request), but this template lacks a constructor. Generated middleware classes will fail at runtime with an argument-count error.Suggested fix
class Auth extends Middleware { + public function __construct(Request $request) + { + } + public function apply(Request $request, Closure $next): Response🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoWeb/src/Middlewares/Auth.php.tpl` around lines 17 - 35, The Auth middleware template must accept the Request argument because MiddlewareManager::getMiddleware() instantiates middleware with new $middlewareClass($request); add a public constructor to the Auth class that accepts Request $request (typed), and either call parent::__construct($request) or store it on a private property (e.g., $this->request) so the class can be instantiated without an argument-count error; ensure the constructor signature matches the Request type used in apply().src/Migration/Templates/MigrationTemplate.php (1)
28-50:⚠️ Potential issue | 🔴 CriticalEmit
voidreturn type and use non-nullable parameter in all generated migration methods.
Quantum\Migration\Migrationrequiresup(TableFactory $tableFactory): voidanddown(TableFactory $tableFactory): void, but the templates emitup(?TableFactory $tableFactory)anddown(?TableFactory $tableFactory)instead—missing both the: voidreturn type and using a nullable parameter. Generated migrations will not satisfy the abstract contract.Suggested fix
- public function up(?TableFactory $tableFactory) { + public function up(TableFactory $tableFactory): void { $table = $tableFactory->create(\'' . $tableName . '\'); } - public function down(?TableFactory $tableFactory) + public function down(TableFactory $tableFactory): void { $tableFactory->drop(\'' . $tableName . '\'); }Also applies to lines 56–78, 83–105, 110–132.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Migration/Templates/MigrationTemplate.php` around lines 28 - 50, The generated migration method signatures in MigrationTemplate::create currently use nullable parameters and omit the void return type; update the template strings so both methods use a non-nullable TableFactory parameter and a void return type (change "public function up(?TableFactory $tableFactory)" to "public function up(TableFactory $tableFactory): void" and likewise for "down") and apply the same change to the other template occurrences in this file (the other create/template-returning blocks referenced around lines 56–78, 83–105, 110–132) so generated migrations match Quantum\Migration\Migration's contract.
🧹 Nitpick comments (1)
tests/Unit/Migration/Templates/MigrationTemplateTest.php (1)
10-46: ⚡ Quick winCover the generated method headers too.
The current assertions only verify the new import and class name. They will still pass if
up()/down()regress away from theMigrationcontract again, so the test suite won't catch the main compatibility break.Suggested addition
$this->assertStringContainsString('use Quantum\Migration\Migration;', $template); $this->assertStringContainsString('class Create_table_users_1001 extends Migration', $template); +$this->assertStringContainsString('public function up(TableFactory $tableFactory): void', $template); +$this->assertStringContainsString('public function down(TableFactory $tableFactory): void', $template);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Unit/Migration/Templates/MigrationTemplateTest.php` around lines 10 - 46, The tests for MigrationTemplate::create/alter/rename/drop only assert imports and class names; add assertions to each test (testCreateTemplateContainsExpectedOperations, testAlterTemplateContainsExpectedOperations, testRenameTemplateContainsExpectedOperations, testDropTemplateContainsExpectedOperations) that the generated template includes the migration method headers (e.g. the exact strings for the up() and down() signatures used by the Migration contract such as "public function up(): void" and "public function down(): void" or the project's exact signatures) so regressions to the up/down method signatures will be caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/Unit/Migration/MigrationManagerTest.php`:
- Around line 211-215: The test suite lost coverage of legacy QtMigration
because createValidMigrationFile always generates classes extending
\\Quantum\\Migration\\Migration; update tests to also generate a legacy fixture
extending \\Quantum\\Migration\\QtMigration and assert MigrationManager::upgrade
(or the test method that runs upgrades) still succeeds. either enhance
createValidMigrationFile to accept a parent-class parameter (defaulting to
Migration) or add a new helper createLegacyMigrationFile that writes a class
extending \\Quantum\\Migration\\QtMigration, then add a test case that uses this
helper and runs the existing upgrade/assert logic from the MigrationManagerTest
to verify backward compatibility.
---
Outside diff comments:
In `@src/Migration/Templates/MigrationTemplate.php`:
- Around line 28-50: The generated migration method signatures in
MigrationTemplate::create currently use nullable parameters and omit the void
return type; update the template strings so both methods use a non-nullable
TableFactory parameter and a void return type (change "public function
up(?TableFactory $tableFactory)" to "public function up(TableFactory
$tableFactory): void" and likewise for "down") and apply the same change to the
other template occurrences in this file (the other create/template-returning
blocks referenced around lines 56–78, 83–105, 110–132) so generated migrations
match Quantum\Migration\Migration's contract.
In `@src/Module/Templates/DemoWeb/src/Middlewares/Auth.php.tpl`:
- Around line 17-35: The Auth middleware template must accept the Request
argument because MiddlewareManager::getMiddleware() instantiates middleware with
new $middlewareClass($request); add a public constructor to the Auth class that
accepts Request $request (typed), and either call parent::__construct($request)
or store it on a private property (e.g., $this->request) so the class can be
instantiated without an argument-count error; ensure the constructor signature
matches the Request type used in apply().
---
Nitpick comments:
In `@tests/Unit/Migration/Templates/MigrationTemplateTest.php`:
- Around line 10-46: The tests for MigrationTemplate::create/alter/rename/drop
only assert imports and class names; add assertions to each test
(testCreateTemplateContainsExpectedOperations,
testAlterTemplateContainsExpectedOperations,
testRenameTemplateContainsExpectedOperations,
testDropTemplateContainsExpectedOperations) that the generated template includes
the migration method headers (e.g. the exact strings for the up() and down()
signatures used by the Migration contract such as "public function up(): void"
and "public function down(): void" or the project's exact signatures) so
regressions to the up/down method signatures will be caught.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 80faa52f-dbc6-44b3-9bb2-838084f31d48
📒 Files selected for processing (60)
CHANGELOG.mdsrc/Auth/Factories/AuthFactory.phpsrc/Console/CliCommand.phpsrc/Console/CommandDiscovery.phpsrc/Console/Commands/CronRunCommand.phpsrc/Console/Commands/DebugBarCommand.phpsrc/Console/Commands/EnvCommand.phpsrc/Console/Commands/InstallToolkitCommand.phpsrc/Console/Commands/KeyGenerateCommand.phpsrc/Console/Commands/MigrationGenerateCommand.phpsrc/Console/Commands/MigrationMigrateCommand.phpsrc/Console/Commands/ModuleGenerateCommand.phpsrc/Console/Commands/OpenApiCommand.phpsrc/Console/Commands/ResourceCacheClearCommand.phpsrc/Console/Commands/RouteListCommand.phpsrc/Console/Commands/ServeCommand.phpsrc/Console/Commands/VersionCommand.phpsrc/Middleware/Middleware.phpsrc/Middleware/MiddlewareManager.phpsrc/Migration/Enums/ExceptionMessages.phpsrc/Migration/Migration.phpsrc/Migration/MigrationManager.phpsrc/Migration/MigrationTable.phpsrc/Migration/Templates/MigrationTemplate.phpsrc/Module/Templates/DemoApi/src/Middlewares/BaseMiddleware.php.tplsrc/Module/Templates/DemoApi/src/Services/AuthService.php.tplsrc/Module/Templates/DemoApi/src/Services/CommentService.php.tplsrc/Module/Templates/DemoApi/src/Services/PostService.php.tplsrc/Module/Templates/DemoWeb/src/Controllers/CommentController.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/Auth.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/BaseMiddleware.php.tplsrc/Module/Templates/DemoWeb/src/Middlewares/Guest.php.tplsrc/Module/Templates/DemoWeb/src/Services/AuthService.php.tplsrc/Module/Templates/DemoWeb/src/Services/CommandService.php.tplsrc/Module/Templates/DemoWeb/src/Services/CommentService.php.tplsrc/Module/Templates/DemoWeb/src/Services/PostService.php.tplsrc/Module/Templates/Toolkit/src/Controllers/EmailsController.php.tplsrc/Module/Templates/Toolkit/src/Middlewares/BaseMiddleware.php.tplsrc/Module/Templates/Toolkit/src/Middlewares/BasicAuth.php.tplsrc/Module/Templates/Toolkit/src/Services/DashboardService.php.tplsrc/Module/Templates/Toolkit/src/Services/DatabaseService.php.tplsrc/Module/Templates/Toolkit/src/Services/EmailService.php.tplsrc/Module/Templates/Toolkit/src/Services/LogsService.php.tplsrc/Service/Factories/ServiceFactory.phpsrc/Service/Helpers/service.phpsrc/Service/Service.phpsrc/Storage/Factories/FileSystemFactory.phptests/Unit/Console/CliCommandTest.phptests/Unit/Console/CommandDiscoveryTest.phptests/Unit/Di/DiTest.phptests/Unit/Middleware/MiddlewareManagerTest.phptests/Unit/Migration/Exceptions/MigrationExceptionTest.phptests/Unit/Migration/MigrationManagerTest.phptests/Unit/Migration/Templates/MigrationTemplateTest.phptests/Unit/Service/Factories/ServiceFactoryTest.phptests/Unit/Service/Helpers/ServiceHelperFunctionTest.phptests/Unit/Service/ServiceTest.phptests/_root/modules/Test/Services/AuthService.phptests/_root/shared/Commands/TestCommand.phptests/_root/shared/Services/TokenService.php
andrey-smaelov
approved these changes
May 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #500
Summary by CodeRabbit
Release Notes