http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13914 --- Comment #1 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 37267 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=37267&action=edit Bug 13914: Holds statistics report should add values from reserves and old_reserves And not pick one randomly... To reproduce: 1/ Go on Reports › Holds statistics 2/ Fill the form with: Hold status: All checked (Asked Processing Waiting Satisfied Cancelled) Hold Date From 01/01/2014 To 31/01/2014 Pickup library => empty Holding library => empty Home library => empty Row is "Hold status", Column is "Pickup library". 3/ Submit! 4/ Look at the results 5/ Restart mysql 6/ Ctrl+R to resubmit the form 7/ Look at the results The generated query looks like ( SELECT stuffs FROM reserves ) UNION ( SELECT stuffs FROM old_reserves ) It returns something like: +------+---------------+-------------+ | line | col | calculation | +------+---------------+-------------+ | 1 | BRANCHCODE1 | 1 | < from the reserves table | 1 | BRANCHCODE1 | 11 | < from the old_reserves table | 1 | BRANCHCODE2 | 2 | | 1 | BRANCHCODE2 | 22 | | 2 | BRANCHCODE1 | 4 | | 2 | BRANCHCODE1 | 44 | | 2 | BRANCHCODE2 | 5 | | 2 | BRANCHCODE2 | 55 | +------+---------------+-------------+ The screen will display 1 OR 11 for the BRANCHCODE1 line 1. Since ( line, col ) is not uniq, fetchall_hashref (l.280) was not a good idea here. This patch replaces it with fetchall_arrayref, reconstruct the variables used later and add the values from reserves and old_reserves. Test plan: Confirm that the results are now relevant. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13914 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=10188 -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13914 Eivin Giske Skaaren <eskaaren@yahoo.no> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |eskaaren@yahoo.no --- Comment #2 from Eivin Giske Skaaren <eskaaren@yahoo.no> --- I am still very new to Koha so please excuse (and explain) if I am thinking wrong here: The query that gets fired when testing this bug looks like this: (SELECT case when priority>0 then 1 when priority=0 then (case when found='f' then 4 when found='w' then (case when cancellationdate is null then 3 else 5 end ) else 2 end ) else 6 end line, reserves.branchcode col, COUNT(*) calculation FROM reserves LEFT JOIN borrowers USING (borrowernumber) WHERE reservedate >= '2015-04-15' AND reservedate <= '2015-04-30' AND (( case when priority>0 then 1 when priority=0 then (case when found='f' then 4 when found='w' then (case when cancellationdate is null then 3 else 5 end ) else 2 end ) else 6 end = '3' ) OR ( case when priority>0 then 1 when priority=0 then (case when found='f' then 4 when found='w' then (case when cancellationdate is null then 3 else 5 end ) else 2 end ) else 6 end = '1' ) OR ( case when priority>0 then 1 when priority=0 then (case when found='f' then 4 when found='w' then (case when cancellationdate is null then 3 else 5 end ) else 2 end ) else 6 end = '4' ) OR ( case when priority>0 then 1 when priority=0 then (case when found='f' then 4 when found='w' then (case when cancellationdate is null then 3 else 5 end ) else 2 end ) else 6 end = '2' ) OR ( case when priority>0 then 1 when priority=0 then (case when found='f' then 4 when found='w' then (case when cancellationdate is null then 3 else 5 end ) else 2 end ) else 6 end = '5' ) ) GROUP BY line, col ) UNION (SELECT case when priority>0 then 1 when priority=0 then (case when found='f' then 4 when found='w' then (case when cancellationdate is null then 3 else 5 end ) else 2 end ) else 6 end line, old_reserves.branchcode col, COUNT(*) calculation FROM old_reserves LEFT JOIN borrowers USING (borrowernumber) WHERE reservedate >= '2015-04-15' AND reservedate <= '2015-04-30' AND (( case when priority>0 then 1 when priority=0 then (case when found='f' then 4 when found='w' then (case when cancellationdate is null then 3 else 5 end ) else 2 end ) else 6 end = '3' ) OR ( case when priority>0 then 1 when priority=0 then (case when found='f' then 4 when found='w' then (case when cancellationdate is null then 3 else 5 end ) else 2 end ) else 6 end = '1' ) OR ( case when priority>0 then 1 when priority=0 then (case when found='f' then 4 when found='w' then (case when cancellationdate is null then 3 else 5 end ) else 2 end ) else 6 end = '4' ) OR ( case when priority>0 then 1 when priority=0 then (case when found='f' then 4 when found='w' then (case when cancellationdate is null then 3 else 5 end ) else 2 end ) else 6 end = '2' ) OR ( case when priority>0 then 1 when priority=0 then (case when found='f' then 4 when found='w' then (case when cancellationdate is null then 3 else 5 end ) else 2 end ) else 6 end = '5' ) ) GROUP BY line, col ) ORDER BY line, col So one way of getting line numbers that are not unique for the same col is if priority is set to >0 for an item in the old_reserves table. Question: Should priority ever be anything else than 0 for an item in the old_reserves table? The patch still gets the same SQL result and then tries to solve the problem by using the value of line as hash key and adds the values of calculation from any rows with the same line value. But this results in the wrong values being added given my example with priority for an item in the old_reserves table being set to >0. The query will as in Jonathan's example create rows with line value 1 for both the reserves table and the old_reserves table, so the line value should not be used as the hash key since it will then add values from the different tables to the calculation field. Perhaps this is better solved by getting the SQL correct for the use case, perhaps by adding a field to the result set that is unique? Also by looking at the reservestatus I suspect that something else is wrong here since line=1 corresponds to 1-placed which should be wrong for anything in the old_reserves table. This patch then does not solve the problem if the things that I have assumed are correct. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13914 --- Comment #4 from Eivin Giske Skaaren <eskaaren@yahoo.no> --- (In reply to Jonathan Druart from comment #3)

(In reply to Eivin Giske Skaaren from comment #2)

...

So one way of getting line numbers that are not unique for the same col is if priority is set to >0 for an item in the old_reserves table.

Question: Should priority ever be anything else than 0 for an item in the old_reserves table?

Yes, I think so. At least I have some on local installation. I suppose the priority is not updated when a reserve is canceled. So if it has a priority > 0, it will be copied as it to the old_reserves.

Since the query evaluates on priority this means that the result will be wrong if the rows from old_reserves are not to be shown as "1- placed" in the holds report.

Did you have the patience to test/compare the results? I didn't :)

Yes I did test it ;)

But the customer validated the patch, so I supposed he tested it. If you are able to give me a simple example to recreate a issue on my side, I will have a look and try to provide a fix.

This is my test result that you can modify to your installation: I have 2 books in the reserves table with priority 1 I have 3 books in the old_reserves table with priority 0 Run the report and it will say: 1- placed: 2 2- processed : 3 Change the priority to 1 for one of the books in the old_reserves table and rerun the report: 1- placed: 3 2- processed: 2 Perhaps it would be good to refactor out the use of magic numbers so the whole thing gets more maintainable. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13914 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=12082 -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13914 --- Comment #7 from Eivin Giske Skaaren <eivin@sysmystic.com> --- It definitely should be fixed sometime so not "resolved" or "wont fix". -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13914 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|jonathan.druart@biblibre.co |gmcharlt@gmail.com |m | Status|ASSIGNED |NEW -- You are receiving this mail because: You are watching all bug changes.