From 8cfcf073877b9dc09cac1b1792294914c96865d5 Mon Sep 17 00:00:00 2001 From: Anders Olsson Date: Tue, 2 Jun 2026 23:40:10 +0200 Subject: [PATCH] fix(db): publish gate fires only on transition into public, not re-set Preserves the documented set-to-current idempotent no-op: re-setting an already-public object's visibility no longer rejects when a required field was introduced after publish. Adds a regression test. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/db/src/catalog.rs | 7 +++--- crates/db/tests/visibility.rs | 47 +++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/crates/db/src/catalog.rs b/crates/db/src/catalog.rs index c9dc3ce..39739c7 100644 --- a/crates/db/src/catalog.rs +++ b/crates/db/src/catalog.rs @@ -370,10 +370,11 @@ pub async fn set_visibility( let new_visibility = object.visibility.transition_to(target)?; - // The publish gate: a record may only become public once every required field + // The publish gate: a record may only *become* public once every required field // has a value. The typed inventory-minimum columns are already NOT NULL, so only - // the flexible required fields need checking here. - if new_visibility == Visibility::Public { + // the flexible required fields need checking here. Gated on an actual transition + // into public so a set-to-current no-op stays a no-op (never a late rejection). + if new_visibility == Visibility::Public && object.visibility != Visibility::Public { let missing = missing_required_fields(&mut *conn, &object.fields).await?; if !missing.is_empty() { diff --git a/crates/db/tests/visibility.rs b/crates/db/tests/visibility.rs index f709f28..bd775d6 100644 --- a/crates/db/tests/visibility.rs +++ b/crates/db/tests/visibility.rs @@ -256,3 +256,50 @@ async fn publishing_requires_all_required_fields_present(pool: PgPool) { let published = catalog::object_by_id(db.pool(), id).await.unwrap().unwrap(); assert_eq!(published.visibility, Visibility::Public); } + +#[sqlx::test] +async fn republishing_a_public_object_is_a_noop_even_with_a_new_required_field(pool: PgPool) { + use db::fields; + use domain::{FieldType, LocalizedLabel, NewFieldDefinition}; + + let db = Db::from_pool(pool); + + let mut tx = db.pool().begin().await.unwrap(); + + // an already-public object (created public directly at the db layer) + let id = catalog::create_object( + &mut tx, + AuditActor::System, + &object("LM-2", Visibility::Public), + ) + .await + .unwrap(); + + // a required field is introduced AFTER the object is already public + fields::create_field_definition( + &mut tx, + &NewFieldDefinition { + key: "inscription".into(), + field_type: FieldType::Text, + required: true, + group_key: None, + labels: vec![LocalizedLabel { + lang: "en".into(), + label: "Inscription".into(), + }], + }, + ) + .await + .unwrap(); + + // setting visibility to its current value stays an idempotent no-op — the publish + // gate only fires on an actual transition into public, not on a re-set. + catalog::set_visibility(&mut tx, AuditActor::System, id, Visibility::Public) + .await + .unwrap(); + + tx.commit().await.unwrap(); + + let obj = catalog::object_by_id(db.pool(), id).await.unwrap().unwrap(); + assert_eq!(obj.visibility, Visibility::Public); +}