feat: Implement API album display with password protection and expira…#589
feat: Implement API album display with password protection and expira…#589aviv926 wants to merge 32 commits into
Conversation
WalkthroughAdds a password-protected API for aggregating Immich albums, frontend UI for tabbed/searchable album selection, i18n support with many locale files, cache TTL changes (cache.Set signature and usage), and related build/config updates (Go, Dockerfile, biome, package.json). Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Browser
participant Frontend
participant Backend as /api/albums
participant ImmichAPI as Immich
User->>Browser: Open redirects overlay
Browser->>Frontend: render overlay
Frontend->>Backend: GET /api/albums (may include X-Kiosk-Password)
Backend->>Backend: validate X-Kiosk-Password vs config
alt password missing/invalid
Backend-->>Frontend: 401 Unauthorized
Frontend->>User: prompt for password
User->>Frontend: submit password
Frontend->>Backend: GET /api/albums (with X-Kiosk-Password)
end
Backend->>ImmichAPI: Fetch Main Library albums
ImmichAPI-->>Backend: Main albums
Backend->>ImmichAPI: Fetch per-user albums (for each API key)
ImmichAPI-->>Backend: Per-user albums
Backend->>Backend: deduplicate and aggregate albums by ID
Backend-->>Frontend: 200 OK with albums map (Main Library, User libs...)
Frontend->>Frontend: render tabs, search, sort, checkboxes
User->>Frontend: select albums & click Apply
Frontend->>Browser: navigate with selected albums params
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/ts/menu.ts (1)
677-742: Guard redirect keyboard navigation when there are no anchor links
redirectKeyHandlerassumesredirects.length > 0. In configurations with no config redirects and no Home link (no kiosk password), the redirects overlay can contain only the API albums UI (no<a>elements). Pressing ArrowUp/ArrowDown in that state will compute(...) % 0and then try to.focus()onundefined, causing a runtime error.Add a simple length check before handling arrow keys:
function redirectKeyHandler(e: KeyboardEvent) { - if (!redirects) return; + if (!redirects || redirects.length === 0) return; @@ - switch (e.code) { + switch (e.code) { case "ArrowDown": e.preventDefault(); // Prevent page scrolling currentRedirectIndex = (currentRedirectIndex + 1) % redirects.length; redirects[currentRedirectIndex].focus(); break;(Apply the same guard to the ArrowUp path as appropriate.)
🧹 Nitpick comments (7)
internal/immich/immich_album.go (1)
100-134: Album de‑duplication is sound; consider deterministic orderingUsing a
map[string]Albumto merge owned and shared albums by ID correctly removes duplicates and keeps counts consistent for downstream logic (including random selection and API aggregation). The only side effect is that album order becomes non‑deterministic due to map iteration; if any callers rely on a stable order, consider sortingallbyAlbumNameorIDbefore returning.frontend/src/ts/menu.ts (1)
27-616: API albums UI and password flow are well implemented, with a couple of small polish opportunitiesThe client‑side handling for
/api/albums(password prompt + countdown, loading, per‑user tabs, search, sort, selection, and URL application) is coherent and matches the backend contract. A few non‑blocking refinements you might consider:
- Select‑all label with preselected albums: when albums are initialised from URL params, the “Select All” button text/class aren’t recalculated until the user interacts. Calling
updateSelectAllBtn()once afterrenderAlbumsList()inloadApiAlbums()would keep the button state accurate from the start.- Client‑side de‑duplication cost: the
filter(... findIndex(...))pattern for album de‑duplication is O(n²). For typical album counts this is fine, but if you expect very large lists, aMap/Set‑based dedupe keyed byidwould be more scalable. (The backend also de‑dupes, so this is mostly defensive.)- Static analysis innerHTML warnings: the various
innerHTMLassignments use fixed strings/templates without interpolated user input, so there’s no immediate XSS risk. If you ever inject error text or other dynamic content into these templates, prefertextContentor a sanitiser at the insertion point.Overall, this is a substantial but clean addition on the frontend side.
frontend/src/css/redirects.css (5)
166-211: Refactor fragile absolute positioning of.clear-search-btn.Line 196 uses a hardcoded
right: 7.5remvalue that couples the button's position to the adjacent.select-all-btnwidth. This is brittle and prone to misalignment if button widths change or media queries adjust sizing.Consider restructuring the
.albums-search-containerlayout to use flexbox instead of absolute positioning:.albums-search-container { padding: 0.75rem 1rem 0 1rem; display: flex; gap: 0.5rem; - position: relative; + align-items: center; } - #album-search-input { + #album-search-input { - width: 100%; + flex: 1; padding: 0.6rem 2.5rem 0.6rem 0.8rem; ... } - .clear-search-btn { + .clear-search-btn { - position: absolute; + position: relative; - right: 7.5rem; - top: 50%; - transform: translateY(-50%); + flex-shrink: 0; ... }This eliminates the magic number dependency and keeps the layout maintainable.
123-142: Add Firefox scrollbar styling for better cross-browser consistency.The custom scrollbar styling uses only webkit-prefixed properties. Firefox will display default scrollbars instead of the mint-green themed ones, reducing visual consistency.
Add Firefox scrollbar properties to both
.albums-tabsand.api-albums-list:.albums-tabs { display: flex; overflow-x: auto; background: rgba(0, 0, 0, 0.2); border-bottom: 1px solid rgba(255, 255, 255, 0.1); + scrollbar-width: thin; + scrollbar-color: rgba(255, 255, 255, 0.2) rgba(255, 255, 255, 0.05); + /* Custom scrollbar */ &::-webkit-scrollbar {And similarly for
.api-albums-list:.api-albums-list { flex: 1; overflow-y: auto; padding: 0.5rem; max-height: 400px; + scrollbar-width: thin; + scrollbar-color: var(--mint-green) rgba(255, 255, 255, 0.1); + /* Custom scrollbar */ &::-webkit-scrollbar {Also applies to: 297-313
290-314: Consider viewport-relative sizing for.api-albums-listinstead of fixedmax-height.Line 294 sets
max-height: 400px, which is a hardcoded value that may not adapt well to different device heights or content volumes. On mobile (line 571), this is further restricted to250px, potentially cutting off albums that users need to access.Replace the fixed pixel values with viewport-relative sizing:
.api-albums-list { flex: 1; overflow-y: auto; padding: 0.5rem; - max-height: 400px; + max-height: clamp(200px, 50vh, 600px);This provides:
- A minimum of 200px for mobile devices
- Up to 50% of viewport height for larger screens
- A maximum of 600px to prevent excessively tall lists
For mobile:
.api-albums-list { - max-height: 250px; + max-height: clamp(150px, 40vh, 300px); padding: 0.25rem; }
271-288: Specify transition timing for.refresh-btnrotate transforms.Lines 282 and 286 apply
rotate()transforms on hover and active states without explicit transition timing. This could appear instantaneous and jarring. Line 256 setstransition: all 0.2s easefor other properties, but the rotate transform may snap without smoothing.Ensure the rotate transform is explicitly covered by the transition or specify it separately:
.refresh-btn { background-color: rgba(30, 210, 187, 0.1); color: var(--mint-green); border-color: rgba(30, 210, 187, 0.3); font-size: 1rem; padding: 0.4rem 0.6rem; min-width: 2.2rem; + transition: all 0.2s ease; &:hover { background-color: rgba(30, 210, 187, 0.2); border-color: rgba(30, 210, 187, 0.5); transform: rotate(90deg); }The
transition: all 0.2s easeat line 256 is in.sort-btn, not.refresh-btn. Ensure.refresh-btnhas its own transition.
194-211: Address mobile layout issue with.clear-search-btnabsolute positioning.The
.clear-search-btnuses absolute positioning withright: 7.5rem(line 196), which assumes a specific layout width. On mobile devices (max-width: 768px), this button may not align correctly or could overlap with the search input. There is no mobile-specific repositioning in the media query.Add mobile-specific styling to reposition the button or refactor to use flexbox (as suggested earlier). If keeping absolute positioning, add mobile adjustments:
@media (max-width: 768px) { .redirects-content { flex-direction: column; gap: 1rem; padding: 0.5rem; } + .albums-search-container { + flex-wrap: wrap; + gap: 0.75rem; + } + + .clear-search-btn { + right: auto; + position: relative; + transform: none; + top: auto; + }Alternatively (preferred): implement the flexbox refactoring suggested in the earlier comment, which would automatically work on mobile without additional media query rules.
Also applies to: 548-596
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
config.example.yaml(1 hunks)config.schema.json(1 hunks)frontend/src/css/redirects.css(1 hunks)frontend/src/ts/menu.ts(4 hunks)internal/config/config.go(2 hunks)internal/immich/immich_album.go(1 hunks)internal/routes/routes_albums.go(1 hunks)internal/templates/partials/redirects.templ(1 hunks)main.go(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: damongolding
Repo: damongolding/immich-kiosk PR: 261
File: internal/immich/immich_album.go:168-178
Timestamp: 2025-01-13T16:11:39.296Z
Learning: The `AssetsOrdered` field in `ImmichAlbum` struct is persisted in cache through JSON serialization, maintaining the album's ordering state between requests.
📚 Learning: 2025-01-13T16:11:39.296Z
Learnt from: damongolding
Repo: damongolding/immich-kiosk PR: 261
File: internal/immich/immich_album.go:168-178
Timestamp: 2025-01-13T16:11:39.296Z
Learning: The `AssetsOrdered` field in `ImmichAlbum` struct is persisted in cache through JSON serialization, maintaining the album's ordering state between requests.
Applied to files:
internal/immich/immich_album.go
🧬 Code graph analysis (4)
main.go (2)
internal/routes/routes_albums.go (1)
Albums(14-78)internal/immich/immich.go (1)
Albums(218-218)
internal/immich/immich_album.go (1)
internal/immich/immich.go (2)
Album(210-216)Albums(218-218)
internal/routes/routes_albums.go (4)
internal/config/config.go (2)
Config(211-439)New(442-450)internal/common/common.go (1)
Common(22-26)internal/routes/routes.go (1)
InitializeRequestData(66-106)internal/immich/immich.go (1)
Error(85-89)
frontend/src/ts/menu.ts (1)
internal/immich/immich.go (1)
Error(85-89)
🪛 ast-grep (0.40.0)
frontend/src/ts/menu.ts
[warning] 238-238: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: albumsList.innerHTML = '
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 263-270: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: albumsList.innerHTML = <div class="password-prompt"> <div class="password-prompt-title">Enter Password</div> <input type="password" id="api-albums-password" placeholder="Password" /> <button id="submit-password-btn">Submit</button> <button id="cancel-password-btn">Cancel</button> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 290-290: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: albumsList.innerHTML = '
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 326-333: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: albumsList.innerHTML = <div class="password-error"> <div class="password-error-title">Incorrect Password</div> <div class="password-error-message">The password you entered is incorrect. Please try again.</div> <button id="retry-password-btn">Try Again</button> <button id="cancel-password-btn">Cancel</button> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 347-347: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: albumsList.innerHTML = '
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 367-367: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: tabsContainer.innerHTML = ""
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 422-422: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: albumsList.innerHTML = ""
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 432-432: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: albumsList.innerHTML = '
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 238-238: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: albumsList.innerHTML = '
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 263-270: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: albumsList.innerHTML = <div class="password-prompt"> <div class="password-prompt-title">Enter Password</div> <input type="password" id="api-albums-password" placeholder="Password" /> <button id="submit-password-btn">Submit</button> <button id="cancel-password-btn">Cancel</button> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 290-290: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: albumsList.innerHTML = '
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 326-333: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: albumsList.innerHTML = <div class="password-error"> <div class="password-error-title">Incorrect Password</div> <div class="password-error-message">The password you entered is incorrect. Please try again.</div> <button id="retry-password-btn">Try Again</button> <button id="cancel-password-btn">Cancel</button> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 347-347: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: albumsList.innerHTML = '
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 432-432: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: albumsList.innerHTML = '
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
🔇 Additional comments (8)
config.example.yaml (1)
143-157: API albums password config looks consistent
api_albums_passwordis clearly documented, defaults to empty (feature off), and matches the backend field name. No issues here.internal/config/config.go (1)
91-105: Config and env wiring for API albums password is correctThe
APIAlbumsPasswordfield and itsyaml/mapstructuretags line up withkiosk.api_albums_password, and the env binding toKIOSK_API_ALBUMS_PASSWORDmatches existing conventions. Redaction is correctly enabled.Also applies to: 470-495
config.schema.json (1)
409-493: Schema update matches config surface
kiosk.api_albums_passwordis correctly added as astringunderkiosk.properties, aligned with the Go struct and example YAML. Nothing further needed here.main.go (1)
184-187: /api/albums route wiring is straightforward; be aware of double‑password scenariosThe new
GET /api/albumsregistration is consistent with existing route wiring. Note that whenkiosk.passwordis set, this endpoint will still be protected by global KeyAuth in addition to the optionalAPIAlbumsPasswordheader check in the handler, so some setups will effectively require both passwords.internal/templates/partials/redirects.templ (1)
20-69: Template structure and IDs align with the new API albums UIThe split into “Albums from Config” and “Select Albums From API” keeps concerns clear, and the IDs/classes (tabs, search, sort, apply) match the TypeScript logic.
strings.CutPrefixfor redirect names is a nice touch to normalise the URLs. No functional issues spotted.internal/routes/routes_albums.go (1)
1-78: Albums route handler is correctly structured and robust to partial failuresRequest initialisation, optional password enforcement via
X-Kiosk-Password, and per‑user album aggregation all look solid. Per‑user failures are logged and don’t abort the whole response, which is sensible. The copy‑on‑write pattern foruserConfigavoids mutatingbaseConfig. No blocking issues here.frontend/src/css/redirects.css (2)
46-73: Verify design system variables are defined.The gradients reference
var(--fade-gradient)which is not defined in this file. Confirm that this custom property is available in the design system (e.g., in a root CSS file or vars file).
1-596: Verify colour contrast ratios meet WCAG AA standards for all interactive and informational elements.The CSS uses low-opacity white values (e.g.,
rgba(255, 255, 255, 0.05)andrgba(255, 255, 255, 0.1)) for borders and secondary text over the dark overlay background (rgb(51 52 96 / 60%)), which likely fail the 4.5:1 contrast requirement for normal text. The error colour#ff6b6bshould also be validated. Use a WCAG contrast checker to confirm all foreground-background colour pairs meet AA standards, particularly for text and interactive controls.
This change is intended to add the image background blur to zoom and super zoom image effects when background blur is enabled. Current example config where background blur is absent: background_blur: true background_blur_amount: 4 layout: splitview image_fit: contain image_effect: zoom image_effect_amount: 120
much easier to read now IMO
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
locales/ja.toml (1)
1-12: Empty string pattern consistent with other locales.Same keys are empty as in the Finnish and Russian locales.
🧹 Nitpick comments (15)
locales/pt.toml (1)
1-53: Verify empty translations and consider native speaker review.Several translation keys have empty string values (e.g.,
assets,downloading_assets,error_message_*,expiration,limit,processing_images,retrieving_asset,retrieving_image_data,storage_capacity). Whilst the i18n system has fallback logic, consider completing these translations or documenting them as intentional placeholders.Since the PR notes the code is AI-generated, it would be prudent to have a native Portuguese speaker review these translations for accuracy and naturalness.
locales/tr.toml (1)
1-53: Verify empty translations and consider native speaker review.Numerous translation keys have empty values (e.g.,
assets,downloading_assets,error_message_*,expiration,limit,processing_images,retrieving_asset,retrieving_image_data,storage_capacity). Whilst the i18n fallback mechanism will handle these gracefully, completing these translations would improve the user experience for Turkish speakers.Given the AI-generated nature of the code, native Turkish speaker verification would be advisable to ensure translation quality and cultural appropriateness.
locales/hu.toml (1)
1-53: Verify empty translations and consider native speaker review.Multiple translation keys contain empty values (e.g.,
assets,downloading_assets,error_message_*,expiration,limit,processing_images,retrieving_asset,retrieving_image_data,storage_capacity). Consider completing these translations to provide a complete localized experience for Hungarian users.As the PR indicates AI-generated content, having a native Hungarian speaker review the translations would help ensure accuracy and natural language usage.
locales/pl.toml (1)
1-53: Verify empty translations and consider native speaker review.Several translation keys are set to empty strings (e.g.,
assets,downloading_assets,error_message_*,expiration,limit,processing_images,retrieving_asset,retrieving_image_data,storage_capacity). Whilst the i18n fallback will prevent runtime errors, completing these translations would benefit Polish-speaking users.Given that the PR notes AI-generated code, it would be prudent to have a native Polish speaker review these translations for accuracy and idiomatic usage.
internal/templates/views/view_url_builder.templ (1)
20-27: Consider extracting the language detection logic to reduce duplication.The language extraction pattern
strings.Split(string(viewData.SystemLang), "_")is duplicated across multiple template files (e.g., views_home.templ). Consider adding a helper function in the ViewData struct or a utility package to centralise this logic and ensure consistent language handling across templates.internal/templates/views/views_home.templ (1)
20-29: LGTM! Language attribute properly dynamically set.The language extraction and conditional lang attribute implementation is correct. However, note that this logic is duplicated in view_url_builder.templ. Consider extracting to a shared helper as mentioned in the review for that file.
locales/bg.toml (1)
1-53: Avoid empty-string translations to prevent blank UI textSeveral keys (for example
assets,downloading_assets,error_message_*,expiration,limit,processing_images,retrieving_asset,retrieving_image_data,storage_capacity) are set to"". If these are meant as “not translated yet”, the UI will currently show empty labels rather than readable text. It is usually better to either:
- copy the default (e.g. English) text, or
- omit the key entirely until you have a real translation,
so users never see blank labels.
locales/uk.toml (1)
1-53: Handle untranslated Ukrainian keys without rendering empty labelsA number of entries (
assets,downloading_assets,error_message_*,expiration,limit,processing_images,retrieving_asset,retrieving_image_data,storage_capacity, etc.) are defined with"". That will cause those labels to disappear entirely in the UI for this locale. Consider either copying the default language text or omitting those keys until translations are available, so users are never presented with blank labels.internal/routes/routes_offline.go (1)
24-25: Offline “downloading assets” status is correctly localised but could be more flexibleThe new
t := i18n.T()usage and substitution oflimit,assets,storage_capacity,expiration, anddownloading_assetswork as expected and do not affect control flow.Two small i18n polish points you might consider later:
- The list items are assembled as
Label: <strong>value</strong> units, which bakes in English word order. Some languages may want to reorder or inflect around the number; moving these whole phrases into translatable strings (with placeholders for the values) would give translators more control.maxSizeMessage/expiryMessagestill use the literal"None", so in non-English locales that one word will remain English even though the labels are translated. If you care about full localisation here, you could introduce anonekey in the locales and uset("none").These are not blockers, just potential future refinements.
Also applies to: 479-499
locales/sv.toml (1)
1-53: Avoid using empty strings as placeholders in Swedish translationsMultiple keys (
assets,downloading_assets,error_message_*,expiration,limit,processing_images,retrieving_asset,retrieving_image_data,storage_capacity, etc.) are currently"". That will render as missing text for Swedish users. Prefer either reusing the default language strings or leaving keys out until translated, so labels never appear blank.internal/cache/cache_test.go (1)
10-76: Cache expiry tests look correct; consider fixing DemoMode assumptionThe structure of
TestMainandTestCacheSetaligns with the Set semantics:
- 0, negative, and sub-default durations expect the cache’s
defaultExpiration.- Larger durations expect
deviceDuration + time.Minute.- Using
GetWithExpirationwith a ±2s tolerance is a reasonable way to assert expiry without flakiness.One thing to watch:
Initialize()chooses the underlying cache’s default expiry differently when DemoMode is enabled. Since the tests assert againstdefaultExpiration, they implicitly assume DemoMode is off. If DemoMode can be toggled via environment or flags, it might be worth explicitly forcing the non-demo configuration inTestMain(or factoring out the cache instance used in tests) so these tests cannot fail due to external configuration.internal/templates/partials/menu.templ (1)
7-8: Menu i18n integration is solid; consider adding a translated aria-labelUsing
t := i18n.T()and wiringtitle/aria-labelattributes through keys likeprevious_asset,play_pause,next_asset,mute_unmute,more_info,redirects_label,redirects,clear_cache, andfullscreenlooks good and matches the i18n approach elsewhere.Since you already have a
navigation_labelkey in the locales, you might optionally add something likearia-label={ t("navigation_label") }on the main<nav>element to give screen readers a translated description of the navigation region.Also applies to: 50-153
locales/es.toml (1)
1-53: Fill or remove empty Spanish translations to avoid missing labelsFor Spanish, several keys (
assets,downloading_assets,error_message_*,expiration,limit,processing_images,retrieving_asset,retrieving_image_data,storage_capacity, etc.) are defined but set to"". That will show up as empty UI text. To keep the interface usable while translations are incomplete, it is usually better to either copy the default-language strings or leave those keys out so you do not render blanks.locales/ko.toml (1)
1-53: Empty translation keys will fall back to English.Several keys have empty values (e.g.,
assets,downloading_assets,error_message_*,limit,processing_images). When these are looked up, thego-i18nlibrary will use the English fallback fromdefaultLocalizer, which is the expected behaviour. If this is intentional for incremental translation, consider adding a comment at the top of the file to indicate which keys are pending translation.internal/i18n/i18n.go (1)
28-43: Consider logging individual locale file load failures without aborting.Currently, if any single locale file fails to load, the entire
Init()returns an error and no translations are available. You may want to log the error and continue loading other locales, ensuring the application remains functional with partial translations.err := fs.WalkDir(LocaleFS, "locales", func(path string, d fs.DirEntry, err error) error { if err != nil { return err } if d.IsDir() { return nil } _, bundleErr := Bundle.LoadMessageFileFS(LocaleFS, path) if bundleErr != nil { - return bundleErr + log.Warn("Failed to load locale file", "path", path, "error", bundleErr) + return nil } return nil })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlgo.sumis excluded by!**/*.sum
📒 Files selected for processing (57)
.zed/settings.json(2 hunks)Dockerfile(2 hunks)biome.json(1 hunks)config.schema.json(2 hunks)frontend/biome.json(1 hunks)frontend/package.json(2 hunks)frontend/src/css/about.css(1 hunks)go.mod(2 hunks)internal/cache/cache.go(2 hunks)internal/cache/cache_test.go(1 hunks)internal/config/config.go(3 hunks)internal/config/config_validation.go(2 hunks)internal/i18n/i18n.go(1 hunks)internal/immich/immich_helpers.go(1 hunks)internal/immich/immich_memories.go(1 hunks)internal/routes/routes_asset.go(2 hunks)internal/routes/routes_asset_helpers.go(2 hunks)internal/routes/routes_offline.go(2 hunks)internal/routes/routes_previous_asset.go(2 hunks)internal/routes/routes_test.go(1 hunks)internal/routes/routes_webhooks.go(2 hunks)internal/templates/partials/clock.templ(2 hunks)internal/templates/partials/error.templ(2 hunks)internal/templates/partials/menu.templ(7 hunks)internal/templates/partials/more_info.templ(17 hunks)internal/templates/partials/sleep.templ(2 hunks)internal/templates/partials/unauthorized.templ(3 hunks)internal/templates/views/view_url_builder.templ(3 hunks)internal/templates/views/views_about.templ(6 hunks)internal/templates/views/views_home.templ(1 hunks)internal/video/video.go(1 hunks)locales/bg.toml(1 hunks)locales/ca.toml(1 hunks)locales/da.toml(1 hunks)locales/de.toml(1 hunks)locales/el.toml(1 hunks)locales/en.toml(1 hunks)locales/es.toml(1 hunks)locales/fi.toml(1 hunks)locales/fr.toml(1 hunks)locales/hu.toml(1 hunks)locales/it.toml(1 hunks)locales/ja.toml(1 hunks)locales/ko.toml(1 hunks)locales/nb.toml(1 hunks)locales/nl.toml(1 hunks)locales/nn.toml(1 hunks)locales/pl.toml(1 hunks)locales/pt.toml(1 hunks)locales/ro.toml(1 hunks)locales/ru.toml(1 hunks)locales/sv.toml(1 hunks)locales/tr.toml(1 hunks)locales/uk.toml(1 hunks)locales/zh.toml(1 hunks)main.go(4 hunks)taskfile.yml(2 hunks)
✅ Files skipped from review due to trivial changes (8)
- locales/zh.toml
- locales/ro.toml
- locales/nl.toml
- locales/en.toml
- locales/el.toml
- locales/de.toml
- locales/nb.toml
- locales/ca.toml
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/config/config.go
- config.schema.json
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: damongolding
Repo: damongolding/immich-kiosk PR: 261
File: internal/immich/immich_album.go:168-178
Timestamp: 2025-01-13T16:11:39.296Z
Learning: The `AssetsOrdered` field in `ImmichAlbum` struct is persisted in cache through JSON serialization, maintaining the album's ordering state between requests.
📚 Learning: 2024-12-20T16:10:12.736Z
Learnt from: tal-sitton
Repo: damongolding/immich-kiosk PR: 231
File: internal/routes/routes_previous_image.go:118-118
Timestamp: 2024-12-20T16:10:12.736Z
Learning: In internal/routes/routes_previous_image.go, the user field can be empty according to the PR author (tal-sitton).
Applied to files:
internal/routes/routes_previous_asset.go
📚 Learning: 2024-12-03T17:30:32.441Z
Learnt from: damongolding
Repo: damongolding/immich-kiosk PR: 207
File: internal/components/image/frame.templ:39-51
Timestamp: 2024-12-03T17:30:32.441Z
Learning: In the Immich Kiosk codebase, functions defined in `internal/components/image/styles.templ` are accessible and can be used within `internal/components/image/frame.templ`.
Applied to files:
internal/routes/routes_previous_asset.gointernal/routes/routes_asset.gointernal/templates/partials/unauthorized.templ
📚 Learning: 2024-11-28T14:38:42.271Z
Learnt from: damongolding
Repo: damongolding/immich-kiosk PR: 204
File: Dockerfile:1-1
Timestamp: 2024-11-28T14:38:42.271Z
Learning: In the `Dockerfile`, Go version `1.23.3` is being used, which is a valid version.
Applied to files:
taskfile.ymlDockerfile
📚 Learning: 2025-06-26T09:58:14.467Z
Learnt from: damongolding
Repo: damongolding/immich-kiosk PR: 462
File: frontend/src/ts/mute.ts:14-18
Timestamp: 2025-06-26T09:58:14.467Z
Learning: In frontend/src/ts/mute.ts, videos are always muted by default at the HTML level. The kioskVideoIsMuted localStorage value represents the user preference, not the actual video mute state. The initial isMuted = true is correct because videos start muted by default, and the user preference is only applied when the user actively toggles mute.
Applied to files:
internal/templates/partials/menu.templ
📚 Learning: 2025-01-09T11:25:13.964Z
Learnt from: damongolding
Repo: damongolding/immich-kiosk PR: 259
File: main.go:60-62
Timestamp: 2025-01-09T11:25:13.964Z
Learning: The SystemLanguage() function in immich-kiosk checks multiple environment variables (LANG, LC_ALL, LC_MESSAGES) in order, normalizes the found locale to language_REGION format (e.g., en_GB), and falls back to "en_GB" if no valid locale is found.
Applied to files:
main.go
📚 Learning: 2024-11-19T11:46:29.380Z
Learnt from: damongolding
Repo: damongolding/immich-kiosk PR: 187
File: webhooks/webhooks.go:53-102
Timestamp: 2024-11-19T11:46:29.380Z
Learning: The `Trigger` function in `webhooks/webhooks.go` is always called as a goroutine, so processing is already concurrent.
Applied to files:
internal/templates/partials/more_info.templ
📚 Learning: 2024-11-20T10:52:53.804Z
Learnt from: damongolding
Repo: damongolding/immich-kiosk PR: 192
File: frontend/src/ts/clock.ts:1-1
Timestamp: 2024-11-20T10:52:53.804Z
Learning: In the `frontend/src/ts` files of the `immich-kiosk` project, importing `{ format }` from `"date-fns"` causes a TypeScript error. The correct import statement is `import { format } from "date-fns/format";`.
Applied to files:
.zed/settings.json
📚 Learning: 2025-08-01T13:20:56.571Z
Learnt from: damongolding
Repo: damongolding/immich-kiosk PR: 499
File: taskfile.yml:22-26
Timestamp: 2025-08-01T13:20:56.571Z
Learning: Go 1.24 introduced the `tool` directive in go.mod that allows declaring development tool dependencies separately from regular module dependencies. Tools declared in the `tool` directive can be run using `go tool <toolname>` command, making `go tool air` valid when `github.com/air-verse/air` is listed in the tool directive.
Applied to files:
go.mod
🧬 Code graph analysis (9)
internal/routes/routes_previous_asset.go (2)
internal/i18n/i18n.go (1)
T(56-74)internal/routes/routes.go (1)
RenderError(108-118)
internal/immich/immich_helpers.go (1)
internal/cache/cache.go (1)
Set(88-100)
internal/routes/routes_webhooks.go (2)
internal/i18n/i18n.go (1)
T(56-74)internal/routes/routes.go (1)
RenderError(108-118)
internal/routes/routes_asset.go (2)
internal/i18n/i18n.go (1)
T(56-74)internal/routes/routes.go (1)
RenderError(108-118)
internal/immich/immich_memories.go (1)
internal/cache/cache.go (1)
Set(88-100)
internal/routes/routes_asset_helpers.go (2)
internal/immich/immich.go (2)
AssetType(22-22)ImageType(33-33)internal/cache/cache.go (1)
Set(88-100)
internal/routes/routes_offline.go (3)
internal/i18n/i18n.go (1)
T(56-74)internal/config/config.go (1)
OfflineMode(70-81)internal/routes/routes.go (1)
Render(132-144)
internal/cache/cache_test.go (1)
internal/cache/cache.go (3)
Initialize(38-45)Flush(49-51)Set(88-100)
main.go (3)
internal/i18n/i18n.go (2)
LocaleFS(14-14)Init(22-53)internal/utils/utils.go (1)
SystemLanguage(714-726)internal/routes/routes_albums.go (1)
Albums(14-78)
🔇 Additional comments (49)
frontend/src/css/about.css (1)
53-59: Verify template consistency with the renamed.about_contentclass.The class selector has been renamed from
.contentto.about_contentfor better specificity. Ensure the corresponding template file (frontend/templates/views/views_about.templ) references the new class name, and confirm no other CSS rules depend on the old selector.frontend/biome.json (1)
3-3: Schema version aligned with biome dependency upgrade.The schema URL update to 2.3.8 aligns with the @biomejs/biome dependency upgrade in frontend/package.json. The configuration itself remains unchanged, which suggests the 2.3.8 schema is backward-compatible with your existing rules and settings.
Verify that the biome schema URL is accessible and matches the installed @biomejs/biome version by checking the official Biome schema documentation.
frontend/package.json (2)
22-23: Biome and autoprefixer dependency upgrades are patch-level; generally safe.Both @biomejs/biome (2.3.4 → 2.3.8) and autoprefixer (^10.4.21 → ^10.4.22) are patch-version bumps, which typically include bug fixes and minor improvements without breaking changes.
Confirm that these dependency versions are stable by checking:
41-41: packageManager updated to pnpm@10.24.0; verify for breaking changes.The package manager field has been bumped from pnpm 10.22.0 to 10.24.0 (minor version). This typically includes feature additions and bug fixes, but you should verify no breaking CLI changes affect your build scripts.
Check the pnpm 10.24.0 changelog to ensure no breaking changes affect your build or installation process (particularly the "build", "js", "css", and "url-builder" scripts defined above).
biome.json (1)
2-2: Root biome schema updated consistently with frontend.The schema URL has been bumped from 2.3.4 to 2.3.8, aligning with the frontend/biome.json update. The formatter settings remain unchanged, indicating backward compatibility with your configuration.
Verify that the root-level @biomejs/biome dependency (if separately specified in a root package.json or lock file) is also version 2.3.8 or compatible with this schema version.
.zed/settings.json (1)
10-10: External formatter command localised to frontend node_modules.Both the JavaScript and TypeScript external formatter commands have been changed from the global "biome" command to the local "./frontend/node_modules/.bin/biome" binary. This ensures the Zed editor uses the exact biome version installed in your frontend dependencies, avoiding version mismatches.
Verify that when Zed is launched from the project root, the relative path "./frontend/node_modules/.bin/biome" is correctly resolved and executable. If Zed is launched from a different directory, the path may not resolve correctly. Consider confirming that:
- Zed is typically launched from the project root in your workflow
- The biome binary exists at this path after running
pnpm installin the frontend directoryAlso applies to: 23-23
internal/video/video.go (2)
67-67: LGTM - Minor log message capitalisation improvement.The capitalisation change aligns with typical logging conventions.
277-292: Verify intentional removal of EXIF orientation application.The call to
utils.ApplyExifOrientation(img, immichAsset.ExifInfo.Orientation)appears to have been removed from this function. Confirm this removal was intentional and verify that video preview images display correctly without explicit EXIF orientation handling when source images contain rotation metadata.taskfile.yml (2)
3-3: LGTM - Version bump for new feature release.
79-79: LGTM - Dockerfile base image pattern updated to bookworm.The sed pattern correctly reflects the change from Alpine to Debian Bookworm base images for the Go builder stage, maintaining consistency with the Dockerfile updates.
go.mod (2)
3-3: Verify Go toolchain version 1.25.5.Same concern as the Dockerfile — Go 1.25.5 appears to be a future version. Please ensure this matches a valid released Go version.
16-22: LGTM - Dependencies aligned with i18n feature.The new direct dependencies (
go-i18n/v2for internationalisation andgo-toml/v2for TOML locale file parsing) are appropriate additions for the localisation feature introduced in this PR. Theklauspost/compressupdate is a routine minor version bump.internal/cache/cache.go (2)
84-100: LGTM - Well-designed cache expiration logic with duration parameter.The implementation correctly:
- Handles negative durations defensively with a warning
- Adds a 1-minute buffer to prevent premature cache eviction
- Falls back to default expiration for short durations (≤5 min)
One consideration: the parameter name
deviceDurationand typeintimplies seconds. Verify that all callers (includingrequestConfig.Durationat line 167) pass values in seconds to maintain consistency.
167-167: VerifyrequestConfig.Durationunit matches expected seconds.The
Setfunction expectsdeviceDurationin seconds (converted totime.Durationinternally). EnsurerequestConfig.Durationis consistently expressed in seconds across all callers.Dockerfile (2)
20-20: > Likely an incorrect or invalid review comment.
38-38: Good practice: Pinning Alpine to a specific version.Pinning to
alpine:3.22.2instead ofalpine:latestimproves reproducibility, and this version tag is available in the Alpine Docker registry.internal/config/config_validation.go (3)
126-138: LGTM - Weather API key loading from docker secret.The implementation correctly iterates through weather locations and only applies the secret-loaded API key to locations that don't already have one configured. This prevents unintended overwrites.
140-167: LGTM - Systemd credentials loading.The early return when
credsDiris empty is appropriate, and the subsequent loading follows the same pattern as the docker secrets. The weather API key is applied consistently to locations missing an API key.
286-288: Breaking change: explicit API key now required for weather locations.The removal of environment-based API key provisioning means existing deployments relying on
KIOSK_DEMO_WEATHER_APIor similar fallbacks will now require explicit per-location API keys or use of docker secrets/systemd credentials. Ensure this is documented in migration notes.internal/immich/immich_helpers.go (1)
88-91: LGTM - Cache TTL now respects device configuration.The change correctly passes
requestConfig.Durationto the updatedcache.Setsignature, enabling per-device cache expiration. The cache layer handles edge cases (negative/small durations) with appropriate fallbacks to default expiration.locales/fi.toml (1)
1-12: Several translation keys are empty strings.Keys such as
assets,downloading_assets,error_message_*, andexpirationare set to empty strings. Verify that the i18n system has fallback behaviour (e.g., to English) so users don't see blank text where these keys are rendered.internal/templates/partials/sleep.templ (1)
3-15: LGTM! Clean i18n integration.The localization implementation is correct and follows the established pattern. The hardcoded string is appropriately replaced with a translated value.
internal/routes/routes_webhooks.go (1)
12-12: LGTM! Consistent i18n integration.The error message localization follows the established pattern across other route handlers and integrates cleanly with the i18n system.
Also applies to: 140-141
internal/routes/routes_previous_asset.go (1)
18-18: LGTM! Consistent i18n integration.The localization of the error message is implemented correctly and maintains consistency with other route handlers in the codebase.
Also applies to: 139-140
internal/templates/partials/clock.templ (1)
7-7: Verify SystemLang locale format and clockDateFormat compatibility with monday.Format.The switch to
monday.Formatfor locale-aware date formatting requires attention to two points:
The
mondaylibrary expects locale constants (likemonday.LocaleEnUS,monday.LocaleDeDE) rather than arbitrary locale strings. Confirm thatc.SystemLangis initialized with the correctmondaylocale constant format, not a raw language code.The
monday.Formatfunction uses standard Go time layouts (the reference format2006-01-02 15:04:05). EnsureclockDateFormatadheres to Go's time layout format, not any custom format string.internal/templates/views/view_url_builder.templ (1)
42-50: LGTM! Translation integration looks correct.The i18n integration properly obtains the translation function and applies it to UI strings. The fallback behaviour and translation key usage align with the i18n framework introduced in this PR.
internal/routes/routes_asset_helpers.go (3)
373-389: LGTM! The blur-skipping logic is well-centralised.The
shouldSkipBlurhelper function cleanly encapsulates the decision logic for when to skip background blur processing. The logic is straightforward and the function is well-documented with clear comments explaining the conditions.
395-399: LGTM! Refactored blur processing uses the new helper.The refactoring correctly applies the
shouldSkipBlurhelper, making the code more maintainable and testable. The early return when blur should be skipped is appropriate.
673-673: LGTM! Cache duration parameter aligns with broader cache changes.The addition of
requestConfig.Durationto thecache.Setcall is consistent with the cache signature update across the codebase, ensuring cached entries use the configured TTL.internal/routes/routes_asset.go (1)
77-78: LGTM! Error message localisation properly implemented.The error rendering now uses the i18n translation function to display a localised message. The implementation is consistent with the i18n framework introduced across the codebase.
internal/immich/immich_memories.go (1)
119-119: LGTM! Cache expiration now uses configured duration.The addition of
a.requestConfig.Durationparameter ensures the cached memories response uses the proper TTL, consistent with the cache signature update across the codebase.internal/templates/partials/unauthorized.templ (1)
45-66: LGTM! Unauthorised page properly localised.The i18n integration correctly translates both the page title and heading text. The implementation follows the same pattern used across other templates in this PR.
locales/da.toml (1)
1-53: Verify the intentionality of empty translation values.The Danish locale file contains numerous keys with empty string values (e.g., "assets", "downloading_assets", "error_message_assets", "error_message_filters", "limit", "processing_images", "storage_capacity"). Whilst the i18n framework will fall back to the default locale or key name, this may result in inconsistent user experience for Danish-speaking users.
Please confirm whether the empty values are intentional (e.g., work in progress) or if they should be populated with Danish translations.
internal/routes/routes_test.go (1)
159-225: LGTM! Comprehensive test coverage for blur-skipping logic.The table-driven test provides excellent coverage of the
shouldSkipBlurfunction across various configuration combinations. The test cases cover edge cases and expected behaviours, and the assertions align with the function's implementation logic.internal/templates/partials/error.templ (1)
3-38: I18n wiring in error template looks correctUsing
t := i18n.T()and replacing the bullet items and “Full error” label witht("error_message_*")integrates cleanly with the existing template and i18n helper; no functional regressions spotted.main.go (2)
44-53: LGTM: i18n embedding and initialization wiring.The locale files are correctly embedded and assigned to the i18n package during init(). This ensures the filesystem is available before
i18n.Init()is called inmain().
195-196: LGTM: New API albums endpoint.The route is correctly placed before the
/:redirectcatch-all route, ensuring proper matching.internal/templates/views/views_about.templ (3)
21-34: LGTM: Language attribute handling with fallback.The logic correctly extracts the language code from the locale (e.g., "en" from "en_GB") and falls back to "en" when unavailable.
52-53: LGTM: Translation function initialisation.Initialising
t := i18n.T()at template render time ensures the correct localiser is used.
191-191: A TTL of0means items never expire—cached GitHub release data will not refresh.According to the ristretto library, a TTL value of
0incache.Setdoes not use a default TTL; instead, it means the cached item will never expire. This means the GitHub release data cached at line 191 will remain in cache indefinitely and never be refreshed, which is likely not the intended behavior.Consider using an explicit TTL duration (e.g.,
time.Hour,time.Minute) to ensure cached release data is periodically refreshed.internal/templates/partials/more_info.templ (7)
28-29: LGTM: Translation function initialisation pattern.Initialising
t := i18n.T()at the start of each template/function is the correct pattern for templ, ensuring the translation function is available within the template scope.
56-74: LGTM: Localised button labels and UI strings.The button group correctly uses translation keys for all user-facing text, including the Immich link and webhook trigger buttons.
93-146: LGTM: LikeButton localisation.All interactive labels (
unlike_asset,like_asset,liked,like) are properly translated with the correct conditional logic preserved.
155-191: LGTM: HideButton localisation.All labels (
hide_asset,unhide_asset,hide) are correctly translated.
196-218: LGTM: Camera metadata localisation.Fallback strings for unknown camera make and stats are properly localised.
307-324: LGTM: People section heading localisation.The "People" heading is correctly localised using
t("people").
350-368: LGTM: Tags section heading localisation.The "Tags" heading is correctly localised using
t("tags").locales/fr.toml (2)
1-1: Verify empty translation values; clarify whether they are intentional.Eight translation keys have empty string values:
assets,downloading_assets,expiration,limit,processing_images,retrieving_asset,retrieving_image_data, andstorage_capacity. Confirm whether these are intentional placeholders, incomplete translations, or by design (e.g., for conditional UI rendering). Check if the corresponding keys in en.toml contain values or are also empty to determine if these represent incomplete translations.
1-53: Verify French translations align with EN locale and are idiomatic.While the French translations in the visible snippet appear grammatically sound, complete verification requires:
- Confirming all keys from
en.tomlare present infr.toml(no missing keys)- Validating translations accurately reflect intent of English counterparts
- Ensuring Immich kiosk-specific terminology (e.g., "webhook", "album") uses appropriate localisation
| func T() func(string) string { | ||
| return func(key string) string { | ||
| translated, err := localizer.Localize(&i18n.LocalizeConfig{ | ||
| MessageID: key, | ||
| }) | ||
| if err == nil { | ||
| return translated | ||
| } | ||
|
|
||
| defaultTranslated, err := defaultLocalizer.Localize(&i18n.LocalizeConfig{ | ||
| MessageID: key, | ||
| }) | ||
| if err != nil { | ||
| log.Error("failed to translate", "error", err) | ||
| return key // fallback | ||
| } | ||
| return defaultTranslated | ||
| } |
There was a problem hiding this comment.
Nil pointer dereference risk if Init() fails or is not called.
If Init() fails or is never called, both localizer and defaultLocalizer will be nil, causing a panic at line 58 when localizer.Localize() is invoked. Add a nil guard to prevent runtime panics.
func T() func(string) string {
return func(key string) string {
+ if localizer == nil {
+ if defaultLocalizer == nil {
+ return key
+ }
+ translated, err := defaultLocalizer.Localize(&i18n.LocalizeConfig{
+ MessageID: key,
+ })
+ if err != nil {
+ return key
+ }
+ return translated
+ }
+
translated, err := localizer.Localize(&i18n.LocalizeConfig{
MessageID: key,
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func T() func(string) string { | |
| return func(key string) string { | |
| translated, err := localizer.Localize(&i18n.LocalizeConfig{ | |
| MessageID: key, | |
| }) | |
| if err == nil { | |
| return translated | |
| } | |
| defaultTranslated, err := defaultLocalizer.Localize(&i18n.LocalizeConfig{ | |
| MessageID: key, | |
| }) | |
| if err != nil { | |
| log.Error("failed to translate", "error", err) | |
| return key // fallback | |
| } | |
| return defaultTranslated | |
| } | |
| func T() func(string) string { | |
| return func(key string) string { | |
| if localizer == nil { | |
| if defaultLocalizer == nil { | |
| return key | |
| } | |
| translated, err := defaultLocalizer.Localize(&i18n.LocalizeConfig{ | |
| MessageID: key, | |
| }) | |
| if err != nil { | |
| return key | |
| } | |
| return translated | |
| } | |
| translated, err := localizer.Localize(&i18n.LocalizeConfig{ | |
| MessageID: key, | |
| }) | |
| if err == nil { | |
| return translated | |
| } | |
| defaultTranslated, err := defaultLocalizer.Localize(&i18n.LocalizeConfig{ | |
| MessageID: key, | |
| }) | |
| if err != nil { | |
| log.Error("failed to translate", "error", err) | |
| return key // fallback | |
| } | |
| return defaultTranslated | |
| } |
🤖 Prompt for AI Agents
In internal/i18n/i18n.go around lines 56 to 73, T() calls methods on
package-level localizer and defaultLocalizer without checking for nil, which can
panic if Init() failed or wasn't called; add nil guards so you never call
Localize on a nil pointer: check if localizer is nil before using it and if so
skip to trying defaultLocalizer (or return the key immediately), log a clear
error when both are nil, and similarly guard defaultLocalizer before calling
Localize so the function safely falls back to returning the original key when
translators are unavailable.
| retrieving_asset = "" | ||
| retrieving_image_data = "" |
There was a problem hiding this comment.
Provide French translations for retrieving_asset and retrieving_image_data.
These keys are critical for user feedback during asset retrieval operations and should not remain empty. Based on context and the PR objectives (API album display), users need to see localised status messages.
Suggested French translations (if currently intentionally empty):
retrieving_asset: "Récupération de l'élément" or "Chargement de l'élément"retrieving_image_data: "Récupération des données d'image" or "Chargement des données d'image"
🤖 Prompt for AI Agents
In locales/fr.toml around lines 36 to 37, the keys retrieving_asset and
retrieving_image_data are empty; add appropriate French translations for each
key (for example use "Récupération de l'élément" or "Chargement de l'élément"
for retrieving_asset and "Récupération des données d'image" or "Chargement des
données d'image" for retrieving_image_data) so the status messages display
localized text to users during asset retrieval.
| assets = "" | ||
| basic_options = "Visualizza opzioni di base" | ||
| clear_cache = "Cancella cache del server" | ||
| configuration = "Configurazione" | ||
| current_version = "Versione attuale" | ||
| downloading_assets = "" | ||
| error_message_assets = "" | ||
| error_message_filters = "" | ||
| error_message_full_error = "" | ||
| error_message_immich = "" | ||
| error_message_sources = "" | ||
| expiration = "" |
There was a problem hiding this comment.
Several translation keys are empty.
Same as the Norwegian Nynorsk locale — multiple keys have empty string values. Consider completing these translations or ensuring the i18n system has proper fallback behaviour.
🤖 Prompt for AI Agents
In locales/it.toml around lines 1-12 several keys are empty (assets,
downloading_assets, error_message_assets, error_message_filters,
error_message_full_error, error_message_immich, error_message_sources,
expiration); fill each with the correct Italian translation (or copy the English
default if unsure) so the UI shows meaningful text, or alternatively ensure the
i18n system is configured to fall back to the default locale for these keys;
update the file with the completed strings or adjust the i18n fallback settings
accordingly.
| assets = "" | ||
| basic_options = "Vis grunnleggjande val" | ||
| clear_cache = "Tøm tenarbuffer" | ||
| configuration = "Konfigurasjon" | ||
| current_version = "Gjeldande versjon" | ||
| downloading_assets = "" | ||
| error_message_assets = "" | ||
| error_message_filters = "" | ||
| error_message_full_error = "" | ||
| error_message_immich = "" | ||
| error_message_sources = "" | ||
| expiration = "" |
There was a problem hiding this comment.
Several translation keys are empty.
Multiple keys have empty string values (assets, downloading_assets, error_message_*, expiration, etc.). Please verify that the i18n framework falls back to the default locale (English) gracefully when translations are missing, or populate these strings to ensure a complete Norwegian Nynorsk experience.
🤖 Prompt for AI Agents
In locales/nn.toml around lines 1-12, several translation keys (assets,
downloading_assets, error_message_assets, error_message_filters,
error_message_full_error, error_message_immich, error_message_sources,
expiration, etc.) are empty; either supply appropriate Nynorsk translations for
each empty value or update the i18n usage to explicitly fall back to the default
English strings when nn translations are missing (e.g., ensure the i18n config
enables fallbackLocale and that lookup code calls t(key, { fallback: true }) or
similar); implement one of these fixes and run the app to verify missing keys
show the English text or the newly provided Nynorsk strings.
| assets = "" | ||
| basic_options = "Просмотреть основные параметры" | ||
| clear_cache = "Очистить кеш сервера" | ||
| configuration = "Конфигурация" | ||
| current_version = "Текущая версия" | ||
| downloading_assets = "" | ||
| error_message_assets = "" | ||
| error_message_filters = "" | ||
| error_message_full_error = "" | ||
| error_message_immich = "" | ||
| error_message_sources = "" | ||
| expiration = "" |
There was a problem hiding this comment.
Same empty string pattern as other locale files.
The same keys (assets, downloading_assets, error_message_*, etc.) are empty. Ensure i18n fallback is in place across all locales.
🤖 Prompt for AI Agents
In locales/ru.toml around lines 1 to 12, several keys (assets,
downloading_assets, error_message_*, expiration, etc.) are empty which breaks
translations; either provide Russian translations for these keys in this file or
ensure the app's i18n config uses a reliable fallback (e.g., default locale) so
missing keys resolve to the fallback. Update ru.toml with the correct translated
strings where available; if translations are intentionally omitted,
enable/verify the i18n fallback behavior in the app configuration so these keys
return the default locale values instead of empty strings.
| lang := utils.SystemLanguage() | ||
| systemLang := monday.Locale(lang) | ||
| baseConfig.SystemLang = systemLang | ||
| log.Info("System language", "lang", systemLang) | ||
|
|
||
| i18nErr := i18n.Init(lang) | ||
| if i18nErr != nil { | ||
| log.Error("Failed to initialize i18n", "err", i18nErr) | ||
| } |
There was a problem hiding this comment.
Consider whether to continue execution after i18n initialisation failure.
If i18n.Init fails, the application continues but localizer in the i18n package will be nil, causing a panic when T() is called from templates. You may want to either:
- Make this a fatal error, or
- Ensure
T()handles a nil localizer gracefully (see review comment oninternal/i18n/i18n.go).
Currently, the error is only logged but execution proceeds.
damongolding
added
Icebox
3 participants
First of all, it's important to say that the code here is AI-generated code because I was looking for a way to implement the feature quickly and because the kiosk only runs locally, so security concerns are not critical for me.
What PR does this add?
When I wanted to display specific albums I had to add them via config.yaml in the less convenient way of finding the album ID and then manually adding it to the config.yaml file:
To solve this problem I decided to use Immich API to make the process significantly simpler.
Using the API method, the code retrieves a list of all albums that the user has access to and allows a display with the option to select multiple albums through the UI
This saves the whole process of manually adding and finding the album ID from the Immich UI
In addition, I implemented a password feature that will protect against unauthorized access
Before requesting to retrieve albums from the API occurs, it requires a password that is defined via config.yaml and only after entering the correct password is a request sent to the API
api_albums_password: "" # password to protect access to API albums selectionIn addition, when the
immich_users_api_keys:option is defined in config.yaml:
Then the UI displays a category for each user separately with the albums retrieved via that user's API key
There are also buttons:
Sort by name
Sort by number of assets
Select/deselect all
Retrieve albums via the API (AKA refresh)
And a search box (I implemented a mechanism that prevents (from the shortcut keys to operate when the search box is in use)
Summary by CodeRabbit
New Features
UI Improvements
Configuration
✏️ Tip: You can customize this high-level summary in your review settings.