Edgewall Software

Ticket #7424 (closed defect: fixed)

Opened 5 months ago

Last modified 4 months ago

Reports that use URL parameters break when paginating

Reported by: domluc@… Owned by: cboos
Priority: normal Milestone: 0.11.2
Component: report system Version: 0.11-stable
Severity: normal Keywords:
Cc:

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

Change History

  Changed 5 months ago by eblot

  • keywords http://trac-hacks.org/wiki/TimingAndEstimationPlugin removed
  • status changed from new to closed
  • resolution set to invalid
  • severity changed from critical to normal
  • milestone 0.11.1 deleted

About plugin, re-read NewTicket#Whentonotcreateatickethere

  Changed 5 months ago by cboos

  • status changed from closed to reopened
  • resolution invalid deleted
  • milestone set to 0.11.1

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.

  Changed 5 months ago by cboos

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

  Changed 5 months ago by domluc@…

@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!

follow-up: ↓ 6   Changed 5 months ago by 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 ;-)

in reply to: ↑ 5   Changed 5 months ago by domluc@…

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?

  Changed 5 months ago by cboos

  • owner set to cboos
  • status changed from reopened to new

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...

  Changed 5 months ago by domluc@…

  • status changed from new to closed
  • resolution set to fixed

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

I tested with some plugins and native reports too.

  Changed 5 months ago by cboos

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

follow-up: ↓ 12   Changed 4 months ago by anonymous

  • status changed from closed to reopened
  • resolution fixed deleted

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

  Changed 4 months ago by jonas

  • milestone changed from 0.11.1 to 0.11.2

in reply to: ↑ 10   Changed 4 months ago by rblank

  • status changed from reopened to closed
  • resolution set to fixed

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].

Add/Change #7424 (Reports that use URL parameters break when paginating)

Author



Change Properties
<Author field>
Action
as closed
Next status will be 'reopened'
to The owner will change from cboos. Next status will be 'closed'
 
Note: See TracTickets for help on using tickets.