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
| 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.
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.
Addressing OHDSI/WebAPI#2347