Skip to content

Commit 5d0409d

Browse files
pieterlukassepieterlukasse
authored
Merge pull request #74 from uc-cdis/fix/order_of_variables_in_attrition_csv
Feat: adjust concept.RetrieveAttritionTable to produce rows in new order
2 parents 012dc01 + 743c8aa commit 5d0409d

File tree

4 files changed

+138
-138
lines changed

4 files changed

+138
-138
lines changed

‎controllers/concept.go

+44-70
Original file line numberDiff line numberDiff line change
@@ -167,60 +167,8 @@ func generateRowForVariable(variableName string, breakdownConceptValuesToPeopleC
167167
return row
168168
}
169169

170-
func (u ConceptController) GetConceptVariablesAttritionRows(sourceId int, cohortId int, conceptIds []int64, breakdownConceptId int64, sortedConceptValues []string) ([][]string, error) {
171-
conceptIdToName := make(map[int64]string)
172-
conceptInformations, err := u.conceptModel.RetrieveInfoBySourceIdAndConceptIds(sourceId, conceptIds)
173-
if err != nil {
174-
return nil, fmt.Errorf("could not retrieve concept informations due to error: %s", err.Error())
175-
}
176-
for _, conceptInformation := range conceptInformations {
177-
conceptIdToName[conceptInformation.ConceptId] = conceptInformation.ConceptName
178-
}
179-
180-
var rows [][]string
181-
for idx, conceptId := range conceptIds {
182-
// run each query with a longer list of filterConceptIds, until the last query is run with them all:
183-
filterConceptIds := conceptIds[0 : idx+1]
184-
// use empty cohort pairs list:
185-
filterCohortPairs := []utils.CustomDichotomousVariableDef{}
186-
breakdownStats, err := u.conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(sourceId, cohortId, filterConceptIds, filterCohortPairs, breakdownConceptId)
187-
if err != nil {
188-
return nil, fmt.Errorf("could not retrieve concept Breakdown for concepts %v due to error: %s", filterConceptIds, err.Error())
189-
}
190-
191-
conceptValuesToPeopleCount := getConceptValueToPeopleCount(breakdownStats)
192-
variableName := conceptIdToName[conceptId]
193-
log.Printf("Generating row for variable with name %s", variableName)
194-
generatedRow := generateRowForVariable(variableName, conceptValuesToPeopleCount, sortedConceptValues)
195-
rows = append(rows, generatedRow)
196-
}
197-
198-
return rows, nil
199-
}
200-
201-
func (u ConceptController) GetCustomDichotomousVariablesAttritionRows(sourceId int, cohortId int, filterConceptIds []int64, filterCohortPairs []utils.CustomDichotomousVariableDef, breakdownConceptId int64, sortedConceptValues []string) ([][]string, error) {
202-
// TODO - this function is very similar to GetConceptVariablesAttritionRows above and they can probably be merged.
203-
var rows [][]string
204-
for idx, cohortPair := range filterCohortPairs {
205-
// 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:
206-
filterCohortPairs := filterCohortPairs[0 : idx+1]
207-
breakdownStats, err := u.conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(sourceId, cohortId, filterConceptIds, filterCohortPairs, breakdownConceptId)
208-
if err != nil {
209-
return nil, fmt.Errorf("could not retrieve concept Breakdown for dichotomous variables %v due to error: %s", filterConceptIds, err.Error())
210-
}
211-
212-
conceptValuesToPeopleCount := getConceptValueToPeopleCount(breakdownStats)
213-
variableName := cohortPair.ProvidedName
214-
log.Printf("Generating row for variable...")
215-
generatedRow := generateRowForVariable(variableName, conceptValuesToPeopleCount, sortedConceptValues)
216-
rows = append(rows, generatedRow)
217-
}
218-
219-
return rows, nil
220-
}
221-
222170
func (u ConceptController) RetrieveAttritionTable(c *gin.Context) {
223-
sourceId, cohortId, conceptIds, cohortPairs, err := utils.ParseSourceIdAndCohortIdAndVariablesList(c)
171+
sourceId, cohortId, conceptIdsAndCohortPairs, err := utils.ParseSourceIdAndCohortIdAndVariablesAsSingleList(c)
224172
if err != nil {
225173
log.Printf("Error: %s", err.Error())
226174
c.JSON(http.StatusBadRequest, gin.H{"message": "bad request", "error": err.Error()})
@@ -245,7 +193,7 @@ func (u ConceptController) RetrieveAttritionTable(c *gin.Context) {
245193
breakdownStats, err := u.conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortId(sourceId, cohortId, breakdownConceptId)
246194
if err != nil {
247195
log.Printf("Error: %s", err.Error())
248-
c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving concept breakdown with filtered conceptIds", "error": err.Error()})
196+
c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving concept breakdown for given cohortId", "error": err.Error()})
249197
c.Abort()
250198
return
251199
}
@@ -255,32 +203,58 @@ func (u ConceptController) RetrieveAttritionTable(c *gin.Context) {
255203
headerAndNonFilteredRow, err := u.GenerateHeaderAndNonFilteredRow(breakdownStats, sortedConceptValues, cohortName)
256204
if err != nil {
257205
log.Printf("Error: %s", err.Error())
258-
c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving concept breakdown with filtered conceptIds", "error": err.Error()})
206+
c.JSON(http.StatusInternalServerError, gin.H{"message": "Error generating concept breakdown header and cohort rows", "error": err.Error()})
259207
c.Abort()
260208
return
261209
}
262-
263-
// append concepts to attrition table:
264-
conceptVariablesAttritionRows, err := u.GetConceptVariablesAttritionRows(sourceId, cohortId, conceptIds, breakdownConceptId, sortedConceptValues)
210+
otherAttritionRows, err := u.GetAttritionRowForConceptIdsAndCohortPairs(sourceId, cohortId, conceptIdsAndCohortPairs, breakdownConceptId, sortedConceptValues)
265211
if err != nil {
266212
log.Printf("Error: %s", err.Error())
267-
c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving concept breakdown with filtered conceptIds", "error": err.Error()})
213+
c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving concept breakdown rows for filter conceptIds and cohortPairs", "error": err.Error()})
268214
c.Abort()
269215
return
270216
}
271-
// append custom dichotomous items to attrition table:
272-
customDichotomousVariablesAttritionRows, err := u.GetCustomDichotomousVariablesAttritionRows(sourceId, cohortId, conceptIds, cohortPairs, breakdownConceptId, sortedConceptValues)
273-
if err != nil {
274-
log.Printf("Error: %s", err.Error())
275-
c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving concept breakdown with custom dichotomous variables (aka cohortpairs)", "error": err.Error()})
276-
c.Abort()
277-
return
217+
b := GenerateAttritionCSV(headerAndNonFilteredRow, otherAttritionRows)
218+
c.String(http.StatusOK, b.String())
219+
}
220+
221+
func (u ConceptController) GetAttritionRowForConceptIdsAndCohortPairs(sourceId int, cohortId int, conceptIdsAndCohortPairs []interface{}, breakdownConceptId int64, sortedConceptValues []string) ([][]string, error) {
222+
var otherAttritionRows [][]string
223+
for idx, conceptIdOrCohortPair := range conceptIdsAndCohortPairs {
224+
// attrition filter: run each query with an increasingly longer list of filterConceptIdsAndCohortPairs, until the last query is run with them all:
225+
filterConceptIdsAndCohortPairs := conceptIdsAndCohortPairs[0 : idx+1]
226+
227+
attritionRow, err := u.GetAttritionRowForConceptIdOrCohortPair(sourceId, cohortId, conceptIdOrCohortPair, filterConceptIdsAndCohortPairs, breakdownConceptId, sortedConceptValues)
228+
if err != nil {
229+
log.Printf("Error: %s", err.Error())
230+
return nil, err
231+
}
232+
otherAttritionRows = append(otherAttritionRows, attritionRow)
278233
}
234+
return otherAttritionRows, nil
235+
}
279236

280-
// concat all rows:
281-
var allVariablesAttritionRows = append(conceptVariablesAttritionRows, customDichotomousVariablesAttritionRows...)
282-
b := GenerateAttritionCSV(headerAndNonFilteredRow, allVariablesAttritionRows)
283-
c.String(http.StatusOK, b.String())
237+
func (u ConceptController) GetAttritionRowForConceptIdOrCohortPair(sourceId int, cohortId int, conceptIdOrCohortPair interface{}, filterConceptIdsAndCohortPairs []interface{}, breakdownConceptId int64, sortedConceptValues []string) ([]string, error) {
238+
filterConceptIds, filterCohortPairs := utils.GetConceptIdsAndCohortPairsAsSeparateLists(filterConceptIdsAndCohortPairs)
239+
breakdownStats, err := u.conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(sourceId, cohortId, filterConceptIds, filterCohortPairs, breakdownConceptId)
240+
if err != nil {
241+
return nil, fmt.Errorf("could not retrieve concept Breakdown for concepts %v dichotomous variables %v due to error: %s", filterConceptIds, filterCohortPairs, err.Error())
242+
}
243+
conceptValuesToPeopleCount := getConceptValueToPeopleCount(breakdownStats)
244+
variableName := ""
245+
switch convertedItem := conceptIdOrCohortPair.(type) {
246+
case int64:
247+
conceptInformation, err := u.conceptModel.RetrieveInfoBySourceIdAndConceptId(sourceId, convertedItem)
248+
if err != nil {
249+
return nil, fmt.Errorf("could not retrieve concept details for %v due to error: %s", convertedItem, err.Error())
250+
}
251+
variableName = conceptInformation.ConceptName
252+
case utils.CustomDichotomousVariableDef:
253+
variableName = convertedItem.ProvidedName
254+
}
255+
log.Printf("Generating row for variable with name %s", variableName)
256+
generatedRow := generateRowForVariable(variableName, conceptValuesToPeopleCount, sortedConceptValues)
257+
return generatedRow, nil
284258
}
285259

286260
func getSortedConceptValues(breakdownStats []*models.ConceptBreakdown) []string {

‎models/concept.go

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88

99
type ConceptI interface {
1010
RetriveAllBySourceId(sourceId int) ([]*Concept, error)
11+
RetrieveInfoBySourceIdAndConceptId(sourceId int, conceptId int64) (*ConceptSimple, error)
1112
RetrieveInfoBySourceIdAndConceptIds(sourceId int, conceptIds []int64) ([]*ConceptSimple, error)
1213
RetrieveInfoBySourceIdAndConceptTypes(sourceId int, conceptTypes []string) ([]*ConceptSimple, error)
1314
RetrieveBreakdownStatsBySourceIdAndCohortId(sourceId int, cohortDefinitionId int, breakdownConceptId int64) ([]*ConceptBreakdown, error)

‎tests/controllers_tests/controllers_test.go

+43-54
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,6 @@ var cohortDefinitionControllerNeedsDb = controllers.NewCohortDefinitionControlle
5757
// instance of the controller that talks to a mock implementation of the model:
5858
var cohortDefinitionController = controllers.NewCohortDefinitionController(*new(dummyCohortDefinitionDataModel))
5959

60-
var conceptController = controllers.NewConceptController(*new(dummyConceptDataModel), *new(dummyCohortDefinitionDataModel))
61-
6260
type dummyCohortDataModel struct{}
6361

6462
func (h dummyCohortDataModel) RetrieveDataBySourceIdAndCohortIdAndConceptIdsOrderedByPersonId(sourceId int, cohortDefinitionId int, conceptIds []int64) ([]*models.PersonConceptAndValue, error) {
@@ -129,11 +127,28 @@ func (h dummyCohortDefinitionDataModel) GetAllCohortDefinitions() ([]*models.Coh
129127
return nil, nil
130128
}
131129

130+
var conceptController = controllers.NewConceptController(*new(dummyConceptDataModel), *new(dummyCohortDefinitionDataModel))
131+
132132
type dummyConceptDataModel struct{}
133133

134134
func (h dummyConceptDataModel) RetriveAllBySourceId(sourceId int) ([]*models.Concept, error) {
135135
return nil, nil
136136
}
137+
138+
func (h dummyConceptDataModel) RetrieveInfoBySourceIdAndConceptId(sourceId int, conceptId int64) (*models.ConceptSimple, error) {
139+
conceptSimpleItems := []*models.ConceptSimple{
140+
{ConceptId: 1234, ConceptName: "Concept A"},
141+
{ConceptId: 5678, ConceptName: "Concept B"},
142+
{ConceptId: 2090006880, ConceptName: "Concept C"},
143+
}
144+
for _, conceptSimple := range conceptSimpleItems {
145+
if conceptSimple.ConceptId == conceptId {
146+
return conceptSimple, nil
147+
}
148+
}
149+
return nil, fmt.Errorf("concept id %d not found in mock data", conceptId)
150+
}
151+
137152
func (h dummyConceptDataModel) RetrieveInfoBySourceIdAndConceptIds(sourceId int, conceptIds []int64) ([]*models.ConceptSimple, error) {
138153
// dummy data with _some_ of the relevant fields:
139154
conceptSimple := []*models.ConceptSimple{
@@ -169,8 +184,8 @@ func (h dummyConceptDataModel) RetrieveBreakdownStatsBySourceIdAndCohortId(sourc
169184
}
170185
func (h dummyConceptDataModel) RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(sourceId int, cohortDefinitionId int, filterConceptIds []int64, filterCohortPairs []utils.CustomDichotomousVariableDef, breakdownConceptId int64) ([]*models.ConceptBreakdown, error) {
171186
conceptBreakdown := []*models.ConceptBreakdown{
172-
{ConceptValue: "value1", NpersonsInCohortWithValue: 4},
173-
{ConceptValue: "value2", NpersonsInCohortWithValue: 7},
187+
{ConceptValue: "value1", NpersonsInCohortWithValue: 4 - len(filterCohortPairs)}, // simulate decreasing numbers as filter increases - the use of filterCohortPairs instead of filterConceptIds is otherwise meaningless here...
188+
{ConceptValue: "value2", NpersonsInCohortWithValue: 7 - len(filterConceptIds)}, // simulate decreasing numbers as filter increases- the use of filterConceptIds instead of filterCohortPairs is otherwise meaningless here...
174189
}
175190
if dummyModelReturnError {
176191
return nil, fmt.Errorf("error!")
@@ -603,75 +618,49 @@ func TestGenerateHeaderAndNonFilterRow(t *testing.T) {
603618
}
604619
}
605620

606-
func TestGetConceptVariablesAttritionRows(t *testing.T) {
621+
func TestGetAttritionRowForConceptIdsAndCohortPairs(t *testing.T) {
607622
setUp(t)
608623
sourceId := 1
609624
cohortId := 1
610625
var breakdownConceptId int64 = 1
611-
conceptIds := []int64{1234, 5678, 2090006880}
612-
sortedConceptValues := []string{"value1", "value2"}
613-
614-
result, _ := conceptController.GetConceptVariablesAttritionRows(sourceId, cohortId, conceptIds, breakdownConceptId, sortedConceptValues)
615-
if len(result) != 3 {
616-
t.Errorf("Expected 3 data lines, found %d lines in total",
617-
len(result))
618-
t.Errorf("Lines: %s", result)
619-
}
620-
621-
expectedLines := [][]string{
622-
{"Concept A", "11", "4", "7"},
623-
{"Concept B", "11", "4", "7"},
624-
{"Concept C", "11", "4", "7"},
625-
}
626-
627-
i := 0
628-
for _, expectedLine := range expectedLines {
629-
if !reflect.DeepEqual(expectedLine, result[i]) {
630-
t.Errorf("header or non filter row line not as expected. \nExpected: \n%s \nFound: \n%s",
631-
expectedLine, result[i])
632-
}
633-
i++
634-
}
635-
}
626+
sortedConceptValues := []string{"value1", "value2", "value3"}
636627

637-
func TestGetCustomDichotomousVariablesAttritionRows(t *testing.T) {
638-
setUp(t)
639-
sourceId := 1
640-
cohortId := 1
641-
var breakdownConceptId int64 = 1
642-
conceptIds := []int64{1234, 5678, 2090006880}
643-
cohortPairs := []utils.CustomDichotomousVariableDef{
644-
{
628+
// mix of concept ids and CustomDichotomousVariableDef items:
629+
conceptIdsAndCohortPairs := []interface{}{
630+
int64(1234),
631+
int64(5678),
632+
utils.CustomDichotomousVariableDef{
645633
CohortId1: 1,
646634
CohortId2: 2,
647635
ProvidedName: "testA12"},
648-
{
636+
int64(2090006880),
637+
utils.CustomDichotomousVariableDef{
649638
CohortId1: 3,
650639
CohortId2: 4,
651640
ProvidedName: "testB34"},
652641
}
653642

654-
sortedConceptValues := []string{"value1", "value2", "value3"}
655-
656-
result, _ := conceptController.GetCustomDichotomousVariablesAttritionRows(sourceId, cohortId, conceptIds, cohortPairs, breakdownConceptId, sortedConceptValues)
657-
if len(result) != 2 {
658-
t.Errorf("Expected 3 data lines, found %d lines in total",
643+
result, _ := conceptController.GetAttritionRowForConceptIdsAndCohortPairs(sourceId, cohortId, conceptIdsAndCohortPairs, breakdownConceptId, sortedConceptValues)
644+
if len(result) != len(conceptIdsAndCohortPairs) {
645+
t.Errorf("Expected %d data lines, found %d lines in total",
646+
len(conceptIdsAndCohortPairs),
659647
len(result))
660648
t.Errorf("Lines: %s", result)
661649
}
662650

663651
expectedLines := [][]string{
664-
{"testA12", "11", "4", "7", "0"},
665-
{"testB34", "11", "4", "7", "0"},
652+
{"Concept A", "10", "4", "6", "0"},
653+
{"Concept B", "9", "4", "5", "0"},
654+
{"testA12", "8", "3", "5", "0"},
655+
{"Concept C", "7", "3", "4", "0"},
656+
{"testB34", "6", "2", "4", "0"},
666657
}
667658

668-
i := 0
669-
for _, expectedLine := range expectedLines {
659+
for i, expectedLine := range expectedLines {
670660
if !reflect.DeepEqual(expectedLine, result[i]) {
671661
t.Errorf("header or non filter row line not as expected. \nExpected: \n%s \nFound: \n%s",
672662
expectedLine, result[i])
673663
}
674-
i++
675664
}
676665
}
677666

@@ -849,8 +838,8 @@ func TestRetrieveAttritionTable(t *testing.T) {
849838
requestContext.Params = append(requestContext.Params, gin.Param{Key: "breakdownconceptid", Value: "2"})
850839
requestContext.Writer = new(tests.CustomResponseWriter)
851840
requestContext.Request = new(http.Request)
852-
requestBody := "{\"variables\":[{\"variable_type\": \"concept\", \"concept_id\": 2090006880}," +
853-
"{\"variable_type\": \"custom_dichotomous\", \"provided_name\": \"testABC\", \"cohort_ids\": [1, 3]}," +
841+
requestBody := "{\"variables\":[{\"variable_type\": \"custom_dichotomous\", \"provided_name\": \"testABC\", \"cohort_ids\": [1, 3]}," +
842+
"{\"variable_type\": \"concept\", \"concept_id\": 2090006880}," +
854843
"{\"variable_type\": \"custom_dichotomous\", \"cohort_ids\": [4, 5]}]}" // this one with no provided name (to test auto generated one)
855844
requestContext.Request.Body = io.NopCloser(strings.NewReader(requestBody))
856845
requestContext.Writer = new(tests.CustomResponseWriter)
@@ -862,9 +851,9 @@ func TestRetrieveAttritionTable(t *testing.T) {
862851
expectedLines := []string{
863852
"Cohort,Size,value1_name,value2_name",
864853
"dummy cohort name,13,5,8",
865-
"Concept C,11,4,7",
866-
"testABC,11,4,7",
867-
"ID_4_5,11,4,7",
854+
"testABC,10,3,7",
855+
"Concept C,9,3,6",
856+
"ID_4_5,8,2,6",
868857
}
869858
i := 0
870859
for _, expectedLine := range expectedLines {

0 commit comments

Comments
 (0)