Add serializer for DataType retrieval#36
Conversation
| ) | ||
| throw new NoSuchResourceException("No parameter named concepts was given.") | ||
| } | ||
| if (params.get('concepts') == "") { |
There was a problem hiding this comment.
What happens when it is null? You can also use StringUtils.isEmpty() from apache.commons.lang to test for both in one go
There was a problem hiding this comment.
I tested it with "/v1/export/datatypes?concepts=" and "/v1/export/datatypes?concepts". Both return "Parameter concepts has no value."
| def conceptArguments = jsonSlurper.parseText(conceptParameters) | ||
| List dataTypes = [] | ||
| int cohortNumber = 1 | ||
| conceptArguments.each { it -> |
There was a problem hiding this comment.
There is quite some logics in this call, preferably you would put as much transformation and business logic in a Service. Leaving Http handling and top level control flow to the Controler
| cohortNumber += 1 | ||
| } | ||
| respond(restExportService.formatDataTypes(datatypes)) | ||
| respond(dataTypes) |
There was a problem hiding this comment.
stylistic remark: in this call you use brackets - in the other one you use groovy bracketless call (index call). This is inconsistent, would look better to use just one way everywhere. (but this is not a big deal)
| def cohortsMap = [:] | ||
| dataTypeRetrieved.OntologyTermsMap.each { ID, terms -> | ||
| terms.collect { term -> | ||
| if (ID in cohortsMap.keySet()) { |
There was a problem hiding this comment.
Could you describe the desired control flow here? I am not sure if I get it
There was a problem hiding this comment.
The frontend wants to know which concepts are in which cohort. Now a cohort can only contain one study, so the most logical thing to do would be to differentiate between studies. However in the near future cross studies will be added, which would result in this serialiser not functioning properly. So there needed to be a way to easily differentiate between the cohorts. This way it also supports multiple cohorts, which is also planned for the future. So based on (cohort)ID I create the structure for cohorts.
| String noPatients = "There are no saved patient sets by $currentUser.username" | ||
| def patientSets = queriesResource.getQueryResultsSummaryByUsername(currentUser.getUsername() ) | ||
| patientSets = patientSets == null ? noPatients : patientSets | ||
| respond patientSets |
There was a problem hiding this comment.
Does it wrap the noPatients string in a JSON object here?
Why not return an empty Json object instead?
Jarvanerp
force-pushed
the
gb-dev-data-export
branch
from
8b0d78f to
4a4784e
Compare
2 participants
Along with:
thehyve/transmart-core-db#66