Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FM2-513: Observations classed as LabSet to be categorised in the 'laboratory' category #430

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Ruhanga
Copy link
Member

@Ruhanga Ruhanga commented Sep 28, 2022

Description of what I changed

I've added a liquibase changeset that maps LabSet concept class to laboratory category.

Issue I worked on

see https://issues.openmrs.org/browse/FM2-513

Checklist: I completed these to help reviewers :)

  • My IDE is configured to follow the code style of this project.

  • I have added tests to cover my changes. (If you refactored
    existing code that was well tested you do not have to add tests)

  • I ran mvn clean package right before creating this pull request and
    added all formatting changes to my commit.

  • All new and existing tests passed.

  • My pull request is based on the latest changes of the master branch.

@codecov
Copy link

codecov bot commented Sep 28, 2022

Codecov Report

Base: 79.70% // Head: 79.70% // No change to project coverage 👍

Coverage data is based on head (4832e1d) compared to base (d0b34f7).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff            @@
##             master     #430   +/-   ##
=========================================
  Coverage     79.70%   79.70%           
  Complexity     2489     2489           
=========================================
  Files           232      232           
  Lines          6774     6774           
  Branches        815      815           
=========================================
  Hits           5399     5399           
  Misses          873      873           
  Partials        502      502           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Ruhanga Ruhanga changed the title FM2-513: Observations classed as LabSet to be categorised in the 'lab… FM2-513: Observations classed as LabSet to be categorised in the 'laboratory' category Sep 29, 2022
@itstanany
Copy link

Thanks for submitting this pull request! It addresses a limitation in how observations are categorized within the OpenMRS FHIR2 module. Previously, only observations with concepts classified as "Test" were categorized as "laboratory," excluding those classified as "LabSet" (lab panels).

The proposed solution using a Liquibase changeset to update the fhir_observation_category_map table and map the "LabSet" concept class to the "laboratory" category seems like a straightforward and effective approach.

Here are some additional points to consider:

Testing: While the pull request itself might not involve complex logic changes, consider if there's a way to add a basic test to ensure the Liquibase changeset is applied correctly and the mapping is functional after deployment.

Documentation: Double-check if there's any relevant documentation that could be updated to reflect this change in how LabSet observations are categorized. This could help with future maintenance and understanding of the system's behavior.

Overall, this pull request offers a clear solution to a documented issue. The use of a Liquibase changeset simplifies the implementation and likely reduces the risk of errors. Adding a basic test and potentially updating documentation could further strengthen this contribution.

@ibacher
Copy link
Member

ibacher commented Mar 28, 2024

My preference here would be to actually merge in the Iniz PR for this domain, so I can remove the metadata completely from the Liquibase scripts.

@Ruhanga Ruhanga marked this pull request as ready for review October 3, 2024 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants