Bug 1750 - Report bor_issues_top erroneous and truncated results
Summary: Report bor_issues_top erroneous and truncated results
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: Main
Hardware: All All
: P3 normal (vote)
Assignee: Chris Cormack
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-01-07 10:32 UTC by Chris Cormack
Modified: 2016-12-05 21:25 UTC (History)
6 users (show)

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


Attachments
Bug 1750 - Report bor_issues_top erroneous and truncated results. (1.02 KB, patch)
2016-04-01 02:26 UTC, Mason James
Details | Diff | Splinter Review
before/after pic (181.41 KB, image/png)
2016-04-01 02:29 UTC, Mason James
Details
Bug 1750 - Report bor_issues_top erroneous and truncated results. (1.07 KB, patch)
2016-04-01 03:13 UTC, Mason James
Details | Diff | Splinter Review
Bug 1750 - Report bor_issues_top erroneous and truncated results. (1.53 KB, patch)
2016-04-01 03:58 UTC, Mark Tompsett
Details | Diff | Splinter Review
[PASSED QA] Bug 1750 - Report bor_issues_top erroneous and truncated results. (1.60 KB, patch)
2016-04-03 12:57 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Cormack 2010-05-21 00:34:10 UTC


---- Reported by joe.atzberger@liblime.com 2008-01-07 10:32:45 ----

Report w/ default params should return "Top 5".  When I have 5 items checked out amongst 3 users, my issues table looks like:

mysql> select borrowernumber,count(*) from issues GROUP BY borrowernumber;
+----------------+----------+
| borrowernumber | count(*) |
+----------------+----------+
|             51 |        1 | 
|             53 |        1 | 
|             64 |        3 | 
+----------------+----------+
3 rows in set (0.00 sec)


But the report outputs only 1 user:

  	Patron 	Issues count
1 Albertson Aleph 	1
2 		
3 		
4 		
5 		

Both the number of rows (users) displayed and the count are incorrect, though it correctly identified the top-most user.



---- Additional Comments From oleonard@myacpl.org 2008-04-09 09:40:38 ----

I'm having different problems with this report. When I try that SQL I get this:

+----------------+----------+
| borrowernumber | count(*) |
+----------------+----------+
|              9 |        1 |
|             51 |        6 |
|             52 |        1 |
+----------------+----------+

And when I run the default report I get this:

1  	Admin Koha   	22
2 	Welch Marcus 	22
3 	Daniels Tanya 	22
4 	Dillon Eva 	22
5 	Acosta Edna 	22

The borrowers are those with borrowernumbers 1-5.



---- Additional Comments From nengard@gmail.com 2009-12-23 17:58:44 ----

This bug hasn't been touched in nearly a year, please revisit and test and close if appropriate.



--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 00:34 UTC  ---

This bug was previously known as _bug_ 1750 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=1750

Actual time not defined. Setting to 0.0
The original reporter of this bug does not have
   an account here. Reassigning to the person who moved
   it here: chris@bigballofwax.co.nz.
   Previous reporter was joe.atzberger@liblime.com.

Comment 1 Mason James 2016-04-01 02:19:34 UTC
Woah, i think just hit this bug in 2016?!

I'll reopen this now..
Comment 2 Mason James 2016-04-01 02:26:34 UTC Comment hidden (obsolete)
Comment 3 Mason James 2016-04-01 02:29:14 UTC
Created attachment 49759 [details]
before/after pic
Comment 4 Mason James 2016-04-01 02:30:57 UTC
sorry, no test plan provided
Comment 5 Mason James 2016-04-01 02:34:34 UTC
FYI: the way this bug can be revealed is to have a borrower in the report with either a NULL 'surname'  or 'firstname' value

If this occurs, CONCAT() makes the borrower.name value NULL (bad!)
Comment 6 Mason James 2016-04-01 03:13:39 UTC Comment hidden (obsolete)
Comment 7 Mark Tompsett 2016-04-01 03:53:06 UTC
Comment on attachment 49762 [details] [review]
Bug 1750 - Report bor_issues_top erroneous and truncated results.

Review of attachment 49762 [details] [review]:
-----------------------------------------------------------------

::: reports/bor_issues_top.pl
@@ +279,4 @@
>      my $strcalc ;
>  
>  # Processing average loanperiods
> +    $strcalc .= "SELECT  CONCAT_WS('', borrowers.surname , \",\\t\", borrowers.firstname),  COUNT(*) AS RANK, borrowers.borrowernumber AS ID";

http://dev.mysql.com/doc/refman/5.7/en/string-functions.html#function_concat-ws
Nice! CONCAT_WS skips NULL entries, but still concat's, while CONCAT gives up and returns NULL.
Comment 8 Mark Tompsett 2016-04-01 03:58:26 UTC Comment hidden (obsolete)
Comment 9 Mark Tompsett 2016-04-01 04:00:03 UTC
URL deleted as it links to the WRONG location.
Comment 10 Katrin Fischer 2016-04-03 12:56:38 UTC
This works and I know that those scripts are problematic up to the point of 'should be rewritten from scratch'. So I am passing it.
Anyway, I think a better solution would be to get rid of the concat altogether and handle the existence of firstname and surname on template level including the ','.
Comment 11 Katrin Fischer 2016-04-03 12:57:49 UTC
Created attachment 49820 [details] [review]
[PASSED QA] Bug 1750 - Report bor_issues_top erroneous and truncated results.

Signed-off-by: Mason James <mtj@kohaaloha.com>

TEST PLAN
---------
1) Ensure you have some checkouts
2) Home -> Reports
        -> Patrons with the most checkouts
3) Click 'Submit'
   -- you get a list
4) Click a patron name.
5) Note the borrower number.
6) In MySQL run something like:
   > UPDATE borrower SET firstname=NULL WHERE borrowernumber=####
7) Refresh the report page
   -- name goes totally blank
8) apply patch
9) Refresh the report page
   -- only first name is lost
10) run koha qa test tools

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Brendan Gallagher 2016-04-22 03:22:08 UTC
Pushed to Master - Should be in the May 2016 Release.  Thanks!
Comment 13 Julian Maurice 2016-04-22 09:17:34 UTC
Patch pushed to 3.22.x, will be in 3.22.6
Comment 14 Frédéric Demians 2016-04-27 15:31:28 UTC
Pushed to 3.22.x, will be in 3.20.11.