feat: experiment filters in UI #424

Datron · 2025-02-19T13:39:47Z

Problem

It is hard to search for experiments

Solution

Add the ability to filter and manage experiments. Search is done on the backend, so even paginated experiments can be searched for

Screen.Recording.2025-02-19.at.7.06.34.PM.mov

ShubhranshuSanjeev · 2025-02-27T13:16:17Z

crates/frontend/src/pages/experiment_list.rs

+    combined_resource: Resource<
+        (String, ExperimentListFilters, PaginationParams, String),
+        CombinedResource,
+    >,


Suggested change

combined_resource: Resource<

(String, ExperimentListFilters, PaginationParams, String),

CombinedResource,

>,

combined_resource: CombinedResource,

ShubhranshuSanjeev · 2025-02-27T13:18:01Z

crates/frontend/src/pages/experiment_list.rs

+    let dim = combined_resource
+        .get()
+        .unwrap_or(CombinedResource {
+            experiments: PaginatedResponse {
+                total_items: 0,
+                total_pages: 0,
+                data: vec![],
+            },
+            dimensions: vec![],
+            default_config: vec![],
+        })
+        .dimensions;


I think there is no point of taking a Resource<...> here as this is not going to reactive since its outside a move closure, better to send in the CombinedResource.

ShubhranshuSanjeev · 2025-02-27T13:27:19Z

crates/frontend/src/pages/experiment_list.rs

+                            checked=move || {
+                                filters_buffer_rws
+                                    .get()
+                                    .status
+                                    .unwrap_or_default()
+                                    .iter()
+                                    .any(|item| *item == ExperimentStatusType::CREATED)
+                            }


Passing a move clousre to checked prop does it really work as intented?

Yeah! Hopefully. Maybe...

ShubhranshuSanjeev · 2025-02-27T13:30:07Z

crates/frontend/src/pages/experiment_list.rs

+
+    let status_filter_management =
+        move |checked: bool, filter_status: ExperimentStatusType| {
+            let vec = filters_buffer_rws.get().status.unwrap_or_default();


vec name is already used by macro vec!.

ShubhranshuSanjeev · 2025-02-27T13:48:25Z

crates/frontend/src/pages/experiment_list.rs

+                                let filters = filters_buffer_rws.get();
+                                let filters = ExperimentListFilters {
+                                    experiment_ids: id,
+                                    ..filters
+                                };
+                                filters_buffer_rws.set(filters);


Suggested change

let filters = filters_buffer_rws.get();

let filters = ExperimentListFilters {

experiment_ids: id,

..filters

};

filters_buffer_rws.set(filters);

filters_buffer_rws.update(|value| {

value.experiments_ids = id;

})

ShreyBana · 2025-02-25T15:03:30Z

crates/experimentation_platform/src/api/experiments/handlers.rs

-                sql::<Bool>("context::text LIKE ")
-                    .bind::<Text, _>(format!("%{}%", context_search)),
-            );
+            for c in context_search.iter() {


Little bit confused by this filter, isn't the context column a json? How does this exactly work?

Take an example:

{"and":[{"==":[{"var":"city"},"BLR"]}]} {"and":[{"==":[{"var":"city"},"MUM"]},{"==":[{"var":"vehicle_type"},"suv"]}]}

Lets say you want to find the second one, the query formed will look like:

SELECT * FROM <schema>.experiments WHERE context::text = '%{"==":[{"var":"city"},"MUM"]}%' AND context = '%{"==":[{"var":"vehicle_type"},"suv"]}%';

FAQs
Q. The type is JSON, but you're using LIKE which needs text. How does this work?
A. The ::text does the conversion

Q. Why not just do the whole context?
A. Imagine single searches, like

city = BLR

If I searched by the whole context, it would only match records that had this structure,

{"and":[{"==":[{"var":"city"},"BLR"]}]}

and would ignore records like these:

{"and":[{"==":[{"var":"city"},"BLR"]}]} {"and":[{"==":[{"var":"city"},"BLR"]},{"==":[{"var":"vehicle_type"},"suv"]}]} {"and":[{"==":[{"var":"city"},"BLR"]},{"==":[{"var":"vehicle_type"},"sedan"]}]}

Might be a bit compute heavy, so we should think about a conservative LIMIT clause here.
PS: @knutties looks like we CAN query w/ JSON as well :P

ShreyBana · 2025-02-25T15:40:27Z

crates/frontend/src/logic.rs

@@ -340,12 +341,12 @@ impl Conditions {
            None => Condition::try_from_condition_map(&context).map(|v| vec![v])?,
        }))
    }
