1. Odd Simon Simonsen
  2. trac-t10425

Source

trac-t10425 / new_order_format.diff

# HG changeset patch
# Parent f9526778aa521e1cd6528729128c1988bce9a182

diff --git a/trac/ticket/query.py b/trac/ticket/query.py
--- a/trac/ticket/query.py
+++ b/trac/ticket/query.py
@@ -88,7 +88,7 @@ class Query(object):
     clause_re = re.compile(r'(?P<clause>\d+)_(?P<field>.+)$')
 
     def __init__(self, env, report=None, constraints=None, cols=None,
-                 order=None, desc=0, group=None, groupdesc=None, verbose=0,
+                 order=None, desc=None, group=None, groupdesc=None, verbose=0,
                  rows=None, page=None, max=None, format=None):
         self.env = env
         self.id = report # if not None, it's the corresponding saved query
@@ -97,8 +97,6 @@ class Query(object):
             constraints = [constraints]
         self.constraints = constraints
         synonyms = TicketSystem(self.env).get_field_synonyms()
-        self.order = synonyms.get(order, order)     # 0.11 compatibility
-        self.desc = desc
         self.format = format
         self.default_page = 1
         self.items_per_page = QueryModule(self.env).items_per_page
@@ -146,15 +144,30 @@ class Query(object):
         self.cols = [c for c in cols or [] if c in field_names or 
                      c == 'id']
         self.rows = [c for c in rows if c in field_names]
+        
+        self.order = field_from_string(order)
+        if self.order:
+            # 0.11 compatibility
+            field = self.order['field']
+            self.order['field'] = synonyms.get(field, field)
+        
+            # 0.12 compatibility
+            if desc is not None:
+                self.order['desc'] = True if desc else False
 
-        self.order = self.order or QueryModule(self.env).default_order
-        if self.order:
-            if self.order[0] == '-':
-                (self.order, self.desc) = (self.order[1:], 1)
-            elif self.order[0] == '+':
-                (self.order, self.desc) = (self.order[1:], 0)
-        if self.order != 'id' and self.order not in field_names:
-            (self.order, self.desc) = ('priority', 0)
+        if not self.order or (self.order['field'] != 'id' and \
+                              self.order['field'] not in field_names):
+            self.order = field_from_string(QueryModule(self.env).default_order)
+            
+            # 0.11 compatibility
+            if self.order:
+                field = self.order['field']
+                self.order['field'] = synonyms.get(field, field)
+            
+            if not self.order or (self.order['field'] != 'id' and \
+                                  self.order['field'] not in field_names):
+                self.order = field_from_string('priority')
+
 
         if group == '':
             self.group = group
@@ -306,8 +319,8 @@ class Query(object):
             cols = cols[:7]
 
         # Make sure the column we order by is visible.
-        if self.order and self.order not in cols:
-            cols.append(self.order)
+        if self.order and self.order['field'] not in cols:
+            cols.append(self.order['field'])
 
         # Make sure to not show the column we group on.
         if self.group and self.group['field'] in cols:
@@ -421,14 +434,14 @@ class Query(object):
 
         if id is None:
             id = self.id
-        if desc is None:
-            desc = self.desc
-        if order is None:
-            order = self.order
         if max is None:
             max = self.max
         if page is None:
             page = self.page
+        
+        order = field_from_string(order)
+        if order and desc is not None:              # 0.12 compatibility
+            order['desc'] = True if desc else False
 
         cols = self.get_columns()
         # don't specify the columns in the href if they correspond to
