diff --git a/AGENTS.md b/AGENTS.md index 2f5673a9..2954d707 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -43,6 +43,8 @@ When the user says "commit and close issue", include `(fixes #N)` in the commit Filter rules must **not** apply to collection/folder container queries — only to content items. See `get_by_filter` in the db layer. +The one deliberate exception is `CollectionId`: a user policy can use it to hide specific collections from that user's browse views (userviews, boxset listings). It matches the *collection row itself*, never its member content, so a hidden collection's items stay visible everywhere else they'd otherwise appear (other collections, search, general browsing) — this is enforced with a Rust-side post-query filter in `get_by_filter_inner` (`collection_visibility_filters`), not a SQL clause threaded through the container-query path. Do not use `CollectionMember`-style "content that belongs to collection X" filtering for this: it operates on `media_relations`, which only exists for manually-curated collections — smart collections have no stored membership, so it silently filters nothing for them. + ## API conventions - API handler paths must always be lowercase (e.g. `#[get("/useritems/{id}")]`, not `/UserItems/{Id}`). diff --git a/crates/remux-dashboard/src/components/filters.rs b/crates/remux-dashboard/src/components/filters.rs index ffb2a114..e5cd5afa 100644 --- a/crates/remux-dashboard/src/components/filters.rs +++ b/crates/remux-dashboard/src/components/filters.rs @@ -163,7 +163,7 @@ async fn fetch_suggestions( } results } - "collection_id" => { + "collection_id" | "collection_member" => { let q_lower = query.to_lowercase(); match client .execute(remux_sdks::remux::GetItems( @@ -233,6 +233,7 @@ fn field_label(key: &str) -> &'static str { "person" => "Person", "catalog" => "Catalog", "collection_id" => "Collection", + "collection_member" => "Collection", "favorite" => "Favorite", "played" => "Played", "media_kind" => "Media Kind", @@ -451,14 +452,21 @@ fn raw_to_rule(field: &str, op: &str, value_str: &str) -> FilterRule { "catalog" => FilterRule::Catalog { op: set_op, catalog_ids: value_str - .split(", ") + .split(',') .filter_map(|s| Uuid::parse_str(s.trim()).ok()) .collect(), }, "collection_id" => FilterRule::CollectionId { op: set_op, ids: value_str - .split(", ") + .split(',') + .filter_map(|s| Uuid::parse_str(s.trim()).ok()) + .collect(), + }, + "collection_member" => FilterRule::CollectionMember { + op: set_op, + collection_ids: value_str + .split(',') .filter_map(|s| Uuid::parse_str(s.trim()).ok()) .collect(), }, @@ -752,8 +760,12 @@ pub fn FilterRuleRow( #[props(default)] allowed_fields: Vec<&'static str>, ) -> Element { let app_state = use_context::(); + let (field_val, op_val, value_val) = rule_to_raw(&rule); + let is_collection_id = + field_val == "collection_id" || field_val == "collection_member"; let client_for_ratings = app_state.clone(); let client_for_catalogs = app_state.clone(); + let client_for_collections = app_state.clone(); let mut parental_ratings: Signal> = use_signal(Vec::new); use_effect(move || { let client = client_for_ratings.clone(); @@ -801,12 +813,62 @@ pub fn FilterRuleRow( }); }); - let (field_val, op_val, value_val) = rule_to_raw(&rule); + // 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. Only fetched for + // rows that actually need it — otherwise every rule row in the editor + // would fire this same request redundantly. + let mut collection_options: Signal> = use_signal(Vec::new); + use_effect(move || { + if !is_collection_id { + return; + } + let client = client_for_collections.clone(); + spawn(async move { + let Ok(r) = client + .execute(remux_sdks::remux::GetItems( + remux_sdks::remux::GetItemsQuery { + include_item_types: Some(vec![ + remux_sdks::remux::MediaType::BoxSet, + ]), + include_childless: Some(true), + ..Default::default() + }, + )) + .await + else { + return; + }; + let options = r + .items + .into_iter() + // Group containers ("collection of collections") aren't + // selectable collection targets — same exclusion as the + // search picker in fetch_suggestions. + .filter(|item| { + item.collection_type + .as_ref() + != Some(&remux_sdks::remux::CollectionType::Boxsets) + }) + .filter_map(|item| { + item.name + .map(|n| { + ( + n, + item.id + .to_string(), + ) + }) + }) + .collect(); + collection_options.set(options); + }); + }); + let ops = ops_for_field(&field_val); let is_trailer = field_val == "has_trailer"; let is_parental_rating = field_val == "parental_rating"; let is_catalog = field_val == "catalog"; - let is_collection_id = field_val == "collection_id"; let is_favorite = field_val == "favorite"; let is_watched = field_val == "played"; let is_media_kind = field_val == "media_kind"; @@ -893,6 +955,12 @@ pub fn FilterRuleRow( if show_field("original_language") { option { value: "original_language", selected: field_val == "original_language", { field_label("original_language") } } } if show_field("person") { option { value: "person", selected: field_val == "person", { field_label("person") } } } if show_field("catalog") { option { value: "catalog", selected: field_val == "catalog", { field_label("catalog") } } } + // "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 + // stay visible everywhere else they'd otherwise appear. if show_field("collection_id") { option { value: "collection_id", selected: field_val == "collection_id", { field_label("collection_id") } } } if show_field("favorite") { option { value: "favorite", selected: field_val == "favorite", { field_label("favorite") } } } if show_field("played") { option { value: "played", selected: field_val == "played", { field_label("played") } } } @@ -982,11 +1050,12 @@ pub fn FilterRuleRow( } } else if is_collection_id { ChipInput { - field_key: "collection_id".to_string(), + field_key: field_val.clone(), op_val: op_val.clone(), values: rule_values(&rule), idx, rules, + value_labels: Some(collection_options), } } else if is_favorite { select { diff --git a/crates/remux-server/src/db/media.rs b/crates/remux-server/src/db/media.rs index 419e3876..a66ab66f 100644 --- a/crates/remux-server/src/db/media.rs +++ b/crates/remux-server/src/db/media.rs @@ -4354,6 +4354,38 @@ impl Media { ); } } + + // Hide specific collections from browse views per the user's + // policy — the one deliberate exception to "policy filters don't + // apply to containers" (see AGENTS.md). Applied here (in SQL, + // gated the opposite way — only when container_only) rather than + // as a post-query Rust filter, so it composes correctly with + // LIMIT/OFFSET and count_qb's total stays accurate. + if container_only { + if let Some(ref pf) = filter.policy_filter { + let (deny, allow) = collection_visibility_filters(pf); + if let Some(allow) = &allow { + if allow.is_empty() { + qb.push(" AND 0"); + } else { + qb.push(" AND media.id IN ("); + let mut sep = qb.separated(", "); + for id in allow { + sep.push_bind(*id); + } + qb.push(")"); + } + } + if !deny.is_empty() { + qb.push(" AND media.id NOT IN ("); + let mut sep = qb.separated(", "); + for id in &deny { + sep.push_bind(*id); + } + qb.push(")"); + } + } + } } // Close the filtered CTE and build the UNION ALL structure. @@ -5414,7 +5446,10 @@ impl Media { // Drop empty containers when requested. child_count is already populated // for all container kinds (including smart/catalog) by the branches above. // A structural collection-of-collections is visible only when it has a - // non-empty descendant collection. + // non-empty descendant collection. Collections hidden by policy (see + // `collection_visibility_filters`) were already excluded in SQL above, + // so a group container whose only child is a hidden collection + // correctly evaluates as empty here too. let sql_total = count?; if filter.exclude_childless { @@ -7420,6 +7455,49 @@ pub fn push_release_date_filter( } } +/// Collects `CollectionId` rules from a policy filter into (deny, allow) id +/// sets, ignoring the filter's AND/OR group structure — this is a deliberate, +/// narrow exception to "policy filters don't apply to container queries" +/// (see AGENTS.md): it hides the *collection row itself* from browse views +/// (userviews, boxset listings), never its member content, which stays +/// visible everywhere else it would otherwise appear. Applied by +/// `get_by_filter_inner` as a SQL `WHERE` condition (gated on `container_only`, +/// the opposite of the general policy-filter gate) inside the `count_qb` / +/// `records_qb` loop, so it composes correctly with LIMIT/OFFSET and +/// `count_qb`'s total — a post-query Rust-side retain would silently produce +/// short pages and an inaccurate total once a query is paginated. +fn collection_visibility_filters( + pf: &remux_sdks::remux::CollectionFilter, +) -> (HashSet, Option>) { + use remux_sdks::remux::{FilterRule, SetOp}; + + let mut deny = HashSet::new(); + let mut allow: Option> = None; + for group in &pf.groups { + for rule in &group.rules { + if let FilterRule::CollectionId { op, ids } = rule { + if ids.is_empty() { + continue; + } + if matches!(op, SetOp::IsNot | SetOp::NotIn) { + deny.extend( + ids.iter() + .copied(), + ); + } else { + allow + .get_or_insert_with(HashSet::new) + .extend( + ids.iter() + .copied(), + ); + } + } + } + } + (deny, allow) +} + /// Append WHERE clauses for a set of `FilterRule`s onto a query builder. /// /// Called once for both the count and records builders inside `get_by_filter`. @@ -7833,19 +7911,12 @@ fn filter_rule_to_sql( let sql = "media.parent_id IS NOT NULL".to_string(); Some((sql, !value)) } - R::CollectionMember { op, collection_ids } if !collection_ids.is_empty() => { - let in_clause = collection_ids - .iter() - .map(|id| format!("X'{}'", id.simple())) - .collect::>() - .join(", "); - let sql = format!( - "media.id IN (SELECT mr.right_media_id FROM media_relations mr \ - WHERE mr.role = 'collection' AND mr.left_media_id IN ({in_clause}))" - ); - let negated = matches!(op, SetOp::IsNot | SetOp::NotIn); - Some((sql, negated)) - } + // Disabled: only ever matches manual collections (media_relations has + // no rows for smart collections — their contents are computed from + // their own filter at query time, never materialized), so this + // silently filtered nothing for anyone using it on a smart collection. + // No-op regardless of content so any rule already saved in a user's + // policy stops being applied rather than half-working. R::CollectionMember { .. } => None, R::CollectionId { op, ids } if !ids.is_empty() => { let in_clause = ids