[#1431] Revert sort to "newest first" as it always was. Add sort para…

…m to allow chronological, which makes more sense.

ckan/logic/action/get.py

@@ -186,7 +186,8 @@ def current_package_list_with_resources(context, data_dict):
 
 
 def revision_list(context, data_dict):
-    '''Return a list of the IDs of the site's revisions in chronological order.
+    '''Return a list of the IDs of the site's revisions. They are sorted with
+    the newest first.
 
     Since the results are limited to 50 IDs, you can page through them using
     parameter ``since_id``.
@@ -195,12 +196,16 @@ def revision_list(context, data_dict):
     :type id: string
     :param since_time: the timestamp after which you want the revisions
     :type id: string
+    :param sort: the order to sort the related items in, possible values are
+      'time_asc', 'time_desc' (default). (optional)
+    :type sort: string
     :rtype: list of revision IDs, limited to 50
 
     '''
     model = context['model']
     since_id = data_dict.get('since_id')
     since_time_str = data_dict.get('since_time')
+    sort_str = data_dict.get('sort')
     PAGE_LIMIT = 50
 
     _check_access('revision_list', context, data_dict)
@@ -220,8 +225,18 @@ def revision_list(context, data_dict):
     revs = model.Session.query(model.Revision)
     if since_time:
         revs = revs.filter(model.Revision.timestamp > since_time)
-    revs = revs.order_by(model.Revision.timestamp) \
-               .limit(PAGE_LIMIT)
+
+    sortables = {
+        'time_asc': model.Revision.timestamp.asc,
+        'time_desc': model.Revision.timestamp.desc,
+    }
+    if sort_str and sort_str not in sortables:
+        raise logic.ValidationError(
+            'Invalid sort value. Allowable values: %r' % sortables.keys())
+    sort_func = sortables.get(sort_str or 'time_desc')
+    revs = revs.order_by(sort_func())
+
+    revs = revs.limit(PAGE_LIMIT)
     return [rev_.id for rev_ in revs]
 
 

ckan/tests/logic/action/test_get.py

@@ -1674,6 +1674,13 @@ def test_revision_doesnt_exist(self):
             'revision_list',
             since_id='1234')
 
+    def test_sort_param_not_valid(self):
+        nose.tools.assert_raises(
+            logic.ValidationError,
+            helpers.call_action,
+            'revision_list',
+            sort='invalid')
+
     # Normal usage
 
     @classmethod
@@ -1690,23 +1697,33 @@ def _create_revisions(cls, num_revisions):
     def test_all_revisions(self):
         rev_ids = self._create_revisions(2)
         revs = helpers.call_action('revision_list')
-        eq(revs[-len(rev_ids):], rev_ids)
+        # only test the 2 newest revisions, since the system creates one at
+        # start-up.
+        eq(revs[:2], rev_ids[::-1])
 
     def test_revisions_since_id(self):
-        rev_ids = self._create_revisions(4)
-        revs = helpers.call_action('revision_list', since_id=rev_ids[1])
-        eq(revs, rev_ids[2:])
+        self._create_revisions(4)
+        revs = helpers.call_action('revision_list', since_id='1')
+        eq(revs, ['3', '2'])
 
     def test_revisions_since_time(self):
         from ckan import model
-        rev_ids = self._create_revisions(4)
+        self._create_revisions(4)
 
-        rev1 = model.Session.query(model.Revision).get(rev_ids[1])
+        rev1 = model.Session.query(model.Revision).get('1')
         revs = helpers.call_action('revision_list',
                                    since_time=rev1.timestamp.isoformat())
-        eq(revs, rev_ids[2:])
+        eq(revs, ['3', '2'])
 
     def test_revisions_returned_are_limited(self):
-        rev_ids = self._create_revisions(55)
-        revs = helpers.call_action('revision_list', since_id=rev_ids[1])
-        eq(revs, rev_ids[2:52])  # i.e. limited to 50
+        self._create_revisions(55)
+        revs = helpers.call_action('revision_list', since_id='1')
+        eq(len(revs), 50)  # i.e. limited to 50
+        eq(revs[0], '54')
+        eq(revs[-1], '5')
+
+    def test_sort_asc(self):
+        self._create_revisions(4)
+        revs = helpers.call_action('revision_list', since_id='1',
+                                   sort='time_asc')
+        eq(revs, ['2', '3'])