Appointment Mapper#11
Conversation
SatwikSShanbhag
left a comment
There was a problem hiding this comment.
Plz look at the comments
| dbMappings.setFindIcdSnomedMapping(repositories.getIcdSnomedRepository().getIcdSnomedMap()); | ||
| dbMappings.setFindOrphaSnomedMapping( | ||
| repositories.getOrphaSnomedMappingRepository().getOrphaSnomedMap()); | ||
| dbMappings | ||
| .getOmopConceptMapWrapper() | ||
| .setFindValidSnomedConcept( | ||
| repositories.getConceptRepository().findValidConceptId(VOCABULARY_SNOMED)); | ||
| dbMappings | ||
| .getOmopConceptMapWrapper() | ||
| .setFindValidIPRDConcept( | ||
| repositories.getConceptRepository().findValidConceptId(VOCABULARY_IPRD)); | ||
| dbMappings.getOmopConceptMapWrapper().setFindValidWHOConcept( | ||
| repositories.getConceptRepository().findValidConceptId(VOCABULARY_WHO) | ||
| ); | ||
| dbMappings | ||
| .getOmopConceptMapWrapper() | ||
| .setFindValidIcd10GmConcept( | ||
| repositories.getConceptRepository().findValidConceptId(VOCABULARY_ICD10GM)); | ||
| } | ||
| dbMappings.setFindHardCodeConcept( | ||
| repositories.getSourceToConceptRepository().sourceToConceptMap()); | ||
| } |
There was a problem hiding this comment.
can you set only those things whose getters are used in mapper. remove unwanted setters.
| var wrapper = new OmopModelWrapper(); | ||
| var appointmentLogicalId = fhirReferenceUtils.extractId(srcAppointment); | ||
| if (Strings.isNullOrEmpty(appointmentLogicalId)){ | ||
| return null; |
There was a problem hiding this comment.
if you are skipping the resource keep that count and show it in summary log with reason
| } | ||
| var appointmentStatus = srcAppointment.getStatusElement().getValueAsString(); | ||
| if (Strings.isNullOrEmpty(appointmentStatus)){ | ||
| return null; |
There was a problem hiding this comment.
same as above and also it will be good if status is missing in the resource we can add custom valid status.
There was a problem hiding this comment.
Adding custom status can be dangerous in this case because we can't say that the appointment is booked or done directly from the code it should come from the resource itself. If the status does not exist it is better to skip it.
| var createdDateTime = srcAppointment.getCreated().toInstant().atZone(ZoneId.systemDefault()).toLocalDateTime(); | ||
| var appointmentDateTime = srcAppointment.getStart().toInstant().atZone(ZoneId.systemDefault()).toLocalDateTime(); |
There was a problem hiding this comment.
are we sure that srcAppointment.getCreated() and srcAppointment.getStart() always be non-null?
can we check that first and then assign the value to variable even we can skip if those values are not found
| if (createdDateTime == null || appointmentDateTime == null){ | ||
| return null; | ||
| } | ||
| var appointment = org.miracum.etl.fhirtoomop.model.omop.Appointment.builder() |
There was a problem hiding this comment.
org.miracum.etl.fhirtoomop.model.omop import this above
There was a problem hiding this comment.
This cannot be done because these will cause the conflict between the fhir appointment resource import.
| var provenanceLogicalId = fhirReferenceUtils.extractId(srcProvenance); | ||
| var questionnaireResponseLogicalId = fhirReferenceUtils.extractId(srcProvenance.getEntityFirstRep().getWhat().getReferenceElement().getResourceType(),srcProvenance.getEntityFirstRep().getWhat().getReferenceElement().getIdPart()); | ||
| if (Strings.isNullOrEmpty(provenanceLogicalId)){ | ||
| return null; |
There was a problem hiding this comment.
same keep count of skipped resources
| } | ||
| if (!srcProvenance.hasTarget()){ | ||
| return null; | ||
| } |
There was a problem hiding this comment.
same keep count of skipped resources
| var resourceType = provenanceTarget.getReferenceElement().getResourceType(); | ||
| var resourceId = provenanceTarget.getReferenceElement().getIdPart(); | ||
| var resourceLogicalId = fhirReferenceUtils.extractId(resourceType, resourceId); | ||
| var combinedId = resourceLogicalId + ";" + questionnaireResponseLogicalId; |
There was a problem hiding this comment.
use string concate method here
| import org.miracum.etl.fhirtoomop.model.omop.ConditionOccurrence; | ||
| import org.miracum.etl.fhirtoomop.model.omop.DeviceExposure; | ||
| import org.miracum.etl.fhirtoomop.model.omop.DrugExposure; | ||
| import org.miracum.etl.fhirtoomop.model.omop.Measurement; | ||
| import org.miracum.etl.fhirtoomop.model.omop.OmopObservation; | ||
| import org.miracum.etl.fhirtoomop.model.omop.Person; | ||
| import org.miracum.etl.fhirtoomop.model.omop.ProcedureOccurrence; | ||
| import org.miracum.etl.fhirtoomop.model.omop.Provider; | ||
| import org.miracum.etl.fhirtoomop.model.omop.VisitDetail; | ||
| import org.miracum.etl.fhirtoomop.model.omop.VisitOccurrence; | ||
| import org.miracum.etl.fhirtoomop.model.omop.Specimen; | ||
| import org.miracum.etl.fhirtoomop.model.omop.*; |
There was a problem hiding this comment.
remove all import only keep the import of model which is used in wrapper
SatwikSShanbhag
left a comment
There was a problem hiding this comment.
Looks good to me have a look at one comment
| if (createdDateTime == null || appointmentDateTime == null){ | ||
| return null; | ||
| } |
There was a problem hiding this comment.
wherever you are skipping the resource add counter
|
Can we also add
|
joiskash
left a comment
There was a problem hiding this comment.
Thanks, please address the comments.
| .next(stepProcessProvenance) | ||
| .next(stepProcessAppointments) |
There was a problem hiding this comment.
| .next(stepProcessProvenance) | |
| .next(stepProcessAppointments) | |
| .next(stepProcessProvenance) | |
| .next(stepProcessAppointments) |
| IGenericClient client, | ||
| IParser fhirParser) { | ||
|
|
||
| var resourceType = "Appointment"; |
There was a problem hiding this comment.
| var resourceType = "Appointment"; | |
| var resourceType = ResourceType.Appointment.getDisplay(); |
| if (StringUtils.isBlank(fhirBaseUrl)) { | ||
| return createResourceReader(resourceType, dataSource); | ||
| } | ||
| return fhirServerItemReader(client, fhirParser, ResourceType.APPOINTMENT.getDisplay(), ""); |
There was a problem hiding this comment.
Please reuse the variable resourceType set above
| import org.springframework.util.StopWatch; | ||
| import javax.sql.DataSource; | ||
|
|
||
| import static org.miracum.etl.fhirtoomop.Constants.*; |
There was a problem hiding this comment.
These import is not being used anywhere. Removing it.
| .scheduledDateTime(appointmentDateTime) | ||
| .createdDateTime(createdDateTime) | ||
| .appointmentStatus(appointmentStatus) | ||
| .fhirLogicalId(appointmentLogicalId).build(); |
There was a problem hiding this comment.
Let us also please consider the appointment description
| import lombok.Data; | ||
| import lombok.NoArgsConstructor; | ||
|
|
||
| import javax.persistence.*; |
There was a problem hiding this comment.
Please fix this import statement
| select * , | ||
| split_part(data_two, ';', 2) as questionnaire_response_logical_id, | ||
| split_part(data_two, ';', 1) as appointment_logical_id | ||
| from cds_etl_helper.post_process_map ppm where "type" = 'PROVENANCE' and data_one = 'Appointment' |
There was a problem hiding this comment.
Please check if Appointment should be APPOINTMENT to maintain uniformity in variable naming.
There was a problem hiding this comment.
date_one is supposed to be in lowercase in the post_process_map table because it can also contain not only texts but other type of data like date and date time.
SatwikSShanbhag
left a comment
There was a problem hiding this comment.
please address the comment.
| noFhirReferenceCounter.increment(); | ||
| return null; | ||
| } | ||
| var appReasonList = srcQuestionnaireResponse.getItem().stream().filter(questionnaireResponseItemComponent -> questionnaireResponseItemComponent.getLinkId().equals("service-type")).toList(); |
There was a problem hiding this comment.
can we change variable name as appointmentReasonList to avoid confusion.
also define a constant for service-type and check whether we can process appointment reason thing only for schedule appointment questionnaire response
thanks
SatwikSShanbhag
left a comment
There was a problem hiding this comment.
Looks good to me.
Description:
I have created a custom schema "Appointment" to convert the FHIR Appointment resources to OMOP. The schema for "Appointment" table looks like these:
appointment_id -> auto generated id
care_site_id -> care site id where the appointment was scheduled
person_id -> person id for whom the appointment was scheduled
visit_occurrence_id -> visit that the appointment is linked to
scheduled_date_time -> Appointment scheduled datetime
created_datetime -> Appointment created datetime
appointment_status -> Scheduled, booked, completed, etc..
fhir_logical_id -> FHIR Resource Id.
questionnaire_response_id -> questionnaire response id for the appointment resource
appointment_description -> description of the appointment from the appointment resource
appointment_reason -> reason for the appointment
I have added the AppointmentMapper and AppointmentStepListener and other necessary files just following the ETL pipeline codebase approach like for other tables.
Initially the appointment table will be filled without the person_id, visit_occurrence_id and care_site_id. In the post process I have wrote a script post_process_provenance_appointment.sql which will update the entries in the appointment table with person_id, visit_occurrence_id and care_site_id.