[Bug 23059] New: reserves_stats.pl needs overhaul
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 Bug ID: 23059 Summary: reserves_stats.pl needs overhaul Change sponsored?: --- Product: Koha Version: 18.11 Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Reports Assignee: koha-bugs@lists.koha-community.org Reporter: andrew@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org Target Milestone: --- This bug is sort of a catch-all for changes that need to be made to the holds statistic wizard, reserves_stats.pl Two big issues: - By default, the wizard sets a value for items.homebranch and items.holdingbranch. That means it unexpectedly cuts out all bib-level holds and gives no clear indication that it's done so. You *can* change home and holding branch to not limit, but it's not intuitive. - The wizard splits holds into 5 statuses (Placed, Processed, Pending, Satisfied, and Cancelled) that aren't used anywhere else in Koha. These statuses aren't explained without diving into the code and don't match how we otherwise divide and talk about holds. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 Andrew <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 Ron Houk <rhouk@ottumwapubliclibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |rhouk@ottumwapubliclibrary. | |org --- Comment #1 from Ron Houk <rhouk@ottumwapubliclibrary.org> --- I have run into issues with reserves_stats.pl mentioned by Andrew where no bib level holds were showing up, and I didn't understand why. I think it might be nice to make the defaults be so that all holds show up and then allow users to filter later on. It might also be nice to have an explanation of the different statuses in the documentation. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|18.11 |master CC| |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=33792 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 --- Comment #2 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- When you do not confirm a hold, Koha stored it without itemnumber in old holds when directly checking out to same patron. These filled holds are not taken into consideration in statistics. See further 33791/33792. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 --- Comment #3 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Andrew Fuerste-Henry from comment #0)
Two big issues: - By default, the wizard sets a value for items.homebranch and items.holdingbranch. That means it unexpectedly cuts out all bib-level holds and gives no clear indication that it's done so. You *can* change home and holding branch to not limit, but it's not intuitive.
I submitted a tiny fix for this problem on bug 33792 with a possible major impact on your results :) -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 --- Comment #4 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Andrew Fuerste-Henry from comment #0)
- The wizard splits holds into 5 statuses (Placed, Processed, Pending, Satisfied, and Cancelled) that aren't used anywhere else in Koha. These statuses aren't explained without diving into the code and don't match how we otherwise divide and talk about holds.
Yes this is weird code. Will try to improve that a bit. But it wont be the major overhaul you requested. But small steps may bring us further too. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|reserves_stats.pl needs |reserves_stats.pl: Simplify |overhaul |reserve status handling Assignee|koha-bugs@lists.koha-commun |m.de.rooy@rijksmuseum.nl |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 --- Comment #5 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 151579 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=151579&action=edit Bug 23059: reserve_stats.pl: Simplify reservestatus This patch does the following: [1] Go back to four statuses: Cancelled, Filled, Waiting or Placed. Placed is used as collective name for all other statuses: pending (placed), processing (found==P) or transit (found==T). Placed before anyway. [2] Allow translation of these statuses in the template. Remove the sub reservestatushuman. [3] The output of changeifreservestatus is considerably shorter and less ugly in the constructed sql statement. Test plan: [1] Use reservestatus as row, as column and only as filter (clicking few statuses). [2] Verify that the shown statistics meet your expectations. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- NOTE for interested ones: The former code - 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 ) made a lot of cancelled holds count under processing status 2 ! The only ones that were counted were those which were already waiting. This bug is resolved now too. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=33862 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Most cancelled holds look like: found, cancellationdate | NULL | 2022-05-16 | Only cancelled waits look like: found, cancellationdate | W | 2022-06-16 | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 152679 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152679&action=edit Bug 23059: (QA follow-up) Put status strings in a span No test plan. Read the patch. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #151579|0 |1 is obsolete| | Attachment #152679|0 |1 is obsolete| | --- Comment #9 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 152824 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152824&action=edit Bug 23059: reserve_stats.pl: Simplify reservestatus This patch does the following: [1] Go back to four statuses: Cancelled, Filled, Waiting or Placed. Placed is used as collective name for all other statuses: pending (placed), processing (found==P) or transit (found==T). Placed before anyway. [2] Allow translation of these statuses in the template. Remove the sub reservestatushuman. [3] The output of changeifreservestatus is considerably shorter and less ugly in the constructed sql statement. Test plan: [1] Use reservestatus as row, as column and only as filter (clicking few statuses). [2] Verify that the shown statistics meet your expectations. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 --- Comment #10 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 152825 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152825&action=edit Bug 23059: (QA follow-up) Put status strings in a span No test plan. Read the patch. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Owen Leonard from comment #10)
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Thx ! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #12 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Why not using reservestatus for + <input type="checkbox" name="filter_reservestatus_or_C" value="C" /> Cancelled + <input type="checkbox" name="filter_reservestatus_or_F" value="F" /> Filled + <input type="checkbox" name="filter_reservestatus_or_P" value="P" /> Placed + <input type="checkbox" name="filter_reservestatus_or_W" value="W" /> Waiting ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #13 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #12) Thanks for QA.
Why not using reservestatus for
+ <input type="checkbox" name="filter_reservestatus_or_C" value="C" /> Cancelled + <input type="checkbox" name="filter_reservestatus_or_F" value="F" /> Filled + <input type="checkbox" name="filter_reservestatus_or_P" value="P" /> Placed + <input type="checkbox" name="filter_reservestatus_or_W" value="W" /> Waiting
?
For several reasons: [1] Completely improving this part of code is not in scope. Look at the changes here. They improve and resolve a bug. Further work can be done, but is not required here. [2] The reservestatus refers to the routine in C4/Reserves. Which has some interesting comments: ## FIXME: I don't think this does what it thinks it does. ## It only ever checks the first reserve result, even though ## multiple reserves for that bib can have the itemnumber set ## the sub is only used once in the codebase. It does not know Cancelled or Filled which are important in reporting here. So in short, that routine needs attention. Cannot be forced here. [3] This script needs a solution in SQL not in Perl. We cannot call Perl for each row in the query and impact performance negatively. Implementing this needs much more changes than the current scope requires. Please have another look. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152824|0 |1 is obsolete| | --- Comment #14 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 156548 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=156548&action=edit Bug 23059: reserve_stats.pl: Simplify reservestatus This patch does the following: [1] Go back to four statuses: Cancelled, Filled, Waiting or Placed. Placed is used as collective name for all other statuses: pending (placed), processing (found==P) or transit (found==T). Placed before anyway. [2] Allow translation of these statuses in the template. Remove the sub reservestatushuman. [3] The output of changeifreservestatus is considerably shorter and less ugly in the constructed sql statement. Test plan: [1] Use reservestatus as row, as column and only as filter (clicking few statuses). [2] Verify that the shown statistics meet your expectations. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152825|0 |1 is obsolete| | --- Comment #15 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 156549 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=156549&action=edit Bug 23059: (QA follow-up) Put status strings in a span No test plan. Read the patch. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA QA Contact|testopia@bugs.koha-communit |emily.lamancusa@montgomeryc |y.org |ountymd.gov CC| |emily.lamancusa@montgomeryc | |ountymd.gov --- Comment #16 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- This is a nice improvement to clarity and consistency. Works as intended and QA tool is happy. One thing that would be nice to see as a follow-up would be the ability for libraries that have in-processing and in-transit statuses to be able to separate those out from "Placed". But that requires extra processing to determine whether those statuses are relevant to the library, so it's a bit out of scope for this bug, and doesn't need to block the alignment of the report with the universally-relevant hold statuses. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 --- Comment #17 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Emily Lamancusa from comment #15)
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Thx Emily ! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 --- Comment #18 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Emily Lamancusa from comment #16)
This is a nice improvement to clarity and consistency. Works as intended and QA tool is happy.
One thing that would be nice to see as a follow-up would be the ability for libraries that have in-processing and in-transit statuses to be able to separate those out from "Placed". But that requires extra processing to determine whether those statuses are relevant to the library, so it's a bit out of scope for this bug, and doesn't need to block the alignment of the report with the universally-relevant hold statuses.
Opened bug 34988 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |34988 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34988 [Bug 34988] reserve_stats.pl: Further improvements to refine hold status -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |23.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 --- Comment #19 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23059 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Resolution|--- |FIXED Status|Pushed to master |RESOLVED --- Comment #20 from Fridolin Somers <fridolin.somers@biblibre.com> --- Enhancement not pushed to 23.05.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org