@@ -455,7 +468,7 @@ class Query(object):
         
         return href.query(constraints,
                           report=id,
-                          order=order, desc=1 if desc else None,
+                          order=field_to_string(order or self.order),
                           group=group,
                           col=cols,
                           row=self.rows,
@@ -495,7 +508,7 @@ class Query(object):
             add_cols(group_field)
         if self.rows:
             add_cols('reporter', *self.rows)
-        add_cols('status', 'priority', 'time', 'changetime', self.order)
+        add_cols('status', 'priority', 'time', 'changetime', self.order['field'])
         cols.extend([c for c in self.constraint_cols if not c in cols])
 
         custom_fields = [f['name'] for f in self.fields if f.get('custom')]
@@ -516,14 +529,14 @@ class Query(object):
 
         # Join with the enum table for proper sorting
         for col in [c for c in enum_columns
-                    if c in (self.order, group_field, 'priority')]:
+                    if c in (self.order['field'], group_field, 'priority')]:
             sql.append("\n  LEFT OUTER JOIN enum AS %s ON "
                        "(%s.type='%s' AND %s.name=%s)"
                        % (col, col, col, col, col))
 
         # Join with the version/milestone tables for proper sorting
         for col in [c for c in ['milestone', 'version']
-                    if c in (self.order, group_field)]:
+                    if c in (self.order['field'], group_field)]:
             sql.append("\n  LEFT OUTER JOIN %s ON (%s.name=%s)"
                        % (col, col, col))
 
@@ -674,8 +687,8 @@ class Query(object):
                            (','.join([str(id) for id in cached_ids])))
             
         sql.append("\nORDER BY ")
-        order_cols = [(self.order, self.desc)]
-        if self.group and self.group['field'] != self.order:
+        order_cols = [(self.order['field'], self.order['desc'])]
+        if self.group and self.group['field'] != self.order['field']:
             order_cols.insert(0, (self.group['field'], self.group['desc']))
 
         for name, desc in order_cols:
@@ -706,9 +719,9 @@ class Query(object):
                            % (desc, desc, col, desc))
             else:
                 sql.append("%s%s" % (col, desc))
-            if name == group_field and not name == self.order:
+            if name == group_field and not name == self.order['field']:
                 sql.append(",")
-        if self.order != 'id':
+        if self.order['field'] != 'id':
             sql.append(",t.id")  
 
         if errors:
@@ -767,11 +780,16 @@ class Query(object):
         wikify = set(f['name'] for f in self.fields 
                      if f['type'] == 'text' and f.get('format') == 'wiki')
 
+        def get_order_string(col):
+            order = field_from_string(col)
+            if col == self.order['field']:
+                order['desc'] = not self.order['desc']
+            return field_to_string(order)
         headers = [{
-            'name': col, 'label': labels.get(col, _('Ticket')),
+            'name': col,
+            'label': labels.get(col, _('Ticket')),
             'wikify': col in wikify,
-            'href': self.get_href(context.href, order=col,
-                                  desc=(col == self.order and not self.desc))
+            'href': self.get_href(context.href, order=get_order_string(col))
         } for col in cols]
 
         fields = {'id': {'type': 'id', 'label': _("Ticket")}}
@@ -963,12 +981,10 @@ class QueryModule(Component):
                 constraints = self._get_constraints(arg_list=arg_list)
             else:
                 query = Query.from_string(self.env, qstring)
