Merge branch 'evals' of https://github.com/buster-so/buster into evals

api/libs/database/src/types/metric_yml.rs

@@ -11,6 +11,8 @@ use serde::{Deserialize, Serialize};
 use serde_json::Value;
 use std::io::Write;
 use uuid::Uuid;
+use crate::types::{DataMetadata, ColumnMetaData, SimpleType, ColumnType};
+use serde_json::json;
 
 #[derive(Debug, Serialize, Deserialize, Clone, FromSqlRow, AsExpression)]
 #[diesel(sql_type = Jsonb)]
@@ -373,6 +375,102 @@ pub struct ColumnLabelFormat {
     pub convert_number_to: Option<String>,
 }
 
+impl ColumnLabelFormat {
+    /// Creates a new ColumnLabelFormat with appropriate defaults for the given type
+    pub fn new_for_type(simple_type: &crate::types::SimpleType) -> Self {
+        match simple_type {
+            crate::types::SimpleType::Number => Self::new_number(),
+            crate::types::SimpleType::String => Self::new_string(),
+            crate::types::SimpleType::Date => Self::new_date(),
+            crate::types::SimpleType::Boolean => Self::new_boolean(),
+            crate::types::SimpleType::Other => Self::new_string(),
+        }
+    }
+
+    /// Creates a new ColumnLabelFormat for number type
+    pub fn new_number() -> Self {
+        Self {
+            column_type: "number".to_string(),
+            style: "number".to_string(),
+            display_name: None,
+            number_separator_style: Some(",".to_string()),
+            minimum_fraction_digits: Some(0),
+            maximum_fraction_digits: Some(2),
+            multiplier: None,
+            prefix: None,
+            suffix: None,
+            replace_missing_data_with: None,
+            compact_numbers: None,
+            currency: None,
+            date_format: None,
+            use_relative_time: None,
+            is_utc: None,
+            convert_number_to: None,
+        }
+    }
+
+    /// Creates a new ColumnLabelFormat for string type
+    pub fn new_string() -> Self {
+        Self {
+            column_type: "string".to_string(),
+            style: "string".to_string(),
+            display_name: None,
+            number_separator_style: None,
+            minimum_fraction_digits: None,
+            maximum_fraction_digits: None,
+            multiplier: None,
+            prefix: None,
+            suffix: None,
+            replace_missing_data_with: None,
+            compact_numbers: None,
+            currency: None,
+            date_format: None,
+            use_relative_time: None,
+            is_utc: None,
+            convert_number_to: None,
+        }
+    }
+
+    /// Creates a new ColumnLabelFormat for date type
+    pub fn new_date() -> Self {
+        Self {
+            column_type: "date".to_string(),
+            style: "date".to_string(),
+            display_name: None,
+            number_separator_style: None,
+            minimum_fraction_digits: None,
+            maximum_fraction_digits: None,
+            multiplier: None,
+            prefix: None,
+            suffix: None,
+            replace_missing_data_with: None,
+            compact_numbers: None,
+            currency: None,
+            date_format: Some("auto".to_string()),
+            use_relative_time: None,
+            is_utc: Some(false),
+            convert_number_to: None,
+        }
+    }
+
+    /// Creates a new ColumnLabelFormat for boolean type
+    pub fn new_boolean() -> Self {
+        Self::new_string() // Booleans use string formatting by default
+    }
+    
+    /// Generate column formats from data metadata
+    pub fn generate_formats_from_metadata(metadata: &crate::types::DataMetadata) -> indexmap::IndexMap<String, Self> {
+        let mut formats = indexmap::IndexMap::new();
+        
+        for column in &metadata.column_metadata {
+            let format = Self::new_for_type(&column.simple_type);
+            formats.insert(column.name.clone(), format);
+        }
+        
+        formats
+    }
+}
+
 #[derive(Debug, Serialize, Deserialize, Clone)]
 #[serde(rename_all = "camelCase")]
 pub struct ColumnSettings {
@@ -711,4 +809,85 @@ mod tests {
 
         Ok(())
     }
+
+    #[test]
+    fn test_column_label_format_constructors() {
+        // Test number format
+        let number_format = ColumnLabelFormat::new_number();
+        assert_eq!(number_format.column_type, "number");
+        assert_eq!(number_format.style, "number");
+        assert_eq!(number_format.number_separator_style, Some(",".to_string()));
+        assert_eq!(number_format.minimum_fraction_digits, Some(0));
+        assert_eq!(number_format.maximum_fraction_digits, Some(2));
+        
+        // Test string format
+        let string_format = ColumnLabelFormat::new_string();
+        assert_eq!(string_format.column_type, "string");
+        assert_eq!(string_format.style, "string");
+        assert_eq!(string_format.number_separator_style, None);
+        
+        // Test date format
+        let date_format = ColumnLabelFormat::new_date();
+        assert_eq!(date_format.column_type, "date");
+        assert_eq!(date_format.style, "date");
+        assert_eq!(date_format.date_format, Some("auto".to_string()));
+        assert_eq!(date_format.is_utc, Some(false));
+        
+        // Test boolean format - should be same as string
+        let boolean_format = ColumnLabelFormat::new_boolean();
+        assert_eq!(boolean_format.column_type, "string");
+        assert_eq!(boolean_format.style, "string");
+    }
+    
+    #[test]
+    fn test_generate_formats_from_metadata() {
+        // Create test metadata
+        let metadata = DataMetadata {
+            column_count: 3,
+            row_count: 10,
+            column_metadata: vec![
+                ColumnMetaData {
+                    name: "id".to_string(),
+                    min_value: json!(1),
+                    max_value: json!(100),
+                    unique_values: 10,
+                    simple_type: SimpleType::Number,
+                    column_type: ColumnType::Int4,
+                },
+                ColumnMetaData {
+                    name: "name".to_string(),
+                    min_value: json!("A"),
+                    max_value: json!("Z"),
+                    unique_values: 5,
+                    simple_type: SimpleType::String,
+                    column_type: ColumnType::Varchar,
+                },
+                ColumnMetaData {
+                    name: "created_at".to_string(),
+                    min_value: json!("2023-01-01T00:00:00"),
+                    max_value: json!("2023-12-31T23:59:59"),
+                    unique_values: 10,
+                    simple_type: SimpleType::Date,
+                    column_type: ColumnType::Timestamp,
+                },
+            ],
+        };
+        
+        // Generate formats
+        let formats = ColumnLabelFormat::generate_formats_from_metadata(&metadata);
+        
+        // Check we have formats for all columns
+        assert_eq!(formats.len(), 3);
+        
+        // Check individual formats
+        let id_format = formats.get("id").unwrap();
+        assert_eq!(id_format.column_type, "number");
+        
+        let name_format = formats.get("name").unwrap();
+        assert_eq!(name_format.column_type, "string");
+        
+        let date_format = formats.get("created_at").unwrap();
+        assert_eq!(date_format.column_type, "date");
+        assert_eq!(date_format.style, "date");
+    }
 }

