create assets to get collection

2025-03-20 09:46:49 -06:00 · 2025-03-20 09:46:49 -06:00 · 160fa5fa6f
parent a7d0f0d206
commit 160fa5fa6f
3 changed files with 321 additions and 4 deletions
--- 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<String>,
 }

+/// Type for querying asset data from database
+#[derive(Queryable)]
+struct AssetQueryResult {
+    id: Uuid,
+    name: String,
+    user_name: Option<String>,
+    email: String,
+    created_at: DateTime<Utc>,
+    updated_at: DateTime<Utc>,
+    asset_type: AssetType,
+}
+
 /// Handler for getting a single collection by ID
 ///
 /// # Arguments
@ -23,6 +36,27 @@ struct AssetPermissionInfo {
 ///
 /// # Returns
 /// * `Result<CollectionState>` - The collection state if found and accessible
+
+/// Format database asset query results into CollectionAsset objects
+fn format_assets(assets: Vec<AssetQueryResult>) -> Vec<CollectionAsset> {
+    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::<AssetQueryResult>(&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::<AssetQueryResult>(&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,
--- a/api/prds/active/api_add_assets_to_get_collection.md
+++ 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::<AssetQueryResult>(&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::<AssetQueryResult>(&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<AssetQueryResult>) -> Vec<CollectionAsset> {
+    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
--- 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::<diesel::sql_types::Uuid, _>(metric_id)
+        .bind::<diesel::sql_types::Uuid, _>(org_id)
+        .bind::<diesel::sql_types::Uuid, _>(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::<diesel::sql_types::Uuid, _>(dashboard_id)
+        .bind::<diesel::sql_types::Uuid, _>(org_id)
+        .bind::<diesel::sql_types::Uuid, _>(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::<diesel::sql_types::Uuid, _>(collection_id)
+        .bind::<diesel::sql_types::Uuid, _>(metric_id)
+        .bind::<diesel::sql_types::Text, _>("metric_file")
+        .bind::<diesel::sql_types::Uuid, _>(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::<diesel::sql_types::Uuid, _>(collection_id)
+        .bind::<diesel::sql_types::Uuid, _>(dashboard_id)
+        .bind::<diesel::sql_types::Text, _>("dashboard_file")
+        .bind::<diesel::sql_types::Uuid, _>(user_id)
+        .execute(&mut conn)
+        .await
+        .unwrap();
+    
    collection_id
 }