Edgewall Software
Modify

Ticket #7424 (closed defect: fixed)

Opened 4 years ago

Last modified 4 years 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:
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

Change History

comment:1 Changed 4 years ago by eblot

  • Keywords http://trac-hacks.org/wiki/TimingAndEstimationPlugin removed
  • Milestone 0.11.1 deleted
  • Resolution set to invalid
  • Severity changed from critical to normal
  • Status changed from new to closed

About plugin, re-read NewTicket#Whentonotcreateatickethere

comment:2 Changed 4 years ago by cboos

  • Milestone set to 0.11.1
  • Resolution invalid deleted
  • Status changed from closed to reopened

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 Changed 4 years ago by cboos

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

comment:4 Changed 4 years 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!

comment:5 follow-up: Changed 4 years 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 ;-)

comment:6 in reply to: ↑ 5 Changed 4 years 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?

comment:7 Changed 4 years 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…

comment:8 Changed 4 years ago by domluc@…

  • Resolution set to fixed
  • Status changed from new to closed

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

I tested with some plugins and native reports too.

comment:9 Changed 4 years 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).

comment:10 follow-up: Changed 4 years ago by anonymous

  • Resolution fixed deleted
  • Status changed from closed to reopened

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 Changed 4 years ago by jonas

  • Milestone changed from 0.11.1 to 0.11.2

comment:12 in reply to: ↑ 10 Changed 4 years ago by rblank

  • Resolution set to fixed
  • Status changed from reopened to closed

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

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
to The owner will be changed from cboos. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.