Add 2 new smart albums: My rated pictures and My best rated pictures #4041
Conversation
📝 WalkthroughWalkthroughAdds two user-rating smart albums (My Rated Pictures, My Best Pictures): enum cases, factory registrations, two smart-album classes with per-user rating queries and top‑N tie logic, config keys and migration, policy/middleware updates, translations, tests, docs, and phpstan in dev deps. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/specs/4-architecture/roadmap.md (1)
94-94: Update the "Last updated" date.The document shows completion dates of 2026-01-28 (lines 21, 23, 24), but the footer indicates "Last updated: 2026-01-26". This should be updated to reflect the actual modification date.
📝 Suggested fix
-*Last updated: 2026-01-26* +*Last updated: 2026-01-28*As per coding guidelines: "At the bottom of documentation files, add an hr line followed by 'Last updated: [date of the update]'"
docs/specs/4-architecture/open-questions.md (1)
1752-1754: Update the “Last updated” date.The file was modified for this PR; the footer should reflect the current update date.
As per coding guidelines: “At the bottom of documentation files, add an hr line followed by *Last updated: [date of the update]*.”✏️ Suggested update
-*Last updated: 2026-01-02* +*Last updated: 2026-01-28*
🧹 Nitpick comments (7)
lang/cz/all_settings.php (1)
468-470: Minor grammatical inconsistency in verb form.Line 468 uses "Shows" (third person singular) while line 469 uses "Show" (imperative/base form). Consider using consistent verb forms.
Suggested fix for consistency
- 'enable_my_rated_pictures' => 'Shows all photos rated by the current user.', - 'enable_my_best_pictures' => 'Show top-rated photos by the current user.', + 'enable_my_rated_pictures' => 'Shows all photos rated by the current user.', + 'enable_my_best_pictures' => 'Shows top-rated photos by the current user.',lang/pt/all_settings.php (2)
138-140: Translations appear to be in English rather than Portuguese.These strings are added to the Portuguese translation file (
lang/pt/) but are in English. If this is a placeholder pending translation, consider adding a TODO or tracking this for future localization. If the existing pattern in this file intentionally uses English as a base, this is consistent.
468-470: Minor grammatical inconsistency between "Shows" and "Show".Line 468 uses "Shows all photos..." while line 469 uses "Show top-rated photos..." — consider using consistent verb forms for parallel entries.
Suggested fix for consistency
- 'enable_my_rated_pictures' => 'Shows all photos rated by the current user.', - 'enable_my_best_pictures' => 'Show top-rated photos by the current user.', + 'enable_my_rated_pictures' => 'Show all photos rated by the current user.', + 'enable_my_best_pictures' => 'Show top-rated photos by the current user.',lang/nl/all_settings.php (2)
138-140: Align My Best Pictures count description with tie-inclusion behavior.To match the existing
best_pictures_countdocumentation and avoid ambiguity, consider mentioning the tie-at-cutoff behavior here too.♻️ Suggested tweak
- 'my_best_pictures_count' => 'My Best Pictures album photo count.', + 'my_best_pictures_count' => 'Number of top-rated photos to show in My Best Pictures album. Photos tied at the cutoff are included.',
468-470: Keepenable_*detail phrasing consistent.Neighboring entries use “Enable … smart album.”; matching that pattern improves uniformity.
♻️ Suggested tweak
- 'enable_my_rated_pictures' => 'Shows all photos rated by the current user.', - 'enable_my_best_pictures' => 'Show top-rated photos by the current user.', + 'enable_my_rated_pictures' => 'Enable My Rated Pictures smart album.', + 'enable_my_best_pictures' => 'Enable My Best Pictures smart album.',lang/fr/all_settings.php (2)
136-138: Optional: Consider adding French translations.The new entries are in English while this is a French language file. Nearby rating-related settings (lines 128-133) have French translations. Consider translating these for consistency:
enable_my_rated_pictures: "Affiche toutes les photos notées par l'utilisateur actuel."enable_my_best_pictures: "Afficher les photos les mieux notées par l'utilisateur actuel."my_best_pictures_count: "Nombre de photos les mieux notées à afficher dans l'album Mes meilleures photos. Les photos à égalité au seuil sont incluses."
466-468: Optional: Consider adding French translations for documentation section.Same as above - these entries are in English. Consider translating for consistency with other French entries in the file.
ildyria
force-pushed
the
add-my-rated-photos
branch
from
5b16269 to
d66ced0
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/specs/4-architecture/roadmap.md (1)
92-94: Update “Last updated” to reflect today’s change.This roadmap now includes completions dated 2026-01-28, but the footer still shows 2026-01-26. Please update the “Last updated” line to the actual edit date.
As per coding guidelines: “At the bottom of documentation files, add an hr line followed by ‘Last updated: [date of the update]’.”
🧹 Nitpick comments (9)
lang/sv/gallery.php (1)
57-58: Consider translating these strings to Swedish.The new keys
my_rated_picturesandmy_best_pictureshave English values in the Swedish locale file. While this is consistent with other untranslated entries in this file, consider providing proper Swedish translations (e.g., "Mina betygsatta bilder" and "Mina bästa bilder") for a complete localization.💬 Suggested Swedish translations
- 'my_rated_pictures' => 'My Rated Pictures', - 'my_best_pictures' => 'My Best Pictures', + 'my_rated_pictures' => 'Mina betygsatta bilder', + 'my_best_pictures' => 'Mina bästa bilder',lang/ar/gallery.php (1)
56-57: Missing Arabic translations for new entries.The new keys
my_rated_picturesandmy_best_pictureshave English values instead of Arabic translations. While this follows the pattern of other untranslated strings in this file (e.g.,Untagged,Unrated, star ratings), consider providing proper Arabic translations for consistency with the translated portion of this locale file.lang/ru/all_settings.php (1)
138-140: Align “My Best Pictures” count documentation with existing wording.The
my_best_pictures_countdoc string is much shorter thanbest_pictures_countand omits the tie-at-cutoff behavior. Consider mirroring the existing phrasing for clarity and consistency.Proposed wording update
- 'my_best_pictures_count' => 'My Best Pictures album photo count.', + 'my_best_pictures_count' => 'Number of top-rated photos to show in My Best Pictures album. Photos tied at the cutoff are included.',tests/Unit/Enum/SmartAlbumTypeTest.php (1)
68-74: Update the docblock count to 14 for consistency.
The assertion now expects 14 cases, but the comment still says 12.💡 Proposed tweak
- * Test that all 12 smart album types exist. + * Test that all 14 smart album types exist.lang/fr/gallery.php (1)
56-57: Translation keys are in English instead of French.The new entries
my_rated_picturesandmy_best_pictureshave English values. Suggested French translations:
'my_rated_pictures' => 'Mes photos évaluées''my_best_pictures' => 'Mes meilleures photos'Note: This is consistent with some other untranslated entries in this file (e.g., line 55
best_pictures, lines 67-68filter).lang/nl/gallery.php (1)
57-58: Translation keys are in English instead of Dutch.The new entries have English values. Suggested Dutch translations:
'my_rated_pictures' => 'Mijn beoordeelde foto's''my_best_pictures' => 'Mijn beste foto's'This follows the same pattern as other recently added entries in this file.
tests/Feature_v2/SmartAlbums/MyBestPicturesAlbumTest.php (1)
42-50: Misleading docblock comment.The docblock states "limit=2" but line 50 sets
my_best_pictures_countto3. The comment should be updated to match the actual test configuration: with limit=3, the 3rd position is a 4★ photo, so all photos with rating ≥4★ are included (2×5★ + 3×4★ = 5 total).Suggested comment fix
/** * S-011-02: Test top N with ties. - * With 2×5★ and 3×4★, limit=2 means cutoff is at 2nd photo (5★). - * So only photos with 5★ are included (2 photos total). + * With 2×5★ and 3×4★, limit=3 means cutoff is at 3rd photo (4★). + * So all photos with rating ≥4★ are included (5 photos total due to ties). */tests/Feature_v2/SmartAlbums/MyRatedSmartAlbumsIntegrationTest.php (2)
148-152: Verify comment accuracy regarding photo ownership.The comment states photo1 is "accessible" to
userMayUpload1, but doesn't clarify ownership. InBaseApiWithDataTest, photos are typically owned by admin. Ifphoto1is owned by admin, the comment should reflect thatuserMayUpload1has view access rather than ownership. This doesn't affect test correctness but may confuse future maintainers.Suggested comment clarification
- // userMayUpload1 rates their own photo1 (accessible) + // userMayUpload1 rates photo1 (has view access)
267-271: Inconsistent comment about photo ownership.Line 267 says "userMayUpload1 rates their own photo1" but photo1 is owned by admin in the typical test fixture. Line 283 correctly says "their own photo1" for the assertion message, which may be misleading if photo1 isn't actually owned by
userMayUpload1.Suggested comment fix
- // userMayUpload1 rates their own photo1 (which they can see) + // userMayUpload1 rates photo1 (which they can see via permissions)
Codecov Report❌ Patch coverage is 🚀 New features to boost your workflow:
|
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.