Conversation
Added translations for Bulgarian language.
…eld legacy_id from base_album table.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds middleware alias Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Codecov Report❌ Patch coverage is 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/Http/Middleware/LegacyLocalIdRedirect.php (1)
32-37: Cache schema-column detection and fetch only required column.Line 34 executes
Schema::hasColumn()on every request, and line 36 fetches the full row when only theidis needed. Cache the schema check using a static property and select only theidcolumn to reduce per-request database overhead.♻️ Proposed refactor
class LegacyLocalIdRedirect { + private static ?bool $has_legacy_id_column = null; + /** * Handles redirection from old albums (Lychee v3 format). @@ public function handle(Request $request, \Closure $next) { @@ $album_id = $request->route('albumId'); + $has_legacy_id_column = self::$has_legacy_id_column ??= Schema::hasColumn('base_albums', 'legacy_id'); if ($album_id !== null && preg_match('/^\d{14}$/', (string) $album_id) === 1 && - Schema::hasColumn('base_albums', 'legacy_id') + $has_legacy_id_column ) { - $album = DB::table('base_albums')->where('legacy_id', '=', $album_id)->first(); + $album = DB::table('base_albums') + ->select('id') + ->where('legacy_id', '=', (string) $album_id) + ->first(); if ($album === null) { throw new NotFoundHttpException(); }
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
database/migrations/2025_05_27_081037_drop_legacy_id.php (1)
49-54: Use the migration constant for column name consistently.Line 52 hardcodes
'legacy_id'even thoughself::LEGACY_ID_NAMEexists.♻️ Proposed refactor
if (!Schema::hasColumn('base_albums', self::LEGACY_ID_NAME)) { Schema::table('base_albums', function (Blueprint $table) { - $table->unsignedBigInteger('legacy_id')->after('id')->nullable(false); + $table->unsignedBigInteger(self::LEGACY_ID_NAME)->after('id')->nullable(false); }); }
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/Http/Kernel.phpdatabase/migrations/2025_05_27_081037_drop_legacy_id.phproutes/web_v2.php
|
tested. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
database/migrations/2025_05_27_081037_drop_legacy_id.php (1)
35-37:⚠️ Potential issue | 🟠 MajorPreserve uniqueness for
base_albums.legacy_idif the column is retained
up()keeps the column (Line 36) but still dropsbase_albums_legacy_id_unique(Line 25). That can create duplicate legacy IDs and ambiguous redirect targets.Suggested fix
try { Schema::table('photos', function (Blueprint $table) { $table->dropUnique('photos_legacy_id_unique'); }); - Schema::table('base_albums', function (Blueprint $table) { - $table->dropUnique('base_albums_legacy_id_unique'); - }); } catch (\Throwable $e) { // Do nothing }
3 participants
Added functionality that resolves Lychee v3 album url to v7, using field legacy_id from base_album table.
Summary by CodeRabbit
New Features
Bug Fixes / Reliability