Skip to content

Commit 4826d19

Browse files
pieterlukassepieterlukasse
committed
feat: add filter and transformation support to breakdown / attrition endpoint
1 parent 924ebe3 commit 4826d19

File tree

8 files changed

+140
-65
lines changed

8 files changed

+140
-65
lines changed

‎controllers/cohortdata.go

+1-5
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")

‎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
}

‎models/cohortdata.go

+6-7
Original file line numberDiff line numberDiff line change
@@ -104,25 +104,24 @@ func (h CohortData) RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlus
104104
omopDataSource := dataSourceModel.GetDataSource(sourceId, Omop)
105105
resultsDataSource := dataSourceModel.GetDataSource(sourceId, Results)
106106
finalCohortAlias := "final_cohort_alias"
107-
histogramConcept, err := utils.CheckAndGetLastCustomConceptVariableDef(filterConceptDefsAndCohortPairs)
108-
if err != nil {
109-
log.Fatalf("failed: %v", err)
110-
return nil, err
111-
}
112107
// get the observations for the subjects and the concepts, to build up the data rows to return:
113108
var cohortData []*PersonConceptAndValue
114109
session := resultsDataSource.Db.Session(&gorm.Session{})
115-
err = session.Transaction(func(query *gorm.DB) error { // TODO - rename query?
110+
err := session.Transaction(func(query *gorm.DB) error { // TODO - rename query?
116111
query, tmpTableName := QueryFilterByConceptDefsPlusCohortPairsHelper(query, sourceId, cohortDefinitionId, filterConceptDefsAndCohortPairs, omopDataSource, resultsDataSource, finalCohortAlias)
117112
if tmpTableName != "" {
118113
query = query.Select("distinct(" + tmpTableName + ".person_id), " + tmpTableName + ".observation_concept_id as concept_id, " + tmpTableName + ".value_as_number as concept_value_as_number")
119114
} else {
115+
histogramConcept, errGetLast := utils.CheckAndGetLastCustomConceptVariableDef(filterConceptDefsAndCohortPairs)
116+
if errGetLast != nil {
117+
log.Fatalf("failed: %v", errGetLast)
118+
return errGetLast
119+
}
120120
query = query.Select("distinct(observation.person_id), observation.observation_concept_id as concept_id, observation.value_as_number as concept_value_as_number").
121121
Joins("INNER JOIN "+omopDataSource.Schema+".observation_continuous as observation"+omopDataSource.GetViewDirective()+" ON "+finalCohortAlias+".subject_id = observation.person_id").
122122
Where("observation.observation_concept_id = ?", histogramConcept.ConceptId).
123123
Where("observation.value_as_number is not null")
124124
}
125-
126125
query, cancel := utils.AddTimeoutToQuery(query)
127126
defer cancel()
128127
meta_result := query.Scan(&cohortData)

‎models/concept.go

+45-3
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"fmt"
55

66
"github.com/uc-cdis/cohort-middleware/utils"
7+
"gorm.io/gorm"
78
)
89

910
type ConceptI interface {
@@ -13,6 +14,7 @@ type ConceptI interface {
1314
RetrieveInfoBySourceIdAndConceptTypes(sourceId int, conceptTypes []string) ([]*ConceptSimple, error)
1415
RetrieveBreakdownStatsBySourceIdAndCohortId(sourceId int, cohortDefinitionId int, breakdownConceptId int64) ([]*ConceptBreakdown, error)
1516
RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(sourceId int, cohortDefinitionId int, filterConceptIds []int64, filterCohortPairs []utils.CustomDichotomousVariableDef, breakdownConceptId int64) ([]*ConceptBreakdown, error)
17+
RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId int, cohortDefinitionId int, filterConceptDefsAndCohortPairs []interface{}, breakdownConceptId int64) ([]*ConceptBreakdown, error)
1618
}
1719
type Concept struct {
1820
ConceptId int64 `json:"concept_id"`
@@ -127,11 +129,11 @@ func (h Concept) RetrieveInfoBySourceIdAndConceptTypes(sourceId int, conceptType
127129
// {ConceptValue: "B", NPersonsInCohortWithValue: N-M},
128130
func (h Concept) RetrieveBreakdownStatsBySourceIdAndCohortId(sourceId int, cohortDefinitionId int, breakdownConceptId int64) ([]*ConceptBreakdown, error) {
129131
// this is identical to the result of the function below if called with empty filterConceptIds[] and empty filterCohortPairs... so call that:
130-
filterConceptIds := []int64{}
131-
filterCohortPairs := []utils.CustomDichotomousVariableDef{}
132-
return h.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(sourceId, cohortDefinitionId, filterConceptIds, filterCohortPairs, breakdownConceptId)
132+
var filterConceptDefsAndCohortPairs []interface{}
133+
return h.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId, cohortDefinitionId, filterConceptDefsAndCohortPairs, breakdownConceptId)
133134
}
134135

136+
// DEPRECATED - use RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs instead.
135137
// Basically same goal as described in function above, but only count persons that have a non-null value for each
136138
// of the ids in the given filterConceptIds. So, using the example documented in the function above, it will
137139
// return something like:
@@ -172,3 +174,43 @@ func (h Concept) RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCoho
172174
}
173175
return conceptBreakdownList, meta_result.Error
174176
}
177+
178+
// Same as RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs above, but
179+
// applies transformations and filtering, if these items are specified for the given concept definitions.
180+
func (h Concept) RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId int, cohortDefinitionId int, filterConceptDefsAndCohortPairs []interface{}, breakdownConceptId int64) ([]*ConceptBreakdown, error) {
181+
var dataSourceModel = new(Source)
182+
omopDataSource := dataSourceModel.GetDataSource(sourceId, Omop)
183+
resultsDataSource := dataSourceModel.GetDataSource(sourceId, Results)
184+
finalSetAlias := "final_set_alias"
185+
var conceptBreakdownList []*ConceptBreakdown
186+
session := resultsDataSource.Db.Session(&gorm.Session{})
187+
err := session.Transaction(func(query *gorm.DB) error {
188+
query, tmpTableName := QueryFilterByConceptDefsPlusCohortPairsHelper(query, sourceId, cohortDefinitionId, filterConceptDefsAndCohortPairs, omopDataSource, resultsDataSource, finalSetAlias)
189+
// count persons, grouping by concept value:
190+
if tmpTableName != "" {
191+
query = query.Select(tmpTableName + ".value_as_concept_id), count(distinct(" + tmpTableName + ".person_id)) as npersons_in_cohort_with_value").
192+
Group(tmpTableName + ".value_as_concept_id")
193+
} else {
194+
query = query.Select("observation.value_as_concept_id, count(distinct(observation.person_id)) as npersons_in_cohort_with_value").
195+
Joins("INNER JOIN "+omopDataSource.Schema+".observation_continuous as observation"+omopDataSource.GetViewDirective()+" ON "+finalSetAlias+".subject_id = observation.person_id").
196+
Where("observation.observation_concept_id = ?", breakdownConceptId).
197+
Where(GetConceptValueNotNullCheckBasedOnConceptType("observation", sourceId, breakdownConceptId)).
198+
Group("observation.value_as_concept_id")
199+
}
200+
query, cancel := utils.AddTimeoutToQuery(query)
201+
defer cancel()
202+
meta_result := query.Scan(&conceptBreakdownList)
203+
return meta_result.Error
204+
})
205+
206+
// Add concept value (coded value) and concept name for each of the value_as_concept_id values:
207+
for _, conceptBreakdownItem := range conceptBreakdownList {
208+
conceptInfo, error := h.RetrieveInfoBySourceIdAndConceptId(sourceId, conceptBreakdownItem.ValueAsConceptId)
209+
if error != nil {
210+
return nil, error
211+
}
212+
conceptBreakdownItem.ConceptValue = conceptInfo.ConceptCode
213+
conceptBreakdownItem.ValueName = conceptInfo.ConceptName
214+
}
215+
return conceptBreakdownList, err
216+
}

‎models/helper.go

+1
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ func QueryFilterByConceptDefHelper(query *gorm.DB, sourceId int, filterConceptDe
8282
omopDataSource, personIdFieldForObservationJoin, "observation_continuous", observationAliasSimpleQuery)
8383
tmpTransformedTable, err := TransformDataIntoTempTable(omopDataSource, query, filterConceptDef)
8484
// TODO - the resulting query should actually be Select * from temptable.... as this collapses all underlying queries.
85+
// Apply any filtering AFTER the transformation:
8586
query = queryJoinAndFilterByConceptDef(query, sourceId, filterConceptDef,
8687
omopDataSource, personIdFieldForObservationJoin, tmpTransformedTable, observationAliasFullQuery)
8788
return query, observationAliasFullQuery, err

‎tests/controllers_tests/controllers_test.go

+24-6
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,18 @@ func (h dummyConceptDataModel) RetrieveBreakdownStatsBySourceIdAndCohortIdAndCon
284284
}
285285
return conceptBreakdown, nil
286286
}
287+
func (h dummyConceptDataModel) RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId int, cohortDefinitionId int, filterConceptDefsAndCohortPairs []interface{}, breakdownConceptId int64) ([]*models.ConceptBreakdown, error) {
288+
filterConceptDefs, filterCohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(filterConceptDefsAndCohortPairs)
289+
290+
conceptBreakdown := []*models.ConceptBreakdown{
291+
{ConceptValue: "value1", NpersonsInCohortWithValue: 4 - len(filterCohortPairs)}, // simulate decreasing numbers as filter increases - the use of filterCohortPairs instead of filterConceptIds is otherwise meaningless here...
292+
{ConceptValue: "value2", NpersonsInCohortWithValue: 7 - len(filterConceptDefs)}, // simulate decreasing numbers as filter increases- the use of filterConceptIds instead of filterCohortPairs is otherwise meaningless here...
293+
}
294+
if dummyModelReturnError {
295+
return nil, fmt.Errorf("error!")
296+
}
297+
return conceptBreakdown, nil
298+
}
287299

