Repository navigation
Conversation
Skips assets that are also in any album not listed under Albums, including albums shared with the user. The Immich mobile app's album sync only ever adds assets to albums, so a photo moved between device albums stays in both server albums. Today the only fix is listing every other album in ExcludedAlbums and keeping that list current as new albums appear. The other albums are resolved when the excluded-album cache is filled, so new albums are picked up on the next refresh. The setting has no effect without Albums, where every album would count as "other". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (13)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughAdds ChangesHide assets in other albums
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant AccountSettings
participant AlbumAssetsPool
participant AssetHelper
participant ImmichApi
AccountSettings->>AlbumAssetsPool: Enable HideAssetsInOtherAlbums
AlbumAssetsPool->>AssetHelper: Resolve excluded album assets
AssetHelper->>ImmichApi: GetAllAlbumsAsync
ImmichApi-->>AssetHelper: Return selected and unselected albums
AssetHelper-->>AlbumAssetsPool: Return distinct exclusion IDs
AlbumAssetsPool-->>AccountSettings: Return filtered assets
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The new setting is represented across configuration and UI surfaces and is guarded when no albums are selected; no actionable merge risk is established. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 7 files. (6 skipped: 6 unsupported.)
✨ 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 |
|
I should also mention: Related: This works alongside #706 (the Testing: 4 new unit tests. On Linux CI in my fork, Core tests passed 88/88 and WebApi tests passed 26/26. In a benchmark with 8,000 selected assets and 30 other albums, one request took 81 s with this PR alone and 18 to 39 ms with #706 as well. |
|
Hey, I reworked the whole Search to the new v2 Search. (#712) Immich now handles most of the work with the albums, including a 'Not In Album'-Filter which should fix the issue you had and would make this PR obsolete. Sorry that I never replied to your PR, I missed it. You can check #712 out and see if that works for you :) Thank you for your time. |
2 participants
Adds a checkbox option to skip assets that are also in any album not listed under Albums, including albums shared with the user.
This is needed because sometimes photos can be put into multiple albums by accident when immich syncs them, and I don't want photos in my "documents" folder to be shown on my immichframe.
The Immich mobile app's album sync only ever adds assets to albums, so a photo moved between device albums stays in both server albums. Today the only fix is listing every other album in ExcludedAlbums and keeping that list current as new albums appear.
The other albums are resolved when the excluded-album cache is filled, so new albums are picked up on the next refresh. The setting has no effect without Albums, where every album would count as "other".
Summary by CodeRabbit
New Features
Documentation