feat: edit/delete field definitions — audited, blocked when in use (#36)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

crates/api/src/admin_objects.rs

@@ -6,7 +6,7 @@ use axum::{
     Json, Router,
     extract::{Path, Query, State},
     http::StatusCode,
-    response::IntoResponse,
+    response::{IntoResponse, Response},
     routing::{get, put},
 };
 use domain::{
@@ -510,6 +510,133 @@ pub(crate) async fn create_field_definition(
     }
 }
 
+/// Fields that may be changed on an existing field definition. `key`, `data_type`, and
+/// binding are immutable and intentionally absent from this request.
+#[derive(Deserialize, ToSchema)]
+pub(crate) struct UpdateFieldDefinitionRequest {
+    pub required: bool,
+    pub group: Option<String>,
+    pub labels: Vec<LabelInput>,
+}
+
+/// Update a field definition's mutable attributes (labels, group, required).
+/// `key`, `data_type`, and binding are immutable. Requires `EditCatalogue`.
+#[utoipa::path(
+    patch, path = "/api/admin/field-definitions/{key}",
+    request_body = UpdateFieldDefinitionRequest,
+    params(("key" = String, Path, description = "Field definition key")),
+    responses(
+        (status = 204),
+        (status = 401),
+        (status = 403),
+        (status = 404),
+        (status = 422, description = "CHECK constraint violated (e.g. empty label)")
+    )
+)]
+pub(crate) async fn update_field_definition(
+    auth: Authorized<EditCatalogue>,
+    State(state): State<AppState>,
+    Path(key): Path<String>,
+    Json(req): Json<UpdateFieldDefinitionRequest>,
+) -> Result<StatusCode, StatusCode> {
+    let labels: Vec<LocalizedLabel> = req
+        .labels
+        .into_iter()
+        .map(|l| LocalizedLabel {
+            lang: l.lang,
+            label: l.label,
+        })
+        .collect();
+
+    let mut tx = state
+        .db
+        .pool()
+        .begin()
+        .await
+        .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
+
+    let result = db::fields::update_field_definition(
+        &mut tx,
+        actor(&auth.user),
+        &key,
+        req.required,
+        req.group.as_deref(),
+        &labels,
+    )
+    .await;
+
+    match result {
+        Ok(true) => {
+            tx.commit()
+                .await
+                .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
+
+            Ok(StatusCode::NO_CONTENT)
+        }
+        Ok(false) => {
+            let _ = tx.rollback().await;
+
+            Err(StatusCode::NOT_FOUND)
+        }
+        Err(err) => {
+            let _ = tx.rollback().await;
+
+            match err.as_database_error().and_then(|e| e.code()).as_deref() {
+                Some("23514") => Err(StatusCode::UNPROCESSABLE_ENTITY),
+                _ => Err(StatusCode::INTERNAL_SERVER_ERROR),
+            }
+        }
+    }
+}
+
+/// Delete a field definition. Blocked (409) when catalogue objects store a value under
+/// this key. Requires `EditCatalogue`.
+#[utoipa::path(
+    delete, path = "/api/admin/field-definitions/{key}",
+    params(("key" = String, Path, description = "Field definition key")),
+    responses(
+        (status = 204),
+        (status = 401),
+        (status = 403),
+        (status = 404),
+        (status = 409, body = crate::admin_vocab::InUseView,
+         description = "Field is used by catalogue objects")
+    )
+)]
+pub(crate) async fn delete_field_definition(
+    auth: Authorized<EditCatalogue>,
+    State(state): State<AppState>,
+    Path(key): Path<String>,
+) -> Response {
+    use crate::admin_vocab::InUseView;
+
+    let Ok(mut tx) = state.db.pool().begin().await else {
+        return StatusCode::INTERNAL_SERVER_ERROR.into_response();
+    };
+
+    match db::fields::delete_field_definition(&mut tx, actor(&auth.user), &key).await {
+        Ok(db::DeleteOutcome::Deleted) => match tx.commit().await {
+            Ok(()) => StatusCode::NO_CONTENT.into_response(),
+            Err(_) => StatusCode::INTERNAL_SERVER_ERROR.into_response(),
+        },
+        Ok(db::DeleteOutcome::InUse { count }) => {
+            let _ = tx.rollback().await;
+
+            (StatusCode::CONFLICT, Json(InUseView { count })).into_response()
+        }
+        Ok(db::DeleteOutcome::NotFound) => {
+            let _ = tx.rollback().await;
+
+            StatusCode::NOT_FOUND.into_response()
+        }
+        Err(_) => {
+            let _ = tx.rollback().await;
+
+            StatusCode::INTERNAL_SERVER_ERROR.into_response()
+        }
+    }
+}
+
 /// Field-level rejection detail for `set_fields`, so the UI can highlight the field.
 #[derive(Serialize, ToSchema)]
 pub(crate) struct FieldErrorView {
@@ -609,4 +736,8 @@ pub(crate) fn routes() -> Router<AppState> {
             "/api/admin/field-definitions",
             get(list_field_definitions).post(create_field_definition),
         )
+        .route(
+            "/api/admin/field-definitions/{key}",
+            axum::routing::patch(update_field_definition).delete(delete_field_definition),
+        )
 }

