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

Parse reference ranges that use < and > #450

Merged
merged 8 commits into from
Mar 15, 2024

Conversation

jean-the-coder
Copy link
Contributor

Description

Right now only the reference range's high and low attributes are checked which leads to reference ranges in text format not being parsed or displayed. There are also issues displaying when only a low is set as the chart library doesn't know how to plot from <number> to null.

This PR updates how we parse Observation reference ranges so that both object forms and string forms of these attributes are parsed and displayed correctly. I've also moved the bar chart code into its own component so that it can be used for both the resource observation and the report labs observation without duplicating code.

Fixes #440

Changes

  • Added vscode config
  • Added Fishery for creating test models easier
  • Updated chart.js and test dependencies
  • Fixed some deprecations in _mixins.scss because the deprecation warnings were getting annoying
  • Updated the Observation model with logic to parse reference ranges
  • Updated the Observation model to try parsing valueQuantity.text if valueQuantity.value isn't set
    • Note: this will not work for everything as it is just doing a quick parseFloat() which will error out if the text isn't only a string. I plan to fix this in my next PR. This PR is large enough as is so I figured better to get its review started while I work on this functionality.
  • Pulled out bar chart logic into new ObservationBarChartComponent
    • Updated tooltips to display range/value with unit in a more human readable way instead of as an array
  • Updated ObservationComponent and ReportLabsObservationComponent to use ObservationBarChartComponent

Updated dependencies

Production dependencies

  • chart.js: ^4.0.1 -> ^4.4.2

Dev dependencies

  • fishery
  • jasmine-core
  • jasmine-spec-reporter
  • karma
  • karma-chrome-launcher
  • karma-coverage
  • karma-jasmine
  • karma-jasmine-html-reporter

Copy link
Member

@AnalogJ AnalogJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 question around the new Fishery factory pattern.

I'm not a huge fan of embedding the actual "base" observations in the typescript file:

  • its hard to update, most test observations we'll find in the wild are JSON blobs, which are easy to copy paste into a new file, but converting to a TS Object is a bit annoying.
  • we only have 1 "base", I wonder if it'd be possible to import from any JSON fixture.

Other than that, fantastic work!

@AnalogJ AnalogJ merged commit bcffbb4 into fastenhealth:main Mar 15, 2024
4 checks passed
@jean-the-coder jean-the-coder deleted the reference-range-parsing branch March 15, 2024 23:06
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.

[Bug]: Doesn't Display Reference Range on Chart if range listed as "<" value
2 participants