OLMIS-5415: Added check for the isFhirLocationOwner flag in fhir resources

Activity

FEOLMIS-3346 44

Keyboard shortcuts  
  • Summarize the review outcomes (optional)
     
    #permalink

    Details

    Warning: no files are visible, they have all been filtered.
    Participant Role Time Spent Comments Latest Comment
    Author 1h 39m 23 Yes we have a test for that: shouldRejectIfFhirFlagWasCha...
    Reviewer - Complete 34m 3 I love how detailed they are.
    Reviewer - 66% reviewed 10m    
    Reviewer - Complete 30m 2 The properties defined in this interface don't seem to ha...
    Reviewer - Complete 1h 8m 1 is this required?
    Reviewer - Complete 1h 40m 15 Is there a test for update where the user sent a request,...
    Total   5h 40m 44  
    #permalink

    Objectives

    There are no specific objectives for this review.

    Branches in review

    #permalink

    Issues Raised From Comments

    Key Summary State Assignee
    #permalink

    General Comments

    Chongsun Ahn

    I looked at the validation logic and I'm not sure it's correct. We may need t...

    I looked at the validation logic and I'm not sure it's correct. We may need to discuss.

    Chongsun Ahn

    I'm not sure if you've taken this into account, but when updating a facility ...

    I'm not sure if you've taken this into account, but when updating a facility or geo zone, we'll need to be "smart" about it. Example: FHIR service updates a facility. We need to ensure properties that FHIR doesn't care about, like supported programs, facility type, go live date, etc. are not overwritten.

    Łukasz Lewczyński

    I started thinking how to get data that should not be changed to FHIR service...

    I started thinking how to get data that should not be changed to FHIR service. For example, if a user creates a location which is a geo zone how could I get geo level from openlmis. Same for facility's type, operator and so on.

    Chongsun Ahn

    You mean if something outside of OpenLMIS POSTs to /hapifhir/Location, which ...

    You mean if something outside of OpenLMIS POSTs to /hapifhir/Location, which is a geo zone and needs to be created in reference data?

    Łukasz Lewczyński

    Maybe a better example will be with a facility/location that in the partOf ha...

    Maybe a better example will be with a facility/location that in the partOf has a reference to a geo zone - which exists in the system. Because the reference will have an FHIR's ID how to get an OpenLMIS's ID of the resource.

    Łukasz Lewczyński

    On the other hand, what type should be set for a facility that has been creat...

    On the other hand, what type should be set for a facility that has been created by FHIR service? From what I know the FHIR location has no information about the type. Same for other fields that are required but data are not stored in FHIR location:

    • GeographicZone.level
    • Facility.type
    • Facility.enabled - probably always true

    Chongsun Ahn

    From what I can tell, it is possible to create a Location resource in FHIR us...

    From what I can tell, it is possible to create a Location resource in FHIR using a specified UUID. If there is a corresponding OpenLMIS ID from reference data, we can use that. Or if it is from an external system, the system can generate a UUID and use that to save to FHIR.

    For the others, it does seem like we may need those fields as identifiers in order to save the corresponding resource in reference data.

    Łukasz Lewczyński

    on option could be to allow send a not complete request if the flag is set so...

    on option could be to allow send a not complete request if the flag is set so the backend would retrieve old values from the database. In this case, we would need to change validator to verify that those values are null

    /src/.../web/FacilityControllerIntegrationTest.java Changed
    /src/.../web/GeographicZoneControllerIntegrationTest.java Changed
    /src/main/.../domain/Facility.java Changed 5
    /src/main/.../domain/FhirLocation.java Added
    /src/main/.../domain/FhirResource.java Deleted 2
    Open in IDE #permalink
    /src/main/.../domain/GeographicLevel.java Changed
    Open in IDE #permalink
    /src/main/.../domain/GeographicZone.java Changed
    /src/main/.../dto/GeographicZoneSimpleDto.java Changed
    /src/.../repository/GeographicLevelRepository.java Changed
    Open in IDE #permalink
    /src/.../service/GeographicZoneBuilder.java Added
    Open in IDE #permalink
    /src/.../messagekeys/FacilityMessageKeys.java Changed
    Open in IDE #permalink
    /src/.../messagekeys/GeographicZoneMessageKeys.java Changed
    Open in IDE #permalink
    /src/main/.../messagekeys/MessageKeys.java Changed
    /src/main/.../validate/FacilityValidator.java Changed
    Open in IDE #permalink
    /src/.../validate/FhirLocationValidator.java Added 17
    /src/.../validate/FhirResourceValidator.java Deleted 2
    Open in IDE #permalink
    /src/.../validate/GeographicZoneValidator.java Added
    Open in IDE #permalink
    /src/.../web/GeographicZoneController.java Changed
    /src/main/resources/messages_en.properties Changed
    /src/test/.../domain/FhirLocationTest.java Added
    /src/test/.../domain/GeographicZoneTest.java Changed
    Open in IDE #permalink
    /src/.../service/GeographicZoneBuilderTest.java Added
    Open in IDE #permalink
    /src/.../testbuilder/FacilityDataBuilder.java Changed 5
    /src/.../testbuilder/GeographicZoneDataBuilder.java Changed
    /src/.../testbuilder/ProgramDataBuilder.java Changed
    Open in IDE #permalink
    /src/.../validate/FacilityValidatorTest.java Changed 2
    Open in IDE #permalink
    /src/.../validate/FhirLocationValidatorTest.java Added 2
    Open in IDE #permalink
    /src/.../validate/FhirResourceValidatorTest.java Deleted
    Open in IDE #permalink
    /src/.../validate/GeographicZoneValidatorTest.java Added 1
    Open in IDE #permalink
    /CHANGELOG.md Changed
    /Jenkinsfile Changed

    Review updated: Reload | Ignore | Collapse

    You cannot reload the review while writing a comment.

    Log time against