crates/api/src/openapi.rs

@@ -26,6 +26,8 @@ use crate::{
         admin_objects::delete_object,
         admin_objects::list_field_definitions,
         admin_objects::create_field_definition,
+        admin_objects::update_field_definition,
+        admin_objects::delete_field_definition,
         admin_objects::set_fields,
         admin_vocab::list_vocabularies,
         admin_vocab::create_vocabulary,
@@ -58,6 +60,7 @@ use crate::{
         admin_objects::CreatedObject,
         admin_objects::FieldDefinitionView,
         admin_objects::NewFieldDefinitionRequest,
+        admin_objects::UpdateFieldDefinitionRequest,
         admin_objects::CreatedField,
         admin_objects::FieldErrorView,
         admin_vocab::VocabularyView,

crates/api/tests/admin_catalog.rs

@@ -714,3 +714,167 @@ async fn edit_and_delete_authority(pool: PgPool) {
     .await;
     assert_eq!(deleted.status(), StatusCode::NO_CONTENT);
 }
+
+#[sqlx::test(migrations = "../db/migrations")]
+async fn edit_and_delete_field_definition(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;
+
+    // create a field definition
+    send(
+        &app,
+        &cookie,
+        "POST",
+        "/api/admin/field-definitions",
+        Some(r#"{"key":"weight","data_type":"integer","vocabulary_id":null,"authority_kind":null,"required":false,"group":null,"labels":[{"lang":"sv","label":"Vikt"}]}"#),
+    )
+    .await;
+
+    // PATCH: update required + group + labels
+    let patched = send(
+        &app,
+        &cookie,
+        "PATCH",
+        "/api/admin/field-definitions/weight",
+        Some(r#"{"required":true,"group":"Mått","labels":[{"lang":"sv","label":"Vikt (g)"}]}"#),
+    )
+    .await;
+    assert_eq!(patched.status(), StatusCode::NO_CONTENT);
+
+    // PATCH unknown key → 404
+    let missing = send(
+        &app,
+        &cookie,
+        "PATCH",
+        "/api/admin/field-definitions/nope",
+        Some(r#"{"required":false,"group":null,"labels":[]}"#),
+    )
+    .await;
+    assert_eq!(missing.status(), StatusCode::NOT_FOUND);
+
+    // DELETE the (unreferenced) field definition
+    let deleted = send(
+        &app,
+        &cookie,
+        "DELETE",
+        "/api/admin/field-definitions/weight",
+        None,
+    )
+    .await;
+    assert_eq!(deleted.status(), StatusCode::NO_CONTENT);
+
+    // DELETE again → 404
+    let again = send(
+        &app,
+        &cookie,
+        "DELETE",
+        "/api/admin/field-definitions/weight",
+        None,
+    )
+    .await;
+    assert_eq!(again.status(), StatusCode::NOT_FOUND);
+}
+
+#[sqlx::test(migrations = "../db/migrations")]
+async fn delete_field_definition_referenced_is_409(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;
+
+    // create a field definition
+    send(
+        &app,
+        &cookie,
+        "POST",
+        "/api/admin/field-definitions",
+        Some(r#"{"key":"weight","data_type":"integer","vocabulary_id":null,"authority_kind":null,"required":false,"group":null,"labels":[{"lang":"sv","label":"Vikt"}]}"#),
+    )
+    .await;
+
+    // create an object and set the field
+    let obj = send(
+        &app,
+        &cookie,
+        "POST",
+        "/api/admin/objects",
+        Some(r#"{"object_number":"T-2","object_name":"test","number_of_objects":1,"visibility":"draft"}"#),
+    )
+    .await;
+    assert_eq!(obj.status(), StatusCode::CREATED);
+
+    let obj_json: serde_json::Value =
+        serde_json::from_slice(&obj.into_body().collect().await.unwrap().to_bytes()).unwrap();
+    let obj_id = obj_json["id"].as_str().unwrap().to_owned();
+
+    let set = send(
+        &app,
+        &cookie,
+        "PUT",
+        &format!("/api/admin/objects/{obj_id}/fields"),
+        Some(r#"{"weight":42}"#),
+    )
+    .await;
+    assert_eq!(set.status(), StatusCode::NO_CONTENT);
+
+    // delete the field definition — must be blocked
+    let blocked = send(
+        &app,
+        &cookie,
+        "DELETE",
+        "/api/admin/field-definitions/weight",
+        None,
+    )
+    .await;
+    assert_eq!(blocked.status(), StatusCode::CONFLICT);
+
+    let body: serde_json::Value =
+        serde_json::from_slice(&blocked.into_body().collect().await.unwrap().to_bytes()).unwrap();
+    assert_eq!(body["count"], 1);
+}
+
+#[sqlx::test(migrations = "../db/migrations")]
+async fn field_definition_edit_delete_requires_auth(pool: PgPool) {
+    migrate_sessions(&db::Db::from_pool(pool.clone()))
+        .await
+        .unwrap();
+
+    let app = build_app(state(pool));
+
+    let patch_resp = app
+        .clone()
+        .oneshot(
+            Request::builder()
+                .method("PATCH")
+                .uri("/api/admin/field-definitions/weight")
+                .header(header::CONTENT_TYPE, "application/json")
+                .body(Body::from(r#"{"required":false,"group":null,"labels":[]}"#))
+                .unwrap(),
+        )
+        .await
+        .unwrap();
+    assert_eq!(patch_resp.status(), StatusCode::UNAUTHORIZED);
+
+    let delete_resp = app
+        .clone()
+        .oneshot(
+            Request::builder()
+                .method("DELETE")
+                .uri("/api/admin/field-definitions/weight")
+                .body(Body::empty())
+                .unwrap(),
+        )
+        .await
+        .unwrap();
+    assert_eq!(delete_resp.status(), StatusCode::UNAUTHORIZED);
+}

crates/db/src/fields.rs

@@ -1,11 +1,15 @@
 //! Registry of flexible field definitions.
 
 use domain::{
-    AuthorityKind, FieldDefinition, FieldDefinitionId, FieldType, LocalizedLabel,
+    AuditAction, AuditActor, AuthorityKind, FieldDefinition, FieldDefinitionId, FieldType,
-    NewFieldDefinition, VocabularyId,
+    LocalizedLabel, NewAuditEvent, NewFieldDefinition, VocabularyId,
 };
 use sqlx::Row;
 
+use crate::audit;
+
+const FIELD_DEFINITION_ENTITY_TYPE: &str = "field_definition";
+
 /// Labels aggregated per row as JSON, to read a definition and its labels in one query.
 const LABELS_JSON: &str = "COALESCE(json_agg(json_build_object('lang', fdl.lang, 'label', fdl.label) \
      ORDER BY fdl.lang) FILTER (WHERE fdl.field_definition_id IS NOT NULL), '[]'::json)";
@@ -121,3 +125,115 @@ fn map_field_definition(row: sqlx::postgres::PgRow) -> Result<FieldDefinition, s
         labels: labels.0,
     })
 }
+
+/// Update a field definition's mutable attributes (`required`, `group_key`, labels);
+/// `key`, `data_type`, and binding are immutable and untouched. Records an `updated`
+/// audit entry. Returns `false` if no such key. Pass a transaction connection.
+pub async fn update_field_definition(
+    conn: &mut sqlx::PgConnection,
+    actor: AuditActor,
+    key: &str,
+    required: bool,
+    group_key: Option<&str>,
+    labels: &[LocalizedLabel],
+) -> Result<bool, sqlx::Error> {
+    let id: Option<uuid::Uuid> =
+        sqlx::query_scalar("SELECT id FROM field_definition WHERE key = $1")
+            .bind(key)
+            .fetch_optional(&mut *conn)
+            .await?;
+
+    let Some(id) = id else { return Ok(false) };
+
+    sqlx::query("UPDATE field_definition SET required = $2, group_key = $3 WHERE id = $1")
+        .bind(id)
+        .bind(required)
+        .bind(group_key)
+        .execute(&mut *conn)
+        .await?;
+
+    sqlx::query("DELETE FROM field_definition_label WHERE field_definition_id = $1")
+        .bind(id)
+        .execute(&mut *conn)
+        .await?;
+
+    for label in labels {
+        sqlx::query(
+            "INSERT INTO field_definition_label (field_definition_id, lang, label) \
+             VALUES ($1, $2, $3)",
+        )
+        .bind(id)
+        .bind(&label.lang)
+        .bind(&label.label)
+        .execute(&mut *conn)
+        .await?;
+    }
+
+    audit::record(
+        &mut *conn,
+        &NewAuditEvent {
+            actor,
+            action: AuditAction::Updated,
+            entity_type: FIELD_DEFINITION_ENTITY_TYPE.to_owned(),
+            entity_id: id,
+            changes: Vec::new(),
+        },
+    )
+    .await?;
+
+    Ok(true)
+}
+
+/// Count catalogue objects that store a value under field `key`.
+pub async fn count_objects_using_field<'e, E>(executor: E, key: &str) -> Result<i64, sqlx::Error>
+where
+    E: sqlx::PgExecutor<'e>,
+{
+    sqlx::query_scalar("SELECT count(*) FROM object WHERE jsonb_exists(fields, $1)")
+        .bind(key)
+        .fetch_one(executor)
+        .await
+}
+
+/// Delete a field definition (labels cascade) unless catalogue objects use its key,
+/// recording a `deleted` audit entry. Pass a transaction connection.
+pub async fn delete_field_definition(
+    conn: &mut sqlx::PgConnection,
+    actor: AuditActor,
+    key: &str,
+) -> Result<crate::DeleteOutcome, sqlx::Error> {
+    let id: Option<uuid::Uuid> =
+        sqlx::query_scalar("SELECT id FROM field_definition WHERE key = $1")
+            .bind(key)
+            .fetch_optional(&mut *conn)
+            .await?;
+
+    let Some(id) = id else {
+        return Ok(crate::DeleteOutcome::NotFound);
+    };
+
+    let count = count_objects_using_field(&mut *conn, key).await?;
+
+    if count > 0 {
+        return Ok(crate::DeleteOutcome::InUse { count });
+    }
+
+    sqlx::query("DELETE FROM field_definition WHERE id = $1")
+        .bind(id)
+        .execute(&mut *conn)
+        .await?;
+
+    audit::record(
+        &mut *conn,
+        &NewAuditEvent {
+            actor,
+            action: AuditAction::Deleted,
+            entity_type: FIELD_DEFINITION_ENTITY_TYPE.to_owned(),
+            entity_id: id,
+            changes: Vec::new(),
+        },
+    )
+    .await?;
+
+    Ok(crate::DeleteOutcome::Deleted)
+}

crates/db/tests/fields.rs

@@ -1,7 +1,24 @@
-use db::{Db, fields, vocab};
+use db::{Db, DeleteOutcome, catalog, fields, vocab};
-use domain::{AuditActor, AuthorityKind, FieldType, LocalizedLabel, NewFieldDefinition};
+use domain::{
+    AuditActor, AuthorityKind, FieldType, LocalizedLabel, NewFieldDefinition, ObjectInput,
+    Visibility,
+};
 use sqlx::PgPool;
 
+fn sample_object_input() -> ObjectInput {
+    ObjectInput {
+        object_number: "X.1".into(),
+        object_name: "Test".into(),
+        number_of_objects: 1,
+        brief_description: None,
+        current_location: None,
+        current_owner: None,
+        recorder: None,
+        recording_date: None,
+        visibility: Visibility::Draft,
+    }
+}
+
 fn labels() -> Vec<LocalizedLabel> {
     vec![
         LocalizedLabel {
@@ -171,3 +188,107 @@ async fn any_authority_scalar_and_zero_labels_round_trip(pool: PgPool) {
     let keys: Vec<&str> = all.iter().map(|d| d.key.as_str()).collect();
     assert_eq!(keys, vec!["donor", "on_display"]);
 }
+
+#[sqlx::test(migrations = "../db/migrations")]
+async fn update_field_definition_edits_labels_group_required(pool: PgPool) {
+    let db = Db::from_pool(pool);
+    let mut tx = db.pool().begin().await.unwrap();
+
+    fields::create_field_definition(
+        &mut tx,
+        &NewFieldDefinition {
+            key: "weight".into(),
+            field_type: FieldType::Integer,
+            required: false,
+            group_key: None,
+            labels: vec![LocalizedLabel {
+                lang: "sv".into(),
+                label: "Vikt".into(),
+            }],
+        },
+    )
+    .await
+    .unwrap();
+
+    let existed = fields::update_field_definition(
+        &mut tx,
+        AuditActor::System,
+        "weight",
+        true,
+        Some("Mått"),
+        &[LocalizedLabel {
+            lang: "sv".into(),
+            label: "Vikt (g)".into(),
+        }],
+    )
+    .await
+    .unwrap();
+    assert!(existed);
+
+    tx.commit().await.unwrap();
+
+    let def = fields::field_definition_by_key(db.pool(), "weight")
+        .await
+        .unwrap()
+        .unwrap();
+    assert!(def.required);
+    assert_eq!(def.group_key.as_deref(), Some("Mått"));
+    assert_eq!(def.labels[0].label, "Vikt (g)");
+}
+
+#[sqlx::test(migrations = "../db/migrations")]
+async fn delete_field_definition_blocks_when_objects_use_it(pool: PgPool) {
+    let db = Db::from_pool(pool);
+    let mut tx = db.pool().begin().await.unwrap();
+
+    fields::create_field_definition(
+        &mut tx,
+        &NewFieldDefinition {
+            key: "weight".into(),
+            field_type: FieldType::Integer,
+            required: false,
+            group_key: None,
+            labels: vec![LocalizedLabel {
+                lang: "sv".into(),
+                label: "Vikt".into(),
+            }],
+        },
+    )
+    .await
+    .unwrap();
+
+    let obj = catalog::create_object(&mut tx, AuditActor::System, &sample_object_input())
+        .await
+        .unwrap();
+
+    let mut map = serde_json::Map::new();
+    map.insert("weight".into(), serde_json::Value::from(42));
+    catalog::set_object_fields(&mut tx, AuditActor::System, obj, &map)
+        .await
+        .unwrap();
+
+    assert_eq!(
+        fields::delete_field_definition(&mut tx, AuditActor::System, "weight")
+            .await
+            .unwrap(),
+        DeleteOutcome::InUse { count: 1 }
+    );
+
+    catalog::set_object_fields(&mut tx, AuditActor::System, obj, &serde_json::Map::new())
+        .await
+        .unwrap();
+
+    assert_eq!(
+        fields::delete_field_definition(&mut tx, AuditActor::System, "weight")
+            .await
+            .unwrap(),
+        DeleteOutcome::Deleted
+    );
+
+    assert_eq!(
+        fields::delete_field_definition(&mut tx, AuditActor::System, "weight")
+            .await
+            .unwrap(),
+        DeleteOutcome::NotFound
+    );
+}