Catalog sort_on on multiple columns is broken

[volkerjaenisch]

ZCatalog can handle multiple sort_indexes.

Products.ZCatalog.Catalog.py [1014] accepts a list of search indexes:

    def _getSortIndex(self, args):
        """Returns a list of search index objects or None."""
        sort_index_names = self._get_sort_attr("on", args)
        if sort_index_names is not None:
            # self.indexes is always a dict, so get() w/ 1 arg works
            sort_indexes = []
            if not isinstance(sort_index_names, (list, tuple)):
                sort_index_names = [sort_index_names]
            for name in sort_index_names:
                sort_index = self.indexes.get(name)

But commit 0909383eb2bbc8d31794a7dbf4392eec6236be22 breaks this

CatalogTool.py [461]

        sort_on = kw.get('sort_on')
        if sort_on and sort_on not in self.indexes():
            # I get crazy sort_ons like '194' or 'null'.
           kw.pop('sort_on')

This assumes that sort_on is a index_name and fails if it is a list of index names.

Also the QueryBuilder has the same flaw: plone.app.querystring.queryparser [53]

     # Add sorting (sort_on and sort_order) to the query
    if sort_on:
        catalog = getToolByName(context, 'portal_catalog')
        # I get crazy sort_ons like '194' or 'null'.
        if sort_on in catalog.indexes():
            query['sort_on'] = sort_on
            if sort_order:
                query['sort_order'] = sort_order

The later can be circumvented by using the custom-query parameter, but not the former.

What I did:

results = catalog(**{ ..., 'sort_on' : ['title','description']], ... })

What I expect to happen:

The result should be sorted after both indexes.

What actually happened:

The result is not sorted at all.

What version of Plone/ Addons I am using:

5.1

Shall I make a pull request?

[volkerjaenisch]

Here a simple fix for the impatient

        # remove sort_on fields not backed by an index
        sort_on = kw.get('sort_on')
        if sort_on :
            indexes = self.indexes()
            if not isinstance(sort_on, (list, tuple)):
                sort_on = [sort_on]
            for sort_idx in sort_on:
                if sort_idx not in indexes:
                    sort_on.delete(sort_idx)
            kw['sort_on'] = sort_on

[ale-rt]

@volkerjaenisch I can confirm your analysis, thanks a lot :) I will try to make a PR to fix this!

[mauritsvanrees]

Thanks! This was on my list of things to look at, since it broke after a commit by me, but I didn't make time for it yet.