refactor(db): share update path so set_visibility avoids a redundant fetch; tie public-visibility const to the enum; test internal exclusion

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
2026-06-02 13:35:36 +02:00
parent 14cdd2a04a
commit b948cae269
3 changed files with 55 additions and 21 deletions
+39 -20
View File
@@ -14,7 +14,7 @@ use crate::{audit, authority, fields, vocab};
const ENTITY_TYPE: &str = "object"; const ENTITY_TYPE: &str = "object";
/// The visibility value eligible for the public surface. /// The visibility value eligible for the public surface.
const PUBLIC_VISIBILITY: &str = "public"; const PUBLIC_VISIBILITY: &str = Visibility::Public.as_str();
const OBJECT_COLUMNS: &str = "id, object_number, object_name, number_of_objects, \ const OBJECT_COLUMNS: &str = "id, object_number, object_name, number_of_objects, \
brief_description, current_location, current_owner, recorder, recording_date, \ brief_description, current_location, current_owner, recorder, recording_date, \
@@ -117,6 +117,9 @@ where
} }
/// List **public** objects ordered by object number, with `limit`/`offset` paging. /// List **public** objects ordered by object number, with `limit`/`offset` paging.
///
/// `limit` and `offset` must be non-negative (Postgres rejects a negative `LIMIT`);
/// the public API layer clamps them before calling.
pub async fn list_public_objects<'e, E>( pub async fn list_public_objects<'e, E>(
executor: E, executor: E,
limit: i64, limit: i64,
@@ -249,10 +252,25 @@ pub async fn update_object(
return Ok(false); return Ok(false);
}; };
let changes = update_changes(&old.to_input(), input); apply_object_update(&mut *conn, actor, id, &old.to_input(), input).await?;
Ok(true)
}
/// Diff `old`→`new`, write the changed columns + an `updated` audit entry, both on
/// `conn`. A no-op (no field changed) touches neither the row's `updated_at` nor the
/// audit log.
async fn apply_object_update(
conn: &mut sqlx::PgConnection,
actor: AuditActor,
id: ObjectId,
old: &ObjectInput,
new: &ObjectInput,
) -> Result<(), sqlx::Error> {
let changes = update_changes(old, new);
if changes.is_empty() { if changes.is_empty() {
// No-op: don't touch updated_at or the audit log. return Ok(());
return Ok(true);
} }
sqlx::query( sqlx::query(
@@ -263,15 +281,15 @@ pub async fn update_object(
WHERE id = $1", WHERE id = $1",
) )
.bind(id.to_uuid()) .bind(id.to_uuid())
.bind(&input.object_number) .bind(&new.object_number)
.bind(&input.object_name) .bind(&new.object_name)
.bind(input.number_of_objects) .bind(new.number_of_objects)
.bind(input.brief_description.as_deref()) .bind(new.brief_description.as_deref())
.bind(input.current_location.as_deref()) .bind(new.current_location.as_deref())
.bind(input.current_owner.as_deref()) .bind(new.current_owner.as_deref())
.bind(input.recorder.as_deref()) .bind(new.recorder.as_deref())
.bind(input.recording_date) .bind(new.recording_date)
.bind(input.visibility.as_str()) .bind(new.visibility.as_str())
.execute(&mut *conn) .execute(&mut *conn)
.await?; .await?;
@@ -287,7 +305,7 @@ pub async fn update_object(
) )
.await?; .await?;
Ok(true) Ok(())
} }
/// Why changing an object's visibility failed. /// Why changing an object's visibility failed.
@@ -302,9 +320,9 @@ pub enum VisibilityError {
} }
/// Move an object to `target` visibility, enforcing the stepwise state machine, and /// Move an object to `target` visibility, enforcing the stepwise state machine, and
/// audit the change. Reuses [`update_object`]'s diff/audit path, so only `visibility` /// audit the change. Uses the same diff/audit path as `update_object`, so only
/// appears in the audit entry — and setting to the current value is an idempotent no-op /// `visibility` appears in the audit entry — and setting to the current value is an
/// (no row touch, no audit). Pass a transaction connection. /// idempotent no-op (no row touch, no audit). Pass a transaction connection.
pub async fn set_visibility( pub async fn set_visibility(
conn: &mut sqlx::PgConnection, conn: &mut sqlx::PgConnection,
actor: AuditActor, actor: AuditActor,
@@ -317,10 +335,11 @@ pub async fn set_visibility(
let new_visibility = object.visibility.transition_to(target)?; let new_visibility = object.visibility.transition_to(target)?;
let mut input = object.to_input(); let old_input = object.to_input();
let mut new_input = old_input.clone();
input.visibility = new_visibility; new_input.visibility = new_visibility;
update_object(&mut *conn, actor, id, &input).await?; apply_object_update(&mut *conn, actor, id, &old_input, &new_input).await?;
Ok(()) Ok(())
} }
+15
View File
@@ -140,6 +140,13 @@ async fn public_reads_return_only_public_records(pool: PgPool) {
) )
.await .await
.unwrap(); .unwrap();
let internal = catalog::create_object(
&mut tx,
AuditActor::System,
&object("I-1", Visibility::Internal),
)
.await
.unwrap();
tx.commit().await.unwrap(); tx.commit().await.unwrap();
assert!( assert!(
@@ -168,4 +175,12 @@ async fn public_reads_return_only_public_records(pool: PgPool) {
.unwrap() .unwrap()
.is_empty() .is_empty()
); );
// internal records are excluded from public reads too (not just draft)
assert!(
catalog::public_object_by_id(db.pool(), internal)
.await
.unwrap()
.is_none()
);
} }
+1 -1
View File
@@ -17,7 +17,7 @@ pub enum Visibility {
} }
impl Visibility { impl Visibility {
pub fn as_str(&self) -> &'static str { pub const fn as_str(&self) -> &'static str {
match self { match self {
Visibility::Draft => "draft", Visibility::Draft => "draft",
Visibility::Internal => "internal", Visibility::Internal => "internal",