what test tasks need doing?

lluismf · December 31, 2015, 9:52pm

I’m not so sure is correct. The getList function belongs to Restangular library, and the sorting is probably a normal alphabetical sort.

r0bby · January 1, 2016, 1:11pm

This is partially why I never contributed – it’s a pain in the butt to get running – some of the GCI students got it running last year, I gave up personally

darius · January 2, 2016, 6:39am

@elliott if you see this, perhaps you can comment on how to get set up?

lluismf · January 4, 2016, 9:28pm

If the fix is JS and it’s easy why not just do it in production? I know it’s unorthodox but desperate times …

Or install Modulus UI locally but pointing to http://modules.openmrs.org:8080 for the REST services (if possible).

leebreisacher · January 4, 2016, 10:02pm

Yeah, that would be ok with me, but I don’t actually know what a proper fix would be. I have not found documentation for getList() that clearly describes that object parameter ({sort:…, order:…}), although I’ve not looked very hard yet.

Lee

darius · January 4, 2016, 10:27pm

I poked around a bit, I would speculate that the server-side behavior is coming from ReleaseController.listAllResources

github.com

openmrs/openmrs-contrib-modulus/blob/master/grails-app/controllers/org/openmrs/modulus/ReleaseController.groovy#L142


protected Map getParametersToBind() {
    params
}


/**
 * List all resources, optionally only those belonging to a specific module
 * @param params Parameters used to generate the list. If a <code>module</code> is one of the params,
 *               look up releases for that module only.
 */
@Override
protected List listAllResources(Map params) {
    if (params.module) {
        def module = Module.get(params.module.id)
        def releases = Release.findAllByModule(module, params)
        // special case because letting the framework sort this as a string as part of the db query is wrong behavior
        if (params.sort == "moduleVersion") {
            releases.sort()
            if (params.order == "asc") {
                releases.reverse()
            }
        }

And the relevant grails magic is this: https://grails.github.io/grails-doc/3.0.x/ref/Domain%20Classes/findAllBy.html

I think that as per @lluismf’s comment last week, the “sort” is being handled as a string comparison, and it’s because we’re relying on Grails’s “dynamic finder” magic to sort for us.

A server-side fix would be to just sort the list before returning it (because the natural sort order does use the right comparator). A quick web search makes me think it’s not possible to embed this sorting in the GORM layer. (I would lean towards this quick fix.)

The client-side fix would be to just not ask the server to do any sorting, and sort in javascript (this would require adding a semver js sorting library).

lluismf · January 4, 2016, 10:46pm

Sorting in JS is very similar to Java comparators:

It’s a matter of translating the java class we already have to JS.

Option B: sort releases descending in the controller (maybe it already works like this) and do not sort in JS.

leebreisacher · March 13, 2016, 1:03am

Sorry for the long delay. I did spend quite some time trying to get something setup for testing/reproducing the problem described in MOD-89, but was unsuccessful. At this point, I would like to back to UI testing, that’s at least something I think I can contribute to. I’ll start a separate thread on that.

Lee