Laravel 12 #3656
Laravel 12 #3656
Conversation
|
Warning Rate limit exceeded@ildyria has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 50 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughMultiple dependency bumps (Laravel, Safe, lycheeverify, PHPUnit) plus small code updates: Safe imports used, a SizeVariant method renamed, resource call sites updated, PHPUnit schema versions bumped, and several test classes/namespaces adjusted. No functional behavior changes beyond Safe wrapper and method rename. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
✨ Finishing Touches
🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
composer.json(3 hunks)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: 3️⃣ PHP dist / 8.4 - postgresql
- GitHub Check: 3️⃣ PHP dist / 8.3 - mariadb
- GitHub Check: 3️⃣ PHP dist / 8.4 - mariadb
- GitHub Check: 3️⃣ PHP dist / 8.3 - postgresql
- GitHub Check: 3️⃣ PHP dist / 8.3 - sqlite
- GitHub Check: 3️⃣ PHP dist / 8.4 - sqlite
🔇 Additional comments (3)
composer.json (3)
65-65: Verify lycheeverify upgrade to ^1.0.3
Confirm no new PHP/Laravel version constraints and that our override for lychee-org/lycheeverify still resolves correctly.
61-61: Run composer install and verify Laravel 12 compatibility
Before merging, install dependencies and rerun the dry-run and why-not checks, then confirm each ecosystem package supports Laravel 12 and PHP 8.3 (e.g., barryvdh/laravel-debugbar, graham-campbell/markdown, larastan, opcodesio/log-viewer, socialiteproviders/, spatie/laravel-, laragear/webauthn, laravel/socialite).Suggested commands:
composer install composer update laravel/framework --with-all-dependencies --dry-run || true composer why-not laravel/framework ^12.0 || true composer outdated --direct || true rg -nP 'opcodesio/log-viewer|graham-campbell/markdown|larastan|debugbar|spatie/laravel-|laragear/webauthn|laravel/socialite|socialiteproviders' composer.json
92-92: Safe namespace imports are correctAll Safe function calls use the fully qualified
\Safe\…namespace and are covered by Composer’s autoload. No missinguse function Safe\…statements detected.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Actions/Import/Pipes/BuildTree.php (1)
251-254: Ignore-list filtering logic invertedDoc says “ignore lines that are empty or start with #”, but the predicate keeps empty lines. Fix to keep only non-empty, non-comment entries.
- $result = array_filter($result, fn ($v) => $v === '' || !str_starts_with($v, '#')); + $result = array_filter($result, fn (string $v): bool => $v !== '' && !str_starts_with($v, '#'));
🧹 Nitpick comments (8)
tests/Feature_v2/Image/WatermarkerTest.php (3)
111-116: Remove duplicate config set call.
Configs::set('watermark_photo_id', $photoId);is called twice; drop the duplicate.Apply this diff:
- Configs::set('watermark_photo_id', $photoId); Configs::set('watermark_photo_id', $photoId);
176-181: Remove duplicate config set call.Same duplication here; keep a single assignment.
- Configs::set('watermark_photo_id', $photoId); Configs::set('watermark_photo_id', $photoId);
62-71: Prefer assertTrue/assertFalse over assertEquals for booleans.Improves intent and strictness.
- self::assertEquals(false, $watermarker->can_watermark); + self::assertFalse($watermarker->can_watermark);(Apply similarly on Lines 70, 79, 88, 118, 185.)
app/Console/Commands/FixPermissions.php (2)
141-143: Add parentheses to clarify precedence in recursion conditionWithout parentheses, the intention is easy to misread. No behavior change, just readability.
- if ($dir_entry->isDir() && !$dir_entry->isDot() || $dir_entry->isFile()) { + if (($dir_entry->isDir() && !$dir_entry->isDot()) || $dir_entry->isFile()) {
146-147: Avoid manual newline in console warningSymfony’s console methods handle newlines; the explicit PHP_EOL creates an extra blank line.
- } catch (FilesystemException) { - $this->warn(sprintf('Unable to determine permissions for %s' . PHP_EOL, $path)); + } catch (FilesystemException) { + $this->warn(sprintf('Unable to determine permissions for %s', $path));app/Actions/Import/Pipes/BuildTree.php (1)
216-219: Strengthen regex for UTF‑8 and type the callbackUse the Unicode modifier and a typed arrow function to avoid splitting multibyte chars and aid static analysis.
- $pattern = preg_replace_callback('/([^*])/', fn($a) => self::preg_quote_callback_fct($a), $pattern); + $pattern = preg_replace_callback('/([^*])/u', fn (array $m): string => self::preg_quote_callback_fct($m), $pattern); $pattern = str_replace('*', '.*', $pattern); - - return preg_match('/^' . $pattern . '$/i', $filename) === 1; + return preg_match('/^' . $pattern . '$/iu', $filename) === 1;Alternative (simpler and faster):
$pattern = preg_quote($pattern, '/'); $pattern = str_replace('\*', '.*', $pattern); return preg_match('/^' . $pattern . '$/iu', $filename) === 1;app/Http/Resources/Models/SizeVariantsResouce.php (1)
20-20: Typo in class name “SizeVariantsResouce”PSR-4 and readability: consider renaming to “SizeVariantsResource” (file and class). If you defer, at least add a @deprecated alias.
app/Models/SizeVariant.php (1)
202-205: Public API rename — consider BC aliasAdd a deprecated toResource() wrapper to reduce downstream breakage during the Laravel 12 upgrade.
public function toDataResource(bool $no_url = false): SizeVariantResource { return new SizeVariantResource($this, no_url: $no_url); } + + /** + * @deprecated Use toDataResource() instead. Will be removed in a future release. + */ + public function toResource(bool $no_url = false): SizeVariantResource + { + return $this->toDataResource($no_url); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
app/Actions/Import/Pipes/BuildTree.php(2 hunks)app/Console/Commands/FixPermissions.php(1 hunks)app/Http/Resources/Models/SizeVariantsResouce.php(1 hunks)app/Models/SizeVariant.php(1 hunks)phpunit.ci.xml(1 hunks)phpunit.pgsql.xml(1 hunks)phpunit.xml(1 hunks)tests/Feature_v2/Commands/GenerateThumbsTest.php(1 hunks)tests/Feature_v2/Image/WatermarkerTest.php(1 hunks)tests/Feature_v2/SmartAlbums/OverridePermissionsTest.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Any new PHP file must include the license header and have a single blank line after the opening <?php tag
Use snake_case for PHP variable names
Apply PSR-4 coding standard
Always call in_array with strict mode: in_array($needle, $haystack, true)
Use only booleans in if conditions; avoid integers or strings
Use strict comparison (===, !==) instead of loose comparison (==, !=)
Avoid duplicating code in both if and else branches
Do not use empty(); prefer explicit checks
Files:
app/Http/Resources/Models/SizeVariantsResouce.phpapp/Models/SizeVariant.phptests/Feature_v2/Commands/GenerateThumbsTest.phptests/Feature_v2/SmartAlbums/OverridePermissionsTest.phpapp/Console/Commands/FixPermissions.phptests/Feature_v2/Image/WatermarkerTest.phpapp/Actions/Import/Pipes/BuildTree.php
app/{Http/Resources,Data}/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Resource classes should extend Spatie Data instead of JsonResource
Files:
app/Http/Resources/Models/SizeVariantsResouce.php
tests/Feature_v2/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Feature_v2 tests should extend BaseApiWithDataTest
Files:
tests/Feature_v2/Commands/GenerateThumbsTest.phptests/Feature_v2/SmartAlbums/OverridePermissionsTest.phptests/Feature_v2/Image/WatermarkerTest.php
tests/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not mock the database in tests; use the in-memory SQLite database
Files:
tests/Feature_v2/Commands/GenerateThumbsTest.phptests/Feature_v2/SmartAlbums/OverridePermissionsTest.phptests/Feature_v2/Image/WatermarkerTest.php
🧠 Learnings (1)
📚 Learning: 2025-08-24T07:15:25.203Z
Learnt from: CR
PR: LycheeOrg/Lychee#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-24T07:15:25.203Z
Learning: Applies to tests/Feature_v2/**/*.php : Feature_v2 tests should extend BaseApiWithDataTest
Applied to files:
tests/Feature_v2/Commands/GenerateThumbsTest.phptests/Feature_v2/SmartAlbums/OverridePermissionsTest.phptests/Feature_v2/Image/WatermarkerTest.php
🧬 Code graph analysis (4)
app/Http/Resources/Models/SizeVariantsResouce.php (1)
app/Models/SizeVariant.php (1)
toDataResource(202-205)
app/Models/SizeVariant.php (1)
resources/js/lychee.d.ts (1)
SizeVariantResource(557-565)
tests/Feature_v2/Commands/GenerateThumbsTest.php (1)
tests/Feature_v2/Base/BaseApiWithDataTest.php (1)
BaseApiWithDataTest(91-249)
tests/Feature_v2/Image/WatermarkerTest.php (1)
tests/Feature_v2/Image/Handlers/BaseImageHandler.php (1)
BaseImageHandler(40-604)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: 2️⃣ PHP tests / 8.4 - mariadb -- Unit,Feature_v2
- GitHub Check: 2️⃣ PHP tests / 8.4 - postgresql -- Unit,Feature_v2
- GitHub Check: 2️⃣ PHP tests / 8.4 - sqlite -- Unit,Feature_v2
- GitHub Check: 2️⃣ PHP tests / 8.3 - mariadb -- Unit,Feature_v2
- GitHub Check: 2️⃣ PHP tests / 8.3 - postgresql -- Unit,Feature_v2
- GitHub Check: 2️⃣ PHP tests / 8.3 - sqlite -- Unit,Feature_v2
🔇 Additional comments (11)
phpunit.xml (1)
2-2: Schema bump to PHPUnit 11 is correct; existing elements remain valid.
<coverage><report><text showUncoveredFiles="false"/>and<extensions><bootstrap .../>are supported in PHPUnit 11, so no further config changes are required. LGTM. (docs.phpunit.de)phpunit.ci.xml (1)
2-2: Consistent schema update to 11.0; config is compatible.The
<extensions>registration pattern and coverage config continue to be valid in PHPUnit 11. LGTM. (docs.phpunit.de)phpunit.pgsql.xml (1)
2-2: PHPUnit 11 schema bump looks good.No incompatible elements detected;
<coverage>and<report>usage match PHPUnit 11 docs. LGTM. (docs.phpunit.de)tests/Feature_v2/Image/WatermarkerTest.php (2)
19-19: Namespace realignment is fine.Moving the test under
Tests\Feature_v2\Imagekeeps PSR-4 and PHPUnit discovery intact. LGTM.
26-26: Extending the correct base for Feature_v2 tests.Importing
Handlers\BaseImageHandlerensures inheritance fromBaseApiWithDataTestvia the chain. LGTM.tests/Feature_v2/Commands/GenerateThumbsTest.php (1)
28-28: Class rename matches filename and PSR-4.Test base remains
BaseApiWithDataTest. LGTM.app/Console/Commands/FixPermissions.php (1)
18-18: Safe wrapper adoption for filetype — good changeEnsures exceptions are thrown and caught consistently with other Safe FS calls.
app/Actions/Import/Pipes/BuildTree.php (1)
25-25: Safe\preg_replace_callback import — goodAligns with Safe usage elsewhere; consistent error handling.
tests/Feature_v2/SmartAlbums/OverridePermissionsTest.php (2)
19-19: Namespace aligns with pathMatches folder structure and autoloading.
24-24: Extends BaseApiWithDataTest as requiredConforms to Feature_v2 testing guideline.
app/Http/Resources/Models/SizeVariantsResouce.php (1)
47-54: Confirm notoResource()usages remain
Search fortoResource(across PHP files returned no results; please manually verify there are no remaining calls in strings, comments, or other code paths.
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
Summary by CodeRabbit
Chores
Tests
Bug Fixes
Notes