Edgewall Software
Modify

Opened 11 years ago

Closed 11 years ago

#7424 closed defect (fixed)

Reports that use URL parameters break when paginating

Reported by: domluc@… Owned by: Christian Boos
Priority: normal Milestone: 0.11.2
Component: report system Version: 0.11-stable
Severity: normal Keywords:
Cc: Branch:
Release Notes:
API Changes:

Description

Some plugins (ie:TimingAndEstimationPlugin) add custom reports to trac and use aditional parameters, like:

http://example.com/trac/report/63?BILLABLE=1&UNBILLABLE=1&NOVO=novo&EMANDAMENTO=em_andamento&REABERTO=reaberto&RESOLVIDO=resolvido&FECHADO=fechado&TESTE=teste&ATRIBUIDO=atribuido&NEW=new&VERIFICADO=verificado&STARTDATE=0&ENDDATE=2000000000

that report on first load, works fine, but page '2' link breaks, pointing to

http://example.com/trac/report/63?page2

loosing URL query info, needed by report.

Plugin site: http://trac-hacks.org/wiki/TimingAndEstimationPlugin

Attachments (0)

Change History (12)

comment:1 by Emmanuel Blot, 11 years ago

Keywords: http://trac-hacks.org/wiki/TimingAndEstimationPlugin removed
Milestone: 0.11.1
Resolution: invalid
Severity: criticalnormal
Status: newclosed

About plugin, re-read NewTicket#Whentonotcreateatickethere

comment:2 by Christian Boos, 11 years ago

Milestone: 0.11.1
Resolution: invalid
Status: closedreopened

No, I think it's a rightful one.

I haven't verified yet, but from the report it seems that extra request parameters are not part of the paging links, which could then break any report which expects extra report variabes.

comment:3 by Christian Boos, 11 years ago

Easy to verify actually: report:14?PRIORITY=high → then click on a page link.

comment:4 by domluc@…, 11 years ago

@eblot:you don't need to close a ticket just 'cause you're guessing that is invalid, read ticket before , not personally, but this way you will loose users contrib.

@cboos: perfect!

comment:5 by Christian Boos, 11 years ago

well, well, you can't imagine how many such invalid reports we get, especially for the AccountManager plugin, so it's basically a reflex when we see it mentioned in a ticket, and I nearly did it myself as well ;-)

in reply to:  5 comment:6 by domluc@…, 11 years ago

Replying to cboos:

well, well, you can't imagine how many such invalid reports we get, especially for the AccountManager plugin, so it's basically a reflex when we see it mentioned in a ticket, and I nearly did it myself as well ;-)

I understood, and really isn't personally with eblot.but let's leave this and focus on ticket :-). I'll try to make a patch next weekend, some tip in how i can get parameters like in query module?

comment:7 by Christian Boos, 11 years ago

Owner: set to Christian Boos
Status: reopenednew

Would have loved to get a patch from you, but as I was looking at the code for providing the hints you asked for, I also saw the bug ;-)

  • trac/ticket/report.py

     
    323323            shown_pages = paginator.get_shown_pages(21)
    324324            for p in shown_pages:
    325325                pagedata.append([req.href.report(id, asc=asc, sort=sort_col,
    326                                                  USER=user, page=p),
     326                                                 page=p, **args),
    327327                                 None, str(p), _('Page %(num)d', num=p)])
    328328
    329329            fields = ['href', 'class', 'string', 'title']

So now you can help by testing it…

comment:8 by domluc@…, 11 years ago

Resolution: fixed
Status: newclosed

Ow..this is fast! I just apply this directly and works quite fine.

I tested with some plugins and native reports too.

comment:9 by Christian Boos, 11 years ago

Ok, patch applied in r7346 (and sorry, I apparently have a bug in my commit script, as the message didn't get through).

comment:10 by anonymous, 11 years ago

Resolution: fixed
Status: closedreopened

The applied patch works for paging, but it doesn't include changes for navigation arrows for previous and next. They don't work properly.

comment:11 by Jonas Borgström, 11 years ago

Milestone: 0.11.10.11.2

in reply to:  10 comment:12 by Remy Blank, 11 years ago

Resolution: fixed
Status: reopenedclosed

Replying to anonymous:

The applied patch works for paging, but it doesn't include changes for navigation arrows for previous and next. They don't work properly.

Right, the same fix is needed for these links, as well as the query_href session parameter, otherwise the "Back to Query" link in the ticket view doesn't work either.

Fixed in [7476].

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Christian Boos.
The resolution will be deleted. Next status will be 'reopened'.
to as closed The owner will be changed from Christian Boos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.