[#529] Skip dependencies for disabled modules in ModuleLoader#530
Conversation
|
Warning Review limit reached
More reviews will be available in 17 minutes and 17 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughModuleLoader now checks each module's ChangesModule Dependency Loading Fix
Sequence DiagramsequenceDiagram
participant ModuleLoader
participant ModuleConfig
participant DependencyRegistry
ModuleLoader->>ModuleConfig: iterate configured modules
loop per module
ModuleConfig-->>ModuleLoader: return module options (including enabled)
alt enabled = false
ModuleLoader->>ModuleLoader: continue (skip dependency loading)
else enabled = true
ModuleLoader->>DependencyRegistry: load & merge module dependencies
end
end
ModuleLoader-->>DependencyRegistry: finalize merged dependencies
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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.
🧹 Nitpick comments (1)
tests/Unit/Module/ModuleLoaderTest.php (1)
60-60: ⚡ Quick winTighten the Meme “disabled” assertion in ModuleLoaderTest
tests/_root/shared/config/modules.phpsets'Meme' => ['enabled' => false], andsrc/Module/ModuleLoader.php::loadModulesDependencies()skips disabled modules viaisModuleEnabled().ModuleLoaderTest::testLoadModulesDependencies()already guards this behavior indirectly by assertingTokenServiceInterfaceresolves toQuantum\Tests\_root\shared\Services\TokenService::classand is notDisabledTokenService::class.Add an explicit assertion on the fixture config (e.g.,
getModuleConfigs()['Meme']['enabled'] === false) and/or require Meme’sconfig/dependencies.phpexists, so the test can’t pass just because the Meme module/files are missing.🤖 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/Unit/Module/ModuleLoaderTest.php` at line 60, Add an explicit check in ModuleLoaderTest::testLoadModulesDependencies() to assert the fixture config disables the Meme module and that its dependency file exists: verify getModuleConfigs()['Meme']['enabled'] === false and assert the Meme module's config/dependencies.php (or equivalent fixture path used by ModuleLoader::loadModulesDependencies()/isModuleEnabled()) is present, so the test fails if the Meme module/config is missing rather than silently skipped; update assertions around TokenServiceInterface only after these explicit checks.
🤖 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.
Nitpick comments:
In `@tests/Unit/Module/ModuleLoaderTest.php`:
- Line 60: Add an explicit check in
ModuleLoaderTest::testLoadModulesDependencies() to assert the fixture config
disables the Meme module and that its dependency file exists: verify
getModuleConfigs()['Meme']['enabled'] === false and assert the Meme module's
config/dependencies.php (or equivalent fixture path used by
ModuleLoader::loadModulesDependencies()/isModuleEnabled()) is present, so the
test fails if the Meme module/config is missing rather than silently skipped;
update assertions around TokenServiceInterface only after these explicit checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e3a8f57a-1adf-45ac-9eaa-a6a8ce0ea2af
📒 Files selected for processing (5)
CHANGELOG.mdsrc/Module/ModuleLoader.phptests/Unit/Module/ModuleLoaderTest.phptests/_root/modules/Meme/Services/DisabledTokenService.phptests/_root/modules/Meme/config/dependencies.php
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 `@composer.json`:
- Line 40: composer.json currently pins "twig/twig" to ">=3.0 <3.26", but
upgrading to the security-patched 3.26+ requires PHP >=8.2; either update the
project's supported PHP versions in CI/config (bump matrix to PHP 8.2+) and
change the twig constraint to "^3.26" in composer.json, or keep PHP 7.4/8.0/8.1
support and change the twig constraint to the latest patched release that still
supports those PHP versions (identify and set the specific version instead of
^3.26); update composer.json accordingly and run composer update/test to verify
compatibility.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #530 +/- ##
============================================
+ Coverage 90.86% 90.88% +0.01%
- Complexity 3078 3079 +1
============================================
Files 263 263
Lines 8115 8117 +2
============================================
+ Hits 7374 7377 +3
+ Misses 741 740 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Fixes #529
Summary by CodeRabbit
Bug Fixes
Tests
Documentation
Chores