From ce11968d5416ea8b679338bf0845c687500002af Mon Sep 17 00:00:00 2001 From: dal Date: Thu, 3 Apr 2025 15:29:25 -0600 Subject: [PATCH] column label format update on sql change --- api/libs/database/src/types/metric_yml.rs | 179 ++++++++++++++++++ .../src/metrics/update_metric_handler.rs | 136 +++++++++++-- 2 files changed, 300 insertions(+), 15 deletions(-) diff --git a/api/libs/database/src/types/metric_yml.rs b/api/libs/database/src/types/metric_yml.rs index a80b35433..f6a23f18c 100644 --- a/api/libs/database/src/types/metric_yml.rs +++ b/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, } +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 { + 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"); + } } diff --git a/api/libs/handlers/src/metrics/update_metric_handler.rs b/api/libs/handlers/src/metrics/update_metric_handler.rs index 1e963e389..dfce32afb 100644 --- a/api/libs/handlers/src/metrics/update_metric_handler.rs +++ b/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; @@ -102,26 +102,33 @@ pub async fn update_metric_handler( let metric_file_with_permissions = fetch_metric_file_with_permissions(metric_id, &user.id) .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 let metric = get_metric_handler(metric_id, user, None).await?; @@ -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" + ); + } }