diff --git a/crates/api/src/admin_objects.rs b/crates/api/src/admin_objects.rs index c053d85..ef32f2d 100644 --- a/crates/api/src/admin_objects.rs +++ b/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, + pub labels: Vec, +} + +/// 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, + State(state): State, + Path(key): Path, + Json(req): Json, +) -> Result { + let labels: Vec = 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, + State(state): State, + Path(key): Path, +) -> 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 { "/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), + ) } diff --git a/crates/api/src/openapi.rs b/crates/api/src/openapi.rs index 7c7383a..1827dec 100644 --- a/crates/api/src/openapi.rs +++ b/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, diff --git a/crates/api/tests/admin_catalog.rs b/crates/api/tests/admin_catalog.rs index 5157c31..a8d957c 100644 --- a/crates/api/tests/admin_catalog.rs +++ b/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); +} diff --git a/crates/db/src/fields.rs b/crates/db/src/fields.rs index d07ba6e..83dd6a2 100644 --- a/crates/db/src/fields.rs +++ b/crates/db/src/fields.rs @@ -1,11 +1,15 @@ //! Registry of flexible field definitions. use domain::{ - AuthorityKind, FieldDefinition, FieldDefinitionId, FieldType, LocalizedLabel, - NewFieldDefinition, VocabularyId, + AuditAction, AuditActor, AuthorityKind, FieldDefinition, FieldDefinitionId, FieldType, + 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, + labels: &[LocalizedLabel], +) -> Result { + let id: Option = + 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 +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 { + let id: Option = + 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) +} diff --git a/crates/db/tests/fields.rs b/crates/db/tests/fields.rs index faf8ecf..c0efef6 100644 --- a/crates/db/tests/fields.rs +++ b/crates/db/tests/fields.rs @@ -1,7 +1,24 @@ -use db::{Db, fields, vocab}; -use domain::{AuditActor, AuthorityKind, FieldType, LocalizedLabel, NewFieldDefinition}; +use db::{Db, DeleteOutcome, catalog, fields, vocab}; +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 { 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 + ); +}