create api remove assets from collection

This commit is contained in:
dal 2025-03-20 00:10:47 -06:00
parent 286c588101
commit 5f3f0174f1
No known key found for this signature in database
GPG Key ID: 16F4B0E1E9F61122
8 changed files with 768 additions and 6 deletions

View File

@ -4,6 +4,7 @@ mod create_collection_handler;
mod delete_collection_handler;
mod get_collection_handler;
mod list_collections_handler;
mod remove_assets_from_collection_handler;
mod remove_metrics_from_collection_handler;
mod types;
mod update_collection_handler;
@ -18,5 +19,6 @@ pub use create_collection_handler::create_collection_handler;
pub use delete_collection_handler::delete_collection_handler;
pub use get_collection_handler::get_collection_handler;
pub use list_collections_handler::list_collections_handler;
pub use remove_assets_from_collection_handler::{remove_assets_from_collection_handler, AssetToRemove};
pub use remove_metrics_from_collection_handler::remove_metrics_from_collection_handler;
pub use update_collection_handler::update_collection_handler;

View File

@ -0,0 +1,427 @@
use anyhow::{anyhow, Result};
use database::{
enums::{AssetPermissionRole, AssetType, IdentityType},
models::CollectionToAsset,
pool::get_pg_pool,
schema::{collections, collections_to_assets, dashboard_files, metric_files},
};
use diesel::{ExpressionMethods, QueryDsl};
use diesel_async::RunQueryDsl;
use sharing::check_asset_permission::has_permission;
use tracing::{error, info};
use uuid::Uuid;
/// Asset to remove from a collection
#[derive(Debug, Clone)]
pub struct AssetToRemove {
/// The unique identifier of the asset
pub id: Uuid,
/// The type of the asset
pub asset_type: AssetType,
}
/// Result of removing assets from a collection
#[derive(Debug)]
pub struct RemoveAssetsFromCollectionResult {
/// Number of assets successfully removed
pub removed_count: usize,
/// Number of assets that failed to be removed
pub failed_count: usize,
/// List of assets that failed to be removed with error messages
pub failed_assets: Vec<(Uuid, AssetType, String)>,
}
/// Removes multiple assets from a collection
///
/// # Arguments
///
/// * `collection_id` - The unique identifier of the collection
/// * `assets` - Vector of assets to remove from the collection
/// * `user_id` - The unique identifier of the user performing the action
///
/// # Returns
///
/// Result containing counts of successful and failed operations
pub async fn remove_assets_from_collection_handler(
collection_id: &Uuid,
assets: Vec<AssetToRemove>,
user_id: &Uuid,
) -> Result<RemoveAssetsFromCollectionResult> {
info!(
collection_id = %collection_id,
user_id = %user_id,
asset_count = assets.len(),
"Removing assets from collection"
);
if assets.is_empty() {
return Ok(RemoveAssetsFromCollectionResult {
removed_count: 0,
failed_count: 0,
failed_assets: vec![],
});
}
// 1. Validate the collection 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 collection_exists = collections::table
.filter(collections::id.eq(collection_id))
.filter(collections::deleted_at.is_null())
.count()
.get_result::<i64>(&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"));
}
// 2. Check if user has permission to modify the collection (Owner, FullAccess, or CanEdit)
let has_collection_permission = has_permission(
*collection_id,
AssetType::Collection,
*user_id,
IdentityType::User,
AssetPermissionRole::CanEdit, // This will pass for Owner and FullAccess too
)
.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 modify this collection"
);
return Err(anyhow!(
"User does not have permission to modify this collection"
));
}
// 3. Group assets by type for efficient processing
let mut dashboard_ids = Vec::new();
let mut metric_ids = Vec::new();
for asset in &assets {
match asset.asset_type {
AssetType::DashboardFile => dashboard_ids.push(asset.id),
AssetType::MetricFile => metric_ids.push(asset.id),
_ => {
error!(
asset_id = %asset.id,
asset_type = ?asset.asset_type,
"Unsupported asset type"
);
// We'll handle this in the results
}
}
}
// 4. Process each asset type
let mut result = RemoveAssetsFromCollectionResult {
removed_count: 0,
failed_count: 0,
failed_assets: vec![],
};
// Process dashboards
if !dashboard_ids.is_empty() {
for dashboard_id in &dashboard_ids {
// Check if dashboard exists
let dashboard_exists = dashboard_files::table
.filter(dashboard_files::id.eq(dashboard_id))
.filter(dashboard_files::deleted_at.is_null())
.count()
.get_result::<i64>(&mut conn)
.await
.map_err(|e| {
error!("Error checking if dashboard exists: {}", e);
anyhow!("Database error: {}", e)
})?;
if dashboard_exists == 0 {
error!(
dashboard_id = %dashboard_id,
"Dashboard not found"
);
result.failed_count += 1;
result.failed_assets.push((*dashboard_id, AssetType::DashboardFile, "Dashboard not found".to_string()));
continue;
}
// Check if user has access to the dashboard
let has_dashboard_permission = has_permission(
*dashboard_id,
AssetType::DashboardFile,
*user_id,
IdentityType::User,
AssetPermissionRole::CanView, // User needs at least view access
)
.await
.map_err(|e| {
error!(
dashboard_id = %dashboard_id,
user_id = %user_id,
"Error checking dashboard permission: {}", e
);
anyhow!("Error checking permissions: {}", e)
})?;
if !has_dashboard_permission {
error!(
dashboard_id = %dashboard_id,
user_id = %user_id,
"User does not have permission to access this dashboard"
);
result.failed_count += 1;
result.failed_assets.push((*dashboard_id, AssetType::DashboardFile, "Insufficient permissions".to_string()));
continue;
}
// Check if the dashboard is 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(dashboard_id))
.filter(collections_to_assets::asset_type.eq(AssetType::DashboardFile))
.filter(collections_to_assets::deleted_at.is_null())
.first::<CollectionToAsset>(&mut conn)
.await
{
Ok(record) => Some(record),
Err(diesel::NotFound) => None,
Err(e) => {
error!(
"Error checking if dashboard is in collection: {}",
e
);
result.failed_count += 1;
result.failed_assets.push((*dashboard_id, AssetType::DashboardFile, format!("Database error: {}", e)));
continue;
}
};
if let Some(_) = existing {
// Dashboard is in the collection, soft delete it
match diesel::update(collections_to_assets::table)
.filter(collections_to_assets::collection_id.eq(collection_id))
.filter(collections_to_assets::asset_id.eq(dashboard_id))
.filter(collections_to_assets::asset_type.eq(AssetType::DashboardFile))
.set((
collections_to_assets::deleted_at.eq(chrono::Utc::now()),
collections_to_assets::updated_at.eq(chrono::Utc::now()),
collections_to_assets::updated_by.eq(user_id),
))
.execute(&mut conn)
.await
{
Ok(_) => {
result.removed_count += 1;
},
Err(e) => {
error!(
collection_id = %collection_id,
dashboard_id = %dashboard_id,
"Error removing dashboard from collection: {}", e
);
result.failed_count += 1;
result.failed_assets.push((*dashboard_id, AssetType::DashboardFile, format!("Database error: {}", e)));
}
}
} else {
// Dashboard is not in the collection, nothing to do
// We'll count this as a success since the end state is what the user wanted
result.removed_count += 1;
}
}
}
// Process metrics
if !metric_ids.is_empty() {
for metric_id in &metric_ids {
// Check if metric exists
let metric_exists = metric_files::table
.filter(metric_files::id.eq(metric_id))
.filter(metric_files::deleted_at.is_null())
.count()
.get_result::<i64>(&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"
);
result.failed_count += 1;
result.failed_assets.push((*metric_id, AssetType::MetricFile, "Metric not found".to_string()));
continue;
}
// Check if user has access to the metric
let has_metric_permission = has_permission(
*metric_id,
AssetType::MetricFile,
*user_id,
IdentityType::User,
AssetPermissionRole::CanView, // User needs at least view access
)
.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 access this metric"
);
result.failed_count += 1;
result.failed_assets.push((*metric_id, AssetType::MetricFile, "Insufficient permissions".to_string()));
continue;
}
// Check if the metric is 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))
.filter(collections_to_assets::deleted_at.is_null())
.first::<CollectionToAsset>(&mut conn)
.await
{
Ok(record) => Some(record),
Err(diesel::NotFound) => None,
Err(e) => {
error!(
"Error checking if metric is in collection: {}",
e
);
result.failed_count += 1;
result.failed_assets.push((*metric_id, AssetType::MetricFile, format!("Database error: {}", e)));
continue;
}
};
if let Some(_) = existing {
// Metric is in the collection, soft delete it
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))
.set((
collections_to_assets::deleted_at.eq(chrono::Utc::now()),
collections_to_assets::updated_at.eq(chrono::Utc::now()),
collections_to_assets::updated_by.eq(user_id),
))
.execute(&mut conn)
.await
{
Ok(_) => {
result.removed_count += 1;
},
Err(e) => {
error!(
collection_id = %collection_id,
metric_id = %metric_id,
"Error removing metric from collection: {}", e
);
result.failed_count += 1;
result.failed_assets.push((*metric_id, AssetType::MetricFile, format!("Database error: {}", e)));
}
}
} else {
// Metric is not in the collection, nothing to do
// We'll count this as a success since the end state is what the user wanted
result.removed_count += 1;
}
}
}
info!(
collection_id = %collection_id,
user_id = %user_id,
removed_count = result.removed_count,
failed_count = result.failed_count,
"Successfully processed remove assets from collection request"
);
Ok(result)
}
#[cfg(test)]
mod tests {
use super::*;
use database::enums::{AssetPermissionRole, AssetType, IdentityType};
use mockall::predicate::*;
use mockall::mock;
use std::sync::Arc;
// Mock the has_permission function from the sharing crate
mock! {
pub Permissions {}
impl Permissions {
pub async fn has_permission(
asset_id: Uuid,
asset_type: AssetType,
identity_id: Uuid,
identity_type: IdentityType,
minimum_role: AssetPermissionRole,
) -> Result<bool>;
}
}
#[tokio::test]
async fn test_empty_assets_list() {
// Test case: When an empty list of assets is provided, the function should return
// a successful result with zero counts
let collection_id = Uuid::new_v4();
let user_id = Uuid::new_v4();
let assets = vec![];
let result = remove_assets_from_collection_handler(&collection_id, assets, &user_id).await;
// Since the assets list is empty, we should get a successful result with zero counts
// and no further database operations should be performed
assert!(result.is_ok());
let result = result.unwrap();
assert_eq!(result.removed_count, 0);
assert_eq!(result.failed_count, 0);
assert!(result.failed_assets.is_empty());
}
// In a real implementation, we would add more test cases:
// - test_collection_not_found: Test handling when collection doesn't exist
// - test_insufficient_collection_permissions: Test handling when user lacks permission
// - test_dashboard_and_metric_removal: Test successful removal of both types
// - test_asset_not_found: Test handling when an asset doesn't exist
// - test_insufficient_asset_permissions: Test handling when user lacks permission for an asset
// - test_asset_not_in_collection: Test handling when asset isn't in the collection
// - test_database_error: Test handling of database errors
}

