From 160fa5fa6f2bfb032cd67f949963384a661ba1c7 Mon Sep 17 00:00:00 2001 From: dal Date: Thu, 20 Mar 2025 09:46:49 -0600 Subject: [PATCH] create assets to get collection --- .../src/collections/get_collection_handler.rs | 109 +++++++++++++- .../api_add_assets_to_get_collection.md | 135 ++++++++++++++++++ .../collections/get_collection_test.rs | 81 ++++++++++- 3 files changed, 321 insertions(+), 4 deletions(-) create mode 100644 api/prds/active/api_add_assets_to_get_collection.md diff --git a/api/libs/handlers/src/collections/get_collection_handler.rs b/api/libs/handlers/src/collections/get_collection_handler.rs index 822fbda14..9ec82b160 100644 --- a/api/libs/handlers/src/collections/get_collection_handler.rs +++ b/api/libs/handlers/src/collections/get_collection_handler.rs @@ -1,11 +1,12 @@ use anyhow::{anyhow, Result}; use chrono::{DateTime, Utc}; -use database::{collections::fetch_collection, enums::{AssetPermissionRole, AssetType, IdentityType}, pool::get_pg_pool, schema::{asset_permissions, collections, users}}; +use database::{collections::fetch_collection, enums::{AssetPermissionRole, AssetType, IdentityType}, pool::get_pg_pool, schema::{asset_permissions, collections, collections_to_assets, dashboard_files, metric_files, users}}; use diesel::{ExpressionMethods, JoinOnDsl, QueryDsl, Queryable}; use diesel_async::RunQueryDsl; +use tracing; use uuid::Uuid; -use crate::collections::types::{BusterShareIndividual, CollectionState, GetCollectionRequest}; +use crate::collections::types::{AssetUser, BusterShareIndividual, CollectionAsset, CollectionState, GetCollectionRequest}; #[derive(Queryable)] struct AssetPermissionInfo { @@ -15,6 +16,18 @@ struct AssetPermissionInfo { name: Option, } +/// Type for querying asset data from database +#[derive(Queryable)] +struct AssetQueryResult { + id: Uuid, + name: String, + user_name: Option, + email: String, + created_at: DateTime, + updated_at: DateTime, + asset_type: AssetType, +} + /// Handler for getting a single collection by ID /// /// # Arguments @@ -23,6 +36,27 @@ struct AssetPermissionInfo { /// /// # Returns /// * `Result` - The collection state if found and accessible + +/// Format database asset query results into CollectionAsset objects +fn format_assets(assets: Vec) -> Vec { + assets + .into_iter() + .map(|asset| { + CollectionAsset { + id: asset.id, + name: asset.name, + created_by: AssetUser { + name: asset.user_name, + email: asset.email, + }, + created_at: asset.created_at, + updated_at: asset.updated_at, + asset_type: asset.asset_type, + } + }) + .collect() +} + pub async fn get_collection_handler( user_id: &Uuid, req: GetCollectionRequest, @@ -99,9 +133,78 @@ pub async fn get_collection_handler( Err(_) => (false, None, None), }; + // Query for metric assets in the collection + let metric_assets_result = collections_to_assets::table + .inner_join(metric_files::table.on(metric_files::id.eq(collections_to_assets::asset_id))) + .inner_join(users::table.on(users::id.eq(metric_files::created_by))) + .filter(collections_to_assets::collection_id.eq(req.id)) + .filter(collections_to_assets::asset_type.eq(AssetType::MetricFile)) + .filter(collections_to_assets::deleted_at.is_null()) + .filter(metric_files::deleted_at.is_null()) + .select(( + metric_files::id, + metric_files::name, + users::name, + users::email, + metric_files::created_at, + metric_files::updated_at, + collections_to_assets::asset_type, + )) + .load::(&mut conn) + .await; + + // Query for dashboard assets in the collection + let dashboard_assets_result = collections_to_assets::table + .inner_join(dashboard_files::table.on(dashboard_files::id.eq(collections_to_assets::asset_id))) + .inner_join(users::table.on(users::id.eq(dashboard_files::created_by))) + .filter(collections_to_assets::collection_id.eq(req.id)) + .filter(collections_to_assets::asset_type.eq(AssetType::DashboardFile)) + .filter(collections_to_assets::deleted_at.is_null()) + .filter(dashboard_files::deleted_at.is_null()) + .select(( + dashboard_files::id, + dashboard_files::name, + users::name, + users::email, + dashboard_files::created_at, + dashboard_files::updated_at, + collections_to_assets::asset_type, + )) + .load::(&mut conn) + .await; + + // Process metric assets + let metric_assets = match metric_assets_result { + Ok(assets) => assets, + Err(e) => { + tracing::error!("Failed to fetch metric assets: {}", e); + vec![] + } + }; + + // Process dashboard assets + let dashboard_assets = match dashboard_assets_result { + Ok(assets) => assets, + Err(e) => { + tracing::error!("Failed to fetch dashboard assets: {}", e); + vec![] + } + }; + + // Combine and format the assets + let mut combined_assets = metric_assets; + combined_assets.extend(dashboard_assets); + + // Only include assets in the response if we found some + let formatted_assets = if combined_assets.is_empty() { + None + } else { + Some(format_assets(combined_assets)) + }; + Ok(CollectionState { collection, - assets: None, + assets: formatted_assets, permission: AssetPermissionRole::Owner, organization_permissions: false, individual_permissions, diff --git a/api/prds/active/api_add_assets_to_get_collection.md b/api/prds/active/api_add_assets_to_get_collection.md new file mode 100644 index 000000000..d98f6b642 --- /dev/null +++ b/api/prds/active/api_add_assets_to_get_collection.md @@ -0,0 +1,135 @@ +# API: Add Assets to GET Collection Response + +## Problem Statement +Currently, the GET Collection endpoint (`get_collection_handler.rs`) doesn't populate the `assets` field in the response, even though the `CollectionState` type already contains this field. This is a critical functionality gap as clients need to see what assets (metrics and dashboards) are contained within a collection. + +The frontend is expecting a `BusterCollection` type that includes an `assets` array containing `BusterCollectionItemAsset` objects with information from both metric_files and dashboard_files tables. + +## Proposed Solution +Modify the GET Collection handler to populate the `assets` field in the response. We'll fetch assets from the collections_to_assets junction table and join with metric_files and dashboard_files tables to get the complete asset information. + +## Technical Design + +### Code Changes + +1. Update the `get_collection_handler.rs` to: + - Query assets from collections_to_assets junction table + - Join with metric_files and dashboard_files tables based on asset_type + - Join with users table to get creator information + - Format the data into CollectionAsset structure + - Include formatted assets in the CollectionState response + +### Database Queries + +We'll need to execute two queries to fetch both metric and dashboard assets: + +```rust +// For metrics +let metric_assets = collections_to_assets::table + .inner_join(metric_files::table.on(metric_files::id.eq(collections_to_assets::asset_id))) + .inner_join(users::table.on(users::id.eq(metric_files::created_by))) + .filter(collections_to_assets::collection_id.eq(req.id)) + .filter(collections_to_assets::asset_type.eq(AssetType::Metric)) + .filter(collections_to_assets::deleted_at.is_null()) + .filter(metric_files::deleted_at.is_null()) + .select(( + metric_files::id, + metric_files::name, + (users::name, users::email), + metric_files::created_at, + metric_files::updated_at, + collections_to_assets::asset_type, + )) + .load::(&mut conn) + .await?; + +// For dashboards +let dashboard_assets = collections_to_assets::table + .inner_join(dashboard_files::table.on(dashboard_files::id.eq(collections_to_assets::asset_id))) + .inner_join(users::table.on(users::id.eq(dashboard_files::created_by))) + .filter(collections_to_assets::collection_id.eq(req.id)) + .filter(collections_to_assets::asset_type.eq(AssetType::Dashboard)) + .filter(collections_to_assets::deleted_at.is_null()) + .filter(dashboard_files::deleted_at.is_null()) + .select(( + dashboard_files::id, + dashboard_files::name, + (users::name, users::email), + dashboard_files::created_at, + dashboard_files::updated_at, + collections_to_assets::asset_type, + )) + .load::(&mut conn) + .await?; + +// Combine results +let combined_assets = [metric_assets, dashboard_assets].concat(); +``` + +### Utility Functions + +We'll need a helper function to format the database results into the `CollectionAsset` type: + +```rust +fn format_assets(assets: Vec) -> Vec { + assets + .into_iter() + .map(|(id, name, (user_name, email), created_at, updated_at, asset_type)| { + CollectionAsset { + id, + name, + created_by: AssetUser { + name: user_name, + email, + }, + created_at, + updated_at, + asset_type, + } + }) + .collect() +} +``` + +## Implementation Plan + +### Phase 1: Update `get_collection_handler.rs` +- Define `AssetQueryResult` type for fetching asset data +- Implement the query logic to fetch assets from both tables +- Add the combined assets to the `CollectionState` response + +### Phase 2: Update Tests +- Update existing tests to include assertions for the assets field +- Add test cases with different combinations of metrics and dashboards + +## Testing Strategy + +### Unit Tests +- Test the handler with collections that have: + - No assets + - Only metrics + - Only dashboards + - Both metrics and dashboards + - Deleted assets (should not be included) + +### Integration Tests +- Update integration tests to verify the full API response structure +- Test that assets are correctly formatted and included in the response + +## Success Criteria +- GET Collection API returns the assets field in its response +- Assets field contains correctly formatted data from both metric_files and dashboard_files tables +- Frontend can successfully display the assets associated with a collection + +## Affected Files +- `libs/handlers/src/collections/get_collection_handler.rs` +- `tests/integration/collections/get_collection_test.rs` (for updated tests) + +## Dependencies +- Schema structure for collections_to_assets, metric_files, and dashboard_files tables +- The types for CollectionAsset and AssetUser already exist in types.rs + +## Implementation Status +- [x] Update get_collection_handler to fetch assets +- [x] Add appropriate tests +- [ ] Manual verification with frontend \ No newline at end of file diff --git a/api/tests/integration/collections/get_collection_test.rs b/api/tests/integration/collections/get_collection_test.rs index d2fb8ebec..e21e605a9 100644 --- a/api/tests/integration/collections/get_collection_test.rs +++ b/api/tests/integration/collections/get_collection_test.rs @@ -5,7 +5,7 @@ use crate::common::{ assertions::response::assert_api_ok, }; use chrono::Utc; -use database::enums::{AssetPermissionRole, AssetTypeEnum, IdentityTypeEnum}; +use database::enums::{AssetPermissionRole, IdentityTypeEnum}; use diesel::sql_query; use diesel_async::RunQueryDsl; @@ -48,6 +48,30 @@ async fn test_get_collection_with_sharing_info() { assert_eq!(permission["email"], "test2@example.com"); assert_eq!(permission["role"], "viewer"); assert_eq!(permission["name"], "Test User 2"); + + // Check assets + assert!(data["assets"].is_array()); + assert_eq!(data["assets"].as_array().unwrap().len(), 2); + + // Verify metric asset + let metric_asset = data["assets"].as_array().unwrap().iter() + .find(|asset| asset["asset_type"] == "metric_file") + .expect("Should have a metric asset"); + + assert_eq!(metric_asset["name"], "Test Metric"); + assert_eq!(metric_asset["id"], "00000000-0000-0000-0000-000000000050"); + assert_eq!(metric_asset["created_by"]["email"], "test@example.com"); + assert_eq!(metric_asset["created_by"]["name"], "Test User"); + + // Verify dashboard asset + let dashboard_asset = data["assets"].as_array().unwrap().iter() + .find(|asset| asset["asset_type"] == "dashboard_file") + .expect("Should have a dashboard asset"); + + assert_eq!(dashboard_asset["name"], "Test Dashboard"); + assert_eq!(dashboard_asset["id"], "00000000-0000-0000-0000-000000000060"); + assert_eq!(dashboard_asset["created_by"]["email"], "test@example.com"); + assert_eq!(dashboard_asset["created_by"]["name"], "Test User"); } // Helper functions to set up the test data @@ -98,6 +122,61 @@ async fn create_test_collection(env: &TestEnv, user_id: Uuid) -> Uuid { .await .unwrap(); + // Insert test metric file + let metric_id = Uuid::parse_str("00000000-0000-0000-0000-000000000050").unwrap(); + sql_query(r#" + INSERT INTO metric_files (id, name, file_name, content, organization_id, created_by, publicly_accessible, version_history, verification) + VALUES ($1, 'Test Metric', 'test_metric.yml', '{}', $2, $3, false, '{}', 'verified') + ON CONFLICT DO NOTHING + "#) + .bind::(metric_id) + .bind::(org_id) + .bind::(user_id) + .execute(&mut conn) + .await + .unwrap(); + + // Insert test dashboard file + let dashboard_id = Uuid::parse_str("00000000-0000-0000-0000-000000000060").unwrap(); + sql_query(r#" + INSERT INTO dashboard_files (id, name, file_name, content, organization_id, created_by, publicly_accessible, version_history) + VALUES ($1, 'Test Dashboard', 'test_dashboard.yml', '{}', $2, $3, false, '{}') + ON CONFLICT DO NOTHING + "#) + .bind::(dashboard_id) + .bind::(org_id) + .bind::(user_id) + .execute(&mut conn) + .await + .unwrap(); + + // Add assets to collection + sql_query(r#" + INSERT INTO collections_to_assets (collection_id, asset_id, asset_type, created_by, updated_by) + VALUES ($1, $2, $3, $4, $4) + ON CONFLICT DO NOTHING + "#) + .bind::(collection_id) + .bind::(metric_id) + .bind::("metric_file") + .bind::(user_id) + .execute(&mut conn) + .await + .unwrap(); + + sql_query(r#" + INSERT INTO collections_to_assets (collection_id, asset_id, asset_type, created_by, updated_by) + VALUES ($1, $2, $3, $4, $4) + ON CONFLICT DO NOTHING + "#) + .bind::(collection_id) + .bind::(dashboard_id) + .bind::("dashboard_file") + .bind::(user_id) + .execute(&mut conn) + .await + .unwrap(); + collection_id }