Extending Cohort generation results with simple demographics available in Cohort Characterization#2959
Conversation
|
Please add additional information to this PR:
|
# Conflicts: # js/pages/cohort-definitions/cohort-definition-manager.js # js/services/CohortDefinition.js
…s (for cohorts generated with demographics checkbox enabled)
oleg-odysseus
force-pushed
the
cohort-demographics
branch
from
9bdeec0 to
b875cbb
Compare
| const urlGetReportDemographic = `${config.webAPIRoot}cohortdefinition/${(cohortDefinitionId || '-1')}/report/${sourceKey}?mode=${modeId || 0}&ccGenerateId=${ccGenerateId}` | ||
| var reportPromise = $.ajax({ | ||
| url: `${config.webAPIRoot}cohortdefinition/${(cohortDefinitionId || '-1')}/report/${sourceKey}?mode=${modeId || 0}`, | ||
| url: modeId !== 2 ? `${config.webAPIRoot}cohortdefinition/${(cohortDefinitionId || '-1')}/report/${sourceKey}?mode=${modeId || 0}` : urlGetReportDemographic, |
There was a problem hiding this comment.
for future reference, the mode ID was intended for the mode of the inclusion report, so adding a new mode_id == 2 confuses what mode_id represents in the report. I think this should be re-factored later.
chrisknoll
left a comment
chrisknoll
left a comment
There was a problem hiding this comment.
For purposes of expediency, we can approve this PR, however the LoC of ~1800 is very high to just access a new endpoint to fetch demographic data, and present two tables. I understand that it might have been an easier path to lift code from characterization reporting, there's a lot of over-engineering involved with the implementation, which has now been duplicated in 2 places. For example having all of the conversion classes duplicated is a lot of code, and can probably be greatly simplified since the ultimate goal is to render a data-table.
|
In the process of migrating to webAPI and making changes to the AtlasUI to understand places that need to be changed, this feature came across (as it only shows up as a checkbox in the cohort generation page), and I have no idea how to view reports or what the functional requirements are. I believe I asked in both this PR and OHDSI/WebAPI#2402 for documentation on this feature. To date, I've recieved none. As WebAPI 3.0 work is involving removing undocumented and obsolete features, I'm considering if this feature should remain. @alex-odysseus and @oleg-odysseus , please provide information on this as soon as you can, and we can decide if this is something that can be maintained. |
5 participants
Addressing OHDSI/WebAPI#2347