From 7df9a6415a9178f044b86ecc42754d6b388b8d82 Mon Sep 17 00:00:00 2001 From: Nils Schneider Date: Tue, 28 Oct 2025 01:18:35 +0100 Subject: [PATCH] refactor --- .../down.sql | 7 + .../up.sql | 24 + backend/src/models.rs | 4 +- backend/src/openapi.rs | 26 +- backend/src/routes/correspondents.rs | 58 +-- backend/src/routes/documents.rs | 224 +++------ .../routes/documents/correspondent_utils.rs | 49 +- backend/src/schema.rs | 4 +- backend/tests/correspondents_flow.rs | 276 +++++++++++ backend/tests/documents_flow.rs | 308 +----------- frontend/src/constants/correspondents.js | 3 - .../correspondents/CorrespondentsPanel.jsx | 9 +- frontend/src/detail/DetailPanel.jsx | 106 ++-- frontend/src/documents/DocumentsTable.jsx | 138 +++++- frontend/src/index.jsx | 460 +++++++----------- frontend/src/preview/PreviewWorkspace.jsx | 132 +++++ frontend/src/skeuomorphic_ws.jsx | 42 +- 17 files changed, 935 insertions(+), 935 deletions(-) create mode 100644 backend/migrations/202512010050_drop_correspondent_roles/down.sql create mode 100644 backend/migrations/202512010050_drop_correspondent_roles/up.sql create mode 100644 backend/tests/correspondents_flow.rs delete mode 100644 frontend/src/constants/correspondents.js diff --git a/backend/migrations/202512010050_drop_correspondent_roles/down.sql b/backend/migrations/202512010050_drop_correspondent_roles/down.sql new file mode 100644 index 0000000..e8b059d --- /dev/null +++ b/backend/migrations/202512010050_drop_correspondent_roles/down.sql @@ -0,0 +1,7 @@ +ALTER TABLE document_correspondents DROP CONSTRAINT document_correspondents_pkey; +ALTER TABLE document_correspondents ADD COLUMN role VARCHAR(32) NOT NULL DEFAULT 'other'; +UPDATE document_correspondents SET role = 'other'; +ALTER TABLE document_correspondents ALTER COLUMN role DROP DEFAULT; +ALTER TABLE document_correspondents + ADD CONSTRAINT document_correspondents_pkey + PRIMARY KEY (document_id, correspondent_id, role); diff --git a/backend/migrations/202512010050_drop_correspondent_roles/up.sql b/backend/migrations/202512010050_drop_correspondent_roles/up.sql new file mode 100644 index 0000000..43fb1d6 --- /dev/null +++ b/backend/migrations/202512010050_drop_correspondent_roles/up.sql @@ -0,0 +1,24 @@ +WITH ranked AS ( + SELECT + document_id, + correspondent_id, + role, + assigned_at, + assigned_by, + tenant_id, + ROW_NUMBER() OVER (PARTITION BY document_id, correspondent_id ORDER BY assigned_at DESC) AS rn + FROM document_correspondents +) +DELETE FROM document_correspondents dc +USING ranked r +WHERE dc.document_id = r.document_id + AND dc.correspondent_id = r.correspondent_id + AND dc.role = r.role + AND dc.tenant_id = r.tenant_id + AND r.rn > 1; + +ALTER TABLE document_correspondents DROP CONSTRAINT document_correspondents_pkey; +ALTER TABLE document_correspondents DROP COLUMN role; +ALTER TABLE document_correspondents + ADD CONSTRAINT document_correspondents_pkey + PRIMARY KEY (document_id, correspondent_id); diff --git a/backend/src/models.rs b/backend/src/models.rs index 14d6d9e..e96c4aa 100644 --- a/backend/src/models.rs +++ b/backend/src/models.rs @@ -281,11 +281,10 @@ pub struct NewCorrespondent { #[diesel(table_name = document_correspondents)] #[diesel(belongs_to(Document))] #[diesel(belongs_to(Correspondent))] -#[diesel(primary_key(document_id, correspondent_id, role))] +#[diesel(primary_key(document_id, correspondent_id))] pub struct DocumentCorrespondent { pub document_id: Uuid, pub correspondent_id: Uuid, - pub role: String, pub assigned_at: NaiveDateTime, pub assigned_by: Option, pub tenant_id: Uuid, @@ -296,7 +295,6 @@ pub struct DocumentCorrespondent { pub struct NewDocumentCorrespondent { pub document_id: Uuid, pub correspondent_id: Uuid, - pub role: String, pub assigned_by: Option, pub tenant_id: Uuid, } diff --git a/backend/src/openapi.rs b/backend/src/openapi.rs index b0f1ac2..9ce5684 100644 --- a/backend/src/openapi.rs +++ b/backend/src/openapi.rs @@ -76,7 +76,6 @@ use uuid::Uuid; schemas::BulkCorrespondentsResponse, schemas::BulkCorrespondentAction, schemas::AssignCorrespondentsRequest, - schemas::RemoveCorrespondentParams, schemas::ReanalyzeRequest, schemas::ReanalyzeResponse, schemas::DocumentAssetRequestParams, @@ -95,6 +94,7 @@ use uuid::Uuid; schemas::TagCatalogEntry, schemas::CreateTagRequest, schemas::UpdateTagRequest, + schemas::CorrespondentUsage, schemas::CorrespondentCatalogEntry, schemas::CreateCorrespondentRequest, schemas::UpdateCorrespondentRequest, @@ -351,8 +351,7 @@ mod doc { path = "/api/documents/{id}/correspondents/{correspondent_id}", params( ("id" = Uuid, Path, description = "Document ID"), - ("correspondent_id" = Uuid, Path, description = "Correspondent ID"), - RemoveCorrespondentParams + ("correspondent_id" = Uuid, Path, description = "Correspondent ID") ), responses((status = 204, description = "Correspondent removed")), tag = "Documents" @@ -683,7 +682,6 @@ pub mod schemas { pub struct DocumentCorrespondent { pub id: Uuid, pub name: String, - pub role: String, pub metadata: Value, pub assigned_at: String, } @@ -706,8 +704,8 @@ pub mod schemas { pub issued_at: Option, pub metadata: Value, pub tags: Vec, - #[schema(nullable)] - pub correspondents: Option>, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub correspondents: Vec, #[schema(nullable)] pub current_version: Option, } @@ -787,7 +785,6 @@ pub mod schemas { #[derive(Serialize, Deserialize, ToSchema)] pub struct CorrespondentAssignment { pub correspondent_id: Uuid, - pub role: String, } #[derive(Serialize, Deserialize, ToSchema)] @@ -822,12 +819,6 @@ pub mod schemas { pub removed: usize, } - #[derive(Serialize, Deserialize, IntoParams, ToSchema)] - #[into_params(parameter_in = Query)] - pub struct RemoveCorrespondentParams { - pub role: String, - } - #[derive(Serialize, Deserialize, ToSchema)] pub struct ReanalyzeRequest { pub document_ids: Vec, @@ -981,12 +972,19 @@ pub mod schemas { pub color: Option>, } + #[derive(Serialize, Deserialize, ToSchema)] + pub struct CorrespondentUsage { + pub total: i64, + } + #[derive(Serialize, Deserialize, ToSchema)] pub struct CorrespondentCatalogEntry { pub id: Uuid, pub name: String, pub metadata: Value, - pub role_counts: Value, + pub created_at: String, + pub updated_at: String, + pub usage: CorrespondentUsage, } #[derive(Serialize, Deserialize, ToSchema)] diff --git a/backend/src/routes/correspondents.rs b/backend/src/routes/correspondents.rs index 2e277ba..7e730c6 100644 --- a/backend/src/routes/correspondents.rs +++ b/backend/src/routes/correspondents.rs @@ -1,4 +1,4 @@ -use std::collections::{BTreeMap, HashMap}; +use std::collections::HashMap; use axum::{extract::Path, http::StatusCode, Json}; use chrono::Utc; @@ -21,8 +21,6 @@ use crate::{ #[derive(Serialize)] pub struct CorrespondentUsage { pub total: i64, - #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] - pub by_role: BTreeMap, } #[derive(Serialize)] @@ -67,31 +65,21 @@ pub async fn list_correspondents( .order(correspondents::name.asc()) .load(&mut conn)?; - let usage_rows: Vec<(Uuid, String, i64)> = document_correspondents::table + let usage_rows: Vec<(Uuid, i64)> = document_correspondents::table .filter(document_correspondents::tenant_id.eq(tenant_id)) - .group_by(( - document_correspondents::correspondent_id, - document_correspondents::role, - )) - .select(( - document_correspondents::correspondent_id, - document_correspondents::role, - count_star(), - )) + .group_by(document_correspondents::correspondent_id) + .select((document_correspondents::correspondent_id, count_star())) .load(&mut conn)?; - let mut usage_map: HashMap> = HashMap::new(); - for (correspondent_id, role, count) in usage_rows { - usage_map - .entry(correspondent_id) - .or_default() - .insert(role, count); + let mut usage_map: HashMap = HashMap::new(); + for (correspondent_id, count) in usage_rows { + usage_map.insert(correspondent_id, count); } let mut response = Vec::with_capacity(correspondents_list.len()); for correspondent in correspondents_list { - let role_counts = usage_map.remove(&correspondent.id).unwrap_or_default(); - response.push(build_summary(correspondent, role_counts)); + let total = usage_map.remove(&correspondent.id).unwrap_or(0); + response.push(build_summary(correspondent, total)); } response.into_json() @@ -136,7 +124,7 @@ pub async fn create_correspondent( .first(&mut conn) .one()?; - build_summary(correspondent, BTreeMap::new()).into_json() + build_summary(correspondent, 0).into_json() } pub async fn update_correspondent( @@ -245,21 +233,14 @@ pub async fn delete_correspondent( no_content() } -fn build_summary( - correspondent: Correspondent, - role_counts: BTreeMap, -) -> CorrespondentSummary { - let total = role_counts.values().copied().sum(); +fn build_summary(correspondent: Correspondent, total: i64) -> CorrespondentSummary { CorrespondentSummary { id: correspondent.id, name: correspondent.name, metadata: correspondent.metadata, created_at: to_iso(correspondent.created_at), updated_at: to_iso(correspondent.updated_at), - usage: CorrespondentUsage { - total, - by_role: role_counts, - }, + usage: CorrespondentUsage { total }, } } @@ -274,17 +255,12 @@ fn load_usage_for_correspondent( conn: &mut PgConnection, tenant_id: Uuid, correspondent_id: Uuid, -) -> AppResult> { - let rows: Vec<(String, i64)> = document_correspondents::table +) -> AppResult { + let total: i64 = document_correspondents::table .filter(document_correspondents::correspondent_id.eq(correspondent_id)) .filter(document_correspondents::tenant_id.eq(tenant_id)) - .group_by(document_correspondents::role) - .select((document_correspondents::role, count_star())) - .load(conn)?; + .select(count_star()) + .get_result(conn)?; - let mut map = BTreeMap::new(); - for (role, count) in rows { - map.insert(role, count); - } - Ok(map) + Ok(total) } diff --git a/backend/src/routes/documents.rs b/backend/src/routes/documents.rs index 741713c..81bf059 100644 --- a/backend/src/routes/documents.rs +++ b/backend/src/routes/documents.rs @@ -48,10 +48,7 @@ use asset_utils::{ build_download_path, derive_document_title, filename_with_retained_extension, to_asset_detail_response, to_asset_object_response, to_asset_summary, to_version_response, }; -use correspondent_utils::{ - is_valid_correspondent_role, normalize_correspondent_assignments, normalize_role, - CORRESPONDENT_ROLES, -}; +use correspondent_utils::normalize_correspondent_assignments; use search_utils::{build_quickwit_query, extract_document_id}; const PRESIGNED_URL_EXPIRY_SECONDS: u64 = 300; @@ -186,7 +183,6 @@ pub struct DocumentVersionDetailResponse { pub struct DocumentCorrespondentResponse { pub id: Uuid, pub name: String, - pub role: String, pub metadata: Value, pub assigned_at: String, } @@ -280,7 +276,6 @@ pub struct BulkCorrespondentResponse { #[derive(Deserialize, ToSchema)] pub struct CorrespondentAssignmentInput { pub correspondent_id: Uuid, - pub role: String, } #[derive(Deserialize, ToSchema)] @@ -319,12 +314,6 @@ pub struct BulkCorrespondentsRequest { pub action: BulkCorrespondentAction, } -#[derive(Deserialize, IntoParams, ToSchema)] -#[into_params(parameter_in = Query)] -pub struct CorrespondentRoleQuery { - pub role: String, -} - #[derive(Deserialize, ToSchema)] pub struct BulkReanalyzeSelectionRequest { pub document_ids: Vec, @@ -853,7 +842,7 @@ pub async fn upload_document( })?; correspondents = serde_json::from_str(&value).map_err(|err| { let msg = format!( - "correspondents must be a JSON array of {{correspondent_id, role}} objects: {err}" + "correspondents must be a JSON array of {{correspondent_id}} objects: {err}" ); error!(error = %err, "invalid correspondents json"); AppError::bad_request(msg) @@ -1672,8 +1661,7 @@ pub async fn assign_correspondents( return Err(AppError::bad_request("assignments must not be empty")); } - let (normalized_pairs, _correspondent_ids, roles_vec) = - normalize_correspondent_assignments(&payload.assignments)?; + let correspondent_ids = normalize_correspondent_assignments(&payload.assignments)?; let replace = payload.replace; conn.transaction::<(), AppError, _>(|conn| { @@ -1685,21 +1673,41 @@ pub async fn assign_correspondents( return Err(AppError::not_found()); } - let mut deleted = 0; + let mut updated = false; if replace { - deleted = diesel::delete( - document_correspondents::table - .filter(document_correspondents::document_id.eq(document_id)) - .filter(document_correspondents::tenant_id.eq(tenant_id)) - .filter(document_correspondents::role.eq_any(&roles_vec)), - ) - .execute(conn)?; + use diesel::dsl::not; + + let base = document_correspondents::table + .filter(document_correspondents::document_id.eq(document_id)) + .filter(document_correspondents::tenant_id.eq(tenant_id)); + + let removed = if correspondent_ids.is_empty() { + diesel::delete(base).execute(conn)? + } else { + diesel::delete(base.filter(not( + document_correspondents::correspondent_id.eq_any(&correspondent_ids), + ))) + .execute(conn)? + }; + + if removed > 0 { + updated = true; + } } - let inserted = - insert_document_correspondents(conn, tenant_id, &document, user_id, &normalized_pairs)?; + let inserted = insert_document_correspondents( + conn, + tenant_id, + document.id, + user_id, + &correspondent_ids, + )?; - if replace && deleted > 0 && inserted == 0 { + if inserted > 0 { + updated = true; + } + + if updated && inserted == 0 { diesel::update( documents::table .find(document_id) @@ -1731,8 +1739,7 @@ pub async fn bulk_assign_correspondents( let mut document_ids = payload.document_ids; validate_bulk_ids(&mut document_ids, "document_ids")?; - let (normalized_pairs, correspondents_vec, _roles_vec) = - normalize_correspondent_assignments(&payload.assignments)?; + let correspondent_ids = normalize_correspondent_assignments(&payload.assignments)?; let action = payload.action; let user_id_val = user_id; let (assigned, removed) = conn.transaction::<(usize, usize), AppError, _>(|conn| { @@ -1754,12 +1761,12 @@ pub async fn bulk_assign_correspondents( )); } - if !correspondents_vec.is_empty() { + if !correspondent_ids.is_empty() { let existing: Vec = correspondents::table - .filter(correspondents::id.eq_any(&correspondents_vec)) + .filter(correspondents::id.eq_any(&correspondent_ids)) .filter(correspondents::tenant_id.eq(tenant_id)) .load(conn)?; - if existing.len() != correspondents_vec.len() { + if existing.len() != correspondent_ids.len() { return Err(AppError::bad_request( "one or more correspondents do not exist", )); @@ -1768,92 +1775,34 @@ pub async fn bulk_assign_correspondents( match action { BulkCorrespondentAction::Add => { - use diesel::dsl::not; - - let mut grouped_by_role: HashMap> = HashMap::new(); - for (correspondent_id, role) in &normalized_pairs { - grouped_by_role - .entry(role.clone()) - .or_default() - .push(*correspondent_id); + let mut assigned_total = 0; + for (doc_id, _) in &docs { + assigned_total += insert_document_correspondents( + conn, + tenant_id, + *doc_id, + user_id_val, + &correspondent_ids, + )?; } - let mut removed = 0; - for (role, ids) in grouped_by_role.iter() { - if ids.is_empty() { - continue; - } - let maintained_ids = ids.clone(); - let deleted = diesel::delete( - document_correspondents::table - .filter(document_correspondents::document_id.eq_any(&document_ids)) - .filter(document_correspondents::tenant_id.eq(tenant_id)) - .filter(document_correspondents::role.eq(role.as_str())) - .filter(not( - document_correspondents::correspondent_id.eq_any(maintained_ids) - )), - ) - .execute(conn)?; - removed += deleted; - } - - let mut new_rows = Vec::with_capacity(document_ids.len() * normalized_pairs.len()); - for doc_id in &document_ids { - for (correspondent_id, role) in &normalized_pairs { - new_rows.push(NewDocumentCorrespondent { - document_id: *doc_id, - correspondent_id: *correspondent_id, - role: role.clone(), - assigned_by: Some(user_id_val), - tenant_id, - }); - } - } - - let assigned = if new_rows.is_empty() { - 0 - } else { - diesel::insert_into(document_correspondents::table) - .values(&new_rows) - .on_conflict_do_nothing() - .execute(conn)? - }; - - if assigned > 0 || removed > 0 { - diesel::update( - documents::table - .filter(documents::id.eq_any(&document_ids)) - .filter(documents::tenant_id.eq(tenant_id)), - ) - .set(documents::updated_at.eq(Utc::now().naive_utc())) - .execute(conn)?; - } - - Ok((assigned, removed)) + Ok((assigned_total, 0)) } BulkCorrespondentAction::Remove => { - let mut removed = 0; - if !normalized_pairs.is_empty() { - let mut grouped: HashMap> = HashMap::new(); - for (correspondent_id, role) in &normalized_pairs { - grouped - .entry(role.clone()) - .or_default() - .push(*correspondent_id); - } - - for (role, ids) in grouped { - removed += diesel::delete( - document_correspondents::table - .filter(document_correspondents::document_id.eq_any(&document_ids)) - .filter(document_correspondents::tenant_id.eq(tenant_id)) - .filter(document_correspondents::role.eq(role.as_str())) - .filter(document_correspondents::correspondent_id.eq_any(&ids)), - ) - .execute(conn)?; - } + if correspondent_ids.is_empty() { + return Ok((0, 0)); } + let removed = diesel::delete( + document_correspondents::table + .filter(document_correspondents::document_id.eq_any(&document_ids)) + .filter(document_correspondents::tenant_id.eq(tenant_id)) + .filter( + document_correspondents::correspondent_id.eq_any(&correspondent_ids), + ), + ) + .execute(conn)?; + if removed > 0 { diesel::update( documents::table @@ -1875,24 +1824,12 @@ pub async fn bulk_assign_correspondents( pub async fn remove_correspondent( Path((document_id, correspondent_id)): Path<(Uuid, Uuid)>, - Query(query): Query, TenantScopedConn { mut conn, tenant_id, .. }: TenantScopedConn, ) -> AppResult { - let role = normalize_role(&query.role); - if role.is_empty() { - return Err(AppError::bad_request("role must not be empty")); - } - if !is_valid_correspondent_role(&role) { - return Err(AppError::bad_request(format!( - "invalid correspondent role '{role}'. Allowed roles: {}", - CORRESPONDENT_ROLES.join(", ") - ))); - } - let document: Document = documents::table .find(document_id) .filter(documents::tenant_id.eq(tenant_id)) @@ -1905,8 +1842,7 @@ pub async fn remove_correspondent( document_correspondents::table .filter(document_correspondents::document_id.eq(document_id)) .filter(document_correspondents::tenant_id.eq(tenant_id)) - .filter(document_correspondents::correspondent_id.eq(correspondent_id)) - .filter(document_correspondents::role.eq(&role)), + .filter(document_correspondents::correspondent_id.eq(correspondent_id)), ) .execute(&mut conn)?; @@ -2374,47 +2310,45 @@ fn assign_correspondents_internal( return Ok(0); } - let (normalized_pairs, _correspondent_ids, _roles) = - normalize_correspondent_assignments(assignments)?; + let ids = normalize_correspondent_assignments(assignments)?; - insert_document_correspondents(conn, tenant_id, document, user_id, &normalized_pairs) + insert_document_correspondents(conn, tenant_id, document.id, user_id, &ids) } fn insert_document_correspondents( conn: &mut PgConnection, tenant_id: Uuid, - document: &Document, + document_id: Uuid, user_id: Uuid, - normalized_pairs: &[(Uuid, String)], + correspondent_ids: &[Uuid], ) -> AppResult { - if normalized_pairs.is_empty() { + if correspondent_ids.is_empty() { return Ok(0); } - let mut correspondent_ids: Vec = normalized_pairs.iter().map(|(id, _)| *id).collect(); - correspondent_ids.sort_unstable(); - correspondent_ids.dedup(); + let mut unique_ids: Vec = correspondent_ids.to_vec(); + unique_ids.sort_unstable(); + unique_ids.dedup(); - if !correspondent_ids.is_empty() { + if !unique_ids.is_empty() { let existing: Vec = correspondents::table - .filter(correspondents::id.eq_any(&correspondent_ids)) + .filter(correspondents::id.eq_any(&unique_ids)) .filter(correspondents::tenant_id.eq(tenant_id)) .select(correspondents::id) .load(conn)?; - if existing.len() != correspondent_ids.len() { + if existing.len() != unique_ids.len() { return Err(AppError::bad_request( "one or more correspondents do not exist", )); } } - let new_rows: Vec = normalized_pairs - .iter() - .map(|(correspondent_id, role)| NewDocumentCorrespondent { - document_id: document.id, - correspondent_id: *correspondent_id, - role: role.clone(), + let new_rows: Vec = unique_ids + .into_iter() + .map(|correspondent_id| NewDocumentCorrespondent { + document_id, + correspondent_id, assigned_by: Some(user_id), tenant_id, }) @@ -2432,7 +2366,7 @@ fn insert_document_correspondents( if inserted > 0 { diesel::update( documents::table - .find(document.id) + .find(document_id) .filter(documents::tenant_id.eq(tenant_id)), ) .set(documents::updated_at.eq(Utc::now().naive_utc())) @@ -2490,7 +2424,6 @@ pub(crate) fn load_correspondents_for_documents( .filter(document_correspondents::document_id.eq_any(document_ids)) .order(( document_correspondents::document_id.asc(), - document_correspondents::role.asc(), document_correspondents::assigned_at.asc(), )) .load(conn)?; @@ -2502,7 +2435,6 @@ pub(crate) fn load_correspondents_for_documents( .push(DocumentCorrespondentResponse { id: correspondent.id, name: correspondent.name, - role: assignment.role, metadata: correspondent.metadata, assigned_at: to_iso(assignment.assigned_at), }); diff --git a/backend/src/routes/documents/correspondent_utils.rs b/backend/src/routes/documents/correspondent_utils.rs index 2d3a7d5..93f5615 100644 --- a/backend/src/routes/documents/correspondent_utils.rs +++ b/backend/src/routes/documents/correspondent_utils.rs @@ -6,56 +6,23 @@ use crate::error::{AppError, AppResult}; use super::CorrespondentAssignmentInput; -pub const CORRESPONDENT_ROLES: &[&str] = &["sender", "receiver", "other"]; - -pub fn normalize_role(value: &str) -> String { - value.trim().to_lowercase() -} - -pub fn is_valid_correspondent_role(role: &str) -> bool { - CORRESPONDENT_ROLES.iter().any(|allowed| *allowed == role) -} - pub fn normalize_correspondent_assignments( assignments: &[CorrespondentAssignmentInput], -) -> AppResult<(Vec<(Uuid, String)>, Vec, Vec)> { - let mut unique_pairs: HashSet<(Uuid, String)> = HashSet::new(); - let mut normalized_pairs: Vec<(Uuid, String)> = Vec::new(); - let mut role_set: HashSet = HashSet::new(); - let mut correspondent_ids: HashSet = HashSet::new(); +) -> AppResult> { + let mut unique_ids = HashSet::new(); + let mut result = Vec::new(); for assignment in assignments { - let role = normalize_role(&assignment.role); - if role.is_empty() { - return Err(AppError::bad_request("role must not be empty")); + if unique_ids.insert(assignment.correspondent_id) { + result.push(assignment.correspondent_id); } - if !is_valid_correspondent_role(&role) { - return Err(AppError::bad_request(format!( - "invalid correspondent role '{role}'. Allowed roles: {}", - CORRESPONDENT_ROLES.join(", ") - ))); - } - - if !unique_pairs.insert((assignment.correspondent_id, role.clone())) { - continue; - } - - normalized_pairs.push((assignment.correspondent_id, role.clone())); - role_set.insert(role); - correspondent_ids.insert(assignment.correspondent_id); } - if normalized_pairs.is_empty() { + if result.is_empty() { return Err(AppError::bad_request( - "assignments must contain at least one unique correspondent/role pair", + "assignments must contain at least one correspondent", )); } - let mut correspondents_vec: Vec = correspondent_ids.into_iter().collect(); - correspondents_vec.sort(); - - let mut roles_vec: Vec = role_set.into_iter().collect(); - roles_vec.sort(); - - Ok((normalized_pairs, correspondents_vec, roles_vec)) + Ok(result) } diff --git a/backend/src/schema.rs b/backend/src/schema.rs index 13a3217..84d7b4d 100644 --- a/backend/src/schema.rs +++ b/backend/src/schema.rs @@ -37,11 +37,9 @@ diesel::table! { } diesel::table! { - document_correspondents (document_id, correspondent_id, role) { + document_correspondents (document_id, correspondent_id) { document_id -> Uuid, correspondent_id -> Uuid, - #[max_length = 32] - role -> Varchar, assigned_at -> Timestamptz, assigned_by -> Nullable, tenant_id -> Uuid, diff --git a/backend/tests/correspondents_flow.rs b/backend/tests/correspondents_flow.rs new file mode 100644 index 0000000..887a9b9 --- /dev/null +++ b/backend/tests/correspondents_flow.rs @@ -0,0 +1,276 @@ +mod common; + +use anyhow::Result; +use common::{acquire_db_lock, body_to_vec, TestApp}; +use serde::Deserialize; +use serde_json::json; +use uuid::Uuid; + +#[derive(Deserialize)] +struct DocumentDetail { + document: DocumentSummary, +} + +#[derive(Deserialize)] +struct DocumentSummary { + id: Uuid, + title: String, + #[serde(default)] + correspondents: Vec, +} + +#[derive(Deserialize)] +struct DocumentCorrespondentSummary { + id: Uuid, + name: String, +} + +#[derive(Deserialize)] +struct CorrespondentSummary { + id: Uuid, +} + +#[derive(Deserialize)] +struct BulkCorrespondentResult { + assigned: usize, + removed: usize, +} + +struct TestContext { + app: TestApp, + token: String, + document_ids: Vec, + sender_id: Uuid, + receiver_id: Uuid, +} + +impl TestContext { + const SENDER_NAME: &'static str = "Acme Corp"; + const RECEIVER_NAME: &'static str = "Bank Ltd"; + + async fn new(prefix: &str) -> Result { + let app = TestApp::new().await?; + let username = format!("{prefix}_user"); + let password = format!("{prefix}_pw"); + app.insert_user(&username, &password, "admin").await?; + let token = app.login_token(&username, &password).await?; + + let first_id = + upload_document(&app, &token, &format!("{prefix}-one.txt"), b"letter one").await?; + let second_id = + upload_document(&app, &token, &format!("{prefix}-two.txt"), b"letter two").await?; + let sender_id = create_correspondent(&app, &token, Self::SENDER_NAME).await?; + let receiver_id = create_correspondent(&app, &token, Self::RECEIVER_NAME).await?; + + Ok(Self { + app, + token, + document_ids: vec![first_id, second_id], + sender_id, + receiver_id, + }) + } + + async fn assign(&self, correspondent_ids: &[Uuid]) -> Result { + self.assign_with_action(correspondent_ids, None).await + } + + async fn assign_with_action( + &self, + correspondent_ids: &[Uuid], + action: Option<&str>, + ) -> Result { + let assignments: Vec<_> = correspondent_ids + .iter() + .map(|id| json!({ "correspondent_id": id })) + .collect(); + + let mut payload = json!({ + "document_ids": self.document_ids, + "assignments": assignments, + }); + + if let Some(action) = action { + if let Some(obj) = payload.as_object_mut() { + obj.insert("action".to_string(), json!(action)); + } + } + + let response = self + .app + .post_json( + "/api/documents/bulk/correspondents", + &payload, + Some(&self.token), + ) + .await?; + assert!(response.status().is_success()); + let body = body_to_vec(response.into_body()).await?; + Ok(serde_json::from_slice(&body)?) + } + + async fn fetch_correspondents( + &self, + document_id: Uuid, + ) -> Result> { + let detail = fetch_document_detail(&self.app, &self.token, document_id).await?; + Ok(detail.document.correspondents) + } + + async fn create_correspondent(&self, name: &str) -> Result { + create_correspondent(&self.app, &self.token, name).await + } +} + +#[tokio::test] +async fn bulk_assign_correspondents_adds_new_links() -> Result<()> { + let _lock = acquire_db_lock().await; + let context = TestContext::new("corresp_add").await?; + + let result = context + .assign(&[context.sender_id, context.receiver_id]) + .await?; + assert_eq!(result.assigned, 4); + assert_eq!(result.removed, 0); + + for doc_id in &context.document_ids { + let correspondents = context.fetch_correspondents(*doc_id).await?; + let names: Vec<_> = correspondents + .iter() + .map(|entry| entry.name.as_str()) + .collect(); + assert!(names.contains(&TestContext::SENDER_NAME)); + assert!(names.contains(&TestContext::RECEIVER_NAME)); + let ids: Vec<_> = correspondents.iter().map(|entry| entry.id).collect(); + assert!(ids.contains(&context.sender_id)); + assert!(ids.contains(&context.receiver_id)); + } + + context.app.cleanup().await?; + Ok(()) +} + +#[tokio::test] +async fn bulk_assign_correspondents_is_idempotent() -> Result<()> { + let _lock = acquire_db_lock().await; + let context = TestContext::new("corresp_idempotent").await?; + + context + .assign(&[context.sender_id, context.receiver_id]) + .await?; + let repeat = context + .assign(&[context.sender_id, context.receiver_id]) + .await?; + assert_eq!(repeat.assigned, 0); + assert_eq!(repeat.removed, 0); + + context.app.cleanup().await?; + Ok(()) +} + +#[tokio::test] +async fn bulk_remove_correspondents_detaches_links() -> Result<()> { + let _lock = acquire_db_lock().await; + let context = TestContext::new("corresp_remove").await?; + + context + .assign(&[context.sender_id, context.receiver_id]) + .await?; + let removal = context + .assign_with_action(&[context.sender_id], Some("remove")) + .await?; + assert_eq!(removal.assigned, 0); + assert_eq!(removal.removed, 2); + + for doc_id in &context.document_ids { + let correspondents = context.fetch_correspondents(*doc_id).await?; + assert_eq!(correspondents.len(), 1); + let entry = &correspondents[0]; + assert_eq!(entry.id, context.receiver_id); + assert_eq!(entry.name, TestContext::RECEIVER_NAME); + } + + context.app.cleanup().await?; + Ok(()) +} + +#[tokio::test] +async fn bulk_assign_correspondents_appends_new_entries() -> Result<()> { + let _lock = acquire_db_lock().await; + let context = TestContext::new("corresp_append").await?; + + context + .assign(&[context.sender_id, context.receiver_id]) + .await?; + context + .assign_with_action(&[context.sender_id], Some("remove")) + .await?; + + let charlie_name = "Charlie"; + let charlie_id = context.create_correspondent(charlie_name).await?; + let add_result = context.assign(&[charlie_id]).await?; + assert_eq!(add_result.assigned, 2); + assert_eq!(add_result.removed, 0); + + for doc_id in &context.document_ids { + let correspondents = context.fetch_correspondents(*doc_id).await?; + assert_eq!(correspondents.len(), 2); + let ids: Vec<_> = correspondents.iter().map(|entry| entry.id).collect(); + assert!(ids.contains(&context.receiver_id)); + assert!(ids.contains(&charlie_id)); + let names: Vec<_> = correspondents + .iter() + .map(|entry| entry.name.as_str()) + .collect(); + assert!(names.contains(&TestContext::RECEIVER_NAME)); + assert!(names.contains(&charlie_name)); + } + + context.app.cleanup().await?; + Ok(()) +} + +async fn upload_document( + app: &TestApp, + token: &str, + filename: &str, + contents: &[u8], +) -> Result { + let response = app + .upload_document( + "/api/documents", + filename, + "text/plain", + contents, + None, + token, + ) + .await?; + assert!(response.status().is_success()); + let body = body_to_vec(response.into_body()).await?; + let detail: DocumentDetail = serde_json::from_slice(&body)?; + Ok(detail.document.id) +} + +async fn create_correspondent(app: &TestApp, token: &str, name: &str) -> Result { + let response = app + .post_json("/api/correspondents", &json!({ "name": name }), Some(token)) + .await?; + assert!(response.status().is_success()); + let body = body_to_vec(response.into_body()).await?; + let summary: CorrespondentSummary = serde_json::from_slice(&body)?; + Ok(summary.id) +} + +async fn fetch_document_detail( + app: &TestApp, + token: &str, + document_id: Uuid, +) -> Result { + let response = app + .get(&format!("/api/documents/{document_id}"), Some(token)) + .await?; + assert!(response.status().is_success()); + let body = body_to_vec(response.into_body()).await?; + Ok(serde_json::from_slice(&body)?) +} diff --git a/backend/tests/documents_flow.rs b/backend/tests/documents_flow.rs index 845dfe1..db051c1 100644 --- a/backend/tests/documents_flow.rs +++ b/backend/tests/documents_flow.rs @@ -83,19 +83,8 @@ struct TagSummary { #[derive(Deserialize)] struct DocumentCorrespondentInfo { - name: String, - role: String, -} - -#[derive(Deserialize)] -struct CorrespondentSummary { id: Uuid, -} - -#[derive(Deserialize)] -struct BulkCorrespondentResult { - assigned: usize, - removed: usize, + name: String, } #[derive(Deserialize)] @@ -809,300 +798,6 @@ async fn bulk_update_tags_for_selection() -> Result<()> { Ok(()) } -#[tokio::test] -async fn bulk_assign_correspondents_to_selection() -> Result<()> { - let _lock = acquire_db_lock().await; - let app = TestApp::new().await?; - - let password = "bulkcorresp"; - app.insert_user("corra", password, "admin").await?; - let token = app.login_token("corra", password).await?; - - let first = app - .upload_document( - "/api/documents", - "letter-one.txt", - "text/plain", - b"letter one", - None, - &token, - ) - .await?; - { - let status = first.status(); - assert!( - status == StatusCode::OK - || status == StatusCode::CREATED - || status == StatusCode::NO_CONTENT - ); - } - let first_body = body_to_vec(first.into_body()).await?; - let first_detail: DocumentDetail = serde_json::from_slice(&first_body)?; - - let second = app - .upload_document( - "/api/documents", - "letter-two.txt", - "text/plain", - b"letter two", - None, - &token, - ) - .await?; - { - let status = second.status(); - assert!( - status == StatusCode::OK - || status == StatusCode::CREATED - || status == StatusCode::NO_CONTENT - ); - } - let second_body = body_to_vec(second.into_body()).await?; - let second_detail: DocumentDetail = serde_json::from_slice(&second_body)?; - - let sender = app - .post_json( - "/api/correspondents", - &serde_json::json!({ "name": "Acme Corp" }), - Some(&token), - ) - .await?; - { - let status = sender.status(); - assert!( - status == StatusCode::OK - || status == StatusCode::CREATED - || status == StatusCode::NO_CONTENT - ); - } - let sender_body = body_to_vec(sender.into_body()).await?; - let sender_summary: CorrespondentSummary = serde_json::from_slice(&sender_body)?; - - let receiver = app - .post_json( - "/api/correspondents", - &serde_json::json!({ "name": "Bank Ltd" }), - Some(&token), - ) - .await?; - { - let status = receiver.status(); - assert!( - status == StatusCode::OK - || status == StatusCode::CREATED - || status == StatusCode::NO_CONTENT - ); - } - let receiver_body = body_to_vec(receiver.into_body()).await?; - let receiver_summary: CorrespondentSummary = serde_json::from_slice(&receiver_body)?; - - let assign_payload = serde_json::json!({ - "document_ids": [ - first_detail.document.id, - second_detail.document.id - ], - "assignments": [ - { - "correspondent_id": sender_summary.id, - "role": "sender" - }, - { - "correspondent_id": receiver_summary.id, - "role": "receiver" - } - ] - }); - - let assign_resp = app - .post_json( - "/api/documents/bulk/correspondents", - &assign_payload, - Some(&token), - ) - .await?; - { - let status = assign_resp.status(); - assert!( - status == StatusCode::OK - || status == StatusCode::CREATED - || status == StatusCode::NO_CONTENT - ); - } - let assign_body = body_to_vec(assign_resp.into_body()).await?; - let assign_result: BulkCorrespondentResult = serde_json::from_slice(&assign_body)?; - assert_eq!(assign_result.assigned, 4); - assert_eq!(assign_result.removed, 0); - - for doc_id in [first_detail.document.id, second_detail.document.id] { - let refreshed = app - .get(&format!("/api/documents/{doc_id}"), Some(&token)) - .await?; - { - let status = refreshed.status(); - assert!(status == StatusCode::OK || status == StatusCode::CREATED); - } - let refreshed_body = body_to_vec(refreshed.into_body()).await?; - let detail: DocumentDetail = serde_json::from_slice(&refreshed_body)?; - assert_eq!(detail.document.correspondents.len(), 2); - assert!(detail - .document - .correspondents - .iter() - .any(|entry| entry.role == "sender" && entry.name == "Acme Corp")); - assert!(detail - .document - .correspondents - .iter() - .any(|entry| entry.role == "receiver" && entry.name == "Bank Ltd")); - } - - let duplicate_resp = app - .post_json( - "/api/documents/bulk/correspondents", - &assign_payload, - Some(&token), - ) - .await?; - { - let status = duplicate_resp.status(); - assert!( - status == StatusCode::OK - || status == StatusCode::CREATED - || status == StatusCode::NO_CONTENT - ); - } - let duplicate_body = body_to_vec(duplicate_resp.into_body()).await?; - let duplicate_result: BulkCorrespondentResult = serde_json::from_slice(&duplicate_body)?; - assert_eq!(duplicate_result.assigned, 0); - assert_eq!(duplicate_result.removed, 0); - - let replacement = app - .post_json( - "/api/correspondents", - &serde_json::json!({ "name": "Charlie" }), - Some(&token), - ) - .await?; - { - let status = replacement.status(); - assert!( - status == StatusCode::OK - || status == StatusCode::CREATED - || status == StatusCode::NO_CONTENT - ); - } - let replacement_body = body_to_vec(replacement.into_body()).await?; - let replacement_summary: CorrespondentSummary = serde_json::from_slice(&replacement_body)?; - - let replace_payload = serde_json::json!({ - "document_ids": [ - first_detail.document.id, - second_detail.document.id - ], - "assignments": [ - { - "correspondent_id": replacement_summary.id, - "role": "sender" - } - ] - }); - - let replace_resp = app - .post_json( - "/api/documents/bulk/correspondents", - &replace_payload, - Some(&token), - ) - .await?; - { - let status = replace_resp.status(); - assert!( - status == StatusCode::OK - || status == StatusCode::CREATED - || status == StatusCode::NO_CONTENT - ); - } - let replace_body = body_to_vec(replace_resp.into_body()).await?; - let replace_result: BulkCorrespondentResult = serde_json::from_slice(&replace_body)?; - assert_eq!(replace_result.assigned, 2); - assert_eq!(replace_result.removed, 2); - - for doc_id in [first_detail.document.id, second_detail.document.id] { - let refreshed = app - .get(&format!("/api/documents/{doc_id}"), Some(&token)) - .await?; - let refreshed_body = body_to_vec(refreshed.into_body()).await?; - let detail: DocumentDetail = serde_json::from_slice(&refreshed_body)?; - assert_eq!(detail.document.correspondents.len(), 2); - assert!(detail - .document - .correspondents - .iter() - .any(|entry| entry.role == "sender" && entry.name == "Charlie")); - assert!(detail - .document - .correspondents - .iter() - .any(|entry| entry.role == "receiver" && entry.name == "Bank Ltd")); - } - - let remove_payload = serde_json::json!({ - "document_ids": [ - first_detail.document.id, - second_detail.document.id - ], - "assignments": [ - { - "correspondent_id": receiver_summary.id, - "role": "receiver" - } - ], - "action": "remove" - }); - - let remove_resp = app - .post_json( - "/api/documents/bulk/correspondents", - &remove_payload, - Some(&token), - ) - .await?; - { - let status = remove_resp.status(); - assert!( - status == StatusCode::OK - || status == StatusCode::CREATED - || status == StatusCode::NO_CONTENT - ); - } - let remove_body = body_to_vec(remove_resp.into_body()).await?; - let remove_result: BulkCorrespondentResult = serde_json::from_slice(&remove_body)?; - assert_eq!(remove_result.assigned, 0); - assert_eq!(remove_result.removed, 2); - - for doc_id in [first_detail.document.id, second_detail.document.id] { - let refreshed = app - .get(&format!("/api/documents/{doc_id}"), Some(&token)) - .await?; - let refreshed_body = body_to_vec(refreshed.into_body()).await?; - let detail: DocumentDetail = serde_json::from_slice(&refreshed_body)?; - assert_eq!(detail.document.correspondents.len(), 1); - assert!(detail - .document - .correspondents - .iter() - .any(|entry| entry.role == "sender" && entry.name == "Charlie")); - assert!(!detail - .document - .correspondents - .iter() - .any(|entry| entry.role == "receiver")); - } - - app.cleanup().await?; - Ok(()) -} - #[tokio::test] async fn bulk_reanalyze_selected_documents() -> Result<()> { let _lock = acquire_db_lock().await; @@ -1803,6 +1498,7 @@ async fn list_document_versions_and_fetch_detail() -> Result<()> { let versions: Vec = serde_json::from_slice(&list_body)?; assert_eq!(versions.len(), 1); assert_eq!(versions[0].id, version_id); + assert_eq!(versions[0].version_number, 1); let detail_resp = app .get( diff --git a/frontend/src/constants/correspondents.js b/frontend/src/constants/correspondents.js deleted file mode 100644 index eb9f496..0000000 --- a/frontend/src/constants/correspondents.js +++ /dev/null @@ -1,3 +0,0 @@ -export const CORRESPONDENT_ROLES = ['sender', 'receiver', 'other']; - -export default CORRESPONDENT_ROLES; diff --git a/frontend/src/correspondents/CorrespondentsPanel.jsx b/frontend/src/correspondents/CorrespondentsPanel.jsx index b1ad5c8..db383b8 100644 --- a/frontend/src/correspondents/CorrespondentsPanel.jsx +++ b/frontend/src/correspondents/CorrespondentsPanel.jsx @@ -107,14 +107,7 @@ function CorrespondentsPanel({ return '0'; } const total = typeof usage.total === 'number' ? usage.total : 0; - const entries = usage.by_role ? Object.entries(usage.by_role) : []; - if (!entries.length) { - return total.toString(); - } - const roleSummary = entries - .map(([role, count]) => `${role}: ${count}`) - .join(', '); - return `${total} (${roleSummary})`; + return total.toString(); }, []); return ( diff --git a/frontend/src/detail/DetailPanel.jsx b/frontend/src/detail/DetailPanel.jsx index 3e257b4..d6770c0 100644 --- a/frontend/src/detail/DetailPanel.jsx +++ b/frontend/src/detail/DetailPanel.jsx @@ -14,13 +14,10 @@ import { getTagColorStyle } from '../utils/colors'; import { formatFileSize } from '../utils/format'; import { resolveDocumentAssetUrl, createAssetView } from '../asset_manager'; import { useAssetNavigator } from '../hooks/useAssetNavigator'; -import { CORRESPONDENT_ROLES } from '../constants/correspondents'; import PreviewZoomOverlay from './PreviewZoomOverlay'; const MAX_PREVIEW_STACK_ITEMS = 15; -const normalizeRole = (role) => (role || '').toLowerCase(); - const derivePreviewOrientation = (metadata) => { const width = Number(metadata?.width); const height = Number(metadata?.height); @@ -30,56 +27,41 @@ const derivePreviewOrientation = (metadata) => { return 'landscape'; }; -const formatRoleLabel = (role) => { - const normalized = normalizeRole(role); - if (!normalized) return 'Other'; - return normalized.charAt(0).toUpperCase() + normalized.slice(1); -}; - -const compareCorrespondents = (a, b) => { - const indexA = CORRESPONDENT_ROLES.indexOf(a.role); - const indexB = CORRESPONDENT_ROLES.indexOf(b.role); - const rankedA = indexA === -1 ? Number.MAX_SAFE_INTEGER : indexA; - const rankedB = indexB === -1 ? Number.MAX_SAFE_INTEGER : indexB; - if (rankedA !== rankedB) { - return rankedA - rankedB; - } - return (a.name || '').localeCompare(b.name || ''); -}; - const sortCorrespondents = (entries = []) => entries .map((entry) => ({ id: entry.id, name: entry.name || '', - role: normalizeRole(entry.role), + count: entry.count, })) - .sort(compareCorrespondents); + .sort((a, b) => (a.name || '').localeCompare(b.name || '')); const CorrespondentPills = ({ entries = [], onRemove, showCount = false }) => (
{entries.length ? ( - entries.map((entry) => ( - - - {formatRoleLabel(entry.role)} - - {entry.name} - {showCount && entry.count ? ` (${entry.count})` : ''} + entries.map((entry, index) => { + const key = entry.id ?? `${entry.name}-${index}`; + return ( + + + + {entry.name} + {showCount && entry.count ? ` (${entry.count})` : ''} + + {onRemove ? ( + + ) : null} - {onRemove ? ( - - ) : null} - - )) + ); + }) ) : ( No correspondents yet. )} @@ -174,11 +156,9 @@ const CorrespondentSection = ({ event.preventDefault(); const form = event.currentTarget; const nameInput = form.elements.correspondent; - const roleSelect = form.elements.role; const value = nameInput.value.trim(); - const role = roleSelect.value; if (!value) return; - onAdd({ name: value, role, input: nameInput }); + onAdd({ name: value, input: nameInput }); form.reset(); }} > @@ -187,13 +167,6 @@ const CorrespondentSection = ({ placeholder={addPlaceholder} list={datalistId} /> - {datalistId ? ( @@ -705,17 +678,14 @@ const DetailPanel = ({ if (!doc?.id) return; (doc.correspondents || []).forEach((entry) => { if (!entry?.id) return; - const normalizedRole = normalizeRole(entry.role); - const key = `${entry.id}:${normalizedRole}`; - if (!map.has(key)) { - map.set(key, { + if (!map.has(entry.id)) { + map.set(entry.id, { id: entry.id, name: entry.name || '', - role: normalizedRole, documentIds: new Set(), }); } - map.get(key).documentIds.add(doc.id); + map.get(entry.id).documentIds.add(doc.id); }); }); @@ -723,23 +693,20 @@ const DetailPanel = ({ .map((entry) => ({ id: entry.id, name: entry.name, - role: entry.role, documentIds: [...entry.documentIds], count: entry.documentIds.size, })) - .sort(compareCorrespondents); + .sort((a, b) => (a.name || '').localeCompare(b.name || '')); }, [selectedDocuments]); const handleBulkCorrespondentRemove = useCallback( (entry) => { if (!entry?.id) return; - const normalizedRole = normalizeRole(entry.role); if (onBulkCorrespondentRemove) { return onBulkCorrespondentRemove({ assignments: [ { correspondent_id: entry.id, - role: normalizedRole, }, ], documentIds: entry.documentIds, @@ -750,11 +717,7 @@ const DetailPanel = ({ const targets = entry.documentIds && entry.documentIds.length ? entry.documentIds : selectedDocuments - .filter((doc) => - (doc.correspondents || []).some( - (item) => item.id === entry.id && normalizeRole(item.role) === normalizedRole, - ), - ) + .filter((doc) => (doc.correspondents || []).some((item) => item.id === entry.id)) .map((doc) => doc.id); return Promise.all( @@ -762,12 +725,11 @@ const DetailPanel = ({ onCorrespondentRemove({ documentId, correspondentId: entry.id, - role: normalizedRole, }), ), ).catch(() => {}); }, - [bulkCorrespondents, onBulkCorrespondentRemove, onCorrespondentRemove, selectedDocuments], + [onBulkCorrespondentRemove, onCorrespondentRemove, selectedDocuments], ); const openZoomPreview = useCallback((config) => { @@ -1158,14 +1120,12 @@ const DetailPanel = ({ onCorrespondentRemove?.({ documentId: singleDoc.id, correspondentId: entry.id, - role: entry.role, }) } - onAdd={({ name, role, input }) => + onAdd={({ name, input }) => onCorrespondentAdd?.({ document: singleDoc, name, - role, input, }) } @@ -1310,8 +1270,8 @@ const DetailPanel = ({ title="Correspondents" entries={bulkCorrespondents} onRemove={handleBulkCorrespondentRemove} - onAdd={({ name, role, input }) => - onBulkCorrespondentAdd?.({ name, role, input }) + onAdd={({ name, input }) => + onBulkCorrespondentAdd?.({ name, input }) } addPlaceholder="Add correspondent to selection" datalistId="correspondent-catalog-bulk" diff --git a/frontend/src/documents/DocumentsTable.jsx b/frontend/src/documents/DocumentsTable.jsx index 00ea171..c1fef76 100644 --- a/frontend/src/documents/DocumentsTable.jsx +++ b/frontend/src/documents/DocumentsTable.jsx @@ -1,7 +1,18 @@ import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { getAssetFromVersion, resolveDocumentAssetUrl, createAssetView } from '../asset_manager'; import { getTagColorStyle } from '../utils/colors'; -import { DownloadIcon, EditIcon, ViewListIcon, ViewGridIcon, FolderIcon, TrashIcon } from '../ui/icons'; +import { + DownloadIcon, + EditIcon, + ViewListIcon, + ViewGridIcon, + FolderIcon, + TrashIcon, + RefreshIcon, + FolderPlusIcon, + MinusVerticalIcon, + ArrowUpIcon, +} from '../ui/icons'; const TAG_MIME_TYPES = ['application/x-papercrate-tag', 'text/papercrate-tag']; const DEFAULT_GRID_ICON_SIZE = 144; @@ -39,7 +50,6 @@ const resolveCorrespondents = (doc) => { results.push({ id, name, - role: entry.role || null, key: id ?? `${name}-${index}`, }); }); @@ -1048,3 +1058,127 @@ const DocumentsTable = ({ export default DocumentsTable; export { DocumentThumbnailImage }; + +export const createDocumentsTableHeaderActions = ({ + viewMode, + onViewModeChange, + onRequestCreateFolder, + creatingFolder, + onRefresh, + onShowSkeuoWorkspace, +}) => { + const isGridView = viewMode === 'grid'; + + return ( + <> +
+ + +
+ + + + + + ); +}; + +export const createDocumentsSurface = ({ + tableProps, + parentBreadcrumb, + onNavigateParent, + renderSidebarToggle, +}) => { + const { + currentFolderName, + searchResults, + viewMode, + onViewModeChange, + onRequestCreateFolder, + creatingFolder, + onRefresh, + onShowSkeuoWorkspace, + } = tableProps; + + const title = Array.isArray(searchResults) ? 'Search results' : currentFolderName; + const subtitle = Array.isArray(searchResults) + ? `${searchResults.length} matching document${searchResults.length === 1 ? '' : 's'}` + : null; + + const actions = createDocumentsTableHeaderActions({ + viewMode, + onViewModeChange, + onRequestCreateFolder, + creatingFolder, + onRefresh, + onShowSkeuoWorkspace, + }); + + const sidebarToggle = renderSidebarToggle ? renderSidebarToggle() : null; + const parentControl = parentBreadcrumb + ? ( + + ) + : null; + const leading = sidebarToggle || parentControl + ? ( + <> + {sidebarToggle} + {parentControl} + + ) + : null; + + return { + key: 'documents', + variant: 'documents', + header: { title, subtitle, leading, actions }, + content: , + supportsDetail: true, + }; +}; diff --git a/frontend/src/index.jsx b/frontend/src/index.jsx index 4bd889a..8ba8bdd 100644 --- a/frontend/src/index.jsx +++ b/frontend/src/index.jsx @@ -25,29 +25,15 @@ import { import './styles.css'; import AssetManager, { getAssetFromVersion, resolveDocumentAssetUrl, createAssetView } from './asset_manager'; import useApiError from './hooks/useApiError'; -import SkeuomorphicWorkspace from './skeuomorphic_ws'; -import PreviewWorkspace from './preview/PreviewWorkspace'; import DetailPanel from './detail/DetailPanel'; import TagsPanel from './tags/TagsPanel'; import CorrespondentsPanel from './correspondents/CorrespondentsPanel'; -import { CORRESPONDENT_ROLES } from './constants/correspondents'; import TagManager from './tag_manager'; import Sidebar from './sidebar/Sidebar'; -import { - ChevronsRightIcon, - ChevronsLeftIcon, - ViewListIcon, - ViewGridIcon, - FolderPlusIcon, - RefreshIcon, - ArrowUpIcon, - MinusVerticalIcon, - DownloadIcon, - TextScanIcon, - AnalyzeIcon, - CloseIcon, -} from './ui/icons'; -import DocumentsTable from './documents/DocumentsTable'; +import { ChevronsRightIcon } from './ui/icons'; +import { createDocumentsSurface } from './documents/DocumentsTable'; +import { createPreviewSurface } from './preview/PreviewWorkspace'; +import { createSkeuomorphicSurface } from './skeuomorphic_ws'; import { AppShellContext, useAppShell } from './appShellContext'; const runtimeApiBase = @@ -416,6 +402,33 @@ const DocumentsLayout = ({ sidebarProps, children, sidebarCollapsed }) => ( ); +const MainContentShell = ({ + className, + header = {}, + sidebarToggleButton = null, + children, +}) => { + const { title = '', subtitle = null, leading = null, actions = null } = header || {}; + + return ( +
+
+
+ {sidebarToggleButton} + {leading} +

+ {title} + {subtitle ? {subtitle} : null} +

+
+ {actions} +
+
+ {children} +
+ ); +}; + const AppLayout = () => { const navigate = useNavigate(); const location = useLocation(); @@ -1856,15 +1869,15 @@ const AppLayout = () => { ); async function handleDocumentCorrespondentAttach( - { documentId, correspondentId, role }, + { documentId, correspondentId }, { notify = true, refresh = true } = {}, ) { - if (!documentId || !correspondentId || !role) { - throw new Error('Missing document, correspondent or role.'); + if (!documentId || !correspondentId) { + throw new Error('Missing document or correspondent.'); } try { await api.post(`/documents/${documentId}/correspondents`, { - assignments: [{ correspondent_id: correspondentId, role }], + assignments: [{ correspondent_id: correspondentId }], replace: false, }); if (refresh) { @@ -1882,16 +1895,14 @@ const AppLayout = () => { } async function handleCorrespondentRemove( - { documentId, correspondentId, role }, + { documentId, correspondentId }, { notify = true, refresh = true } = {}, ) { - if (!documentId || !correspondentId || !role) { - throw new Error('Missing document, correspondent or role.'); + if (!documentId || !correspondentId) { + throw new Error('Missing document or correspondent.'); } try { - await api.delete(`/documents/${documentId}/correspondents/${correspondentId}`, { - params: { role }, - }); + await api.delete(`/documents/${documentId}/correspondents/${correspondentId}`); if (refresh) { await refreshCurrentFolder(); } @@ -1907,7 +1918,7 @@ const AppLayout = () => { } const handleCorrespondentAdd = useCallback( - async ({ document, name, role, input }) => { + async ({ document, name, input }) => { if (!document?.id) { throw new Error('Missing document for correspondent assignment.'); } @@ -1916,11 +1927,6 @@ const AppLayout = () => { setStatusMessage('Correspondent name is required.', 'error'); return; } - const normalizedRole = (role || '').trim().toLowerCase(); - if (!CORRESPONDENT_ROLES.includes(normalizedRole)) { - setStatusMessage('Select a valid correspondent role.', 'error'); - return; - } let target = correspondentLookupByName.get(trimmed.toLowerCase()) || null; if (!target) { @@ -1940,7 +1946,6 @@ const AppLayout = () => { await handleDocumentCorrespondentAttach({ documentId: document.id, correspondentId: target.id, - role: normalizedRole, }); if (input) { input.value = ''; @@ -2228,7 +2233,7 @@ const AppLayout = () => { }, [selectedFolder, ensureFolderData, applySelectedFolder, notifyApiError]); const handleBulkCorrespondentAdd = useCallback( - async ({ name, role, input }) => { + async ({ name, input }) => { const trimmed = (name || '').trim(); if (!trimmed) { setStatusMessage('Correspondent name is required.', 'error'); @@ -2238,11 +2243,6 @@ const AppLayout = () => { setStatusMessage('Select documents before assigning correspondents.', 'error'); return; } - const normalizedRole = (role || '').trim().toLowerCase(); - if (!CORRESPONDENT_ROLES.includes(normalizedRole)) { - setStatusMessage('Select a valid correspondent role.', 'error'); - return; - } let target = correspondentLookupByName.get(trimmed.toLowerCase()) || null; if (!target) { @@ -2263,7 +2263,6 @@ const AppLayout = () => { assignments: [ { correspondent_id: target.id, - role: normalizedRole, }, ], action: 'add', @@ -2318,7 +2317,6 @@ const AppLayout = () => { const normalizedAssignments = assignments.map((entry) => ({ correspondent_id: entry.correspondent_id, - role: (entry.role || '').toLowerCase(), })); const response = await api.post('/documents/bulk/correspondents', { @@ -5414,7 +5412,6 @@ const DocumentsRoute = () => { skeuoWorkspaceProps, openTagsModal, openCorrespondentsModal, - exitSkeuoWorkspace, previewWorkspaceDocument, previewWorkspaceEntry, closeDocumentPreview, @@ -5440,281 +5437,156 @@ const DocumentsRoute = () => { [sidebarProps, openTagsModal, openCorrespondentsModal, collapseSidebar], ); - const { - currentFolderName, - viewMode, - onViewModeChange, - onRequestCreateFolder, - creatingFolder, - onRefresh, - onShowSkeuoWorkspace, - searchResults, - breadcrumbs, - } = documentsTableProps; + const breadcrumbs = documentsTableProps?.breadcrumbs || null; + const parentBreadcrumb = useMemo(() => { + if (!Array.isArray(breadcrumbs) || breadcrumbs.length <= 1) { + return null; + } + return breadcrumbs[breadcrumbs.length - 2]; + }, [breadcrumbs]); + + const handleNavigateParent = useCallback(() => { + if (!parentBreadcrumb) { + return; + } + const target = parentBreadcrumb.id === 'root' + ? '/documents' + : `/documents/folder/${parentBreadcrumb.id}`; + navigate(target); + }, [navigate, parentBreadcrumb]); const isWorkspace = workspaceMode === 'skeuo'; const isPreviewWorkspace = Boolean(previewWorkspaceDocument); const showPreviewWorkspace = !isWorkspace && isPreviewWorkspace; - const isGridView = viewMode === 'grid'; - const showingSearchResults = Array.isArray(searchResults); - let headerTitle; - let headerSubtitle = null; - if (showPreviewWorkspace) { - const previewTitle = previewWorkspaceDocument?.title - || previewWorkspaceDocument?.original_name - || 'Document preview'; - headerTitle = previewTitle; - } else if (showingSearchResults) { - headerTitle = 'Search results'; - headerSubtitle = `${searchResults.length} matching document${searchResults.length === 1 ? '' : 's'}`; - } else { - headerTitle = currentFolderName; - } - const parentBreadcrumb = breadcrumbs && breadcrumbs.length > 1 ? breadcrumbs[breadcrumbs.length - 2] : null; - const handleNavigateParent = parentBreadcrumb - ? () => { - const target = parentBreadcrumb.id === 'root' - ? '/documents' - : `/documents/folder/${parentBreadcrumb.id}`; - navigate(target); - } - : null; - const detailHasContent = !isWorkspace && !showPreviewWorkspace - && detailPanelProps.selectedDocuments?.length > 0; - const toggleSidebar = sidebarCollapsed ? expandSidebar : collapseSidebar; - const ToggleIcon = sidebarCollapsed ? ChevronsRightIcon : ChevronsLeftIcon; - const toggleLabel = sidebarCollapsed ? 'Expand sidebar' : 'Collapse sidebar'; - const mainContentClass = (() => { - if (isWorkspace) { - return 'main-content main-content--workspace'; - } - if (showPreviewWorkspace) { - return 'main-content main-content--preview'; - } - return `main-content main-content--documents${ - detailHasContent ? ' main-content--has-detail' : '' - }`; - })(); - - const bodyClass = (() => { - if (isWorkspace) { - return 'main-content__body main-content__body--workspace'; - } - if (showPreviewWorkspace) { - return 'main-content__body main-content__body--preview'; - } - return `main-content__body main-content__body--documents${ - detailHasContent ? ' main-content__body--has-detail' : '' - }`; - })(); - - const previewDownloadHref = useMemo(() => { - if (!previewWorkspaceDocument?.current_version?.download_path) { + const renderSidebarToggle = useCallback(() => { + if (!sidebarCollapsed) { return null; } - return resolveApiPath(previewWorkspaceDocument.current_version.download_path); - }, [previewWorkspaceDocument, resolveApiPath]); + return ( + + ); + }, [sidebarCollapsed, expandSidebar]); - const previewHasOcr = useMemo( - () => Boolean( - previewWorkspaceDocument && getDocumentAsset(previewWorkspaceDocument, 'ocr-text'), - ), - [previewWorkspaceDocument, getDocumentAsset], - ); - - const handlePreviewOcr = useCallback(async () => { - if (!previewWorkspaceDocument) { - return; - } - const asset = getDocumentAsset(previewWorkspaceDocument, 'ocr-text'); - if (!asset) { - return; - } - try { - const ensured = await ensureAssetUrl(previewWorkspaceDocument.id, asset, { force: false }); - const entry = ensured || asset; - const url = entry?.url - || resolveDocumentAssetUrl(previewWorkspaceDocument, 'ocr-text', { - ensureAssetUrl, - getAsset: getDocumentAsset, - }); - if (!url) { - throw new Error('OCR text URL unavailable.'); - } - window.open(url, '_blank', 'noopener,noreferrer'); - } catch (error) { - notifyApiError(error, 'Unable to open OCR text.'); + const documentsSurface = useMemo(() => { + if (!documentsTableProps) { + return null; } + return createDocumentsSurface({ + tableProps: documentsTableProps, + parentBreadcrumb, + onNavigateParent: parentBreadcrumb ? handleNavigateParent : null, + renderSidebarToggle, + }); }, [ + documentsTableProps, + parentBreadcrumb, + handleNavigateParent, + renderSidebarToggle, + ]); + + const previewSurface = useMemo(() => { + if (!showPreviewWorkspace || !previewWorkspaceDocument) { + return null; + } + return createPreviewSurface({ + document: previewWorkspaceDocument, + previewEntry: previewWorkspaceEntry, + ensureAssetUrl, + getDocumentAsset, + resolveApiPath, + notifyApiError, + onRegenerate: handleThumbnailRegeneration, + onClose: closeDocumentPreview, + renderSidebarToggle, + }); + }, [ + showPreviewWorkspace, previewWorkspaceDocument, + previewWorkspaceEntry, ensureAssetUrl, getDocumentAsset, + resolveApiPath, notifyApiError, + handleThumbnailRegeneration, + closeDocumentPreview, + renderSidebarToggle, ]); + const workspaceSurface = useMemo(() => { + if (!isWorkspace) { + return null; + } + return createSkeuomorphicSurface({ + workspaceProps: skeuoWorkspaceProps, + renderSidebarToggle, + }); + }, [isWorkspace, skeuoWorkspaceProps, renderSidebarToggle]); + + const surface = showPreviewWorkspace + ? previewSurface + : isWorkspace + ? workspaceSurface + : documentsSurface; + + if (!surface) { + return ( + +
+
+
+ + ); + } + + const supportsDetail = Boolean(surface.supportsDetail); + const detailCount = detailPanelProps.selectedDocuments?.length || 0; + const showDetailPanel = supportsDetail && detailCount > 0; + const variant = surface.variant || 'documents'; + + const mainContentClass = `main-content main-content--${variant}${ + showDetailPanel ? ' main-content--has-detail' : '' + }`; + const bodyClass = `main-content__body main-content__body--${variant}${ + showDetailPanel ? ' main-content__body--has-detail' : '' + }`; + + const header = surface.header || null; + return (
-
-
- {sidebarCollapsed ? ( - - ) : null} - {parentBreadcrumb && !showPreviewWorkspace ? ( - - ) : null} -

- {headerTitle} - {headerSubtitle ? ( - {headerSubtitle} - ) : null} -

-
- {isWorkspace ? ( - <> - - - - ) : showPreviewWorkspace ? ( - <> - {previewDownloadHref ? ( - - - + {header ? ( +
+
+ {header.leading} +

+ {header.title} + {header.subtitle ? ( + {header.subtitle} ) : null} - {previewHasOcr ? ( - - ) : null} - - - - ) : ( - <> -
- - -
- - - - - - )} +

+
+ {header.actions} +
-
-
- {isWorkspace ? ( - - ) : showPreviewWorkspace ? ( - - ) : ( - - )} -
- {!isWorkspace && !showPreviewWorkspace && detailHasContent ? ( - ) : null} +
{surface.content}
+ {showDetailPanel ? : null}
); diff --git a/frontend/src/preview/PreviewWorkspace.jsx b/frontend/src/preview/PreviewWorkspace.jsx index e7cbf8f..dc9727a 100644 --- a/frontend/src/preview/PreviewWorkspace.jsx +++ b/frontend/src/preview/PreviewWorkspace.jsx @@ -1,5 +1,7 @@ import React from 'react'; +import { resolveDocumentAssetUrl } from '../asset_manager'; import { formatFileSize } from '../utils/format'; +import { DownloadIcon, TextScanIcon, AnalyzeIcon, CloseIcon } from '../ui/icons'; const PreviewWorkspace = ({ document, @@ -42,3 +44,133 @@ const PreviewWorkspace = ({ }; export default PreviewWorkspace; + +export const createPreviewWorkspaceHeaderActions = ({ + document, + ensureAssetUrl, + getDocumentAsset, + resolveApiPath, + notifyApiError, + onRegenerate, + onClose, +}) => { + if (!document) { + return null; + } + + const downloadHref = document.current_version?.download_path + ? resolveApiPath(document.current_version.download_path) + : null; + + const ocrAsset = getDocumentAsset(document, 'ocr-text'); + const hasOcr = Boolean(ocrAsset); + + const handleOcrClick = async () => { + if (!ocrAsset) { + return; + } + try { + const ensured = await ensureAssetUrl(document.id, ocrAsset, { force: false }); + const entry = ensured || ocrAsset; + const url = entry?.url + || resolveDocumentAssetUrl(document, 'ocr-text', { + ensureAssetUrl, + getAsset: getDocumentAsset, + }); + if (!url) { + throw new Error('OCR text URL unavailable.'); + } + window.open(url, '_blank', 'noopener,noreferrer'); + } catch (error) { + notifyApiError(error, 'Unable to open OCR text.'); + } + }; + + return ( + <> + {downloadHref ? ( + + + + ) : null} + {hasOcr ? ( + + ) : null} + + + + ); +}; + +export const createPreviewSurface = ({ + document, + previewEntry, + ensureAssetUrl, + getDocumentAsset, + resolveApiPath, + notifyApiError, + onRegenerate, + onClose, + renderSidebarToggle, +}) => { + if (!document) { + return null; + } + + const title = document.title || document.original_name || 'Document preview'; + const sidebarToggle = renderSidebarToggle ? renderSidebarToggle() : null; + const leading = sidebarToggle ? <>{sidebarToggle} : null; + const header = { + title, + subtitle: null, + leading, + actions: createPreviewWorkspaceHeaderActions({ + document, + ensureAssetUrl, + getDocumentAsset, + resolveApiPath, + notifyApiError, + onRegenerate, + onClose, + }), + }; + + return { + key: 'preview', + variant: 'preview', + header, + content: , + supportsDetail: false, + }; +}; diff --git a/frontend/src/skeuomorphic_ws.jsx b/frontend/src/skeuomorphic_ws.jsx index eca756c..24b3c8a 100644 --- a/frontend/src/skeuomorphic_ws.jsx +++ b/frontend/src/skeuomorphic_ws.jsx @@ -8,7 +8,7 @@ import React, { } from 'react'; import { resolveDocumentAssetUrl, createAssetView } from './asset_manager'; import { useAssetNavigator } from './hooks/useAssetNavigator'; -import { ArrowLeftIcon, ArrowRightIcon } from './ui/icons'; +import { ArrowLeftIcon, ArrowRightIcon, RefreshIcon } from './ui/icons'; import PreviewZoomOverlay from './detail/PreviewZoomOverlay'; import { getTagColorStyle } from './utils/colors'; import './skeuomorphic_ws.css'; @@ -1785,3 +1785,43 @@ const SkeuomorphicWorkspace = ({ }; export default SkeuomorphicWorkspace; + +export const createSkeuomorphicSurface = ({ workspaceProps, renderSidebarToggle }) => { + if (!workspaceProps) { + return null; + } + + const { currentFolderName, searchResults, onRefresh, onExit } = workspaceProps; + const title = Array.isArray(searchResults) ? 'Search results' : currentFolderName; + const subtitle = Array.isArray(searchResults) + ? `${searchResults.length} matching document${searchResults.length === 1 ? '' : 's'}` + : null; + + const sidebarToggle = renderSidebarToggle ? renderSidebarToggle() : null; + const leading = sidebarToggle ? <>{sidebarToggle} : null; + + const actions = ( + <> + + + + ); + + return { + key: 'workspace', + variant: 'workspace', + header: { title, subtitle, leading, actions }, + content: , + supportsDetail: false, + }; +};