-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat(#114): Create OpenMRS Mediator #115
base: main
Are you sure you want to change the base?
Conversation
docker/docker-compose.openmrs.yml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to add this to the startup.sh?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another thing about openmrs when testing locally it is now working.
A call to POST localhost:5001/mediator/cht/sync
retutrn socket hang up
In OpenHIM transaction logs also:
GET openhim-core:5001/openmrs/Patient/?_lastUpdated=gt2024-05-27T16:41:02.989Z
getaddrinfo ENOTFOUND openmrs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
latest commit should fix the error. but if openmrs is not running, sync doesn't do anything
also added it to startup.sh temporarily, although not every deployment will want a local copy of openmrs
the next problem is that this image has a very old version of the fhir module that doesn't work right, and its missing the cht identifier types; I fixed that locally by downloading the latest fhir omod from https://addons.openmrs.org/show/org.openmrs.module.openmrs-fhir2-module and running ocker cp ~/Downloads/fhir2-2.1.0.omod chis-interop-openmrs-1:/usr/local/tomcat/.OpenMRS/modules/fhir2-2.1.0.omod
and manually creating the identifier types in the UI but need to dockerize somehow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@witash, can you please add the configs to cht-config so that we can reproduce the entire workflow in the e2e tests using only this repo and not depending on config-gandaki?
Let me know if I can help.
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the config be private @binokaryg, as is the case with most of our projects?
@lorerod is there an alternative to testing in case the config cannot be copied in a public repo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the config is private with limited access to collaborators.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andrablaj, from what I understand, we do not need the entire config. It's just an outbound push configuration and a couple of forms that do not need to be precisely the same. I’m right, @witash?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea, for the LTFU the configurator put the outbound push configuration in the settings automatically.
What we need to add/modify for tests is not anything specific to the gandaki config. a sample outbound push for cht form submission and inbound form for patient creation and inblound forms
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added the inbound forms and outbound for patient creation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, with this, can I create a patient from CHT to Openmrs and vice versa?
@witash I see that the last run for e2e test here is not failing with
The configurator container name changed from |
retry_startup | ||
|
||
echo 'Waiting for configurator to finish...' | ||
docker container wait chis-interop-configurator-1 | ||
docker container wait chis-interop-cht-configurator-1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The container name should be: chis-interop-configurator-1
As this is failing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
chis-interop-configurator and interop-cht-configurator are two different containers. chis-interop-configurator sets up openhim configuration, chis-interop-cht-configurator runs cht-conf to add the cht config for e2e-tests
I guess it should be waiting for both here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I don't see interop-cht-configurator
creation in the logs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, I don't see an interop-cht-configurator
container. @witash are we missing anything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When running the test locally, there is an interop-cht-configurator.
In CI the behavior is different, maybe due to the [too](toomanyrequests: Rate exceede) problem.
Locally the OpenMRS tests are failing :
Workflows
OpenMRS workflow
✕ Should follow the CHT Patient to OpenMRS workflow (4710 ms)
✕ Should follow the OpenMRS Patient to CHT workflow (204 ms)
Loss To Follow-Up (LTFU) workflow
✓ Should follow the LTFU workflow (8371 ms)
● Workflows › OpenMRS workflow › Should follow the CHT Patient to OpenMRS workflow
> 140 | expect(retrieveFhirPatientIdResponse.body.total).toBe(1);
● Workflows › OpenMRS workflow › Should follow the OpenMRS Patient to CHT workflow
> 177 | expect(retrieveFhirPatientIdResponse.body.total).toBe(1);
Both asserting that the patiend ID was created in Fhir.
I'm testing this manually to see if I can reproduce it.
mediator/test/e2e-test.sh
Outdated
export OPENMRS_USERNAME=admin | ||
export OPENMRS_PASSWORD=Admin123 | ||
|
||
# Cleanup from last test, in case of interruptions | ||
retry_startup() { | ||
max_attempts=5 | ||
count=0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In L17
I introduced a bug. I think the correct command would be ./startup.sh up-test
instead of ./startup.sh init
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed with 59bf494
Changes to make e2e tests pass:
|
Change mediator to accept unformatted documents from CHT and map them to FHIR resources that will be accepted by OpenMRS in the mediator.
Closes https://github.com/medic/config-gandaki/issues/25 https://github.com/medic/config-gandaki/issues/5 #124 #125