1. Odd Simon Simonsen
  2. trac-t10425

Source

trac-t10425 / new_group_format.diff

# HG changeset patch
# Parent b6671c2768564e26143ebece3ac97464346e7c9f

diff --git a/trac/ticket/query.py b/trac/ticket/query.py
--- a/trac/ticket/query.py
+++ b/trac/ticket/query.py
@@ -49,6 +49,29 @@ from trac.wiki.api import IWikiSyntaxPro
 from trac.wiki.macros import WikiMacroBase # TODO: should be moved in .api
 
 
+def field_to_string(d):
+    try:
+        s = ''
+        if d['desc']:
+            s = '-'
+        s += d['field']
+        return s
+    except (KeyError, TypeError), e:
+        pass
+    return None
+
+def field_from_string(s):
+    try:
+        if s[0] == '-':
+            return {'field': s[1:], 'desc': True}
+        elif s[0] == '+':
+            return {'field': s[1:], 'desc': False}
+        else:
+            return {'field': s, 'desc': False}
+    except (IndexError, TypeError), e:
+        pass
+    return None
+
 class QuerySyntaxError(TracError):
     """Exception raised when a ticket query cannot be parsed from a string."""
 
@@ -65,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=0, verbose=0,
+                 order=None, desc=0, 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
@@ -76,8 +99,6 @@ class Query(object):
         synonyms = TicketSystem(self.env).get_field_synonyms()
         self.order = synonyms.get(order, order)     # 0.11 compatibility
         self.desc = desc
-        self.group = group
-        self.groupdesc = groupdesc
         self.format = format
         self.default_page = 1
         self.items_per_page = QueryModule(self.env).items_per_page
@@ -135,14 +156,21 @@ class Query(object):
         if self.order != 'id' and self.order not in field_names:
             (self.order, self.desc) = ('priority', 0)
 
-        self.group = self.group or QueryModule(self.env).default_group
-        if self.group:
-            if self.group[0] == '-':
-                (self.group, self.groupdesc) = (self.group[1:], 1)
-            elif self.group[0] == '+':
-                (self.group, self.groupdesc) = (self.group[1:], 0)
-        if self.group not in field_names:
-            (self.group, self.groupdesc) = (None, 0)
+        if group == '':
+            self.group = group
+        else:
+            self.group = field_from_string(group)
+            if self.group and groupdesc is not None:
+                # 0.12 compatibility
+                self.group['desc'] = True if groupdesc else False
+
+            if not self.group or self.group['field'] not in field_names:
+                default_group = QueryModule(self.env).default_group
+                if default_group:
+                    self.group = field_from_string(default_group)
+                
+                if not self.group or self.group['field'] not in field_names:
+                    self.group = None
 
         constraint_cols = {}
         for clause in self.constraints:
@@ -282,8 +310,8 @@ class Query(object):
             cols.append(self.order)
 
         # Make sure to not show the column we group on.
-        if self.group and self.group in cols:
-            cols.remove(self.group)
+        if self.group and self.group['field'] in cols:
+            cols.remove(self.group['field'])
 
         return cols
 
@@ -420,11 +448,15 @@ class Query(object):
             constraints.append(("or", empty))
         del constraints[-1:]
         
+        # The empty string is an indicator for no group
+        group = ''
+        if self.group != '':
+            group = field_to_string(self.group)
+        
         return href.query(constraints,
                           report=id,
                           order=order, desc=1 if desc else None,
-                          group=self.group,
-                          groupdesc=1 if self.groupdesc else None,
+                          group=group,
                           col=cols,
                           row=self.rows,
                           max=max,
@@ -450,6 +482,7 @@ class Query(object):
             locale = req.locale
         self.get_columns()
         db = self.env.get_read_db()
+        group_field = self.group['field'] if self.group else None
 
         enum_columns = ('resolution', 'priority', 'severity')
         # Build the list of actual columns to query
@@ -458,8 +491,8 @@ class Query(object):
             for col in args:
                 if not col in cols:
                     cols.append(col)
-        if self.group and not self.group in cols:
-            add_cols(self.group)
+        if group_field and group_field not in cols:
+            add_cols(group_field)
         if self.rows:
             add_cols('reporter', *self.rows)
         add_cols('status', 'priority', 'time', 'changetime', self.order)
@@ -483,14 +516,14 @@ class Query(object):
 
         # Join with the enum table for proper sorting
         for col in [c for c in enum_columns
-                    if c == self.order or c == self.group or c == 'priority']:
+                    if c in (self.order, 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 == self.order or c == self.group]:
+                    if c in (self.order, group_field)]:
             sql.append("\n  LEFT OUTER JOIN %s ON (%s.name=%s)"
                        % (col, col, col))
 