-                args = {'order': query.order,
+                args = {'order': field_to_string(query.order),
                         'group': field_to_string(query.group),
                         'col': query.cols,
                         'max': query.max}
-                if query.desc:
-                    args['desc'] = '1'
                 constraints = query.constraints
 
             # Substitute $USER, or ensure no field constraints that depend
@@ -982,6 +998,11 @@ class QueryModule(Component):
                             del clause[field]
                             break
 
+        order = field_from_string(args.get('order'))
+        if order and 'desc' in args:
+            order['desc'] = True if args.get('desc') else False
+            args['order'] = field_to_string(order)
+
         group = field_from_string(args.get('group'))
         if group and 'groupdesc' in args:
             group['desc'] = True if args.get('groupdesc') else False
@@ -1003,7 +1024,7 @@ class QueryModule(Component):
             max = 0 # unlimited unless specified explicitly
         query = Query(self.env, report=req.args.get('report'),
                       constraints=constraints, cols=cols,
-                      order=args.get('order'), desc='desc' in args,
+                      order=args.get('order'),
                       group=args.get('group'),
                       verbose='verbose' in args, rows=rows,
                       page=args.get('page'), max=max)
diff --git a/trac/ticket/templates/query.html b/trac/ticket/templates/query.html
--- a/trac/ticket/templates/query.html
+++ b/trac/ticket/templates/query.html
@@ -211,7 +211,6 @@
         <div class="buttons">
           <input py:if="report_resource" type="hidden" name="report" value="$report_resource.id" />
           <input type="hidden" name="order" value="$query.order" />
-          <input py:if="query.desc" type="hidden" name="desc" value="1" />
           <input type="submit" name="update" value="${_('Update')}" />
         </div>
         <hr />
diff --git a/trac/ticket/templates/query_results.html b/trac/ticket/templates/query_results.html
--- a/trac/ticket/templates/query_results.html
+++ b/trac/ticket/templates/query_results.html
@@ -33,10 +33,10 @@
   <py:def function="column_headers()">
     <tr class="trac-columns">
       <th py:for="header in headers"
-          class="$header.name${(' desc' if query.desc else ' asc') if query.order == header.name else ''}">
+          class="$header.name${(' desc' if query.order['desc'] else ' asc') if query.order['field'] == header.name else ''}">
         <?python asc = _('(ascending)'); desc = _('(descending)') ?>
         <a title="${_('Sort by %(col)s %(direction)s', col=header.label,
-                      direction=(desc if query.order == header.name and not query.desc else asc))}"
+                      direction=(desc if query.order['field'] == header.name and not query.order['desc'] else asc))}"
            href="$header.href">${header.label}</a>
       </th>
     </tr>
