Edgewall Software
Modify

Ticket #7267 (closed defect: fixed)

Opened 4 years ago

Last modified 4 years ago

Pagination variables are temporarly shared betweens user sessions in ReportModule

Reported by: farialima@… Owned by: cboos
Priority: normal Milestone: 0.11
Component: report system Version: 0.11rc1
Severity: major Keywords:
Cc:
Release Notes:
API Changes:

Description (last modified by cboos) (diff)

[6901] has added pagination handling for report. From looking at the code, it adds data attributes ('page', 'limit', 'num_rows', as well as 'asc', 'sort', 'USER') to the class "ReportModule?" (see source:/trunk/trac/ticket/report.py@6901#L25 ) . But, AFAIK, only one instance of each "Component" class (of which ReportModule? inherits) is shared across all user requests, so I believe that these attributes will be shared across users. Or maybe I'm reading the code wrong...

The risk is low, but unpredictable: it will be a problem when two users request a (different) report at the same time; in that case, the result will be unpredictable.

It also makes it impossible to use the ReportModule?.execute_report without first settings the 'limit' attribute (which is why I caught this issue: I had some automated tests that would call execute_report and that suddenly started failing...)

Why not instead add an optional parameter to execute_report instead, that would contain (and return, as attributes) the pagination information ?

I can propose a patch if you want,

Again, maybe I'm wrong and not reading the code correctly...

Attachments

fix-report-pagination-r7157.diff (6.1 KB) - added by cboos 4 years ago.
Should fix the scoping issues for report pagination

Download all attachments as: .zip

Change History

comment:1 Changed 4 years ago by francois@…

the code is actually on line source:/trunk/trac/ticket/report.py@6901#L293, not 25 as I wrote above

comment:2 Changed 4 years ago by cboos

  • Description modified (diff)
  • Milestone set to 0.11

Oh, you're completely right, this has been overlooked when cleaning up the #216 patch.

comment:3 Changed 4 years ago by cboos

  • Description modified (diff)

([OT] looks like we also have a problem with eating up the blank lines of a ticket description, as the (diff) above happened without any expliciting editing from my part)

Changed 4 years ago by cboos

Should fix the scoping issues for report pagination

comment:4 Changed 4 years ago by cboos

  • Owner changed from mgood to cboos
  • Status changed from new to assigned

The patch above should fix the issue and apparently doesn't break things - but more tests are welcomed.

comment:5 Changed 4 years ago by cboos

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

Patch checked in as r7212 for 0.11 and as part of r7214 for 0.12.

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.