🏷️ 1141 Add Exception Manifest to Donor tsv - Githubissues

icgc-argo / argo-clinical

Clinical data submission for ARGO programs.

GNU Affero General Public License v3.0

2 stars 0 forks source link

🏷️ 1141 Add Exception Manifest to Donor tsv #1178

Closed demariadaniel closed 2 months ago

demariadaniel commented 2 months ago

Link to Issue

Description

Adds Exception Manifest to getDonorDataByIdAsTsvsInZip at /donors/tsv
Allows downloading exceptions from multiple programs if files from multiple programs are selected (which is possible, tested in QA) Example output in ticket

Checklist

Type of Change

[ ] Bug
[ ] Refactor
[x] New Feature
[ ] Release Candidate

Checklist before requesting review:

[ ] Check branch (code change PRs go to develop not master)
[ ] Check copyrights for new files
[ ] Manual testing
[ ] Regression tests completed and passing (double check number of tests).
[ ] Spelling has been checked.
[ ] Updated swagger docs accordingly (check it's still valid)
[ ] Set validationDependency in meta tag for Argo Dictionary fields used in code

ciaranschutte commented 2 months ago

I don't think any of this as TS usage is correct. I understand you want to narrow the type but you're not actually checking to make sure the value is of that type.

We are telling TS already that these things are of a certain type eg. https://github.com/icgc-argo/argo-clinical/blob/develop/src/clinical/clinical-entities.ts#L115 and this usage negates that.

I believe the fix is to add explicit check if TS can't infer. eg. if you're saying as string make sure typeof foo === 'string'

calling TS gods for 2nd opinion @joneubank