api/libs/handlers/src/metrics/update_metric_handler.rs

@@ -5,7 +5,7 @@ use database::{
     helpers::metric_files::fetch_metric_file_with_permissions,
     pool::get_pg_pool,
     schema::{datasets, metric_files},
-    types::{MetricYml, VersionContent, VersionHistory},
+    types::{ColumnLabelFormat, MetricYml, VersionContent, VersionHistory},
 };
 use diesel::{ExpressionMethods, QueryDsl};
 use diesel_async::RunQueryDsl;
@@ -103,23 +103,30 @@ pub async fn update_metric_handler(
         .await
         .map_err(|e| anyhow!("Failed to fetch metric file with permissions: {}", e))?;
 
-    let (permission, organization_id) = if let Some(file_with_permission) = metric_file_with_permissions {
-        (
-            file_with_permission.permission,
-            file_with_permission.metric_file.organization_id,
-        )
-    } else {
-        return Err(anyhow!("Metric file not found"));
-    };
+    let (permission, organization_id) =
+        if let Some(file_with_permission) = metric_file_with_permissions {
+            (
+                file_with_permission.permission,
+                file_with_permission.metric_file.organization_id,
+            )
+        } else {
+            return Err(anyhow!("Metric file not found"));
+        };
 
     // Verify the user has at least Editor, FullAccess, or Owner permission
     if !check_permission_access(
         permission,
-        &[AssetPermissionRole::Editor, AssetPermissionRole::FullAccess, AssetPermissionRole::Owner],
+        &[
+            AssetPermissionRole::Editor,
+            AssetPermissionRole::FullAccess,
+            AssetPermissionRole::Owner,
+        ],
         organization_id,
         &user.organizations,
     ) {
-        return Err(anyhow!("You don't have permission to update this metric. Editor or higher role required."));
+        return Err(anyhow!(
+            "You don't have permission to update this metric. Editor or higher role required."
+        ));
     }
 
     // Now get the full metric with all its data needed for the update
@@ -134,7 +141,7 @@ pub async fn update_metric_handler(
         .map_err(|e| anyhow!("Failed to get version history: {}", e))?;
 
     // Version restoration takes highest precedence
-    let content = if let Some(version_number) = request.restore_to_version {
+    let mut content = if let Some(version_number) = request.restore_to_version {
         // Fetch the requested version
         let version = current_version_history
             .get_version(version_number)
@@ -233,6 +240,25 @@ pub async fn update_metric_handler(
             .await
             .map_err(|e| anyhow!("Failed to execute SQL for metadata calculation: {}", e))?;
 
+        // Generate default column formats based on metadata using the new method
+        let default_formats =
+            ColumnLabelFormat::generate_formats_from_metadata(&query_result.metadata);
+
+        // Get existing chart config
+        let existing_config = serde_json::to_value(&content.chart_config)?;
+
+        // Create a new JSON object with column_label_formats
+        let column_formats_json = serde_json::to_value(&default_formats)?;
+        let format_update = serde_json::json!({
+            "columnLabelFormats": column_formats_json
+        });
+
+        // Merge the formats with existing config
+        let merged_config = merge_json_objects(existing_config, format_update)?;
+
+        // Update the content's chart_config
+        content.chart_config = serde_json::from_value(merged_config)?;
+
         // Return metadata
         Some(query_result.metadata)
     } else {
@@ -305,6 +331,9 @@ pub async fn update_metric_handler(
 
 #[cfg(test)]
 mod tests {
+    use database::types::{ColumnMetaData, ColumnType, DataMetadata, SimpleType};
+    use serde_json::json;
+
     use super::*;
 
     #[test]
@@ -484,4 +513,81 @@ mod tests {
         let result = merge_json_objects(base, update).unwrap();
         assert_eq!(result["arr"], serde_json::json!([4, 5])); // Array is replaced, not merged
     }
+
+    #[test]
+    fn test_update_column_formats_from_metadata() {
+        // Create a mock existing chart_config with some existing column formats
+        let existing_config = serde_json::json!({
+            "selectedChartType": "bar",
+            "columnLabelFormats": {
+                "existing_column": {
+                    "columnType": "number",
+                    "style": "currency",
+                    "currency": "USD"
+                }
+            }
+        });
+
+        // Create a test DataMetadata with two columns - one existing and one new
+        let metadata = DataMetadata {
+            column_count: 2,
+            row_count: 10,
+            column_metadata: vec![
+                ColumnMetaData {
+                    name: "existing_column".to_string(),
+                    min_value: json!(0),
+                    max_value: json!(100),
+                    unique_values: 10,
+                    simple_type: SimpleType::Number,
+                    column_type: ColumnType::Float8,
+                },
+                ColumnMetaData {
+                    name: "new_column".to_string(),
+                    min_value: json!("2023-01-01"),
+                    max_value: json!("2023-12-31"),
+                    unique_values: 12,
+                    simple_type: SimpleType::Date,
+                    column_type: ColumnType::Date,
+                },
+            ],
+        };
+
+        // Generate default column formats
+        let default_formats = ColumnLabelFormat::generate_formats_from_metadata(&metadata);
+
+        // Convert to JSON for merging
+        let column_formats_json = serde_json::to_value(&default_formats).unwrap();
+        let format_update = serde_json::json!({
+            "columnLabelFormats": column_formats_json
+        });
+
+        // Merge with existing config
+        let merged_config = merge_json_objects(existing_config, format_update).unwrap();
+
+        // Verify the merged config
+
+        // Check that existing column kept its custom currency format
+        assert_eq!(
+            merged_config["columnLabelFormats"]["existing_column"]["style"],
+            "currency"
+        );
+        assert_eq!(
+            merged_config["columnLabelFormats"]["existing_column"]["currency"],
+            "USD"
+        );
+
+        // Check that new column has the default date format
+        assert_eq!(
+            merged_config["columnLabelFormats"]["new_column"]["columnType"],
+            "date"
+        );
+        assert_eq!(
+            merged_config["columnLabelFormats"]["new_column"]["style"],
+            "date"
+        );
+        assert_eq!(
+            merged_config["columnLabelFormats"]["new_column"]["dateFormat"],
+            "auto"
+        );
+    }
 }