feat: adds four new default FHIR resources to the mix #283
Conversation
| @@ -47,7 +47,6 @@ | |||
| {"path": "nodesByType('ContactDetail')", "method": "redact"}, | |||
| {"path": "nodesByType('ContactPoint')", "method": "redact"}, | |||
| {"path": "nodesByType('HumanName')", "method": "redact"}, | |||
| {"path": "nodesByType('Identifier')", "method": "redact"}, | |||
There was a problem hiding this comment.
This was previously a blanket injunction against using Identifier elements, because I'd only seen them in Resource.identifier contexts, where the EHR stores PHI identifiers like MRNs or other such business-case oriented identifiers (I think in Patients, the SSN goes there? Or maybe that's an extension, I don't remember.)
Anyway. Device has a version.component field that for some reason uses the Identifier element, so I can't blanket-ban it now. But it is also never allow-listed anywhere else. This line was just a second line of defense against mistakes. But now that we have a real use case, we can drop this.
cumulus_etl/deid/ms-config.json
Outdated
| {"path": "Immunization.reasonReference", "method": "keep"}, | ||
| {"path": "Immunization.isSubpotent", "method": "keep"}, | ||
| {"path": "Immunization.subpotentReason", "method": "keep"}, | ||
| {"path": "Immunization.education.publicationDate", "method": "keep", "comment": "we aren't keeping documentType or uri for fear of any PHI slipping in, so maybe these two dates aren't very useful, but better more data than less"}, |
There was a problem hiding this comment.
I'm still not 100% on whether it's safe to include documentType/uri -- they seem kinda harmless, but I also don't think they would be very valuable... 🤷
| "code" : { "text": "kept" }, | ||
| "subject" : { "reference": "Patient/x" }, | ||
| "encounter" : { "reference": "Encounter/x" }, | ||
| "effectiveDateTime" : "2023", |
There was a problem hiding this comment.
also effectivePeriod?
There was a problem hiding this comment.
For "variant" fields, I don't usually unit-test each possible variation. Only the "interesting" variations (like if we drop the string variant, but keep the others, I'll have a unit test for an arbitrary one that we keep and also a unit test for the dropped variant).
We can't put multiple variants in one test file because the MS tool would complain about invalid FHIR.
| "meta": { | ||
| "security": [{ | ||
| "code": "REDACTED", | ||
| "display": "redacted", | ||
| "system": "http://terminology.hl7.org/CodeSystem/v3-ObservationValue" | ||
| }] | ||
| }, |
There was a problem hiding this comment.
mostly for my own edification: how does this work on the MS side?
There was a problem hiding this comment.
Ah OK for context: this file is the expected output of the MS tool. It's input counterpart is in the input/ folder.
To your actual question: this bit here is inserted by the MS tool automatically (can't turn it off). I find this annoying, and pointless for Cumulus' purposes, so the ETL drops this field again when processing files.
But this unit test doesn't do the full ETL processing, it's just testing our ms-config.json file, so the field is still present.
| "manifestation" : [{ "text": "kept" }], | ||
| "onset" : "2023", | ||
| "severity" : "mild", | ||
| "exposureRoute" : { "text": "kept" } |
There was a problem hiding this comment.
note is dropped per the ms-config.json config. So it doesn't show up in this "expected output" file. Whether we should drop it or not, is open for discussion. But we tend to drop freeform text fields like that because of high risk of PHI in them, and it's not machine-readable, unless we wanted to NLP it.
There was a problem hiding this comment.
two comment/questions about nodes down in the test data that also apply here.
There was a problem hiding this comment.
Yeah this is the juicier file for sure 😄 Where all the redaction decisions live.
You mentioned note and effectivePeriod:
AllergyIntolerance.noteis missing from this file (not allow-listed) intentionally - but that's open to discussion.DiagnosticReport.effectivePeriodis picked up by keepingDiagnosticReport.effectivewhich keeps all the variants of that field.
Happy to talk more about each line in here - honestly, for safety's sake, maybe it's worth you and I going over this file's changes line by line sometime next week.
mikix
force-pushed
the
mikix/4-new-resources
branch
2 times, most recently
from
6bf54bc to
12aa139
Compare
New & default tasks: - allergyintollerance - device - diagnosticreport - immunization I've also modified some of the existing unit testing to be more specific about which tests to run, in the cases where it doesn't make sense to update tests when you add a new task.
mikix
force-pushed
the
mikix/4-new-resources
branch
from
12aa139 to
14ae0f7
Compare
New & default tasks:
I've also modified some of the existing unit testing to be more specific about which tests to run, in the cases where it doesn't make sense to update tests when you add a new task.
Checklist
docs/) needs to be updated