chop-dbhi / cilantro

Official client for Harvest (http://harvest.research.chop.edu)
http://cilantro.harvest.io
Other
28 stars 8 forks source link

Add config option to set the default model tree #742

Open bruth opened 9 years ago

bruth commented 9 years ago

This involves setting the tree GET parameter on outbound request URLs. This would be implemented similarly to #686

sgithens commented 9 years ago

This should restrict the concepts and fields returned from /api/concepts and /api/fields in addition to being a parameter to contexts, correct? (So that the options in the left hand pane are represented correctly).

Is this get parameter currently available on concepts/fields? I'm the trying the following, but it's not returning the subset of DataConcepts and DataFields that are traversable by the modeltree named 'volunteer' in settings.py.

http://localhost:8000/ktb/api/concepts/?tree=volunteer
bruth commented 9 years ago

@sgithens There is an issue for Serrano (https://github.com/chop-dbhi/serrano/issues/214) to add support for filtering by tree.

bruth commented 9 years ago

I just posted a PR for the Serrano issue: https://github.com/chop-dbhi/serrano/pull/258

sgithens commented 9 years ago

Great! Will try it out now.

sgithens commented 9 years ago

Filtering api/concepts looks good so far from the URL bar. Going to start messing with the cilantro UI.

sgithens commented 9 years ago

I'm currently hacking on this here: https://github.com/sgithens/cilantro/compare/cilantro-742

So far I've just experimented with adding a 'tree' parameter to the config, and slapping it on the end of all the bulk urls that get pulled from _links. On the surface this actually does quite a bit, by restricting the concepts available in the left pane, and also what columns are available for selection in the results page.

And while this partially implicitily limits what is actually be queried (because you can only select the concepts on the left), I'm assuming we should be putting the 'tree' in the PUT request to api/context/ID to actually properly put the tree in the context?

Does the api/context PUT endpoint support a tree parameter in serrano already?

Also, my other concern is detecting and resetting the context when you hit different cilantro instances in the same app. For instance, starting with this functionality I'll likely create multiple links on my dashboard for searching different modeltree, and each cilantro view from django will put in a different modeltree root. So if you click between these cilantro views in the webapp, on each change it will need to reset the feeds and query that is in the process of refinement, etc, etc.

Thoughts on the best way to detect and swap contexts like that would be helpful. I'm guessing maybe in cilantro we would look at the if a different modeltree is configured for that instance, and then look at stored contexts for the current user, and pull whatever one has that modeltree already, or create a new one if it doesn't exist.

I'm also wondering, if under the saved queries, we should list all the users saved queries, or just the ones for the explicitly modeltree in the c.config.

bruth commented 9 years ago

@sgithens Nice analysis. There are a few approaches to handling the tree parameter:

My gut tells me that third option makes the most sense. For a bit of background, I implemented the Session class to enable connecting to multiple Serrano APIs from one client. The piece I have not finished is the UI control for actually switching the session, i.e. initializing the new one and updating the views on the page. This approach would handle both cases: switching contexts (i.e. the tree) and switching endpoints.

I'm also wondering, if under the saved queries, we should list all the users saved queries, or just the ones for the explicitly modeltree in the c.config.

That is an interesting question. Queries are not bound to particular tree, although some may not be valid simply because they included fields that are reachable by the current tree. This feels like there would need to be a higher-level means (more metadata, etc.) for switching context than just toggling the tree parameter. I presume the underlying need is a workflow change (e.g. sample-centric vs. patient-centric)?

sgithens commented 9 years ago

I've only had a bit of time to look at this again this week, and unfortunately am about to leave town through the next week. I hope to keep working on it again the week of Feb 9th.