Skip to content

Commit 1fd5ac9

Browse files
pieterlukassepieterlukasse
committed
feat: add filter and transformation support to breakdown / attrition endpoint
Also: - feat: update RetrieveStatsForCohortIdAndConceptId and remove deprecated method - feat: refactor RetrieveCohortOverlapStats and fix breakdown stats method ...and fix model tests - feat: comment on a comment ...see also #5 (comment) - feat: refactor RetrieveDataBySourceIdAndCohortIdAndVariables ... and deprecate model method in favor of the histogram one that retrieves the same data but is already refactored. - feat: remove deprecated breakdown stats method
1 parent 859816c commit 1fd5ac9

File tree

9 files changed

+243
-369
lines changed

9 files changed

+243
-369
lines changed

‎controllers/cohortdata.go

+46-50
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,7 @@ func (u CohortDataController) RetrieveHistogramForCohortIdAndConceptId(c *gin.Co
3838

3939
// parse cohortPairs separately as well, so we can validate permissions
4040
_, cohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptIdsAndCohortPairs)
41-
if err != nil {
42-
c.JSON(http.StatusInternalServerError, gin.H{"message": "Error parsing request body for prefixed concept ids", "error": err.Error()})
43-
c.Abort()
44-
return
45-
}
41+
4642
validAccessRequest := u.teamProjectAuthz.TeamProjectValidation(c, []int{cohortId}, cohortPairs)
4743
if !validAccessRequest {
4844
log.Printf("Error: invalid request")
@@ -69,22 +65,15 @@ func (u CohortDataController) RetrieveHistogramForCohortIdAndConceptId(c *gin.Co
6965
}
7066

7167
func (u CohortDataController) RetrieveStatsForCohortIdAndConceptId(c *gin.Context) {
72-
sourceIdStr := c.Param("sourceid")
73-
log.Printf("Querying source: %s", sourceIdStr)
74-
cohortIdStr := c.Param("cohortid")
75-
log.Printf("Querying cohort for cohort definition id: %s", cohortIdStr)
76-
conceptIdStr := c.Param("conceptid")
77-
if sourceIdStr == "" || cohortIdStr == "" || conceptIdStr == "" {
78-
c.JSON(http.StatusBadRequest, gin.H{"message": "bad request"})
68+
sourceId, cohortId, conceptIdsAndCohortPairs, err := utils.ParseSourceIdAndCohortIdAndVariablesAsSingleList(c)
69+
if err != nil {
70+
c.JSON(http.StatusBadRequest, gin.H{"message": "bad request", "error": err.Error()})
7971
c.Abort()
8072
return
8173
}
8274

83-
filterConceptIdsAndValues, cohortPairs, _ := utils.ParseConceptDefsAndDichotomousDefs(c)
84-
85-
sourceId, _ := strconv.Atoi(sourceIdStr)
86-
cohortId, _ := strconv.Atoi(cohortIdStr)
87-
conceptId, _ := strconv.ParseInt(conceptIdStr, 10, 64)
75+
// parse cohortPairs separately as well, so we can validate permissions
76+
_, cohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptIdsAndCohortPairs)
8877

8978
validAccessRequest := u.teamProjectAuthz.TeamProjectValidation(c, []int{cohortId}, cohortPairs)
9079
if !validAccessRequest {
@@ -94,7 +83,7 @@ func (u CohortDataController) RetrieveStatsForCohortIdAndConceptId(c *gin.Contex
9483
return
9584
}
9685

97-
cohortData, err := u.cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsAndCohortPairs(sourceId, cohortId, conceptId, filterConceptIdsAndValues, cohortPairs)
86+
cohortData, err := u.cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId, cohortId, conceptIdsAndCohortPairs)
9887
if err != nil {
9988
c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving concept details", "error": err.Error()})
10089
c.Abort()
@@ -105,37 +94,30 @@ func (u CohortDataController) RetrieveStatsForCohortIdAndConceptId(c *gin.Contex
10594
for _, personData := range cohortData {
10695
conceptValues = append(conceptValues, float64(*personData.ConceptValueAsNumber))
10796
}
97+
conceptToStat, errGetLast := utils.CheckAndGetLastCustomConceptVariableDef(conceptIdsAndCohortPairs)
98+
if errGetLast != nil {
99+
c.JSON(http.StatusInternalServerError, gin.H{"message": "Error: last variable should be of numeric type", "error": errGetLast.Error()})
100+
c.Abort()
101+
return
102+
}
108103

109-
statsData := utils.GenerateStatsData(cohortId, conceptId, conceptValues)
104+
statsData := utils.GenerateStatsData(cohortId, conceptToStat.ConceptId, conceptValues)
110105

111106
c.JSON(http.StatusOK, gin.H{"statsData": statsData})
112107
}
113108

114109
func (u CohortDataController) RetrieveDataBySourceIdAndCohortIdAndVariables(c *gin.Context) {
115110
// TODO - add some validation to ensure that only calls from Argo are allowed through since it outputs FULL data?
116-
117-
// parse and validate all parameters:
118-
sourceIdStr := c.Param("sourceid")
119-
log.Printf("Querying source: %s", sourceIdStr)
120-
cohortIdStr := c.Param("cohortid")
121-
log.Printf("Querying cohort for cohort definition id: %s", cohortIdStr)
122-
if sourceIdStr == "" || cohortIdStr == "" {
123-
c.JSON(http.StatusBadRequest, gin.H{"message": "bad request"})
124-
c.Abort()
125-
return
126-
}
127-
128-
conceptIdsAndValues, cohortPairs, err := utils.ParseConceptDefsAndDichotomousDefs(c)
129-
conceptIds := utils.ExtractConceptIdsFromCustomConceptVariablesDef(conceptIdsAndValues)
130-
111+
// -> this concern is considered to be addressed by https://github.com/uc-cdis/cloud-automation/pull/1884
112+
sourceId, cohortId, conceptDefsAndCohortPairs, err := utils.ParseSourceIdAndCohortIdAndVariablesAsSingleList(c)
131113
if err != nil {
132-
c.JSON(http.StatusInternalServerError, gin.H{"message": "Error parsing request body for prefixed concept ids and dichotomous Ids", "error": err.Error()})
114+
c.JSON(http.StatusBadRequest, gin.H{"message": "bad request", "error": err.Error()})
133115
c.Abort()
134116
return
135117
}
136118

137-
sourceId, _ := strconv.Atoi(sourceIdStr)
138-
cohortId, _ := strconv.Atoi(cohortIdStr)
119+
// parse cohortPairs separately as well, so we can validate permissions
120+
conceptDefs, cohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptDefsAndCohortPairs)
139121

140122
validAccessRequest := u.teamProjectAuthz.TeamProjectValidation(c, []int{cohortId}, cohortPairs)
141123
if !validAccessRequest {
@@ -145,17 +127,32 @@ func (u CohortDataController) RetrieveDataBySourceIdAndCohortIdAndVariables(c *g
145127
return
146128
}
147129

148-
// call model method:
149-
cohortData, err := u.cohortDataModel.RetrieveDataBySourceIdAndCohortIdAndConceptIdsOrderedByPersonId(sourceId, cohortId, conceptIds)
150-
if err != nil {
151-
c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving concept details", "error": err.Error()})
152-
c.Abort()
153-
return
130+
// Iterate over conceptDefsAndCohortPairs and collect the concept values for each person:
131+
// {PersonId:1, ConceptId:1, ConceptValue: "A value with, comma!"},
132+
// {PersonId:1, ConceptId:2, ConceptValue: B},
133+
// {PersonId:2, ConceptId:1, ConceptValue: C},
134+
var variablesToQuery []interface{}
135+
var finalConceptDataset []*models.PersonConceptAndValue
136+
for _, item := range conceptDefsAndCohortPairs {
137+
variablesToQuery = append(variablesToQuery, item)
138+
// if item is of type CustomConceptVariableDef, get the data:
139+
if _, ok := item.(utils.CustomConceptVariableDef); ok {
140+
// use variablesToQuery to query an increasingly tight set (simulating the attrition table that generated this query)
141+
cohortData, err := u.cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId, cohortId, variablesToQuery)
142+
if err != nil {
143+
c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving concept details", "error": err.Error()})
144+
c.Abort()
145+
return
146+
}
147+
// add to final concept data set:
148+
finalConceptDataset = append(finalConceptDataset, cohortData...)
149+
}
154150
}
155151

156-
partialCSV := GeneratePartialCSV(sourceId, cohortData, conceptIds)
152+
conceptIds := utils.ExtractConceptIdsFromCustomConceptVariablesDef(conceptDefs)
153+
partialCSV := GeneratePartialCSV(sourceId, finalConceptDataset, conceptIds) // use conceptdefs to improve column description? nah...no person is reading this table....just needs to be unique
157154

158-
personIdToCSVValues, err := u.RetrievePeopleIdAndCohort(sourceId, cohortId, cohortPairs, cohortData)
155+
personIdToCSVValues, err := u.RetrievePeopleIdAndCohort(sourceId, cohortId, cohortPairs, finalConceptDataset)
159156
if err != nil {
160157
c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving people ID to csv value map", "error": err.Error()})
161158
c.Abort()
@@ -287,13 +284,12 @@ func populateConceptValue(row []string, cohortItem models.PersonConceptAndValue,
287284
func (u CohortDataController) RetrieveCohortOverlapStats(c *gin.Context) {
288285
errors := make([]error, 4)
289286
var sourceId, caseCohortId, controlCohortId int
290-
var conceptIdsAndValues []utils.CustomConceptVariableDef
291-
var cohortPairs []utils.CustomDichotomousVariableDef
287+
var conceptDefsAndCohortPairs []interface{}
292288
sourceId, errors[0] = utils.ParseNumericArg(c, "sourceid")
293289
caseCohortId, errors[1] = utils.ParseNumericArg(c, "casecohortid")
294290
controlCohortId, errors[2] = utils.ParseNumericArg(c, "controlcohortid")
295-
conceptIdsAndValues, cohortPairs, errors[3] = utils.ParseConceptDefsAndDichotomousDefs(c)
296-
conceptIds := utils.ExtractConceptIdsFromCustomConceptVariablesDef(conceptIdsAndValues)
291+
conceptDefsAndCohortPairs, errors[3] = utils.ParseConceptDefsAndDichotomousDefsAsSingleList(c)
292+
_, cohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptDefsAndCohortPairs)
297293

298294
validAccessRequest := u.teamProjectAuthz.TeamProjectValidation(c, []int{caseCohortId, controlCohortId}, cohortPairs)
299295
if !validAccessRequest {
@@ -309,7 +305,7 @@ func (u CohortDataController) RetrieveCohortOverlapStats(c *gin.Context) {
309305
return
310306
}
311307
overlapStats, err := u.cohortDataModel.RetrieveCohortOverlapStats(sourceId, caseCohortId,
312-
controlCohortId, conceptIds, cohortPairs)
308+
controlCohortId, conceptDefsAndCohortPairs)
313309
if err != nil {
314310
c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving stats", "error": err.Error()})
315311
c.Abort()

‎controllers/concept.go

+17-14
Original file line numberDiff line numberDiff line change
@@ -125,13 +125,16 @@ func (u ConceptController) RetrieveBreakdownStatsBySourceIdAndCohortId(c *gin.Co
125125
}
126126

127127
func (u ConceptController) RetrieveBreakdownStatsBySourceIdAndCohortIdAndVariables(c *gin.Context) {
128-
sourceId, cohortId, conceptIds, cohortPairs, err := utils.ParseSourceIdAndCohortIdAndVariablesList(c)
128+
sourceId, cohortId, conceptDefsAndCohortPairs, err := utils.ParseSourceIdAndCohortIdAndVariablesAsSingleList(c)
129129
if err != nil {
130-
log.Printf("Error: %s", err.Error())
131130
c.JSON(http.StatusBadRequest, gin.H{"message": "bad request", "error": err.Error()})
132131
c.Abort()
133132
return
134133
}
134+
135+
// parse cohortPairs separately as well, so we can validate permissions
136+
_, cohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptDefsAndCohortPairs)
137+
135138
validAccessRequest := u.teamProjectAuthz.TeamProjectValidation(c, []int{cohortId}, cohortPairs)
136139
if !validAccessRequest {
137140
log.Printf("Error: invalid request")
@@ -147,7 +150,7 @@ func (u ConceptController) RetrieveBreakdownStatsBySourceIdAndCohortIdAndVariabl
147150
c.Abort()
148151
return
149152
}
150-
breakdownStats, err := u.conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(sourceId, cohortId, conceptIds, cohortPairs, breakdownConceptId)
153+
breakdownStats, err := u.conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId, cohortId, conceptDefsAndCohortPairs, breakdownConceptId)
151154
if err != nil {
152155
log.Printf("Error: %s", err.Error())
153156
c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving stats", "error": err.Error()})
@@ -190,14 +193,14 @@ func generateRowForVariable(variableName string, breakdownConceptValuesToPeopleC
190193
}
191194

192195
func (u ConceptController) RetrieveAttritionTable(c *gin.Context) {
193-
sourceId, cohortId, conceptIdsAndCohortPairs, err := utils.ParseSourceIdAndCohortIdAndVariablesAsSingleList(c)
196+
sourceId, cohortId, conceptDefsAndCohortPairs, err := utils.ParseSourceIdAndCohortIdAndVariablesAsSingleList(c)
194197
if err != nil {
195198
log.Printf("Error: %s", err.Error())
196199
c.JSON(http.StatusBadRequest, gin.H{"message": "bad request", "error": err.Error()})
197200
c.Abort()
198201
return
199202
}
200-
_, cohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptIdsAndCohortPairs)
203+
_, cohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptDefsAndCohortPairs)
201204
validAccessRequest := u.teamProjectAuthz.TeamProjectValidation(c, []int{cohortId}, cohortPairs)
202205
if !validAccessRequest {
203206
log.Printf("Error: invalid request")
@@ -238,7 +241,7 @@ func (u ConceptController) RetrieveAttritionTable(c *gin.Context) {
238241
c.Abort()
239242
return
240243
}
241-
otherAttritionRows, err := u.GetAttritionRowForConceptIdsAndCohortPairs(sourceId, cohortId, conceptIdsAndCohortPairs, breakdownConceptId, sortedConceptValues)
244+
otherAttritionRows, err := u.GetAttritionRowForConceptDefsAndCohortPairs(sourceId, cohortId, conceptDefsAndCohortPairs, breakdownConceptId, sortedConceptValues)
242245
if err != nil {
243246
log.Printf("Error: %s", err.Error())
244247
c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving concept breakdown rows for filter conceptIds and cohortPairs", "error": err.Error()})
@@ -249,13 +252,13 @@ func (u ConceptController) RetrieveAttritionTable(c *gin.Context) {
249252
c.String(http.StatusOK, b.String())
250253
}
251254

252-
func (u ConceptController) GetAttritionRowForConceptIdsAndCohortPairs(sourceId int, cohortId int, conceptIdsAndCohortPairs []interface{}, breakdownConceptId int64, sortedConceptValues []string) ([][]string, error) {
255+
func (u ConceptController) GetAttritionRowForConceptDefsAndCohortPairs(sourceId int, cohortId int, conceptDefsAndCohortPairs []interface{}, breakdownConceptId int64, sortedConceptValues []string) ([][]string, error) {
253256
var otherAttritionRows [][]string
254-
for idx, conceptIdOrCohortPair := range conceptIdsAndCohortPairs {
255-
// attrition filter: run each query with an increasingly longer list of filterConceptIdsAndCohortPairs, until the last query is run with them all:
256-
filterConceptIdsAndCohortPairs := conceptIdsAndCohortPairs[0 : idx+1]
257+
for idx, conceptIdOrCohortPair := range conceptDefsAndCohortPairs {
258+
// attrition filter: run each query with an increasingly longer list of filterConceptDefsAndCohortPairs, until the last query is run with them all:
259+
filterConceptDefsAndCohortPairs := conceptDefsAndCohortPairs[0 : idx+1]
257260

258-
attritionRow, err := u.GetAttritionRowForConceptIdOrCohortPair(sourceId, cohortId, conceptIdOrCohortPair, filterConceptIdsAndCohortPairs, breakdownConceptId, sortedConceptValues)
261+
attritionRow, err := u.GetAttritionRowForConceptDefOrCohortPair(sourceId, cohortId, conceptIdOrCohortPair, filterConceptDefsAndCohortPairs, breakdownConceptId, sortedConceptValues)
259262
if err != nil {
260263
log.Printf("Error: %s", err.Error())
261264
return nil, err
@@ -265,10 +268,10 @@ func (u ConceptController) GetAttritionRowForConceptIdsAndCohortPairs(sourceId i
265268
return otherAttritionRows, nil
266269
}
267270

268-
func (u ConceptController) GetAttritionRowForConceptIdOrCohortPair(sourceId int, cohortId int, conceptIdOrCohortPair interface{}, filterConceptIdsAndCohortPairs []interface{}, breakdownConceptId int64, sortedConceptValues []string) ([]string, error) {
269-
filterConceptDefsAndValues, filterCohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(filterConceptIdsAndCohortPairs)
271+
func (u ConceptController) GetAttritionRowForConceptDefOrCohortPair(sourceId int, cohortId int, conceptIdOrCohortPair interface{}, filterConceptDefsAndCohortPairs []interface{}, breakdownConceptId int64, sortedConceptValues []string) ([]string, error) {
272+
filterConceptDefsAndValues, filterCohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(filterConceptDefsAndCohortPairs)
270273
filterConceptIds := utils.ExtractConceptIdsFromCustomConceptVariablesDef(filterConceptDefsAndValues)
271-
breakdownStats, err := u.conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(sourceId, cohortId, filterConceptIds, filterCohortPairs, breakdownConceptId)
274+
breakdownStats, err := u.conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId, cohortId, filterConceptDefsAndCohortPairs, breakdownConceptId)
272275
if err != nil {
273276
return nil, fmt.Errorf("could not retrieve concept Breakdown for concepts %v dichotomous variables %v due to error: %s", filterConceptIds, filterCohortPairs, err.Error())
274277
}

0 commit comments

Comments
 (0)