Summary: | Cancel expired holds cronjob does not cancel holds in transit if ExpireReservesMaxPickUpDelay not set | ||
---|---|---|---|
Product: | Koha | Reporter: | Nick Clemens (kidclamp) <nick> |
Component: | Command-line Utilities | Assignee: | Nick Clemens (kidclamp) <nick> |
Status: | CLOSED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | normal | ||
Priority: | P5 - low | CC: | alexbuckley, fridolin.somers, jonathan.druart, m.de.rooy, robin |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | Trivial patch | Documentation contact: | |
Documentation submission: | Text to go in the release notes: |
This patch corrects a behaviour where an in transit hold would not be cancelled if even the patron specified they did not need the hold after a certain date. In some cases they would receive a notice to pickup a hold they no longer wanted.
Now these holds will be cancelled while in transit, and should be routed to their home location when checked in with no notice to the patron.
|
|
Version(s) released in: |
19.11.00
|
Circulation function: | |
Attachments: |
Bug 21181: Make cancel_expired_reserves.pl Cancel all holds not waiting if ExpireReservesMaxPickupDelay not set
Bug 21181: Make cancel_expired_reserves.pl Cancel all holds not waiting if ExpireReservesMaxPickupDelay not set Bug 21181: Unit tests Bug 21181: Make cancel_expired_reserves.pl Cancel all holds not waiting if ExpireReservesMaxPickupDelay not set [SIGNED-OFF] Bug 21181: Unit tests [SIGNED-OFF] Bug 21181: Make cancel_expired_reserves.pl Cancel all holds not waiting if ExpireReservesMaxPickupDelay not set Bug 21181: Unit tests Bug 21181: Make cancel_expired_reserves.pl Cancel all holds not waiting if ExpireReservesMaxPickupDelay not set |
Description
Nick Clemens (kidclamp)
2018-08-08 17:18:51 UTC
Created attachment 77582 [details] [review] Bug 21181: Make cancel_expired_reserves.pl Cancel all holds not waiting if ExpireReservesMaxPickupDelay not set To test: 0 - Make sure ExpireReservesMaxPickupDelay is set to 'Don't allow' 1 - Place a hold for a patron 2 - Check in at another library to set it in transit 3 - Update expirationdate of hold to be in the past UPDATE reserves SET expirationdate='1999-12-31' 4 - Run cancel_expired_holds.pl 5 - Hold is not canceled 6 - Apply patch 7 - Run again 8 - Hold is cancelled Created attachment 81049 [details] [review] Bug 21181: Make cancel_expired_reserves.pl Cancel all holds not waiting if ExpireReservesMaxPickupDelay not set To test: 0 - Make sure ExpireReservesMaxPickupDelay is set to 'Don't allow' 1 - Place a hold for a patron 2 - Check in at another library to set it in transit 3 - Update expirationdate of hold to be in the past UPDATE reserves SET expirationdate='1999-12-31' 4 - Run cancel_expired_holds.pl 5 - Hold is not canceled 6 - Apply patch 7 - Run again 8 - Hold is cancelled Followed test plan and patch works as described, also passes QA test tool Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> It seems that you forgot to attach the tests you wrote before this patch. Created attachment 88583 [details] [review] Bug 21181: Unit tests Created attachment 88584 [details] [review] Bug 21181: Make cancel_expired_reserves.pl Cancel all holds not waiting if ExpireReservesMaxPickupDelay not set To test: 0 - Make sure ExpireReservesMaxPickupDelay is set to 'Don't allow' 1 - Place a hold for a patron 2 - Check in at another library to set it in transit 3 - Update expirationdate of hold to be in the past UPDATE reserves SET expirationdate='1999-12-31' 4 - Run cancel_expired_holds.pl 5 - Hold is not canceled 6 - Apply patch 7 - Run again 8 - Hold is cancelled Created attachment 90489 [details] [review] [SIGNED-OFF] Bug 21181: Unit tests Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de> Created attachment 90490 [details] [review] [SIGNED-OFF] Bug 21181: Make cancel_expired_reserves.pl Cancel all holds not waiting if ExpireReservesMaxPickupDelay not set To test: 0 - Make sure ExpireReservesMaxPickupDelay is set to 'Don't allow' 1 - Place a hold for a patron 2 - Check in at another library to set it in transit 3 - Update expirationdate of hold to be in the past UPDATE reserves SET expirationdate='1999-12-31' 4 - Run cancel_expired_holds.pl 5 - Hold is not canceled 6 - Apply patch 7 - Run again 8 - Hold is cancelled Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de> (In reply to Nick Clemens from comment #0) > Debatable, but that problem as it arose: Should we ask the list then? QAing (In reply to Jonathan Druart from comment #8) > (In reply to Nick Clemens from comment #0) > > Debatable, but that problem as it arose: > > Should we ask the list then? Yeah, might be safer way to go. +1 (little late for a comment on the list) List comments were positive, I also taked with our trainers. They agreed, if a patron has said they don't want a book after a certain date, then they don't want it after that date. In terms of patron experience, they don't know the book is in-transit, so would be a mystery why they sometimes get a notice for a hold they said they didn't want. Back to Signed Off Created attachment 91313 [details] [review] Bug 21181: Unit tests Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 91314 [details] [review] Bug 21181: Make cancel_expired_reserves.pl Cancel all holds not waiting if ExpireReservesMaxPickupDelay not set To test: 0 - Make sure ExpireReservesMaxPickupDelay is set to 'Don't allow' 1 - Place a hold for a patron 2 - Check in at another library to set it in transit 3 - Update expirationdate of hold to be in the past UPDATE reserves SET expirationdate='1999-12-31' 4 - Run cancel_expired_holds.pl 5 - Hold is not canceled 6 - Apply patch 7 - Run again 8 - Hold is cancelled Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Nice work! Pushed to master for 19.11.00 This is a small behavior change but I think we should avoid backport to stable. This should have a text to go in the release notes no ? (In reply to Fridolin SOMERS from comment #16) > This should have a text to go in the release notes no ? Thanks a lot Nick ;) |