We are now going to implement another main feature for OpenMRS Add Ons this week and that is to display the compatibility of modules based on the OpenMRS platform version selected by the user.
This feature was planned during the discussion here.
We also plan to add another page containing instructions on how a user can his/her platform version. This is being done to address the concerns by developers like @ssmusoke and @dkayiwa.
Moreover, the default location pointed to by the download button would be the latest version.
How do we plan to implement it?
The user can set his Platform version a select box that will look like this
Now, when a user visits a module page , he will see a large download button similar to the one currently there in modules.openmrs.org. Something like this:
How does the UI change after you choose a platform version?
When a platform version is selected , a big download button appears in every module’s description.
If, no version is selected then the button does not appear.
Your flow shows “taken to page showing steps…” but this isn’t anywhere in the mockup
We haven’t created that page yet but for visualisation purposes, it will be similar to something like current Add Ons about page.
EDIT: You may find the corresponding Jira ticket here.
For any questions or suggestions, you may contact me, @darius or @mseaton
I think we should conceptualize this feature more broadly. Modules can have more requirements than just the OpenMRS platform version, e.g. they can require other modules. So, we should treat this more like “Your OpenMRS Configuration”. For now clicking on this would only let you specify your platform version, but eventually we’d also allow you to pick a Distribution + Version, or indicate what modules you’re running.
Why add a tooltip? If we design the UI right it should be obvious what’s happening.
We can do better than this. The most common thing that people do when they go to this site is to download the latest version of a module, so if a big download button is desirable, we should add it in all cases.
I propose:
when no platform version is selected, there’s a big download button saying:
Download
Latest Version: 4.3.11
when a platform version is selected, the big download button says:
Download
Compatible Version: 3.1.0
In addition, if we know some versions are incompatible, they should be greyed out in the list of module versions.
Let’s not add this to the addons application, because it’s going to be one more place that instructions are duplicated and we’ll forget to keep out of date if we change the OpenMRS UI.
Instead, the instructions should be on a wiki page (maybe it exists already?) and addons can just link to it.
True but we will have to look at ways of implementing this. Is there any file which the user can upload which contains the list of all modules installed with their versions? If so then we could use that file to get the details we need and choose the versions accordingly.
Makes sense.[quote=“darius, post:2, topic:11855”]
We also plan to add another page containing instructions on how a user can his/her platform version. This is being done to address the concerns by developers like @ssmusoke and @dkayiwa.
[/quote]
I don’t mean to say that we need to implement this now, we can think about how exactly to implement it later. I was assuming we’d just have the user type in module versions. (Thought it would be nice to add a feature to OpenMRS to export this.)
I just mean, when implementing the “platform version” feature, keep in mind that there’s a broader use case, so let’s leave the UI a bit flexible for that.
@raff , @darius Is there any api or something of that sort which could provide us with a list of all platform versions currently available. We will be needing that for implementing the “display of compatibility of modules in Add Ons” feature . The last option would be to manually type all platform versions but that would mean we will have to keep editing it every time a new platform version is released.
Personally I think it would be a good thing if OpenMRS published all of the openmrs-distro.properties files for all of our releases in a fixed place, accessible via HTTP, and addons could fetch it from this location. Rafal, what do you think about that?
Otherwise, @reubenv, we’d have to manually type all of these somewhere in the addons codebase, which would be annoying to keep up to date.
@darius Do we be version specific as in is it fine if I just take the first 2 digits and get their values? For example : If the platform version is 1.12.04 .
Will it be okay if I just take 1.12 into consideration?
Because in that case the comparison of these version strings will be much less complex and hopefully that does serve our purpose as well!
This is great. So, @reubenv all we’d need to do is to have a scheduled task that looks at those two directories and finds all the final release version numbers (e.g. not -SNAPSHOT, -alpha, etc).
(Probably we should use the artifactory REST API like this instead of a plain HTTP call.)
Also, thinking more, we shouldn’t actually be looking at compatibility with a version of OpenMRS Platform but rather compatibility with a version of openmrs-core. Those are not 100% the same, and core is actually what a module requires. So in the UI we’d ask for “OpenMRS Core version”, and instead of the distro/platform link Rafal provided we could look at JFrog
No that’s not okay. But you don’t have to write this logic, it has already been done. See here, where I copied this code from openmrs-core into the addons codebase.
I have made 2 java classes within the domain folder with the names CoreVersionDetails and CoreVersionsSummary . Hope those names are fine. These are going to be used by jackson to parse our url that contains the OpenMRS Core versions.
The problem with using the api is that the versions are all randomly ordered and hence if we have make them ordered then we will have to probably create our own functions.
The xml file on the other hand has all the versions in an orderly manner
I guess that is fine. Put them in a subpackage like “artifactory” because
those are not part of our core domain they are just an implementation
detail.
I would think you want a service method that returns a List<Version> as
the actual public API.
My understanding is that in the UI you want to have a dropdown where the user specified what openmrs-core version they are using. (You showed this in a mockup.)
=> therefore the UI needs to be able to ask our server “what openmrs-core versions are there?”
=> therefore our server should expose a REST API like “/api/v1/openmrs-core-versions” which returns ["2.1.0", "2.0.5", ...]
Since we don’t want to hardcode and maintain that list, we will build it by querying artifactory. We don’t want to do this every time the user loads a page. Instead we can have a scheduled task that does this every hour, and we just return this cached version from our REST API.
Eventually we might also allow the user to indicate what module versions they are running (e.g. to show them if they have any missing dependencies when getting a module that depends on other modules), but this won’t happen until (much) later.
True but my question was as to which of the two methods should we use in fetching the data from artifactory? The 2 methods are listed below
For getting the list of core versions instead of hard coding it,
we have 2 reliable sources:
1)The rest api which will get us the list of versions in json format
2) We could use the Maven metadata xml located here within artifactory
Now, on fetching data using the rest api or even by visiting this link, you will find that the versions are not ordered . Hence when we populate our drop down ,
the list will be of this 1.6.1 , 1.7.1, 2.1.1, 1.2.1 etc form. Here the versions are randomly placed because that’s how they will stored in the list. ( Note that the versions are randomly sorted)
On using the maven config file , our drop down elements will be in this order
1.2.1 ,1.6.1 , 1.7.1 , 2.1.1 . As you may notice , here as you go to the right you get the newer versions i.e. the data is ordered .( Note that the versions are ordered)
Hence I feel that it would be easier for the user to find and choose his option from a list of around 20 versions or so if we use the xml file.
My question hence is as to do you still feel that we must use method 1 over method 2 (considering the benefits of method 2 i,e, get data from the metadata.xml file)
It doesn’t really matter. Regardless it should be hidden as an implementation detail of our FetchOpenmrsCoreVersions scheduled task.
Personally I think it’s hacky to parse a maven metadata.xml file so I would prefer to use the artifactory REST API.
Note that the Version class already implements Comparable<Version> so if you just store a SortedSet<Version> in our code then it doesn’t matter if artifactory is returning them sorted or not.
@darius You mentioned that we should get rid of the service itself and instead just store the Sortedlist as a variable right? In that case I was wondering as to how about we have it such that the scheduler class has a variable which basically is the sortedlist and the controller class will just return that value. This would save us quite a bit of unnecessary processing each time the api is called .
For example: Now for getting the sortedlist , assuming that the versions are already filtered for words like “Snapshot” etc, we have to pass it through a function which converts the list into a sorted list . This is not needed each time as the list will remain exactly the same each time and hence I feel it would be wiser to just store it as is.
@darius Another problem worth noticing is that the format of the required OpenMRS version is quite inconsistent across the various modules. Check out the older versions of https://modules.openmrs.org/#/show/58/htmlformentry . Notice the versions ike 1.5.0.8979 .
My solution was to maybe split it and only use 1.5.0
@reubenv, let me explain further something that I didn’t explain well in my comments on your commit.
The “correct” design is that our application has multiple services, which handle related kinds of functionality. We already have an IndexingService which handles things related to the index, and for this we’d probably add a VersionsService which handles things related to versions. In the pull request I said you could skip the service and take a shortcut, because right now a service which just holds a single Set variable is a bit anemic, and silly. But the correct thing to do is to have a service, since we do assume it will grow to have some more methods.
The way I would structure this code for best style (even though this is perhaps overkill for the specific purpose) is as follows (and the reason for structuring it like this is to have each :
class FetchOpenmrsCoreVersions
scheduled task
has logic of making a REST call to artifactory, and producing a List
class VersionList
has the business logic of dropping the alpha/beta/SNAPSHOT/etc
has a constructor that takes a Collection
internally stores a SortedSet
class VersionsService
has a setVersions(Collection), called by the scheduled task periodically
has a getVersions(), called by the rest controller on every page load
internally it stores a VersionList
has no business logic in this class itself, it just wires together logic that’s in tasks, controllers, and domain objects
You want to completely transform the version list into the form we can return to the client as part of the logic that fetches/builds the version list. You don’t want any additional processing to happen on every page load.
No, it is a requirement to treat this value exactly as the module says. If a module says it requires openmrs-core 1.5.0.8979 then we have to trust it is doing this intentionally.
Also, you don’t have to write any logic for this, because the logic is already written in the Version class that we copied from openmrs-core. And in fact it’s desirable for our logic to be identical to the logic that OpenMRS uses.
You can just do something like:
// String userOpenmrsCoreVersion, submitted via REST call;
// Version requiredOpenmrsCoreVersion, from a module
if (new Version(userOpenmrsCoreVersion).compareTo(requiredOpenmrsCoreVersion) >= 0) {
// it works
}
I had assumed that there’s another way to pass this prop down to the children that’s compatible with react router, without needing to use cloneElement, but after a bit of googling I didn’t find it.
In fact the correct design here is to:
introduce a SelectUserVersions component (instead of putting the UI element directly in App)
App passes a callback function into SelectUserVersions, and this is used to set the global state
App passes this state as props into the children (like you’re doing now)