288300
type dummyDataDictionaryModel struct{}
289301

@@ -742,11 +754,17 @@ func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndVariablesModelError(t *te
742754
requestContext.Params = append(requestContext.Params, gin.Param{Key: "cohortid", Value: "1"})
743755
requestContext.Params = append(requestContext.Params, gin.Param{Key: "breakdownconceptid", Value: "1"})
744756
requestContext.Request = new(http.Request)
745-
requestContext.Request.Body = io.NopCloser(strings.NewReader("{\"ConceptIds\":[1234,5678]}"))
757+
requestContext.Request.Body = io.NopCloser(strings.NewReader("{\"variables\":[]}"))
746758
requestContext.Writer = new(tests.CustomResponseWriter)
747759
// set flag to let mock model layer return error instead of mock data:
748760
dummyModelReturnError = true
749761
conceptController.RetrieveBreakdownStatsBySourceIdAndCohortIdAndVariables(requestContext)
762+
763+
// Expected response status is "500":
764+
if requestContext.Writer.Status() != 500 {
765+
t.Errorf("Expected status to start with '500', got '%d'", requestContext.Writer.Status())
766+
}
767+
750768
if !requestContext.IsAborted() {
751769
t.Errorf("Expected aborted request")
752770
}
@@ -903,15 +921,15 @@ func TestGenerateHeaderAndNonFilterRow(t *testing.T) {
903921
}
904922
}
905923

