From eacab2ced608882184001d549e1b3413df0a754d Mon Sep 17 00:00:00 2001 From: Nils Schneider Date: Fri, 31 Oct 2025 02:19:38 +0100 Subject: [PATCH] cleanup --- backend/src/routes/documents.rs | 52 +++++++++- backend/src/utils/named_entity.rs | 28 ++++++ backend/src/workers/common.rs | 95 +++++++++++++++++++ backend/tests/auth_flow.rs | 9 +- backend/tests/common/mod.rs | 16 ++-- backend/tests/document_types.rs | 127 +++++++++++++++++++++++++ backend/tests/documents_flow.rs | 152 ++++++++++++++++++++++++++++-- 7 files changed, 462 insertions(+), 17 deletions(-) create mode 100644 backend/src/utils/named_entity.rs create mode 100644 backend/src/workers/common.rs create mode 100644 backend/tests/document_types.rs diff --git a/backend/src/routes/documents.rs b/backend/src/routes/documents.rs index 6bfaf2f..52c2fd4 100644 --- a/backend/src/routes/documents.rs +++ b/backend/src/routes/documents.rs @@ -325,6 +325,7 @@ struct UploadRequest { correspondents: Vec, issued_at_override: Option, skip_if_existing: bool, + document_type_id: Option, } enum UploadOutcome { @@ -785,6 +786,7 @@ pub async fn upload_document( let mut issued_at_override: Option = None; let mut skip_if_existing = false; let mut title_override: Option = None; + let mut document_type_id: Option = None; while let Some(field) = multipart.next_field().await.map_err(|err| { let msg = format!("invalid multipart data: {err}"); @@ -903,6 +905,20 @@ pub async fn upload_document( "1" | "true" | "yes" ); } + Some("document_type_id") => { + let value = field.text().await.map_err(|err| { + let msg = format!("invalid document_type_id: {err}"); + error!(error = %err, "invalid document_type payload"); + AppError::bad_request(msg) + })?; + let trimmed = value.trim(); + if !trimmed.is_empty() { + let parsed = Uuid::parse_str(trimmed).map_err(|_| { + AppError::bad_request("document_type_id must be a valid UUID") + })?; + document_type_id = Some(parsed); + } + } _ => {} } } @@ -933,6 +949,7 @@ pub async fn upload_document( correspondents, issued_at_override, skip_if_existing, + document_type_id, }; let outcome = match process_upload(&state, request, tenant_id, user_id).await { @@ -2178,8 +2195,23 @@ async fn process_upload( correspondents, issued_at_override, skip_if_existing, + document_type_id, } = request; + if let Some(type_id) = document_type_id { + let mut conn = state.db_for_tenant(tenant_id)?; + let exists = document_types::table + .filter(document_types::tenant_id.eq(tenant_id)) + .find(type_id) + .first::(&mut conn) + .optional()?; + if exists.is_none() { + return Err(AppError::bad_request( + "document_type_id does not exist for this tenant", + )); + } + } + if let Some(folder) = folder_id { let mut conn = state.db_for_tenant(tenant_id)?; ensure_folder_exists_on_conn(&mut conn, tenant_id, folder)?; @@ -2260,6 +2292,24 @@ async fn process_upload( )?; } + if let Some(type_id) = document_type_id { + if document.document_type_id != Some(type_id) { + let now = Utc::now().naive_utc(); + diesel::update( + documents::table + .find(document.id) + .filter(documents::tenant_id.eq(tenant_id)), + ) + .set(( + documents::document_type_id.eq(Some(type_id)), + documents::updated_at.eq(now), + )) + .execute(&mut conn)?; + document.document_type_id = Some(type_id); + document.updated_at = now; + } + } + if document.deleted_at.is_some() { let now = Utc::now().naive_utc(); diesel::update(documents::table.find(document.id)) @@ -2336,7 +2386,7 @@ async fn process_upload( title: derived_title.clone(), metadata: metadata_value.clone(), tenant_id, - document_type_id: None, + document_type_id, }; diesel::insert_into(documents::table) .values(&new_document) diff --git a/backend/src/utils/named_entity.rs b/backend/src/utils/named_entity.rs new file mode 100644 index 0000000..13f8954 --- /dev/null +++ b/backend/src/utils/named_entity.rs @@ -0,0 +1,28 @@ +use diesel::QueryResult; + +use crate::error::{AppError, AppResult}; + +/// Trim and validate a user-supplied entity name, returning an owned String. +/// +/// The `on_empty` closure is only invoked when the trimmed name is empty, giving +/// callers control over the concrete error that should be surfaced. +pub fn normalize_name(raw: &str, on_empty: impl Fn() -> AppError) -> AppResult { + let trimmed = raw.trim(); + if trimmed.is_empty() { + return Err(on_empty()); + } + Ok(trimmed.to_string()) +} + +/// Ensure that no conflicting entity exists by executing the provided query +/// closure. If a record is returned, the `on_duplicate` closure is evaluated to +/// produce the appropriate error. +pub fn ensure_name_available( + query: impl FnOnce() -> QueryResult>, + on_duplicate: impl Fn() -> AppError, +) -> AppResult<()> { + if query()?.is_some() { + return Err(on_duplicate()); + } + Ok(()) +} diff --git a/backend/src/workers/common.rs b/backend/src/workers/common.rs new file mode 100644 index 0000000..0cbdaf5 --- /dev/null +++ b/backend/src/workers/common.rs @@ -0,0 +1,95 @@ +use std::collections::HashMap; + +use diesel::{prelude::*, PgConnection}; +use uuid::Uuid; + +use crate::models::{Document, DocumentAsset, DocumentAssetObject, DocumentVersion}; +use crate::schema::{document_asset_objects, document_assets, document_versions, documents}; +use crate::state::AppState; + +pub(crate) struct DocumentVersionContext { + pub document: Document, + pub version: DocumentVersion, + pub tenant_id: Uuid, +} + +pub(crate) fn load_document_version( + state: &AppState, + document_id: Uuid, + version_id: Uuid, +) -> Result { + let mut conn = state.db_unscoped().map_err(|err| format!("{err:?}"))?; + + let version: DocumentVersion = document_versions::table + .find(version_id) + .first(&mut conn) + .map_err(|err| format!("{err:?}"))?; + + if version.document_id != document_id { + return Err("document/version mismatch".into()); + } + + let document: Document = documents::table + .find(document_id) + .first(&mut conn) + .map_err(|err| format!("{err:?}"))?; + + let tenant_id = document.tenant_id; + + Ok(DocumentVersionContext { + document, + version, + tenant_id, + }) +} + +pub(crate) struct LoadedAsset { + pub asset: DocumentAsset, + pub objects: Vec, +} + +pub(crate) fn load_version_assets( + conn: &mut PgConnection, + tenant_id: Uuid, + version_id: Uuid, + asset_types: &[&str], +) -> Result, String> { + let mut query = document_assets::table + .filter(document_assets::document_version_id.eq(version_id)) + .filter(document_assets::tenant_id.eq(tenant_id)) + .into_boxed(); + + if !asset_types.is_empty() { + let types: Vec = asset_types.iter().map(|ty| (*ty).to_string()).collect(); + query = query.filter(document_assets::asset_type.eq_any(types)); + } + + let assets: Vec = query + .order(document_assets::created_at.asc()) + .load(conn) + .map_err(|err| format!("{err:?}"))?; + + let asset_ids: Vec = assets.iter().map(|asset| asset.id).collect(); + + let mut object_map: HashMap> = HashMap::new(); + if !asset_ids.is_empty() { + let objects: Vec = document_asset_objects::table + .filter(document_asset_objects::asset_id.eq_any(&asset_ids)) + .filter(document_asset_objects::tenant_id.eq(tenant_id)) + .order(document_asset_objects::ordinal.asc()) + .load(conn) + .map_err(|err| format!("{err:?}"))?; + + for object in objects { + object_map.entry(object.asset_id).or_default().push(object); + } + } + + let mut result = HashMap::with_capacity(assets.len()); + for asset in assets { + let objects = object_map.remove(&asset.id).unwrap_or_default(); + result.insert(asset.asset_type.clone(), LoadedAsset { asset, objects }); + } + + Ok(result) +} diff --git a/backend/tests/auth_flow.rs b/backend/tests/auth_flow.rs index 3225ee6..089f017 100644 --- a/backend/tests/auth_flow.rs +++ b/backend/tests/auth_flow.rs @@ -3,7 +3,7 @@ mod common; use anyhow::{anyhow, Context, Result}; use axum::http::{header::SET_COOKIE, StatusCode}; use chrono::{Duration as ChronoDuration, Utc}; -use common::{acquire_db_lock, body_to_vec, ApiErrorResponse, TestApp}; +use common::{acquire_db_lock, body_to_vec, TestApp}; use diesel::prelude::*; use papercrate::auth::passkeys::{ PasskeyLoginFinishPayload, PasskeyLoginStartPayload, PasskeyRegistrationFinishPayload, @@ -28,6 +28,13 @@ struct AuthenticatedUser { username: String, } +#[derive(Deserialize)] +struct ApiErrorResponse { + error: String, + #[serde(default)] + _code: Option, +} + #[derive(Deserialize)] struct LoginTenant { id: Uuid, diff --git a/backend/tests/common/mod.rs b/backend/tests/common/mod.rs index 593aa1e..c2e6957 100644 --- a/backend/tests/common/mod.rs +++ b/backend/tests/common/mod.rs @@ -29,7 +29,7 @@ use papercrate::state::AppState; use papercrate::storage::ObjectStorage; use rand::rngs::OsRng; use rand::RngCore; -use serde::{Deserialize, Serialize}; +use serde::Serialize; use serde_json::{self, json}; use sha2::{Digest, Sha256}; use tokio::sync::Mutex; @@ -122,13 +122,6 @@ pub struct TestApp { storage: Arc, } -#[derive(Debug, Deserialize)] -pub struct ApiErrorResponse { - pub error: String, - #[serde(default)] - pub code: Option, -} - impl TestApp { pub async fn new() -> Result { let database_url = env::var("TEST_DATABASE_URL") @@ -625,6 +618,13 @@ impl TestApp { body.extend(b"\r\n"); } + if let Some(document_type_id) = extras.document_type_id { + body.extend(format!("--{boundary}\r\n").as_bytes()); + body.extend(b"Content-Disposition: form-data; name=\"document_type_id\"\r\n\r\n"); + body.extend(document_type_id.to_string().as_bytes()); + body.extend(b"\r\n"); + } + if extras.skip_existing { body.extend(format!("--{boundary}\r\n").as_bytes()); body.extend(b"Content-Disposition: form-data; name=\"skip_existing\"\r\n\r\ntrue\r\n"); diff --git a/backend/tests/document_types.rs b/backend/tests/document_types.rs new file mode 100644 index 0000000..9080fac --- /dev/null +++ b/backend/tests/document_types.rs @@ -0,0 +1,127 @@ +mod common; + +use anyhow::Result; +use axum::http::StatusCode; +use common::{acquire_db_lock, body_to_vec, TestApp}; +use serde_json::json; +use uuid::Uuid; + +#[derive(serde::Deserialize)] +struct DocumentTypeInfo { + id: Uuid, + name: String, +} + +#[derive(serde::Deserialize)] +struct ApiErrorResponse { + error: String, + #[serde(default)] + code: Option, +} + +#[tokio::test] +async fn document_type_crud_flow() -> Result<()> { + let _lock = acquire_db_lock().await; + let app = TestApp::new().await?; + + let password = "doctypecrud"; + let username = "doctype_admin"; + app.insert_user(username, password, "admin").await?; + let token = app.login_token(username, password).await?; + + let create_invoices = app + .post_json( + "/api/document-types", + &json!({"name": "Invoices"}), + Some(&token), + ) + .await?; + assert_eq!(create_invoices.status(), StatusCode::CREATED); + let invoices_body = body_to_vec(create_invoices.into_body()).await?; + let invoices: DocumentTypeInfo = serde_json::from_slice(&invoices_body)?; + let invoices_id = invoices.id; + + let create_receipts = app + .post_json( + "/api/document-types", + &json!({"name": "Receipts"}), + Some(&token), + ) + .await?; + assert_eq!(create_receipts.status(), StatusCode::CREATED); + let receipts_body = body_to_vec(create_receipts.into_body()).await?; + let receipts: DocumentTypeInfo = serde_json::from_slice(&receipts_body)?; + + let list_resp = app.get("/api/document-types", Some(&token)).await?; + assert!(list_resp.status().is_success()); + let list_body = body_to_vec(list_resp.into_body()).await?; + let mut all_types: Vec = serde_json::from_slice(&list_body)?; + all_types.sort_by(|a, b| a.name.cmp(&b.name)); + assert_eq!(all_types.len(), 2); + assert_eq!(all_types[0].name, "Invoices"); + assert_eq!(all_types[1].name, "Receipts"); + + let update_resp = app + .patch_json( + &format!("/api/document-types/{}", invoices_id), + &json!({"name": "Bills"}), + Some(&token), + ) + .await?; + assert_eq!(update_resp.status(), StatusCode::OK); + let update_body = body_to_vec(update_resp.into_body()).await?; + let updated: DocumentTypeInfo = serde_json::from_slice(&update_body)?; + assert_eq!(updated.id, invoices_id); + assert_eq!(updated.name, "Bills"); + + let conflict_resp = app + .patch_json( + &format!("/api/document-types/{}", receipts.id), + &json!({"name": "Bills"}), + Some(&token), + ) + .await?; + assert_eq!(conflict_resp.status(), StatusCode::CONFLICT); + let conflict_body = body_to_vec(conflict_resp.into_body()).await?; + let conflict_json: ApiErrorResponse = serde_json::from_slice(&conflict_body)?; + assert_eq!( + conflict_json.error, + "a document type with that name already exists" + ); + assert_eq!( + conflict_json.code.as_deref(), + Some("duplicate_document_type") + ); + + let delete_receipts = app + .delete( + &format!("/api/document-types/{}", receipts.id), + Some(&token), + ) + .await?; + assert_eq!(delete_receipts.status(), StatusCode::NO_CONTENT); + + let list_after_resp = app.get("/api/document-types", Some(&token)).await?; + assert!(list_after_resp.status().is_success()); + let list_after_body = body_to_vec(list_after_resp.into_body()).await?; + let list_after: Vec = serde_json::from_slice(&list_after_body)?; + assert_eq!(list_after.len(), 1); + assert_eq!(list_after[0].id, invoices_id); + assert_eq!(list_after[0].name, "Bills"); + + let delete_bills = app + .delete( + &format!("/api/document-types/{}", invoices_id), + Some(&token), + ) + .await?; + assert_eq!(delete_bills.status(), StatusCode::NO_CONTENT); + + let final_list_resp = app.get("/api/document-types", Some(&token)).await?; + let final_list_body = body_to_vec(final_list_resp.into_body()).await?; + let final_list: Vec = serde_json::from_slice(&final_list_body)?; + assert!(final_list.is_empty()); + + app.cleanup().await?; + Ok(()) +} diff --git a/backend/tests/documents_flow.rs b/backend/tests/documents_flow.rs index 4ff9fec..c48525a 100644 --- a/backend/tests/documents_flow.rs +++ b/backend/tests/documents_flow.rs @@ -2,7 +2,7 @@ mod common; use anyhow::Result; use axum::http::StatusCode; -use common::{acquire_db_lock, body_to_vec, ApiErrorResponse, TestApp, UploadExtras}; +use common::{acquire_db_lock, body_to_vec, TestApp, UploadExtras}; use serde::{Deserialize, Serialize}; use serde_json::{json, Value}; use uuid::Uuid; @@ -12,6 +12,13 @@ struct DocumentDetail { document: DocumentInfo, } +#[derive(Deserialize)] +struct ApiErrorResponse { + error: String, + #[serde(default)] + code: Option, +} + #[derive(Deserialize)] struct DocumentInfo { id: Uuid, @@ -25,11 +32,19 @@ struct DocumentInfo { metadata: Value, #[serde(default)] document_type_id: Option, + #[serde(default)] + document_type: Option, tags: Vec, #[serde(default)] current_version: Option, } +#[derive(Deserialize)] +struct DocumentTypeInfo { + id: Uuid, + name: String, +} + #[derive(Deserialize)] struct DocumentVersionPayload { id: Uuid, @@ -83,12 +98,6 @@ struct TagSummary { label: String, } -#[derive(Deserialize)] -struct DocumentTypeInfo { - id: Uuid, - name: String, -} - #[derive(Deserialize)] struct AnalyzeJobPayload { document_id: Uuid, @@ -411,6 +420,7 @@ async fn upload_skips_existing_when_requested() -> Result<()> { correspondents_json: None, issued_at: None, skip_existing: false, + document_type_id: None, }; let first_upload = app @@ -461,6 +471,7 @@ async fn upload_skips_existing_when_requested() -> Result<()> { correspondents_json: None, issued_at: None, skip_existing: true, + document_type_id: None, }; let skip_resp = app @@ -1442,6 +1453,133 @@ async fn list_documents_by_status_filter() -> Result<()> { Ok(()) } +#[tokio::test] +async fn document_upload_and_patch_document_type() -> Result<()> { + let _lock = acquire_db_lock().await; + let app = TestApp::new().await?; + + let password = "doctypeupload"; + let username = "doctype_user"; + app.insert_user(username, password, "admin").await?; + let token = app.login_token(username, password).await?; + + let invoices_resp = app + .post_json( + "/api/document-types", + &json!({"name": "Invoices"}), + Some(&token), + ) + .await?; + assert_eq!(invoices_resp.status(), StatusCode::CREATED); + let invoices_body = body_to_vec(invoices_resp.into_body()).await?; + let invoices: DocumentTypeInfo = serde_json::from_slice(&invoices_body)?; + + let receipts_resp = app + .post_json( + "/api/document-types", + &json!({"name": "Receipts"}), + Some(&token), + ) + .await?; + assert_eq!(receipts_resp.status(), StatusCode::CREATED); + let receipts_body = body_to_vec(receipts_resp.into_body()).await?; + let receipts: DocumentTypeInfo = serde_json::from_slice(&receipts_body)?; + + let upload_extras = UploadExtras { + title: Some("Invoice #1"), + metadata_json: None, + tag_ids_json: None, + correspondents_json: None, + issued_at: None, + skip_existing: false, + document_type_id: Some(invoices.id), + }; + + let upload_resp = app + .upload_document_with_extras( + "/api/documents", + "invoice.pdf", + "application/pdf", + b"invoice-bytes", + None, + upload_extras, + &token, + ) + .await?; + assert!(upload_resp.status().is_success()); + let upload_body = body_to_vec(upload_resp.into_body()).await?; + let mut detail: DocumentDetail = serde_json::from_slice(&upload_body)?; + assert_eq!(detail.document.document_type_id, Some(invoices.id)); + let doc_type_info = detail + .document + .document_type + .as_ref() + .expect("document type present"); + assert_eq!(doc_type_info.name, "Invoices"); + + let document_id = detail.document.id; + + let update_resp = app + .patch_json( + &format!("/api/documents/{}", document_id), + &json!({"document_type_id": receipts.id}), + Some(&token), + ) + .await?; + assert!(update_resp.status().is_success()); + let update_body = body_to_vec(update_resp.into_body()).await?; + detail = serde_json::from_slice(&update_body)?; + assert_eq!(detail.document.document_type_id, Some(receipts.id)); + assert_eq!( + detail + .document + .document_type + .as_ref() + .map(|info| info.name.as_str()), + Some("Receipts") + ); + + let clear_resp = app + .patch_json( + &format!("/api/documents/{}", document_id), + &json!({"document_type_id": null}), + Some(&token), + ) + .await?; + assert!(clear_resp.status().is_success()); + let clear_body = body_to_vec(clear_resp.into_body()).await?; + detail = serde_json::from_slice(&clear_body)?; + assert!(detail.document.document_type_id.is_none()); + assert!(detail.document.document_type.is_none()); + + let invalid_id = Uuid::new_v4(); + let invalid_resp = app + .patch_json( + &format!("/api/documents/{}", document_id), + &json!({"document_type_id": invalid_id}), + Some(&token), + ) + .await?; + assert_eq!(invalid_resp.status(), StatusCode::BAD_REQUEST); + let invalid_body = body_to_vec(invalid_resp.into_body()).await?; + let invalid_error: ApiErrorResponse = serde_json::from_slice(&invalid_body)?; + assert_eq!( + invalid_error.error, + "document_type_id does not exist for this tenant" + ); + + let final_detail_resp = app + .get(&format!("/api/documents/{}", document_id), Some(&token)) + .await?; + let final_detail_body = body_to_vec(final_detail_resp.into_body()).await?; + let final_detail: DocumentDetail = serde_json::from_slice(&final_detail_body)?; + assert!(final_detail.document.document_type_id.is_none()); + assert!(final_detail.document.document_type.is_none()); + + app.cleanup().await?; + Ok(()) +} + #[tokio::test] async fn list_documents_filtered_by_document_type() -> Result<()> { let _lock = acquire_db_lock().await;