@@ -642,8 +675,8 @@ class Query(object):
             
         sql.append("\nORDER BY ")
         order_cols = [(self.order, self.desc)]
-        if self.group and self.group != self.order:
-            order_cols.insert(0, (self.group, self.groupdesc))
+        if self.group and self.group['field'] != self.order:
+            order_cols.insert(0, (self.group['field'], self.group['desc']))
 
         for name, desc in order_cols:
             if name in enum_columns:
@@ -673,7 +706,7 @@ class Query(object):
                            % (desc, desc, col, desc))
             else:
                 sql.append("%s%s" % (col, desc))
-            if name == self.group and not name == self.order:
+            if name == group_field and not name == self.order:
                 sql.append(",")
         if self.order != 'id':
             sql.append(",t.id")  
@@ -771,7 +804,7 @@ class Query(object):
                 elif ticket['changetime'] > orig_time:
                     ticket['changed'] = True
             if self.group:
-                group_key = ticket[self.group]
+                group_key = ticket[self.group['field']]
                 groups.setdefault(group_key, []).append(ticket)
                 if not groupsequence or group_key not in groupsequence:
                     groupsequence.append(group_key)
@@ -930,12 +963,12 @@ class QueryModule(Component):
                 constraints = self._get_constraints(arg_list=arg_list)
             else:
                 query = Query.from_string(self.env, qstring)
-                args = {'order': query.order, 'group': query.group,
-                        'col': query.cols, 'max': query.max}
+                args = {'order': query.order,
+                        'group': field_to_string(query.group),
+                        'col': query.cols,
+                        'max': query.max}
                 if query.desc:
                     args['desc'] = '1'
-                if query.groupdesc:
-                    args['groupdesc'] = '1'
                 constraints = query.constraints
 
             # Substitute $USER, or ensure no field constraints that depend
@@ -949,6 +982,11 @@ class QueryModule(Component):
                             del clause[field]
                             break
 
+        group = field_from_string(args.get('group'))
+        if group and 'groupdesc' in args:
+            group['desc'] = True if args.get('groupdesc') else False
+            args['group'] = field_to_string(group)
+        
         cols = args.get('col')
         if isinstance(cols, basestring):
             cols = [cols]
@@ -963,13 +1001,12 @@ class QueryModule(Component):
         max = args.get('max')
         if max is None and format in ('csv', 'tab'):
             max = 0 # unlimited unless specified explicitly
-        query = Query(self.env, req.args.get('report'),
-                      constraints, cols, args.get('order'),
-                      'desc' in args, args.get('group'),
-                      'groupdesc' in args, 'verbose' in args,
-                      rows,
-                      args.get('page'), 
-                      max)
+        query = Query(self.env, report=req.args.get('report'),
+                      constraints=constraints, cols=cols,
+                      order=args.get('order'), desc='desc' in args,
+                      group=args.get('group'),
+                      verbose='verbose' in args, rows=rows,
+                      page=args.get('page'), max=max)
 
         if 'update' in req.args:
             # Reset session vars
@@ -1423,18 +1460,18 @@ class TicketQueryMacro(WikiMacroBase):
 
         def ticket_groups():
             groups = []
-            for v, g in groupby(tickets, lambda t: t[query.group]):
+            for v, g in groupby(tickets, lambda t: t[query.group.get('field')]):
                 q = Query.from_string(self.env, query_string)
                 # produce the hint for the group
-                q.group = q.groupdesc = None
+                q.group = None
                 order = q.order
                 q.order = None
                 title = _("%(groupvalue)s %(groupname)s tickets matching "
-                          "%(query)s", groupvalue=v, groupname=query.group,
+                          "%(query)s", groupvalue=v, groupname=query.group.get('field'),
                           query=q.to_string())
                 # produce the href for the query corresponding to the group
                 for constraint in q.constraints:
-                    constraint[str(query.group)] = v
+                    constraint[str(query.group.get('field'))] = v
                 q.order = order
                 href = q.get_href(formatter.context)
                 groups.append((v, [t for t in g], href, title))
@@ -1456,7 +1493,7 @@ class TicketQueryMacro(WikiMacroBase):
                     [(tag.p(tag_('%(groupvalue)s %(groupname)s tickets:',
                                  groupvalue=tag.a(v, href=href, class_='query',
                                                   title=title),
-                                 groupname=query.group)),
+                                 groupname=query.group.get('field'))),
                       tag.dl([(tag.dt(ticket_anchor(t)),
                                tag.dd(t['summary'])) for t in g],
                              class_='wiki compact'))
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
@@ -184,12 +184,12 @@
             <option></option>
             <py:for each="field_name in field_names" py:with="field = fields[field_name]">
               <option py:if="field.type in ('select', 'radio') or field_name in ('owner', 'reporter')"
