FI-2832 Add multi-version support #113
Conversation
…cible/fhir_models into FI-2832-version-support
…cible/fhir_models into FI-2832-version-support
lib/fhir_models/bootstrap/json.rb
Outdated
| end | ||
|
|
||
| def self.module_version | ||
| FHIR.module_version |
There was a problem hiding this comment.
I don't understand these methods. They don't look like they will return the correct version.
There was a problem hiding this comment.
What I was trying to accomplish here was deferring the version to the FHIR module whenever methods are called through FHIR instead of through a model. i.e., when making a new resource from a json/xml, we can call FHIR::Account.new(*payload*) and the version will be inferred from the Account class. But if you run FHIR::Json.from_json (as we do, at least in our unit tests), it has no class to find the version from. The FHIR module defaults to R4 and these just retrieve that.
That being said, I'm sure there is a cleaner way to model this behavior. If there is a cleaner inheritance structure that is preferred, I can make that change.
There was a problem hiding this comment.
But this means FHIR.from_json will always create an R4 resource and we don't have a way to do something like FHIR::R4B.from_json to create an R4B resource.
There was a problem hiding this comment.
I believe I've made the changes to allow what you are proposing, but the inheritance looks a little funky. I would appreciate a closer look at r4/fhir_ext/json.rb to make sure that seems alright, especially the set up of FHIR::Json to default to r4.
lib/fhir_models/bootstrap/model.rb
Outdated
| elsif FHIR::PRIMITIVES.include?(datatype) | ||
| primitive_meta = FHIR::PRIMITIVES[datatype] | ||
| elsif module_version::PRIMITIVES.include?(datatype) | ||
| primitive_meta = module_version::PRIMITIVES[datatype] |
There was a problem hiding this comment.
Line 219 needs updating also, right?
There was a problem hiding this comment.
Currently primitive? is not versioned and just under the FHIR module. I think it makes sense to make it a versioned method in case primitives change from version to version.
So, it doesn't currently need an update, but we can change primitive to be versioned, and then it would need to be.
There was a problem hiding this comment.
At a glance, I don't think primitive? supports all of the types in PRIMITIVES.
There was a problem hiding this comment.
You're right, it was missing canonical, uuid, and url. each case has been added now. I'm still wondering if maybe we want this to be a versioned method because that might change from version to version?
There was a problem hiding this comment.
I don't think it's worth spending time on now. If it's an issue it can be addressed later.
|
There are still some comments that were collapsed and not addressed.
|
Summary
This PR add support to generate FHIR models for mulitple FHIR versions.
There is no update for fhir_client at this time. As we discussed, we will redeign fhir_client later.
Change Log
generatorfoldergeneratedfolderigfolder for NPM packagetask.raketo use the new generator class ingeneratorfolder.