From 616a6f05c642c7c5b7e964d01b6d12ecc5d9bc70 Mon Sep 17 00:00:00 2001 From: Anders Olsson Date: Tue, 2 Jun 2026 09:29:40 +0200 Subject: [PATCH] refactor(db): DRY object SELECT columns, consistent date json; test date + all-none round-trip Co-Authored-By: Claude Sonnet 4.6 --- crates/db/Cargo.toml | 1 + crates/db/src/catalog.rs | 29 +++++++++++---------------- crates/db/tests/catalog.rs | 41 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 17 deletions(-) diff --git a/crates/db/Cargo.toml b/crates/db/Cargo.toml index d6ca912..b263b5a 100644 --- a/crates/db/Cargo.toml +++ b/crates/db/Cargo.toml @@ -14,3 +14,4 @@ serde_json.workspace = true [dev-dependencies] tokio.workspace = true +time.workspace = true diff --git a/crates/db/src/catalog.rs b/crates/db/src/catalog.rs index 910b707..d324657 100644 --- a/crates/db/src/catalog.rs +++ b/crates/db/src/catalog.rs @@ -13,13 +13,9 @@ use crate::audit; /// The entity_type recorded in the audit log for catalogue objects. const ENTITY_TYPE: &str = "object"; -const SELECT_OBJECT_BY_ID: &str = "SELECT id, object_number, object_name, number_of_objects, brief_description, \ - current_location, current_owner, recorder, recording_date, visibility, \ - created_at, updated_at FROM object WHERE id = $1"; - -const SELECT_OBJECTS_ORDERED: &str = "SELECT id, object_number, object_name, number_of_objects, brief_description, \ - current_location, current_owner, recorder, recording_date, visibility, \ - created_at, updated_at FROM object ORDER BY object_number"; +const OBJECT_COLUMNS: &str = "id, object_number, object_name, number_of_objects, \ + brief_description, current_location, current_owner, recorder, recording_date, \ + visibility, created_at, updated_at"; /// Create an object and record a `created` audit entry, both on `conn` /// (pass a transaction connection `&mut *tx` so they commit atomically). @@ -74,7 +70,9 @@ pub async fn object_by_id<'e, E>( where E: sqlx::PgExecutor<'e>, { - let row = sqlx::query(SELECT_OBJECT_BY_ID) + let sql = format!("SELECT {OBJECT_COLUMNS} FROM object WHERE id = $1"); + + let row = sqlx::query(&sql) .bind(id.to_uuid()) .fetch_optional(executor) .await?; @@ -88,9 +86,9 @@ where E: sqlx::PgExecutor<'e>, { // TODO: add LIMIT/keyset pagination before exposing this via the API. - let rows = sqlx::query(SELECT_OBJECTS_ORDERED) - .fetch_all(executor) - .await?; + let sql = format!("SELECT {OBJECT_COLUMNS} FROM object ORDER BY object_number"); + + let rows = sqlx::query(&sql).fetch_all(executor).await?; rows.into_iter().map(map_object).collect() } @@ -137,17 +135,14 @@ fn field_values(input: &ObjectInput) -> Vec<(&'static str, Option)> { input.current_owner.as_ref().map(|v| json!(v)), ), ("recorder", input.recorder.as_ref().map(|v| json!(v))), - ( - "recording_date", - input - .recording_date - .and_then(|d| serde_json::to_value(d).ok()), - ), + ("recording_date", input.recording_date.map(|d| json!(d))), ("visibility", Some(json!(input.visibility.as_str()))), ] } /// Audit changes for a newly created object: every set field as an `after` value. +/// Unset (`None`) optional fields are omitted — absence is conveyed by their not +/// appearing, consistent with `FieldChange`'s `None`-means-no-value convention. fn creation_changes(input: &ObjectInput) -> Vec { field_values(input) .into_iter() diff --git a/crates/db/tests/catalog.rs b/crates/db/tests/catalog.rs index 9406e19..27718d2 100644 --- a/crates/db/tests/catalog.rs +++ b/crates/db/tests/catalog.rs @@ -75,3 +75,44 @@ async fn object_by_id_missing_is_none(pool: PgPool) { .is_none() ); } + +#[sqlx::test] +async fn object_with_date_and_all_none_optionals_round_trips(pool: PgPool) { + let db = Db::from_pool(pool); + let date = time::Date::from_calendar_date(2020, time::Month::January, 28).unwrap(); + let input = ObjectInput { + object_number: "LM-3".into(), + object_name: "drawing".into(), + number_of_objects: 1, + brief_description: None, + current_location: None, + current_owner: None, + recorder: None, + recording_date: Some(date), + visibility: Visibility::Internal, + }; + + let mut tx = db.pool().begin().await.unwrap(); + let id = catalog::create_object(&mut tx, AuditActor::System, &input) + .await + .unwrap(); + tx.commit().await.unwrap(); + + let obj = catalog::object_by_id(db.pool(), id).await.unwrap().unwrap(); + assert_eq!(obj.recording_date, Some(date)); + assert_eq!(obj.brief_description, None); + assert_eq!(obj.current_location, None); + assert_eq!(obj.current_owner, None); + assert_eq!(obj.recorder, None); + assert_eq!(obj.visibility, Visibility::Internal); + + let history = audit::history_for(db.pool(), "object", id.to_uuid()) + .await + .unwrap(); + assert!( + history[0] + .changes + .iter() + .any(|c| c.field == "recording_date") + ); +}