-                      selected="${field_name == query.group or None}"
+                      selected="${query.group and field_name == query.group.get('field') or None}"
                       value="${field_name}">${field.label}</option>
             </py:for>
           </select>
           <input type="checkbox" name="groupdesc" id="groupdesc"
-                 checked="${query.groupdesc or None}" />
+                 checked="${query.group and query.group.get('desc') or None}" />
           <label for="groupdesc">descending</label>
         </p>
 
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
@@ -24,8 +24,8 @@
   <py:def function="group_heading(groupname, results)">
     <h2 class="report-result" py:if="groupname is not None"
         i18n:msg="grouplabel, groupname, count"
-        py:with="grouplabel = fields[query.group].label;
-                 groupname = authorinfo(groupname) if query.group in ['owner', 'reporter'] else (groupname or _('None'));
+        py:with="grouplabel = fields[query.group.get('field')].label;
+                 groupname = authorinfo(groupname) if query.group and query.group.get('field') in ['owner', 'reporter'] else (groupname or _('None'));
                  count = ngettext('%(num)s match', '%(num)s matches', len(results))">
       ${grouplabel}: ${groupname} <span class="numrows">(${count})</span>
     </h2>
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
@@ -161,6 +161,18 @@ ORDER BY COALESCE(t.milestone,'')='',COA
         tickets = query.execute(self.req)
 
     def test_all_grouped_by_milestone_desc(self):
+        query = Query(self.env, order='id', group='-milestone')
+        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)
+  LEFT OUTER JOIN milestone ON (milestone.name=milestone)
+ORDER BY COALESCE(t.milestone,'')='' DESC,COALESCE(milestone.completed,0)=0 DESC,milestone.completed DESC,COALESCE(milestone.due,0)=0 DESC,milestone.due DESC,t.milestone DESC,COALESCE(t.id,0)=0,t.id""")
+        self.assertEqual([], args)
+        tickets = query.execute(self.req)
+
+    def test_all_grouped_by_milestone_desc_old_format(self):
         query = Query(self.env, order='id', group='milestone', groupdesc=1)
         sql, args = query.get_sql()
         self.assertEqualSQL(sql,
@@ -559,43 +571,42 @@ ORDER BY COALESCE(t.time,0)=0 DESC,t.tim
 
     def test_default_group_invalid_field_1(self):
         # invalid argument and no config
+        query = Query(self.env, group='-foo')
+        self.assertEqual(None, query.group)
+
+    def test_default_group_invalid_field_1_old_format(self):
+        # invalid argument and no config
         query = Query(self.env, group='foo', groupdesc=1)
         self.assertEqual(None, query.group)
-        self.assertEqual(0, query.groupdesc)
 
     def test_default_group_invalid_field_2(self):
         # valid argument and no config
         query = Query(self.env, group='status')
-        self.assertEqual('status', query.group)
-        self.assertEqual(0, query.groupdesc)
+        self.assertEqual({'field': 'status', 'desc': False}, query.group)
 
     def test_default_group_invalid_field_3(self):
         # no argument and invalid config
         self.env.config.set('query', 'default_group', '-foo')
         query = Query(self.env)
         self.assertEqual(None, query.group)
-        self.assertEqual(0, query.groupdesc)
 
     def test_default_group_invalid_field_4(self):
         # invalid argument and invalid config
         self.env.config.set('query', 'default_group', '-foo')
         query = Query(self.env, group='bar')
         self.assertEqual(None, query.group)
-        self.assertEqual(0, query.groupdesc)
 
     def test_default_group_invalid_field_5(self):
         # invalid argument but valid config
         self.env.config.set('query', 'default_group', '-milestone')
         query = Query(self.env, group='bar')
-        self.assertEqual('milestone', query.group)
-        self.assertEqual(1, query.groupdesc)
+        self.assertEqual({'field': 'milestone', 'desc': True}, query.group)
 
     def test_default_group_invalid_field_6(self):
         # valid argument and valid config
         self.env.config.set('query', 'default_group', '-milestone')
         query = Query(self.env, group='type')
-        self.assertEqual('type', query.group)
-        self.assertEqual(0, query.groupdesc)
+        self.assertEqual({'field': 'type', 'desc': False}, query.group)
 
     def test_default_order_no_config_with_request(self):
         query = Query(self.env, order='summary')
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?order=priority&amp;row=description">query:verbose=1</a>
+<a class="query" href="/query?row=description&amp;order=priority">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>