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

Updated the observation mapper to use valueQuantity.code #248

Merged
merged 5 commits into from
Jun 4, 2024

Conversation

shubhamparikh927
Copy link
Contributor

@shubhamparikh927 shubhamparikh927 commented May 31, 2024

Overview

  • Updated the observation mapper to use valueQuantity.code
  • Updated the unit test case

How it was tested

  • Tested by running showcase app locally

Checklist

  • The title contains a short meaningful summary
  • I have added a link to this PR in the Jira issue
  • I have described how this was tested
  • I have included screen shots for changes that affect the user interface
  • I have updated unit tests
  • I have run unit tests locally
  • I have updated documentation (including README)

@semicolin
Copy link
Collaborator

This is a potentially high-impact change. I'm going to do some testing on this branch.

@semicolin
Copy link
Collaborator

I don't think we should merge this PR. It could result in inaccurate information being displayed in the chart. For example, the demo patient "Ocelot Apricot" has multiple hemoglobin observations. Some are measured in mmol/L and others are measured in g/dL. In this branch, they both show up on the same scale, which is labelled g/dL. The measurement of 15 mmol/L is misrepresented as 15 g/dL. Those are not equivalent. We would need to do unit conversion in order to display those on the same chart.

Instead, we should change the observation mappers to use resource.valueQuantity.code instead of resource.valueQuantity.unit for the scale label. That will eliminate duplicate layers that are caused by cosmetic differences in the human-readable display units without merging layers that really use different units of measure.

We should also continue troubleshooting the bugs that we see when there are two layers with the same name. We should be able to support multiple layers with the same name. Maybe it is actually an issue with the dataset labels being the same? Dataset labels do need to be unique due to chart.js requirement.

@shubhamparikh927 shubhamparikh927 changed the title Updated the data layer merge service to create hashcode using layer name Updated the observation mapper to use valueQuantity.code Jun 3, 2024
Copy link

sonarcloud bot commented Jun 4, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@shubhamparikh927 shubhamparikh927 merged commit dbaf57f into main Jun 4, 2024
6 checks passed
@shubhamparikh927 shubhamparikh927 deleted the charts-on-fhir-duplicate-layers branch June 4, 2024 08:22
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.

4 participants