906-
func TestGetAttritionRowForConceptIdsAndCohortPairs(t *testing.T) {
924+
func TestGetAttritionRowForConceptDefsAndCohortPairs(t *testing.T) {
907925
setUp(t)
908926
sourceId := 1
909927
cohortId := 1
910928
var breakdownConceptId int64 = 1
911929
sortedConceptValues := []string{"value1", "value2", "value3"}
912930

913931
// mix of concept ids and CustomDichotomousVariableDef items:
914-
conceptIdsAndCohortPairs := []interface{}{
932+
conceptDefsAndCohortPairs := []interface{}{
915933
utils.CustomConceptVariableDef{ConceptId: int64(1234)},
916934
utils.CustomConceptVariableDef{ConceptId: int64(5678)},
917935
utils.CustomDichotomousVariableDef{
@@ -925,10 +943,10 @@ func TestGetAttritionRowForConceptIdsAndCohortPairs(t *testing.T) {
925943
ProvidedName: "testB34"},
926944
}
927945

928-
result, _ := conceptController.GetAttritionRowForConceptIdsAndCohortPairs(sourceId, cohortId, conceptIdsAndCohortPairs, breakdownConceptId, sortedConceptValues)
929-
if len(result) != len(conceptIdsAndCohortPairs) {
946+
result, _ := conceptController.GetAttritionRowForConceptDefsAndCohortPairs(sourceId, cohortId, conceptDefsAndCohortPairs, breakdownConceptId, sortedConceptValues)
947+
if len(result) != len(conceptDefsAndCohortPairs) {
930948
t.Errorf("Expected %d data lines, found %d lines in total",
931-
len(conceptIdsAndCohortPairs),
949+
len(conceptDefsAndCohortPairs),
932950
len(result))
933951
t.Errorf("Lines: %s", result)
934952
}

0 commit comments

Comments
 (0)