From 485acde1b71632b39b95235c8acf35359f6931b9 Mon Sep 17 00:00:00 2001 From: Shubhranshu Sanjeev Date: Mon, 6 Jan 2025 18:30:50 +0530 Subject: [PATCH] fix: using concise expression implementation --- .../src/components/condition_pills.rs | 24 ++--- .../frontend/src/components/context_card.rs | 9 +- .../frontend/src/components/context_form.rs | 92 +++++++++---------- crates/frontend/src/components/experiment.rs | 10 +- crates/frontend/src/logic.rs | 1 + crates/frontend/src/pages/context_override.rs | 9 +- 6 files changed, 68 insertions(+), 77 deletions(-) diff --git a/crates/frontend/src/components/condition_pills.rs b/crates/frontend/src/components/condition_pills.rs index f28da555..e75376ed 100644 --- a/crates/frontend/src/components/condition_pills.rs +++ b/crates/frontend/src/components/condition_pills.rs @@ -1,5 +1,5 @@ use crate::{ - logic::{Conditions, Constant, Expression, Operator}, + logic::{Condition, Conditions, Expression, Operator}, schema::HtmlDisplay, }; @@ -32,10 +32,10 @@ pub fn use_condition_collapser() -> WindowListenerHandle { pub fn condition_expression( #[prop(into)] id: String, #[prop(into)] list_id: String, - expression: Expression, + condition: Condition, ) -> impl IntoView { let id = store_value(id); - let expression = store_value(expression); + let condition = store_value(condition); let (expand_rs, expand_ws) = create_signal(false); let condition_id_rs = use_context::>().expect( @@ -57,12 +57,12 @@ pub fn condition_expression( } else { ("condition-item-collapsed", "condition-value-collapsed") }; - let (dimension, operator, operands): (String, Operator, Vec) = expression - .with_value(|exp| { + let (dimension, operator, operands): (String, Operator, Vec) = condition + .with_value(|v| { ( - exp.variable_name(), - exp.to_operator(), - exp.to_constants_vec().iter().map(|c| c.html_display()).collect(), + v.variable.clone(), + <&Condition as Into>::into(v), + v.expression.to_constants_vec().iter().map(|c| c.html_display()).collect(), ) }); view! { @@ -83,8 +83,8 @@ pub fn condition_expression( {operator.to_string()} - {match expression.get_value() { - Expression::Between(Constant(c1), _, Constant(c2)) => { + {match condition.get_value().expression { + Expression::Between(c1, c2) => { view! { <> @@ -135,11 +135,11 @@ pub fn condition( .get_value() .iter() .enumerate() - .map(|(idx, expression)| { + .map(|(idx, condition)| { let item_id = format!("{}-{}", id, idx); view! { diff --git a/crates/frontend/src/components/context_card.rs b/crates/frontend/src/components/context_card.rs index 16efc45d..7094e1e1 100644 --- a/crates/frontend/src/components/context_card.rs +++ b/crates/frontend/src/components/context_card.rs @@ -48,16 +48,13 @@ pub fn context_card( Column::default("VALUE".to_string()), ]; - let actions_supported = show_actions - && !conditions - .0 - .iter() - .any(|expression| expression.variable_name() == "variantIds"); + let actions_supported = + show_actions && !conditions.0.iter().any(|c| c.variable == "variantIds"); let edit_unsupported = conditions .0 .iter() - .any(|expression| matches!(expression, Expression::Other(_, _))); + .any(|c| matches!(c.expression, Expression::Other(_, _))); view! {
diff --git a/crates/frontend/src/components/context_form.rs b/crates/frontend/src/components/context_form.rs index f73a0679..ff136ee3 100644 --- a/crates/frontend/src/components/context_form.rs +++ b/crates/frontend/src/components/context_form.rs @@ -2,7 +2,7 @@ pub mod utils; use std::collections::{HashMap, HashSet}; use crate::components::input::{Input, InputType}; -use crate::logic::{Conditions, Constant, Expression, Operator, Variable}; +use crate::logic::{Condition, Conditions, Expression, Operator}; use crate::schema::EnumVariants; use crate::types::Dimension; use crate::{ @@ -17,66 +17,56 @@ pub fn condition_input( disabled: bool, resolve_mode: bool, allow_remove: bool, - expression: StoredValue, + condition: StoredValue, input_type: StoredValue, schema_type: StoredValue, #[prop(into)] on_remove: Callback, #[prop(into)] on_value_change: Callback, #[prop(into)] on_operator_change: Callback, ) -> impl IntoView { - let (dimension, operator) = - expression.with_value(|v| (v.variable_name(), v.to_operator())); + let (dimension, operator): (String, Operator) = + condition.with_value(|v| (v.variable.clone(), v.into())); - let inputs: Vec<(Value, Callback)> = match expression.get_value() { - Expression::Is(Variable(v), Constant(c)) => { + let inputs: Vec<(Value, Callback)> = match condition.get_value().expression + { + Expression::Is(c) => { vec![( c, Callback::new(move |value: Value| { - on_value_change - .call(Expression::Is(Variable(v.clone()), Constant(value))); + on_value_change.call(Expression::Is(value)); }), )] } - Expression::In(Variable(v), Constant(c)) => { + Expression::In(c) => { vec![( c, Callback::new(move |value: Value| { - on_value_change - .call(Expression::In(Variable(v.clone()), Constant(value))); + on_value_change.call(Expression::In(value)); }), )] } - Expression::Has(Constant(c), Variable(v)) => { + Expression::Has(c) => { vec![( c, Callback::new(move |value: Value| { - on_value_change - .call(Expression::Has(Constant(value), Variable(v.clone()))); + on_value_change.call(Expression::Has(value)); }), )] } - Expression::Between(Constant(c1), Variable(v), Constant(c2)) => { - let v_clone = v.clone(); + Expression::Between(c1, c2) => { let c2_clone = c2.clone(); vec![ ( c1.clone(), Callback::new(move |value: Value| { - on_value_change.call(Expression::Between( - Constant(value), - Variable(v_clone.clone()), - Constant(c2_clone.clone()), - )); + on_value_change + .call(Expression::Between(value, c2_clone.clone())); }), ), ( c2.clone(), Callback::new(move |value: Value| { - on_value_change.call(Expression::Between( - Constant(c1.clone()), - Variable(v.clone()), - Constant(value), - )); + on_value_change.call(Expression::Between(c1.clone(), value)); }), ), ] @@ -154,9 +144,13 @@ pub fn condition_input( disabled=disabled id=format!( "{}-{}", - expression + condition .with_value(|v| { - format!("{}-{}", v.variable_name(), v.to_operator()) + format!( + "{}-{}", + v.variable, + (<&Condition as Into>::into(v)), + ) }), idx, ) @@ -172,7 +166,7 @@ pub fn condition_input( class="btn btn-ghost btn-circle btn-sm mt-1" disabled=disabled on:click=move |_| { - on_remove.call(expression.with_value(|v| v.variable_name())); + on_remove.call(condition.with_value(|v| v.variable.clone())); } > @@ -206,7 +200,7 @@ where let (used_dimensions_rs, used_dimensions_ws) = create_signal( context .iter() - .map(|expression| expression.variable_name()) + .map(|condition| condition.variable.clone()) .collect::>(), ); let (context_rs, context_ws) = create_signal(context.clone()); @@ -237,7 +231,10 @@ where value.insert(dimension_name.clone()); }); context_ws.update(|value| { - value.push(Expression::is(dimension_name.as_str(), r#type)) + value.push(Condition::new_with_default_expression( + dimension_name, + r#type, + )) }); } // TODO show alert in case of invalid dimension @@ -247,7 +244,7 @@ where Callback::new(move |(idx, expression): (usize, Expression)| { context_ws.update(|v| { if idx < v.len() { - v[idx] = expression; + v[idx].expression = expression; } }); }); @@ -255,7 +252,7 @@ where let on_value_change = Callback::new(move |(idx, expression): (usize, Expression)| { context_ws.update(|v| { if idx < v.len() { - v[idx] = expression; + v[idx].expression = expression; } }) }); @@ -299,38 +296,35 @@ where .0 .into_iter() .enumerate() - .collect::>() + .collect::>() } - key=|(idx, expression)| { + key=|(idx, condition)| { format!( "{}-{}-{}", - expression.variable_name(), + condition.variable, idx, - expression.to_operator(), + condition.expression.to_operator(), ) } - children=move |(idx, expression)| { + children=move |(idx, condition)| { let (schema_type, enum_variants) = dimension_map .with_value(|v| { - let d = v.get(&expression.variable_name()).unwrap(); + let d = v.get(&condition.variable).unwrap(); ( SchemaType::try_from(d.schema.clone()), EnumVariants::try_from(d.schema.clone()), ) }); - let operator = Operator::from(&expression); - let dimension_name = expression.variable_name(); + let operator = Operator::from(&condition); if schema_type.is_err() || enum_variants.is_err() { return view! { An error occured } .into_view(); } let schema_type = store_value(schema_type.unwrap()); let allow_remove = !disabled - && !mandatory_dimensions - .get_value() - .contains(&expression.variable_name()); + && !mandatory_dimensions.get_value().contains(&condition.variable); let input_type = store_value( InputType::from(( schema_type.get_value(), @@ -338,7 +332,7 @@ where operator, )), ); - let expression = store_value(expression); + let condition = store_value(condition); let on_remove = move |d_name| on_remove.call((idx, d_name)); let on_value_change = move |expression| { on_value_change.call((idx, expression)) @@ -347,11 +341,7 @@ where on_operator_change .call(( idx, - Expression::from(( - dimension_name.as_str(), - schema_type.get_value(), - operator, - )), + Expression::from((schema_type.get_value(), operator)), )) }; view! { @@ -359,7 +349,7 @@ where disabled resolve_mode allow_remove - expression + condition input_type schema_type on_remove diff --git a/crates/frontend/src/components/experiment.rs b/crates/frontend/src/components/experiment.rs index bded6359..86721a93 100644 --- a/crates/frontend/src/components/experiment.rs +++ b/crates/frontend/src/components/experiment.rs @@ -2,8 +2,9 @@ use leptos::*; use serde_json::{Map, Value}; use std::collections::HashMap; -use crate::{components::table::Table, schema::HtmlDisplay}; +use crate::components::table::Table; +use crate::schema::HtmlDisplay; use crate::types::{Experiment, ExperimentStatusType}; use crate::{ components::table::types::Column, @@ -186,9 +187,10 @@ where {contexts .iter() - .map(|expression| { - let dimension = expression.variable_name(); - let operand_views = expression + .map(|condition| { + let dimension = condition.variable.clone(); + let operand_views = condition + .expression .to_constants_vec() .iter() .map(|c| { diff --git a/crates/frontend/src/logic.rs b/crates/frontend/src/logic.rs index 891e7c8c..857b6833 100644 --- a/crates/frontend/src/logic.rs +++ b/crates/frontend/src/logic.rs @@ -355,3 +355,4 @@ impl TryFrom<&Context> for Conditions { Self::from_context_json(&context.condition.as_ref()) } } + diff --git a/crates/frontend/src/pages/context_override.rs b/crates/frontend/src/pages/context_override.rs index e9b9d576..6c84a4f9 100644 --- a/crates/frontend/src/pages/context_override.rs +++ b/crates/frontend/src/pages/context_override.rs @@ -15,7 +15,7 @@ use crate::components::delete_modal::DeleteModal; use crate::components::drawer::{close_drawer, open_drawer, Drawer, DrawerBtn}; use crate::components::override_form::OverrideForm; use crate::components::skeleton::{Skeleton, SkeletonVariant}; -use crate::logic::{Conditions, Expression}; +use crate::logic::{Condition, Conditions}; use crate::providers::alert_provider::enqueue_alert; use crate::providers::condition_collapse_provider::ConditionCollapseProvider; use crate::providers::editor_provider::EditorProvider; @@ -178,9 +178,10 @@ pub fn context_override() -> impl IntoView { return; } - let expression = Expression::is(dim.dimension.as_str(), r#type.unwrap()); - - default_ctx.push(expression); + default_ctx.push(Condition::new_with_default_expression( + dim.dimension.clone(), + r#type.unwrap(), + )); } selected_context_ws.set(Some(Data {