@mksd I had a change to play around with the Visit Documents module this week… looks great and I think it could be quite useful to us.
I took a first pass at making it compile and fire up under 1.10.x. There were only a couple small changes that had to be made to the code itself to get it to compile, but then I had to make some modifications to the poms to get the context-sensitive tests to fair up correctly. Not sure I did things in the best way if you are tryin maintain usage of the distro pom. I’ve issued a pull request–can you take a look when you get a chance and see if I’m on a path that is accept to you.
I don’t think things will 100% work yet with 1.10.x–I was getting an error when trying to upload a form that I suspect was caused by a missing StringToProviderConverter. I can continue to develop, but wanted to show you guys what I was looking to do earlier rather than later.
Let me know if you have any questions or comments!
Great that you started looking into it!
I haven’t looked yet at your PR in details, I will do next week.
I have one question though, is there a way to have this module producing several OMODs upon building it with Maven? One for each supported family of Platform versions?
Because anyway I know that we will have to do something along these lines to make it Platform 2.0 compatible.
Isn’t it the sort of thing that Core Apps or EMR API are doing?
I would find this a lot cleaner and more sustainable than reverting some of the code to make it comply to an older version of the Core. And as I said, we will need to go through this effort because of the gap between Core 1.x and Platform 2.x.
Thanks @dkayiwa, looks like exactly what I was looking for. I need to figure out all the plumbing now.
@mogoodrich, I will try to get this Spring pattern setup and working for 1.11.x vs 2.x compatibility issues that I know of already.
And then this will be easily repeated for 1.10.x compatibility issues as well, so your use case of VDUI. But of course, should you know how to do all this already, then please go ahead and modify your PR in that direction.
Unfortunately, I haven’t worked with setting up different sub-modules to support multiple versions of OpenMRS. For what it’s worth, the actual code changes to make it 1.10.x compatible were so minimal not sure if is worth creating a separate 1.10.x component for it.
That being said, probably the best option would be for us just to get on 1.11.x.
FWIW, when the changes for compatibility are too small, you may just avoid sub modules by reflection. I have done that in the xforms module.
Could you point me to it?
Sure, and we could start this way anyway so that you can move on with using VDUI.
But as I said, we may need this anyway, I’m not sure yet.
I’m sorry Mark, I won’t be able to look more into it before the end of the month, is that ok with you?
Yep, that’s fine, no rush! Things are quite busy here as well.
By the way this is not confirmed yet but VDUI might be pushed to a production instance for the first time on Wednesday evening CEST. So if it goes that way, we will know more about its behaviour by the end of the month already!
Ok I could finally spend some time on this! I have not used your PR as such (thanks a lot for it btw), I have instead copied your changes where applicable on a new branch that is used for the 1.10.x compatibility work: here.
There are a few caveats & issues, see below.
I started from the official Ref App distro 2.1 but it is not restrictive enough. For the record, I chose Ref App distro 2.1 because it was the last one running on Core 1.10.x.
See below my minimum set of versions, I left as comments the original versions as pulled by the SDK and that I had to increase:
<openMRSVersion>1.10.2</openMRSVersion> <!-- 1.10.0 -->
<uicommonsVersion>1.4</uicommonsVersion> <!-- 1.3 -->
<webservices.restVersion>2.12</webservices.restVersion> <!-- 2.6 -->
<emrapiVersion>1.12</emrapiVersion> <!-- 1.10 -->
<serialization.xstreamVersion>0.2.8</serialization.xstreamVersion> <!-- 0.2.7 -->
<appuiVersion>1.3</appuiVersion> <!-- 1.2.2 -->
Core 1.10.2 was needed for
@OpenmrsProfile to work with the
webservices.rest 2.12 because that is the first version that contains
ObsResource1_9 that I have inherited from. I may be able to just inherit from
ObsResource1_8 though if that is too restrictive.
EMR API 2.12: I saw that you did set it to 2.10 in your PR, however 2.12 is the first one after 2.10 that does not refer to a SNAPSHOT distro artifact and allows to build out of an empty .m2 folder.
UI Commons 1.4 because the version found in distro 2.1 refers to SNAPSHOT artifacts that can’t be found anymore.
serialization.xstream 0.2.8: same as above.
appui 1.3: same as above.
Then I had to put together a modified distro 2.1 that aligns with the above changes for VDUI to load. Here is my current openmrs-distro.properties.
As you will see I needed to upgrade Core Apps to 1.7. This is because prior to that version the patient’s UUID is not passed along to the CF patient dashboard widgets: see here. And that is needed for VDUI’s dashboard widget to work.
The ad-hoc branch of VDUI will install on the custom distro, but anyway before that the upgrade of Core Apps to 1.7 breaks it I cannot look into it in more details now but you may be able to figure out quickly what is going wrong, or even better: your own distro running on Core 1.10.x might just be ok anyway.
Please let me know your thoughts. I will be a little under water until the end of the month, but after that I will have time again.
Thanks again for the PR.
Just wondered if you ever had a chance to look at the branch where I integrated the changes from your PR? (Please see my last post.)
I also kept looking into it on my side but this is not anymore Core 1.10.x compatibility work really, but rather former Ref App distro UI compatibility… and it is going in all sort of directions depending on the set of modules involved.
Please let me know if you are still interested in this, in which case I’d be more than happy to help.
In the meantime I will be spending time on making VDUI Ref App 2.5 (Platform 2.0) compatible.
Sorry @mksd this fell off my radar somehow… it’s not a core need for us right now, but I should be able to take a closer look in the next wek or so.
@mksd just to clarify, are you looking to merge this branch back into the core line if we get it to work, or would the idea be to maintain a 1.10.x compatible separate branch. (Maintaining a separate branch is probably not worth the effort, imo).
Yes, we will merge it back into the core branch when all the compatibility work is completed.
Cool, great, I will try to test before the end of the week.