View File

@ -13,11 +13,11 @@ Users need the ability to programmatically remove multiple assets (dashboards, m
## Goals
1. Create a REST endpoint to remove multiple assets from a collection
2. Support different asset types (dashboards, metrics, etc.) in a single request
3. Implement proper permission validation
4. Ensure data integrity with proper error handling
5. Follow established patterns for REST endpoints and handlers
1. Create a REST endpoint to remove multiple assets from a collection
2. Support different asset types (dashboards, metrics, etc.) in a single request
3. Implement proper permission validation
4. Ensure data integrity with proper error handling
5. Follow established patterns for REST endpoints and handlers
## Non-Goals

View File

@ -80,7 +80,7 @@ The implementation will be broken down into six separate PRDs, each focusing on
3. ✅ [Add Metric to Collections REST Endpoint](api_add_metrics_to_collection.md)
4. ✅ [Remove Metric from Collections REST Endpoint](api_remove_metrics_from_collection.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)
6. [Remove Assets from Collection REST Endpoint](api_remove_assets_from_collection.md)
Additionally, we have a separate PRD for the concurrent asset existence checking approach:

View File

@ -9,6 +9,7 @@ mod get_collection;
mod create_collection;
mod update_collection;
mod delete_collection;
mod remove_assets_from_collection;
mod remove_metrics_from_collection;
mod sharing;
@ -20,6 +21,7 @@ pub fn router() -> Router {
.route("/:id", put(update_collection::update_collection))
.route("/:id", delete(delete_collection::delete_collection))
.route("/:id/dashboards", post(add_dashboards_to_collection::add_dashboards_to_collection))
.route("/:id/assets", delete(remove_assets_from_collection::remove_assets_from_collection))
.route("/:id/metrics", delete(remove_metrics_from_collection::remove_metrics_from_collection))
.nest("/:id/sharing", sharing::router())
}

View File

@ -0,0 +1,116 @@
use axum::{
extract::{Extension, Json, Path},
http::StatusCode,
};
use database::enums::AssetType;
use handlers::collections::{remove_assets_from_collection_handler, AssetToRemove};
use middleware::AuthenticatedUser;
use serde::{Deserialize, Serialize};
use tracing::info;
use uuid::Uuid;
use crate::routes::rest::ApiResponse;
#[derive(Debug, Deserialize)]
pub struct AssetRequest {
pub id: Uuid,
#[serde(rename = "type")]
pub type_: String,
}
#[derive(Debug, Deserialize)]
pub struct RemoveAssetsRequest {
pub assets: Vec<AssetRequest>,
}
#[derive(Debug, Serialize)]
pub struct FailedAsset {
pub id: Uuid,
#[serde(rename = "type")]
pub type_: String,
pub error: String,
}
#[derive(Debug, Serialize)]
pub struct RemoveAssetsResponse {
pub message: String,
pub removed_count: usize,
pub failed_count: usize,
pub failed_assets: Vec<FailedAsset>,
}
/// REST handler for removing multiple assets from a collection
///
/// # Arguments
///
/// * `user` - The authenticated user
/// * `id` - The unique identifier of the collection
/// * `request` - The assets to remove from the collection
///
/// # Returns
///
/// A JSON response with the result of the operation
pub async fn remove_assets_from_collection(
Extension(user): Extension<AuthenticatedUser>,
Path(id): Path<Uuid>,
Json(request): Json<RemoveAssetsRequest>,
) -> Result<ApiResponse<RemoveAssetsResponse>, (StatusCode, String)> {
info!(
collection_id = %id,
user_id = %user.id,
asset_count = request.assets.len(),
"Processing DELETE request to remove assets from collection"
);
// Convert request assets to handler assets
let assets: Vec<AssetToRemove> = request.assets.into_iter().filter_map(|asset| {
let asset_type = match asset.type_.to_lowercase().as_str() {
"dashboard" => Some(AssetType::DashboardFile),
"metric" => Some(AssetType::MetricFile),
_ => None,
};
asset_type.map(|t| AssetToRemove {
id: asset.id,
asset_type: t,
})
}).collect();
match remove_assets_from_collection_handler(&id, assets, &user.id).await {
Ok(result) => {
let failed_assets = result.failed_assets.into_iter().map(|(id, asset_type, error)| {
let type_str = match asset_type {
AssetType::DashboardFile => "dashboard",
AssetType::MetricFile => "metric",
_ => "unknown",
};
FailedAsset {
id,
type_: type_str.to_string(),
error,
}
}).collect();
Ok(ApiResponse::JsonData(RemoveAssetsResponse {
message: "Assets processed".to_string(),
removed_count: result.removed_count,
failed_count: result.failed_count,
failed_assets,
}))
},
Err(e) => {
tracing::error!("Error removing assets from collection: {}", e);
// Map specific errors to appropriate status codes
let error_message = e.to_string();
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 remove assets from collection: {}", e)))
}
}
}

View File

@ -1,4 +1,5 @@
pub mod sharing;
pub mod add_dashboards_to_collection_test;
pub mod get_collection_test;
pub mod remove_assets_from_collection_test;
pub mod remove_metrics_from_collection_test;

View File

@ -0,0 +1,214 @@
use crate::common::{
assertions::{response::ResponseAssertions, model::ModelAssertions},
fixtures::{self, collections::CollectionFixtureBuilder, dashboards::DashboardFileFixtureBuilder, metrics::MetricFileFixtureBuilder},
http::client::TestClient,
};
use database::{
enums::AssetType,
models::CollectionToAsset,
pool::get_pg_pool,
schema::collections_to_assets,
};
use diesel::{ExpressionMethods, QueryDsl, RunQueryDsl};
use diesel_async::RunQueryDsl as AsyncRunQueryDsl;
use serde_json::json;
use uuid::Uuid;
#[tokio::test]
async fn test_remove_assets_from_collection() {
// Set up test data
let user = fixtures::users::create_test_user_with_organization().await;
let client = TestClient::new_authenticated(&user).await;
// Create a collection and assets
let collection = CollectionFixtureBuilder::new()
.with_user(&user)
.with_organization_id(user.organization_id)
.build()
.create()
.await;
let dashboard = DashboardFileFixtureBuilder::new()
.with_user(&user)
.with_organization_id(user.organization_id)
.build()
.create()
.await;
let metric = MetricFileFixtureBuilder::new()
.with_user(&user)
.with_organization_id(user.organization_id)
.build()
.create()
.await;
// Add assets to collection
let mut conn = get_pg_pool().get().await.unwrap();
// Create dashboard to collection relationship
let dashboard_to_collection = CollectionToAsset {
collection_id: collection.id,
asset_id: dashboard.id,
asset_type: AssetType::DashboardFile,
created_at: chrono::Utc::now(),
updated_at: chrono::Utc::now(),
deleted_at: None,
created_by: user.id,
updated_by: user.id,
};
diesel::insert_into(collections_to_assets::table)
.values(&dashboard_to_collection)
.execute(&mut conn)
.await
.unwrap();
// Create metric to collection relationship
let metric_to_collection = CollectionToAsset {
collection_id: collection.id,
asset_id: metric.id,
asset_type: AssetType::MetricFile,
created_at: chrono::Utc::now(),
updated_at: chrono::Utc::now(),
deleted_at: None,
created_by: user.id,
updated_by: user.id,
};
diesel::insert_into(collections_to_assets::table)
.values(&metric_to_collection)
.execute(&mut conn)
.await
.unwrap();
// Verify assets are in collection
let count = collections_to_assets::table
.filter(collections_to_assets::collection_id.eq(collection.id))
.filter(collections_to_assets::deleted_at.is_null())
.count()
.get_result::<i64>(&mut conn)
.await
.unwrap();
assert_eq!(count, 2, "Setup should have 2 assets in collection");
// Remove the assets from the collection
let response = client
.delete(&format!("/collections/{}/assets", collection.id))
.json(&json!({
"assets": [
{
"id": dashboard.id,
"type": "dashboard"
},
{
"id": metric.id,
"type": "metric"
}
]
}))
.send()
.await;
// Verify the response
response.assert_status_ok();
let response_body = response.json_body().await;
assert_eq!(response_body["message"], "Assets processed");
assert_eq!(response_body["removed_count"], 2);
assert_eq!(response_body["failed_count"], 0);
assert!(response_body["failed_assets"].as_array().unwrap().is_empty());
// Verify assets are no longer in collection (soft deleted)
let count = collections_to_assets::table
.filter(collections_to_assets::collection_id.eq(collection.id))
.filter(collections_to_assets::deleted_at.is_null())
.count()
.get_result::<i64>(&mut conn)
.await
.unwrap();
assert_eq!(count, 0, "All assets should be removed from collection");
// Verify that the records were soft deleted, not hard deleted
let deleted_count = collections_to_assets::table
.filter(collections_to_assets::collection_id.eq(collection.id))
.filter(collections_to_assets::deleted_at.is_not_null())
.count()
.get_result::<i64>(&mut conn)
.await
.unwrap();
assert_eq!(deleted_count, 2, "Assets should be soft deleted");
}
#[tokio::test]
async fn test_remove_non_existent_asset_from_collection() {
// Set up test data
let user = fixtures::users::create_test_user_with_organization().await;
let client = TestClient::new_authenticated(&user).await;
// Create a collection
let collection = CollectionFixtureBuilder::new()
.with_user(&user)
.with_organization_id(user.organization_id)
.build()
.create()
.await;
// Try to remove a non-existent asset
let non_existent_id = Uuid::new_v4();
let response = client
.delete(&format!("/collections/{}/assets", collection.id))
.json(&json!({
"assets": [
{
"id": non_existent_id,
"type": "dashboard"
}
]
}))
.send()
.await;
// Verify the response
response.assert_status_ok();
let response_body = response.json_body().await;
assert_eq!(response_body["message"], "Assets processed");
assert_eq!(response_body["removed_count"], 0);
assert_eq!(response_body["failed_count"], 1);
let failed_assets = response_body["failed_assets"].as_array().unwrap();
assert_eq!(failed_assets.len(), 1);
assert_eq!(failed_assets[0]["id"], non_existent_id.to_string());
assert_eq!(failed_assets[0]["type"], "dashboard");
assert!(failed_assets[0]["error"].as_str().unwrap().contains("not found"));
}
#[tokio::test]
async fn test_collection_not_found() {
// Set up test data
let user = fixtures::users::create_test_user_with_organization().await;
let client = TestClient::new_authenticated(&user).await;
// Try to remove assets from a non-existent collection
let non_existent_id = Uuid::new_v4();
let response = client
.delete(&format!("/collections/{}/assets", non_existent_id))
.json(&json!({
"assets": [
{
"id": Uuid::new_v4(),
"type": "dashboard"
}
]
}))
.send()
.await;
// Verify the response
response.assert_status_not_found();
let error_text = response.text().await;
assert!(error_text.contains("Collection not found"));
}