-    pub fn to_context_json(self) -> serde_json::Value {
+    pub fn to_context_json(&self) -> serde_json::Value {


Since it's not longer consuming, maybe it's better to call it as_context_json. Would be more coherent imo.

ShreyBana · 2025-02-25T15:40:48Z

crates/frontend/src/logic.rs

        json!({
            "and": self.iter().map(|v| v.to_condition_json()).collect::<Vec<serde_json::Value>>()
        })
    }
-    pub fn to_query_string(self) -> String {
+    pub fn to_query_string(&self) -> String {


Same for this one.

ShreyBana · 2025-02-27T12:53:21Z

crates/frontend/src/components/input.rs

+                on:change=move |e| {
+                    let date = format!("{}T00:00:00Z", event_target_value(&e));
+                    logging::log!("The date selected is: {}", date);
+                    match DateTime::parse_from_rfc3339(&date) {


Since the user can't really type anything & all the input is via the calender-widget, I think we can just unwrap w/o any consequences.

They can still programmatically change the field in inspect, better safe than sorry.

How is what I'm saying any worse? It'll just panic and nothing will happen. Anyway the user chose to break the UI.

If we're willing to believe that, then the user can also just run the API call themselves, so any protection here isn't real anyway. So there's no point in bothering about any sort of protection if we are assuming such case.

ShreyBana · 2025-02-27T13:06:29Z

crates/frontend/src/components/table.rs

@@ -46,8 +46,8 @@ pub fn table(
                                                    {
                                                        match (currently_sorted, sort_by) {


You know, maybe you can keep sort_by as Option<SortBy>, that way you can avoid keeping currently_sorted. Also this way it's also less error prone, as you if forget to change currently_sorted, the UI change will not be performed.

Could you elaborate?

Just saying that you can collapse the current_sorted field (in ColumnSortable::Yes) into sort_by by changing it's type to Option<SortBy>. So, if a column is not sorted, instead of making currently_sorted false you'd just change sort_by to None. Basically in experiment_table_columns, you'd change it to:
sort_by: if current_sort_on == ExperimentSortOn::LastModifiedAt { Some(current_sort_by) } else { None }
To simplify, currently there is possibility of one changing the sort_by but forgetting to correctly set the value of current_sorted, w/ this you eliminate that possibility itself. Quite minute, but something I personally prefer.
Just a suggestion.

ShreyBana · 2025-02-27T13:26:09Z

crates/frontend/src/pages/experiment_list.rs

+
+    let dim = combined_resource
+        .get()
+        .unwrap_or(CombinedResource {


Why not unwrap_or_default? CombinedResource already has Default derived for it.

The resource it wraps isn't defined

ShreyBana · 2025-02-27T14:09:02Z

crates/frontend/src/pages/experiment_list.rs

+                            event.prevent_default();
+                            let filter = filters_buffer_rws.get();
+                            filters_rws.set(filter);
+                            close_drawer("experiment_filter_drawer")


This string probably should either an enum or a constant.

Its the element ID, too dynamic for enum. If its wrong it won't work and its pretty obvious to the person who is building and testing the drawer

ShreyBana · 2025-02-27T14:10:28Z

crates/frontend/src/pages/experiment_list.rs

+                            close_drawer("experiment_filter_drawer")
+                        }
+                    />
+                    <Button


This should be styled as some kind of cancellation button, maybe a cross?

It resets your filters, don't think cancellation is the right word here.

I'll try restart

What I meant was, it should convey a drop of filters visually, that's all, restart/reset is good.

ShreyBana · 2025-02-27T14:15:24Z

crates/frontend/src/types.rs

+impl Default for StatusTypes {
+    fn default() -> Self {
+        Self(vec![
+            ExperimentStatusType::CREATED,


For listing out all the variants there is a macro, & if add a new variant for this enum, we will probably miss it this way. Can you use that macro? Here's the link: https://docs.rs/strum/latest/strum/derive.EnumIter.html

ShreyBana · 2025-02-27T14:17:25Z

crates/frontend/src/pages/experiment_list.rs

+
+                <div class="form-control w-full">
+                    <label class="label">
+                        <span class="label-text">Experiment Status</span>


Could have looped over the variants to get this out, in-fact, if some body adds a new variant, then we might forget it. Can you loop over the variants instead?

The colors change for each variant which isn't captured by the enum, how would I apply that here?

If I am understanding you correctly, that can just be a match inside the loop:
let color = match status ...
Not sure if that was your concern though.

Datron requested a review from a team as a code owner February 19, 2025 13:39

ShubhranshuSanjeev requested changes Feb 27, 2025

View reviewed changes

ShreyBana requested changes Feb 27, 2025

View reviewed changes

Datron force-pushed the experiment-filters branch from 8702820 to 909cd94 Compare February 28, 2025 08:02

Datron requested review from ShubhranshuSanjeev and ShreyBana February 28, 2025 08:02

Datron force-pushed the experiment-filters branch 2 times, most recently from 7022fb4 to 7d1efa3 Compare February 28, 2025 09:25

feat: experiment filters in UI

551133e

Datron force-pushed the experiment-filters branch from 7d1efa3 to 551133e Compare February 28, 2025 12:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: experiment filters in UI #424

feat: experiment filters in UI #424

Datron commented Feb 19, 2025 •

edited

Loading

ShubhranshuSanjeev Feb 27, 2025

ShubhranshuSanjeev Feb 27, 2025

ShubhranshuSanjeev Feb 27, 2025

Datron Feb 28, 2025

ShubhranshuSanjeev Feb 27, 2025

ShubhranshuSanjeev Feb 27, 2025

ShreyBana Feb 25, 2025

Datron Feb 28, 2025

ShreyBana Feb 28, 2025

ShreyBana Feb 25, 2025

ShreyBana Feb 25, 2025

ShreyBana Feb 27, 2025

Datron Feb 28, 2025

ShreyBana Feb 28, 2025

ShreyBana Feb 27, 2025

Datron Feb 28, 2025

ShreyBana Feb 28, 2025

ShreyBana Feb 27, 2025

Datron Feb 28, 2025

ShreyBana Feb 27, 2025

Datron Feb 28, 2025

ShreyBana Feb 27, 2025

Datron Feb 28, 2025

Datron Feb 28, 2025

ShreyBana Feb 28, 2025

ShreyBana Feb 27, 2025 •

edited

Loading

ShreyBana Feb 27, 2025

Datron Feb 28, 2025

ShreyBana Feb 28, 2025

		@@ -46,8 +46,8 @@ pub fn table(
		{
		match (currently_sorted, sort_by) {

feat: experiment filters in UI #424

Are you sure you want to change the base?

feat: experiment filters in UI #424

Conversation

Datron commented Feb 19, 2025 • edited Loading

Problem

Solution

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShreyBana Feb 27, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Datron commented Feb 19, 2025 •

edited

Loading

ShreyBana Feb 27, 2025 •

edited

Loading