Bug 16184 - Report bor_issues_top shows incorrect number of rows
Summary: Report bor_issues_top shows incorrect number of rows
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Galen Charlton
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-04-01 02:58 UTC by Mason James
Modified: 2016-12-05 21:25 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:

Attachments
Bug 16184 - Report bor_issues_top shows incorrect number of rows (1.23 KB, patch)
2016-04-01 03:01 UTC, Mason James 		Details | Diff | Splinter Review
before/after pic (169.86 KB, image/png)
2016-04-01 03:03 UTC, Mason James 		Details
Bug 16184 - Report bor_issues_top shows incorrect number of rows (1.63 KB, patch)
2016-04-01 04:32 UTC, Mark Tompsett 		Details | Diff | Splinter Review
[PASSED QA] Bug 16184 - Report bor_issues_top shows incorrect number of rows (1.70 KB, patch)
2016-04-03 13:06 UTC, Katrin Fischer 		Details | Diff | Splinter Review
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Mason James 2016-04-01 03:00:04 UTC 
This report always returns double the number of rows requested
Comment 2 Mason James 2016-04-01 03:01:51 UTC Comment hidden (obsolete)
Comment 3 Mason James 2016-04-01 03:03:03 UTC 
Created attachment 49761 [details]
before/after pic
Comment 4 Mason James 2016-04-01 03:04:51 UTC 
A report with a LIMIT of 5 rows, will incorrectly return 10 rows (bad!)
Comment 5 Mark Tompsett 2016-04-01 04:21:55 UTC 
I'm not sure whether this patch is the best way to do it.
The whole query issues and old_issues via Perl seems like the wrong way to handle the logic. I'm sure a better SQL query could be built with a UNION that way the LIMIT and GROUP BY would be handled by MySQL instead.
Comment 6 Mark Tompsett 2016-04-01 04:32:08 UTC Comment hidden (obsolete)
Comment 7 Katrin Fischer 2016-04-03 13:06:46 UTC 
Created attachment 49821 [details] [review]
[PASSED QA] Bug 16184 - Report bor_issues_top shows incorrect number of rows

TEST PLAN
---------
1) Have at least 6 patrons with checkouts and some checkins.
2) Reports -> Patrons with the most checkouts
3) Click 'Submit' (default is 5)
   -- more than 5 entries listed.
4) Apply patch
5) Refresh page
   -- only 5 entries listed.
6) Run koha qa test tools

NOTE: While this works, I'd be much happier with a refactor
      as it would also speed up the report. See comment #5.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Brendan Gallagher 2016-04-06 22:11:01 UTC 
Pushed to Master - Should be in the May 2016 Release.  Thanks!
Comment 9 Julian Maurice 2016-04-07 08:22:18 UTC 
Patch pushed to 3.22.x, will be in 3.22.6
Comment 10 Frédéric Demians 2016-04-27 15:35:37 UTC 
Pushed to 3.22.x, will be in 3.20.11.