From 59fb8c8b7974bfe89124e2f6aa82136f7c38fc6e Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Tue, 28 Jan 2025 20:50:55 +0100 Subject: [PATCH] feat: add filter and transformation support to breakdown / attrition endpoint --- controllers/cohortdata.go | 6 +- controllers/concept.go | 31 ++++----- models/cohortdata.go | 13 ++-- models/concept.go | 48 +++++++++++++- models/helper.go | 1 + tests/controllers_tests/controllers_test.go | 32 +++++++--- tests/models_tests/models_test.go | 70 ++++++++++++--------- utils/parsing.go | 6 +- 8 files changed, 141 insertions(+), 66 deletions(-) diff --git a/controllers/cohortdata.go b/controllers/cohortdata.go index 0f00368..f122d9b 100644 --- a/controllers/cohortdata.go +++ b/controllers/cohortdata.go @@ -38,11 +38,7 @@ func (u CohortDataController) RetrieveHistogramForCohortIdAndConceptId(c *gin.Co // parse cohortPairs separately as well, so we can validate permissions _, cohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptIdsAndCohortPairs) - if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"message": "Error parsing request body for prefixed concept ids", "error": err.Error()}) - c.Abort() - return - } + validAccessRequest := u.teamProjectAuthz.TeamProjectValidation(c, []int{cohortId}, cohortPairs) if !validAccessRequest { log.Printf("Error: invalid request") diff --git a/controllers/concept.go b/controllers/concept.go index 7e3e149..7a14798 100644 --- a/controllers/concept.go +++ b/controllers/concept.go @@ -125,13 +125,16 @@ func (u ConceptController) RetrieveBreakdownStatsBySourceIdAndCohortId(c *gin.Co } func (u ConceptController) RetrieveBreakdownStatsBySourceIdAndCohortIdAndVariables(c *gin.Context) { - sourceId, cohortId, conceptIds, cohortPairs, err := utils.ParseSourceIdAndCohortIdAndVariablesList(c) + sourceId, cohortId, conceptDefsAndCohortPairs, err := utils.ParseSourceIdAndCohortIdAndVariablesAsSingleList(c) if err != nil { - log.Printf("Error: %s", err.Error()) c.JSON(http.StatusBadRequest, gin.H{"message": "bad request", "error": err.Error()}) c.Abort() return } + + // parse cohortPairs separately as well, so we can validate permissions + _, cohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptDefsAndCohortPairs) + validAccessRequest := u.teamProjectAuthz.TeamProjectValidation(c, []int{cohortId}, cohortPairs) if !validAccessRequest { log.Printf("Error: invalid request") @@ -147,7 +150,7 @@ func (u ConceptController) RetrieveBreakdownStatsBySourceIdAndCohortIdAndVariabl c.Abort() return } - breakdownStats, err := u.conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(sourceId, cohortId, conceptIds, cohortPairs, breakdownConceptId) + breakdownStats, err := u.conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId, cohortId, conceptDefsAndCohortPairs, breakdownConceptId) if err != nil { log.Printf("Error: %s", err.Error()) c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving stats", "error": err.Error()}) @@ -190,14 +193,14 @@ func generateRowForVariable(variableName string, breakdownConceptValuesToPeopleC } func (u ConceptController) RetrieveAttritionTable(c *gin.Context) { - sourceId, cohortId, conceptIdsAndCohortPairs, err := utils.ParseSourceIdAndCohortIdAndVariablesAsSingleList(c) + sourceId, cohortId, conceptDefsAndCohortPairs, err := utils.ParseSourceIdAndCohortIdAndVariablesAsSingleList(c) if err != nil { log.Printf("Error: %s", err.Error()) c.JSON(http.StatusBadRequest, gin.H{"message": "bad request", "error": err.Error()}) c.Abort() return } - _, cohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptIdsAndCohortPairs) + _, cohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptDefsAndCohortPairs) validAccessRequest := u.teamProjectAuthz.TeamProjectValidation(c, []int{cohortId}, cohortPairs) if !validAccessRequest { log.Printf("Error: invalid request") @@ -238,7 +241,7 @@ func (u ConceptController) RetrieveAttritionTable(c *gin.Context) { c.Abort() return } - otherAttritionRows, err := u.GetAttritionRowForConceptIdsAndCohortPairs(sourceId, cohortId, conceptIdsAndCohortPairs, breakdownConceptId, sortedConceptValues) + otherAttritionRows, err := u.GetAttritionRowForConceptDefsAndCohortPairs(sourceId, cohortId, conceptDefsAndCohortPairs, breakdownConceptId, sortedConceptValues) if err != nil { log.Printf("Error: %s", err.Error()) 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) { c.String(http.StatusOK, b.String()) } -func (u ConceptController) GetAttritionRowForConceptIdsAndCohortPairs(sourceId int, cohortId int, conceptIdsAndCohortPairs []interface{}, breakdownConceptId int64, sortedConceptValues []string) ([][]string, error) { +func (u ConceptController) GetAttritionRowForConceptDefsAndCohortPairs(sourceId int, cohortId int, conceptDefsAndCohortPairs []interface{}, breakdownConceptId int64, sortedConceptValues []string) ([][]string, error) { var otherAttritionRows [][]string - for idx, conceptIdOrCohortPair := range conceptIdsAndCohortPairs { - // attrition filter: run each query with an increasingly longer list of filterConceptIdsAndCohortPairs, until the last query is run with them all: - filterConceptIdsAndCohortPairs := conceptIdsAndCohortPairs[0 : idx+1] + for idx, conceptIdOrCohortPair := range conceptDefsAndCohortPairs { + // attrition filter: run each query with an increasingly longer list of filterConceptDefsAndCohortPairs, until the last query is run with them all: + filterConceptDefsAndCohortPairs := conceptDefsAndCohortPairs[0 : idx+1] - attritionRow, err := u.GetAttritionRowForConceptIdOrCohortPair(sourceId, cohortId, conceptIdOrCohortPair, filterConceptIdsAndCohortPairs, breakdownConceptId, sortedConceptValues) + attritionRow, err := u.GetAttritionRowForConceptDefOrCohortPair(sourceId, cohortId, conceptIdOrCohortPair, filterConceptDefsAndCohortPairs, breakdownConceptId, sortedConceptValues) if err != nil { log.Printf("Error: %s", err.Error()) return nil, err @@ -265,10 +268,10 @@ func (u ConceptController) GetAttritionRowForConceptIdsAndCohortPairs(sourceId i return otherAttritionRows, nil } -func (u ConceptController) GetAttritionRowForConceptIdOrCohortPair(sourceId int, cohortId int, conceptIdOrCohortPair interface{}, filterConceptIdsAndCohortPairs []interface{}, breakdownConceptId int64, sortedConceptValues []string) ([]string, error) { - filterConceptDefsAndValues, filterCohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(filterConceptIdsAndCohortPairs) +func (u ConceptController) GetAttritionRowForConceptDefOrCohortPair(sourceId int, cohortId int, conceptIdOrCohortPair interface{}, filterConceptDefsAndCohortPairs []interface{}, breakdownConceptId int64, sortedConceptValues []string) ([]string, error) { + filterConceptDefsAndValues, filterCohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(filterConceptDefsAndCohortPairs) filterConceptIds := utils.ExtractConceptIdsFromCustomConceptVariablesDef(filterConceptDefsAndValues) - breakdownStats, err := u.conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(sourceId, cohortId, filterConceptIds, filterCohortPairs, breakdownConceptId) + breakdownStats, err := u.conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId, cohortId, filterConceptDefsAndCohortPairs, breakdownConceptId) if err != nil { return nil, fmt.Errorf("could not retrieve concept Breakdown for concepts %v dichotomous variables %v due to error: %s", filterConceptIds, filterCohortPairs, err.Error()) } diff --git a/models/cohortdata.go b/models/cohortdata.go index e5c9ff7..92cd534 100644 --- a/models/cohortdata.go +++ b/models/cohortdata.go @@ -104,25 +104,24 @@ func (h CohortData) RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlus omopDataSource := dataSourceModel.GetDataSource(sourceId, Omop) resultsDataSource := dataSourceModel.GetDataSource(sourceId, Results) finalSetAlias := "final_set_alias" - histogramConcept, err := utils.CheckAndGetLastCustomConceptVariableDef(filterConceptDefsAndCohortPairs) - if err != nil { - log.Fatalf("failed: %v", err) - return nil, err - } // get the observations for the subjects and the concepts, to build up the data rows to return: var cohortData []*PersonConceptAndValue session := resultsDataSource.Db.Session(&gorm.Session{}) - err = session.Transaction(func(query *gorm.DB) error { // TODO - rename query? + err := session.Transaction(func(query *gorm.DB) error { // TODO - rename query? query, tmpTableName := QueryFilterByConceptDefsPlusCohortPairsHelper(query, sourceId, cohortDefinitionId, filterConceptDefsAndCohortPairs, omopDataSource, resultsDataSource, finalSetAlias) if tmpTableName != "" { query = query.Select("distinct(" + tmpTableName + ".person_id), " + tmpTableName + ".observation_concept_id as concept_id, " + tmpTableName + ".value_as_number as concept_value_as_number") } else { + histogramConcept, errGetLast := utils.CheckAndGetLastCustomConceptVariableDef(filterConceptDefsAndCohortPairs) + if errGetLast != nil { + log.Fatalf("failed: %v", errGetLast) + return errGetLast + } query = query.Select("distinct(observation.person_id), observation.observation_concept_id as concept_id, observation.value_as_number as concept_value_as_number"). Joins("INNER JOIN "+omopDataSource.Schema+".observation_continuous as observation"+omopDataSource.GetViewDirective()+" ON "+finalSetAlias+".subject_id = observation.person_id"). Where("observation.observation_concept_id = ?", histogramConcept.ConceptId). Where("observation.value_as_number is not null") } - query, cancel := utils.AddTimeoutToQuery(query) defer cancel() meta_result := query.Scan(&cohortData) diff --git a/models/concept.go b/models/concept.go index 33f870d..9ad11d2 100644 --- a/models/concept.go +++ b/models/concept.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/uc-cdis/cohort-middleware/utils" + "gorm.io/gorm" ) type ConceptI interface { @@ -13,6 +14,7 @@ type ConceptI interface { RetrieveInfoBySourceIdAndConceptTypes(sourceId int, conceptTypes []string) ([]*ConceptSimple, error) RetrieveBreakdownStatsBySourceIdAndCohortId(sourceId int, cohortDefinitionId int, breakdownConceptId int64) ([]*ConceptBreakdown, error) RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(sourceId int, cohortDefinitionId int, filterConceptIds []int64, filterCohortPairs []utils.CustomDichotomousVariableDef, breakdownConceptId int64) ([]*ConceptBreakdown, error) + RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId int, cohortDefinitionId int, filterConceptDefsAndCohortPairs []interface{}, breakdownConceptId int64) ([]*ConceptBreakdown, error) } type Concept struct { ConceptId int64 `json:"concept_id"` @@ -127,11 +129,11 @@ func (h Concept) RetrieveInfoBySourceIdAndConceptTypes(sourceId int, conceptType // {ConceptValue: "B", NPersonsInCohortWithValue: N-M}, func (h Concept) RetrieveBreakdownStatsBySourceIdAndCohortId(sourceId int, cohortDefinitionId int, breakdownConceptId int64) ([]*ConceptBreakdown, error) { // this is identical to the result of the function below if called with empty filterConceptIds[] and empty filterCohortPairs... so call that: - filterConceptIds := []int64{} - filterCohortPairs := []utils.CustomDichotomousVariableDef{} - return h.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(sourceId, cohortDefinitionId, filterConceptIds, filterCohortPairs, breakdownConceptId) + var filterConceptDefsAndCohortPairs []interface{} + return h.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId, cohortDefinitionId, filterConceptDefsAndCohortPairs, breakdownConceptId) } +// DEPRECATED - use RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs instead. // Basically same goal as described in function above, but only count persons that have a non-null value for each // of the ids in the given filterConceptIds. So, using the example documented in the function above, it will // return something like: @@ -172,3 +174,43 @@ func (h Concept) RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCoho } return conceptBreakdownList, meta_result.Error } + +// Same as RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs above, but +// applies transformations and filtering, if these items are specified for the given concept definitions. +func (h Concept) RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId int, cohortDefinitionId int, filterConceptDefsAndCohortPairs []interface{}, breakdownConceptId int64) ([]*ConceptBreakdown, error) { + var dataSourceModel = new(Source) + omopDataSource := dataSourceModel.GetDataSource(sourceId, Omop) + resultsDataSource := dataSourceModel.GetDataSource(sourceId, Results) + finalSetAlias := "final_set_alias" + var conceptBreakdownList []*ConceptBreakdown + session := resultsDataSource.Db.Session(&gorm.Session{}) + err := session.Transaction(func(query *gorm.DB) error { + query, tmpTableName := QueryFilterByConceptDefsPlusCohortPairsHelper(query, sourceId, cohortDefinitionId, filterConceptDefsAndCohortPairs, omopDataSource, resultsDataSource, finalSetAlias) + // count persons, grouping by concept value: + if tmpTableName != "" { + query = query.Select(tmpTableName + ".value_as_concept_id), count(distinct(" + tmpTableName + ".person_id)) as npersons_in_cohort_with_value"). + Group(tmpTableName + ".value_as_concept_id") + } else { + query = query.Select("observation.value_as_concept_id, count(distinct(observation.person_id)) as npersons_in_cohort_with_value"). + Joins("INNER JOIN "+omopDataSource.Schema+".observation_continuous as observation"+omopDataSource.GetViewDirective()+" ON "+finalSetAlias+".subject_id = observation.person_id"). + Where("observation.observation_concept_id = ?", breakdownConceptId). + Where(GetConceptValueNotNullCheckBasedOnConceptType("observation", sourceId, breakdownConceptId)). + Group("observation.value_as_concept_id") + } + query, cancel := utils.AddTimeoutToQuery(query) + defer cancel() + meta_result := query.Scan(&conceptBreakdownList) + return meta_result.Error + }) + + // Add concept value (coded value) and concept name for each of the value_as_concept_id values: + for _, conceptBreakdownItem := range conceptBreakdownList { + conceptInfo, error := h.RetrieveInfoBySourceIdAndConceptId(sourceId, conceptBreakdownItem.ValueAsConceptId) + if error != nil { + return nil, error + } + conceptBreakdownItem.ConceptValue = conceptInfo.ConceptCode + conceptBreakdownItem.ValueName = conceptInfo.ConceptName + } + return conceptBreakdownList, err +} diff --git a/models/helper.go b/models/helper.go index de30ae1..76feb47 100644 --- a/models/helper.go +++ b/models/helper.go @@ -74,6 +74,7 @@ func QueryFilterByConceptDefHelper(query *gorm.DB, sourceId int, filterConceptDe omopDataSource, personIdFieldForObservationJoin, "observation_continuous", observationTableAlias+"_a") tmpTransformedTable, err := TransformDataIntoTempTable(omopDataSource, query, filterConceptDef) // TODO - the resulting query should actually be Select * from temptable.... as this collapses all underlying queries. + // Apply any filtering AFTER the transformation: query = queryJoinAndFilterByConceptDef(query, sourceId, filterConceptDef, omopDataSource, personIdFieldForObservationJoin, tmpTransformedTable, observationTableAlias+"_b") return query, observationTableAlias + "_b", err diff --git a/tests/controllers_tests/controllers_test.go b/tests/controllers_tests/controllers_test.go index 15f5692..091d608 100644 --- a/tests/controllers_tests/controllers_test.go +++ b/tests/controllers_tests/controllers_test.go @@ -284,6 +284,18 @@ func (h dummyConceptDataModel) RetrieveBreakdownStatsBySourceIdAndCohortIdAndCon } return conceptBreakdown, nil } +func (h dummyConceptDataModel) RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId int, cohortDefinitionId int, filterConceptDefsAndCohortPairs []interface{}, breakdownConceptId int64) ([]*models.ConceptBreakdown, error) { + filterConceptDefs, filterCohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(filterConceptDefsAndCohortPairs) + + conceptBreakdown := []*models.ConceptBreakdown{ + {ConceptValue: "value1", NpersonsInCohortWithValue: 4 - len(filterCohortPairs)}, // simulate decreasing numbers as filter increases - the use of filterCohortPairs instead of filterConceptIds is otherwise meaningless here... + {ConceptValue: "value2", NpersonsInCohortWithValue: 7 - len(filterConceptDefs)}, // simulate decreasing numbers as filter increases- the use of filterConceptIds instead of filterCohortPairs is otherwise meaningless here... + } + if dummyModelReturnError { + return nil, fmt.Errorf("error!") + } + return conceptBreakdown, nil +} type dummyDataDictionaryModel struct{} @@ -317,7 +329,7 @@ func TestRetrieveHistogramForCohortIdAndConceptIdWithWrongParams(t *testing.T) { setUp(t) requestContext := new(gin.Context) requestContext.Params = append(requestContext.Params, gin.Param{Key: "sourceid", Value: strconv.Itoa(tests.GetTestSourceId())}) - requestContext.Params = append(requestContext.Params, gin.Param{Key: "cohortid", Value: "4"}) + requestContext.Params = append(requestContext.Params, gin.Param{Key: "cohortidwrong", Value: "4"}) requestContext.Writer = new(tests.CustomResponseWriter) requestContext.Request = new(http.Request) requestBody := "{\"variables\":[{\"variable_type\": \"custom_dichotomous\", \"cohort_ids\": [1, 3]}]}" @@ -742,11 +754,17 @@ func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndVariablesModelError(t *te requestContext.Params = append(requestContext.Params, gin.Param{Key: "cohortid", Value: "1"}) requestContext.Params = append(requestContext.Params, gin.Param{Key: "breakdownconceptid", Value: "1"}) requestContext.Request = new(http.Request) - requestContext.Request.Body = io.NopCloser(strings.NewReader("{\"ConceptIds\":[1234,5678]}")) + requestContext.Request.Body = io.NopCloser(strings.NewReader("{\"variables\":[]}")) requestContext.Writer = new(tests.CustomResponseWriter) // set flag to let mock model layer return error instead of mock data: dummyModelReturnError = true conceptController.RetrieveBreakdownStatsBySourceIdAndCohortIdAndVariables(requestContext) + + // Expected response status is "500": + if requestContext.Writer.Status() != 500 { + t.Errorf("Expected status to start with '500', got '%d'", requestContext.Writer.Status()) + } + if !requestContext.IsAborted() { t.Errorf("Expected aborted request") } @@ -903,7 +921,7 @@ func TestGenerateHeaderAndNonFilterRow(t *testing.T) { } } -func TestGetAttritionRowForConceptIdsAndCohortPairs(t *testing.T) { +func TestGetAttritionRowForConceptDefsAndCohortPairs(t *testing.T) { setUp(t) sourceId := 1 cohortId := 1 @@ -911,7 +929,7 @@ func TestGetAttritionRowForConceptIdsAndCohortPairs(t *testing.T) { sortedConceptValues := []string{"value1", "value2", "value3"} // mix of concept ids and CustomDichotomousVariableDef items: - conceptIdsAndCohortPairs := []interface{}{ + conceptDefsAndCohortPairs := []interface{}{ utils.CustomConceptVariableDef{ConceptId: int64(1234)}, utils.CustomConceptVariableDef{ConceptId: int64(5678)}, utils.CustomDichotomousVariableDef{ @@ -925,10 +943,10 @@ func TestGetAttritionRowForConceptIdsAndCohortPairs(t *testing.T) { ProvidedName: "testB34"}, } - result, _ := conceptController.GetAttritionRowForConceptIdsAndCohortPairs(sourceId, cohortId, conceptIdsAndCohortPairs, breakdownConceptId, sortedConceptValues) - if len(result) != len(conceptIdsAndCohortPairs) { + result, _ := conceptController.GetAttritionRowForConceptDefsAndCohortPairs(sourceId, cohortId, conceptDefsAndCohortPairs, breakdownConceptId, sortedConceptValues) + if len(result) != len(conceptDefsAndCohortPairs) { t.Errorf("Expected %d data lines, found %d lines in total", - len(conceptIdsAndCohortPairs), + len(conceptDefsAndCohortPairs), len(result)) t.Errorf("Lines: %s", result) } diff --git a/tests/models_tests/models_test.go b/tests/models_tests/models_test.go index f47350f..33be968 100644 --- a/tests/models_tests/models_test.go +++ b/tests/models_tests/models_test.go @@ -221,13 +221,13 @@ func TestRetrieveInfoBySourceIdAndConceptTypesWrongType(t *testing.T) { } } -func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairsNoResults(t *testing.T) { +func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairsNoResults(t *testing.T) { setUp(t) // empty: - filterCohortPairs := []utils.CustomDichotomousVariableDef{} - stats, _ := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(testSourceId, + var filterConceptDefsAndCohortPairs []interface{} + stats, _ := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, smallestCohort.Id, - allConceptIds, filterCohortPairs, allConceptIds[0]) + filterConceptDefsAndCohortPairs, allConceptIds[0]) // none of the subjects has a value in all the concepts, so we expect len==0 here: if len(stats) != 0 { t.Errorf("Expected no results, found %d", len(stats)) @@ -413,18 +413,21 @@ func TestQueryFilterByCohortPairsHelper(t *testing.T) { func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndTwoCohortPairsWithResults(t *testing.T) { setUp(t) - filterIds := []int64{hareConceptId} populationCohort := largestCohort // setting the largest and smallest cohorts here as a pair: - filterCohortPairs := []utils.CustomDichotomousVariableDef{ - { + filterConceptDefsAndCohortPairs := []interface{}{ + utils.CustomConceptVariableDef{ + ConceptId: hareConceptId, + }, + utils.CustomDichotomousVariableDef{ CohortDefinitionId1: smallestCohort.Id, CohortDefinitionId2: largestCohort.Id, ProvidedName: "test"}, } + breakdownConceptId := hareConceptId // not normally the case...but we'll use the same here just for the test... - stats, _ := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(testSourceId, - populationCohort.Id, filterIds, filterCohortPairs, breakdownConceptId) + stats, _ := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, + populationCohort.Id, filterConceptDefsAndCohortPairs, breakdownConceptId) // we expect results, and we expect the total of persons to be 6, since only 6 of the persons // in largestCohort have a HARE value (and smallestCohort does not overlap with largest): countPersons := 0 @@ -438,18 +441,18 @@ func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndTwoCohortPai // then we should expect a reduction in the number of persons found. The reduction in this case // will take place because of a smaller intersection of the new cohorts with the population cohort, // and because of an overlaping person found in the two cohorts of the new pair. - filterCohortPairs = []utils.CustomDichotomousVariableDef{ - { + filterConceptDefsAndCohortPairs = []interface{}{ + utils.CustomDichotomousVariableDef{ CohortDefinitionId1: smallestCohort.Id, CohortDefinitionId2: largestCohort.Id, ProvidedName: "test"}, - { + utils.CustomDichotomousVariableDef{ CohortDefinitionId1: secondLargestCohort.Id, CohortDefinitionId2: extendedCopyOfSecondLargestCohort.Id, ProvidedName: "test2"}, } - stats, _ = conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(testSourceId, - populationCohort.Id, filterIds, filterCohortPairs, breakdownConceptId) + stats, _ = conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, + populationCohort.Id, filterConceptDefsAndCohortPairs, breakdownConceptId) countPersons = 0 for _, stat := range stats { countPersons += stat.NpersonsInCohortWithValue @@ -459,19 +462,21 @@ func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndTwoCohortPai } } -func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairsWithResults(t *testing.T) { +func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairsWithResults(t *testing.T) { setUp(t) - filterIds := []int64{hareConceptId} - // setting the same cohort id here (artificial...but just to check if that returns the same value as when this filter is not there): - filterCohortPairs := []utils.CustomDichotomousVariableDef{ - { + // setting the same extendedCopyOfSecondLargestCohort.Id here (artificial...but just to check if that returns the same value as when this filter is not there): + filterConceptDefsAndCohortPairs := []interface{}{ + utils.CustomConceptVariableDef{ + ConceptId: hareConceptId, + }, + utils.CustomDichotomousVariableDef{ CohortDefinitionId1: secondLargestCohort.Id, CohortDefinitionId2: extendedCopyOfSecondLargestCohort.Id, ProvidedName: "test"}, } breakdownConceptId := hareConceptId // not normally the case...but we'll use the same here just for the test... - stats, _ := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(testSourceId, - extendedCopyOfSecondLargestCohort.Id, filterIds, filterCohortPairs, breakdownConceptId) + stats, _ := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, + extendedCopyOfSecondLargestCohort.Id, filterConceptDefsAndCohortPairs, breakdownConceptId) // we expect values since secondLargestCohort has multiple subjects with hare info: if len(stats) < 4 { t.Errorf("Expected at least 4 results, found %d", len(stats)) @@ -488,10 +493,14 @@ func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairsW } prevName = stat.ValueName } - // test without the filterCohortPairs, should return the same result: - filterCohortPairs = []utils.CustomDichotomousVariableDef{} - stats2, _ := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(testSourceId, - extendedCopyOfSecondLargestCohort.Id, filterIds, filterCohortPairs, breakdownConceptId) + // test without the CustomDichotomousVariableDefs, should return the same result: + filterConceptDefsAndCohortPairs = []interface{}{ + utils.CustomConceptVariableDef{ + ConceptId: hareConceptId, + }, + } + stats2, _ := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, + extendedCopyOfSecondLargestCohort.Id, filterConceptDefsAndCohortPairs, breakdownConceptId) // very rough check (ideally we would check the individual stats as well...TODO?): if len(stats) > len(stats2) { t.Errorf("First query is more restrictive, so its stats should not be larger than stats2 of second query. Got %d and %d", len(stats), len(stats2)) @@ -500,14 +509,17 @@ func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairsW // test filtering with secondLargestCohort, smallest and largestCohort. // Lenght of result set should be 2 persons (one HIS, one ASN), since there is a overlap of 1 between secondLargestCohort and smallest cohort, // and overlap of 2 between secondLargestCohort and largestCohort, BUT only 1 has a HARE value: - filterCohortPairs = []utils.CustomDichotomousVariableDef{ - { + filterConceptDefsAndCohortPairs = []interface{}{ + utils.CustomConceptVariableDef{ + ConceptId: hareConceptId, + }, + utils.CustomDichotomousVariableDef{ CohortDefinitionId1: smallestCohort.Id, CohortDefinitionId2: largestCohort.Id, ProvidedName: "test"}, } - stats3, _ := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(testSourceId, - secondLargestCohort.Id, filterIds, filterCohortPairs, breakdownConceptId) + stats3, _ := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, + secondLargestCohort.Id, filterConceptDefsAndCohortPairs, breakdownConceptId) if len(stats3) != 2 { t.Errorf("Expected only two items in resultset, found %d", len(stats3)) } diff --git a/utils/parsing.go b/utils/parsing.go index 0e44092..4292b6b 100644 --- a/utils/parsing.go +++ b/utils/parsing.go @@ -175,7 +175,11 @@ func ParseConceptDefsAndDichotomousDefsAsSingleList(c *gin.Context) ([]interface log.Printf("Error decoding request body: %v", err) return nil, errors.New("failed to parse JSON request body") } - + // Check if the "variables" field is absent (nil) + if requestBody.Variables == nil { + log.Println("Missing 'variables' field in the request body") + return nil, errors.New("'variables' field must be present in the request body") + } conceptDefsAndDichotomousDefs := make([]interface{}, 0) for _, variable := range requestBody.Variables {