Edgewall Software

Opened 9 years ago

Closed 8 years ago

Last modified 8 years ago

#10838 closed defect (fixed)

TicketQuery progress display link ignores order= keys in milestone_groups query_args causing query error — at Version 9

Reported by: ilewismsl Owned by: Peter Suter
Priority: normal Milestone: 1.0.2
Component: ticket system Version: 1.0
Severity: minor Keywords: TicketQuery progress
Cc: Branch:
Release Notes:

Fixed error in TicketQuery(format=progress) macro when e.g. order keys appear in query_args of milestone_groups.

API Changes:
Internal Changes:

Description

Using the macro invocation

[[TicketQuery(owner=ian, format=progress)]]

in a wiki page creates a progress bar with links for various classes of ticket.

The resulting progress bar creates query URLs that work for all classes of ticket except for closed tickets. For closed tickets it produces a link that looks like:

/trac/msl/query?owner=ian&status=closed&group=resolution&order=time&col=id&col=summary&col=owner&col=type&col=priority&col=component&col=severity&col=time&col=changetime&max=0&order=id

This URL includes two order= keys: order=time near the begining and order=id at the end.

This confuses Trac and results in a runtime error.

All other classes of ticket, such as accepted tickets produce a link that looks like:

/trac/msl/query?owner=ian&status=accepted&max=0&order=id

with a single order=id statement.

The source of this error is that our milestone groups includes an order=time key.

We got this by direct copying from the sample at TracIni without much thought about what it meant.

[milestone-groups]
...
# optional extra param for the query (two additional columns: created and modified and sort on created)
closed.query_args = group=resolution,order=time,col=id,col=summary,col=owner,col=type,col=priority,col=component,col=severity,col=time,col=changetime
...

Error message

Trac detected an internal error:
TypeError: unhashable type: 'list'

Python traceback

Most recent call last: 
File "build/bdist.win32/egg/trac/web/main.py", line 497, in _dispatch_request
  dispatcher.dispatch(req)
File "build/bdist.win32/egg/trac/web/main.py", line 214, in dispatch
  resp = chosen_handler.process_request(req)
File "build/bdist.win32/egg/trac/ticket/query.py", line 939, in process_request
  max)
File "build/bdist.win32/egg/trac/ticket/query.py", line 77, in __init__
  self.order = synonyms.get(order, order)     # 0.11 compatibility

System information

User Agent: Mozilla/5.0 (compatible; MSIE 9.0; Windows NT 6.1; WOW64; Trident/5.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; .NET4.0C; .NET4.0E)

Trac 1.0
Docutils 0.8.1
Genshi 0.6 (without speedups)
mod_python 3.3.1
Pygments 1.4
pysqlite 2.4.1
Python 2.6.6 (r266:84297, Aug 24 2010, 18:46:32) [MSC v.1500 32 bit (Intel)]
setuptools 0.6c11
SQLite 3.5.9
Subversion 1.7.1 (r1186859)
jQuery 1.7.2

Change History (13)

comment:1 by Christian Boos, 9 years ago

Milestone: next-stable-1.0.x

In fact this happens as soon as order is specified multiple times, as #10907 demonstrates.

So we have to be robust against this condition (and even do something with that, one day → #10178), and also find out why the progress type generates multiple values for order.

in reply to:  1 comment:2 by Peter Suter, 9 years ago

Replying to cboos:

and also find out why the progress type generates multiple values for order.

I guess the problem is here, where the [milestone-groups] .query_args alias qry_args are appended to the other constraints.

(I think I was trying to follow roadmap.milestone_stats_data.query_href, but there the "other constraints" can not include order.)

by Peter Suter, 9 years ago

comment:3 by Peter Suter, 9 years ago

In this patch I try to let Query.from_string deal with these additional arguments.

Adding some unit tests might be a good idea.

by Peter Suter, 9 years ago

Attempt at unit test

comment:4 by Peter Suter, 9 years ago

I tried writing a test for this, but have some questions:

  • Should this go in trac/ticket/tests/query.py (already has TicketQueryMacroTestCase, but that does not really expand the macro) or trac/ticket/tests/wikisyntax.py (already has QUERY_TEST_CASES and QUERY2_TEST_CASES that do expand the macro) or somewhere else?
  • req.chrome and req.session are required for this. (e.g. in render_template) These were apparently not required for any other such test. Is this a flaw in this macro or in the test setup? Does simply adding empty dictionaries for these make sense or should I do something else? Should I only add them for this one test case?
  • I copied the ticket_setup / ticket_teardown to tweak [milestone-groups]. Should this instead be merged into the old testcase (possibly changing the existing reference output)?

comment:5 by Peter Suter, 9 years ago

Owner: set to Peter Suter
Status: newassigned

#11065 was closed as a duplicate.

comment:6 by Peter Suter, 8 years ago

Milestone: next-stable-1.0.x1.0.2

comment:7 by Peter Suter, 8 years ago

req.chrome and req.session are required for [testing] this.

I now see that many tests do this in the Mock() constructor.

The suggested fix in the attached patch has problems if &or& is used.

Last edited 8 years ago by Peter Suter (previous) (diff)

by Peter Suter, 8 years ago

comment:8 by Peter Suter, 8 years ago

attachment:T10838-TicketQueryProgressOrder-2.patch solves the &or& issue and includes more tests with the Mock(chrome={}, session={}) change.

comment:9 by Peter Suter, 8 years ago

Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

Committed in [12615] and merged in [12616].

by Jun Omae, 8 years ago

Attachment: py25-sqlite.log.gz added
Note: See TracTickets for help on using tickets.