From e972a8dddbe11033d8272bd0a2e01b0e5bf1bb75 Mon Sep 17 00:00:00 2001 From: Nils Schneider Date: Sat, 25 Oct 2025 02:11:05 +0200 Subject: [PATCH] fix folder move to root --- backend/src/routes/folders.rs | 101 ++++++++++++++++++++++++---------- backend/src/routes/mod.rs | 7 +-- backend/tests/common/mod.rs | 5 +- backend/tests/folders_flow.rs | 87 +++++++++++++++++++++++++---- docs/api.txt | 1 + 5 files changed, 153 insertions(+), 48 deletions(-) diff --git a/backend/src/routes/folders.rs b/backend/src/routes/folders.rs index 49c5a27..ebb0490 100644 --- a/backend/src/routes/folders.rs +++ b/backend/src/routes/folders.rs @@ -4,6 +4,7 @@ use axum::{ }; use diesel::{dsl::exists, prelude::*, PgConnection}; use serde::{Deserialize, Serialize}; +use serde_json::Value; use uuid::Uuid; use crate::models::{Document, Folder, NewFolder}; @@ -18,7 +19,10 @@ use super::documents::{ load_correspondents_for_documents, load_primary_assets, load_tags_for_documents, to_document_response, DocumentResponse, }; -use crate::utils::time::to_iso; +use crate::utils::{ + json::{classify_nullable, NullableValue}, + time::to_iso, +}; #[derive(Deserialize)] pub struct CreateFolderRequest { @@ -32,13 +36,6 @@ pub struct EnsureFolderPathRequest { pub segments: Vec, } -#[derive(Deserialize)] -pub struct UpdateFolderRequest { - #[serde(default)] - pub parent_id: Option>, - pub name: Option, -} - #[derive(Serialize)] pub struct FolderResponse { pub folder: FolderInfo, @@ -70,6 +67,24 @@ pub struct FolderInfo { pub updated_at: String, } +pub async fn get_folder( + Path(folder_id): Path, + TenantScopedConn { + mut conn, + tenant_id, + .. + }: TenantScopedConn, +) -> AppResult> { + let folder: Folder = folders::table + .find(folder_id) + .filter(folders::tenant_id.eq(tenant_id)) + .first(&mut conn)?; + + Ok(Json(FolderResponse { + folder: folder_to_info(folder), + })) +} + pub async fn ensure_folder_path( TenantScopedConn { mut conn, @@ -322,8 +337,15 @@ pub async fn update_folder( tenant_id, .. }: TenantScopedConn, - Json(payload): Json, + Json(body): Json, ) -> AppResult { + if !body.is_object() { + return Err(AppError::bad_request("request body must be a JSON object")); + } + + let parent_class = classify_nullable(body.get("parent_id")).map_err(AppError::bad_request)?; + let name_class = classify_nullable(body.get("name")).map_err(AppError::bad_request)?; + conn.transaction::<(), AppError, _>(|conn| { let folder: Folder = folders::table .find(folder_id) @@ -332,42 +354,61 @@ pub async fn update_folder( let mut next_parent = folder.parent_id; let mut parent_changed = false; - - if let Some(parent_request) = payload.parent_id { - if parent_request == Some(folder_id) { - return Err(AppError::bad_request("folder cannot be its own parent")); + match parent_class { + NullableValue::Omitted => {} + NullableValue::Null => { + if folder.parent_id.is_some() { + parent_changed = true; + } + next_parent = None; } + NullableValue::String(value) => { + let trimmed = value.trim(); + if trimmed.is_empty() { + return Err(AppError::bad_request("parent_id must not be empty")); + } + let parent_id = Uuid::parse_str(trimmed) + .map_err(|_| AppError::bad_request("parent_id must be a valid UUID or null"))?; + if parent_id == folder_id { + return Err(AppError::bad_request("folder cannot be its own parent")); + } - if let Some(parent_id) = parent_request { let _parent: Folder = folders::table .find(parent_id) .filter(folders::tenant_id.eq(tenant_id)) .first(conn)?; - let descendant_ids = gather_descendant_folder_ids(conn, tenant_id, folder_id)?; - if descendant_ids.contains(&parent_id) { - return Err(AppError::bad_request( - "cannot move folder into itself or a descendant", - )); + if folder.parent_id != Some(parent_id) { + let descendant_ids = gather_descendant_folder_ids(conn, tenant_id, folder_id)?; + if descendant_ids.contains(&parent_id) { + return Err(AppError::bad_request( + "cannot move folder into itself or a descendant", + )); + } + parent_changed = true; } - } - parent_changed = parent_request != folder.parent_id; - next_parent = parent_request; + next_parent = Some(parent_id); + } } let mut new_name = folder.name.clone(); let mut name_changed = false; - - if let Some(name) = payload.name { - let trimmed = name.trim(); - if trimmed.is_empty() { - return Err(AppError::bad_request("name must not be empty")); + match name_class { + NullableValue::Omitted => {} + NullableValue::Null => { + return Err(AppError::bad_request("name cannot be null")); } + NullableValue::String(value) => { + let trimmed = value.trim(); + if trimmed.is_empty() { + return Err(AppError::bad_request("name must not be empty")); + } - if trimmed != folder.name { - new_name = trimmed.to_string(); - name_changed = true; + if trimmed != folder.name { + new_name = trimmed.to_string(); + name_changed = true; + } } } diff --git a/backend/src/routes/mod.rs b/backend/src/routes/mod.rs index 707d696..ec2f419 100644 --- a/backend/src/routes/mod.rs +++ b/backend/src/routes/mod.rs @@ -97,10 +97,9 @@ pub fn create_router(state: AppState) -> Router<()> { let folders_routes = Router::new() .route("/", post(folders::create_folder)) .route("/path", post(folders::ensure_folder_path)) - .route( - "/:id", - delete(folders::delete_folder).patch(folders::update_folder), - ) + .route("/:id", get(folders::get_folder)) + .route("/:id", delete(folders::delete_folder)) + .route("/:id", patch(folders::update_folder)) .route("/:id/contents", get(folders::list_folder_contents)); let tags_routes = Router::new() diff --git a/backend/tests/common/mod.rs b/backend/tests/common/mod.rs index fffa711..0be3e8a 100644 --- a/backend/tests/common/mod.rs +++ b/backend/tests/common/mod.rs @@ -165,7 +165,7 @@ impl TestApp { pub async fn cleanup(&self) -> Result<()> { let pool = self.state.pool.clone(); - tokio::task::spawn_blocking(move || -> Result<()> { + let _ = tokio::task::spawn_blocking(move || -> Result<()> { let mut conn = pool .get() .map_err(|err| anyhow!("failed to get cleanup connection: {err}"))?; @@ -184,6 +184,7 @@ impl TestApp { self.storage.clone() } + #[allow(dead_code)] pub async fn storage_key_for(&self, key: &str) -> Result { let tenant = self .state @@ -322,7 +323,7 @@ impl TestApp { #[derive(Deserialize)] struct TenantSummary { tenant_id: Uuid, - slug: String, + _slug: String, } #[derive(Deserialize)] diff --git a/backend/tests/folders_flow.rs b/backend/tests/folders_flow.rs index 7a399a5..8599a2c 100644 --- a/backend/tests/folders_flow.rs +++ b/backend/tests/folders_flow.rs @@ -5,6 +5,7 @@ use axum::http::StatusCode; use common::{acquire_db_lock, body_to_vec, TestApp}; use serde::Deserialize; use serde::Serialize; +use serde_json::json; use uuid::Uuid; #[derive(Deserialize)] @@ -16,6 +17,7 @@ struct FolderResponse { struct FolderInfo { id: Uuid, name: String, + parent_id: Option, } #[derive(Deserialize)] @@ -42,14 +44,6 @@ struct EnsureFolderPath<'a> { segments: &'a [&'a str], } -#[derive(Serialize)] -struct UpdateFolderRequest { - #[serde(skip_serializing_if = "Option::is_none")] - parent_id: Option>, - #[serde(skip_serializing_if = "Option::is_none")] - name: Option, -} - #[derive(Serialize)] struct MoveDocumentRequest { folder_id: Option, @@ -150,6 +144,78 @@ async fn folder_move_and_delete_flow() -> Result<()> { Ok(()) } +#[tokio::test] +async fn update_folder_parent_to_root() -> Result<()> { + let _lock = acquire_db_lock().await; + let app = TestApp::new().await?; + + let password = "rootpass"; + app.insert_user("root-admin", password, "admin").await?; + let token = app.login_token("root-admin", password).await?; + + // Create a parent folder under root + let parent_resp = app + .post_json( + "/api/folders", + &CreateFolder { + name: "Parent", + parent_id: None, + }, + Some(&token), + ) + .await?; + assert_eq!(parent_resp.status(), StatusCode::OK); + let parent_body = body_to_vec(parent_resp.into_body()).await?; + let parent: FolderResponse = serde_json::from_slice(&parent_body)?; + + // Create a child folder inside the parent + let child_resp = app + .post_json( + "/api/folders", + &CreateFolder { + name: "Child", + parent_id: Some(parent.folder.id), + }, + Some(&token), + ) + .await?; + assert_eq!(child_resp.status(), StatusCode::OK); + let child_body = body_to_vec(child_resp.into_body()).await?; + let child: FolderResponse = serde_json::from_slice(&child_body)?; + + // Move the child back to the root by setting parent_id to null + let update_resp = app + .patch_json( + &format!("/api/folders/{}", child.folder.id), + &json!({ "parent_id": null }), + Some(&token), + ) + .await?; + assert_eq!(update_resp.status(), StatusCode::NO_CONTENT); + + // Fetch the child folder and ensure parent_id is now null + let updated_resp = app + .get(&format!("/api/folders/{}", child.folder.id), Some(&token)) + .await?; + assert_eq!(updated_resp.status(), StatusCode::OK); + let updated_body = body_to_vec(updated_resp.into_body()).await?; + let updated_folder: FolderResponse = serde_json::from_slice(&updated_body)?; + assert!(updated_folder.folder.parent_id.is_none()); + + // Root contents should include the child folder by name + let root_contents = app.get("/api/folders/root/contents", Some(&token)).await?; + assert_eq!(root_contents.status(), StatusCode::OK); + let root_body = body_to_vec(root_contents.into_body()).await?; + let root: FolderContents = serde_json::from_slice(&root_body)?; + assert!(root + .subfolders + .iter() + .any(|folder| folder.id == child.folder.id)); + + app.cleanup().await?; + Ok(()) +} + #[tokio::test] async fn ensure_path_creates_nested_folders() -> Result<()> { let _lock = acquire_db_lock().await; @@ -267,10 +333,7 @@ async fn folder_rename_updates_name_and_child_paths() -> Result<()> { let rename_resp = app .patch_json( &format!("/api/folders/{}", parent.folder.id), - &UpdateFolderRequest { - parent_id: None, - name: Some("Archive".to_string()), - }, + &json!({ "name": "Archive" }), Some(&token), ) .await?; diff --git a/docs/api.txt b/docs/api.txt index 21084fb..d48c0c0 100644 --- a/docs/api.txt +++ b/docs/api.txt @@ -46,6 +46,7 @@ Folders ------- - POST /api/folders - Create a folder (optionally under a parent). - POST /api/folders/path - Ensure a nested folder path exists, creating missing segments. +- GET /api/folders/:id - Fetch folder metadata. - GET /api/folders/:id/contents - List subfolders and documents inside a folder; use `root` for the workspace root. - DELETE /api/folders/:id - Soft-delete a folder. - PATCH /api/folders/:id - Update a folder's parent (`parent_id`) and/or rename it (`name`).