diff --git a/trac/ticket/tests/query.py b/trac/ticket/tests/query.py
--- a/trac/ticket/tests/query.py
+++ b/trac/ticket/tests/query.py
@@ -56,6 +56,17 @@ ORDER BY COALESCE(t.id,0)=0,t.id""")
         tickets = query.execute(self.req)
 
     def test_all_ordered_by_id_desc(self):
+        query = Query(self.env, order='-id')
+        sql, args = query.get_sql()
+        self.assertEqualSQL(sql,
+"""SELECT t.id AS id,t.summary AS summary,t.owner AS owner,t.type AS type,t.status AS status,t.priority AS priority,t.milestone AS milestone,t.time AS time,t.changetime AS changetime,priority.value AS priority_value
+FROM ticket AS t
+  LEFT OUTER JOIN enum AS priority ON (priority.type='priority' AND priority.name=priority)
+ORDER BY COALESCE(t.id,0)=0 DESC,t.id DESC""")
+        self.assertEqual([], args)
+        tickets = query.execute(self.req)
+
+    def test_all_ordered_by_id_desc_old_format(self):
         query = Query(self.env, order='id', desc=1)
         sql, args = query.get_sql()
         self.assertEqualSQL(sql,
@@ -101,6 +112,18 @@ ORDER BY COALESCE(priority.value,'')='',
         tickets = query.execute(self.req)
 
     def test_all_ordered_by_priority_desc(self):
+        query = Query(self.env, order='-priority')
+        sql, args = query.get_sql()
+        self.assertEqualSQL(sql,
+"""SELECT t.id AS id,t.summary AS summary,t.owner AS owner,t.type AS type,t.status AS status,t.priority AS priority,t.milestone AS milestone,t.time AS time,t.changetime AS changetime,priority.value AS priority_value
+FROM ticket AS t
+  LEFT OUTER JOIN enum AS priority ON (priority.type='priority' AND priority.name=priority)
+ORDER BY COALESCE(priority.value,'')='' DESC,%(cast_priority)s DESC,t.id""" % {
+          'cast_priority': self.env.get_read_db().cast('priority.value', 'int')})
+        self.assertEqual([], args)
+        tickets = query.execute(self.req)
+
+    def test_all_ordered_by_priority_desc_old_format(self):
         query = Query(self.env, order='priority', desc=1)
         sql, args = query.get_sql()
         self.assertEqualSQL(sql,
@@ -125,6 +148,18 @@ ORDER BY COALESCE(t.version,'')='',COALE
         tickets = query.execute(self.req)
 
     def test_all_ordered_by_version_desc(self):
+        query = Query(self.env, order='-version')
+        sql, args = query.get_sql()
+        self.assertEqualSQL(sql,
+"""SELECT t.id AS id,t.summary AS summary,t.owner AS owner,t.type AS type,t.status AS status,t.priority AS priority,t.milestone AS milestone,t.version AS version,t.time AS time,t.changetime AS changetime,priority.value AS priority_value
+FROM ticket AS t
+  LEFT OUTER JOIN enum AS priority ON (priority.type='priority' AND priority.name=priority)
+  LEFT OUTER JOIN version ON (version.name=version)
+ORDER BY COALESCE(t.version,'')='' DESC,COALESCE(version.time,0)=0 DESC,version.time DESC,t.version DESC,t.id""")
+        self.assertEqual([], args)
+        tickets = query.execute(self.req)
+
+    def test_all_ordered_by_version_desc_old_format(self):
         query = Query(self.env, order='version', desc=1)
         sql, args = query.get_sql()
         self.assertEqualSQL(sql,
@@ -668,43 +703,42 @@ ORDER BY COALESCE(t.time,0)=0 DESC,t.tim
 
     def test_default_order_invalid_field_1(self):
         # invalid argument and no config
+        query = Query(self.env, order='-foo')
+        self.assertEqual({'field': 'priority', 'desc': False}, query.order)
+
+    def test_default_order_invalid_field_1_old_format(self):
+        # invalid argument and no config
         query = Query(self.env, order='foo', desc=1)
-        self.assertEqual('priority', query.order)
-        self.assertEqual(0, query.desc)
+        self.assertEqual({'field': 'priority', 'desc': False}, query.order)
 
     def test_default_order_invalid_field_2(self):
         # valid argument and no config
         query = Query(self.env, order='status')
-        self.assertEqual('status', query.order)
-        self.assertEqual(0, query.desc)
+        self.assertEqual({'field': 'status', 'desc': False}, query.order)
 
     def test_default_order_invalid_field_3(self):
         # no argument and invalid config
         self.env.config.set('query', 'default_order', '-foo')
         query = Query(self.env)
-        self.assertEqual('priority', query.order)
-        self.assertEqual(0, query.desc)
+        self.assertEqual({'field': 'priority', 'desc': False}, query.order)
 
     def test_default_order_invalid_field_4(self):
         # invalid argument and invalid config
         self.env.config.set('query', 'default_order', '-foo')
         query = Query(self.env, order='bar')
-        self.assertEqual('priority', query.order)
-        self.assertEqual(0, query.desc)
+        self.assertEqual({'field': 'priority', 'desc': False}, query.order)
 
     def test_default_order_invalid_field_5(self):
         # invalid argument but valid config
         self.env.config.set('query', 'default_order', '-milestone')
         query = Query(self.env, order='bar')
-        self.assertEqual('milestone', query.order)
-        self.assertEqual(1, query.desc)
+        self.assertEqual({'field': 'milestone', 'desc': True}, query.order)
 
     def test_default_order_invalid_field_6(self):
         # valid argument and valid config
         self.env.config.set('query', 'default_order', '-milestone')
         query = Query(self.env, order='type')
-        self.assertEqual('type', query.order)
-        self.assertEqual(0, query.desc)
+        self.assertEqual({'field': 'type', 'desc': False}, query.order)
 
     def test_default_cols_with_config_no_request(self):
         self.env.config.set('query', 'default_columns', 'id,time,changetime,summary,milestone,type')
diff --git a/trac/ticket/tests/wikisyntax.py b/trac/ticket/tests/wikisyntax.py
--- a/trac/ticket/tests/wikisyntax.py
+++ b/trac/ticket/tests/wikisyntax.py
@@ -235,7 +235,7 @@ query:summary=résumé
 <a class="query" href="/query?group=owner&amp;order=priority">query:group=owner</a>
 </p>
 <p>
-<a class="query" href="/query?row=description&amp;order=priority">query:verbose=1</a>
+<a class="query" href="/query?order=priority&amp;row=description">query:verbose=1</a>
 </p>
 <p>
 <a class="query" href="/query?summary=r%C3%A9sum%C3%A9&amp;order=priority">query:summary=résumé</a>