Refactor thumb selection to avoid queries #3926
Conversation
📝 WalkthroughWalkthroughAdds privilege-based cover relations to Album and centralizes cover-selection logic in HasAlbumThumb; updates queries to include/exclude these relations where appropriate and adds unit tests covering the new cover-selection behavior. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
Pre-merge checks❌ Failed checks (1 warning)
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/Relations/HasAlbumThumb.php (1)
188-200: Redundant null checks can be simplified.The null checks (e.g.,
$album->cover_id !== null) are redundant becausegetCoverTypeForAlbum()only returns'cover_id'when$album->cover_id !== null. The same applies to the other cover type checks.While defensive, this creates a slight inconsistency. Consider simplifying:
🔎 Proposed simplification
foreach ($models as $album) { $cover_type = $this->getCoverTypeForAlbum($album); - if ($cover_type === 'cover_id' && $album->cover_id !== null) { - // We do not need to do anything here, because we already have the cover - // loaded via the `cover` relation of `Album`. + if ($cover_type === 'cover_id') { $album->setRelation($relation, Thumb::createFromPhoto($album->cover)); - } elseif ($cover_type === 'auto_cover_id_max_privilege' && $album->auto_cover_id_max_privilege !== null) { + } elseif ($cover_type === 'auto_cover_id_max_privilege') { $album->setRelation($relation, Thumb::createFromPhoto($album->max_priviledge_cover)); - } elseif ($cover_type === 'auto_cover_id_least_privilege' && $album->auto_cover_id_least_privilege !== null) { + } else { + // auto_cover_id_least_privilege - may be null for albums without photos $album->setRelation($relation, Thumb::createFromPhoto($album->min_priviledge_cover)); - } else { - $album->setRelation($relation, null); } }Note:
Thumb::createFromPhoto()already handlesnullinput gracefully, returningnull.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/Actions/Album/Delete.phpapp/Actions/Albums/Flow.phpapp/Actions/User/Notify.phpapp/Models/Album.phpapp/Relations/HasAlbumThumb.php
🧰 Additional context used
📓 Path-based instructions (1)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Any new PHP file should contain the license header and have a single blank line after the opening PHP tag
Variable names should be in snake_case in PHP
Apply the PSR-4 coding standard in PHP
Use in_array() with true as the third parameter in PHP
Only use booleans in if statements, not integers or strings
Use strict comparison (===) instead of loose comparison (==)
Avoid code duplication in both if and else statements
Do not use empty() in PHP
Use the moneyphp/money library for handling monetary values in PHP
Never use floats or doubles to represent monetary values; use integers representing the smallest currency unit (e.g., cents for USD)
**/*.php: Write or extend executable specifications (unit, behaviour, or scenario tests) ahead of implementation, confirm they fail, and then drive code to green before refactoring. List the expected success, validation, and failure branches and add thin failing tests for each path.
For PHP code, adhere to conventions: license headers in new files, strict comparison (===), no empty(), in_array() with third parameter true, snake_case variables, PSR-4 standard, test base classes (AbstractTestCase for Unit, BaseApiWithDataTest for Feature_v2).
Always run phpunit tests. If a test remains red, disable it with a TODO, note the reason, and capture the follow-up in the relevant plan.
Spotless now uses Palantir Java Format 2.78.0 with a 120-character wrap; configure IDE formatters to match before pushing code changes.
Keep each increment's control flow flat by delegating validation/normalisation into tiny pure helpers that return simple enums or result records, then compose them instead of introducing inline branching that inflates the branch count per change.
When introducing new helpers/utilities or editing files prone to style violations (records, DTOs, generated adapters), run the narrowest applicable lint target (for example phpstan) before the full pipeline. Note the command in the related plan/task.
For PHP changes, ru...
Files:
app/Actions/User/Notify.phpapp/Actions/Albums/Flow.phpapp/Models/Album.phpapp/Actions/Album/Delete.phpapp/Relations/HasAlbumThumb.php
🧠 Learnings (9)
📓 Common learnings
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/OrderService.php:65-73
Timestamp: 2025-09-16T21:56:01.607Z
Learning: The validation for album-photo membership in PurchasableService::getEffectivePurchasableForPhoto() uses a JOIN query with the photo_album pivot table to ensure that only purchasables for albums that actually contain the specified photo are returned, preventing price spoofing attacks where clients could use arbitrary album_ids.
📚 Learning: 2025-09-16T21:56:01.607Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/OrderService.php:65-73
Timestamp: 2025-09-16T21:56:01.607Z
Learning: The validation for album-photo membership in PurchasableService::getEffectivePurchasableForPhoto() uses a JOIN query with the photo_album pivot table to ensure that only purchasables for albums that actually contain the specified photo are returned, preventing price spoofing attacks where clients could use arbitrary album_ids.
Applied to files:
app/Actions/User/Notify.phpapp/Actions/Albums/Flow.phpapp/Models/Album.php
📚 Learning: 2025-09-16T21:56:01.607Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/OrderService.php:65-73
Timestamp: 2025-09-16T21:56:01.607Z
Learning: The validation for album-photo membership in OrderService::addPhotoToOrder() is implemented within PurchasableService::getEffectivePurchasableForPhoto() using a whereHas('albums') constraint that ensures the photo belongs to the specified album_id before applying pricing logic.
Applied to files:
app/Actions/User/Notify.phpapp/Actions/Albums/Flow.phpapp/Models/Album.php
📚 Learning: 2025-09-13T14:01:20.039Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/OrderService.php:0-0
Timestamp: 2025-09-13T14:01:20.039Z
Learning: In PurchasableService::getEffectivePurchasableForPhoto(), both the photo-specific and album-level SQL queries include ->where('is_active', true), so the returned purchasable is guaranteed to be active and doesn't need additional is_active checks in calling code.
Applied to files:
app/Actions/User/Notify.php
📚 Learning: 2025-12-28T18:12:55.752Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3901
File: app/Providers/AppServiceProvider.php:0-0
Timestamp: 2025-12-28T18:12:55.752Z
Learning: When using Laravel Octane's tick API, Octane::tick(...) returns an InvokeTickCallable that only has ->seconds(int) and ->immediate() methods. There is no ->every(N) method. Use the correct usage: Octane::tick('name', fn() => ...)->seconds(N) or Octane::tick('name', fn() => ..., N). Apply this guideline to PHP files across the project (not just AppServiceProvider.php).
Applied to files:
app/Actions/User/Notify.phpapp/Actions/Albums/Flow.phpapp/Models/Album.phpapp/Actions/Album/Delete.phpapp/Relations/HasAlbumThumb.php
📚 Learning: 2025-08-14T10:17:35.082Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3616
File: app/Actions/Album/PositionData.php:38-39
Timestamp: 2025-08-14T10:17:35.082Z
Learning: PositionDataResource uses toPhotoResources() method to convert photos to PhotoResource instances, and PhotoResource accesses the tags relationship ($photo->tags->pluck('name')->all()), making the 'tags' eager-loading in PositionData necessary to avoid N+1 queries.
Applied to files:
app/Models/Album.php
📚 Learning: 2026-01-03T12:36:02.514Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3922
File: app/Listeners/RecomputeAlbumSizeOnPhotoMutation.php:63-69
Timestamp: 2026-01-03T12:36:02.514Z
Learning: In Lychee, "deleting a photo from an album" (removing the photo-album association) is distinct from "deleting a photo entirely". When PhotoDeleted event is dispatched with a specific album_id, it indicates the photo was removed from that particular album only. The photo may still exist in other albums due to the many-to-many relationship, so only the specified album's size statistics need recomputation.
Applied to files:
app/Actions/Album/Delete.php
📚 Learning: 2025-09-13T14:07:11.118Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Http/Controllers/Shop/CatalogController.php:48-52
Timestamp: 2025-09-13T14:07:11.118Z
Learning: In CatalogController, the children_purchasables query intentionally uses parent_id to fetch only direct children albums, not the entire subtree via nested set bounds. This is a design decision to show purchasables from immediate child albums only.
Applied to files:
app/Actions/Album/Delete.php
📚 Learning: 2025-09-21T20:07:11.825Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3702
File: app/Http/Resources/Shop/CatalogResource.php:11-16
Timestamp: 2025-09-21T20:07:11.825Z
Learning: Spatie\LaravelData\DataCollection does NOT extend Illuminate\Support\Collection - it only implements some collection methods. To use Collection type, you must explicitly collect with Collection::class as the second parameter.
Applied to files:
app/Relations/HasAlbumThumb.php
🧬 Code graph analysis (3)
app/Actions/User/Notify.php (1)
app/Models/Album.php (1)
Album(146-548)
app/Actions/Album/Delete.php (2)
app/Models/Album.php (1)
descendants(253-263)app/Actions/Albums/Flow.php (1)
getQuery(107-138)
app/Relations/HasAlbumThumb.php (2)
app/Models/Album.php (4)
Album(146-548)cover(270-273)max_priviledge_cover(290-293)min_priviledge_cover(280-283)app/Models/Extensions/Thumb.php (2)
Thumb(23-168)createFromPhoto(134-167)
🔇 Additional comments (7)
app/Actions/Album/Delete.php (1)
89-106: LGTM!The exclusion of
cover,thumb,min_priviledge_cover, andmax_priviledge_coverrelationships from both the main album query and the descendants query is appropriate for deletion operations where only structural data (id,parent_id,_lft,_rgt,track_short_path) is needed.app/Actions/Albums/Flow.php (1)
114-122: LGTM!The expanded eager loading correctly includes the new privilege-based cover relationships along with their size variants. This aligns with the refactoring goal to avoid additional queries when determining album thumbnails.
app/Models/Album.php (2)
275-293: Relationship methods are correctly defined.The
min_priviledge_cover()andmax_priviledge_cover()HasOne relationships correctly map the album's auto-cover ID fields to Photo records. The PHPDoc return types are accurate.Note: The typo in method names ("priviledge" → "privilege") was flagged in
Notify.php.
206-211: I need the review comment to rewrite. Please provide the original review comment within<review_comment>tags, along with the relevant code context, file information, and any other details needed to verify and rewrite the comment.app/Relations/HasAlbumThumb.php (3)
73-89: Well-structured cover type decision logic.The
getCoverTypeForAlbum()method cleanly centralizes the priority-based cover selection:
- Explicit
cover_idtakes precedence- Admin/owner gets
max_privilegecover- Public users get
least_privilegecoverThis eliminates duplicate decision-making across the class.
155-159: Simplified eager constraints disable any eager loading.Using
whereRaw('1 = 0')is an effective pattern to return an empty result set. This works because the thumb data is now derived from the preloaded cover relationships on Album rather than from a separate query.
218-232: The fallback query is reachable and should be retained.Album instances are loaded without eager-loading cover relationships in many parts of the codebase (e.g.,
MovePhotosRequest,PurchasableAlbumRequest,FromUrlRequest). Additionally, cover ID attributes (cover_id,auto_cover_id_max_privilege,auto_cover_id_least_privilege) can legitimately be null. TheThumb::createFromQueryable()fallback executes when either condition is true and is necessary for correctness. The null-checks and fallback should not be simplified or removed.
Codecov Report❌ Patch coverage is 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/Relations/HasAlbumThumb.php (1)
73-89: Consider using constants or an enum for cover type identifiers.The method returns string literals (
'cover_id','auto_cover_id_max_privilege','auto_cover_id_least_privilege') that are used in multiplematchexpressions throughout the class. Using class constants or an enum would improve type safety and reduce the risk of typos.🔎 Example using class constants
private const COVER_TYPE_EXPLICIT = 'cover_id'; private const COVER_TYPE_MAX_PRIVILEGE = 'auto_cover_id_max_privilege'; private const COVER_TYPE_LEAST_PRIVILEGE = 'auto_cover_id_least_privilege'; protected function getCoverTypeForAlbum(Album $album): string { if ($album->cover_id !== null) { return self::COVER_TYPE_EXPLICIT; } // ... rest of logic }tests/Unit/CoverageTest.php (1)
339-347: Redundant mock configuration.The
owner_idis set both as a direct property assignment (Line 340 via cover_id pattern would be similar) and viashouldReceive('getAttribute'). SincemakePartial()allows property access, theshouldReceivecall is likely redundant and could be removed for clarity.🔎 Simplified mock setup
$album = \Mockery::mock(Album::class)->makePartial(); $album->cover_id = null; $album->auto_cover_id_max_privilege = null; $album->auto_cover_id_least_privilege = null; $album->is_nsfw = false; - $album->shouldReceive('getAttribute') - ->with('owner_id') - ->andReturn(999); + $album->owner_id = 999;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/Actions/Album/Delete.phpapp/Actions/Albums/Flow.phpapp/Actions/User/Notify.phpapp/Models/Album.phpapp/Relations/HasAlbumThumb.phptests/Unit/CoverageTest.php
🚧 Files skipped from review as they are similar to previous changes (2)
- app/Models/Album.php
- app/Actions/User/Notify.php
🧰 Additional context used
📓 Path-based instructions (3)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Any new PHP file should contain the license header and have a single blank line after the opening PHP tag
Variable names should be in snake_case in PHP
Apply the PSR-4 coding standard in PHP
Use in_array() with true as the third parameter in PHP
Only use booleans in if statements, not integers or strings
Use strict comparison (===) instead of loose comparison (==)
Avoid code duplication in both if and else statements
Do not use empty() in PHP
Use the moneyphp/money library for handling monetary values in PHP
Never use floats or doubles to represent monetary values; use integers representing the smallest currency unit (e.g., cents for USD)
**/*.php: Write or extend executable specifications (unit, behaviour, or scenario tests) ahead of implementation, confirm they fail, and then drive code to green before refactoring. List the expected success, validation, and failure branches and add thin failing tests for each path.
For PHP code, adhere to conventions: license headers in new files, strict comparison (===), no empty(), in_array() with third parameter true, snake_case variables, PSR-4 standard, test base classes (AbstractTestCase for Unit, BaseApiWithDataTest for Feature_v2).
Always run phpunit tests. If a test remains red, disable it with a TODO, note the reason, and capture the follow-up in the relevant plan.
Spotless now uses Palantir Java Format 2.78.0 with a 120-character wrap; configure IDE formatters to match before pushing code changes.
Keep each increment's control flow flat by delegating validation/normalisation into tiny pure helpers that return simple enums or result records, then compose them instead of introducing inline branching that inflates the branch count per change.
When introducing new helpers/utilities or editing files prone to style violations (records, DTOs, generated adapters), run the narrowest applicable lint target (for example phpstan) before the full pipeline. Note the command in the related plan/task.
For PHP changes, ru...
Files:
app/Actions/Albums/Flow.phpapp/Relations/HasAlbumThumb.phptests/Unit/CoverageTest.phpapp/Actions/Album/Delete.php
tests/Unit/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Tests in the tests/Unit directory should extend from AbstractTestCase
Files:
tests/Unit/CoverageTest.php
tests/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
No need to mock the database in tests; use the in-memory SQLite database instead
Files:
tests/Unit/CoverageTest.php
🧠 Learnings (6)
📓 Common learnings
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/OrderService.php:65-73
Timestamp: 2025-09-16T21:56:01.607Z
Learning: The validation for album-photo membership in PurchasableService::getEffectivePurchasableForPhoto() uses a JOIN query with the photo_album pivot table to ensure that only purchasables for albums that actually contain the specified photo are returned, preventing price spoofing attacks where clients could use arbitrary album_ids.
📚 Learning: 2025-09-16T21:56:01.607Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/OrderService.php:65-73
Timestamp: 2025-09-16T21:56:01.607Z
Learning: The validation for album-photo membership in PurchasableService::getEffectivePurchasableForPhoto() uses a JOIN query with the photo_album pivot table to ensure that only purchasables for albums that actually contain the specified photo are returned, preventing price spoofing attacks where clients could use arbitrary album_ids.
Applied to files:
app/Actions/Albums/Flow.php
📚 Learning: 2025-09-16T21:56:01.607Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Actions/Shop/OrderService.php:65-73
Timestamp: 2025-09-16T21:56:01.607Z
Learning: The validation for album-photo membership in OrderService::addPhotoToOrder() is implemented within PurchasableService::getEffectivePurchasableForPhoto() using a whereHas('albums') constraint that ensures the photo belongs to the specified album_id before applying pricing logic.
Applied to files:
app/Actions/Albums/Flow.phptests/Unit/CoverageTest.php
📚 Learning: 2025-12-28T18:12:55.752Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3901
File: app/Providers/AppServiceProvider.php:0-0
Timestamp: 2025-12-28T18:12:55.752Z
Learning: When using Laravel Octane's tick API, Octane::tick(...) returns an InvokeTickCallable that only has ->seconds(int) and ->immediate() methods. There is no ->every(N) method. Use the correct usage: Octane::tick('name', fn() => ...)->seconds(N) or Octane::tick('name', fn() => ..., N). Apply this guideline to PHP files across the project (not just AppServiceProvider.php).
Applied to files:
app/Actions/Albums/Flow.phpapp/Relations/HasAlbumThumb.phptests/Unit/CoverageTest.phpapp/Actions/Album/Delete.php
📚 Learning: 2025-09-21T20:07:11.825Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3702
File: app/Http/Resources/Shop/CatalogResource.php:11-16
Timestamp: 2025-09-21T20:07:11.825Z
Learning: Spatie\LaravelData\DataCollection does NOT extend Illuminate\Support\Collection - it only implements some collection methods. To use Collection type, you must explicitly collect with Collection::class as the second parameter.
Applied to files:
app/Relations/HasAlbumThumb.php
📚 Learning: 2025-09-13T14:07:11.118Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3683
File: app/Http/Controllers/Shop/CatalogController.php:48-52
Timestamp: 2025-09-13T14:07:11.118Z
Learning: In CatalogController, the children_purchasables query intentionally uses parent_id to fetch only direct children albums, not the entire subtree via nested set bounds. This is a design decision to show purchasables from immediate child albums only.
Applied to files:
app/Actions/Album/Delete.php
🧬 Code graph analysis (3)
app/Relations/HasAlbumThumb.php (3)
app/Models/Album.php (4)
Album(146-548)cover(270-273)max_privilege_cover(290-293)min_privilege_cover(280-283)app/Relations/BaseHasManyPhotos.php (1)
getRelationQuery(76-85)app/Models/Extensions/Thumb.php (2)
Thumb(23-168)createFromPhoto(134-167)
tests/Unit/CoverageTest.php (1)
app/Relations/HasAlbumThumb.php (1)
HasAlbumThumb(32-234)
app/Actions/Album/Delete.php (2)
app/Models/Album.php (1)
descendants(253-263)app/Eloquent/FixedQueryBuilderTrait.php (1)
select(134-144)
⏰ 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). (20)
- GitHub Check: 2️⃣ PHP dist / 8.5 - postgresql
- GitHub Check: 2️⃣ PHP dist / 8.5 - mariadb
- GitHub Check: 2️⃣ PHP dist / 8.4 - postgresql
- GitHub Check: 2️⃣ PHP dist / 8.4 - sqlite
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Precomputing
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.4 - mariadb -- Install
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Precomputing
- GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- ImageProcessing
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Unit
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Precomputing
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Webshop
- GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Feature_v2
- GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- ImageProcessing
🔇 Additional comments (12)
app/Actions/Album/Delete.php (2)
90-90: LGTM!The exclusion of
min_privilege_coverandmax_privilege_coverfrom eager loading during deletion is consistent with the new Album relationships and aligns with similar exclusions made inNotify.php.
103-103: LGTM!Consistent exclusion of the new privilege-based cover relations for sub-album queries.
app/Actions/Albums/Flow.php (1)
115-121: LGTM!The expanded eager-loading for
max_privilege_coverandmin_privilege_cover(with theirsize_variants) aligns with the new Album relationships and enablesHasAlbumThumbto use preloaded covers instead of executing additional queries.app/Relations/HasAlbumThumb.php (4)
103-111: LGTM!The
selectCoverIdForAlbummethod cleanly maps cover types to actual ID values using a match expression. Thedefault => nullcase provides defensive handling, though it should never be reached givengetCoverTypeForAlbum's exhaustive logic.
155-159: LGTM!Disabling eager loading via
whereRaw('1 = 0')is intentional per the updated design. Thematch()method now relies on preloaded cover relations (cover,max_privilege_cover,min_privilege_cover) instead of query results.
188-200: Verify fallback behavior when privilege cover is null.If
getCoverTypeForAlbumreturns'auto_cover_id_max_privilege'but$album->auto_cover_id_max_privilegeis null (e.g., cover photo deleted), the relation is set to null. This is safe, but consider whether a fallback toauto_cover_id_least_privilegewould be more appropriate in such edge cases, or if the current behavior is intentional.
218-232: LGTM with a note on consistency.The
getResults()logic correctly mirrors thematch()method, using preloaded covers when available and falling back toThumb::createFromQueryable()when no cover ID is set. The fallback query ensures single-album access still works correctly.tests/Unit/CoverageTest.php (5)
34-36: LGTM!Appropriate imports added for the new test coverage of
HasAlbumThumbfunctionality.
194-207: LGTM!Good test coverage for the explicit cover priority case. Reflection is used appropriately to test the protected method.
209-253: LGTM!Comprehensive tests for admin and owner scenarios correctly verifying that
auto_cover_id_max_privilegeis selected when the user has elevated privileges.
255-272: LGTM!Correctly tests the public/unauthenticated user case where
auto_cover_id_least_privilegeshould be selected.
328-360: Well-documented edge case test.The comment explaining why a separate parent album is needed to avoid triggering DB queries during
HasAlbumThumbconstruction is helpful. The test correctly verifies thatselectCoverIdForAlbumreturns null when all cover IDs are null.
Ambitious untested (yet) change...
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.