fix: user policy collection filter (hide collections from browse views) - #424
Merged
Conversation
CollectionMember only ever matched manual collections — media_relations has no rows for smart collections, since their contents are computed from their own filter at query time and never materialized. Any user policy using this on a smart collection silently filtered nothing. Removes it from the filter editor UI and makes it a hard no-op server-side, so any rule already saved in a policy stops being applied too. CollectionId (used by the smart group-container builder, which works correctly) is untouched.
Re-adds the collection filter, but backed by CollectionId (collection's own id) instead of CollectionMember (content membership) — the latter only ever worked for manual collections, since smart collections have no stored membership to check. CollectionId now has a second, deliberate application: a Rust-side post-query filter in get_by_filter_inner hides the matched collection rows from container/userview queries specifically, never touching content queries, so a hidden collection's items stay visible everywhere else they'd normally appear. Documented as an explicit exception in AGENTS.md's policy filter rule.
Existing collection_id/collection_member chips showed a raw UUID after reopening the editor, since only their search-driven label_cache had names and it starts empty. Eagerly fetch every collection's name (same pattern already used for catalog_options) so saved chips render their name immediately.
lostb1t
changed the title
There was a problem hiding this comment.
🟡 Changes recommended
Server-side post-query filtering can break paginated browse semantics (LIMIT/OFFSET + TotalRecordCount), and the dashboard changes introduce correctness/performance issues that should be addressed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adjusts how user policy filters interact with collection browse views by introducing a deliberate CollectionId exception so specific collections can be hidden from user-facing container listings without affecting the visibility of the collections’ member items elsewhere (per Jellyfin compatibility expectations and the documented policy-filter rules).
Changes:
- Applies a Rust-side post-query filter to hide
MediaKind::Collectionrows based onCollectionIdrules from the user policy filter. - Disables
CollectionMemberSQL filtering server-side (to avoid misleading/partial behavior for smart collections) and updates the dashboard to better handle collection-related rule UX. - Documents the
CollectionIdexception inAGENTS.md.
File summaries
| File | Description |
|---|---|
| crates/remux-server/src/db/media.rs | Adds policy-driven collection visibility filtering and introduces collection_visibility_filters; disables CollectionMember SQL translation. |
| crates/remux-dashboard/src/components/filters.rs | Extends collection rule suggestion/label handling and adds eager collection label loading for chip rendering. |
| AGENTS.md | Documents the CollectionId exception and clarifies why CollectionMember-style filtering is not appropriate for hiding collections. |
Review details
Suppressed comments (2)
crates/remux-dashboard/src/components/filters.rs:838
collection_optionseagerly loads all BoxSets but does not exclude group containers (collection_type == Boxsets), even thoughfetch_suggestionsexplicitly filters them out. This can cause chips to resolve labels for (and potentially allow selecting) the structural "collection of collections" containers as if they were normal collections.
let options = r
.items
.into_iter()
.filter_map(|item| {
item.name
crates/remux-dashboard/src/components/filters.rs:949
- If an existing saved rule has
field_val == "collection_member", the field<select>has no corresponding<option>value, which can lead to the UI displaying an incorrect selection (and potentially overwriting the rule on change). Ifcollection_memberis meant to be retained for backwards compatibility, it should be representable in the dropdown (ideally marked deprecated/disabled).
// "collection_id" matches a *collection's own id* — used both to pick
// which collections belong in a group container, and (server-side, as
// the one deliberate exception to "policy filters don't touch container
// queries") to hide specific collections from a user's browse views.
// It never filters content items, so a hidden collection's members
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+462
to
+466
| @@ -462,6 +463,13 @@ fn raw_to_rule(field: &str, op: &str, value_str: &str) -> FilterRule { | |||
| .filter_map(|s| Uuid::parse_str(s.trim()).ok()) | |||
| .collect(), | |||
| }, | |||
| "collection_member" => FilterRule::CollectionMember { | |||
Comment on lines
+813
to
+817
| // Eagerly fetch every collection's name so chips for an already-saved | ||
| // rule (a raw UUID with no label yet in the search-driven label_cache) | ||
| // render as a name immediately instead of a bare UUID. | ||
| let mut collection_options: Signal<Vec<(String, String)>> = use_signal(Vec::new); | ||
| use_effect(move || { |
Comment on lines
+5424
to
+5428
| let mut collections_were_filtered = false; | ||
| if let Some(ref pf) = filter.policy_filter { | ||
| let (deny, allow) = collection_visibility_filters(pf); | ||
| if !deny.is_empty() || allow.is_some() { | ||
| let before = records.len(); |
- Move collection exclusion from a post-query Rust retain into the SQL
WHERE clause (inside the shared count_qb/records_qb loop), gated on
container_only. The retain approach ran after LIMIT/OFFSET, producing
short pages and an inaccurate total_count once paginated.
- Fix UUID split parsing for catalog/collection_id/collection_member
(split(', ') required an exact comma-space separator; split(',') with
per-item trim matches every other set field).
- Only fetch collection names (for chip labels) on rows that are
actually collection fields, and exclude group containers from that
fetch — matching the search picker's own exclusion.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.