Search experiments UI #288
Conversation
Datron
force-pushed
the
search-experiments-ui
branch
from
836577f to
f832429
Compare
|
dependent on #280 |
Datron
force-pushed
the
search-experiments-ui
branch
2 times, most recently
from
4623eed to
5e4039e
Compare
| #[serde(deserialize_with = "deserialize_stringified_list")] pub Vec<String>, | ||
| ); | ||
|
|
||
| pub fn deserialize_stringified_list<'de, D, I>( |
There was a problem hiding this comment.
can we instead implement the Deserialize trait on StringArgs and write the logic in it instead of writing this as a function
There was a problem hiding this comment.
Examples for string array deserialization online use this method, it seems more verbose to implement deserialize here. We can though if needed
docker-compose/postgres/Dockerfile
Outdated
| @@ -1,4 +1,4 @@ | |||
| FROM public.ecr.aws/docker/library/postgres:12-alpine | |||
|
|
|||
| COPY ./db_init.sql /docker-entrypoint-initdb.d/db_init.sql | |||
| # CMD ["postgres", "-c", "log_statement=all"] | |||
There was a problem hiding this comment.
is there any change here ?
There was a problem hiding this comment.
No, I added this back for debugging
| pub created_by: Option<StringArgs>, | ||
| pub context: Option<String>, | ||
| pub sort_on: Option<ExperimentSortOn>, | ||
| pub sort_by: Option<SortBy>, |
There was a problem hiding this comment.
do we want to have this as a separate filter ?
if so, then shouldn't we change it for context/list as well
there I had coupled sort_on and sort_by to a single enum list
There was a problem hiding this comment.
Yeah we can do that, but it is out of scope of this PR
| #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] | ||
| pub struct ExpListFilters { | ||
| pub struct ExperimentListFilters { |
There was a problem hiding this comment.
ia this type not there in experimentation_platform for the api type ?
| > | ||
| {column_name} | ||
| { | ||
| if current { |
There was a problem hiding this comment.
what is the meaning of the this current, I mean what does it signify ( I couldn't get it) ?
There was a problem hiding this comment.
current checks if this column is currently being sorted on, if it is - it will only show the up arrow or down arrow. If it is not, it will show both arrows
| } | ||
| if let Some(ref context_search) = filters.context { | ||
| builder = builder.filter( | ||
| sql::<Bool>("context::text LIKE ") |
There was a problem hiding this comment.
what is this query doing here ?
There was a problem hiding this comment.
So the way we're storing contexts is like so:
{"and":[{"==":[{"var":"city"},"simple"]},{"==":[{"var":"vehicle_type"},"auto"]}]}
The query here is converting this (which is JSON in pg) to TEXT so we can use LIKE on it, for eg:
context::text LIKE '%{"==":[{"var":"city"},"simple"]}%'
Which would match any context that has city = simple as a dimension. It works for more than 1 dimension as well, where it does fail is if you specify vehicle=auto and city=simple but the experiment has city=simple and vehicle=auto (though city=simple or vehicle=auto would work). I want to see how folks use this feature, and if they encounter an issue I'll write the context search similar to how you've done it.
Datron
force-pushed
the
search-experiments-ui
branch
3 times, most recently
from
dfdeb62 to
3ea2b9b
Compare
Datron
force-pushed
the
search-experiments-ui
branch
from
3ea2b9b to
afbb8df
Compare
| @@ -134,11 +144,28 @@ pub struct ExperimentListFilters { | |||
| pub experiment_name: Option<String>, | |||
| pub experiment_ids: Option<String>, | |||
| pub created_by: Option<String>, | |||
| pub context: Option<String>, | |||
| pub context: Option<Value>, | |||
There was a problem hiding this comment.
the API is expecting String, why are we changing it to Value over here
There was a problem hiding this comment.
Value is stringified when sent over, yes I am sending JSON in query params
There was a problem hiding this comment.
We can change it later, I want to analyze how people use context search first
| @@ -190,6 +518,12 @@ pub fn experiment_list() -> impl IntoView { | |||
| }; | |||
| view! { | |||
| <ConditionCollapseProvider> | |||
| <div class="flex justify-start m-1"> | |||
There was a problem hiding this comment.
this div is not need here, it has only one child
There was a problem hiding this comment.
I need the flex and justify-start to align the button
| } | ||
| > | ||
| <div class="card-body"> | ||
| <div class="my-4"> |
There was a problem hiding this comment.
single child, div not needed, also can we please avoid using margins and model everything on paddings and gaps only, using margins is not very friendly for responsiveness
There was a problem hiding this comment.
paddings and gaps only, using margins is not very friendly for responsiveness
What level of responsiveness are we looking for here? Also, can you share some resources that help me understand more about why padding and gaps are better than margins for responsiveness
| </label> | ||
| <DateInput | ||
| id="experiment_from_date_input".into() | ||
| class="w-[19rem] flex-auto mt-3 mr-3".into() |
There was a problem hiding this comment.
not commenting in all the places, but can we make sure that margins are totally avoided, at least whatever new code has been added
There was a problem hiding this comment.
| @@ -33,21 +45,338 @@ struct CombinedResource { | |||
| default_config: Vec<DefaultConfig>, | |||
| } | |||
|
|
|||
| #[component] | |||
| fn experiment_table_filter_widget( | |||
There was a problem hiding this comment.
at some point, we should make this widget more generic, to simply take in structure and auto generate stuff
Problem
Filtering and searching for experiments is not possible from the UI
Solution
Add support for filtering by experiment status, date, user who created it, experiment name or experiment ID
Screen.Recording.2024-12-12.at.3.50.41.PM.mov