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

Refactor transformation helpers #1085

Merged
merged 5 commits into from
May 10, 2024
Merged

Conversation

basiliskus
Copy link
Contributor

@basiliskus basiliskus commented May 9, 2024

Refactor transformation helpers

  • Refactored to create more generic helper functions:
    • addMetaTag instead of addEtorTagToBundle
    • setMessageTypeCoding instead of convertToOmlOrder
  • Consolidated hapi helper methods in HapiHelper (removed HapiMessageConverterHelper and HapiOrderConverterHelper)
  • Moved logic from helper to transformation methods when unique to the transformation

Issue

#1024

Checklist

  • I have added tests to cover my changes
  • I have added logging where useful (with appropriate log level)
  • I have added JavaDocs where required
  • I have updated the documentation accordingly

@basiliskus basiliskus marked this pull request as ready for review May 9, 2024 22:36
Copy link

sonarcloud bot commented May 9, 2024

@jorg3lopez
Copy link
Contributor

I like that the logic is in transformation object. At lease we don't have to go looking else where for it. The only thing that I realize now is that this tightly couples the fhir library to each transformation object. I'm ok leaving it like this or adding the logic to the fhir humble object.

@basiliskus
Copy link
Contributor Author

I like that the logic is in transformation object. At lease we don't have to go looking else where for it. The only thing that I realize now is that this tightly couples the fhir library to each transformation object. I'm ok leaving it like this or adding the logic to the fhir humble object.

Right. I think it may be ok as the transformations deal with the inners of the bundle and it may be cumbersome to abstract them from the hapi library, but I'm sure we can make improvements in that direction. This could be a good topic for eng. block. I'll add it

@jorg3lopez
Copy link
Contributor

I like that the logic is in transformation object. At lease we don't have to go looking else where for it. The only thing that I realize now is that this tightly couples the fhir library to each transformation object. I'm ok leaving it like this or adding the logic to the fhir humble object.

Right. I think it may be ok as the transformations deal with the inners of the bundle and it may be cumbersome to abstract them from the hapi library, but I'm sure we can make improvements in that direction. This could be a good topic for eng. block. I'll add it

Yes. I also realize that the FHIR bundle dependency will be a part of the transformation object regardless of where we put the logic. The bundle will have to get passed to the helper function or we will iterate through it if we keep the logic in the transformation objects.

@jcrichlake
Copy link
Contributor

Small note to please move the magic strings to enums/ constants in the next PR, approving with that note

@basiliskus basiliskus merged commit a378de0 into main May 10, 2024
16 checks passed
@basiliskus basiliskus deleted the story/1024/refactor-converter-helper branch May 10, 2024 14:36
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