diff --git a/crates/context_aware_config/src/api/config/handlers.rs b/crates/context_aware_config/src/api/config/handlers.rs index e301477e..a89e18ff 100644 --- a/crates/context_aware_config/src/api/config/handlers.rs +++ b/crates/context_aware_config/src/api/config/handlers.rs @@ -380,10 +380,31 @@ fn construct_new_payload( }, )?; - return Ok(web::Json(PutReq { - context: context, + let description = match res.get("description") { + Some(Value::String(s)) => Some(s.clone()), + Some(_) => { + log::error!("construct new payload: Description is not a valid string"); + return Err(bad_argument!("Description must be a string")); + } + None => None, + }; + + // Handle change_reason + let change_reason = res + .get("change_reason") + .and_then(|val| val.as_str()) + .map(|s| s.to_string()) + .ok_or_else(|| { + log::error!("construct new payload: Change reason not present or invalid"); + bad_argument!("Change reason is required and must be a string") + })?; + + Ok(web::Json(PutReq { + context, r#override: override_, - })); + description, + change_reason, + })) } #[allow(clippy::too_many_arguments)] diff --git a/crates/context_aware_config/src/api/context/handlers.rs b/crates/context_aware_config/src/api/context/handlers.rs index 20508186..bb071bf5 100644 --- a/crates/context_aware_config/src/api/context/handlers.rs +++ b/crates/context_aware_config/src/api/context/handlers.rs @@ -198,6 +198,15 @@ fn create_ctx_from_put_req( tenant_config: &TenantConfig, ) -> superposition::Result { let ctx_condition = req.context.to_owned().into_inner(); + let description = if req.description.is_none() { + let ctx_condition_value = json!(ctx_condition); + ensure_description(ctx_condition_value, conn)? + } else { + req.description + .clone() + .ok_or_else(|| bad_argument!("Description should not be empty"))? + }; + let change_reason = req.change_reason.clone(); let condition_val = Value::Object(ctx_condition.clone().into()); let r_override = req.r#override.clone().into_inner(); let ctx_override = Value::Object(r_override.clone().into()); @@ -228,6 +237,8 @@ fn create_ctx_from_put_req( last_modified_at: Utc::now().naive_utc(), last_modified_by: user.get_email(), weight, + description: description, + change_reason, }) } @@ -297,6 +308,8 @@ fn db_update_override( dsl::override_id.eq(ctx.override_id), dsl::last_modified_at.eq(Utc::now().naive_utc()), dsl::last_modified_by.eq(user.get_email()), + dsl::description.eq(ctx.description), + dsl::change_reason.eq(ctx.change_reason), )) .get_result::(conn)?; Ok(get_put_resp(update_resp)) @@ -307,6 +320,8 @@ fn get_put_resp(ctx: Context) -> PutResp { context_id: ctx.id, override_id: ctx.override_id, weight: ctx.weight, + description: ctx.description, + change_reason: ctx.change_reason, } } @@ -320,7 +335,6 @@ pub fn put( ) -> superposition::Result { use contexts::dsl::contexts; let new_ctx = create_ctx_from_put_req(req, conn, user, tenant_config)?; - if already_under_txn { diesel::sql_query("SAVEPOINT put_ctx_savepoint").execute(conn)?; } @@ -345,6 +359,33 @@ pub fn put( } } +fn ensure_description( + context: Value, + transaction_conn: &mut diesel::PgConnection, +) -> Result { + use superposition_types::database::schema::contexts::dsl::{ + contexts as contexts_table, id as context_id, + }; + + let context_id_value = hash(&context); + + // Perform the database query + let existing_context = contexts_table + .filter(context_id.eq(context_id_value)) + .first::(transaction_conn); + + match existing_context { + Ok(ctx) => Ok(ctx.description), // If the context is found, return the description + Err(diesel::result::Error::NotFound) => Err(superposition::AppError::NotFound( + "Description not found in the existing context".to_string(), + )), + Err(e) => { + log::error!("Database error while fetching context: {:?}", e); + Err(superposition::AppError::DbError(e)) // Use the `DbError` variant for other Diesel-related errors + } + } +} + #[put("")] async fn put_handler( state: Data, @@ -356,18 +397,43 @@ async fn put_handler( tenant_config: TenantConfig, ) -> superposition::Result { let tags = parse_config_tags(custom_headers.config_tags)?; + let (put_response, version_id) = db_conn .transaction::<_, superposition::AppError, _>(|transaction_conn| { - let put_response = - put(req, transaction_conn, true, &user, &tenant_config, false).map_err( - |err: superposition::AppError| { - log::info!("context put failed with error: {:?}", err); - err - }, - )?; - let version_id = add_config_version(&state, tags, transaction_conn)?; + let mut req_mut = req.into_inner(); + + // Use the helper function to ensure the description + if req_mut.description.is_none() { + req_mut.description = Some(ensure_description( + Value::Object(req_mut.context.clone().into_inner().into()), + transaction_conn, + )?); + } + let put_response = put( + Json(req_mut.clone()), + transaction_conn, + true, + &user, + &tenant_config, + false, + ) + .map_err(|err: superposition::AppError| { + log::info!("context put failed with error: {:?}", err); + err + })?; + let description = req_mut.description.unwrap_or_default(); + let change_reason = req_mut.change_reason; + + let version_id = add_config_version( + &state, + tags, + transaction_conn, + description, + change_reason, + )?; Ok((put_response, version_id)) })?; + let mut http_resp = HttpResponse::Ok(); http_resp.insert_header(( @@ -396,14 +462,32 @@ async fn update_override_handler( let tags = parse_config_tags(custom_headers.config_tags)?; let (override_resp, version_id) = db_conn .transaction::<_, superposition::AppError, _>(|transaction_conn| { - let override_resp = - put(req, transaction_conn, true, &user, &tenant_config, true).map_err( - |err: superposition::AppError| { - log::info!("context put failed with error: {:?}", err); - err - }, - )?; - let version_id = add_config_version(&state, tags, transaction_conn)?; + let mut req_mut = req.into_inner(); + if req_mut.description.is_none() { + req_mut.description = Some(ensure_description( + Value::Object(req_mut.context.clone().into_inner().into()), + transaction_conn, + )?); + } + let override_resp = put( + Json(req_mut.clone()), + transaction_conn, + true, + &user, + &tenant_config, + true, + ) + .map_err(|err: superposition::AppError| { + log::info!("context put failed with error: {:?}", err); + err + })?; + let version_id = add_config_version( + &state, + tags, + transaction_conn, + req_mut.description.unwrap().clone(), + req_mut.change_reason.clone(), + )?; Ok((override_resp, version_id)) })?; let mut http_resp = HttpResponse::Ok(); @@ -431,6 +515,17 @@ fn r#move( ) -> superposition::Result { use contexts::dsl; let req = req.into_inner(); + + let ctx_condition = req.context.to_owned().into_inner(); + let ctx_condition_value = Value::Object(ctx_condition.clone().into()); + let description = if req.description.is_none() { + ensure_description(ctx_condition_value.clone(), conn)? + } else { + req.description + .ok_or_else(|| bad_argument!("Description should not be empty"))? + }; + + let change_reason = req.change_reason.clone(); let ctx_condition = req.context.to_owned().into_inner(); let ctx_condition_value = Value::Object(ctx_condition.clone().into()); let new_ctx_id = hash(&ctx_condition_value); @@ -471,6 +566,8 @@ fn r#move( last_modified_at: Utc::now().naive_utc(), last_modified_by: user.get_email(), weight, + description: description, + change_reason, }; let handle_unique_violation = @@ -534,7 +631,14 @@ async fn move_handler( log::info!("move api failed with error: {:?}", err); err })?; - let version_id = add_config_version(&state, tags, transaction_conn)?; + let version_id = add_config_version( + &state, + tags, + transaction_conn, + move_response.description.clone(), + move_response.change_reason.clone(), + )?; + Ok((move_response, version_id)) })?; let mut http_resp = HttpResponse::Ok(); @@ -709,12 +813,26 @@ async fn delete_context( #[cfg(feature = "high-performance-mode")] tenant: Tenant, mut db_conn: DbConnection, ) -> superposition::Result { + use superposition_types::database::schema::contexts::dsl::{ + contexts as contexts_table, id as context_id, + }; let ctx_id = path.into_inner(); let tags = parse_config_tags(custom_headers.config_tags)?; let version_id = db_conn.transaction::<_, superposition::AppError, _>(|transaction_conn| { - delete_context_api(ctx_id, user, transaction_conn)?; - let version_id = add_config_version(&state, tags, transaction_conn)?; + let context = contexts_table + .filter(context_id.eq(ctx_id.clone())) + .first::(transaction_conn)?; + delete_context_api(ctx_id.clone(), user.clone(), transaction_conn)?; + let description = context.description; + let change_reason = format!("Deleted context by {}", user.username); + let version_id = add_config_version( + &state, + tags, + transaction_conn, + description, + change_reason, + )?; Ok(version_id) })?; cfg_if::cfg_if! { @@ -743,6 +861,9 @@ async fn bulk_operations( ) -> superposition::Result { use contexts::dsl::contexts; let DbConnection(mut conn) = db_conn; + let mut all_descriptions = Vec::new(); + let mut all_change_reasons = Vec::new(); + let tags = parse_config_tags(custom_headers.config_tags)?; let (response, version_id) = conn.transaction::<_, superposition::AppError, _>(|transaction_conn| { @@ -751,7 +872,7 @@ async fn bulk_operations( match action { ContextAction::Put(put_req) => { let put_resp = put( - Json(put_req), + Json(put_req.clone()), transaction_conn, true, &user, @@ -765,12 +886,39 @@ async fn bulk_operations( ); err })?; + + let ctx_condition = put_req.context.to_owned().into_inner(); + let ctx_condition_value = + Value::Object(ctx_condition.clone().into()); + + let description = if put_req.description.is_none() { + ensure_description( + ctx_condition_value.clone(), + transaction_conn, + )? + } else { + put_req + .description + .expect("Description should not be empty") + }; + all_descriptions.push(description); + all_change_reasons.push(put_req.change_reason.clone()); response.push(ContextBulkResponse::Put(put_resp)); } ContextAction::Delete(ctx_id) => { + let context: Context = contexts + .filter(id.eq(&ctx_id)) + .first::(transaction_conn)?; + let deleted_row = delete(contexts.filter(id.eq(&ctx_id))) .execute(transaction_conn); - let email: String = user.get_email(); + let description = context.description; + + let email: String = user.clone().get_email(); + let change_reason = + format!("Context deleted by {}", email.clone()); + all_descriptions.push(description.clone()); + all_change_reasons.push(change_reason.clone()); match deleted_row { // Any kind of error would rollback the tranction but explicitly returning rollback tranction allows you to rollback from any point in transaction. Ok(0) => { @@ -807,12 +955,25 @@ async fn bulk_operations( ); err })?; + all_descriptions.push(move_context_resp.description.clone()); + all_change_reasons.push(move_context_resp.change_reason.clone()); + response.push(ContextBulkResponse::Move(move_context_resp)); } } } - let version_id = add_config_version(&state, tags, transaction_conn)?; + let combined_description = all_descriptions.join(","); + + let combined_change_reasons = all_change_reasons.join(","); + + let version_id = add_config_version( + &state, + tags, + transaction_conn, + combined_description, + combined_change_reasons, + )?; Ok((response, version_id)) })?; let mut http_resp = HttpResponse::Ok(); @@ -835,20 +996,25 @@ async fn weight_recompute( #[cfg(feature = "high-performance-mode")] tenant: Tenant, user: User, ) -> superposition::Result { - use superposition_types::database::schema::contexts::dsl::*; + use superposition_types::database::schema::contexts::dsl::{ + contexts, last_modified_at, last_modified_by, weight, + }; let DbConnection(mut conn) = db_conn; + // Fetch all contexts from the database let result: Vec = contexts.load(&mut conn).map_err(|err| { - log::error!("failed to fetch contexts with error: {}", err); + log::error!("Failed to fetch contexts with error: {}", err); unexpected_error!("Something went wrong") })?; + // Get dimension data and map for weight calculation let dimension_data = get_dimension_data(&mut conn)?; let dimension_data_map = get_dimension_data_map(&dimension_data)?; let mut response: Vec = vec![]; let tags = parse_config_tags(custom_headers.config_tags)?; - let contexts_new_weight: Vec<(BigDecimal, String)> = result + // Recompute weights and add descriptions + let contexts_new_weight: Vec<(BigDecimal, String, String, String)> = result .clone() .into_iter() .map(|context| { @@ -864,8 +1030,15 @@ async fn weight_recompute( condition: context.value.clone(), old_weight: context.weight.clone(), new_weight: val.clone(), + description: context.description.clone(), + change_reason: context.change_reason.clone(), }); - Ok((val, context.id.clone())) + Ok(( + val, + context.id.clone(), + context.description.clone(), + context.change_reason.clone(), + )) } Err(e) => { log::error!("failed to calculate context weight: {}", e); @@ -873,26 +1046,35 @@ async fn weight_recompute( } } }) - .collect::>>()?; + .collect::>>()?; + // Update database and add config version let last_modified_time = Utc::now().naive_utc(); let config_version_id = conn.transaction::<_, superposition::AppError, _>(|transaction_conn| { - for (context_weight, context_id) in contexts_new_weight { + for (context_weight, context_id, _description, _change_reason) in contexts_new_weight.clone() { diesel::update(contexts.filter(id.eq(context_id))) - .set((weight.eq(context_weight), last_modified_at.eq(last_modified_time.clone()), last_modified_by.eq(user.get_email()))) - .execute(transaction_conn).map_err(|err| { + .set(( + weight.eq(context_weight), + last_modified_at.eq(last_modified_time.clone()), + last_modified_by.eq(user.get_email()), + )) + .execute(transaction_conn) + .map_err(|err| { log::error!( "Failed to execute query while recomputing weight, error: {err}" ); db_error!(err) })?; } - let version_id = add_config_version(&state, tags, transaction_conn)?; + let description = "Recomputed weight".to_string(); + let change_reason = "Recomputed weight".to_string(); + let version_id = add_config_version(&state, tags, transaction_conn, description, change_reason)?; Ok(version_id) })?; #[cfg(feature = "high-performance-mode")] put_config_in_redis(config_version_id, state, tenant, &mut conn).await?; + let mut http_resp = HttpResponse::Ok(); http_resp.insert_header(( AppHeader::XConfigVersion.to_string(), diff --git a/crates/context_aware_config/src/api/context/types.rs b/crates/context_aware_config/src/api/context/types.rs index 10bd728b..75068395 100644 --- a/crates/context_aware_config/src/api/context/types.rs +++ b/crates/context_aware_config/src/api/context/types.rs @@ -9,12 +9,16 @@ use superposition_types::{ pub struct PutReq { pub context: Cac, pub r#override: Cac, + pub description: Option, + pub change_reason: String, } #[cfg_attr(test, derive(Debug, PartialEq))] // Derive traits only when running tests #[derive(Deserialize, Clone)] pub struct MoveReq { pub context: Cac, + pub description: Option, + pub change_reason: String, } #[derive(Deserialize, Clone)] @@ -27,6 +31,8 @@ pub struct PutResp { pub context_id: String, pub override_id: String, pub weight: BigDecimal, + pub description: String, + pub change_reason: String, } #[derive(Deserialize)] @@ -81,6 +87,8 @@ pub struct WeightRecomputeResponse { pub condition: Condition, pub old_weight: BigDecimal, pub new_weight: BigDecimal, + pub description: String, + pub change_reason: String, } #[cfg(test)] @@ -100,7 +108,9 @@ mod tests { }, "override": { "foo": "baz" - } + }, + "description": "", + "change_reason": "" }); let action_str = json!({ @@ -122,6 +132,8 @@ mod tests { let expected_action = ContextAction::Put(PutReq { context: context, r#override: override_, + description: Some("".to_string()), + change_reason: "".to_string(), }); let action_deserialized = diff --git a/crates/context_aware_config/src/api/default_config/handlers.rs b/crates/context_aware_config/src/api/default_config/handlers.rs index e4efcfa6..b078eb76 100644 --- a/crates/context_aware_config/src/api/default_config/handlers.rs +++ b/crates/context_aware_config/src/api/default_config/handlers.rs @@ -65,6 +65,8 @@ async fn create_default_config( let req = request.into_inner(); let key = req.key; let tags = parse_config_tags(custom_headers.config_tags)?; + let description = req.description; + let change_reason = req.change_reason; if req.schema.is_empty() { return Err(bad_argument!("Schema cannot be empty.")); @@ -82,6 +84,8 @@ async fn create_default_config( created_at: Utc::now(), last_modified_at: Utc::now().naive_utc(), last_modified_by: user.get_email(), + description: description.clone(), + change_reason: change_reason.clone(), }; let schema_compile_result = JSONSchema::options() @@ -130,9 +134,16 @@ async fn create_default_config( "Something went wrong, failed to create DefaultConfig" ) })?; - let version_id = add_config_version(&state, tags, transaction_conn)?; + let version_id = add_config_version( + &state, + tags, + transaction_conn, + description, + change_reason, + )?; Ok(version_id) })?; + #[cfg(feature = "high-performance-mode")] put_config_in_redis(version_id, state, tenant, &mut conn).await?; let mut http_resp = HttpResponse::Ok(); @@ -141,6 +152,7 @@ async fn create_default_config( AppHeader::XConfigVersion.to_string(), version_id.to_string(), )); + Ok(http_resp.json(default_config)) } @@ -172,6 +184,11 @@ async fn update_default_config( } })?; + let description = req + .description + .unwrap_or_else(|| existing.description.clone()); + let change_reason = req.change_reason; + let value = req.value.unwrap_or_else(|| existing.value.clone()); let schema = req .schema @@ -191,6 +208,8 @@ async fn update_default_config( created_at: existing.created_at, last_modified_at: Utc::now().naive_utc(), last_modified_by: user.get_email(), + description: description.clone(), + change_reason: change_reason.clone(), }; let jschema = JSONSchema::options() @@ -235,7 +254,13 @@ async fn update_default_config( unexpected_error!("Failed to update DefaultConfig") })?; - let version_id = add_config_version(&state, tags.clone(), transaction_conn)?; + let version_id = add_config_version( + &state, + tags.clone(), + transaction_conn, + description, + change_reason, + )?; Ok(version_id) })?; @@ -365,6 +390,12 @@ async fn delete( )) .execute(transaction_conn)?; + let default_config: DefaultConfig = dsl::default_configs + .filter(dsl::key.eq(&key)) + .first::(transaction_conn)?; + let description = default_config.description; + let change_reason = format!("Context Deleted by {}", user.get_email()); + let deleted_row = diesel::delete(dsl::default_configs.filter(dsl::key.eq(&key))) .execute(transaction_conn); @@ -373,7 +404,13 @@ async fn delete( Err(not_found!("default config key `{}` doesn't exists", key)) } Ok(_) => { - version_id = add_config_version(&state, tags, transaction_conn)?; + version_id = add_config_version( + &state, + tags, + transaction_conn, + description, + change_reason, + )?; log::info!( "default config key: {key} deleted by {}", user.get_email() diff --git a/crates/context_aware_config/src/api/default_config/types.rs b/crates/context_aware_config/src/api/default_config/types.rs index 2bcb67be..c8a65174 100644 --- a/crates/context_aware_config/src/api/default_config/types.rs +++ b/crates/context_aware_config/src/api/default_config/types.rs @@ -9,6 +9,8 @@ pub struct CreateReq { pub value: Value, pub schema: Map, pub function_name: Option, + pub description: String, + pub change_reason: String, } #[derive(Debug, Deserialize)] @@ -17,6 +19,8 @@ pub struct UpdateReq { pub value: Option, pub schema: Option>, pub function_name: Option, + pub description: Option, + pub change_reason: String, } #[derive(Debug, Clone)] diff --git a/crates/context_aware_config/src/api/dimension/handlers.rs b/crates/context_aware_config/src/api/dimension/handlers.rs index 875edce3..cf586a78 100644 --- a/crates/context_aware_config/src/api/dimension/handlers.rs +++ b/crates/context_aware_config/src/api/dimension/handlers.rs @@ -76,6 +76,8 @@ async fn create( function_name: create_req.function_name.clone(), last_modified_at: Utc::now().naive_utc(), last_modified_by: user.get_email(), + description: create_req.description, + change_reason: create_req.change_reason, }; conn.transaction::<_, superposition::AppError, _>(|transaction_conn| { @@ -152,6 +154,11 @@ async fn update( dimension_row.schema = schema_value; } + dimension_row.change_reason = update_req.change_reason; + dimension_row.description = update_req + .description + .unwrap_or_else(|| dimension_row.description); + dimension_row.function_name = match update_req.function_name { Some(FunctionNameEnum::Name(func_name)) => Some(func_name), Some(FunctionNameEnum::Remove) => None, diff --git a/crates/context_aware_config/src/api/dimension/types.rs b/crates/context_aware_config/src/api/dimension/types.rs index 503a6e36..6eb015d5 100644 --- a/crates/context_aware_config/src/api/dimension/types.rs +++ b/crates/context_aware_config/src/api/dimension/types.rs @@ -9,6 +9,8 @@ pub struct CreateReq { pub position: Position, pub schema: Value, pub function_name: Option, + pub description: String, + pub change_reason: String, } #[derive(Debug, Deserialize, AsRef, Deref, DerefMut, Into, Clone)] @@ -42,6 +44,8 @@ pub struct UpdateReq { pub position: Option, pub schema: Option, pub function_name: Option, + pub description: Option, + pub change_reason: String, } #[derive(Debug, Clone)] diff --git a/crates/context_aware_config/src/api/functions/handlers.rs b/crates/context_aware_config/src/api/functions/handlers.rs index 48b929d6..c5590a65 100644 --- a/crates/context_aware_config/src/api/functions/handlers.rs +++ b/crates/context_aware_config/src/api/functions/handlers.rs @@ -64,9 +64,10 @@ async fn create( published_at: None, published_by: None, published_runtime_version: None, - function_description: req.description, + description: req.description, last_modified_at: Utc::now().naive_utc(), last_modified_by: user.get_email(), + change_reason: req.change_reason, }; let insert: Result = diesel::insert_into(functions) @@ -137,7 +138,7 @@ async fn update( draft_runtime_version: req .runtime_version .unwrap_or(result.draft_runtime_version), - function_description: req.description.unwrap_or(result.function_description), + description: req.description.unwrap_or(result.description), draft_edited_by: user.get_email(), draft_edited_at: Utc::now().naive_utc(), published_code: result.published_code, @@ -146,6 +147,7 @@ async fn update( published_runtime_version: result.published_runtime_version, last_modified_at: Utc::now().naive_utc(), last_modified_by: user.get_email(), + change_reason: req.change_reason, }; let mut updated_function = diesel::update(functions) diff --git a/crates/context_aware_config/src/api/functions/types.rs b/crates/context_aware_config/src/api/functions/types.rs index c09b7f7c..93d4503b 100644 --- a/crates/context_aware_config/src/api/functions/types.rs +++ b/crates/context_aware_config/src/api/functions/types.rs @@ -8,6 +8,7 @@ pub struct UpdateFunctionRequest { pub function: Option, pub runtime_version: Option, pub description: Option, + pub change_reason: String, } #[derive(Debug, Deserialize)] @@ -16,6 +17,7 @@ pub struct CreateFunctionRequest { pub function: String, pub runtime_version: String, pub description: String, + pub change_reason: String, } #[derive(Debug, Deserialize, AsRef, Deref, DerefMut, Into)] diff --git a/crates/context_aware_config/src/api/type_templates/handlers.rs b/crates/context_aware_config/src/api/type_templates/handlers.rs index 0832d7d2..be3daa68 100644 --- a/crates/context_aware_config/src/api/type_templates/handlers.rs +++ b/crates/context_aware_config/src/api/type_templates/handlers.rs @@ -1,7 +1,7 @@ use actix_web::web::{Json, Path, Query}; use actix_web::{delete, get, post, put, HttpResponse, Scope}; use chrono::Utc; -use diesel::{ExpressionMethods, QueryDsl, RunQueryDsl}; +use diesel::{ExpressionMethods, OptionalExtension, QueryDsl, RunQueryDsl}; use jsonschema::JSONSchema; use serde_json::Value; use service_utils::service::types::DbConnection; @@ -15,7 +15,7 @@ use superposition_types::{ result as superposition, PaginatedResponse, User, }; -use crate::api::type_templates::types::{TypeTemplateName, TypeTemplateRequest}; +use crate::api::type_templates::types::{TypeTemplateCreateRequest, TypeTemplateName}; pub fn endpoints() -> Scope { Scope::new("") @@ -27,7 +27,7 @@ pub fn endpoints() -> Scope { #[post("")] async fn create_type( - request: Json, + request: Json, db_conn: DbConnection, user: User, ) -> superposition::Result { @@ -50,6 +50,8 @@ async fn create_type( type_templates::type_name.eq(type_name), type_templates::created_by.eq(user.email.clone()), type_templates::last_modified_by.eq(user.email.clone()), + type_templates::description.eq(request.description.clone()), + type_templates::change_reason.eq(request.change_reason.clone()), )) .get_result::(&mut conn) .map_err(|err| { @@ -78,7 +80,30 @@ async fn update_type( err.to_string() ) })?; + + let description = request.get("description").cloned(); let type_name: String = path.into_inner().into(); + let final_description = if description.is_none() { + let existing_template = type_templates::table + .filter(type_templates::type_name.eq(&type_name)) + .first::(&mut conn) + .optional() + .map_err(|err| { + log::error!("Failed to fetch existing type template: {}", err); + db_error!(err) + })?; + + match existing_template { + Some(template) => template.description.clone(), // Use existing description + None => { + return Err(bad_argument!( + "Description is required as the type template does not exist." + )); + } + } + } else { + description.unwrap().to_string() + }; let timestamp = Utc::now().naive_utc(); let updated_type = diesel::update(type_templates::table) @@ -87,6 +112,7 @@ async fn update_type( type_templates::type_schema.eq(request.clone()), type_templates::last_modified_at.eq(timestamp), type_templates::last_modified_by.eq(user.email), + type_templates::description.eq(final_description), )) .get_result::(&mut conn) .map_err(|err| { diff --git a/crates/context_aware_config/src/api/type_templates/types.rs b/crates/context_aware_config/src/api/type_templates/types.rs index d7cbbd1f..1b9c90be 100644 --- a/crates/context_aware_config/src/api/type_templates/types.rs +++ b/crates/context_aware_config/src/api/type_templates/types.rs @@ -4,9 +4,11 @@ use serde_json::Value; use superposition_types::RegexEnum; #[derive(Serialize, Deserialize, Clone, Debug)] -pub struct TypeTemplateRequest { +pub struct TypeTemplateCreateRequest { pub type_schema: Value, pub type_name: TypeTemplateName, + pub description: String, + pub change_reason: String, } #[derive(Serialize, Deserialize, Clone, Debug)] @@ -16,6 +18,8 @@ pub struct TypeTemplateResponse { pub created_at: String, pub last_modified: String, pub created_by: String, + pub description: String, + pub change_reason: String, } #[derive(Debug, Deserialize, Serialize, AsRef, Deref, DerefMut, Into, Clone)] diff --git a/crates/context_aware_config/src/helpers.rs b/crates/context_aware_config/src/helpers.rs index 1aebac11..2f47b031 100644 --- a/crates/context_aware_config/src/helpers.rs +++ b/crates/context_aware_config/src/helpers.rs @@ -294,6 +294,8 @@ pub fn add_config_version( state: &Data, tags: Option>, db_conn: &mut PooledConnection>, + description: String, + change_reason: String, ) -> superposition::Result { use config_versions::dsl::config_versions; let version_id = generate_snowflake_id(state)?; @@ -306,6 +308,8 @@ pub fn add_config_version( config_hash, tags, created_at: Utc::now().naive_utc(), + description, + change_reason, }; diesel::insert_into(config_versions) .values(&config_version) diff --git a/crates/experimentation_platform/src/api/experiments/handlers.rs b/crates/experimentation_platform/src/api/experiments/handlers.rs index 898fbbc6..69f05d86 100644 --- a/crates/experimentation_platform/src/api/experiments/handlers.rs +++ b/crates/experimentation_platform/src/api/experiments/handlers.rs @@ -137,6 +137,8 @@ async fn create( use superposition_types::database::schema::experiments::dsl::experiments; let mut variants = req.variants.to_vec(); let DbConnection(mut conn) = db_conn; + let description = req.description.clone(); + let change_reason = req.change_reason.clone(); // Checking if experiment has exactly 1 control variant, and // atleast 1 experimental variant @@ -211,6 +213,8 @@ async fn create( })? .clone(), r#override: json!(variant.overrides), + description: Some(description.clone()), + change_reason: change_reason.clone(), }; cac_operations.push(ContextAction::PUT(payload)); } @@ -280,6 +284,8 @@ async fn create( variants: Variants::new(variants), last_modified_by: user.get_email(), chosen_variant: None, + description, + change_reason, }; let mut inserted_experiments = diesel::insert_into(experiments) @@ -361,12 +367,18 @@ pub async fn conclude( ) -> superposition::Result<(Experiment, Option)> { use superposition_types::database::schema::experiments::dsl; + let change_reason = req.change_reason.clone(); + let winner_variant_id: String = req.chosen_variant.to_owned(); let experiment: Experiment = dsl::experiments .find(experiment_id) .get_result::(&mut conn)?; + let description = match req.description.clone() { + Some(desc) => desc, + None => experiment.description.clone(), + }; if matches!(experiment.status, ExperimentStatusType::CONCLUDED) { return Err(bad_argument!( "experiment with id {} is already concluded", @@ -389,6 +401,8 @@ pub async fn conclude( if !experiment_context.is_empty() { let context_move_req = ContextMoveReq { context: experiment_context.clone(), + description: description.clone(), + change_reason: change_reason.clone(), }; operations.push(ContextAction::MOVE((context_id, context_move_req))); } else { @@ -665,6 +679,8 @@ async fn ramp( ) -> superposition::Result> { let DbConnection(mut conn) = db_conn; let exp_id = params.into_inner(); + let description = req.description.clone(); + let change_reason = req.change_reason.clone(); let experiment: Experiment = experiments::experiments .find(exp_id) @@ -695,6 +711,8 @@ async fn ramp( experiments::last_modified.eq(Utc::now()), experiments::last_modified_by.eq(user.get_email()), experiments::status.eq(ExperimentStatusType::INPROGRESS), + experiments::description.eq(description), + experiments::change_reason.eq(change_reason), )) .get_result(&mut conn)?; @@ -738,6 +756,8 @@ async fn update_overrides( ) -> superposition::Result { let DbConnection(mut conn) = db_conn; let experiment_id = params.into_inner(); + let description = req.description.clone(); + let change_reason = req.change_reason.clone(); let payload = req.into_inner(); let variants = payload.variants; @@ -875,6 +895,8 @@ async fn update_overrides( })? .clone(), r#override: json!(variant.overrides), + description: description.clone(), + change_reason: change_reason.clone(), }; cac_operations.push(ContextAction::PUT(payload)); } diff --git a/crates/experimentation_platform/src/api/experiments/types.rs b/crates/experimentation_platform/src/api/experiments/types.rs index d4ada350..c57ff465 100644 --- a/crates/experimentation_platform/src/api/experiments/types.rs +++ b/crates/experimentation_platform/src/api/experiments/types.rs @@ -14,6 +14,8 @@ pub struct ExperimentCreateRequest { pub name: String, pub context: Exp, pub variants: Vec, + pub description: String, + pub change_reason: String, } #[derive(Serialize)] @@ -49,6 +51,8 @@ pub struct ExperimentResponse { pub variants: Vec, pub last_modified_by: String, pub chosen_variant: Option, + pub description: String, + pub change_reason: String, } impl From for ExperimentResponse { @@ -68,6 +72,8 @@ impl From for ExperimentResponse { variants: experiment.variants.into_inner(), last_modified_by: experiment.last_modified_by, chosen_variant: experiment.chosen_variant, + description: experiment.description, + change_reason: experiment.change_reason, } } } @@ -77,6 +83,8 @@ impl From for ExperimentResponse { #[derive(Deserialize, Debug)] pub struct ConcludeExperimentRequest { pub chosen_variant: String, + pub description: Option, + pub change_reason: String, } /********** Context Bulk API Type *************/ @@ -85,6 +93,8 @@ pub struct ConcludeExperimentRequest { pub struct ContextPutReq { pub context: Map, pub r#override: Value, + pub description: Option, + pub change_reason: String, } #[derive(Deserialize, Serialize, Clone)] @@ -185,6 +195,8 @@ pub struct ExperimentListFilters { #[derive(Deserialize, Debug)] pub struct RampRequest { pub traffic_percentage: u64, + pub description: String, + pub change_reason: String, } /********** Update API type ********/ @@ -198,11 +210,15 @@ pub struct VariantUpdateRequest { #[derive(Deserialize, Debug)] pub struct OverrideKeysUpdateRequest { pub variants: Vec, + pub description: Option, + pub change_reason: String, } #[derive(Deserialize, Serialize, Clone)] pub struct ContextMoveReq { pub context: Map, + pub description: String, + pub change_reason: String, } #[derive(Debug, Clone, Deserialize)] diff --git a/crates/experimentation_platform/tests/experimentation_tests.rs b/crates/experimentation_platform/tests/experimentation_tests.rs index 4e85706f..7dc283ad 100644 --- a/crates/experimentation_platform/tests/experimentation_tests.rs +++ b/crates/experimentation_platform/tests/experimentation_tests.rs @@ -13,6 +13,7 @@ use superposition_types::{ enum Dimensions { Os(String), Client(String), + #[allow(dead_code)] VariantIds(String), } @@ -72,6 +73,8 @@ fn experiment_gen( context: context.clone(), variants: Variants::new(variants.clone()), chosen_variant: None, + description: "".to_string(), + change_reason: "".to_string(), } } diff --git a/crates/frontend/src/components/context_form/utils.rs b/crates/frontend/src/components/context_form/utils.rs index ef252443..ce0f5944 100644 --- a/crates/frontend/src/components/context_form/utils.rs +++ b/crates/frontend/src/components/context_form/utils.rs @@ -211,6 +211,8 @@ pub fn construct_request_payload( overrides: Map, conditions: Vec, dimensions: Vec, + description: String, + change_reason: String, ) -> Value { // Construct the override section let override_section: Map = overrides; @@ -221,7 +223,9 @@ pub fn construct_request_payload( // Construct the entire request payload let request_payload = json!({ "override": override_section, - "context": context_section + "context": context_section, + "description": description, + "change_reason": change_reason }); request_payload @@ -232,10 +236,18 @@ pub async fn create_context( overrides: Map, conditions: Vec, dimensions: Vec, + description: String, + change_reason: String, ) -> Result { let host = get_host(); let url = format!("{host}/context"); - let request_payload = construct_request_payload(overrides, conditions, dimensions); + let request_payload = construct_request_payload( + overrides, + conditions, + dimensions, + description, + change_reason, + ); let response = request( url, reqwest::Method::PUT, @@ -252,11 +264,18 @@ pub async fn update_context( overrides: Map, conditions: Vec, dimensions: Vec, + description: String, + change_reason: String, ) -> Result { let host = get_host(); let url = format!("{host}/context/overrides"); - let request_payload = - construct_request_payload(overrides, conditions, dimensions.clone()); + let request_payload = construct_request_payload( + overrides, + conditions, + dimensions.clone(), + description, + change_reason, + ); let response = request( url, reqwest::Method::PUT, diff --git a/crates/frontend/src/components/default_config_form.rs b/crates/frontend/src/components/default_config_form.rs index 517ac626..a83565c5 100644 --- a/crates/frontend/src/components/default_config_form.rs +++ b/crates/frontend/src/components/default_config_form.rs @@ -36,6 +36,8 @@ pub fn default_config_form( #[prop(default = Value::Null)] config_value: Value, #[prop(default = None)] function_name: Option, #[prop(default = None)] prefix: Option, + #[prop(default = String::new())] description: String, + #[prop(default = String::new())] change_reason: String, handle_submit: NF, ) -> impl IntoView where @@ -49,6 +51,8 @@ where let (config_value_rs, config_value_ws) = create_signal(config_value); let (function_name_rs, function_name_ws) = create_signal(function_name); let (req_inprogess_rs, req_inprogress_ws) = create_signal(false); + let (description_rs, description_ws) = create_signal(description); + let (change_reason_rs, change_reason_ws) = create_signal(change_reason); let functions_resource: Resource> = create_blocking_resource( move || tenant_rs.get(), @@ -92,18 +96,24 @@ where let f_value = config_value_rs.get(); let fun_name = function_name_rs.get(); + let description = description_rs.get(); + let change_reason = change_reason_rs.get(); let create_payload = DefaultConfigCreateReq { key: config_key_rs.get(), schema: f_schema.clone(), value: f_value.clone(), function_name: fun_name.clone(), + description: description.clone(), + change_reason: change_reason.clone(), }; let update_payload = DefaultConfigUpdateReq { schema: f_schema, value: f_value, function_name: fun_name, + description, + change_reason, }; let handle_submit_clone = handle_submit.clone(); @@ -168,6 +178,40 @@ where +
+ +
+ +
+
+ + +
{move || { diff --git a/crates/frontend/src/components/function_form/types.rs b/crates/frontend/src/components/function_form/types.rs index f4fe5897..047765d1 100644 --- a/crates/frontend/src/components/function_form/types.rs +++ b/crates/frontend/src/components/function_form/types.rs @@ -6,6 +6,7 @@ pub struct FunctionCreateRequest { pub function: String, pub runtime_version: String, pub description: String, + pub change_reason: String, } #[derive(Serialize)] @@ -13,4 +14,5 @@ pub struct FunctionUpdateRequest { pub function: String, pub runtime_version: String, pub description: String, + pub change_reason: String, } diff --git a/crates/frontend/src/components/function_form/utils.rs b/crates/frontend/src/components/function_form/utils.rs index 5e4a06fc..650ca8dc 100644 --- a/crates/frontend/src/components/function_form/utils.rs +++ b/crates/frontend/src/components/function_form/utils.rs @@ -13,6 +13,7 @@ pub async fn create_function( function: String, runtime_version: String, description: String, + change_reason: String, tenant: String, ) -> Result { let payload = FunctionCreateRequest { @@ -20,6 +21,7 @@ pub async fn create_function( function, runtime_version, description, + change_reason, }; let host = get_host(); @@ -40,12 +42,14 @@ pub async fn update_function( function: String, runtime_version: String, description: String, + change_reason: String, tenant: String, ) -> Result { let payload = FunctionUpdateRequest { function, runtime_version, description, + change_reason, }; let host = get_host(); diff --git a/crates/frontend/src/components/type_template_form.rs b/crates/frontend/src/components/type_template_form.rs index 2b3910e0..a431112c 100644 --- a/crates/frontend/src/components/type_template_form.rs +++ b/crates/frontend/src/components/type_template_form.rs @@ -18,6 +18,8 @@ pub fn type_template_form( #[prop(default = String::new())] type_name: String, #[prop(default = json!({"type": "number"}))] type_schema: Value, handle_submit: NF, + #[prop(default = String::new())] description: String, + #[prop(default = String::new())] change_reason: String, ) -> impl IntoView where NF: Fn() + 'static + Clone, @@ -28,6 +30,8 @@ where let (type_name_rs, type_name_ws) = create_signal(type_name); let (type_schema_rs, type_schema_ws) = create_signal(type_schema); let (req_inprogess_rs, req_inprogress_ws) = create_signal(false); + let (description_rs, description_ws) = create_signal(description); + let (change_reason_rs, change_reason_ws) = create_signal(change_reason); let on_submit = move |ev: MouseEvent| { req_inprogress_ws.set(true); @@ -42,9 +46,13 @@ where let result = if edit { update_type(tenant_rs.get(), type_name, type_schema).await } else { + let description = description_rs.get(); + let change_reason = change_reason_rs.get(); let payload = json!({ "type_name": type_name, - "type_schema": type_schema + "type_schema": type_schema, + "description": description, + "change_reason": change_reason }); create_type(tenant_rs.get(), payload.clone()).await }; @@ -81,6 +89,39 @@ where />
+
+ +
+ +