advanced search#4
Conversation
davidwaroquiers
left a comment
There was a problem hiding this comment.
Great start!
Here are a few comments:
1/ Make it general (not tied to samples). The DynamicDataTable is used for the Samples view but is also used for other views (e.g. equipment, starting materials, ...) and might be used in the future for other new views. It should be possible to make this implementation general so that it would be directly (maybe needs to specify which route(s) needs to be used for the queries) usable for any new view.
2/ For the query options, there are two "ways" I would see for this. The one you implemented is the "general" way, i.e. the schemas are automatically detected and query options are automatically generated.
I would also see another way where the query options are "predefined" within the definition of the item type (e.g. in the json_schema_extra key of the model_config). Something like: "query_options_list": ...)
3/ Also add the possibility for subfields.
I'll make a more thorough review once that is in.
| <AdvancedQueryBuilder | ||
| v-if="dataType === 'samples'" | ||
| :list-view="'samples'" | ||
| @query-results="$emit('advanced-query-results', $event)" | ||
| /> | ||
|
|
There was a problem hiding this comment.
I think this should be changed. It "ties" the advanced query feature to the sample view. That's what I was referring to as "generalizable" during our discussion
There was a problem hiding this comment.
To be more explicit, if you look at where the DynamicDataTable is used, it used in many places (e.g. equipment view etc ...). The idea of being general is that you should not need to change DynamicDataTable if you add a new place where it is used. I would say though that the AdvancedQueryBuilder is tightly coupled with the DynamicDataTable, i.e. I would not expect the AdvancedQueryBuilder to work outside of the DynamicDataTable. The problem here is that there is the "v-if". We can discuss if to clarify if needed.
Here is the advanced search button.
For now it is in the process