From 8fe017a941093160acf2017b086a0d9daaba3b5e Mon Sep 17 00:00:00 2001 From: dal Date: Thu, 20 Mar 2025 09:08:54 -0600 Subject: [PATCH] create remove metric from collections --- .../handlers/src/favorites/favorites_utils.rs | 36 +-- api/libs/handlers/src/metrics/mod.rs | 2 + .../remove_metrics_from_collection_handler.rs | 293 ++++++++++++++++++ api/src/routes/rest/routes/metrics/mod.rs | 5 + .../metrics/remove_metrics_from_collection.rs | 74 +++++ 5 files changed, 392 insertions(+), 18 deletions(-) create mode 100644 api/libs/handlers/src/metrics/remove_metrics_from_collection_handler.rs create mode 100644 api/src/routes/rest/routes/metrics/remove_metrics_from_collection.rs diff --git a/api/libs/handlers/src/favorites/favorites_utils.rs b/api/libs/handlers/src/favorites/favorites_utils.rs index 1a36ccaf9..d29c04c65 100644 --- a/api/libs/handlers/src/favorites/favorites_utils.rs +++ b/api/libs/handlers/src/favorites/favorites_utils.rs @@ -77,7 +77,7 @@ pub async fn list_user_favorites(user: &AuthenticatedUser) -> Result>(), ); - tokio::spawn(async move { get_favorite_dashboards(dashboard_ids) }) + tokio::spawn(async move { get_favorite_dashboards(dashboard_ids).await }) }; let collection_favorites = { @@ -88,7 +88,7 @@ pub async fn list_user_favorites(user: &AuthenticatedUser) -> Result>(), ); - tokio::spawn(async move { get_assets_from_collections(collection_ids) }) + tokio::spawn(async move { get_assets_from_collections(collection_ids).await }) }; let threads_favorites = { @@ -99,7 +99,7 @@ pub async fn list_user_favorites(user: &AuthenticatedUser) -> Result>(), ); - tokio::spawn(async move { get_favorite_threads(thread_ids) }) + tokio::spawn(async move { get_favorite_threads(thread_ids).await }) }; let metrics_favorites = { @@ -110,7 +110,7 @@ pub async fn list_user_favorites(user: &AuthenticatedUser) -> Result>(), ); - tokio::spawn(async move { get_favorite_metrics(metric_ids) }) + tokio::spawn(async move { get_favorite_metrics(metric_ids).await }) }; let (dashboard_fav_res, collection_fav_res, threads_fav_res, metrics_fav_res) = @@ -124,7 +124,7 @@ pub async fn list_user_favorites(user: &AuthenticatedUser) -> Result dashboards, Err(e) => { tracing::error!("Error getting favorite dashboards: {}", e); @@ -132,7 +132,7 @@ pub async fn list_user_favorites(user: &AuthenticatedUser) -> Result collections, Err(e) => { tracing::error!("Error getting favorite collections: {}", e); @@ -140,7 +140,7 @@ pub async fn list_user_favorites(user: &AuthenticatedUser) -> Result threads, Err(e) => { tracing::error!("Error getting favorite threads: {}", e); @@ -148,7 +148,7 @@ pub async fn list_user_favorites(user: &AuthenticatedUser) -> Result metrics, Err(e) => { tracing::error!("Error getting favorite metrics: {}", e); @@ -216,7 +216,7 @@ async fn get_favorite_threads(thread_ids: Arc>) -> Result>) -> Result) -> Result> { +async fn get_collection_names(collection_ids: &[Uuid]) -> Result> { let mut conn = match get_pg_pool().get().await { Ok(conn) => conn, Err(e) => return Err(anyhow!("Error getting connection from pool: {:?}", e)), @@ -359,7 +359,7 @@ async fn get_collection_names(collection_ids: &Vec) -> Result, + collection_ids: &[Uuid], ) -> Result> { let mut conn = match get_pg_pool().get().await { Ok(conn) => conn, @@ -403,7 +403,7 @@ async fn get_dashboards_from_collections( } async fn get_threads_from_collections( - collection_ids: &Vec, + collection_ids: &[Uuid], ) -> Result> { let mut conn = match get_pg_pool().get().await { Ok(conn) => conn, @@ -439,9 +439,9 @@ async fn get_threads_from_collections( .iter() .map(|(collection_id, id, name)| { ( - collection_id.clone(), + *collection_id, FavoriteObject { - id: id.clone(), + id: *id, name: name.clone().unwrap_or_else(|| String::from("Untitled")), type_: AssetType::Thread, }, @@ -472,7 +472,7 @@ async fn get_favorite_metrics(metric_ids: Arc>) -> Result>) -> Result) -> Result<()> { +pub async fn update_favorites(user: &AuthenticatedUser, favorites: &[Uuid]) -> Result<()> { let mut conn = match get_pg_pool().get().await { Ok(conn) => conn, Err(e) => return Err(anyhow!("Error getting connection from pool: {:?}", e)), @@ -509,7 +509,7 @@ pub async fn update_favorites(user: &AuthenticatedUser, favorites: &Vec) - let new_fav = UserFavorite { user_id: user.id, asset_id: *favorite_id, - asset_type: asset_type.clone(), + asset_type: *asset_type, order_index: index as i32, created_at: chrono::Utc::now(), deleted_at: None, diff --git a/api/libs/handlers/src/metrics/mod.rs b/api/libs/handlers/src/metrics/mod.rs index a56d166c7..076182a13 100644 --- a/api/libs/handlers/src/metrics/mod.rs +++ b/api/libs/handlers/src/metrics/mod.rs @@ -4,6 +4,7 @@ pub mod get_metric_data_handler; pub mod get_metric_handler; pub mod list_metrics_handler; pub mod post_metric_dashboard_handler; +pub mod remove_metrics_from_collection_handler; pub mod update_metric_handler; pub mod types; pub mod sharing; @@ -14,6 +15,7 @@ pub use get_metric_data_handler::*; pub use get_metric_handler::*; pub use list_metrics_handler::*; pub use post_metric_dashboard_handler::*; +pub use remove_metrics_from_collection_handler::*; pub use update_metric_handler::*; pub use types::*; pub use sharing::*; \ No newline at end of file diff --git a/api/libs/handlers/src/metrics/remove_metrics_from_collection_handler.rs b/api/libs/handlers/src/metrics/remove_metrics_from_collection_handler.rs new file mode 100644 index 000000000..78fb86be4 --- /dev/null +++ b/api/libs/handlers/src/metrics/remove_metrics_from_collection_handler.rs @@ -0,0 +1,293 @@ +use anyhow::{anyhow, Result}; +use database::{ + enums::{AssetPermissionRole, AssetType, IdentityType}, + helpers::metric_files::fetch_metric_file, + pool::get_pg_pool, + schema::{collections, collections_to_assets}, +}; +use diesel::{ExpressionMethods, QueryDsl}; +use diesel_async::RunQueryDsl; +use sharing::check_asset_permission::has_permission; +use tracing::{error, info}; +use uuid::Uuid; + +/// Response for removing a metric from collections +#[derive(Debug)] +pub struct RemoveMetricsFromCollectionResponse { + pub removed_count: usize, + pub failed_count: usize, + pub failed_ids: Vec, +} + +/// Removes a metric from multiple collections +/// +/// # Arguments +/// +/// * `metric_id` - The unique identifier of the metric +/// * `collection_ids` - Vector of collection IDs to remove the metric from +/// * `user_id` - The unique identifier of the user performing the action +/// +/// # Returns +/// +/// RemoveMetricsFromCollectionResponse on success, or an error if the operation fails +pub async fn remove_metrics_from_collection_handler( + metric_id: &Uuid, + collection_ids: Vec, + user_id: &Uuid, +) -> Result { + info!( + metric_id = %metric_id, + user_id = %user_id, + collection_count = collection_ids.len(), + "Removing metric from collections" + ); + + if collection_ids.is_empty() { + return Ok(RemoveMetricsFromCollectionResponse { + removed_count: 0, + failed_count: 0, + failed_ids: vec![], + }); + } + + // 1. Validate the metric exists + let _metric = match fetch_metric_file(metric_id).await { + Ok(Some(metric)) => metric, + Ok(None) => { + error!( + metric_id = %metric_id, + "Metric not found" + ); + return Err(anyhow!("Metric not found")); + } + Err(e) => { + error!("Error checking if metric exists: {}", e); + return Err(anyhow!("Database error: {}", e)); + } + }; + + // 2. Check if user has permission to modify the metric + let has_metric_permission = has_permission( + *metric_id, + AssetType::MetricFile, + *user_id, + IdentityType::User, + AssetPermissionRole::CanEdit, // This will pass for Owner and FullAccess too + ) + .await + .map_err(|e| { + error!( + metric_id = %metric_id, + user_id = %user_id, + "Error checking metric permission: {}", e + ); + anyhow!("Error checking permissions: {}", e) + })?; + + if !has_metric_permission { + error!( + metric_id = %metric_id, + user_id = %user_id, + "User does not have permission to modify this metric" + ); + return Err(anyhow!("User does not have permission to modify this metric")); + } + + // 3. Get database connection + let mut conn = get_pg_pool().get().await.map_err(|e| { + error!("Database connection error: {}", e); + anyhow!("Failed to get database connection: {}", e) + })?; + + // 4. Process each collection + let mut failed_ids = Vec::new(); + let mut removed_count = 0; + let now = chrono::Utc::now(); + + for collection_id in &collection_ids { + // Check if collection exists + let collection_exists = collections::table + .filter(collections::id.eq(collection_id)) + .filter(collections::deleted_at.is_null()) + .count() + .get_result::(&mut conn) + .await; + + if let Err(e) = collection_exists { + error!( + collection_id = %collection_id, + "Error checking if collection exists: {}", e + ); + failed_ids.push(*collection_id); + continue; + } + + if collection_exists.unwrap() == 0 { + error!( + collection_id = %collection_id, + "Collection not found" + ); + failed_ids.push(*collection_id); + continue; + } + + // Check if user has permission to modify the collection + let has_collection_permission = has_permission( + *collection_id, + AssetType::Collection, + *user_id, + IdentityType::User, + AssetPermissionRole::CanEdit, + ) + .await; + + if let Err(e) = has_collection_permission { + error!( + collection_id = %collection_id, + user_id = %user_id, + "Error checking collection permission: {}", e + ); + failed_ids.push(*collection_id); + continue; + } + + if !has_collection_permission.unwrap() { + error!( + collection_id = %collection_id, + user_id = %user_id, + "User does not have permission to modify this collection" + ); + failed_ids.push(*collection_id); + continue; + } + + // Mark metric as deleted from this collection + match diesel::update(collections_to_assets::table) + .filter(collections_to_assets::collection_id.eq(collection_id)) + .filter(collections_to_assets::asset_id.eq(metric_id)) + .filter(collections_to_assets::asset_type.eq(AssetType::MetricFile)) + .filter(collections_to_assets::deleted_at.is_null()) + .set(( + collections_to_assets::deleted_at.eq(now), + collections_to_assets::updated_at.eq(now), + collections_to_assets::updated_by.eq(user_id), + )) + .execute(&mut conn) + .await + { + Ok(updated) => { + if updated > 0 { + removed_count += 1; + info!( + metric_id = %metric_id, + collection_id = %collection_id, + "Successfully removed metric from collection" + ); + } else { + error!( + metric_id = %metric_id, + collection_id = %collection_id, + "Metric not found in collection" + ); + failed_ids.push(*collection_id); + } + } + Err(e) => { + error!( + metric_id = %metric_id, + collection_id = %collection_id, + "Error removing metric from collection: {}", e + ); + failed_ids.push(*collection_id); + } + } + } + + let failed_count = failed_ids.len(); + + info!( + metric_id = %metric_id, + user_id = %user_id, + collection_count = collection_ids.len(), + removed_count = removed_count, + failed_count = failed_count, + "Finished removing metric from collections" + ); + + Ok(RemoveMetricsFromCollectionResponse { + removed_count, + failed_count, + failed_ids, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + use database::enums::{AssetPermissionRole, AssetType, IdentityType}; + use uuid::Uuid; + use std::sync::Arc; + use mockall::predicate::*; + use mockall::mock; + + // Mock the database and sharing functions for testing + mock! { + MetricHelper {} + impl MetricHelper { + async fn fetch_metric_file(id: &Uuid) -> Result>; + } + } + + mock! { + SharingHelper {} + impl SharingHelper { + async fn has_permission( + asset_id: Uuid, + asset_type: AssetType, + identity_id: Uuid, + identity_type: IdentityType, + role: AssetPermissionRole, + ) -> Result; + } + } + + mock! { + DatabaseConnection {} + impl DatabaseConnection { + async fn execute_query(&self, query: &str) -> Result; + async fn get_results(&self, query: &str) -> Result>; + } + } + + #[tokio::test] + async fn test_remove_metrics_from_collection_handler() { + // This is a placeholder for the actual test + // In a real implementation, we would use test fixtures and a test database + + // For now, let's just check that the basic input validation works + let metric_id = Uuid::new_v4(); + let user_id = Uuid::new_v4(); + let empty_collections: Vec = vec![]; + + // Test with empty collections - should return success with 0 removed + let result = remove_metrics_from_collection_handler( + &metric_id, + empty_collections, + &user_id + ).await; + + assert!(result.is_ok()); + if let Ok(response) = result { + assert_eq!(response.removed_count, 0); + assert_eq!(response.failed_count, 0); + assert!(response.failed_ids.is_empty()); + } + + // In a real test, we would mock: + // 1. The metric_files database lookup + // 2. The permission checks + // 3. The collections database lookups + // 4. The update operations + // And then verify the behavior with various inputs + } +} \ No newline at end of file diff --git a/api/src/routes/rest/routes/metrics/mod.rs b/api/src/routes/rest/routes/metrics/mod.rs index a82639b27..eb0ee85e9 100644 --- a/api/src/routes/rest/routes/metrics/mod.rs +++ b/api/src/routes/rest/routes/metrics/mod.rs @@ -7,6 +7,7 @@ mod get_metric; mod get_metric_data; mod list_metrics; mod post_metric_dashboard; +mod remove_metrics_from_collection; mod update_metric; mod sharing; @@ -28,5 +29,9 @@ pub fn router() -> Router { "/:id/collections", post(add_metric_to_collections::add_metric_to_collections_rest_handler), ) + .route( + "/:id/collections", + delete(remove_metrics_from_collection::remove_metrics_from_collection), + ) .nest("/:id/sharing", sharing::router()) } diff --git a/api/src/routes/rest/routes/metrics/remove_metrics_from_collection.rs b/api/src/routes/rest/routes/metrics/remove_metrics_from_collection.rs new file mode 100644 index 000000000..5eaac1f9e --- /dev/null +++ b/api/src/routes/rest/routes/metrics/remove_metrics_from_collection.rs @@ -0,0 +1,74 @@ +use axum::{ + extract::{Extension, Json, Path}, + http::StatusCode, +}; +use handlers::metrics::remove_metrics_from_collection_handler; +use middleware::AuthenticatedUser; +use serde::{Deserialize, Serialize}; +use tracing::info; +use uuid::Uuid; + +use crate::routes::rest::ApiResponse; + +#[derive(Debug, Deserialize)] +pub struct RemoveFromCollectionsRequest { + pub collection_ids: Vec, +} + +#[derive(Debug, Serialize)] +pub struct RemoveFromCollectionsResponse { + pub message: String, + pub removed_count: usize, + pub failed_count: usize, + pub failed_ids: Vec, +} + +/// REST handler for removing a metric from multiple collections +/// +/// # Arguments +/// +/// * `user` - The authenticated user making the request +/// * `id` - The unique identifier of the metric +/// * `request` - The collection IDs to remove the metric from +/// +/// # Returns +/// +/// A success message on success, or an appropriate error response +pub async fn remove_metrics_from_collection( + Extension(user): Extension, + Path(id): Path, + Json(request): Json, +) -> Result, (StatusCode, String)> { + info!( + metric_id = %id, + user_id = %user.id, + collection_count = request.collection_ids.len(), + "Processing DELETE request to remove metric from collections" + ); + + match remove_metrics_from_collection_handler(&id, request.collection_ids, &user.id).await { + Ok(result) => { + let response = RemoveFromCollectionsResponse { + message: "Metric removed from collections successfully".to_string(), + removed_count: result.removed_count, + failed_count: result.failed_count, + failed_ids: result.failed_ids, + }; + Ok(ApiResponse::JsonData(response)) + } + Err(e) => { + tracing::error!("Error removing metric from collections: {}", e); + + // Map specific errors to appropriate status codes + let error_message = e.to_string(); + + if error_message.contains("Metric not found") { + return Err((StatusCode::NOT_FOUND, format!("Metric not found: {}", e))); + } else if error_message.contains("permission") { + return Err((StatusCode::FORBIDDEN, format!("Insufficient permissions: {}", e))); + } + + Err((StatusCode::INTERNAL_SERVER_ERROR, format!("Failed to remove metric from collections: {}", e))) + } + } +} \ No newline at end of file