change group_list sorting to use sort

ckan/logic/action/get.py

@@ -266,8 +266,12 @@ def group_list(context, data_dict):
     '''Return a list of the names of the site's groups.
 
     :param order_by: the field to sort the list by, must be ``'name'`` or
-      ``'packages'`` (optional, default: ``'name'``)
+      ``'packages'`` (optional, default: ``'name'``) Deprecated use sort.
     :type order_by: string
+    :param sort: sorting of the search results.  Optional.  Default:
+        "name asc" string of field name and sort-order. The allowed fields are
+        'name' and 'packages'
+    :type sort: string
     :param groups: a list of names of the groups to return, if given only
         groups whose names are in this list will be returned (optional)
     :type groups: list of strings
@@ -278,18 +282,30 @@ def group_list(context, data_dict):
     :rtype: list of strings
 
     '''
+
     model = context['model']
     api = context.get('api_version')
     groups = data_dict.get('groups')
     ref_group_by = 'id' if api == 2 else 'name';
-    order_by = data_dict.get('order_by', 'name')
-    if order_by not in set(('name', 'packages')):
-        raise logic.ParameterError('"order_by" value %r not implemented.' % order_by)
-    all_fields = data_dict.get('all_fields',None)
 
-    # We want the default for packages to be highest first
-    default_order_dir = ("asc", "desc")[order_by == 'packages']
-    order_direction = data_dict.get('order_direction', default_order_dir)
+    sort = data_dict.get('sort', 'name')
+    # order_by deprecated in ckan 1.8
+    # if it is supplied and sort isn't use order_by and raise a warning
+    order_by = data_dict.get('order_by')
+    if order_by:
+        log.warn('`order_by` deprecated please use `sort`')
+        if not data_dict.get('sort'):
+            sort = order_by
+    # if the sort is packages and no sort direction is supplied we want to do a
+    # reverse sort to maintain compatibility.
+    if sort.strip() == 'packages':
+        sort = 'packages desc'
+
+    sort_info = _unpick_search(sort,
+                               allowed_fields=['name', 'packages'],
+                               total=1)
+
+    all_fields = data_dict.get('all_fields', None)
 
     _check_access('group_list', context, data_dict)
 
@@ -301,8 +317,8 @@ def group_list(context, data_dict):
 
     groups = query.all()
     group_list = model_dictize.group_list_dictize(groups, context,
-                                                  lambda x:x[order_by],
-                                                  order_direction == 'desc')
+                                                  lambda x:x[sort_info[0][0]],
+                                                  sort_info[0][1] == 'desc')
 
     if not all_fields:
         group_list = [group[ref_group_by] for group in group_list]
@@ -2217,3 +2233,30 @@ def dashboard_activity_list_html(context, data_dict):
     '''
     activity_stream = dashboard_activity_list(context, data_dict)
     return _activity_list_to_html(context, activity_stream)
+
+
+def _unpick_search(sort, allowed_fields=None, total=None):
+    ''' This is a helper function that takes a sort string
+    eg 'name asc, last_modified desc' and returns a list of
+    split field order eg [('name', 'asc'), ('last_modified', 'desc')]
+    allowed_fields can limit which field names are ok.
+    total controls how many sorts can be specifed '''
+    sorts = []
+    split_sort = sort.split(',')
+    for part in split_sort:
+        split_part = part.strip().split()
+        field = split_part[0]
+        if len(split_part) > 1:
+            order = split_part[1].lower()
+        else:
+            order = 'asc'
+        if allowed_fields:
+            if field not in allowed_fields:
+                raise logic.ParameterError('Cannot sort by field `%s`' % field)
+        if order not in ['asc', 'desc']:
+            raise logic.ParameterError('Invalid sort direction `%s`' % order)
+        sorts.append((field, order))
+    if total and len(sorts) > total:
+        raise logic.ParameterError(
+            'Too many sort criteria provided only %s allowed' % total)
+    return sorts

ckan/tests/functional/test_group.py

@@ -124,31 +124,25 @@ def test_sorting(self):
         assert results[-1]['name'] == u'gamma', results[-1]['name']
 
         # Test name reverse
-        data_dict = {'all_fields': True, 'order_by': 'name', 'order_direction': 'desc'}
-        results = get_action('group_list')(context, data_dict)
-        assert results[0]['name'] == u'gamma', results[0]['name']
-        assert results[-1]['name'] == u'alpha', results[-1]['name']
-
-        # Test default (name) reverse
-        data_dict = {'all_fields': True, 'order_direction': 'desc'}
+        data_dict = {'all_fields': True, 'sort': 'name desc'}
         results = get_action('group_list')(context, data_dict)
         assert results[0]['name'] == u'gamma', results[0]['name']
         assert results[-1]['name'] == u'alpha', results[-1]['name']
 
         # Test packages reversed
-        data_dict = {'all_fields': True, 'order_by': 'packages', 'order_direction': 'desc'}
+        data_dict = {'all_fields': True, 'sort': 'packages desc'}
         results = get_action('group_list')(context, data_dict)
         assert results[0]['name'] == u'beta', results[0]['name']
         assert results[1]['name'] == u'delta', results[1]['name']
 
         # Test packages forward
-        data_dict = {'all_fields': True, 'order_by': 'packages', 'order_direction': 'asc'}
+        data_dict = {'all_fields': True, 'sort': 'packages asc'}
         results = get_action('group_list')(context, data_dict)
         assert results[-2]['name'] == u'delta', results[-2]['name']
         assert results[-1]['name'] == u'beta', results[-1]['name']
 
         # Default ordering for packages
-        data_dict = {'all_fields': True, 'order_by': 'packages'}
+        data_dict = {'all_fields': True, 'sort': 'packages'}
         results = get_action('group_list')(context, data_dict)
         assert results[0]['name'] == u'beta', results[0]['name']
         assert results[1]['name'] == u'delta', results[1]['name']