diff --git a/crates/api/src/admin_objects.rs b/crates/api/src/admin_objects.rs index 6664dbf..0d665f8 100644 --- a/crates/api/src/admin_objects.rs +++ b/crates/api/src/admin_objects.rs @@ -17,7 +17,7 @@ use domain::{ use serde::{Deserialize, Serialize}; use utoipa::ToSchema; -use crate::{AppState, admin_vocab::LabelInput, pagination::Pagination, reindex}; +use crate::{AppState, admin_vocab::LabelInput, reindex}; /// A localized label `{ lang, label }` (shared across admin views). #[derive(Serialize, ToSchema)] @@ -100,12 +100,73 @@ pub(crate) fn parse_date(s: &str) -> Result { time::Date::parse(s, &fmt).map_err(|_| StatusCode::UNPROCESSABLE_ENTITY) } +/// Query parameters for the object list: pagination plus whitelisted sort/order and +/// optional visibility/quick-filter. All values are validated/clamped server-side; the +/// `sort` token maps onto an enum (never a raw column name) before reaching SQL. +#[derive(Deserialize)] +pub(crate) struct ObjectListParams { + pub limit: Option, + pub offset: Option, + pub sort: Option, + pub order: Option, + pub visibility: Option, + pub q: Option, +} + +impl ObjectListParams { + fn limit(&self) -> i64 { + self.limit + .unwrap_or(crate::pagination::DEFAULT_LIMIT) + .clamp(1, crate::pagination::MAX_LIMIT) + } + + fn offset(&self) -> i64 { + self.offset.unwrap_or(0).max(0) + } + + fn sort(&self) -> db::catalog::ObjectSort { + use db::catalog::ObjectSort; + + match self.sort.as_deref() { + Some("object_name") => ObjectSort::ObjectName, + Some("updated_at") => ObjectSort::UpdatedAt, + Some("created_at") => ObjectSort::CreatedAt, + Some("visibility") => ObjectSort::Visibility, + // Unknown or absent → stable default. + _ => ObjectSort::ObjectNumber, + } + } + + fn descending(&self) -> bool { + self.order.as_deref() == Some("desc") + } + + /// Validate `visibility` against the domain enum; an unknown value is ignored + /// (treated as no filter) so hand-edited URLs degrade gracefully instead of 500ing. + fn visibility(&self) -> Option<&str> { + self.visibility + .as_deref() + .filter(|v| Visibility::from_db(v).is_some()) + } + + fn q(&self) -> Option<&str> { + self.q.as_deref().map(str::trim).filter(|s| !s.is_empty()) + } +} + /// List objects (paginated, all visibility levels). Requires `ViewInternal`. #[utoipa::path( get, path = "/api/admin/objects", params( ("limit" = Option, Query, description = "1..=200, default 50"), - ("offset" = Option, Query, description = "default 0") + ("offset" = Option, Query, description = "default 0"), + ("sort" = Option, Query, + description = "object_number | object_name | updated_at | created_at | visibility (default object_number)"), + ("order" = Option, Query, description = "asc | desc (default asc)"), + ("visibility" = Option, Query, + description = "draft | internal | public — filter; unknown values ignored"), + ("q" = Option, Query, + description = "quick filter: ILIKE match on object_number or object_name") ), responses( (status = 200, body = AdminObjectPage), @@ -116,15 +177,22 @@ pub(crate) fn parse_date(s: &str) -> Result { pub(crate) async fn list_objects( _auth: Authorized, State(state): State, - Query(page): Query, + Query(params): Query, ) -> Result, StatusCode> { - let (limit, offset) = (page.limit(), page.offset()); + let (limit, offset) = (params.limit(), params.offset()); - let objects = db::catalog::list_objects_paged(state.db.pool(), limit, offset) + let query = db::catalog::ObjectQuery { + sort: params.sort(), + descending: params.descending(), + visibility: params.visibility(), + q: params.q(), + }; + + let objects = db::catalog::list_objects_query(state.db.pool(), &query, limit, offset) .await .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; - let total = db::catalog::count_objects(state.db.pool()) + let total = db::catalog::count_objects_query(state.db.pool(), query.visibility, query.q) .await .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; diff --git a/crates/api/tests/admin_catalog.rs b/crates/api/tests/admin_catalog.rs index 0c998ae..dc0e589 100644 --- a/crates/api/tests/admin_catalog.rs +++ b/crates/api/tests/admin_catalog.rs @@ -877,6 +877,77 @@ async fn listed_object_carries_timestamps(pool: PgPool) { assert!(!updated_at.is_empty(), "updated_at must be non-empty"); } +#[sqlx::test(migrations = "../db/migrations")] +async fn list_objects_sort_filter_quick_search(pool: PgPool) { + migrate_sessions(&db::Db::from_pool(pool.clone())) + .await + .unwrap(); + + seed_user(&pool, "ed@example.com", "pw-editor-123", Role::Editor).await; + + let app = build_app(state(pool)); + let cookie = login(&app, "ed@example.com", "pw-editor-123").await; + + let create = |number: &str, name: &str| { + format!( + r#"{{"object_number":"{number}","object_name":"{name}","number_of_objects":1,"visibility":"draft"}}"# + ) + }; + + for (number, name) in [ + ("FOO-1", "foo apple"), + ("FOO-2", "foo banana"), + ("BAR-1", "bar cherry"), + ] { + let resp = send( + &app, + &cookie, + "POST", + "/api/admin/objects", + Some(&create(number, name)), + ) + .await; + assert_eq!(resp.status(), StatusCode::CREATED); + } + + // No params → default order is object_number ascending. + let default = send(&app, &cookie, "GET", "/api/admin/objects", None).await; + let body: serde_json::Value = + serde_json::from_slice(&default.into_body().collect().await.unwrap().to_bytes()).unwrap(); + let numbers: Vec<&str> = body["items"] + .as_array() + .unwrap() + .iter() + .map(|i| i["object_number"].as_str().unwrap()) + .collect(); + assert_eq!(numbers, ["BAR-1", "FOO-1", "FOO-2"]); + assert_eq!(body["total"], 3); + + // sort=object_name&order=desc&visibility=draft&q=foo + let filtered = send( + &app, + &cookie, + "GET", + "/api/admin/objects?sort=object_name&order=desc&visibility=draft&q=foo", + None, + ) + .await; + assert_eq!(filtered.status(), StatusCode::OK); + + let body: serde_json::Value = + serde_json::from_slice(&filtered.into_body().collect().await.unwrap().to_bytes()).unwrap(); + + let names: Vec<&str> = body["items"] + .as_array() + .unwrap() + .iter() + .map(|i| i["object_name"].as_str().unwrap()) + .collect(); + // Only the two "foo …" objects, name descending. + assert_eq!(names, ["foo banana", "foo apple"]); + assert_eq!(body["total"], 2); +} + #[sqlx::test(migrations = "../db/migrations")] async fn field_definition_edit_delete_requires_auth(pool: PgPool) { migrate_sessions(&db::Db::from_pool(pool.clone())) diff --git a/crates/db/src/catalog.rs b/crates/db/src/catalog.rs index 39739c7..1217a37 100644 --- a/crates/db/src/catalog.rs +++ b/crates/db/src/catalog.rs @@ -96,37 +96,121 @@ where rows.into_iter().map(map_object).collect() } -/// List objects (all visibility levels) ordered by object number, with paging. -pub async fn list_objects_paged<'e, E>( - executor: E, +/// Whitelisted, injection-safe sort columns for the object list. The client never +/// supplies a column name directly — the API layer maps an opaque token onto a variant, +/// and only [`ObjectSort::column`] (returning a `'static str`) reaches the SQL string. +#[derive(Debug, Clone, Copy)] +pub enum ObjectSort { + ObjectNumber, + ObjectName, + UpdatedAt, + CreatedAt, + Visibility, +} + +impl ObjectSort { + fn column(self) -> &'static str { + match self { + ObjectSort::ObjectNumber => "object_number", + ObjectSort::ObjectName => "object_name", + ObjectSort::UpdatedAt => "updated_at", + ObjectSort::CreatedAt => "created_at", + ObjectSort::Visibility => "visibility", + } + } +} + +/// Filters + ordering for a paged object query. `visibility`/`q` are optional; +/// both are bound as parameters, never interpolated into the SQL string. +pub struct ObjectQuery<'a> { + pub sort: ObjectSort, + pub descending: bool, + pub visibility: Option<&'a str>, + pub q: Option<&'a str>, +} + +/// Build the optional `WHERE` clause and its ordered bind values from the filters. +/// Each clause references a positional placeholder (`$1`, `$2`, …) matching the order +/// the returned `binds` are applied; the client's strings only ever arrive as binds. +fn where_clause(visibility: Option<&str>, q: Option<&str>) -> (String, Vec) { + let mut clauses = Vec::new(); + let mut binds = Vec::new(); + + if let Some(v) = visibility { + binds.push(v.to_owned()); + + clauses.push(format!("visibility = ${}", binds.len())); + } + + if let Some(term) = q { + binds.push(format!("%{term}%")); + + let p = binds.len(); + + clauses.push(format!( + "(object_number ILIKE ${p} OR object_name ILIKE ${p})" + )); + } + + let sql = if clauses.is_empty() { + String::new() + } else { + format!(" WHERE {}", clauses.join(" AND ")) + }; + + (sql, binds) +} + +/// List objects (all visibility levels) with whitelisted sort, optional visibility/quick +/// filters, and paging. Ordering uses [`ObjectSort::column`] (a `'static str`) plus a +/// stable secondary key, so no client-controlled string ever reaches the SQL text. +pub async fn list_objects_query( + pool: &sqlx::PgPool, + query: &ObjectQuery<'_>, limit: i64, offset: i64, -) -> Result, sqlx::Error> -where - E: sqlx::PgExecutor<'e>, -{ - let sql = - format!("SELECT {OBJECT_COLUMNS} FROM object ORDER BY object_number LIMIT $1 OFFSET $2"); +) -> Result, sqlx::Error> { + let (where_sql, binds) = where_clause(query.visibility, query.q); - let rows = sqlx::query(&sql) - .bind(limit) - .bind(offset) - .fetch_all(executor) - .await?; + let dir = if query.descending { "DESC" } else { "ASC" }; + + // Secondary key keeps ordering stable when the primary sort has ties. + let sql = format!( + "SELECT {OBJECT_COLUMNS} FROM object{where_sql} \ + ORDER BY {} {dir}, object_number ASC LIMIT ${} OFFSET ${}", + query.sort.column(), + binds.len() + 1, + binds.len() + 2, + ); + + let mut sql_query = sqlx::query(&sql); + + for bind in &binds { + sql_query = sql_query.bind(bind); + } + + let rows = sql_query.bind(limit).bind(offset).fetch_all(pool).await?; rows.into_iter().map(map_object).collect() } -/// Count all objects (for pagination totals). -pub async fn count_objects<'e, E>(executor: E) -> Result -where - E: sqlx::PgExecutor<'e>, -{ - let row = sqlx::query("SELECT count(*) AS n FROM object") - .fetch_one(executor) - .await?; +/// Count objects matching the optional visibility/quick filters (for pagination totals). +pub async fn count_objects_query( + pool: &sqlx::PgPool, + visibility: Option<&str>, + q: Option<&str>, +) -> Result { + let (where_sql, binds) = where_clause(visibility, q); - row.try_get("n") + let sql = format!("SELECT count(*) AS n FROM object{where_sql}"); + + let mut sql_query = sqlx::query(&sql); + + for bind in &binds { + sql_query = sql_query.bind(bind); + } + + sql_query.fetch_one(pool).await?.try_get("n") } /// Fetch one **public** object by id. Returns `None` if the object is missing **or** diff --git a/crates/db/tests/catalog.rs b/crates/db/tests/catalog.rs index 9cf2ece..6d60f9b 100644 --- a/crates/db/tests/catalog.rs +++ b/crates/db/tests/catalog.rs @@ -65,6 +65,142 @@ async fn list_returns_created_objects(pool: PgPool) { assert_eq!(all[1].object_number, "LM-2"); } +fn input(number: &str, name: &str, visibility: Visibility) -> ObjectInput { + ObjectInput { + object_number: number.into(), + object_name: name.into(), + number_of_objects: 1, + brief_description: None, + current_location: None, + current_owner: None, + recorder: None, + recording_date: None, + visibility, + } +} + +async fn seed(pool: &PgPool, inputs: &[ObjectInput]) { + let db = Db::from_pool(pool.clone()); + let mut tx = db.pool().begin().await.unwrap(); + + for it in inputs { + catalog::create_object(&mut tx, AuditActor::System, it) + .await + .unwrap(); + } + + tx.commit().await.unwrap(); +} + +#[sqlx::test] +async fn query_orders_by_name_descending(pool: PgPool) { + let db = Db::from_pool(pool.clone()); + + seed( + &pool, + &[ + input("LM-1", "alpha", Visibility::Draft), + input("LM-2", "gamma", Visibility::Draft), + input("LM-3", "beta", Visibility::Draft), + ], + ) + .await; + + let query = catalog::ObjectQuery { + sort: catalog::ObjectSort::ObjectName, + descending: true, + visibility: None, + q: None, + }; + + let rows = catalog::list_objects_query(db.pool(), &query, 50, 0) + .await + .unwrap(); + + let names: Vec<&str> = rows.iter().map(|o| o.object_name.as_str()).collect(); + assert_eq!(names, ["gamma", "beta", "alpha"]); +} + +#[sqlx::test] +async fn query_filters_by_visibility(pool: PgPool) { + let db = Db::from_pool(pool.clone()); + + seed( + &pool, + &[ + input("LM-1", "draft one", Visibility::Draft), + input("LM-2", "internal one", Visibility::Internal), + input("LM-3", "draft two", Visibility::Draft), + ], + ) + .await; + + let query = catalog::ObjectQuery { + sort: catalog::ObjectSort::ObjectNumber, + descending: false, + visibility: Some("draft"), + q: None, + }; + + let rows = catalog::list_objects_query(db.pool(), &query, 50, 0) + .await + .unwrap(); + + assert_eq!(rows.len(), 2); + assert!(rows.iter().all(|o| o.visibility == Visibility::Draft)); + + let total = catalog::count_objects_query(db.pool(), Some("draft"), None) + .await + .unwrap(); + assert_eq!(total, 2); +} + +#[sqlx::test] +async fn query_quick_filter_matches_number_or_name(pool: PgPool) { + let db = Db::from_pool(pool.clone()); + + seed( + &pool, + &[ + input("RED-1", "scarlet vase", Visibility::Draft), + input("BLU-1", "azure bowl", Visibility::Draft), + input("LM-9", "red kettle", Visibility::Internal), + ], + ) + .await; + + // Matches the object_number of the first row. + let by_number = catalog::ObjectQuery { + sort: catalog::ObjectSort::ObjectNumber, + descending: false, + visibility: None, + q: Some("red"), + }; + let rows = catalog::list_objects_query(db.pool(), &by_number, 50, 0) + .await + .unwrap(); + // ILIKE: "RED-1" by number and "red kettle" by name. + assert_eq!(rows.len(), 2); + + let total = catalog::count_objects_query(db.pool(), None, Some("red")) + .await + .unwrap(); + assert_eq!(total, 2); + + // A term matching only a name. + let by_name = catalog::ObjectQuery { + sort: catalog::ObjectSort::ObjectNumber, + descending: false, + visibility: None, + q: Some("azure"), + }; + let rows = catalog::list_objects_query(db.pool(), &by_name, 50, 0) + .await + .unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].object_number, "BLU-1"); +} + #[sqlx::test] async fn object_by_id_missing_is_none(pool: PgPool) { let db = Db::from_pool(pool);