Skip to content

Commit

Permalink
feat: remove histogramConceptId from histogram endpoint
Browse files Browse the repository at this point in the history
...and expect it as part of the variables instead
  • Loading branch information
pieterlukasse committed Jan 24, 2025
1 parent 98baf7b commit 8a5dbe8
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 23 deletions.
10 changes: 1 addition & 9 deletions controllers/cohortdata.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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()
Expand Down
14 changes: 10 additions & 4 deletions models/cohortdata.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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")
}

Expand All @@ -125,6 +130,7 @@ func (h CohortData) RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlus
})
if err != nil {
log.Fatalf("Transaction failed: %v", err)
return nil, err
}
return cohortData, err
}
Expand Down
2 changes: 1 addition & 1 deletion tests/controllers_tests/controllers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
28 changes: 19 additions & 9 deletions tests/models_tests/models_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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))
}
Expand All @@ -818,16 +825,19 @@ 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))
}

// another concept filter:
filterConceptDefsPlusCohortPairs = []interface{}{
utils.CustomConceptVariableDef{
ConceptId: 2000006885,
ConceptId: histogramConceptId,
Filters: []utils.Filter{
{
Type: ">=",
Expand All @@ -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))
}
Expand All @@ -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))
Expand All @@ -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))
Expand Down Expand Up @@ -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))
Expand Down
16 changes: 16 additions & 0 deletions utils/parsing.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

0 comments on commit 8a5dbe8

Please sign in to comment.