diff --git a/api/libs/handlers/src/metrics/add_metric_to_collections_handler.rs b/api/libs/handlers/src/metrics/add_metric_to_collections_handler.rs new file mode 100644 index 000000000..b0a03a965 --- /dev/null +++ b/api/libs/handlers/src/metrics/add_metric_to_collections_handler.rs @@ -0,0 +1,233 @@ +use anyhow::{anyhow, Result}; +use database::{ + enums::{AssetPermissionRole, AssetType, IdentityType}, + models::CollectionToAsset, + pool::get_pg_pool, + schema::{collections, collections_to_assets, metric_files}, +}; +use diesel::{ExpressionMethods, QueryDsl}; +use diesel_async::RunQueryDsl; +use sharing::check_asset_permission::has_permission; +use tracing::{error, info}; +use uuid::Uuid; + +/// Adds a metric to multiple collections +/// +/// # Arguments +/// +/// * `metric_id` - The unique identifier of the metric +/// * `collection_ids` - Vector of collection IDs to add the metric to +/// * `user_id` - The unique identifier of the user performing the action +/// +/// # Returns +/// +/// Ok(()) on success, or an error if the operation fails +pub async fn add_metric_to_collections_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(), + "Adding metric to collections" + ); + + if collection_ids.is_empty() { + return Ok(()); + } + + // 1. Validate the metric exists + let mut conn = get_pg_pool().get().await.map_err(|e| { + error!("Database connection error: {}", e); + anyhow!("Failed to get database connection: {}", e) + })?; + + let metric_exists = metric_files::table + .filter(metric_files::id.eq(metric_id)) + .filter(metric_files::deleted_at.is_null()) + .count() + .get_result::(&mut conn) + .await + .map_err(|e| { + error!("Error checking if metric exists: {}", e); + anyhow!("Database error: {}", e) + })?; + + if metric_exists == 0 { + error!( + metric_id = %metric_id, + "Metric not found" + ); + return Err(anyhow!("Metric not found")); + } + + // 2. Check if user has permission to modify the metric (Owner, FullAccess, or CanEdit) + 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. Validate collections exist and user has access to them + 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 + .map_err(|e| { + error!("Error checking if collection exists: {}", e); + anyhow!("Database error: {}", e) + })?; + + if collection_exists == 0 { + error!( + collection_id = %collection_id, + "Collection not found" + ); + return Err(anyhow!("Collection not found: {}", collection_id)); + } + + // Check if user has access to the collection + let has_collection_permission = has_permission( + *collection_id, + AssetType::Collection, + *user_id, + IdentityType::User, + AssetPermissionRole::CanView, // User needs at least view access + ) + .await + .map_err(|e| { + error!( + collection_id = %collection_id, + user_id = %user_id, + "Error checking collection permission: {}", e + ); + anyhow!("Error checking permissions: {}", e) + })?; + + if !has_collection_permission { + error!( + collection_id = %collection_id, + user_id = %user_id, + "User does not have permission to access this collection" + ); + return Err(anyhow!( + "User does not have permission to access collection: {}", + collection_id + )); + } + } + + // 4. Add metric to collections (upsert if previously deleted) + for collection_id in &collection_ids { + // Check if the metric is already in the collection + let existing = match 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)) + .first::(&mut conn) + .await + { + Ok(record) => Some(record), + Err(diesel::NotFound) => None, + Err(e) => { + error!( + "Error checking if metric is already in collection: {}", + e + ); + return Err(anyhow!("Database error: {}", e)); + } + }; + + if let Some(existing_record) = existing { + if existing_record.deleted_at.is_some() { + // If it was previously deleted, update it + 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)) + .set(( + collections_to_assets::deleted_at + .eq::>>(None), + collections_to_assets::updated_at.eq(chrono::Utc::now()), + collections_to_assets::updated_by.eq(user_id), + )) + .execute(&mut conn) + .await + .map_err(|e| { + error!("Error updating collection to asset record: {}", e); + anyhow!("Database error: {}", e) + })?; + } + // If it's already active, do nothing + } else { + // If it doesn't exist, create a new record + diesel::insert_into(collections_to_assets::table) + .values(( + collections_to_assets::collection_id.eq(collection_id), + collections_to_assets::asset_id.eq(metric_id), + collections_to_assets::asset_type.eq(AssetType::MetricFile), + collections_to_assets::created_at.eq(chrono::Utc::now()), + collections_to_assets::updated_at.eq(chrono::Utc::now()), + collections_to_assets::created_by.eq(user_id), + collections_to_assets::updated_by.eq(user_id), + )) + .execute(&mut conn) + .await + .map_err(|e| { + error!("Error creating collection to asset record: {}", e); + anyhow!("Database error: {}", e) + })?; + } + } + + info!( + metric_id = %metric_id, + user_id = %user_id, + collection_count = collection_ids.len(), + "Successfully added metric to collections" + ); + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use database::enums::{AssetPermissionRole, AssetType, IdentityType}; + use uuid::Uuid; + + #[tokio::test] + async fn test_add_metric_to_collections_handler() { + // This is a placeholder for the actual test + // In a real implementation, we would use test fixtures and a test database + assert!(true); + } +} \ No newline at end of file diff --git a/api/libs/handlers/src/metrics/mod.rs b/api/libs/handlers/src/metrics/mod.rs index c5f688aeb..a56d166c7 100644 --- a/api/libs/handlers/src/metrics/mod.rs +++ b/api/libs/handlers/src/metrics/mod.rs @@ -1,3 +1,4 @@ +pub mod add_metric_to_collections_handler; pub mod delete_metric_handler; pub mod get_metric_data_handler; pub mod get_metric_handler; @@ -7,6 +8,7 @@ pub mod update_metric_handler; pub mod types; pub mod sharing; +pub use add_metric_to_collections_handler::*; pub use delete_metric_handler::*; pub use get_metric_data_handler::*; pub use get_metric_handler::*; diff --git a/api/prds/active/api_add_metrics_to_collection.md b/api/prds/active/api_add_metrics_to_collection.md index eb6023b84..1a767b5b7 100644 --- a/api/prds/active/api_add_metrics_to_collection.md +++ b/api/prds/active/api_add_metrics_to_collection.md @@ -61,7 +61,7 @@ The handler will: #### New Files -1. `libs/handlers/src/metrics/add_metric_to_collections_handler.rs` +1. ✅ `libs/handlers/src/metrics/add_metric_to_collections_handler.rs` ```rust use anyhow::{anyhow, Result}; use database::{ @@ -286,7 +286,7 @@ mod tests { } ``` -2. `src/routes/rest/routes/metrics/add_metric_to_collections.rs` +2. ✅ `src/routes/rest/routes/metrics/add_metric_to_collections.rs` ```rust use axum::{ extract::{Extension, Json, Path}, @@ -352,8 +352,8 @@ pub async fn add_metric_to_collections_rest_handler( } ``` -3. Update `libs/handlers/src/metrics/mod.rs` to include the new handler -4. Update `src/routes/rest/routes/metrics/mod.rs` to include the new endpoint +3. ✅ Update `libs/handlers/src/metrics/mod.rs` to include the new handler +4. ✅ Update `src/routes/rest/routes/metrics/mod.rs` to include the new endpoint ### Database Operations diff --git a/api/prds/active/project_collections_rest_endpoints.md b/api/prds/active/project_collections_rest_endpoints.md index b027144e5..369f3d91c 100644 --- a/api/prds/active/project_collections_rest_endpoints.md +++ b/api/prds/active/project_collections_rest_endpoints.md @@ -68,14 +68,10 @@ The implementation will be broken down into four separate PRDs, each focusing on 1. [Add Dashboard to Collections REST Endpoint](api_add_dashboard_to_collections.md) 2. [Remove Dashboard from Collections REST Endpoint](api_remove_dashboard_from_collections.md) -3. [Add Metric to Collections REST Endpoint](api_add_metric_to_collections.md) +3. ✅ [Add Metric to Collections REST Endpoint](api_add_metric_to_collections.md) 4. [Remove Metric from Collections REST Endpoint](api_remove_metric_from_collections.md) 5. [Add Assets to Collection REST Endpoint](api_add_assets_to_collection.md) 6. [Remove Assets from Collection REST Endpoint](api_remove_assets_from_collection.md) -7. [Add Dashboard to Collections REST Endpoint](api_add_dashboard_to_collections.md) -8. [Remove Dashboard from Collections REST Endpoint](api_remove_dashboards_from_collection.md) -9. [Add Metric to Collections REST Endpoint](api_add_metric_to_collections.md) -10. [Remove Metric from Collections REST Endpoint](api_remove_metrics_from_collection.md) Additionally, we have a separate PRD for the concurrent asset existence checking approach: @@ -137,12 +133,8 @@ Each PRD will include its own detailed testing strategy, but at a high level: ### Related PRDs - [Add Dashboard to Collections REST Endpoint](api_add_dashboard_to_collections.md) - [Remove Dashboard from Collections REST Endpoint](api_remove_dashboard_from_collections.md) -- [Add Metric to Collections REST Endpoint](api_add_metric_to_collections.md) +- ✅ [Add Metric to Collections REST Endpoint](api_add_metric_to_collections.md) - [Remove Metric from Collections REST Endpoint](api_remove_metric_from_collections.md) - [Add Assets to Collection REST Endpoint](api_add_assets_to_collection.md) - [Remove Assets from Collection REST Endpoint](api_remove_assets_from_collection.md) -- [Add Dashboard to Collections REST Endpoint](api_add_dashboard_to_collections.md) -- [Remove Dashboard from Collections REST Endpoint](api_remove_dashboards_from_collection.md) -- [Add Metric to Collections REST Endpoint](api_add_metric_to_collections.md) -- [Remove Metric from Collections REST Endpoint](api_remove_metrics_from_collection.md) - [Concurrent Asset Existence Checking](concurrent_asset_existence_checking.md) diff --git a/api/src/routes/rest/routes/metrics/add_metric_to_collections.rs b/api/src/routes/rest/routes/metrics/add_metric_to_collections.rs new file mode 100644 index 000000000..3c952a16f --- /dev/null +++ b/api/src/routes/rest/routes/metrics/add_metric_to_collections.rs @@ -0,0 +1,62 @@ +use axum::{ + extract::{Extension, Json, Path}, + http::StatusCode, +}; +use handlers::metrics::add_metric_to_collections_handler; +use middleware::AuthenticatedUser; +use serde::Deserialize; +use tracing::info; +use uuid::Uuid; + +use crate::routes::rest::ApiResponse; + +#[derive(Debug, Deserialize)] +pub struct AddMetricRequest { + pub collection_ids: Vec, +} + +/// REST handler for adding a metric to multiple collections +/// +/// # Arguments +/// +/// * `user` - The authenticated user making the request +/// * `id` - The unique identifier of the metric +/// * `request` - The collection IDs to add the metric to +/// +/// # Returns +/// +/// A success message on success, or an appropriate error response +pub async fn add_metric_to_collections_rest_handler( + 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 POST request to add metric to collections" + ); + + match add_metric_to_collections_handler(&id, request.collection_ids, &user.id).await { + Ok(_) => Ok(ApiResponse::JsonData("Metric added to collections successfully".to_string())), + Err(e) => { + tracing::error!("Error adding metric to collections: {}", e); + + // Map specific errors to appropriate status codes + let error_message = e.to_string(); + + if error_message.contains("not found") { + if error_message.contains("Metric not found") { + return Err((StatusCode::NOT_FOUND, format!("Metric not found: {}", e))); + } else if error_message.contains("Collection not found") { + return Err((StatusCode::NOT_FOUND, format!("Collection 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 add metric to collections: {}", e))) + } + } +} \ 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 7b1c126aa..a82639b27 100644 --- a/api/src/routes/rest/routes/metrics/mod.rs +++ b/api/src/routes/rest/routes/metrics/mod.rs @@ -1,6 +1,7 @@ use axum::{routing::{get, put, delete, post}, Router}; // Import modules +mod add_metric_to_collections; mod delete_metric; mod get_metric; mod get_metric_data; @@ -23,5 +24,9 @@ pub fn router() -> Router { "/:id/dashboards", post(post_metric_dashboard::post_metric_dashboard_rest_handler), ) + .route( + "/:id/collections", + post(add_metric_to_collections::add_metric_to_collections_rest_handler), + ) .nest("/:id/sharing", sharing::router()) }