From 743c8aae8938874d9d70737ae833601c70fd3e6b Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Wed, 28 Jun 2023 21:36:12 +0200 Subject: [PATCH] feat: adjust concept.RetrieveAttritionTable to produce rows in new order ...where the order of the rows should follow the order of the concept ids and dichotomous pair variables found in the request --- controllers/concept.go | 114 ++++++++------------ models/concept.go | 1 + tests/controllers_tests/controllers_test.go | 97 ++++++++--------- utils/parsing.go | 64 ++++++++--- 4 files changed, 138 insertions(+), 138 deletions(-) diff --git a/controllers/concept.go b/controllers/concept.go index facde74..3f61c4d 100644 --- a/controllers/concept.go +++ b/controllers/concept.go @@ -167,60 +167,8 @@ func generateRowForVariable(variableName string, breakdownConceptValuesToPeopleC return row } -func (u ConceptController) GetConceptVariablesAttritionRows(sourceId int, cohortId int, conceptIds []int64, breakdownConceptId int64, sortedConceptValues []string) ([][]string, error) { - conceptIdToName := make(map[int64]string) - conceptInformations, err := u.conceptModel.RetrieveInfoBySourceIdAndConceptIds(sourceId, conceptIds) - if err != nil { - return nil, fmt.Errorf("could not retrieve concept informations due to error: %s", err.Error()) - } - for _, conceptInformation := range conceptInformations { - conceptIdToName[conceptInformation.ConceptId] = conceptInformation.ConceptName - } - - var rows [][]string - for idx, conceptId := range conceptIds { - // run each query with a longer list of filterConceptIds, until the last query is run with them all: - filterConceptIds := conceptIds[0 : idx+1] - // use empty cohort pairs list: - filterCohortPairs := []utils.CustomDichotomousVariableDef{} - breakdownStats, err := u.conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(sourceId, cohortId, filterConceptIds, filterCohortPairs, breakdownConceptId) - if err != nil { - return nil, fmt.Errorf("could not retrieve concept Breakdown for concepts %v due to error: %s", filterConceptIds, err.Error()) - } - - conceptValuesToPeopleCount := getConceptValueToPeopleCount(breakdownStats) - variableName := conceptIdToName[conceptId] - log.Printf("Generating row for variable with name %s", variableName) - generatedRow := generateRowForVariable(variableName, conceptValuesToPeopleCount, sortedConceptValues) - rows = append(rows, generatedRow) - } - - return rows, nil -} - -func (u ConceptController) GetCustomDichotomousVariablesAttritionRows(sourceId int, cohortId int, filterConceptIds []int64, filterCohortPairs []utils.CustomDichotomousVariableDef, breakdownConceptId int64, sortedConceptValues []string) ([][]string, error) { - // TODO - this function is very similar to GetConceptVariablesAttritionRows above and they can probably be merged. - var rows [][]string - for idx, cohortPair := range filterCohortPairs { - // run each query with the full list of filterConceptIds and an increasingly longer list of filterCohortPairs, until the last query is run with them all: - filterCohortPairs := filterCohortPairs[0 : idx+1] - breakdownStats, err := u.conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(sourceId, cohortId, filterConceptIds, filterCohortPairs, breakdownConceptId) - if err != nil { - return nil, fmt.Errorf("could not retrieve concept Breakdown for dichotomous variables %v due to error: %s", filterConceptIds, err.Error()) - } - - conceptValuesToPeopleCount := getConceptValueToPeopleCount(breakdownStats) - variableName := cohortPair.ProvidedName - log.Printf("Generating row for variable...") - generatedRow := generateRowForVariable(variableName, conceptValuesToPeopleCount, sortedConceptValues) - rows = append(rows, generatedRow) - } - - return rows, nil -} - func (u ConceptController) RetrieveAttritionTable(c *gin.Context) { - sourceId, cohortId, conceptIds, cohortPairs, err := utils.ParseSourceIdAndCohortIdAndVariablesList(c) + sourceId, cohortId, conceptIdsAndCohortPairs, 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()}) @@ -245,7 +193,7 @@ func (u ConceptController) RetrieveAttritionTable(c *gin.Context) { breakdownStats, err := u.conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortId(sourceId, cohortId, breakdownConceptId) if err != nil { log.Printf("Error: %s", err.Error()) - c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving concept breakdown with filtered conceptIds", "error": err.Error()}) + c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving concept breakdown for given cohortId", "error": err.Error()}) c.Abort() return } @@ -255,32 +203,58 @@ func (u ConceptController) RetrieveAttritionTable(c *gin.Context) { headerAndNonFilteredRow, err := u.GenerateHeaderAndNonFilteredRow(breakdownStats, sortedConceptValues, cohortName) if err != nil { log.Printf("Error: %s", err.Error()) - c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving concept breakdown with filtered conceptIds", "error": err.Error()}) + c.JSON(http.StatusInternalServerError, gin.H{"message": "Error generating concept breakdown header and cohort rows", "error": err.Error()}) c.Abort() return } - - // append concepts to attrition table: - conceptVariablesAttritionRows, err := u.GetConceptVariablesAttritionRows(sourceId, cohortId, conceptIds, breakdownConceptId, sortedConceptValues) + otherAttritionRows, err := u.GetAttritionRowForConceptIdsAndCohortPairs(sourceId, cohortId, conceptIdsAndCohortPairs, breakdownConceptId, sortedConceptValues) if err != nil { log.Printf("Error: %s", err.Error()) - c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving concept breakdown with filtered conceptIds", "error": err.Error()}) + c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving concept breakdown rows for filter conceptIds and cohortPairs", "error": err.Error()}) c.Abort() return } - // append custom dichotomous items to attrition table: - customDichotomousVariablesAttritionRows, err := u.GetCustomDichotomousVariablesAttritionRows(sourceId, cohortId, conceptIds, cohortPairs, breakdownConceptId, sortedConceptValues) - if err != nil { - log.Printf("Error: %s", err.Error()) - c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving concept breakdown with custom dichotomous variables (aka cohortpairs)", "error": err.Error()}) - c.Abort() - return + b := GenerateAttritionCSV(headerAndNonFilteredRow, otherAttritionRows) + c.String(http.StatusOK, b.String()) +} + +func (u ConceptController) GetAttritionRowForConceptIdsAndCohortPairs(sourceId int, cohortId int, conceptIdsAndCohortPairs []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] + + attritionRow, err := u.GetAttritionRowForConceptIdOrCohortPair(sourceId, cohortId, conceptIdOrCohortPair, filterConceptIdsAndCohortPairs, breakdownConceptId, sortedConceptValues) + if err != nil { + log.Printf("Error: %s", err.Error()) + return nil, err + } + otherAttritionRows = append(otherAttritionRows, attritionRow) } + return otherAttritionRows, nil +} - // concat all rows: - var allVariablesAttritionRows = append(conceptVariablesAttritionRows, customDichotomousVariablesAttritionRows...) - b := GenerateAttritionCSV(headerAndNonFilteredRow, allVariablesAttritionRows) - c.String(http.StatusOK, b.String()) +func (u ConceptController) GetAttritionRowForConceptIdOrCohortPair(sourceId int, cohortId int, conceptIdOrCohortPair interface{}, filterConceptIdsAndCohortPairs []interface{}, breakdownConceptId int64, sortedConceptValues []string) ([]string, error) { + filterConceptIds, filterCohortPairs := utils.GetConceptIdsAndCohortPairsAsSeparateLists(filterConceptIdsAndCohortPairs) + breakdownStats, err := u.conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(sourceId, cohortId, filterConceptIds, filterCohortPairs, 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()) + } + conceptValuesToPeopleCount := getConceptValueToPeopleCount(breakdownStats) + variableName := "" + switch convertedItem := conceptIdOrCohortPair.(type) { + case int64: + conceptInformation, err := u.conceptModel.RetrieveInfoBySourceIdAndConceptId(sourceId, convertedItem) + if err != nil { + return nil, fmt.Errorf("could not retrieve concept details for %v due to error: %s", convertedItem, err.Error()) + } + variableName = conceptInformation.ConceptName + case utils.CustomDichotomousVariableDef: + variableName = convertedItem.ProvidedName + } + log.Printf("Generating row for variable with name %s", variableName) + generatedRow := generateRowForVariable(variableName, conceptValuesToPeopleCount, sortedConceptValues) + return generatedRow, nil } func getSortedConceptValues(breakdownStats []*models.ConceptBreakdown) []string { diff --git a/models/concept.go b/models/concept.go index a8fb7db..f8801f5 100644 --- a/models/concept.go +++ b/models/concept.go @@ -8,6 +8,7 @@ import ( type ConceptI interface { RetriveAllBySourceId(sourceId int) ([]*Concept, error) + RetrieveInfoBySourceIdAndConceptId(sourceId int, conceptId int64) (*ConceptSimple, error) RetrieveInfoBySourceIdAndConceptIds(sourceId int, conceptIds []int64) ([]*ConceptSimple, error) RetrieveInfoBySourceIdAndConceptTypes(sourceId int, conceptTypes []string) ([]*ConceptSimple, error) RetrieveBreakdownStatsBySourceIdAndCohortId(sourceId int, cohortDefinitionId int, breakdownConceptId int64) ([]*ConceptBreakdown, error) diff --git a/tests/controllers_tests/controllers_test.go b/tests/controllers_tests/controllers_test.go index cac61d5..368b660 100644 --- a/tests/controllers_tests/controllers_test.go +++ b/tests/controllers_tests/controllers_test.go @@ -57,8 +57,6 @@ var cohortDefinitionControllerNeedsDb = controllers.NewCohortDefinitionControlle // instance of the controller that talks to a mock implementation of the model: var cohortDefinitionController = controllers.NewCohortDefinitionController(*new(dummyCohortDefinitionDataModel)) -var conceptController = controllers.NewConceptController(*new(dummyConceptDataModel), *new(dummyCohortDefinitionDataModel)) - type dummyCohortDataModel struct{} func (h dummyCohortDataModel) RetrieveDataBySourceIdAndCohortIdAndConceptIdsOrderedByPersonId(sourceId int, cohortDefinitionId int, conceptIds []int64) ([]*models.PersonConceptAndValue, error) { @@ -129,11 +127,28 @@ func (h dummyCohortDefinitionDataModel) GetAllCohortDefinitions() ([]*models.Coh return nil, nil } +var conceptController = controllers.NewConceptController(*new(dummyConceptDataModel), *new(dummyCohortDefinitionDataModel)) + type dummyConceptDataModel struct{} func (h dummyConceptDataModel) RetriveAllBySourceId(sourceId int) ([]*models.Concept, error) { return nil, nil } + +func (h dummyConceptDataModel) RetrieveInfoBySourceIdAndConceptId(sourceId int, conceptId int64) (*models.ConceptSimple, error) { + conceptSimpleItems := []*models.ConceptSimple{ + {ConceptId: 1234, ConceptName: "Concept A"}, + {ConceptId: 5678, ConceptName: "Concept B"}, + {ConceptId: 2090006880, ConceptName: "Concept C"}, + } + for _, conceptSimple := range conceptSimpleItems { + if conceptSimple.ConceptId == conceptId { + return conceptSimple, nil + } + } + return nil, fmt.Errorf("concept id %d not found in mock data", conceptId) +} + func (h dummyConceptDataModel) RetrieveInfoBySourceIdAndConceptIds(sourceId int, conceptIds []int64) ([]*models.ConceptSimple, error) { // dummy data with _some_ of the relevant fields: conceptSimple := []*models.ConceptSimple{ @@ -169,8 +184,8 @@ func (h dummyConceptDataModel) RetrieveBreakdownStatsBySourceIdAndCohortId(sourc } func (h dummyConceptDataModel) RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(sourceId int, cohortDefinitionId int, filterConceptIds []int64, filterCohortPairs []utils.CustomDichotomousVariableDef, breakdownConceptId int64) ([]*models.ConceptBreakdown, error) { conceptBreakdown := []*models.ConceptBreakdown{ - {ConceptValue: "value1", NpersonsInCohortWithValue: 4}, - {ConceptValue: "value2", NpersonsInCohortWithValue: 7}, + {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(filterConceptIds)}, // simulate decreasing numbers as filter increases- the use of filterConceptIds instead of filterCohortPairs is otherwise meaningless here... } if dummyModelReturnError { return nil, fmt.Errorf("error!") @@ -603,75 +618,49 @@ func TestGenerateHeaderAndNonFilterRow(t *testing.T) { } } -func TestGetConceptVariablesAttritionRows(t *testing.T) { +func TestGetAttritionRowForConceptIdsAndCohortPairs(t *testing.T) { setUp(t) sourceId := 1 cohortId := 1 var breakdownConceptId int64 = 1 - conceptIds := []int64{1234, 5678, 2090006880} - sortedConceptValues := []string{"value1", "value2"} - - result, _ := conceptController.GetConceptVariablesAttritionRows(sourceId, cohortId, conceptIds, breakdownConceptId, sortedConceptValues) - if len(result) != 3 { - t.Errorf("Expected 3 data lines, found %d lines in total", - len(result)) - t.Errorf("Lines: %s", result) - } - - expectedLines := [][]string{ - {"Concept A", "11", "4", "7"}, - {"Concept B", "11", "4", "7"}, - {"Concept C", "11", "4", "7"}, - } - - i := 0 - for _, expectedLine := range expectedLines { - if !reflect.DeepEqual(expectedLine, result[i]) { - t.Errorf("header or non filter row line not as expected. \nExpected: \n%s \nFound: \n%s", - expectedLine, result[i]) - } - i++ - } -} + sortedConceptValues := []string{"value1", "value2", "value3"} -func TestGetCustomDichotomousVariablesAttritionRows(t *testing.T) { - setUp(t) - sourceId := 1 - cohortId := 1 - var breakdownConceptId int64 = 1 - conceptIds := []int64{1234, 5678, 2090006880} - cohortPairs := []utils.CustomDichotomousVariableDef{ - { + // mix of concept ids and CustomDichotomousVariableDef items: + conceptIdsAndCohortPairs := []interface{}{ + int64(1234), + int64(5678), + utils.CustomDichotomousVariableDef{ CohortId1: 1, CohortId2: 2, ProvidedName: "testA12"}, - { + int64(2090006880), + utils.CustomDichotomousVariableDef{ CohortId1: 3, CohortId2: 4, ProvidedName: "testB34"}, } - sortedConceptValues := []string{"value1", "value2", "value3"} - - result, _ := conceptController.GetCustomDichotomousVariablesAttritionRows(sourceId, cohortId, conceptIds, cohortPairs, breakdownConceptId, sortedConceptValues) - if len(result) != 2 { - t.Errorf("Expected 3 data lines, found %d lines in total", + result, _ := conceptController.GetAttritionRowForConceptIdsAndCohortPairs(sourceId, cohortId, conceptIdsAndCohortPairs, breakdownConceptId, sortedConceptValues) + if len(result) != len(conceptIdsAndCohortPairs) { + t.Errorf("Expected %d data lines, found %d lines in total", + len(conceptIdsAndCohortPairs), len(result)) t.Errorf("Lines: %s", result) } expectedLines := [][]string{ - {"testA12", "11", "4", "7", "0"}, - {"testB34", "11", "4", "7", "0"}, + {"Concept A", "10", "4", "6", "0"}, + {"Concept B", "9", "4", "5", "0"}, + {"testA12", "8", "3", "5", "0"}, + {"Concept C", "7", "3", "4", "0"}, + {"testB34", "6", "2", "4", "0"}, } - i := 0 - for _, expectedLine := range expectedLines { + for i, expectedLine := range expectedLines { if !reflect.DeepEqual(expectedLine, result[i]) { t.Errorf("header or non filter row line not as expected. \nExpected: \n%s \nFound: \n%s", expectedLine, result[i]) } - i++ } } @@ -849,8 +838,8 @@ func TestRetrieveAttritionTable(t *testing.T) { requestContext.Params = append(requestContext.Params, gin.Param{Key: "breakdownconceptid", Value: "2"}) requestContext.Writer = new(tests.CustomResponseWriter) requestContext.Request = new(http.Request) - requestBody := "{\"variables\":[{\"variable_type\": \"concept\", \"concept_id\": 2090006880}," + - "{\"variable_type\": \"custom_dichotomous\", \"provided_name\": \"testABC\", \"cohort_ids\": [1, 3]}," + + requestBody := "{\"variables\":[{\"variable_type\": \"custom_dichotomous\", \"provided_name\": \"testABC\", \"cohort_ids\": [1, 3]}," + + "{\"variable_type\": \"concept\", \"concept_id\": 2090006880}," + "{\"variable_type\": \"custom_dichotomous\", \"cohort_ids\": [4, 5]}]}" // this one with no provided name (to test auto generated one) requestContext.Request.Body = io.NopCloser(strings.NewReader(requestBody)) requestContext.Writer = new(tests.CustomResponseWriter) @@ -862,9 +851,9 @@ func TestRetrieveAttritionTable(t *testing.T) { expectedLines := []string{ "Cohort,Size,value1_name,value2_name", "dummy cohort name,13,5,8", - "Concept C,11,4,7", - "testABC,11,4,7", - "ID_4_5,11,4,7", + "testABC,10,3,7", + "Concept C,9,3,6", + "ID_4_5,8,2,6", } i := 0 for _, expectedLine := range expectedLines { diff --git a/utils/parsing.go b/utils/parsing.go index 59dac57..e6a25f4 100644 --- a/utils/parsing.go +++ b/utils/parsing.go @@ -102,27 +102,27 @@ func GetCohortPairKey(firstCohortDefinitionId int, secondCohortDefinitionId int) // {variable_type: "custom_dichotomous", provided_name: "name2", cohort_ids: [cohortM_id, cohortN_id]}, // ... // ]} -// It returns the list of concept_id values and the list of custom dichotomous variable definitions. -func ParseConceptIdsAndDichotomousDefs(c *gin.Context) ([]int64, []CustomDichotomousVariableDef, error) { +// It returns the list with all concept_id values and custom dichotomous variable definitions. +func ParseConceptIdsAndDichotomousDefsAsSingleList(c *gin.Context) ([]interface{}, error) { if c.Request == nil || c.Request.Body == nil { - return nil, nil, errors.New("bad request - no request body") + return nil, errors.New("bad request - no request body") } decoder := json.NewDecoder(c.Request.Body) request := make(map[string][]map[string]interface{}) err := decoder.Decode(&request) if err != nil { log.Printf("Error: %s", err) - return nil, nil, err + return nil, err } variables := request["variables"] - conceptIds := []int64{} - variableDefinitions := []CustomDichotomousVariableDef{} + conceptIdsAndCohortPairs := make([]interface{}, 0) + // TODO - this parsing will throw a lot of "null pointer" errors since it does not validate if specific entries are found in the json before // accessing them...needs to be fixed to throw better errors: for _, variable := range variables { if variable["variable_type"] == "concept" { - conceptIds = append(conceptIds, int64(variable["concept_id"].(float64))) + conceptIdsAndCohortPairs = append(conceptIdsAndCohortPairs, int64(variable["concept_id"].(float64))) } if variable["variable_type"] == "custom_dichotomous" { cohortPair := []int{} @@ -139,10 +139,21 @@ func ParseConceptIdsAndDichotomousDefs(c *gin.Context) ([]int64, []CustomDichoto CohortId2: cohortPair[1], ProvidedName: providedName, } - variableDefinitions = append(variableDefinitions, customDichotomousVariableDef) + conceptIdsAndCohortPairs = append(conceptIdsAndCohortPairs, customDichotomousVariableDef) } } - return conceptIds, variableDefinitions, nil + return conceptIdsAndCohortPairs, nil +} + +// deprecated: for backwards compatibility +func ParseConceptIdsAndDichotomousDefs(c *gin.Context) ([]int64, []CustomDichotomousVariableDef, error) { + conceptIdsAndCohortPairs, err := ParseConceptIdsAndDichotomousDefsAsSingleList(c) + if err != nil { + log.Printf("Error: %s", err) + return nil, nil, err + } + conceptIds, cohortPairs := GetConceptIdsAndCohortPairsAsSeparateLists(conceptIdsAndCohortPairs) + return conceptIds, cohortPairs, nil } func ParseSourceIdAndConceptIds(c *gin.Context) (int, []int64, error) { @@ -217,16 +228,41 @@ func ParseSourceAndCohortId(c *gin.Context) (int, int, error) { return sourceId, cohortId, nil } -// returns sourceid, cohortid, list of variables (formed by concept ids and/or of cohort tuples which are also known as custom dichotomous variables) +// separates a conceptIdsAndCohortPairs into a conceptIds list and a cohortPairs list +func GetConceptIdsAndCohortPairsAsSeparateLists(conceptIdsAndCohortPairs []interface{}) ([]int64, []CustomDichotomousVariableDef) { + conceptIds := []int64{} + cohortPairs := []CustomDichotomousVariableDef{} + for _, item := range conceptIdsAndCohortPairs { + switch convertedItem := item.(type) { + case int64: + conceptIds = append(conceptIds, convertedItem) + case CustomDichotomousVariableDef: + cohortPairs = append(cohortPairs, convertedItem) + } + } + return conceptIds, cohortPairs +} + +// deprecated: returns the conceptIds and cohortPairs as separate lists (for backwards compatibility) func ParseSourceIdAndCohortIdAndVariablesList(c *gin.Context) (int, int, []int64, []CustomDichotomousVariableDef, error) { + sourceId, cohortId, conceptIdsAndCohortPairs, err := ParseSourceIdAndCohortIdAndVariablesAsSingleList(c) + if err != nil { + return -1, -1, nil, nil, err + } + conceptIds, cohortPairs := GetConceptIdsAndCohortPairsAsSeparateLists(conceptIdsAndCohortPairs) + return sourceId, cohortId, conceptIds, cohortPairs, nil +} + +// returns sourceid, cohortid, list of variables (formed by concept ids and/or of cohort tuples which are also known as custom dichotomous variables) +func ParseSourceIdAndCohortIdAndVariablesAsSingleList(c *gin.Context) (int, int, []interface{}, error) { // parse and validate all parameters: sourceId, cohortId, err := ParseSourceAndCohortId(c) if err != nil { - return -1, -1, nil, nil, err + return -1, -1, nil, err } - conceptIds, cohortPairs, err := ParseConceptIdsAndDichotomousDefs(c) + conceptIdsAndCohortPairs, err := ParseConceptIdsAndDichotomousDefsAsSingleList(c) if err != nil { - return -1, -1, nil, nil, err + return -1, -1, nil, err } - return sourceId, cohortId, conceptIds, cohortPairs, nil + return sourceId, cohortId, conceptIdsAndCohortPairs, nil }