From 8a5dbe87e112bb5651a010f768cbbb10bf29e1bd Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Fri, 24 Jan 2025 17:54:14 +0100 Subject: [PATCH] feat: remove histogramConceptId from histogram endpoint ...and expect it as part of the variables instead --- controllers/cohortdata.go | 10 +------- models/cohortdata.go | 14 ++++++++--- tests/controllers_tests/controllers_test.go | 2 +- tests/models_tests/models_test.go | 28 ++++++++++++++------- utils/parsing.go | 16 ++++++++++++ 5 files changed, 47 insertions(+), 23 deletions(-) diff --git a/controllers/cohortdata.go b/controllers/cohortdata.go index 4aaf1c1..0f00368 100644 --- a/controllers/cohortdata.go +++ b/controllers/cohortdata.go @@ -36,14 +36,6 @@ func (u CohortDataController) RetrieveHistogramForCohortIdAndConceptId(c *gin.Co return } - histogramIdStr := c.Param("histogramid") - if histogramIdStr == "" { - c.JSON(http.StatusBadRequest, gin.H{"message": "bad request", "error": "missing histogramid parameter in request"}) - c.Abort() - return - } - histogramConceptId, _ := strconv.ParseInt(histogramIdStr, 10, 64) - // parse cohortPairs separately as well, so we can validate permissions _, cohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptIdsAndCohortPairs) if err != nil { @@ -59,7 +51,7 @@ func (u CohortDataController) RetrieveHistogramForCohortIdAndConceptId(c *gin.Co return } - cohortData, err := u.cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId, cohortId, histogramConceptId, conceptIdsAndCohortPairs) + cohortData, err := u.cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId, cohortId, conceptIdsAndCohortPairs) if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving histogram data", "error": err.Error()}) c.Abort() diff --git a/models/cohortdata.go b/models/cohortdata.go index e15ac8d..60ba6a0 100644 --- a/models/cohortdata.go +++ b/models/cohortdata.go @@ -13,7 +13,7 @@ type CohortDataI interface { RetrieveCohortOverlapStats(sourceId int, caseCohortId int, controlCohortId int, otherFilterConceptIds []int64, filterCohortPairs []utils.CustomDichotomousVariableDef) (CohortOverlapStats, error) RetrieveDataByOriginalCohortAndNewCohort(sourceId int, originalCohortDefinitionId int, cohortDefinitionId int) ([]*PersonIdAndCohort, error) RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsAndCohortPairs(sourceId int, cohortDefinitionId int, histogramConceptId int64, filterConceptIdsAndValues []utils.CustomConceptVariableDef, filterCohortPairs []utils.CustomDichotomousVariableDef) ([]*PersonConceptAndValue, error) - RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId int, cohortDefinitionId int, histogramConceptId int64, filterConceptDefsAndCohortPairs []interface{}) ([]*PersonConceptAndValue, error) + RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId int, cohortDefinitionId int, filterConceptDefsAndCohortPairs []interface{}) ([]*PersonConceptAndValue, error) RetrieveBarGraphDataBySourceIdAndCohortIdAndConceptIds(sourceId int, conceptId int64) ([]*NominalGroupData, error) RetrieveHistogramDataBySourceIdAndConceptId(sourceId int, histogramConceptId int64) ([]*PersonConceptAndValue, error) } @@ -99,22 +99,27 @@ func (h CohortData) RetrieveDataBySourceIdAndCohortIdAndConceptIdsOrderedByPerso return cohortData, meta_result.Error } -func (h CohortData) RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId int, cohortDefinitionId int, histogramConceptId int64, filterConceptDefsAndCohortPairs []interface{}) ([]*PersonConceptAndValue, error) { +func (h CohortData) RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId int, cohortDefinitionId int, filterConceptDefsAndCohortPairs []interface{}) ([]*PersonConceptAndValue, error) { var dataSourceModel = new(Source) omopDataSource := dataSourceModel.GetDataSource(sourceId, Omop) resultsDataSource := dataSourceModel.GetDataSource(sourceId, Results) finalSetAlias := "final_set_alias" + histogramConcept, err := utils.GetLastCustomConceptVariableDef(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 { 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 = ?", histogramConceptId). + Where("observation.observation_concept_id = ?", histogramConcept.ConceptId). Where("observation.value_as_number is not null") } @@ -125,6 +130,7 @@ func (h CohortData) RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlus }) if err != nil { log.Fatalf("Transaction failed: %v", err) + return nil, err } return cohortData, err } diff --git a/tests/controllers_tests/controllers_test.go b/tests/controllers_tests/controllers_test.go index 1381f79..15f5692 100644 --- a/tests/controllers_tests/controllers_test.go +++ b/tests/controllers_tests/controllers_test.go @@ -81,7 +81,7 @@ func (h dummyCohortDataModel) RetrieveHistogramDataBySourceIdAndCohortIdAndConce return cohortData, nil } -func (h dummyCohortDataModel) RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId int, cohortDefinitionId int, histogramConceptId int64, filterConceptDefsAndCohortPairs []interface{}) ([]*models.PersonConceptAndValue, error) { +func (h dummyCohortDataModel) RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId int, cohortDefinitionId int, filterConceptDefsAndCohortPairs []interface{}) ([]*models.PersonConceptAndValue, error) { cohortData := []*models.PersonConceptAndValue{} return cohortData, nil } diff --git a/tests/models_tests/models_test.go b/tests/models_tests/models_test.go index 716d81d..c5f2a0e 100644 --- a/tests/models_tests/models_test.go +++ b/tests/models_tests/models_test.go @@ -783,8 +783,12 @@ func TestRetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsAndCohortPairs( func TestRetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(t *testing.T) { setUp(t) - filterConceptDefsPlusCohortPairs := []interface{}{} - data, error := cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, largestCohort.Id, histogramConceptId, filterConceptDefsPlusCohortPairs) + filterConceptDefsPlusCohortPairs := []interface{}{ + utils.CustomConceptVariableDef{ + ConceptId: histogramConceptId, + }, + } + data, error := cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, largestCohort.Id, filterConceptDefsPlusCohortPairs) if error != nil { t.Errorf("Got error: %s", error) } @@ -800,9 +804,12 @@ func TestRetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs CohortDefinitionId2: extendedCopyOfSecondLargestCohort.Id, ProvidedName: "test", }, + utils.CustomConceptVariableDef{ + ConceptId: histogramConceptId, + }, } // then we expect histogram data for the overlapping population only (which is 5 for extendedCopyOfSecondLargestCohort and largestCohort): - data, _ = cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, largestCohort.Id, histogramConceptId, filterConceptDefsPlusCohortPairs) + data, _ = cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, largestCohort.Id, filterConceptDefsPlusCohortPairs) if len(data) != 5 { t.Errorf("expected 5 histogram data but got %d", len(data)) } @@ -818,8 +825,11 @@ func TestRetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs }, }, }, + utils.CustomConceptVariableDef{ + ConceptId: histogramConceptId, + }, } - data, _ = cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, largestCohort.Id, histogramConceptId, filterConceptDefsPlusCohortPairs) + data, _ = cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, largestCohort.Id, filterConceptDefsPlusCohortPairs) if len(data) != 1 { t.Errorf("expected 1 histogram data but got %d", len(data)) } @@ -827,7 +837,7 @@ func TestRetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs // another concept filter: filterConceptDefsPlusCohortPairs = []interface{}{ utils.CustomConceptVariableDef{ - ConceptId: 2000006885, + ConceptId: histogramConceptId, Filters: []utils.Filter{ { Type: ">=", @@ -836,7 +846,7 @@ func TestRetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs }, }, } - data, _ = cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, largestCohort.Id, histogramConceptId, filterConceptDefsPlusCohortPairs) + data, _ = cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, largestCohort.Id, filterConceptDefsPlusCohortPairs) if len(data) != 2 { t.Errorf("expected 2 histogram data but got %d", len(data)) } @@ -854,7 +864,7 @@ func TestRetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs Transformation: "log", }, } - data, _ = cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, largestCohort.Id, histogramConceptId, filterConceptDefsPlusCohortPairs) + data, _ = cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, largestCohort.Id, filterConceptDefsPlusCohortPairs) // make sure the filter worked on transformed values: if len(data) != 2 { t.Errorf("expected 2 histogram data but got %d", len(data)) @@ -877,7 +887,7 @@ func TestRetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs Transformation: "z_score", }, } - data, _ = cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, largestCohort.Id, histogramConceptId, filterConceptDefsPlusCohortPairs) + data, _ = cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, largestCohort.Id, filterConceptDefsPlusCohortPairs) // make sure the filter worked on transformed values: if len(data) != 2 { t.Errorf("expected 2 histogram data but got %d", len(data)) @@ -908,7 +918,7 @@ func TestRetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs // calculated over two values only }, } - data, _ = cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, largestCohort.Id, histogramConceptId, filterConceptDefsPlusCohortPairs) + data, _ = cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, largestCohort.Id, filterConceptDefsPlusCohortPairs) // make sure the filter worked on transformed values: if len(data) != 1 { t.Errorf("expected 1 histogram data but got %d", len(data)) diff --git a/utils/parsing.go b/utils/parsing.go index d2bff65..da79922 100644 --- a/utils/parsing.go +++ b/utils/parsing.go @@ -479,3 +479,19 @@ func GenerateSynchronizedTimestampID() string { return fmt.Sprintf("%x", time.Now().UnixNano()) } + +// GetLastCustomConceptVariableDef checks if the last item is of type CustomConceptVariableDef and returns it +func GetLastCustomConceptVariableDef(filterConceptDefsAndCohortPairs []interface{}) (*CustomConceptVariableDef, error) { + if len(filterConceptDefsAndCohortPairs) == 0 { + return nil, fmt.Errorf("the slice is empty") + } + + // Get the last item + lastItem := filterConceptDefsAndCohortPairs[len(filterConceptDefsAndCohortPairs)-1] + + // Assert the type + if def, ok := lastItem.(CustomConceptVariableDef); ok { + return &def, nil + } + return nil, fmt.Errorf("last item is not of type CustomConceptVariableDef") +}