-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add new stats endpoint #124
Conversation
Please find the detailed integration test report here Please find the ci env pod logs here |
Please find the ci env pod logs here |
Please find the ci env pod logs here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @tianj7 . I added some minor improvement suggestions
utils/parsing.go
Outdated
conceptIdsAndValues, cohortPairs := GetConceptIdsAndCohortPairsAsSeparateLists(conceptIdsAndCohortPairs) | ||
conceptIds := ExtractConceptIdsFromCustomConceptVariablesDef(conceptIdsAndValues) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe move ExtractConceptIdsFromCustomConceptVariablesDef
into GetConceptIdsAndCohortPairsAsSeparateLists
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This helper function is used elsewhere which is why I extracted it out.
cohort-middleware/controllers/cohortdata.go
Line 304 in 000e75b
conceptIds := utils.ExtractConceptIdsFromCustomConceptVariablesDef(conceptIdsAndValues) |
cohort-middleware/controllers/cohortdata.go
Line 154 in 000e75b
conceptIds := utils.ExtractConceptIdsFromCustomConceptVariablesDef(conceptIdsAndValues) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but you can still use it inside GetConceptIdsAndCohortPairsAsSeparateLists
, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ExtractConceptIdsFromCustomConceptVariablesDef
is created to give a convenient way to plug things into existing functions which only support concept ids to limit the scope of this PR.
GetConceptIdsAndCohortPairsAsSeparateLists
is used in ParseConceptIdsAndDichotomousDefs
which in turn is used in the new stat endpoint, so we need them to pass back both ids and values. If we use 'ExtractConceptIdsFromCustomConceptVariablesDef inside GetConceptIdsAndCohortPairsAsSeparateLists
then we will loss the value part.
Please find the ci env pod logs here |
Please find the detailed integration test report here Please find the ci env pod logs here |
Please find the detailed integration test report here Please find the ci env pod logs here |
https://ctds-planx.atlassian.net/browse/VADC-1576
New Features
Add new stats endpoint for cohort and concepts