1. Odd Simon Simonsen
  2. trac-t10425

Commits

Odd Simon Simonsen  committed 61cdc51

New alternative group format that just stores the "(-)field" and uses helper to extract the group name where needed.

Note: The other new_format_* patches are just commented out in `series` file. Edit `series` to select between the "mini-stacks" of patches for alternative approaches. It is just a text file, but just don't edit lines with patches that are already applied.

  • Participants
  • Parent commits e807b2e
  • Branches default

Comments (0)

Files changed (2)

File new_group_format2.diff

View file
  • Ignore whitespace
+# HG changeset patch
+# Parent 1ffa7261f6b763e9c79817d820e0794993810ec7
+
+diff --git a/trac/ticket/query.py b/trac/ticket/query.py
+--- a/trac/ticket/query.py
++++ b/trac/ticket/query.py
+@@ -76,8 +76,9 @@
+         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
++        if groupdesc and group and group[0] not in ['-', '+']:
++            group = '-' + group
++        self.group = group or QueryModule(self.env).default_group
+         self.format = format
+         self.default_page = 1
+         self.items_per_page = QueryModule(self.env).items_per_page
+@@ -121,10 +122,10 @@
+         self.fields = TicketSystem(self.env).get_ticket_fields()
+         self.time_fields = set(f['name'] for f in self.fields
+                                if f['type'] == 'time')
+-        field_names = set(f['name'] for f in self.fields)
+-        self.cols = [c for c in cols or [] if c in field_names or 
++        self.field_names = set(f['name'] for f in self.fields)
++        self.cols = [c for c in cols or [] if c in self.field_names or 
+                      c == 'id']
+-        self.rows = [c for c in rows if c in field_names]
++        self.rows = [c for c in rows if c in self.field_names]
+ 
+         self.order = self.order or QueryModule(self.env).default_order
+         if self.order:
+@@ -132,22 +133,13 @@
+                 (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:
++        if self.order != 'id' and self.order not in self.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)
+-
+         constraint_cols = {}
+         for clause in self.constraints:
+             for k, v in clause.items():
+-                if k == 'id' or k in field_names:
++                if k == 'id' or k in self.field_names:
+                     constraint_cols.setdefault(k, []).append(v)
+                 else:
+                     clause.pop(k)
+@@ -282,11 +274,20 @@
+             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)
++        group = self.get_group()
++        if group and group in cols:
++            cols.remove(group)
+ 
+         return cols
+ 
++    def get_group(self):
++        group = self.group
++        if group and group[0] in ['-', '+']:
++            group = group[1:]
++        if group and not group in self.field_names:
++            return None
++        return group
++
+     def count(self, req=None, db=None, cached_ids=None, authname=None,
+               tzinfo=None, locale=None):
+         """Get the number of matching tickets for the present query.
+@@ -325,7 +326,7 @@
+ 
+             if self.has_more_pages:
+                 max = self.max
+-                if self.group:
++                if self.get_group():
+                     max += 1
+                 sql = sql + " LIMIT %d OFFSET %d" % (max, self.offset)
+                 if (self.page > int(ceil(float(self.num_items) / self.max)) and
+@@ -424,7 +425,6 @@
+                           report=id,
+                           order=order, desc=1 if desc else None,
+                           group=self.group,
+-                          groupdesc=1 if self.groupdesc else None,
+                           col=cols,
+                           row=self.rows,
+                           max=max,
+@@ -458,8 +458,9 @@
+             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)
++        group = self.get_group()
++        if group and not group in cols:
++            add_cols(group)
+         if self.rows:
+             add_cols('reporter', *self.rows)
+         add_cols('status', 'priority', 'time', 'changetime', self.order)
+@@ -483,14 +484,14 @@
+ 
+         # 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 == self.order or c == group or c == '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 == self.order or c == group]:
+             sql.append("\n  LEFT OUTER JOIN %s ON (%s.name=%s)"
+                        % (col, col, col))
+ 
+@@ -642,8 +643,9 @@
+             
+         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))
++        group = self.get_group()
++        if group and group != self.order:
++            order_cols.insert(0, (group, 1 if self.group[0] == '-' else 0))
+ 
+         for name, desc in order_cols:
+             if name in enum_columns:
+@@ -673,7 +675,7 @@
+                            % (desc, desc, col, desc))
+             else:
+                 sql.append("%s%s" % (col, desc))
+-            if name == self.group and not name == self.order:
++            if name == self.get_group() and not name == self.order:
+                 sql.append(",")
+         if self.order != 'id':
+             sql.append(",t.id")  
+@@ -760,6 +762,7 @@
+                     for (label, milestones) in groups]
+             fields[name] = field
+ 
++        group = self.get_group()
+         groups = {}
+         groupsequence = []
+         for ticket in tickets:
+@@ -770,8 +773,8 @@
+                     ticket['added'] = True
+                 elif ticket['changetime'] > orig_time:
+                     ticket['changed'] = True
+-            if self.group:
+-                group_key = ticket[self.group]
++            if group:
++                group_key = ticket[group]
+                 groups.setdefault(group_key, []).append(ticket)
+                 if not groupsequence or group_key not in groupsequence:
+                     groupsequence.append(group_key)
+@@ -934,8 +937,6 @@
+                         '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 +950,12 @@
+                             del clause[field]
+                             break
+ 
++        # 0.12 groupdesc compat
++        group = args.get('group')
++        groupdesc = True if args.get('groupdesc') else False
++        if group and groupdesc and not group[0] in ['-', '+']:
++            args['group'] = '-' + group
++        
+         cols = args.get('col')
+         if isinstance(cols, basestring):
+             cols = [cols]
+@@ -963,13 +970,11 @@
+         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 +1428,18 @@
+ 
+         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.get_group()]):
+                 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=q.to_string())
++                          "%(query)s", groupvalue=v,
++                          groupname=query.get_group(), 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.get_group())] = v
+                 q.order = order
+                 href = q.get_href(formatter.context)
+                 groups.append((v, [t for t in g], href, title))
+@@ -1456,7 +1461,7 @@
+                     [(tag.p(tag_('%(groupvalue)s %(groupname)s tickets:',
+                                  groupvalue=tag.a(v, href=href, class_='query',
+                                                   title=title),
+-                                 groupname=query.group)),
++                                 groupname=query.get_group())),
+                       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="${field_name == query.get_group() 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.get_group() and query.group[0] == '-' 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.get_group()].label;
++                 groupname = authorinfo(groupname) if query.get_group() 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,7 +161,7 @@
+         tickets = query.execute(self.req)
+ 
+     def test_all_grouped_by_milestone_desc(self):
+-        query = Query(self.env, order='id', group='milestone', groupdesc=1)
++        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
+@@ -560,42 +560,42 @@
+     def test_default_group_invalid_field_1(self):
+         # invalid argument and no config
+         query = Query(self.env, group='foo', groupdesc=1)
+-        self.assertEqual(None, query.group)
+-        self.assertEqual(0, query.groupdesc)
++        self.assertEqual(None, query.get_group())
++        self.assertEqual('-', query.group[0])
+ 
+     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('status', query.get_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)
++        self.assertEqual(None, query.get_group())
++        self.assertEqual('-foo', query.group)
+ 
+     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)
++        self.assertEqual(None, query.get_group())
++        self.assertEqual('bar', query.group)
+ 
+     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('milestone', query.get_group())
++        self.assertEqual('-', 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('type', query.get_group())
+ 
+     def test_default_order_no_config_with_request(self):
+         query = Query(self.env, order='summary')

File series

View file
  • Ignore whitespace
 
-# Support for option to specify default columns instead of 
-# hard-coded in code.
+
+# Basic simple patches to allow for defaults for columns, group and order
 default_columns.diff
 default_order.diff
 default_group.diff
-new_group_format.diff
-new_order_format.diff
+new_group_format2.diff
+
+# Alternative 1: Azoff new format patches
+#new_group_format.diff
+#new_order_format.diff
+
+# Alternative 2: simons new format patches