From 62a352276ce1913bac3b8adf700829ea192aa07c Mon Sep 17 00:00:00 2001 From: Saurav Suman Date: Tue, 3 Sep 2024 01:03:44 +0530 Subject: [PATCH] refactor: Enhance context form and display unsupported operators in overrides UI --- .../src/components/condition_pills.rs | 62 +- .../src/components/condition_pills/types.rs | 38 +- .../frontend/src/components/context_card.rs | 50 +- .../frontend/src/components/context_form.rs | 605 +++++++++++------- .../src/components/context_form/utils.rs | 188 +++++- crates/frontend/src/components/experiment.rs | 47 +- .../src/components/experiment_form.rs | 16 +- .../src/components/experiment_form/utils.rs | 5 +- .../frontend/src/components/override_form.rs | 5 +- crates/frontend/src/pages/context_override.rs | 25 +- crates/frontend/src/pages/experiment.rs | 5 +- crates/frontend/src/pages/home.rs | 39 +- crates/frontend/src/types.rs | 6 +- crates/frontend/src/utils.rs | 172 ++--- 14 files changed, 765 insertions(+), 498 deletions(-) diff --git a/crates/frontend/src/components/condition_pills.rs b/crates/frontend/src/components/condition_pills.rs index 6a083293..fc2faec5 100644 --- a/crates/frontend/src/components/condition_pills.rs +++ b/crates/frontend/src/components/condition_pills.rs @@ -5,6 +5,7 @@ use crate::components::condition_pills::types::ConditionOperator; use self::types::Condition; use leptos::*; +use serde_json::Value; #[component] pub fn condition_pills(#[prop(into)] conditions: Vec) -> impl IntoView { @@ -15,6 +16,25 @@ pub fn condition_pills(#[prop(into)] conditions: Vec) -> impl IntoVie let dimension = condition.left_operand; let op = condition.operator; let val = condition.right_operand; + let filtered_vals: Vec = val.into_iter().filter_map(|v| + if v.is_object() && v.get("var").is_some() { + None + } else { + match v { + Value::String(s) => Some(s.to_string()), + Value::Number(n) => Some(n.to_string()), + Value::Bool(b) => Some(b.to_string()), + Value::Array(arr) => { + Some(arr.iter().map(|v| v.to_string()).collect::>().join(",")) + } + Value::Object(o) => { + serde_json::to_string_pretty(&o).ok() + } + _ => None + } + + } + ).collect(); view! { @@ -26,30 +46,36 @@ pub fn condition_pills(#[prop(into)] conditions: Vec) -> impl IntoVie {match op { ConditionOperator::Between => { - let split_val: Vec = val - .clone() - .split(',') - .map(String::from) - .collect(); - view! { - <> - - {&split_val[0]} - - - {"and"} + if filtered_vals.len() == 2 { + view! { + <> + + {&filtered_vals[0]} + + + {"and"} + + + {&filtered_vals[1]} + + + } + } else { + view! { + <> + - - {&split_val[1]} - - - } + + } } + } _ => { + + let rendered_values = filtered_vals.join(","); view! { <> - {val} + {rendered_values} } diff --git a/crates/frontend/src/components/condition_pills/types.rs b/crates/frontend/src/components/condition_pills/types.rs index 1839ec43..43ea1fbd 100644 --- a/crates/frontend/src/components/condition_pills/types.rs +++ b/crates/frontend/src/components/condition_pills/types.rs @@ -1,10 +1,11 @@ use std::fmt::Display; +use serde::{Deserialize, Serialize}; use serde_json::{Map, Value}; use crate::types::Context; -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Serialize, Deserialize)] pub enum ConditionOperator { Is, In, @@ -25,6 +26,18 @@ impl Display for ConditionOperator { } } +impl From for ConditionOperator { + fn from(op: String) -> Self { + match op.as_str() { + "==" => ConditionOperator::Is, + "<=" => ConditionOperator::Between, + "in" => ConditionOperator::In, + "has" => ConditionOperator::Has, + other => ConditionOperator::Other(other.to_string()), + } + } +} + impl From<(String, &Vec)> for ConditionOperator { fn from(value: (String, &Vec)) -> Self { let (operator, operands) = value; @@ -50,11 +63,11 @@ impl From<(String, &Vec)> for ConditionOperator { } } -#[derive(Clone)] +#[derive(Clone, Debug, Serialize, Deserialize)] pub struct Condition { pub left_operand: String, pub operator: ConditionOperator, - pub right_operand: String, + pub right_operand: Vec, } impl TryFrom<&Map> for Condition { @@ -76,27 +89,10 @@ impl TryFrom<&Map> for Condition { }) .unwrap_or(""); - let other_operands = operands - .iter() - .filter_map(|item| { - if item.is_object() && item.as_object().unwrap().contains_key("var") { - return None; - } - - match item { - Value::Null => String::from("null"), - Value::String(v) => v.clone(), - _ => format!("{}", item), - } - .into() - }) - .collect::>() - .join(","); - return Ok(Condition { operator, left_operand: dimension_name.to_owned(), - right_operand: other_operands, + right_operand: operands.to_vec(), }); } diff --git a/crates/frontend/src/components/context_card.rs b/crates/frontend/src/components/context_card.rs index 57a32a6f..2a83a95b 100644 --- a/crates/frontend/src/components/context_card.rs +++ b/crates/frontend/src/components/context_card.rs @@ -3,7 +3,10 @@ use serde_json::{Map, Value}; use crate::{ components::{ - condition_pills::{types::Condition, ConditionPills}, + condition_pills::{ + types::{Condition, ConditionOperator}, + ConditionPills, + }, table::{types::Column, Table}, }, types::Context, @@ -18,6 +21,7 @@ pub fn context_card( handle_delete: Callback, ) -> impl IntoView { let conditions: Vec = (&context).try_into().unwrap_or(vec![]); + let conditions_clone = conditions.clone(); let override_table_rows = overrides .clone() .into_iter() @@ -44,25 +48,37 @@ pub fn context_card( "Condition" - +
+ +
+ + +
+ + + + {"Edit Unsupported"} + + - - - -
diff --git a/crates/frontend/src/components/context_form.rs b/crates/frontend/src/components/context_form.rs index 03ac1baf..723f3788 100644 --- a/crates/frontend/src/components/context_form.rs +++ b/crates/frontend/src/components/context_form.rs @@ -2,6 +2,7 @@ pub mod utils; use std::collections::{HashMap, HashSet}; use crate::components::{ + condition_pills::types::ConditionOperator, dropdown::{Dropdown, DropdownDirection}, input_components::{BooleanToggle, EnumDropdown}, }; @@ -11,26 +12,28 @@ use leptos::*; use serde_json::{Map, Value}; use web_sys::MouseEvent; +use super::condition_pills::types::Condition; + #[component] pub fn context_form( handle_change: NF, dimensions: Vec, #[prop(default = false)] is_standalone: bool, - context: Vec<(String, String, String)>, + context: Vec, #[prop(default = String::new())] heading_sub_text: String, #[prop(default = false)] disabled: bool, #[prop(default = DropdownDirection::Right)] dropdown_direction: DropdownDirection, #[prop(default = false)] resolve_mode: bool, ) -> impl IntoView where - NF: Fn(Vec<(String, String, String)>) + 'static, + NF: Fn(Vec) + 'static, { - let _has_dimensions = !dimensions.is_empty(); + // let _has_dimensions = !dimensions.is_empty(); let (used_dimensions, set_used_dimensions) = create_signal( context .iter() - .map(|(d, _, _)| d.clone()) + .map(|condition| condition.left_operand.clone()) .collect::>(), ); let (context, set_context) = create_signal(context.clone()); @@ -59,9 +62,9 @@ where }; create_effect(move |_| { - let f_context = context.get(); - logging::log!("effect {:?}", f_context); - handle_change(f_context.clone()); + let f_context = context.get(); // context will now be a Value + logging::log!("Context form effect {:?}", f_context); + handle_change(f_context.clone()); // handle_change now expects Value }); let handle_select_dropdown_option = @@ -71,280 +74,384 @@ where value.insert(dimension_name.clone()); }); set_context.update(|value| { - value.push((dimension_name.clone(), "".to_string(), "".to_string())) + value.push(Condition { + left_operand: dimension_name.clone(), + operator: ConditionOperator::Is, + right_operand: vec![Value::String("".to_string())], + }) }); }); view! { -
-
-
- -
-
-
- -
- -
- - {move || { - let dimensions_map = dimensions - .get_value() - .into_iter() - .map(|ele| (ele.dimension.clone(), ele)) - .collect::>(); - view! { - >() - } - - key=|(idx, (dimension, op, _))| { - format!("{}-{}-{}", dimension, idx, op) - } +
+
+
+ +
+
+
+ +
+ +
+ + {move || { + let dimensions_map = dimensions + .get_value() + .into_iter() + .map(|ele| (ele.dimension.clone(), ele)) + .collect::>(); + view! { + >() + } - children=move |(idx, (dimension, mut operator, value))| { - let dimension_label = dimension.to_string(); - let dimension_name = StoredValue::new( - dimension.to_string(), - ); - let schema: Map = serde_json::from_value( - dimensions_map.get(&dimension_label).unwrap().schema.clone(), - ) - .unwrap(); - let dimension_type = get_key_type(&schema); - if operator.is_empty() { - set_context - .update_untracked(|curr_context| { - curr_context[idx].1 = String::from("=="); - }); - operator = String::from("=="); + key=|(idx, condition)| { + format!("{}-{}-{}", condition.left_operand, idx, condition.operator) } - view! { - // -
-
- - -
-
- + children=move |(idx, condition)| { + let dimension_label = condition.left_operand.to_string(); + // let dimension_label = dimension.to_string(); + let dimension_name = StoredValue::new( + condition.left_operand.to_string(), + ); + let schema: Map = serde_json::from_value( + dimensions_map.get(&dimension_label).unwrap().schema.clone(), + ) + .unwrap(); + let dimension_type = get_key_type(&schema); + if let ConditionOperator::Other(ref op_str) = condition.operator { + if op_str.is_empty() { + set_context.update_untracked(|curr_context| { + curr_context[idx].operator = ConditionOperator::Is; + }); + let mut_operator = String::from("=="); + set_context.update_untracked(|curr_context| { + curr_context[idx].operator = ConditionOperator::Other(mut_operator.clone()); + }); + } + } + view! { + // - +
+
+ + + + + + + -
-
- -
+
+
+ +
- { - let string_input = view! { - ) + let input_fields = match &condition.right_operand { + values => { + // Filter out any elements that are objects containing a "var" key + let filtered_elements: Vec<_> = values + .iter() + .filter(|v| !v.is_object() || !v.get("var").is_some()) // Exclude elements with "var" + .collect(); - name="context-dimension-value" - type="text" - placeholder="Type here" - class="input input-bordered w-full bg-white text-gray-700 shadow-md" - /> - } - .into_view(); - match operator.as_str() { - "==" => { - match dimension_type.as_str() { - "ENUM" => { - view! { - view! { + + }.into_view(), - disabled=disabled - /> - } - .into_view() - } - "BOOLEAN" => { - if value.is_empty() { - set_context - .update_untracked(|curr_context| { - curr_context[idx] - .2 = value.parse::().unwrap_or(false).to_string(); - }); - } - view! { - ().unwrap_or(false)} - update_value=Callback::new(move |flag: bool| { - set_context - .update(|curr_context| { - curr_context[idx].2 = flag.to_string(); - }); - }) + Value::Number(n) => view! { + () { // Try to parse input as f64 + set_context.update(|curr_context| { + if let Some(elem) = curr_context[idx].right_operand.get_mut(i) { + if !elem.is_object() || !elem.get("var").is_some() { // Exclude elements with "var" + *elem = Value::Number(serde_json::Number::from_f64(parsed).unwrap()); + } + } + }); + } + } + name="context-dimension-value" + type="number" + placeholder="Type here" + class="input input-bordered w-full bg-white text-gray-700 shadow-md" + /> + }.into_view(), - class=String::from("mt-2") - disabled=disabled - /> - } - .into_view() - } - _ => string_input, - } - } - _ => string_input, + Value::Bool(b) => view! { + + }.into_view(), + + _ => view! { + + }.into_view(), + }) + .collect::>() // Collect the result into a Vec + } + }; + match condition.operator { + ConditionOperator::Is => { + match dimension_type.as_str() { + "ENUM" => { + view! { + + } + .into_view() + } + "BOOLEAN" => { + // Ensure we handle Value properly and default to false if not a valid boolean + let is_checked = match &condition.right_operand[0] { + Value::Bool(b) => *b, // Extract the boolean value directly + _ => false, // Default to false if not a boolean + }; + view! { + + } + .into_view() + } + _ => view! { + { + logging::log!("Condition operator and saurav {:?}", condition.operator); + input_fields.into_view() + } + } // Fallback to input field if not ENUM or BOOLEAN + } + } + _ => view! { + {input_fields.into_view()} + } // For other operators, use input_fields as default + } } - } - - - + + + +
-
- {move || { - if last_idx.get() != idx { - view! { -
- "&&" -
+ {move || { + if last_idx.get() != idx { + view! { +
+ "&&" +
+ } + .into_view() + } else { + view! {}.into_view() } - .into_view() - } else { - view! {}.into_view() - } - }} + }} + } } - } - /> - } - }} + /> + } + }} - -
+ +
- {move || { - let dimensions = dimensions - .get_value() - .into_iter() - .filter(|dimension| { - !used_dimensions.get().contains(&dimension.dimension) - }) - .collect::>(); - view! { - - } - }} + {move || { + let dimensions = dimensions + .get_value() + .into_iter() + .filter(|dimension| { + !used_dimensions.get().contains(&dimension.dimension) + }) + .collect::>(); + view! { + + } + }} -
- +
+ +
+ +
+ +
+
- -
- -
- -
- } + } } diff --git a/crates/frontend/src/components/context_form/utils.rs b/crates/frontend/src/components/context_form/utils.rs index cc3990a7..6d0a1c76 100644 --- a/crates/frontend/src/components/context_form/utils.rs +++ b/crates/frontend/src/components/context_form/utils.rs @@ -1,62 +1,184 @@ -use crate::types::Dimension; -use crate::utils::{ - construct_request_headers, get_config_value, get_host, parse_json_response, request, - ConfigType, +use crate::{ + components::condition_pills::types::{Condition, ConditionOperator}, + types::Dimension, + utils::{ + construct_request_headers, get_config_value, get_host, parse_json_response, + request, ConfigType, + }, }; use anyhow::Result; use serde_json::{json, Map, Value}; pub fn get_condition_schema( - var: &str, - op: &str, - val: &str, + condition: &Condition, dimensions: Vec, ) -> Result { + let var = &condition.left_operand; // Dimension name + let op = &condition.operator; // Operator type + let val = &condition.right_operand; // Vec + + // Extract non-"var" elements from the right_operand + let filtered_values: Vec<&Value> = val + .iter() + .filter(|v| !v.is_object() || !v.get("var").is_some()) // Ignore objects with "var" + .collect(); + let dimensions_clone = dimensions.clone(); + match op { - "<=" => { - let mut split_value = val.split(','); + ConditionOperator::Between => { + // Expecting three elements for "Between" condition: two operands and one "var" object + if filtered_values.len() != 2 { + return Err( + "Invalid number of operands for 'between' condition.".to_string() + ); + } - let first_operand = - split_value.next().unwrap().trim().parse::().unwrap(); + let first_operand = &filtered_values[0]; // The first value + let third_operand = &filtered_values[1]; // The third value - let dimension_val = get_config_value( + let first_operand_value = get_config_value( var, - split_value.next().unwrap().trim(), + first_operand, &dimensions .into_iter() .map(ConfigType::Dimension) .collect::>(), - ); + )?; + + let third_operand_value = get_config_value( + var, + third_operand, + &dimensions_clone + .into_iter() + .map(ConfigType::Dimension) + .collect::>(), + )?; Ok(json!({ - op: [ - first_operand, + "<=": [ + first_operand_value, { "var": var }, - dimension_val.expect("can't parse dimension value") + third_operand_value ] })) } - _ => { - let dimension_val = get_config_value( + ConditionOperator::Is => { + // Expecting two elements for "Is" condition: one "var" object and one value + if filtered_values.len() != 1 { + return Err("Invalid number of operands for 'is' condition.".to_string()); + } + + let value = &filtered_values[0]; // The value after "var" + let first_operand_value = get_config_value( var, - val, + value, &dimensions .into_iter() .map(ConfigType::Dimension) .collect::>(), - ); + )?; + Ok(json!({ - op: [ - {"var": var}, - dimension_val.expect("can't parse dimension value") + "==": [ + { "var": var }, + first_operand_value ] })) } + ConditionOperator::In => { + if filtered_values.len() != 1 { + return Err("Invalid number of operands for 'in' condition.".to_string()); + } + let value = &filtered_values[0]; // The value after "var" + let first_operand_value = get_config_value( + var, + value, + &dimensions + .into_iter() + .map(ConfigType::Dimension) + .collect::>(), + )?; + + Ok(json!({ + "in": [ + { "var": var }, + first_operand_value + ] + })) + } + ConditionOperator::Has => { + if filtered_values.len() != 1 { + return Err("Invalid number of operands for 'has' condition.".to_string()); + } + let value = &filtered_values[0]; // The value after "var" + let first_operand_value = get_config_value( + var, + value, + &dimensions + .into_iter() + .map(ConfigType::Dimension) + .collect::>(), + )?; + + Ok(json!({ + "in": [ + first_operand_value, + { "var": var } + ] + })) + } + ConditionOperator::Other(op) => { + if filtered_values.len() == 1 { + let value = &filtered_values[0]; // The value after "var" + let first_operand_value = get_config_value( + var, + value, + &dimensions + .into_iter() + .map(ConfigType::Dimension) + .collect::>(), + )?; + Ok(json!({ + op: [ + { "var": var }, + first_operand_value + ] + })) + } else if filtered_values.len() == 2 { + let first_operand = &filtered_values[0]; // The first value + let second_operand = &filtered_values[1]; // The second value + let first_operand_value = get_config_value( + var, + first_operand, + &dimensions + .into_iter() + .map(ConfigType::Dimension) + .collect::>(), + )?; + let second_operand_value = get_config_value( + var, + second_operand, + &dimensions_clone + .into_iter() + .map(ConfigType::Dimension) + .collect::>(), + )?; + Ok(json!({ + op: [ + first_operand_value, + { "var": var }, + second_operand_value + ] + })) + } else { + Err("Invalid number of operands for custom operator.".to_string()) + } + } } } pub fn construct_context( - conditions: Vec<(String, String, String)>, + conditions: Vec, dimensions: Vec, ) -> Value { if conditions.is_empty() { @@ -64,10 +186,7 @@ pub fn construct_context( } else { let condition_schemas = conditions .iter() - .map(|(variable, operator, value)| { - get_condition_schema(variable, operator, value, dimensions.clone()) - .unwrap() - }) + .map(|condition| get_condition_schema(condition, dimensions.clone()).unwrap()) .collect::>(); if condition_schemas.len() == 1 { @@ -80,14 +199,14 @@ pub fn construct_context( pub fn construct_request_payload( overrides: Map, - conditions: Vec<(String, String, String)>, + conditions: Vec, dimensions: Vec, ) -> Value { // Construct the override section let override_section: Map = overrides; // Construct the context section - let context_section = construct_context(conditions, dimensions); + let context_section = construct_context(conditions, dimensions.clone()); // Construct the entire request payload let request_payload = json!({ @@ -101,7 +220,7 @@ pub fn construct_request_payload( pub async fn create_context( tenant: String, overrides: Map, - conditions: Vec<(String, String, String)>, + conditions: Vec, dimensions: Vec, ) -> Result { let host = get_host(); @@ -121,12 +240,13 @@ pub async fn create_context( pub async fn update_context( tenant: String, overrides: Map, - conditions: Vec<(String, String, String)>, + conditions: Vec, dimensions: Vec, ) -> Result { let host = get_host(); let url = format!("{host}/context/overrides"); - let request_payload = construct_request_payload(overrides, conditions, dimensions); + let request_payload = + construct_request_payload(overrides, conditions, dimensions.clone()); let response = request( url, reqwest::Method::PUT, diff --git a/crates/frontend/src/components/experiment.rs b/crates/frontend/src/components/experiment.rs index fc7d1ece..85c6ee36 100644 --- a/crates/frontend/src/components/experiment.rs +++ b/crates/frontend/src/components/experiment.rs @@ -3,8 +3,8 @@ pub mod utils; use std::rc::Rc; use leptos::*; +use serde_json::Value; -use crate::components::condition_pills::utils::extract_conditions; use crate::components::table::Table; use self::utils::gen_variant_table; @@ -25,7 +25,7 @@ where HE: Fn() + 'static + Clone, { let experiment_rc = Rc::new(experiment.clone()); - let contexts = extract_conditions(&experiment_rc.clone().context); + let contexts = Rc::clone(&experiment_rc).context.clone(); view! {
@@ -160,17 +160,38 @@ where {move || { let mut view = Vec::new(); for token in contexts.clone() { - let (dimension, value) = (token.left_operand, token.right_operand); - view.push( - view! { -
-
{dimension}
-
- {&value.replace('"', "")} -
-
- }, - ); + let (dimension, values) = (token.left_operand, token.right_operand); + let mut value_views = Vec::new(); + + for value in values.iter() { + if value.is_object() && value.get("var").is_some() { + continue; + } + + let value_str = match value { + Value::String(s) => s.clone(), + Value::Number(n) => n.to_string(), + Value::Bool(b) => b.to_string(), + Value::Null => String::from("null"), + _ => format!("{}", value), + }; + + value_views.push( + view! { +
+ {&value_str.replace('"', "")} +
+ }, + ); + + } + view.push(view! { +
+
{dimension}
+ {value_views} +
+ }); + } view }} diff --git a/crates/frontend/src/components/experiment_form.rs b/crates/frontend/src/components/experiment_form.rs index 34e38235..8cdc9310 100644 --- a/crates/frontend/src/components/experiment_form.rs +++ b/crates/frontend/src/components/experiment_form.rs @@ -9,6 +9,8 @@ use crate::types::{DefaultConfig, Dimension, VariantFormT, VariantType}; use leptos::*; use web_sys::MouseEvent; +use super::condition_pills::types::Condition; + fn default_variants_for_form() -> Vec<(String, VariantFormT)> { vec![ ( @@ -48,7 +50,7 @@ pub fn experiment_form( #[prop(default = false)] edit: bool, #[prop(default = String::new())] id: String, name: String, - context: Vec<(String, String, String)>, + context: Vec, variants: Vec, handle_submit: NF, default_config: Vec, @@ -65,7 +67,7 @@ where let (f_context, set_context) = create_signal(context.clone()); let (f_variants, set_variants) = create_signal(init_variants); - let handle_context_form_change = move |updated_ctx: Vec<(String, String, String)>| { + let handle_context_form_change = move |updated_ctx: Vec| { set_context.set_untracked(updated_ctx); }; @@ -78,7 +80,7 @@ where let on_submit = move |event: MouseEvent| { event.prevent_default(); logging::log!("Submitting experiment form"); - logging::log!("{:?}", f_variants.get()); + logging::log!("Variant Ids{:?}", f_variants.get()); let f_experiment_name = experiment_name.get(); let f_context = f_context.get(); @@ -91,8 +93,8 @@ where let experiment_id = id.clone(); let handle_submit_clone = handle_submit.clone(); - logging::log!("{:?}", f_experiment_name); - logging::log!("{:?}", f_context); + logging::log!("Experiment name {:?}", f_experiment_name); + logging::log!("Context Experiment form {:?}", f_context); spawn_local({ async move { @@ -104,7 +106,7 @@ where f_variants, f_experiment_name, tenant, - dimensions.get_value(), + dimensions.get_value().clone(), ) .await }; @@ -147,7 +149,7 @@ where let context = f_context.get(); view! { context=context handle_change=handle_context_form_change is_standalone=false diff --git a/crates/frontend/src/components/experiment_form/utils.rs b/crates/frontend/src/components/experiment_form/utils.rs index 820a4af1..431b85fa 100644 --- a/crates/frontend/src/components/experiment_form/utils.rs +++ b/crates/frontend/src/components/experiment_form/utils.rs @@ -1,4 +1,5 @@ use super::types::{ExperimentCreateRequest, ExperimentUpdateRequest}; +use crate::components::condition_pills::types::Condition; use crate::components::context_form::utils::construct_context; use crate::types::{Dimension, VariantFormT}; use crate::utils::{construct_request_headers, get_host, parse_json_response, request}; @@ -12,7 +13,7 @@ pub fn validate_experiment(experiment: &ExperimentCreateRequest) -> Result, + conditions: Vec, variants: Vec, name: String, tenant: String, @@ -21,7 +22,7 @@ pub async fn create_experiment( let payload = ExperimentCreateRequest { name, variants: FromIterator::from_iter(variants), - context: construct_context(conditions, dimensions), + context: construct_context(conditions, dimensions.clone()), }; let _ = validate_experiment(&payload)?; diff --git a/crates/frontend/src/components/override_form.rs b/crates/frontend/src/components/override_form.rs index 8399d92f..8f597e40 100644 --- a/crates/frontend/src/components/override_form.rs +++ b/crates/frontend/src/components/override_form.rs @@ -55,7 +55,7 @@ where let get_default_config_val = move |config_key_value: String, value: String| { get_config_value( &config_key_value, - &value, + &Value::String(value), &default_config .get_value() .into_iter() @@ -168,7 +168,6 @@ where handle_change=Callback::new(move |selected_enum: String| { update_overrides(&config_key_value, selected_enum) }) - class=String::from("mt-2") /> } @@ -209,7 +208,7 @@ where } > - {config_value} + {config_value.to_string()}
} diff --git a/crates/frontend/src/pages/context_override.rs b/crates/frontend/src/pages/context_override.rs index 13b50512..bfd9d273 100644 --- a/crates/frontend/src/pages/context_override.rs +++ b/crates/frontend/src/pages/context_override.rs @@ -2,6 +2,8 @@ use crate::api::fetch_config; use crate::api::{delete_context, fetch_default_config, fetch_dimensions}; use crate::components::alert::AlertType; use crate::components::button::Button; +use crate::components::condition_pills::types::{Condition, ConditionOperator}; +use crate::components::condition_pills::utils::extract_conditions; use crate::components::context_card::ContextCard; use crate::components::context_form::utils::{create_context, update_context}; use crate::components::context_form::ContextForm; @@ -11,7 +13,6 @@ use crate::components::override_form::OverrideForm; use crate::components::skeleton::{Skeleton, SkeletonVariant}; use crate::providers::alert_provider::enqueue_alert; use crate::types::{Config, Context, DefaultConfig, Dimension}; -use crate::utils::extract_conditions; use futures::join; use leptos::*; use serde::{Deserialize, Serialize}; @@ -19,7 +20,7 @@ use serde_json::{json, Map, Value}; #[derive(Clone, Debug, Default)] pub struct Data { - pub context: Vec<(String, String, String)>, + pub context: Vec, pub overrides: Vec<(String, Value)>, } @@ -38,7 +39,7 @@ enum FormMode { #[component] fn form( - context: Vec<(String, String, String)>, + context: Vec, overrides: Vec<(String, Value)>, dimensions: Vec, edit: bool, @@ -54,13 +55,13 @@ fn form( spawn_local(async move { let f_context = context.get(); let f_overrides = overrides.get(); - let dimensions = dimensions.get_value(); + let dimensions = dimensions.get_value().clone(); let result = if edit { update_context( tenant_rs.get().clone(), Map::from_iter(f_overrides), f_context, - dimensions, + dimensions.clone(), ) .await } else { @@ -68,7 +69,7 @@ fn form( tenant_rs.get().clone(), Map::from_iter(f_overrides), f_context, - dimensions, + dimensions.clone(), ) .await }; @@ -156,12 +157,16 @@ pub fn context_override() -> impl IntoView { .into_iter() .filter_map(|dim| { if dim.mandatory { - Some((dim.dimension, String::from(""), String::from(""))) + Some(Condition { + left_operand: dim.dimension, + operator: ConditionOperator::Other(String::from("")), + right_operand: vec![], + }) } else { None } }) - .collect::>(); + .collect::>(); set_selected_data.set(Some(Data { context: context_with_mandatory_dimensions, overrides: vec![], @@ -179,7 +184,7 @@ pub fn context_override() -> impl IntoView { let handle_context_edit = Callback::new(move |data: (Context, Map)| { let (context, overrides) = data; - let conditions = extract_conditions(&context.condition).unwrap_or(vec![]); + let conditions = extract_conditions(&context.condition); set_selected_data.set(Some(Data { context: conditions, @@ -193,7 +198,7 @@ pub fn context_override() -> impl IntoView { let handle_context_clone = Callback::new(move |data: (Context, Map)| { let (context, overrides) = data; - let conditions = extract_conditions(&context.condition).unwrap_or(vec![]); + let conditions = extract_conditions(&context.condition); set_selected_data.set(Some(Data { context: conditions, diff --git a/crates/frontend/src/pages/experiment.rs b/crates/frontend/src/pages/experiment.rs index b3ce75b4..e8937bc5 100644 --- a/crates/frontend/src/pages/experiment.rs +++ b/crates/frontend/src/pages/experiment.rs @@ -14,7 +14,7 @@ use crate::{ skeleton::{Skeleton, SkeletonVariant}, }, types::{DefaultConfig, Dimension, Experiment}, - utils::{close_modal, extract_conditions, show_modal}, + utils::{close_modal, show_modal}, }; use crate::components::experiment_ramp_form::ExperimentRampForm; @@ -133,8 +133,7 @@ pub fn experiment_page() -> impl IntoView { edit=true id=experiment.id name=experiment_ef.name - context=extract_conditions(&experiment_ef.context) - .unwrap_or_default() + context= experiment_ef.context.clone() variants=FromIterator::from_iter(experiment_ef.variants) default_config=default_config dimensions=dimensions diff --git a/crates/frontend/src/pages/home.rs b/crates/frontend/src/pages/home.rs index f30592a6..e17cd459 100644 --- a/crates/frontend/src/pages/home.rs +++ b/crates/frontend/src/pages/home.rs @@ -1,6 +1,7 @@ +use std::borrow::Cow; use std::time::Duration; -use crate::components::condition_pills::types::Condition; +use crate::components::condition_pills::types::{Condition, ConditionOperator}; use crate::components::condition_pills::ConditionPills; use crate::components::skeleton::{Skeleton, SkeletonVariant}; use crate::{ @@ -64,7 +65,7 @@ pub fn home() -> impl IntoView { }, ); - let (context_rs, context_ws) = create_signal::>(vec![]); + let (context_rs, context_ws) = create_signal::>(vec![]); let (selected_tab_rs, selected_tab_ws) = create_signal(ResolveTab::AllConfig); let unstrike = |search_field_prefix: &String, config: &Map| { @@ -121,13 +122,37 @@ pub fn home() -> impl IntoView { } }; - let gen_query_context = |query: Vec<(String, String, String)>| -> String { + let gen_query_context = |query: Vec| -> String { let mut context: Vec = vec![]; - for (dimension, op, value) in query.iter() { - let op = match op.as_str() { - "==" => "=", - _ => break, // query params do not support the other operators : != and IN, do something differently later + for condition in query.iter() { + let dimension = condition.left_operand.clone(); + let op = match condition.operator.clone() { + ConditionOperator::Is => Cow::Borrowed("="), + ConditionOperator::In => Cow::Borrowed("IN"), + ConditionOperator::Has => Cow::Borrowed("HAS"), + ConditionOperator::Between => Cow::Borrowed("BETWEEN"), + ConditionOperator::Other(op) => Cow::Owned(op), }; + let value = condition + .right_operand + .clone() + .into_iter() + .filter_map(|value| { + if value.is_object() && value.get("var").is_some() { + None + } else { + Some(value) + } + }) + .map(|value| match value { + Value::String(s) => s.clone(), + Value::Number(n) => n.to_string(), + Value::Bool(b) => b.to_string(), + Value::Null => String::from("null"), + _ => format!("{}", value), + }) + .collect::>() + .join(","); context.push(format!("{}{op}{}", dimension, value)); } context.join("&").to_string() diff --git a/crates/frontend/src/types.rs b/crates/frontend/src/types.rs index 2d30a5c3..06e2fd75 100644 --- a/crates/frontend/src/types.rs +++ b/crates/frontend/src/types.rs @@ -6,7 +6,9 @@ use chrono::{DateTime, NaiveDateTime, Utc}; use derive_more::{Deref, DerefMut}; use serde_json::{Map, Value}; -use crate::components::dropdown::utils::DropdownOption; +use crate::components::{ + condition_pills::types::Condition, dropdown::utils::DropdownOption, +}; #[derive(Clone, Debug)] pub struct AppRoute { @@ -182,7 +184,7 @@ pub struct Experiment { pub(crate) name: String, pub(crate) id: String, pub(crate) traffic_percentage: u8, - pub(crate) context: Value, + pub(crate) context: Vec, pub(crate) status: ExperimentStatusType, pub(crate) override_keys: Value, pub(crate) created_by: String, diff --git a/crates/frontend/src/utils.rs b/crates/frontend/src/utils.rs index 31964a91..0548426d 100644 --- a/crates/frontend/src/utils.rs +++ b/crates/frontend/src/utils.rs @@ -184,81 +184,6 @@ pub fn close_modal(id: &str) { } } -pub fn get_variable_name_and_value( - operands: &Vec, -) -> Result<(&str, String), String> { - let (obj_pos, variable_obj) = operands - .iter() - .enumerate() - .find(|(_, operand)| { - operand.is_object() - && operand - .as_object() - .expect("unable to parse operands as object") - .get("var") - .is_some() - }) - .ok_or(" failed to get variable name from operands list".to_string())?; - - let variable_name = variable_obj - .as_object() - .and_then(|obj| obj.get("var")) - .and_then(|value| value.as_str()) - .ok_or(" failed to get variable name from operands list".to_string())?; - - let variable_value = operands - .iter() - .enumerate() - .filter(|(idx, _)| *idx != obj_pos) - .map(|(_, val)| val.to_string().replace('"', "")) - .collect::>() - .join(","); - - Ok((variable_name, variable_value)) -} - -pub fn extract_conditions( - context_json: &Value, -) -> Result, String> { - // Assuming max 2-level nesting in context json logic - let context = context_json.as_object().ok_or( - "An error occurred while extracting dimensions: context not a valid JSON object" - .to_string(), - )?; - - let conditions = match context.get("and") { - Some(conditions_json) => conditions_json - .as_array() - .ok_or("An error occurred while extracting dimensions: failed parsing conditions as an array".to_string())? - .clone(), - None => vec![context_json.clone()], - }; - - let mut condition_tuples = Vec::new(); - for condition in &conditions { - let condition_obj = condition - .as_object() - .ok_or("failed to parse condition as an object".to_string())?; - let operators = condition_obj.keys(); - - for operator in operators { - let operands = condition_obj[operator] - .as_array() - .ok_or("failed to parse operands as an arrays".to_string())?; - - let (variable_name, variable_value) = get_variable_name_and_value(operands)?; - - condition_tuples.push(( - String::from(variable_name), - operator.to_owned(), - variable_value.to_owned(), - )); - } - } - - Ok(condition_tuples) -} - pub fn check_url_and_return_val(s: String) -> String { match Url::parse(&s) { Ok(_) => format!( @@ -334,57 +259,82 @@ pub fn get_config_type( }) } -pub fn parse_value(val: &str, config_type: ConfigValueType) -> Result { +pub fn parse_value(val: &Value, config_type: ConfigValueType) -> Result { match config_type { - ConfigValueType::Boolean => bool::from_str(val) - .map(Value::Bool) - .map_err(|_| "Invalid boolean".to_string()), + ConfigValueType::Boolean => { + match val { + Value::Bool(_) => Ok(val.clone()), + + Value::String(s) => { + // Attempting to parse the string as a boolean + match s.to_lowercase().as_str() { + "true" => Ok(Value::Bool(true)), + "false" => Ok(Value::Bool(false)), + _ => Err(format!("Invalid boolean string: {:?}", s)), // Error if not a valid boolean string + } + } + + _ => Err(format!("Invalid boolean value: {:?}", val)), + } + } + ConfigValueType::Number | ConfigValueType::Integer => { - let parsed_value: Value = serde_json::from_str(val) - .map_err(|_| "Invalid number or number array format".to_string())?; - match parsed_value { - Value::Number(num) => Ok(Value::Number(num)), + match val { + Value::Number(num) => Ok(Value::Number(num.clone())), + + Value::String(s) => { + // Attempting to parse as integer first, then as float + if let Ok(int_val) = s.parse::() { + Ok(Value::Number(int_val.into())) + } else if let Ok(float_val) = s.parse::() { + Ok(Value::Number( + serde_json::Number::from_f64(float_val).unwrap(), + )) + } else { + Err(format!("Invalid number format: {:?}", s)) + } + } + Value::Array(arr) => { - for item in &arr { - if !item.is_number() { - return Err("Array contains non-number value".to_string()); - } + // Ensuring all items in the array are numbers + if arr.iter().all(|item| item.is_number()) { + Ok(val.clone()) + } else { + Err("Array contains non-number value".to_string()) } - Ok(Value::Array(arr)) } + _ => Err(format!( - "{:?} is either an invalid number or a invalid number array.", + "{:?} is neither a valid number nor an array of numbers.", val )), } } - ConfigValueType::String => { - let parsed_value: Result = serde_json::from_str(&val); - match parsed_value { - Ok(Value::String(s)) => Ok(Value::String(s)), - Ok(Value::Array(arr)) => { - for item in &arr { - if !item.is_string() { - return Err("Array contains non-string value".to_string()); - } - } - Ok(Value::Array(arr)) + + ConfigValueType::String => match val { + Value::String(_) => Ok(val.clone()), + Value::Array(arr) => { + if arr.iter().all(|item| item.is_string()) { + Ok(val.clone()) + } else { + Err("Array contains non-string value".to_string()) } - Ok(_) => Err(format!( - "{:?} is either an invalid string or a invalid string array.", - val - )), - Err(_) => Ok(Value::String(val.to_string())), } - } - ConfigValueType::Null if val == "null" => Ok(Value::Null), - _ => Value::from_str(val).map_err(|err| format!("Error parsing JSON: {}", err)), + _ => Err(format!( + "{:?} is neither a valid string nor an array of strings.", + val + )), + }, + + ConfigValueType::Null if val.is_null() => Ok(Value::Null), + + _ => Ok(val.clone()), } } pub fn get_config_value( name: &str, - val: &str, + val: &Value, configs: &[ConfigType], ) -> Result { let config_type = get_config_type(configs, name); @@ -398,9 +348,7 @@ pub fn get_config_value( } Err("Error parsing config value".to_string()) } - None => { - Value::from_str(val).map_err(|err| format!("Error parsing JSON: {}", err)) - } + None => Ok(val.clone()), } }