From a2273a81b547ce9b305433db7de9b14c6c698225 Mon Sep 17 00:00:00 2001 From: dal Date: Fri, 28 Mar 2025 08:58:46 -0600 Subject: [PATCH] bulk on favorite reqs --- .../handlers/src/favorites/create_favorite.rs | 144 +++++++++++++++--- .../handlers/src/favorites/delete_favorite.rs | 49 +++++- api/libs/handlers/src/favorites/mod.rs | 4 +- .../routes/users/favorites/create_favorite.rs | 16 +- .../routes/users/favorites/delete_favorite.rs | 24 ++- .../routes/rest/routes/users/favorites/mod.rs | 1 + .../favorites/create_favorites_bulk_test.rs | 114 ++++++++++++++ .../favorites/delete_favorites_bulk_test.rs | 128 ++++++++++++++++ api/tests/integration/favorites/mod.rs | 2 + api/tests/integration/mod.rs | 1 + 10 files changed, 445 insertions(+), 38 deletions(-) create mode 100644 api/tests/integration/favorites/create_favorites_bulk_test.rs create mode 100644 api/tests/integration/favorites/delete_favorites_bulk_test.rs create mode 100644 api/tests/integration/favorites/mod.rs diff --git a/api/libs/handlers/src/favorites/create_favorite.rs b/api/libs/handlers/src/favorites/create_favorite.rs index bc2d7cee9..29b4c187c 100644 --- a/api/libs/handlers/src/favorites/create_favorite.rs +++ b/api/libs/handlers/src/favorites/create_favorite.rs @@ -3,28 +3,37 @@ use chrono::Utc; use diesel::{insert_into, update, upsert::excluded, ExpressionMethods}; use diesel_async::RunQueryDsl; use middleware::AuthenticatedUser; +use serde::{Deserialize, Serialize}; use uuid::Uuid; -use database::{ - enums::AssetType, - pool::get_pg_pool, - models::UserFavorite, - schema::user_favorites, -}; +use database::{enums::AssetType, models::UserFavorite, pool::get_pg_pool, schema::user_favorites}; use super::favorites_utils::{list_user_favorites, FavoriteObject}; +#[derive(Deserialize, Serialize, Clone)] pub struct CreateFavoriteReq { pub id: Uuid, + #[serde(alias = "type")] pub asset_type: AssetType, pub index: Option, } +// Maintain backward compatibility with single item operations pub async fn create_favorite( user: &AuthenticatedUser, req: &CreateFavoriteReq, ) -> Result> { - let index = req.index.unwrap_or(0); + create_favorites_bulk(user, &[req.clone()]).await +} + +// New function to handle bulk operations +pub async fn create_favorites_bulk( + user: &AuthenticatedUser, + favorites: &[CreateFavoriteReq], +) -> Result> { + if favorites.is_empty() { + return list_user_favorites(user).await; + } let mut conn = match get_pg_pool().get().await { Ok(conn) => conn, @@ -34,28 +43,45 @@ pub async fn create_favorite( } }; + // Find the minimum index to know where to start shifting existing favorites + let min_index = favorites + .iter() + .map(|f| f.index.unwrap_or(0)) + .min() + .unwrap_or(0); + + // Shift existing favorites to make room for new ones (one operation for all) match update(user_favorites::table) - .set(user_favorites::order_index.eq(user_favorites::order_index + 1)) + .set(user_favorites::order_index.eq(user_favorites::order_index + favorites.len() as i32)) .filter(user_favorites::user_id.eq(user.id)) - .filter(user_favorites::order_index.ge(index as i32)) + .filter(user_favorites::order_index.ge(min_index as i32)) .execute(&mut conn) .await { Ok(_) => (), - Err(e) => return Err(anyhow!("Error updating user favorite: {}", e)), + Err(e) => return Err(anyhow!("Error updating user favorites: {}", e)), }; - let user_favorite = UserFavorite { - asset_type: req.asset_type, - user_id: user.id, - asset_id: req.id, - order_index: index as i32, - created_at: Utc::now(), - deleted_at: None, - }; + // Prepare all new favorites for bulk insertion + let user_favorites: Vec = favorites + .iter() + .enumerate() + .map(|(i, req)| { + let index = req.index.unwrap_or(0) + i; + UserFavorite { + asset_type: req.asset_type, + user_id: user.id, + asset_id: req.id, + order_index: index as i32, + created_at: Utc::now(), + deleted_at: None, + } + }) + .collect(); + // Insert all favorites in a single operation match insert_into(user_favorites::table) - .values(user_favorite) + .values(&user_favorites) .on_conflict(( user_favorites::user_id, user_favorites::asset_id, @@ -70,8 +96,86 @@ pub async fn create_favorite( .await { Ok(_) => (), - Err(e) => return Err(anyhow!("Error inserting or updating user favorite: {}", e)), + Err(e) => return Err(anyhow!("Error inserting or updating user favorites: {}", e)), }; list_user_favorites(user).await } + +#[cfg(test)] +mod tests { + use super::*; + use anyhow::Result; + use chrono::Utc; + use database::enums::AssetType; + use middleware::AuthenticatedUser; + use mock_pool::MockPool; + use tokio; + use uuid::Uuid; + + // We need to mock the database pool + // This is a very simple mock implementation for testing + mod mock_pool { + use anyhow::Result; + use diesel_async::{AsyncConnection, AsyncPgConnection}; + use std::sync::{Arc, Mutex}; + + pub struct MockConn; + + impl MockConn { + pub async fn execute(&self, _query: String) -> Result<()> { + Ok(()) + } + } + + pub struct MockPool { + pub connections: Arc>>, + } + + impl MockPool { + pub fn new() -> Self { + Self { + connections: Arc::new(Mutex::new(vec![MockConn])), + } + } + + pub async fn get(&self) -> Result { + let conn = self.connections.lock().unwrap().pop().unwrap(); + Ok(conn) + } + } + } + + // Mock the database functions + // This is just a placeholder for the actual unit test + // A real implementation would use a proper testing framework + #[tokio::test] + async fn test_create_favorites_bulk() -> Result<()> { + // Create test user + let user = AuthenticatedUser { + id: Uuid::new_v4(), + // Add other fields as needed + }; + + // Create test favorites + let favorites = vec![ + CreateFavoriteReq { + id: Uuid::new_v4(), + asset_type: AssetType::DashboardFile, + index: None, + }, + CreateFavoriteReq { + id: Uuid::new_v4(), + asset_type: AssetType::Collection, + index: Some(1), + }, + ]; + + // In a real test, we would use a test database + // For now, we'll just assert that the function doesn't panic + let result = create_favorites_bulk(&user, &favorites).await; + assert!(result.is_ok() || result.is_err()); + + Ok(()) + } +} diff --git a/api/libs/handlers/src/favorites/delete_favorite.rs b/api/libs/handlers/src/favorites/delete_favorite.rs index a12786b7b..5b7ee8646 100644 --- a/api/libs/handlers/src/favorites/delete_favorite.rs +++ b/api/libs/handlers/src/favorites/delete_favorite.rs @@ -12,22 +12,67 @@ use database::{ use super::favorites_utils::{list_user_favorites, FavoriteObject}; +// Maintain backward compatibility with single item operations pub async fn delete_favorite(user: &AuthenticatedUser, id: &Uuid) -> Result> { + delete_favorites_bulk(user, &[*id]).await +} + +// New function to handle bulk operations +pub async fn delete_favorites_bulk(user: &AuthenticatedUser, ids: &[Uuid]) -> Result> { + if ids.is_empty() { + return list_user_favorites(user).await; + } + let mut conn = match get_pg_pool().get().await { Ok(conn) => conn, Err(e) => return Err(anyhow!("Error getting connection from pool: {:?}", e)), }; + // Delete all favorites in a single operation match update(user_favorites::table) .set(user_favorites::deleted_at.eq(Some(Utc::now()))) .filter(user_favorites::user_id.eq(user.id)) - .filter(user_favorites::asset_id.eq(id)) + .filter(user_favorites::asset_id.eq_any(ids)) .execute(&mut conn) .await { Ok(_) => (), - Err(e) => return Err(anyhow!("Error deleting favorite: {:?}", e)), + Err(e) => return Err(anyhow!("Error deleting favorites: {:?}", e)), }; list_user_favorites(user).await } + +#[cfg(test)] +mod tests { + use super::*; + use database::enums::AssetType; + use middleware::AuthenticatedUser; + use uuid::Uuid; + use tokio; + use anyhow::Result; + + // Placeholder for a real test + #[tokio::test] + async fn test_delete_favorites_bulk() -> Result<()> { + // Create test user + let user = AuthenticatedUser { + id: Uuid::new_v4(), + // Add other fields as needed + }; + + // Create test favorite ids + let favorite_ids = vec![ + Uuid::new_v4(), + Uuid::new_v4(), + Uuid::new_v4(), + ]; + + // In a real test, we would use a test database + // For now, we'll just assert that the function doesn't panic + let result = delete_favorites_bulk(&user, &favorite_ids).await; + assert!(result.is_ok() || result.is_err()); + + Ok(()) + } +} diff --git a/api/libs/handlers/src/favorites/mod.rs b/api/libs/handlers/src/favorites/mod.rs index 099ff0014..b5f405133 100644 --- a/api/libs/handlers/src/favorites/mod.rs +++ b/api/libs/handlers/src/favorites/mod.rs @@ -6,7 +6,7 @@ mod delete_favorite; mod update_favorites; pub use list_favorites::list_favorites; -pub use create_favorite::{create_favorite, CreateFavoriteReq}; -pub use delete_favorite::delete_favorite; +pub use create_favorite::{create_favorite, create_favorites_bulk, CreateFavoriteReq}; +pub use delete_favorite::{delete_favorite, delete_favorites_bulk}; pub use update_favorites::update_favorites; pub use favorites_utils::{FavoriteObject, FavoriteIdAndType, UserFavoritesReq}; diff --git a/api/src/routes/rest/routes/users/favorites/create_favorite.rs b/api/src/routes/rest/routes/users/favorites/create_favorite.rs index 7502ee88a..4427efb3e 100644 --- a/api/src/routes/rest/routes/users/favorites/create_favorite.rs +++ b/api/src/routes/rest/routes/users/favorites/create_favorite.rs @@ -1,24 +1,18 @@ use axum::{extract::Json, http::StatusCode, Extension}; -use handlers::favorites::{create_favorite, CreateFavoriteReq, FavoriteObject, FavoriteIdAndType}; +use handlers::favorites::{create_favorites_bulk, CreateFavoriteReq, FavoriteObject}; use middleware::AuthenticatedUser; pub async fn create_favorite_handler( Extension(user): Extension, - Json(payload): Json, + Json(favorites): Json>, ) -> Result>, (StatusCode, String)> { - let req = CreateFavoriteReq { - id: payload.id, - asset_type: payload.type_, - index: None, - }; - - match create_favorite(&user, &req).await { + match create_favorites_bulk(&user, &favorites).await { Ok(favorites) => Ok(Json(favorites)), Err(e) => { - tracing::error!("Error creating favorite: {:?}", e); + tracing::error!("Error creating favorites: {:?}", e); Err(( StatusCode::INTERNAL_SERVER_ERROR, - format!("Error creating favorite: {:?}", e), + format!("Error creating favorites: {:?}", e), )) } } diff --git a/api/src/routes/rest/routes/users/favorites/delete_favorite.rs b/api/src/routes/rest/routes/users/favorites/delete_favorite.rs index 2ab4b9015..0b392f344 100644 --- a/api/src/routes/rest/routes/users/favorites/delete_favorite.rs +++ b/api/src/routes/rest/routes/users/favorites/delete_favorite.rs @@ -1,11 +1,12 @@ use axum::{ - extract::{Json, Path, Extension}, + extract::{Extension, Json, Path}, http::StatusCode, }; -use uuid::Uuid; -use handlers::favorites::{delete_favorite, FavoriteObject}; +use handlers::favorites::{delete_favorite, delete_favorites_bulk, FavoriteObject}; use middleware::AuthenticatedUser; +use uuid::Uuid; +// Handler for DELETE /:id path parameter pub async fn delete_favorite_handler( Extension(user): Extension, Path(id): Path, @@ -21,3 +22,20 @@ pub async fn delete_favorite_handler( } } } + +// Handler for DELETE / with request body +pub async fn delete_favorites_bulk_handler( + Extension(user): Extension, + Json(ids): Json>, +) -> Result>, (StatusCode, String)> { + match delete_favorites_bulk(&user, &ids).await { + Ok(favorites) => Ok(Json(favorites)), + Err(e) => { + tracing::error!("Error deleting favorites: {:?}", e); + Err(( + StatusCode::INTERNAL_SERVER_ERROR, + format!("Error deleting favorites: {:?}", e), + )) + } + } +} diff --git a/api/src/routes/rest/routes/users/favorites/mod.rs b/api/src/routes/rest/routes/users/favorites/mod.rs index b31f25190..5729703b8 100644 --- a/api/src/routes/rest/routes/users/favorites/mod.rs +++ b/api/src/routes/rest/routes/users/favorites/mod.rs @@ -14,5 +14,6 @@ pub fn router() -> Router { .route("/", get(list_favorites::list_favorites_handler)) .route("/", post(create_favorite::create_favorite_handler)) .route("/:id", delete(delete_favorite::delete_favorite_handler)) + .route("/", delete(delete_favorite::delete_favorites_bulk_handler)) .route("/", put(update_favorites::update_favorites_handler)) } diff --git a/api/tests/integration/favorites/create_favorites_bulk_test.rs b/api/tests/integration/favorites/create_favorites_bulk_test.rs new file mode 100644 index 000000000..f27b7e976 --- /dev/null +++ b/api/tests/integration/favorites/create_favorites_bulk_test.rs @@ -0,0 +1,114 @@ +use anyhow::Result; +use serde_json::json; +use uuid::Uuid; + +use database::enums::AssetType; +use crate::common::{ + setup_test_env, TestApp, TestDb, TestTaggable, FixtureBuilder, + assertions::ResponseAssertions, +}; + +#[tokio::test] +async fn test_create_single_favorite() -> Result<()> { + // Setup + setup_test_env(); + let test_db = TestDb::new().await?; + test_db.setup_test_data().await?; + + let test_app = TestApp::new().await?; + let client = test_app.client(); + + // Create a test user and dashboard to favorite + let user = FixtureBuilder::create_user().build(); + let dashboard = FixtureBuilder::create_dashboard().with_user(&user).build(); + + // Save fixtures to database + let mut conn = test_db.pool.get().await?; + user.save(&mut conn).await?; + dashboard.save(&mut conn).await?; + + // Test single favorite creation + let response = client + .post(&format!("/api/users/me/favorites")) + .json(&json!({ + "id": dashboard.id, + "asset_type": "DashboardFile" + })) + .header("x-user-id", user.id.to_string()) + .send() + .await?; + + response.assert_status_ok()?; + let body = response.json::>().await?; + + // Assert that response contains the favorited dashboard + assert!(!body.is_empty()); + let favorited_item = body.iter().find(|item| { + item["id"] == dashboard.id.to_string() && item["asset_type"] == "DashboardFile" + }); + assert!(favorited_item.is_some()); + + Ok(()) +} + +#[tokio::test] +async fn test_create_multiple_favorites() -> Result<()> { + // Setup + setup_test_env(); + let test_db = TestDb::new().await?; + test_db.setup_test_data().await?; + + let test_app = TestApp::new().await?; + let client = test_app.client(); + + // Create a test user and multiple assets to favorite + let user = FixtureBuilder::create_user().build(); + let dashboard1 = FixtureBuilder::create_dashboard().with_user(&user).build(); + let dashboard2 = FixtureBuilder::create_dashboard().with_user(&user).build(); + let collection = FixtureBuilder::create_collection().with_user(&user).build(); + + // Save fixtures to database + let mut conn = test_db.pool.get().await?; + user.save(&mut conn).await?; + dashboard1.save(&mut conn).await?; + dashboard2.save(&mut conn).await?; + collection.save(&mut conn).await?; + + // Test bulk favorite creation + let response = client + .post(&format!("/api/users/me/favorites")) + .json(&json!([ + { + "id": dashboard1.id, + "asset_type": "DashboardFile" + }, + { + "id": dashboard2.id, + "asset_type": "DashboardFile" + }, + { + "id": collection.id, + "asset_type": "Collection" + } + ])) + .header("x-user-id", user.id.to_string()) + .send() + .await?; + + response.assert_status_ok()?; + let body = response.json::>().await?; + + // Assert that response contains all favorited items + assert!(body.len() >= 3); + + // Check if all three assets are in the favorites + let dashboard1_found = body.iter().any(|item| item["id"] == dashboard1.id.to_string()); + let dashboard2_found = body.iter().any(|item| item["id"] == dashboard2.id.to_string()); + let collection_found = body.iter().any(|item| item["id"] == collection.id.to_string()); + + assert!(dashboard1_found, "Dashboard 1 not found in favorites"); + assert!(dashboard2_found, "Dashboard 2 not found in favorites"); + assert!(collection_found, "Collection not found in favorites"); + + Ok(()) +} \ No newline at end of file diff --git a/api/tests/integration/favorites/delete_favorites_bulk_test.rs b/api/tests/integration/favorites/delete_favorites_bulk_test.rs new file mode 100644 index 000000000..2dbe6a27b --- /dev/null +++ b/api/tests/integration/favorites/delete_favorites_bulk_test.rs @@ -0,0 +1,128 @@ +use anyhow::Result; +use serde_json::json; +use uuid::Uuid; + +use database::enums::AssetType; +use handlers::favorites::{create_favorite, CreateFavoriteReq}; +use crate::common::{ + setup_test_env, TestApp, TestDb, TestTaggable, FixtureBuilder, + assertions::ResponseAssertions, +}; + +#[tokio::test] +async fn test_delete_single_favorite_by_id() -> Result<()> { + // Setup + setup_test_env(); + let test_db = TestDb::new().await?; + test_db.setup_test_data().await?; + + let test_app = TestApp::new().await?; + let client = test_app.client(); + + // Create a test user and dashboard to favorite + let user = FixtureBuilder::create_user().build(); + let dashboard = FixtureBuilder::create_dashboard().with_user(&user).build(); + + // Save fixtures to database + let mut conn = test_db.pool.get().await?; + user.save(&mut conn).await?; + dashboard.save(&mut conn).await?; + + // Create favorite first + let response = client + .post(&format!("/api/users/me/favorites")) + .json(&json!({ + "id": dashboard.id, + "asset_type": "DashboardFile" + })) + .header("x-user-id", user.id.to_string()) + .send() + .await?; + + response.assert_status_ok()?; + + // Now delete the favorite using path parameter + let response = client + .delete(&format!("/api/users/me/favorites/{}", dashboard.id)) + .header("x-user-id", user.id.to_string()) + .send() + .await?; + + response.assert_status_ok()?; + let body = response.json::>().await?; + + // Assert that the favorite is no longer in the list + let still_favorited = body.iter().any(|item| item["id"] == dashboard.id.to_string()); + assert!(!still_favorited, "Dashboard is still in favorites after deletion"); + + Ok(()) +} + +#[tokio::test] +async fn test_delete_multiple_favorites_bulk() -> Result<()> { + // Setup + setup_test_env(); + let test_db = TestDb::new().await?; + test_db.setup_test_data().await?; + + let test_app = TestApp::new().await?; + let client = test_app.client(); + + // Create a test user and multiple assets to favorite + let user = FixtureBuilder::create_user().build(); + let dashboard1 = FixtureBuilder::create_dashboard().with_user(&user).build(); + let dashboard2 = FixtureBuilder::create_dashboard().with_user(&user).build(); + let collection = FixtureBuilder::create_collection().with_user(&user).build(); + + // Save fixtures to database + let mut conn = test_db.pool.get().await?; + user.save(&mut conn).await?; + dashboard1.save(&mut conn).await?; + dashboard2.save(&mut conn).await?; + collection.save(&mut conn).await?; + + // Create favorites first + let response = client + .post(&format!("/api/users/me/favorites")) + .json(&json!([ + { + "id": dashboard1.id, + "asset_type": "DashboardFile" + }, + { + "id": dashboard2.id, + "asset_type": "DashboardFile" + }, + { + "id": collection.id, + "asset_type": "Collection" + } + ])) + .header("x-user-id", user.id.to_string()) + .send() + .await?; + + response.assert_status_ok()?; + + // Now delete multiple favorites using bulk endpoint + let response = client + .delete(&format!("/api/users/me/favorites")) + .json(&json!([dashboard1.id, dashboard2.id])) + .header("x-user-id", user.id.to_string()) + .send() + .await?; + + response.assert_status_ok()?; + let body = response.json::>().await?; + + // Assert that the deleted favorites are no longer in the list + let dashboard1_found = body.iter().any(|item| item["id"] == dashboard1.id.to_string()); + let dashboard2_found = body.iter().any(|item| item["id"] == dashboard2.id.to_string()); + let collection_found = body.iter().any(|item| item["id"] == collection.id.to_string()); + + assert!(!dashboard1_found, "Dashboard 1 is still in favorites after deletion"); + assert!(!dashboard2_found, "Dashboard 2 is still in favorites after deletion"); + assert!(collection_found, "Collection should still be in favorites"); + + Ok(()) +} \ No newline at end of file diff --git a/api/tests/integration/favorites/mod.rs b/api/tests/integration/favorites/mod.rs new file mode 100644 index 000000000..20a4d250a --- /dev/null +++ b/api/tests/integration/favorites/mod.rs @@ -0,0 +1,2 @@ +pub mod create_favorites_bulk_test; +pub mod delete_favorites_bulk_test; \ No newline at end of file diff --git a/api/tests/integration/mod.rs b/api/tests/integration/mod.rs index 40b445697..558839caf 100644 --- a/api/tests/integration/mod.rs +++ b/api/tests/integration/mod.rs @@ -3,6 +3,7 @@ pub mod chats; pub mod dashboards; pub mod collections; pub mod data_sources; +pub mod favorites; pub mod metrics; pub mod organizations; pub mod routes;