Add chart drilldown to concept set pages#2178
Add chart drilldown to concept set pages#2178anton-abushkevich wants to merge 8 commits intomasterfrom
Conversation
…rilldown_to_concept_set_pages
chrisknoll
left a comment
There was a problem hiding this comment.
This function says it adds 'explore timeseries' to concept sets, but I do not see it when looking at a concept set inside a cohort definition (or any of the other 'embedded' concept set types. I think you should be able to add this tab to the concept set editor that is presented for entities that have embedded concept sets. Is this possible?
| @Path("{sourceKey}/multidrilldown") | ||
| @Consumes(MediaType.APPLICATION_JSON) | ||
| @Produces(MediaType.APPLICATION_JSON) | ||
| public JsonNode getMultipleDrilldown(@PathParam("sourceKey") final String sourceKey, |
There was a problem hiding this comment.
I am not a huge fan of returning a 'JsonNode' as the response because it has no concrete type and there's no real way to know what elements are in the data structure unless you go through each line of implementation to see how the object is built. It's like changing all of our DTOs to HashTables and having no way of knowing what is in smething like 'CohortDefinition' without looking through the code and seeing what fields are defined.
Was there a reason for this? Is the underlying data structure (from achilles results) json? If so, couldn't we have returned List that at least would reflect the return documentation: @return List of JSON results. Each node contains JSON results for single domain?
|
As I described in the UI PR (OHDSI/Atlas#2806), I suggest removing the reports at the 'conceptset' level, but keeping the 'concept' drilldowns. I believe that means that we don't need any WebAPI changes at all to show concept drilldowns in the UI. I think most of this PR is invalidated because the use of SUM() and AVG() to combine concept statistics is incorrect/improper. |
|
@chrisknoll |
|
If there are no requirements in this PR for OHDSI/Atlas#2805, then yes, you can close. |
Backend for OHDSI/Atlas#2805