Add dynamic background modes with enum config #4144
Conversation
Implements Feature 025 - Dynamic Landing Background Options (#1106). Adds 5 background modes for landscape and portrait orientations: - static: URL passthrough (existing behavior, default) - photo_id: Specific photo by ID (no public access check) - random: Random public photo - latest_album_cover: Latest album's cover - random_from_album: Random photo from album Changes: - Add landing_background_landscape_mode and landing_background_portrait_mode enum configs - Implement resolveBackgroundUrl() with graceful fallback (no exceptions) - Update English translations Spec impact: docs/specs/4-architecture/features/025-dynamic-landing-backgrounds/ Resolves: #1106
📝 WalkthroughWalkthroughAdds dynamic landing-page background resolution with five modes (static, photo_id, random, latest_album_cover, random_from_album); introduces a mode enum, migration for two mode configs, backend resolution helpers with fallback and logging, comprehensive feature docs, roadmap update, and many locale string changes. 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 |
Implements Feature 025 - Dynamic Landing Background Options (#1106). Adds 5 background modes for landscape and portrait orientations: - static: URL passthrough (existing behavior, default) - photo_id: Specific photo by ID (no public access check) - random: Random public photo - latest_album_cover: Latest album's cover - random_from_album: Random photo from album Implementation: - Add LandingBackgroundModeType enum with proper backed enum values - Add landing_background_landscape_mode and landing_background_portrait_mode enum configs - Use ConfigManager::getValueAsEnum() for type-safe mode retrieval - Implement resolveBackgroundUrl() with graceful fallback (no exceptions) - Update English translations Spec impact: docs/specs/4-architecture/features/025-dynamic-landing-backgrounds/ Resolves: #1106
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
app/Http/Resources/GalleryConfigs/LandingPageResource.php (1)
111-111: Use nullsafe access for original size variant URL.
getOriginal()->urlcan throw when no original variant object exists, which pushes normal fallback flow into exception/logging.♻️ Suggested fix
-return $photo->size_variants->getMedium()?->url ?? $photo->size_variants->getOriginal()->url ?? self::FALLBACK_IMAGE; +return $photo->size_variants->getMedium()?->url ?? $photo->size_variants->getOriginal()?->url ?? self::FALLBACK_IMAGE;Also applies to: 133-133, 173-173, 208-208
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/Http/Resources/GalleryConfigs/LandingPageResource.phpdatabase/migrations/2026_03_03_120000_add_dynamic_landing_background_modes.phpdocs/specs/4-architecture/features/025-dynamic-landing-backgrounds/plan.mddocs/specs/4-architecture/features/025-dynamic-landing-backgrounds/spec.mddocs/specs/4-architecture/features/025-dynamic-landing-backgrounds/tasks.mddocs/specs/4-architecture/roadmap.mdlang/en/all_settings.php
docs/specs/4-architecture/features/025-dynamic-landing-backgrounds/plan.md
Show resolved
Hide resolved
docs/specs/4-architecture/features/025-dynamic-landing-backgrounds/spec.md
Show resolved
Hide resolved
docs/specs/4-architecture/features/025-dynamic-landing-backgrounds/spec.md
Show resolved
Hide resolved
docs/specs/4-architecture/features/025-dynamic-landing-backgrounds/tasks.md
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
lang/pl/all_settings.php (1)
210-213: Polish translation file contains English text.The new translation entries for
landing_background_landscape_mode,landing_background_portrait_mode,landing_background_landscape, andlanding_background_portraitare in English rather than Polish. If this is intentional (e.g., placeholders for later translation), no action is needed. Otherwise, consider translating these strings to Polish for consistency with the locale.Also applies to: 550-553
lang/cz/all_settings.php (1)
550-553: Add an explicit visibility warning forlatest_album_cover/random_from_albummodes.The option descriptions are clear, but they should explicitly warn admins that these modes can expose configured assets on the public landing page if misconfigured.
💡 Suggested wording update
- 'landing_background_landscape_mode' => 'Options: static (URL), photo_id (specific photo), random (random public photo), latest_album_cover (latest album cover), random_from_album (random from album).', - 'landing_background_portrait_mode' => 'Options: static (URL), photo_id (specific photo), random (random public photo), latest_album_cover (latest album cover), random_from_album (random from album).', + 'landing_background_landscape_mode' => 'Options: static (URL), photo_id (specific photo), random (random public photo), latest_album_cover (latest album cover), random_from_album (random from album). Ensure configured sources are safe for public display.', + 'landing_background_portrait_mode' => 'Options: static (URL), photo_id (specific photo), random (random public photo), latest_album_cover (latest album cover), random_from_album (random from album). Ensure configured sources are safe for public display.',Based on learnings In
LandingPageResource(app/Http/Resources/GalleryConfigs/LandingPageResource.php),resolveLatestAlbumCoverandresolveRandomFromAlbumintentionally do not applyPhotoQueryPolicyfilters.lang/ar/all_settings.php (1)
212-213: Clarify behavior when mode does not use a configured value.At Line 212-213 and Line 552-553, the copy explains URL/photo/album IDs, but it doesn’t explicitly say that
randomandlatest_album_coverignore this value. Adding that note would reduce admin confusion.Proposed wording tweak
- 'landing_background_landscape' => 'Value for landscape background (URL, photo ID, or album ID)', - 'landing_background_portrait' => 'Value for portrait background (URL, photo ID, or album ID)', + 'landing_background_landscape' => 'Value for landscape background (URL, photo ID, or album ID; unused for random and latest_album_cover)', + 'landing_background_portrait' => 'Value for portrait background (URL, photo ID, or album ID; unused for random and latest_album_cover)', ... - 'landing_background_landscape' => 'Depends on mode: URL for static, photo ID for photo_id, album ID for random_from_album. This image is also used when sharing the gallery link directly.', - 'landing_background_portrait' => 'Depends on mode: URL for static, photo ID for photo_id, album ID for random_from_album.', + 'landing_background_landscape' => 'Depends on mode: URL for static, photo ID for photo_id, album ID for random_from_album. Unused for random and latest_album_cover. This image is also used when sharing the gallery link directly.', + 'landing_background_portrait' => 'Depends on mode: URL for static, photo ID for photo_id, album ID for random_from_album. Unused for random and latest_album_cover.',Also applies to: 552-553
app/Http/Resources/GalleryConfigs/LandingPageResource.php (2)
78-82: Avoid logging raw background config values.Line 80 logs the full configured value. For static URL mode, this can include sensitive query tokens. Prefer logging redacted/derived metadata instead of the full raw value.
🔒 Suggested adjustment
\Log::notice('Landing background resolution failed', [ 'mode' => $mode?->value, - 'value' => $value, + 'value_present' => $value !== '', + 'value_kind' => $mode === LandingBackgroundModeType::STATIC ? 'url' : 'id', 'error' => $e->getMessage(), ]);
115-116: Extract repeated photo URL fallback logic into one helper.The same medium→original→fallback selection is duplicated four times; a private helper will reduce drift and simplify future updates.
Also applies to: 137-138, 177-178, 212-213
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
app/Enum/LandingBackgroundModeType.phpapp/Http/Resources/GalleryConfigs/LandingPageResource.phpdatabase/migrations/2026_03_03_120000_add_dynamic_landing_background_modes.phplang/ar/all_settings.phplang/bg/all_settings.phplang/cz/all_settings.phplang/de/all_settings.phplang/el/all_settings.phplang/es/all_settings.phplang/fa/all_settings.phplang/fr/all_settings.phplang/hu/all_settings.phplang/it/all_settings.phplang/ja/all_settings.phplang/nl/all_settings.phplang/no/all_settings.phplang/pl/all_settings.phplang/pt/all_settings.phplang/ru/all_settings.phplang/sk/all_settings.phplang/sv/all_settings.phplang/vi/all_settings.phplang/zh_CN/all_settings.phplang/zh_TW/all_settings.php
✅ Files skipped from review due to trivial changes (1)
- lang/nl/all_settings.php
🚧 Files skipped from review as they are similar to previous changes (1)
- database/migrations/2026_03_03_120000_add_dynamic_landing_background_modes.php
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/Http/Resources/GalleryConfigs/LandingPageResource.php (2)
115-115: Extract duplicated photo URL fallback logic into one helper.The same URL-resolution chain appears in four methods. Centralizing it reduces drift and keeps fallback behavior consistent.
♻️ Proposed refactor
+ private function resolvePhotoUrl(?Photo $photo): string + { + if ($photo === null) { + return self::FALLBACK_IMAGE; + } + + return $photo->size_variants->getMedium()?->url + ?? $photo->size_variants->getOriginal()?->url + ?? self::FALLBACK_IMAGE; + } + private function resolvePhotoById(string $value): string { $photo = Photo::query()->with(['size_variants'])->find($value); - - if ($photo === null) { - return self::FALLBACK_IMAGE; - } - - return $photo->size_variants->getMedium()?->url ?? $photo->size_variants->getOriginal()->url ?? self::FALLBACK_IMAGE; + return $this->resolvePhotoUrl($photo); }As per coding guidelines: "Avoid code duplication in both if and else statements."
Also applies to: 137-137, 177-177, 214-214
131-132: Consider a more scalable random-selection strategy for high-cardinality datasets.
inRandomOrder()->limit(1)is typically expensive on large tables and runs on a hot path. A cached shortlist or precomputed random candidate strategy would reduce DB load.Also applies to: 206-207
Codecov Report❌ Patch coverage is 🚀 New features to boost your workflow:
|
1 participant
Fixes #1106
Summary by CodeRabbit