Extending Cohort generation results with simple demographics available in Cohort Characterization#2959
Extending Cohort generation results with simple demographics available in Cohort Characterization#2959chrisknoll merged 10 commits intomasterfrom
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)
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
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