[Bug 16187] New: Add a script to cancel unfilled holds after a specified number of days
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Bug ID: 16187 Summary: Add a script to cancel unfilled holds after a specified number of days Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Command-line Utilities Assignee: gmcharlt@gmail.com Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: robin@catalyst.net.nz Work to be done: 1) Create new script cancel_unfilled_holds.pl --days DAYS 2) This script would accept a --days parameter, and trigger C4::Reserves::CancelReserve for all unfilled holds with a 'reservedate' older than the the given number of days. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Change sponsored?|--- |Sponsored Assignee|gmcharlt@gmail.com |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=16187 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 53397 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53397&action=edit Bug 16187 - Add a script to cancel unfilled holds after a specified number of days This script takes parameters: days - how many days waiting to concal an unfilled hold on or after library - (repeatable) branches to consider holidays - whether or not to count holidays (default is no) To test: 1 - Place some holds with varying reservedates 2 - Run script with different parameters to verify options are respected (-v for verbosity will assist here) 3 - verify that script does nothing without days parameter Sponsored by: Siskiyou County Library (http://www.siskiyoulibrary.info/) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |amy@bywatersolutions.com, | |nicole@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Nicolas Legrand <nicolas.legrand@bulac.fr> 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=16187 Nicolas Legrand <nicolas.legrand@bulac.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #53397|0 |1 is obsolete| | --- Comment #2 from Nicolas Legrand <nicolas.legrand@bulac.fr> --- Created attachment 56275 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56275&action=edit Bug 16187 - Add a script to cancel unfilled holds after a specified number of days This script takes parameters: days - how many days waiting to concal an unfilled hold on or after library - (repeatable) branches to consider holidays - whether or not to count holidays (default is no) To test: 1 - Place some holds with varying reservedates 2 - Run script with different parameters to verify options are respected (-v for verbosity will assist here) 3 - verify that script does nothing without days parameter Sponsored by: Siskiyou County Library (http://www.siskiyoulibrary.info/) Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Nick, what's the use case for this? How is it different to cancel_expired_holds.pl? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #4 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- We are missing a 'feedback' status, so setting to 'failed qa' instead. Don't take it personally :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #5 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Katrin Fischer from comment #3)
Hi Nick, what's the use case for this? How is it different to cancel_expired_holds.pl?
Cancel expired holds is for holds that have been found and are waiting and expired. This script cancels holds that have never been found, so it can clean up holds placed on items that were lost or damaged and will never be filled. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Liz Rea <liz@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |liz@catalyst.net.nz --- Comment #6 from Liz Rea <liz@catalyst.net.nz> --- Hi, This script seems really dangerous, how will it know if it is cancelling a hold that is for something missing or lost, or one that someone legitimately placed on an item and it's just taking AGES to get through the (possibly very long) reserve list? I'd be concerned especially in libraries that routinely have very long waiting lists on popular titles. I feel like this needs some safeguards. Cheers, Liz -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org Status|Signed Off |Failed QA --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Nick, do you have any good reasons to use DateTime instead of dt_from_string to get today's date? Note that you could have use Koha::Libraries->search->get_column('branchcode') to get an iterator on the branchcodes. Please resubmit tidying the whole file. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 --- Comment #8 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 61173 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61173&action=edit Bug 16187 - (QA followup) Tidy, add confirm, minor changes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 --- Comment #9 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Liz Rea from comment #6)
I feel like this needs some safeguards.
Cheers, Liz
I added a 'confirm' option so this can be run to see what would be deleted before going through with the deletion, is there another specific feedback you would like to see? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 --- Comment #10 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #7)
Nick, do you have any good reasons to use DateTime instead of dt_from_string to get today's date? Nope, was a copy paste job
Note that you could have use Koha::Libraries->search->get_column('branchcode') to get an iterator on the branchcodes. Advice taken
Please resubmit tidying the whole file. Done.
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Should we send a notification if we cancel a hold? Can remember to have seen such a report, but cannot find it now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- You must create a new subroutine to Koha::Holds and cover it with tests. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Is "waiting" in the name of the script appropriate? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56275|0 |1 is obsolete| | Attachment #61173|0 |1 is obsolete| | --- Comment #14 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 61951 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61951&action=edit Bug 16187 - Add a script to cancel unfilled holds after a specified number of days This script takes parameters: days - how many days waiting to concal an unfilled hold on or after library - (repeatable) branches to consider holidays - whether or not to count holidays (default is no) This patchset adds two methods and covers them with tests: Koha::Holds->unfilled(); To return holds where found = undef Koha::Hold->age( $use_calendar ); To return the number of days since a hold was placed (including or excluding holidays) To test: 1 - Place some holds with varying reservedates 2 - Run script with different parameters to verify options are respected (-v for verbosity will assist here) 3 - verify that script does nothing without days parameter Sponsored by: Siskiyou County Library (http://www.siskiyoulibrary.info/) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Marcel de Rooy from comment #11)
Should we send a notification if we cancel a hold? Can remember to have seen such a report, but cannot find it now.
There is bug 12656, but no patch yet. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #61951|0 |1 is obsolete| | --- Comment #16 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 64924 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=64924&action=edit Bug 16187 - Add a script to cancel unfilled holds after a specified number of days This script takes parameters: days - how many days waiting to concal an unfilled hold on or after library - (repeatable) branches to consider holidays - whether or not to count holidays (default is no) This patchset adds two methods and covers them with tests: Koha::Holds->unfilled(); To return holds where found = undef Koha::Hold->age( $use_calendar ); To return the number of days since a hold was placed (including or excluding holidays) To test: 1 - Place some holds with varying reservedates 2 - Run script with different parameters to verify options are respected (-v for verbosity will assist here) 3 - verify that script does nothing without days parameter Sponsored by: Siskiyou County Library (http://www.siskiyoulibrary.info/) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #17 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #13)
Is "waiting" in the name of the script appropriate?
I agree that waiting in the name is confusing. They should not be waiting. Unfilled as mentioned in the first comment would be better. Nick: Please change the filename. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #18 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Hmm Seems OK already -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #19 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- No use for this change? sub waiting { my ( $self ) = @_; - return $self->search( { found => 'W' } ); + return $self->search( { found => 'w' } ); I would prefer to keep the uppercase W here for finding it back. +=head1 NAME + +cancel_waiting_holds.pl + +=head1 SYNOPSIS + +cancel_waiting_holds.pl Old name remains. + -days cancel holds waiting this many days +Specify the number of days waiting upon which to cancel holds. The term waiting may still be confusing here. You mean holds that are so many days old or placed so many days ago. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #64924|0 |1 is obsolete| | --- Comment #20 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 68289 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=68289&action=edit Bug 16187 - Add a script to cancel unfilled holds after a specified number of days This script takes parameters: days - how many days waiting to concal an unfilled hold on or after library - (repeatable) branches to consider holidays - whether or not to count holidays (default is no) This patchset adds two methods and covers them with tests: Koha::Holds->unfilled(); To return holds where found = undef Koha::Hold->age( $use_calendar ); To return the number of days since a hold was placed (including or excluding holidays) To test: 1 - Place some holds with varying reservedates 2 - Run script with different parameters to verify options are respected (-v for verbosity will assist here) 3 - verify that script does nothing without days parameter Sponsored by: Siskiyou County Library (http://www.siskiyoulibrary.info/) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 --- Comment #21 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 68290 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=68290&action=edit Bug 16187 - Followup 1 - Correct use of original (bad) script name 2 - Explain options better 3 - Remove change from 'W' to 'w' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #22 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- How was this tested ? Can't locate object method "next" via package "13" (perhaps you forgot to load "13"?) at misc/cronjobs/holds/cancel_unfilled_holds.pl line 119. Undefined subroutine &main::CancelReserve called at misc/cronjobs/holds/cancel_unfilled_holds.pl line 143. Will submit a fix -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 --- Comment #23 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #22)
How was this tested ?
Undefined subroutine &main::CancelReserve called at misc/cronjobs/holds/cancel_unfilled_holds.pl line 143.
CancelReserve was moved to Koha::Hold of course.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #68289|0 |1 is obsolete| | --- Comment #24 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 68309 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=68309&action=edit Bug 16187 - Add a script to cancel unfilled holds after a specified number of days This script takes parameters: days - how many days waiting to concal an unfilled hold on or after library - (repeatable) branches to consider holidays - whether or not to count holidays (default is no) This patchset adds two methods and covers them with tests: Koha::Holds->unfilled(); To return holds where found = undef Koha::Hold->age( $use_calendar ); To return the number of days since a hold was placed (including or excluding holidays) To test: 1 - Place some holds with varying reservedates 2 - Run script with different parameters to verify options are respected (-v for verbosity will assist here) 3 - verify that script does nothing without days parameter Sponsored by: Siskiyou County Library (http://www.siskiyoulibrary.info/) 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=16187 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #68290|0 |1 is obsolete| | --- Comment #25 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 68310 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=68310&action=edit Bug 16187 - Followup 1 - Correct use of original (bad) script name 2 - Explain options better 3 - Remove change from 'W' to 'w' 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=16187 --- Comment #26 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 68311 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=68311&action=edit Bug 16187: (QA follow-up) Additional fixes Fix: Can't locate object method "next" via package "13" (perhaps you forgot to load "13"?) at misc/cronjobs/holds/cancel_unfilled_holds.pl line 119. Undefined subroutine &main::CancelReserve called at misc/cronjobs/holds/cancel_unfilled_holds.pl line 143. The script does not use Koha::Object's get_column correctly for getting the branch codes. The call to CancelReserve is obsolete. Was moved in the meantime to Koha::Hold->cancel. 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=16187 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | 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=16187 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16187 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #27 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 17.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org