diff --git a/crates/context_aware_config/migrations/2024-11-06-123105_add_description_and_comment_columns/down.sql b/crates/context_aware_config/migrations/2024-11-06-123105_add_description_and_comment_columns/down.sql new file mode 100644 index 00000000..6e67e7d1 --- /dev/null +++ b/crates/context_aware_config/migrations/2024-11-06-123105_add_description_and_comment_columns/down.sql @@ -0,0 +1,20 @@ +ALTER TABLE public.config_versions DROP COLUMN IF EXISTS description; +ALTER TABLE public.config_versions DROP COLUMN IF EXISTS change_reason; + +ALTER TABLE public.functions ALTER COLUMN description DROP NOT NULL; +ALTER TABLE public.functions ALTER COLUMN description DROP DEFAULT; +ALTER TABLE public.functions DROP COLUMN IF EXISTS change_reason; + +ALTER TABLE public.functions RENAME COLUMN description TO function_description; + +ALTER TABLE public.type_templates DROP COLUMN IF EXISTS description; +ALTER TABLE public.type_templates DROP COLUMN IF EXISTS change_reason; + +ALTER TABLE public.default_configs DROP COLUMN IF EXISTS description; +ALTER TABLE public.default_configs DROP COLUMN IF EXISTS change_reason; + +ALTER TABLE public.dimensions DROP COLUMN IF EXISTS description; +ALTER TABLE public.dimensions DROP COLUMN IF EXISTS change_reason; + +ALTER TABLE public.contexts DROP COLUMN IF EXISTS description; +ALTER TABLE public.contexts DROP COLUMN IF EXISTS change_reason; \ No newline at end of file diff --git a/crates/context_aware_config/migrations/2024-11-06-123105_add_description_and_comment_columns/up.sql b/crates/context_aware_config/migrations/2024-11-06-123105_add_description_and_comment_columns/up.sql new file mode 100644 index 00000000..99b82845 --- /dev/null +++ b/crates/context_aware_config/migrations/2024-11-06-123105_add_description_and_comment_columns/up.sql @@ -0,0 +1,21 @@ + +ALTER TABLE public.contexts ADD COLUMN IF NOT EXISTS description TEXT DEFAULT '' NOT NULL; +ALTER TABLE public.contexts ADD COLUMN IF NOT EXISTS change_reason TEXT DEFAULT '' NOT NULL; + +ALTER TABLE public.dimensions ADD COLUMN IF NOT EXISTS description TEXT DEFAULT '' NOT NULL; +ALTER TABLE public.dimensions ADD COLUMN IF NOT EXISTS change_reason TEXT DEFAULT '' NOT NULL; + +ALTER TABLE public.default_configs ADD COLUMN IF NOT EXISTS description TEXT DEFAULT '' NOT NULL; +ALTER TABLE public.default_configs ADD COLUMN IF NOT EXISTS change_reason TEXT DEFAULT '' NOT NULL; + +ALTER TABLE public.type_templates ADD COLUMN IF NOT EXISTS description TEXT DEFAULT '' NOT NULL; +ALTER TABLE public.type_templates ADD COLUMN IF NOT EXISTS change_reason TEXT DEFAULT '' NOT NULL; + +ALTER TABLE public.functions RENAME COLUMN function_description TO description; +ALTER TABLE public.functions ADD COLUMN IF NOT EXISTS change_reason TEXT DEFAULT '' NOT NULL; + +ALTER TABLE public.functions ALTER COLUMN description SET DEFAULT ''; +ALTER TABLE public.functions ALTER COLUMN description SET NOT NULL; + +ALTER TABLE public.config_versions ADD COLUMN IF NOT EXISTS description TEXT DEFAULT '' NOT NULL; +ALTER TABLE public.config_versions ADD COLUMN IF NOT EXISTS change_reason TEXT DEFAULT '' NOT NULL; \ No newline at end of file diff --git a/crates/context_aware_config/src/api/config/handlers.rs b/crates/context_aware_config/src/api/config/handlers.rs index 09c3ca2a..09ba0346 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 b0454b50..40908513 100644 --- a/crates/context_aware_config/src/api/context/handlers.rs +++ b/crates/context_aware_config/src/api/context/handlers.rs @@ -16,7 +16,8 @@ use diesel::{ delete, r2d2::{ConnectionManager, PooledConnection}, result::{DatabaseErrorKind::*, Error::DatabaseError}, - Connection, ExpressionMethods, PgConnection, QueryDsl, RunQueryDsl, + Connection, ExpressionMethods, OptionalExtension, PgConnection, QueryDsl, + RunQueryDsl, }; use jsonschema::{Draft, JSONSchema, ValidationError}; use serde_json::{from_value, json, Map, Value}; @@ -198,6 +199,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 = Value::Object(ctx_condition.clone().into()); + ensure_description(ctx_condition_value, conn)? + } else { + req.description + .clone() + .expect("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 +238,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 +309,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 +321,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 +336,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 +360,28 @@ pub fn put( } } +fn ensure_description( + context: Value, + transaction_conn: &mut diesel::PgConnection, +) -> Result { + use superposition_types::cac::schema::contexts::dsl::{ + contexts as contexts_table, id as context_id, + }; + + let context_id_value = hash(&context); + let existing_context = contexts_table + .filter(context_id.eq(context_id_value)) + .first::(transaction_conn) + .optional()?; // Query the database for existing context + + match existing_context { + Some(ctx) => Ok(ctx.description), + None => Err(superposition::AppError::NotFound(format!( + "Description Not Provided in existing context", + ))), + } +} + #[put("")] async fn put_handler( state: Data, @@ -356,18 +393,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 +458,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 +511,16 @@ 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.expect("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 +561,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 +626,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 +808,26 @@ async fn delete_context( #[cfg(feature = "high-performance-mode")] tenant: Tenant, mut db_conn: DbConnection, ) -> superposition::Result { + use superposition_types::cac::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 +856,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 +867,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 +881,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 +950,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 +991,25 @@ async fn weight_recompute( #[cfg(feature = "high-performance-mode")] tenant: Tenant, user: User, ) -> superposition::Result { - use superposition_types::cac::schema::contexts::dsl::*; + use superposition_types::cac::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 +1025,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 +1041,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 = contexts_new_weight.iter().map(|(_, _, description, _)| description.clone()).collect::>().join(","); + let change_reason = contexts_new_weight.iter().map(|(_, _, change_reason, _)| change_reason.clone()).collect::>().join(","); + 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..043106c3 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)] @@ -75,12 +81,24 @@ pub struct FunctionsInfo { pub code: Option, } +#[derive(Serialize)] +pub struct PriorityRecomputeResponse { + pub id: String, + pub condition: Condition, + pub old_priority: i32, + pub new_priority: i32, + pub description: String, + pub change_reason: String, +} + #[derive(Serialize)] pub struct WeightRecomputeResponse { pub id: String, pub condition: Condition, pub old_weight: BigDecimal, pub new_weight: BigDecimal, + pub description: String, + pub change_reason: String, } #[cfg(test)] @@ -100,7 +118,9 @@ mod tests { }, "override": { "foo": "baz" - } + }, + "description": "", + "change_reason": "" }); let action_str = json!({ @@ -122,6 +142,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 5fc8a3fb..a864d996 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 2d6e4f4d..c322e56b 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 e8cffc9c..deef4df7 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 96919779..a4695732 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 6e187f4b..78f4ba78 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/migrations/2024-11-21-103114_add_comment_and_description_in_experiments/down.sql b/crates/experimentation_platform/migrations/2024-11-21-103114_add_comment_and_description_in_experiments/down.sql new file mode 100644 index 00000000..a78f8fe2 --- /dev/null +++ b/crates/experimentation_platform/migrations/2024-11-21-103114_add_comment_and_description_in_experiments/down.sql @@ -0,0 +1,2 @@ +ALTER TABLE public.experiments DROP COLUMN IF EXISTS description; +ALTER TABLE public.experiments DROP COLUMN IF EXISTS change_reason; diff --git a/crates/experimentation_platform/migrations/2024-11-21-103114_add_comment_and_description_in_experiments/up.sql b/crates/experimentation_platform/migrations/2024-11-21-103114_add_comment_and_description_in_experiments/up.sql new file mode 100644 index 00000000..17fc9a37 --- /dev/null +++ b/crates/experimentation_platform/migrations/2024-11-21-103114_add_comment_and_description_in_experiments/up.sql @@ -0,0 +1,2 @@ +ALTER TABLE public.experiments ADD COLUMN IF NOT EXISTS description TEXT DEFAULT '' NOT NULL; +ALTER TABLE public.experiments ADD COLUMN IF NOT EXISTS change_reason TEXT DEFAULT '' NOT NULL; \ No newline at end of file diff --git a/crates/experimentation_platform/src/api/experiments/handlers.rs b/crates/experimentation_platform/src/api/experiments/handlers.rs index 7705b55d..cfde6c37 100644 --- a/crates/experimentation_platform/src/api/experiments/handlers.rs +++ b/crates/experimentation_platform/src/api/experiments/handlers.rs @@ -135,6 +135,8 @@ async fn create( use superposition_types::experimentation::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 @@ -209,6 +211,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)); } @@ -278,6 +282,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) @@ -359,12 +365,18 @@ pub async fn conclude( ) -> superposition::Result<(Experiment, Option)> { use superposition_types::experimentation::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", @@ -387,6 +399,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 { @@ -663,6 +677,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) @@ -693,6 +709,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)?; @@ -736,6 +754,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; @@ -873,6 +893,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 6ff3c615..6ea495c9 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 fcbccd55..694ff81e 100644 --- a/crates/experimentation_platform/tests/experimentation_tests.rs +++ b/crates/experimentation_platform/tests/experimentation_tests.rs @@ -11,6 +11,7 @@ use superposition_types::{ enum Dimensions { Os(String), Client(String), + #[allow(dead_code)] VariantIds(String), } @@ -70,6 +71,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 a2ce22de..2a2ad98b 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 e05df156..8eaaffa9 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,6 +96,8 @@ 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(), @@ -104,6 +110,8 @@ where schema: f_schema, value: f_value, function_name: fun_name, + description, + change_reason, }; let handle_submit_clone = handle_submit.clone(); @@ -168,6 +176,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 d3d8dd02..ec8c17b4 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 />
+
+ +
+ +