Ling0925
/
buster
mirror of https://github.com/buster-so/buster.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

update version param on updates for dashboard and metric pushed up

This commit is contained in:
dal 2025-03-24 11:15:24 -06:00
parent 7448967a7d
commit 6adf84b8d3
No known key found for this signature in database
GPG Key ID: 16F4B0E1E9F61122
4 changed files with 199 additions and 119 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
api/libs/database/src/types/version_history.rs
View File

@ -73,6 +73,30 @@ impl VersionHistory {
pub fn get_latest_version(&self) -> Option<&Version> {
self.0.values().max_by_key(|v| v.version_number)
}
/// Updates the content of the latest version without creating a new version
///
/// This is used when we want to overwrite the current version instead of creating a new one.
/// If there are no versions yet, this will create a new version with number 1.
///
/// # Arguments
/// * `content` - The new content to replace the latest version's content
pub fn update_latest_version(&mut self, content: impl Into<VersionContent>) {
if let Some(latest_version) = self.get_latest_version() {
let version_number = latest_version.version_number;
self.0.insert(
version_number.to_string(),
Version {
content: content.into(),
version_number,
updated_at: Utc::now(),
},
);
} else {
// If there are no versions yet, create a new one with version 1
self.add_version(1, content);
}
}
}
impl FromSql<Jsonb, Pg> for VersionHistory {

24
api/libs/handlers/src/dashboards/update_dashboard_handler.rs
View File

@ -31,6 +31,7 @@ pub struct DashboardUpdateRequest {
pub public: Option<bool>,
pub public_expiry_date: Option<String>,
pub public_password: Option<String>,
pub update_version: Option<bool>,
}
/// Updates an existing dashboard by ID
@ -140,15 +141,23 @@ pub async fn update_dashboard_handler(
.first::<VersionHistory>(&mut conn)
.await
.unwrap_or_else(|_| VersionHistory::new(0, dashboard_yml.clone()));
// Calculate the next version number
let next_version = current_version_history.get_latest_version()
.map(|v| v.version_number + 1)
.unwrap_or(1);
// Add the new version to the version history
// Add the new version to the version history only if update_version is true (defaults to true)
let should_update_version = request.update_version.unwrap_or(true);
// Only add a new version if has_changes and should_update_version
if has_changes {
current_version_history.add_version(next_version, dashboard_yml.clone());
if should_update_version {
current_version_history.add_version(next_version, dashboard_yml.clone());
} else {
// Overwrite the current version instead of creating a new one
current_version_history.update_latest_version(dashboard_yml.clone());
}
}
// Convert content to JSON for storage
@ -171,8 +180,13 @@ pub async fn update_dashboard_handler(
if let Some(name) = request.name {
// First update the dashboard_yml.name to keep them in sync
dashboard_yml.name = name.clone();
// Update version history with the updated dashboard_yml
current_version_history.add_version(next_version, dashboard_yml.clone());
// Update version history with the updated dashboard_yml based on should_update_version
if should_update_version {
current_version_history.add_version(next_version, dashboard_yml.clone());
} else {
current_version_history.update_latest_version(dashboard_yml.clone());
}
diesel::update(dashboard_files::table)
.filter(dashboard_files::id.eq(dashboard_id))

117
api/libs/handlers/src/metrics/update_metric_handler.rs
View File

@ -12,11 +12,11 @@ use uuid::Uuid;
/// Recursively merges two JSON objects.
/// The second object (update) takes precedence over the first (base) where there are conflicts.
///
///
/// # Arguments
/// * `base` - The base JSON value, typically the existing configuration
/// * `update` - The update JSON value containing fields to update
///
///
/// # Returns
/// * `Result<Value>` - The merged JSON value or an error
fn merge_json_objects(base: Value, update: Value) -> Result<Value> {
@ -58,21 +58,22 @@ pub struct UpdateMetricRequest {
pub verification: Option<database::enums::Verification>,
pub file: Option<String>,
pub sql: Option<String>,
pub update_version: Option<bool>,
}
/// Handler to update a metric by ID
///
///
/// This handler updates a metric file in the database and increments its version number.
/// Each time a metric is updated, the previous version is saved in the version history.
///
///
/// # Arguments
/// * `metric_id` - The UUID of the metric to update
/// * `user_id` - The UUID of the user making the update
/// * `request` - The update request containing the fields to modify
///
///
/// # Returns
/// * `Result<BusterMetric>` - The updated metric on success, or an error
///
///
/// # Versioning
/// The function automatically handles versioning:
/// 1. Retrieves the current metric and extracts its content
@ -86,7 +87,9 @@ pub async fn update_metric_handler(
user_id: &Uuid,
request: UpdateMetricRequest,
) -> Result<BusterMetric> {
let mut conn = get_pg_pool().get().await
let mut conn = get_pg_pool()
.get()
.await
.map_err(|e| anyhow!("Failed to get database connection: {}", e))?;
// Check if metric exists and user has access - use the latest version
@ -108,13 +111,13 @@ pub async fn update_metric_handler(
if let Some(chart_config) = request.chart_config {
// Instead of trying to deserialize directly, we'll merge the JSON objects first
// and then deserialize the combined result
// Convert existing chart_config to Value for merging
let existing_config = serde_json::to_value(&content.chart_config)?;
// Merge the incoming chart_config with the existing one
let merged_config = merge_json_objects(existing_config, chart_config)?;
// Convert merged JSON back to ChartConfig
content.chart_config = serde_json::from_value(merged_config)?;
}
@ -143,13 +146,24 @@ pub async fn update_metric_handler(
.first::<VersionHistory>(&mut conn)
.await
.map_err(|e| anyhow!("Failed to get version history: {}", e))?;
// Calculate the next version number
let next_version = metric.versions.len() as i32 + 1;
current_version_history.add_version(next_version, content.clone());
// Only add a new version if update_version is true (defaults to true)
let should_update_version = request.update_version.unwrap_or(true);
// Add the new version to the version history or update the latest version
if should_update_version {
current_version_history.add_version(next_version, content.clone());
} else {
// Overwrite the current version instead of creating a new one
current_version_history.update_latest_version(content.clone());
}
// Convert content to JSON for storage
let content_json = serde_json::to_value(content.clone())?;
// Build update query - only verification is handled separately from the YAML content
let builder = diesel::update(metric_files::table)
.filter(metric_files::id.eq(metric_id))
@ -177,7 +191,8 @@ pub async fn update_metric_handler(
))
.execute(&mut conn)
.await
}.map_err(|e| anyhow!("Failed to update metric: {}", e))?;
}
.map_err(|e| anyhow!("Failed to update metric: {}", e))?;
// Return the updated metric - latest version
get_metric_handler(metric_id, user_id, None).await
@ -190,7 +205,7 @@ mod tests {
#[test]
fn test_update_metric_request_validation() {
// Test the request struct validation without making actual DB calls
// Create a request with valid UUID format for dataset_ids
let valid_request = UpdateMetricRequest {
name: Some("Valid Title".to_string()),
@ -204,22 +219,23 @@ mod tests {
verification: Some(database::enums::Verification::NotRequested),
file: None,
sql: None,
update_version: None,
};
// Verify the request fields are properly structured
assert_eq!(valid_request.name.unwrap(), "Valid Title");
assert_eq!(valid_request.description.unwrap(), "Valid Description");
assert_eq!(valid_request.time_frame.unwrap(), "daily");
assert!(valid_request.chart_config.is_some());
assert!(valid_request.dataset_ids.unwrap()[0].len() == 36);
// Actual validation logic is tested in integration tests
}
#[test]
fn test_chart_config_partial_update() {
// This test verifies that partial chart_config updates only change specified fields
// Create a mock existing chart_config
let existing_config = serde_json::json!({
"selectedChartType": "bar",
@ -241,7 +257,7 @@ mod tests {
}
}
});
// Create a partial chart_config update that only changes specific fields
let partial_config = serde_json::json!({
"selectedChartType": "bar", // Same as before
@ -254,31 +270,44 @@ mod tests {
}
}
});
// Apply the deep merge
let updated_config = merge_json_objects(existing_config.clone(), partial_config.clone()).unwrap();
let updated_config =
merge_json_objects(existing_config.clone(), partial_config.clone()).unwrap();
// Verify the updated config has the expected changes
// Changed fields should have new values
assert_eq!(updated_config["colors"][0], "#ff0000");
assert_eq!(updated_config["colors"][1], "#00ff00");
assert_eq!(updated_config["show_legend"], false);
// Unchanged fields should retain original values
assert_eq!(updated_config["grid_lines"], true);
assert_eq!(updated_config["bar_and_line_axis"]["x"][0], "date");
assert_eq!(updated_config["bar_and_line_axis"]["y"][0], "value");
// Verify nested column formats are correctly merged
assert_eq!(updated_config["columnLabelFormats"]["date"]["columnType"], "date");
assert_eq!(updated_config["columnLabelFormats"]["date"]["style"], "date");
assert_eq!(
updated_config["columnLabelFormats"]["date"]["columnType"],
"date"
);
assert_eq!(
updated_config["columnLabelFormats"]["date"]["style"],
"date"
);
// This entry should be partially updated - style changed but columnType preserved
assert_eq!(updated_config["columnLabelFormats"]["value"]["columnType"], "number");
assert_eq!(updated_config["columnLabelFormats"]["value"]["style"], "currency");
assert_eq!(
updated_config["columnLabelFormats"]["value"]["columnType"],
"number"
);
assert_eq!(
updated_config["columnLabelFormats"]["value"]["style"],
"currency"
);
}
#[test]
fn test_merge_json_objects() {
// Test basic object merging
@ -290,12 +319,12 @@ mod tests {
"b": 3,
"c": 4
});
let result = merge_json_objects(base, update).unwrap();
assert_eq!(result["a"], 1);
assert_eq!(result["b"], 3);
assert_eq!(result["c"], 4);
// Test nested object merging
let base = serde_json::json!({
"nested": {
@ -310,13 +339,13 @@ mod tests {
"z": 4
}
});
let result = merge_json_objects(base, update).unwrap();
assert_eq!(result["nested"]["x"], 1); // Preserved from base
assert_eq!(result["nested"]["y"], 3); // Updated
assert_eq!(result["nested"]["z"], 4); // Added from update
assert_eq!(result["other"], "value"); // Preserved from base
assert_eq!(result["nested"]["x"], 1); // Preserved from base
assert_eq!(result["nested"]["y"], 3); // Updated
assert_eq!(result["nested"]["z"], 4); // Added from update
assert_eq!(result["other"], "value"); // Preserved from base
// Test array replacement
let base = serde_json::json!({
"arr": [1, 2, 3]
@ -324,8 +353,8 @@ mod tests {
let update = serde_json::json!({
"arr": [4, 5]
});
let result = merge_json_objects(base, update).unwrap();
assert_eq!(result["arr"], serde_json::json!([4, 5])); // Array is replaced, not merged
assert_eq!(result["arr"], serde_json::json!([4, 5])); // Array is replaced, not merged
}
}
}

153
api/libs/handlers/tests/metrics/update_metric_test.rs
View File

@ -1,34 +1,27 @@
use anyhow::Result;
use database::{
enums::Verification,
pool::get_pg_pool,
schema::metric_files,
types::MetricYml,
};
use database::{enums::Verification, pool::get_pg_pool, schema::metric_files, types::MetricYml};
use diesel::{ExpressionMethods, QueryDsl};
use diesel_async::RunQueryDsl;
use handlers::metrics::{update_metric_handler, UpdateMetricRequest};
use uuid::Uuid;
// Import the common setup and test data functions
use super::{
cleanup_test_data, create_test_metric, insert_test_metric, setup_test_environment,
};
use super::{cleanup_test_data, create_test_metric, insert_test_metric, setup_test_environment};
/// Integration test for updating a metric that exists in the database
#[tokio::test]
async fn test_update_metric_integration() -> Result<()> {
// Setup test environment
setup_test_environment().await?;
// Create test organization and user IDs
let organization_id = Uuid::new_v4();
let user_id = Uuid::new_v4();
// Create a test metric
let test_metric = create_test_metric(organization_id, user_id).await?;
let metric_id = test_metric.id;
// Insert the test metric into the database
match insert_test_metric(&test_metric).await {
Ok(_) => println!("Successfully inserted test metric with ID: {}", metric_id),
@ -37,7 +30,7 @@ async fn test_update_metric_integration() -> Result<()> {
return Ok(());
}
}
// Create an update request with various fields to change
let update_request = UpdateMetricRequest {
name: Some("Updated Test Metric".to_string()),
@ -55,8 +48,9 @@ async fn test_update_metric_integration() -> Result<()> {
verification: Some(Verification::Verified),
file: None,
sql: Some("SELECT updated_value FROM updated_table".to_string()),
update_version: None,
};
// Call the handler function to update the metric
match update_metric_handler(&metric_id, &user_id, update_request).await {
Ok(updated_metric) => {
@ -64,53 +58,64 @@ async fn test_update_metric_integration() -> Result<()> {
assert_eq!(updated_metric.name, "Updated Test Metric");
assert_eq!(updated_metric.status, Verification::Verified);
assert_eq!(updated_metric.time_frame, "weekly");
// Verify the metric was updated in the database
let mut conn = get_pg_pool().get().await?;
let db_metric = metric_files::table
.filter(metric_files::id.eq(metric_id))
.first::<database::models::MetricFile>(&mut conn)
.await?;
// Verify database values match the expected updates
assert_eq!(db_metric.name, "Updated Test Metric");
assert_eq!(db_metric.verification, Verification::Verified);
// Verify the content field was updated
let content: MetricYml = db_metric.content;
assert_eq!(content.time_frame, "weekly");
assert_eq!(content.description, Some("Updated test description".to_string()));
assert_eq!(
content.description,
Some("Updated test description".to_string())
);
assert_eq!(content.sql, "SELECT updated_value FROM updated_table");
// Verify version history was updated
assert!(db_metric.version_history.0.contains_key(&"1".to_string()));
assert!(db_metric.version_history.0.contains_key(&"2".to_string()));
// Get the latest version from the version history
let latest_version = db_metric.version_history.get_latest_version().unwrap();
assert_eq!(latest_version.version_number, 2);
// Verify the latest version's content matches the current content
if let database::types::VersionContent::MetricYml(latest_content) = &latest_version.content {
if let database::types::VersionContent::MetricYml(latest_content) =
&latest_version.content
{
assert_eq!(latest_content.time_frame, "weekly");
assert_eq!(latest_content.description, Some("Updated test description".to_string()));
assert_eq!(latest_content.sql, "SELECT updated_value FROM updated_table");
assert_eq!(
latest_content.description,
Some("Updated test description".to_string())
);
assert_eq!(
latest_content.sql,
"SELECT updated_value FROM updated_table"
);
} else {
panic!("Expected MetricYml content in version history");
}
println!("Update metric test passed with ID: {}", metric_id);
},
}
Err(e) => {
// Clean up the test data regardless of test outcome
cleanup_test_data(Some(metric_id), None).await?;
return Err(e);
}
}
// Clean up the test data
cleanup_test_data(Some(metric_id), None).await?;
Ok(())
}
@ -119,11 +124,11 @@ async fn test_update_metric_integration() -> Result<()> {
async fn test_update_nonexistent_metric() -> Result<()> {
// Setup test environment
setup_test_environment().await?;
// Generate random UUIDs for test
let metric_id = Uuid::new_v4();
let user_id = Uuid::new_v4();
// Create a basic update request
let update_request = UpdateMetricRequest {
name: Some("Updated Test Metric".to_string()),
@ -134,17 +139,17 @@ async fn test_update_nonexistent_metric() -> Result<()> {
verification: None,
file: None,
sql: None,
update_version: None,
};
// Attempt to update a nonexistent metric
let result = update_metric_handler(&metric_id, &user_id, update_request).await;
// Verify the operation fails with an appropriate error
assert!(result.is_err());
let error = result.err().unwrap();
assert!(error.to_string().contains("not found") ||
error.to_string().contains("Failed to get"));
assert!(error.to_string().contains("not found") || error.to_string().contains("Failed to get"));
Ok(())
}
@ -153,15 +158,15 @@ async fn test_update_nonexistent_metric() -> Result<()> {
async fn test_update_specific_metric_fields() -> Result<()> {
// Setup test environment
setup_test_environment().await?;
// Create test organization and user IDs
let organization_id = Uuid::new_v4();
let user_id = Uuid::new_v4();
// Create a test metric
let test_metric = create_test_metric(organization_id, user_id).await?;
let metric_id = test_metric.id;
// Insert the test metric into the database
match insert_test_metric(&test_metric).await {
Ok(_) => println!("Successfully inserted test metric with ID: {}", metric_id),
@ -170,7 +175,7 @@ async fn test_update_specific_metric_fields() -> Result<()> {
return Ok(());
}
}
// Test 1: Update only title
let title_request = UpdateMetricRequest {
name: Some("Title Only Update".to_string()),
@ -181,16 +186,17 @@ async fn test_update_specific_metric_fields() -> Result<()> {
verification: None,
file: None,
sql: None,
update_version: None,
};
match update_metric_handler(&metric_id, &user_id, title_request).await {
Ok(metric) => {
assert_eq!(metric.name, "Title Only Update");
// Verify other fields were not changed
assert_eq!(metric.time_frame, "daily");
assert_eq!(metric.status, Verification::NotRequested);
// Verify version number was incremented
assert_eq!(metric.version_number, 2);
@ -200,22 +206,22 @@ async fn test_update_specific_metric_fields() -> Result<()> {
.filter(metric_files::id.eq(metric_id))
.first::<database::models::MetricFile>(&mut conn)
.await?;
// Check version history has exactly 2 versions
assert_eq!(db_metric.version_history.0.len(), 2);
assert!(db_metric.version_history.0.contains_key(&"1".to_string()));
assert!(db_metric.version_history.0.contains_key(&"2".to_string()));
// Get the latest version
let latest_version = db_metric.version_history.get_latest_version().unwrap();
assert_eq!(latest_version.version_number, 2);
},
}
Err(e) => {
cleanup_test_data(Some(metric_id), None).await?;
return Err(e);
}
}
// Test 2: Update only verification
let verification_request = UpdateMetricRequest {
name: None,
@ -226,41 +232,42 @@ async fn test_update_specific_metric_fields() -> Result<()> {
verification: Some(Verification::Verified),
file: None,
sql: None,
update_version: None,
};
match update_metric_handler(&metric_id, &user_id, verification_request).await {
Ok(metric) => {
assert_eq!(metric.status, Verification::Verified);
// Verify title remains from previous update
assert_eq!(metric.name, "Title Only Update");
// Verify version number increased again
assert_eq!(metric.version_number, 3);
// Verify in the database
let mut conn = get_pg_pool().get().await?;
let db_metric = metric_files::table
.filter(metric_files::id.eq(metric_id))
.first::<database::models::MetricFile>(&mut conn)
.await?;
// Check version history has exactly 3 versions
assert_eq!(db_metric.version_history.0.len(), 3);
assert!(db_metric.version_history.0.contains_key(&"1".to_string()));
assert!(db_metric.version_history.0.contains_key(&"2".to_string()));
assert!(db_metric.version_history.0.contains_key(&"3".to_string()));
// Get the latest version
let latest_version = db_metric.version_history.get_latest_version().unwrap();
assert_eq!(latest_version.version_number, 3);
},
}
Err(e) => {
cleanup_test_data(Some(metric_id), None).await?;
return Err(e);
}
}
// Test 3: Update only SQL
let sql_request = UpdateMetricRequest {
name: None,
@ -271,60 +278,66 @@ async fn test_update_specific_metric_fields() -> Result<()> {
verification: None,
file: None,
sql: Some("SELECT new_value FROM new_table".to_string()),
update_version: None,
};
match update_metric_handler(&metric_id, &user_id, sql_request).await {
Ok(metric) => {
// Parse the YAML content to verify SQL update
let content: MetricYml = serde_yaml::from_str(&metric.file).unwrap();
assert_eq!(content.sql, "SELECT new_value FROM new_table");
// Verify other fields remain unchanged
assert_eq!(metric.name, "Title Only Update");
assert_eq!(metric.status, Verification::Verified);
// Verify version number increased to 4
assert_eq!(metric.version_number, 4);
// Verify in the database
let mut conn = get_pg_pool().get().await?;
let db_metric = metric_files::table
.filter(metric_files::id.eq(metric_id))
.first::<database::models::MetricFile>(&mut conn)
.await?;
// Check version history now has 4 versions
assert_eq!(db_metric.version_history.0.len(), 4);
// Verify all version numbers are present
for i in 1..=4 {
assert!(db_metric.version_history.0.contains_key(&i.to_string()),
"Version {} missing from history", i);
assert!(
db_metric.version_history.0.contains_key(&i.to_string()),
"Version {} missing from history",
i
);
}
// Get the latest version
let latest_version = db_metric.version_history.get_latest_version().unwrap();
assert_eq!(latest_version.version_number, 4);
// Check the content of the latest version
if let database::types::VersionContent::MetricYml(latest_content) = &latest_version.content {
if let database::types::VersionContent::MetricYml(latest_content) =
&latest_version.content
{
assert_eq!(latest_content.sql, "SELECT new_value FROM new_table");
// The title should be preserved from earlier updates
let yaml = serde_yaml::to_string(latest_content).unwrap();
assert!(yaml.contains("Title Only Update"));
} else {
panic!("Expected MetricYml content in version history");
}
},
}
Err(e) => {
cleanup_test_data(Some(metric_id), None).await?;
return Err(e);
}
}
// Clean up the test data
cleanup_test_data(Some(metric_id), None).await?;
Ok(())
}
}