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

Fixed odoo.dob.field and odoo.weight.field properties and 'patientVoided' → 'voided'. #23

Merged
merged 4 commits into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<simple>${properties:odoo.dob.field:null} != null &amp;&amp; ${properties:odoo.dob.field:null} != ''</simple>

<setProperty name="odooDobField">
<simple>{{odoo.dob.field}}</simple>
<simple>${properties:odoo.dob.field}</simple>
</setProperty>

<setProperty name="birthdate">
Expand Down
4 changes: 2 additions & 2 deletions src/main/resources/camel/odoo-add-extra-quotation-details.xml
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@
<simple>${properties:odoo.weight.field:null} != null &amp;&amp; ${properties:odoo.weight.field:null} != ''</simple>

<setProperty name="odooWeightField">
<simple>{{odoo.weight.field}}</simple>
<simple>${properties:odoo.weight.field}</simple>
</setProperty>

<toD cacheSize="-1" uri="{{openmrs.baseUrl}}/ws/rest/v1/obs?concept={{emr.weight.concept}}&amp;patient=${exchangeProperty.patient.get('uuid')}" />
<toD cacheSize="-1" uri="{{openmrs.baseUrl}}/ws/rest/v1/obs?concept=${exchangeProperty.odooWeightField}&amp;patient=${exchangeProperty.patient.get('uuid')}" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this meant to address a bug?

Copy link
Contributor Author

@icrc-loliveira icrc-loliveira May 21, 2024

Choose a reason for hiding this comment

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

Yes, everything I have done on this PR is to address bugs that I have encounter.

<unmarshal>
<json library="Jackson"/>
</unmarshal>
Expand Down
4 changes: 2 additions & 2 deletions src/main/resources/camel/odoo-patient-handler.xml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

<choice>
<when>
<simple>${exchangeProperty.patient-odoo-id} == null &amp;&amp; ${exchangeProperty.patient.get('patientVoided')} == true</simple>
<simple>${exchangeProperty.patient-odoo-id} == null &amp;&amp; ${exchangeProperty.patient.get('voided')} == true</simple>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you might want to check both voided and patientVoided, a person can be linked to more than one entity e.g. User, Provider and Patient. Therefore, a patient account can get voided but not necessarily their person record, or user or provider accounts.

Copy link
Contributor

Choose a reason for hiding this comment

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

@wluyima it seems like we have voided attribute in patient model but AFAIK we don't have any attribute as patientVoided
We have personVoided in person model.
CMIIW

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry! That's what I meant to check both Person.personVoided and Patient.voided

Copy link
Contributor

Choose a reason for hiding this comment

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

@VaishSiddharth @wluyima why doing test on both properties personVoided and Patient.voided as in this route we deal with patient data only ( and with patient identifier,...) and only patient are synchronized with Odoo.

IMHO, The use case "a patient is voided but the person is not voided" should be ignored as well as this route need patients only. If a paitent is voided, we will have an error when trying to get the patient identifier and it causes erros in this route when a patient is voided.

Why should the use case "a patient is voided but the person is not voided" be processed in this route ?

Copy link

@wluyima20 wluyima20 Jun 11, 2024

Choose a reason for hiding this comment

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

There was definitely a bug that this PR seems to address. And in fact, these changes aligns with what am saying so I guess it's all good.

<log message="No action to take for voided patient since they have no customer record in odoo" />
</when>
<otherwise>
Expand All @@ -36,7 +36,7 @@
<simple>${exchangeProperty.updateExistingCustomer} == true || ${exchangeProperty.createCustomerIfNotExist} == true || ${properties:create.customer.if.not.exist:false} == true</simple>
<setProperty name="isPatientVoidedOrDeleted">
<!-- TODO support deleted patient -->
<spel>#{getProperty('patient').get('patientVoided') == true}</spel>
<spel>#{getProperty('patient').get('voided') == true}</spel>
</setProperty>

<when>
Expand Down