OCA / vertical-medical

Open Source Healthcare System for Odoo
GNU Affero General Public License v3.0
263 stars 278 forks source link

[ADD] Procedure #186

Closed rgarnau closed 6 years ago

rgarnau commented 7 years ago

This module adds procedures and procedure requests. A Procedure (https://www.hl7.org/fhir/procedure.html) represents an action that is or was performed on a patient whereas a Procedure Request (https://www.hl7.org/fhir/procedurerequest.html) is a record of a request for a Procedure.

This module depends on:

RequestGroup #185 is used to represent a group of optional activities (ProcedureRequests) that may be performed for a specific patient or context. This resource is often the result of applying a specific PlanDefinition #184 to a particular patient throught the CarePlan (https://www.hl7.org/fhir/careplan.html).

@lasley @obulkin we will be glad if you could have a look at our work and if it would be possible to meet in order to discuss our approach implementing this? Thanks!

@jbeficent

This module is still 'Work in progress' as tests are missing.

lasley commented 7 years ago

@rgarnau - hope you don't mind - I edited your comment to make the PR dependencies check boxes. Makes it a bit easier to see from the high level view because they become tasks

obulkin commented 7 years ago

Overall, this looks good and does a commendable job of sticking to the HL7 specs. I'm happy to review more closely once it's finished, but in the meantime, here are some high-level thoughts:

rgarnau commented 7 years ago

Hi @obulkin. This is how it now looks Procedure after your review:

@jbeficent

obulkin commented 7 years ago

Thanks for the changes, @rgarnau.

I'm still concerned with the relationship between the three modules. Requests and procedures are quite useful on their own without the need to introduce request groups, and the same is true for request groups and plans, so the dependency chain should reflect this. Your config approach takes care of this from a user standpoint for request groups and can probably be extended to plans as well. However, the end result is that three modules need to be installed in cases where one should be enough and that extra logic needs to be added, which makes the code harder to maintain. Unless you have a specific business reason for organizing things in this way, I think it's best to bite the bullet and rearrange the code before even more modules are built on top of it.

As for priority, I see that procedures still have an independent field that can be changed after creation, bringing it out of alignment with the associated procedure request's priority. I don't think this is desirable behavior and would recommend just making it a read-only related field.

lasley commented 6 years ago

@obulkin - can you please check in https://github.com/eficent/vertical-medical/tree/11.0-dev to see if your concerns from above are still valid?

obulkin commented 6 years ago

@lasley - The approach there is different, but the same sort of issues are present:

More generally, the overall organization feels unnecessarily convoluted and fragmented, but to be fair, this is how I feel about the underlying FHIR spec as well.

rgarnau commented 6 years ago

Hi @obulkin

@lasley @jbeficent @etobella

obulkin commented 6 years ago

@rgarnau - Thanks for factoring in my input! I have no doubt that implementing the FHIR spec is proving to be a difficult task, as it does not seem to have been created for use with modular systems like Odoo. For this reason, I think we might be better off relying on our own intuitions around high-level code organization rather than following the FHIR approach to the letter (ultimately, we would still wind up with all the same models).

One such intuition is the idea that more atomic models should be at the base of the module hierarchy, with progressively more complex models being introduced in modules that are further and further down the tree. This allows complexity to gradually ramp up as users choose to use more and more of the spec, which is arguably one of the main benefits of modularity.

For example, the spec may bundle requests and plans together into one workflow "module" because this is the general category they both belong to, but we can still put requests (atomic) in a base workflow module, request groups (composite of requests) in a dependent module, and plans (more complex composite) further down the tree. Hopefully, this general reasoning makes sense, and we can at least aim for this in so far as the spec allows.

JordiBForgeFlow commented 6 years ago

@obulkin see the latest changes in Eficent/110-dev. We are reaching to a very good level of abstraction and decoupling between models. The medical_workflow is a cornerstone in FHIR and in the odoo project as well, because it provides essential abstract classes lile event and request, used everywhere in fhir