[Bug 7408] New: Expire holds that have been waiting too long
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Bug #: 7408 Summary: Expire holds that have been waiting too long Classification: Unclassified Change sponsored?: --- Product: Koha Version: master Platform: All OS/Version: All Status: NEW Severity: enhancement Priority: P5 - low Component: Hold requests AssignedTo: ian.walls@bywatersolutions.com ReportedBy: ian.walls@bywatersolutions.com QAContact: ian.walls@bywatersolutions.com CC: gmcharlt@gmail.com For holds that have been waiting for pickup longer than the ReservesMaxPickupDelay number of days, there should be an automated way to mark these holds as "expired". It would also be nice (well, for the library, perhaps) to have the option to charge a fee for the patron not picking up the material. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Nicole C. Engard <nengard@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nengard@gmail.com --- Comment #1 from Nicole C. Engard <nengard@gmail.com> 2012-01-05 22:48:14 UTC --- Doesn't this script do most of that (except the charging part): misc/cronjobs/holds/cancel_expired_holds.pl -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 --- Comment #2 from Ian Walls <ian.walls@bywatersolutions.com> 2012-01-05 23:42:53 UTC --- Nicole, No, cancel_expired_holds.pl moves holds from being expired to being cancelled, and thus moved to the old_reserves table. Not every waiting hold has an expiration date; it's not filled in by the holds fill process. The patron or librarian has to set it manually. What's needed is an automated way of adding these expiration dates based on the ReservesMaxPickupDelay value, so that cancel_expired_holds.pl can capture and cancel them. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 --- Comment #3 from Ian Walls <koha.sekjal@gmail.com> 2012-01-27 22:07:16 UTC --- Created attachment 7357 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=7357 Proposed Patch ENH 7408: Expire holds that have been waiting too long This script updates all holds that have been waiting for pickup longer than ReservesMaxPickupDelay days, and sets the expiration date to today. The script can also take an optional parameter --charge that will fine the patron the amount specified. The fine will appear as type 'F', with the description "Hold waiting too long", and a link back to the item that was used to fill the hold. This script is intended to be run right before cronjobs/holds/cancel_expired_holds.pl If that script is not run, the expiration date will be updated again when this script runs, and the patron will be charged again (if --charge is set). To Test: 1. run perl misc/cronjobs/holds/expire_holds_over.pl -v --charge 1.25 you should get a message confirming how much you're charging, and how many holds were updated 2. View the Holds Awaiting Pickup page, and the Holds Over tab For each hold over, clicking to the title's Holds page should show the hold now has an expiration date of today. 3. Clicking over to the patron should show a fine of 1.25, with a link back to the item that had be waiting for them 4. Running cancel_expired_holds.pl then this script again should result in 0 holds modified -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Ian Walls <koha.sekjal@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff QAContact|koha.sekjal@gmail.com |m.de.rooy@rijksmuseum.nl -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 --- Comment #4 from Owen Leonard <oleonard@myacpl.org> 2012-02-07 19:22:30 UTC --- I've tested this successfully but have one question about how it works: After running the script, the holds get an expiration date of today. Running cronjobs/holds/cancel_expired_holds.pl immediately afterwards will cancel expired holds, but only those with an expiration date *less* than today. So none of the holds expired by this new script will be cancelled. If I then run the new script again with the --charge option the user will get double fines. I know this isn't they way the script would typically be run, but it's worrisome to me. On another note, testing this makes me wonder: Why don't we simply set an expiration date at the time the hold is marked waiting, based on the ReservesMaxPickupDelay setting? -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Kyle M Hall <kyle.m.hall@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #7357|0 |1 is obsolete| | --- Comment #5 from Kyle M Hall <kyle.m.hall@gmail.com> 2012-02-23 17:12:27 UTC --- Created attachment 7838 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=7838 Bug 7408 - Expire holds that have been waiting too long Sets the expiration date at the time the hold is marked waiting, based on the ReservesMaxPickupDelay syspref, assuming the new system preference ExpireReservesMaxPickUpDelay is enabled. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Kyle M Hall <kyle.m.hall@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle.m.hall@gmail.com --- Comment #6 from Kyle M Hall <kyle.m.hall@gmail.com> 2012-02-23 17:13:09 UTC --- (In reply to comment #4) That does seem like a simpler solution, and the fewer cronjobs, the better. Patch submitted.
I've tested this successfully but have one question about how it works: After running the script, the holds get an expiration date of today. Running cronjobs/holds/cancel_expired_holds.pl immediately afterwards will cancel expired holds, but only those with an expiration date *less* than today. So none of the holds expired by this new script will be cancelled. If I then run the new script again with the --charge option the user will get double fines.
I know this isn't they way the script would typically be run, but it's worrisome to me.
On another note, testing this makes me wonder: Why don't we simply set an expiration date at the time the hold is marked waiting, based on the ReservesMaxPickupDelay setting?
-- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Ian Walls <koha.sekjal@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |In Discussion --- Comment #7 from Ian Walls <koha.sekjal@gmail.com> 2012-02-23 17:27:59 UTC --- Kyle, Your patch does not include a provision for charging the patron for not picking up the hold, like the cronjob-based patch did, so it doesn't seem like a sufficient replacement (but, of course, I'm biased here). Reimplementing this to include not only a change to a C4 subroutine, but also a new syspref and consequential DB rev, seems to me to be a less worthwhile tradeoff. I open this for discussion, in the hopes others will be able to provide a more balanced opinion than my own. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Kyle M Hall <kyle.m.hall@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #7838|0 |1 is obsolete| | --- Comment #8 from Kyle M Hall <kyle.m.hall@gmail.com> 2012-02-23 18:26:09 UTC --- Created attachment 7839 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=7839 Bug 7408 - Expire holds that have been waiting too long Sets the expiration date at the time the hold is marked waiting, based on the ReservesMaxPickupDelay syspref, assuming the new system preference ExpireReservesMaxPickUpDelay is enabled. If ExpireReservesMaxPickUpDelayCharge is set, a borrower who allows a waiting hold to expire will be charged the fee set therein. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Kyle M Hall <kyle.m.hall@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #7839|0 |1 is obsolete| | --- Comment #9 from Kyle M Hall <kyle.m.hall@gmail.com> 2012-02-23 18:27:11 UTC --- Created attachment 7840 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=7840 Bug 7408 - Expire holds that have been waiting too long Sets the expiration date at the time the hold is marked waiting, based on the ReservesMaxPickupDelay syspref, assuming the new system preference ExpireReservesMaxPickUpDelay is enabled. If ExpireReservesMaxPickUpDelayCharge is set, a borrower who allows a waiting hold to expire will be charged the fee set therein. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 --- Comment #10 from Kyle M Hall <kyle.m.hall@gmail.com> 2012-02-23 18:37:00 UTC --- (In reply to comment #7) I have updated my patch to address that issue. The benefit of this version is there are no worries about cronjob conflicts. larryb also tested the cronjob version and felt that is was not working quite right. I don't know the specifics of what that meant though.
Kyle,
Your patch does not include a provision for charging the patron for not picking up the hold, like the cronjob-based patch did, so it doesn't seem like a sufficient replacement (but, of course, I'm biased here). Reimplementing this to include not only a change to a C4 subroutine, but also a new syspref and consequential DB rev, seems to me to be a less worthwhile tradeoff.
I open this for discussion, in the hopes others will be able to provide a more balanced opinion than my own.
-- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 --- Comment #11 from Kyle M Hall <kyle.m.hall@gmail.com> 2012-02-23 18:41:14 UTC --- (In reply to comment #7)
Reimplementing this to include not only a change to a C4 subroutine, but also a > new syspref and consequential DB rev, seems to me to be a less worthwhile tradeoff.
My opinion is that the possibility of the two cronjobs stepping on each other warrants the syspref and db revision. But, of course, I'm biased as well. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chris@bigballofwax.co.nz --- Comment #12 from Chris Cormack <chris@bigballofwax.co.nz> 2012-02-23 18:55:27 UTC --- I'm not biased (well about this patch anyway) but this trend of reimplementing people's work without prior discussion has got to stop. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Kyle M Hall <kyle.m.hall@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #7840|0 |1 is obsolete| | --- Comment #13 from Kyle M Hall <kyle.m.hall@gmail.com> 2012-02-23 19:08:47 UTC --- Created attachment 7841 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=7841 Bug 7408 - Expire holds that have been waiting too long Modification to cancel_expired_holds.pl. Gives the option to have any waiting holds past ReservesMaxPickupDelay automatically canceled, with the option of charging a fee. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 --- Comment #14 from Kyle M Hall <kyle.m.hall@gmail.com> 2012-02-23 19:11:51 UTC --- Ian, you may find this more to your liking. By implementing the functionality of your script directly in cancel_expired_holds.pl, we get rid of the possibility of cronjob conflicts, and remove the necessity of new system prefs and db revs. So we gain the advantages of both and the disadvantages of none. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Kyle M Hall <kyle.m.hall@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #7840|1 |0 is obsolete| | -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Kyle M Hall <kyle.m.hall@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #7841|0 |1 is obsolete| | -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 --- Comment #15 from Kyle M Hall <kyle.m.hall@gmail.com> 2012-02-24 13:30:51 UTC --- I've decided to obsolete my patch that modifies the cancel expired reserves script and go back my original approach. Here's the case I will make. I feel that using a system preference is actually a benefit. By having a system preference, it exposes the functionality better. You can learn a lot about what Koha can do simply be looking through all the available system preferences. If the functionality is hidden away in a cronjob, it becomes much harder to new users to know that functionality even exists. In addition, this version has the potential to become much more extensible. We could eventually add the waiting holds fee to the circulation rules table such that each library can choose it's own fee, just like standard fines and rental charges. It almost seems to go against the conventions of Koha to have all fines and fees controlled at such a granular level, with a single exception. If you accept this reasoning, I would be more than happy to make it so. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 --- Comment #16 from Ian Walls <koha.sekjal@gmail.com> 2012-02-24 14:03:08 UTC --- I can agree that having a unified, integrated and systemic way of keeping track of when holds expire is a good one. This should be part of the Holds Rewrite project, as we'll be rethinking a great many aspects of the way we do Holds in Koha. I can also agree that it's better to allow a more granular approach to hold-not-picked-up fines than just the blanket charge. This part of the feature was added for one particular ByWater customer, who had a very simple use case. The idea was that if people were to start using the feature and demand more flexibility to it, it could later be expanded, but for just one library system, it wasn't worth developing more than necessary. The cronjob solved both the specific need of that library system, and the larger issue of holds expiry, without having to muck around with the already-bloated Circ Matrix. As for your (current) patch, there is a typo in the subroutine name (_GetExpireResevesSQL is missing an 'r'). The subroutine is an interesting approach to the problem; it's not done like that elsewhere in Koha, so I'm not sure how to feel about it. The patch also locks in the expiry date when the hold is marked as ready. If the Max Pickup Delay were to change while a hold is waiting, then the new expiry date would not be calculated, leading to incorrect cancellation and possibly incorrect fees. My cronjob-based patch uses the ReservesMaxPickupDelay that is current to when the expiry and possible fees are applied. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Ian Walls <koha.sekjal@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #7357|1 |0 is obsolete| | --- Comment #17 from Ian Walls <koha.sekjal@gmail.com> 2012-02-24 14:04:02 UTC --- Comment on attachment 7357 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=7357 Proposed Patch Unobsoleting, since the patch is a competing implementation, not an obsoleted version. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Kyle M Hall <kyle.m.hall@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #7841|1 |0 is obsolete| | -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Kyle M Hall <kyle.m.hall@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #7841|0 |1 is obsolete| | -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Kyle M Hall <kyle.m.hall@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #7840|0 |1 is obsolete| | --- Comment #18 from Kyle M Hall <kyle.m.hall@gmail.com> 2012-02-24 14:49:55 UTC --- Created attachment 7856 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=7856 Bug 7408 - Expire holds that have been waiting too long If the new syspref ExpireReservesMaxPickUpDelay is enabled, this will cancel holds that have been waiting for longer than the number of days specified in the syspref ReservesMaxPickupDelay. If ExpireReservesMaxPickUpDelayCharge is set, the borrower charged the fee set therein. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 --- Comment #19 from Kyle M Hall <kyle.m.hall@gmail.com> 2012-02-24 14:58:32 UTC --- (In reply to comment #16)
I can agree that having a unified, integrated and systemic way of keeping track of when holds expire is a good one. This should be part of the Holds Rewrite project, as we'll be rethinking a great many aspects of the way we do Holds in Koha.
Agreed.
I can also agree that it's better to allow a more granular approach to hold-not-picked-up fines than just the blanket charge. This part of the feature was added for one particular ByWater customer, who had a very simple use case. The idea was that if people were to start using the feature and demand more flexibility to it, it could later be expanded, but for just one library system, it wasn't worth developing more than necessary. The cronjob solved both the specific need of that library system, and the larger issue of holds expiry, without having to muck around with the already-bloated Circ Matrix.
I can understand that. I guess my instinct when it comes to solutions is to not focus on the specific problem, but to create an extensible system that happens to solve that problem. When the CCFLS asked me to create a system to track summer reading club membership, I created the Clubs & Services module instead. I'm hoping to get it into Koha proper soon.
As for your (current) patch, there is a typo in the subroutine name (_GetExpireResevesSQL is missing an 'r'). The subroutine is an interesting approach to the problem; it's not done like that elsewhere in Koha, so I'm not sure how to feel about it.
Good catch. I am really not a big fan of what I did there. The only reason I used a separate subroutine like that was in an attempt to follow the DRY principle. Since the sql was used in two subroutines, I didn't want a future developer to introduce a bug by changing it in one sub but not in another.
The patch also locks in the expiry date when the hold is marked as ready. If the Max Pickup Delay were to change while a hold is waiting, then the new expiry date would not be calculated, leading to incorrect cancellation and possibly incorrect fees. My cronjob-based patch uses the ReservesMaxPickupDelay that is current to when the expiry and possible fees are applied.
That is a really good point. The new version of the patch I just submitted resolves this issue. It doesn't muck about with expiration dates at all. Instead, when CancelExpiredReserves is called, it then looks for holds that have been waiting for too long and cancels them as well. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Brendan Gallagher <brendan@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |brendan@bywatersolutions.co | |m --- Comment #20 from Brendan Gallagher <brendan@bywatersolutions.com> 2012-02-24 18:31:05 UTC --- This is a little discouraging to see some of the comments in this thread. This doesn't make much sense to me "trend of reimplementing people's work without prior discussion has got to stop" If someone sees "in their opinion" a different way to work out code for a particular BUG - then why discourage them from working on it and submitting the code? Since there is competing code - why not just look at the two options and test them both? In this situation I do agree - less chance of cronjobs stepping on each others toes - is a win. Also for Koha - I don't think this is what one library wants but there are a lot of other libraries that want this sort of functionality to be included (I'm sure that I can find you plenty of libraries that are interested in the same functionality). Holds Rewrite has been going on for years - maybe it would be best to have code that is ready now - be implemented. Thanks, Brendan -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 --- Comment #21 from Chris Cormack <chris@bigballofwax.co.nz> 2012-02-24 19:17:57 UTC --- (In reply to comment #20)
This is a little discouraging to see some of the comments in this thread.
This doesn't make much sense to me "trend of reimplementing people's work without prior discussion has got to stop" If someone sees "in their opinion" a different way to work out code for a particular BUG - then why discourage them from working on it and submitting the code?
Since there is competing code - why not just look at the two options and test them both? In this situation I do agree - less chance of cronjobs stepping on each others toes - is a win. Also for Koha - I don't think this is what one library wants but there are a lot of other libraries that want this sort of functionality to be included (I'm sure that I can find you plenty of libraries that are interested in the same functionality). Holds Rewrite has been going on for years - maybe it would be best to have code that is ready now - be implemented.
Sure, by all means, But at least talk to the persons who patch you are marking obsolete with your one before then. This is not the first bug this has happened on. Competing patches are fine, and now as Ian has unobsoleted his one, this is what we have. Before we had someone replacing another persons patch with their own. The trend is not just this bug, its other ones too. Discuss first, talk to the person who spent hours writing the patch, don't just mark his/hers obsolete and replace with your own. Chris -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Larry Baerveldt <larry@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |larry@bywatersolutions.com --- Comment #22 from Larry Baerveldt <larry@bywatersolutions.com> --- So, there are two patches for this issue. Our site is still waiting on this. Can someone decide on which approach to use and finalize a patch? Ian, have you had a chance to look at Kyle's latest update to this? Thanks. Larry -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Ian Walls <koha.sekjal@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Signed Off --- Comment #23 from Ian Walls <koha.sekjal@gmail.com> --- I like Kyle's implementation. It puts the controls into system preferences, which exposes more of the functionality to the librarian user. It's also immune to the cronjob timing issue that my implementation had. One small nit-pick. In circulation.pref, the class for ExpireReservesMaxPickUpDelayCharge should be 'currency', not 'integer', like maxoutstanding. I'll fix this with my signoff. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 --- Comment #24 from Ian Walls <koha.sekjal@gmail.com> --- Created attachment 8022 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8022&action=edit [SIGNED OFF] Bug 7408 - Expire holds that have been waiting too long If the new syspref ExpireReservesMaxPickUpDelay is enabled, this will cancel holds that have been waiting for longer than the number of days specified in the syspref ReservesMaxPickupDelay. If ExpireReservesMaxPickUpDelayCharge is set, the borrower charged the fee set therein. Signed-off-by: Ian Walls <koha.sekjal@gmail.com> Altered circulation.pref to include currency class and [% local_currency %] param -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Ian Walls <koha.sekjal@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #7357|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Ian Walls <koha.sekjal@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #7856|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Ian Walls <koha.sekjal@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #25 from Ian Walls <koha.sekjal@gmail.com> --- I'm going to mark this as Passed QA, as it's no longer my implementation, and the changes are pretty easy to read and confirm. Paul, if you'd like a second QA on this, I'd certainly understand. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |paul.poulain@biblibre.com --- Comment #26 from Paul Poulain <paul.poulain@biblibre.com> --- (In reply to comment #25)
I'm going to mark this as Passed QA, as it's no longer my implementation, and the changes are pretty easy to read and confirm. Paul, if you'd like a second QA on this, I'd certainly understand.
Fortunatly I made a 2nd QA ;-) There was a trailing ` that made the updatedatabase fail ! Plus, I don't understand well how it works: what makes the expired reserve be automatically cancelled ? I have : * a hold that expired, and appears in /cgi-bin/koha/circ/waitingreserves.pl, on tab "1 hold over" * I switched the syspref ExpireReservesMaxPickUpDelay ON and OFF, no fines and thought that reaching the page would make the expiry automatically be made. But that's not that. Then I thought the button "cancel all" will appear/disappear, but that's not that also. So: what do I misunderstand ? (keeping passed QA status for now, I made the follow-up to remove the `) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 --- Comment #27 from Paul Poulain <paul.poulain@biblibre.com> --- grepped CancelExpiredReserves, I see it's only used in misc/cronjobs/holds/cancel_expired_holds.pl but this script was already here. what did it before the patch that the patch changes (except for the fine thing) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 --- Comment #28 from Kyle M Hall <kyle.m.hall@gmail.com> --- Before this patch is applied, CancelExpiredReserves only cancels reserves that haven't been completed by the expiration date. This really should not cancel reserves that are already waiting or in transit. After the patch, not only does it cancel reserves that have passed their expiration date, but will also optionally cancel waiting reserves that haven't been picked up in the time frame set by ReservesMaxPickUpDelay. This is controlled by the value of ExpireReservesMaxPickUpDelay. Also, if ExpireReservesMaxPickUpDelayCharge is set, a fee will be charged to the patron for not picking up a waiting reserve in the timeframe given. (In reply to comment #27)
grepped CancelExpiredReserves, I see it's only used in
misc/cronjobs/holds/cancel_expired_holds.pl
but this script was already here. what did it before the patch that the patch changes (except for the fine thing)
-- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 --- Comment #29 from Kyle M Hall <kyle.m.hall@gmail.com> --- Created attachment 8101 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8101&action=edit Bug 7408 - Don't cancel found reserves. At the moment, a reserve will be canceled if it has passed its expiration date, even if the item is waiting or in the process of being transferred. The situation could arise where someone has a hold filled, but it is canceled while in transit, or before the borrower can pick it up. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 --- Comment #30 from Kyle M Hall <kyle.m.hall@gmail.com> --- Please let me know if I should file a new bug for this second patch. It is trivially small, though. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #31 from Paul Poulain <paul.poulain@biblibre.com> --- OK, got it now, it took me 2 hours !!! EXPIRY DATE = the date the patron optionnally set when he place the reserve it's different from the waiting date + the ReservesMaxPickUpDelay !!! patch pushed ! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frederic@tamil.fr --- Comment #32 from Frédéric Demians <frederic@tamil.fr> --- The new syspref ExpireReservesMaxPickUpDelayCharge added in circulation.pref YAML file breaks the translator: sysprefs can't be translated at all. I send a patch to fix that. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #8022|0 |1 is obsolete| | --- Comment #33 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 8241 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8241&action=edit Fix syspref def preventing its translation Don't know how to say it has to be signed-off and pushed... -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #8101|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #8241|0 |1 is obsolete| | --- Comment #34 from Liz Rea <wizzyrea@gmail.com> --- Created attachment 8242 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8242&action=edit Bug 7408 Fix ExpireReservesMaxPickUpDelayCharge syspref definition Allow preferences translation Signed-off-by: Liz Rea <wizzyrea@gmail.com> This patch fixes another sysprefs problem - Syck parser (line 300, column -1): syntax error at /usr/lib/perl5/YAML/Syck.pm line 76, <$fh> line 1. when clicking local use. I could not verify that it fixed translations, but fixing the local use prefs is important too, so I'm signing off. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |ASSIGNED CC| |wizzyrea@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Ian Walls <koha.sekjal@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #35 from Ian Walls <koha.sekjal@gmail.com> --- Simple fix for two serious issues. Marking as Passed QA. (sorry about that, everyone!) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #36 from Paul Poulain <paul.poulain@biblibre.com> --- follow-up pushed -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|master |rel_3_8 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #8242|0 |1 is obsolete| | --- Comment #37 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 8323 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8323&action=edit Bug 7408 : Fixing a typo that was breaking the installer -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |ASSIGNED Severity|enhancement |blocker -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff --- Comment #38 from Chris Cormack <chris@bigballofwax.co.nz> --- There was a typo in an earlier patch breaking syspref.sql .. which meant a clean install fails. Upping severity to blocker. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Kristina Hoeppner <kristina@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #8323|0 |1 is obsolete| | --- Comment #39 from Kristina Hoeppner <kristina@catalyst.net.nz> --- Created attachment 8325 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8325&action=edit signed-off patch There was just a ` too many in one of the lines. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Kristina Hoeppner <kristina@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |kristina@catalyst.net.nz -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Juan Romay Sieira <juan.sieira@xercode.es> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |juan.sieira@xercode.es --- Comment #40 from Juan Romay Sieira <juan.sieira@xercode.es> --- I think instead of remove this type, all of this line must be removed, because it's duplicated. The syspref "ExpireReservesMaxPickUpDelayCharge" appears twice at sysprefs.sql file. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #41 from Paul Poulain <paul.poulain@biblibre.com> --- follow-up pushed (with a conflict solved, as other sysprefs had been added in the meantime). Tested that syspref.sql is loading, but please double check... -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=5786 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org