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

Update bundle and processing #83

Merged
merged 25 commits into from
Nov 9, 2022
Merged

Update bundle and processing #83

merged 25 commits into from
Nov 9, 2022

Conversation

stefpiatek
Copy link
Collaborator

@stefpiatek stefpiatek commented Oct 28, 2022

No rush on reviewing, but sending off while I remember. Can wait until you start back on this project, a week on monday

  • Very large PR (sorry!), on encountering real data GOSH found a substantial number of changes were required to the FHIR bundle and this is WIP from that. Probably easiest way to review is to go through each commit.
  • Probably easiest way to go through is per commit, happy also to talk you through it

Parts left to do:

  • Use panelapp conditions instead of snomed ct
  • Use panelapp code along with the specimen id to make a unique identifier

closes #33
closes #53

@stefpiatek stefpiatek requested a review from jhughes982 October 28, 2022 16:23
@stefpiatek stefpiatek temporarily deployed to test-env October 28, 2022 16:23 Inactive
@stefpiatek stefpiatek temporarily deployed to test-env November 7, 2022 13:39 Inactive
@stefpiatek stefpiatek marked this pull request as ready for review November 9, 2022 11:24
@stefpiatek stefpiatek temporarily deployed to test-env November 9, 2022 11:25 Inactive
Copy link
Collaborator

@jhughes982 jhughes982 left a comment

Choose a reason for hiding this comment

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

Looking good. Just a few comments to address.

Also:
At some point I did manage to produce an error when submitting the form, and the console complained of an uncaught error inside this function: const bundle = bundleRequest(values, reportedHened). However, I'm not sure exactly what triggered this error and I wasn't able to reproduce it afterwards. It might have just been a quirk during development (e.g. the dev server refreshed and it messed up the data). But I thought I'd mention it here. Wrapping the function in a try catch might be worth it perhaps?

@stefpiatek stefpiatek temporarily deployed to test-env November 9, 2022 14:37 Inactive
@stefpiatek stefpiatek temporarily deployed to test-env November 9, 2022 14:43 Inactive
@stefpiatek stefpiatek merged commit 300fae6 into main Nov 9, 2022
@stefpiatek stefpiatek deleted the bundle_update branch November 9, 2022 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants