Suspension of holds Users should be able to place a hold 'on suspension'. This means the hold will preserve it's place in the queue, but it will be skipped over to the next non-suspended hold when the item becomes available. Upon un-suspension, the hold will resume it's original place in the queue. There shall be a button to suspend all holds for a user at once, both in the staff client and the OPAC (if OPAC holds management is enabled) Auto unsuspend holds by date For users who know the date when they will be able to receive holds again, there shall be an available box to specify this date at suspension time. When that date rolls around, the hold will be automatically resumed. This box shall also be available when suspending multiple holds at once. This box shall be available in the staff client, and in the OPAC (if OPAC holds management is enabled). The use of this features shall be toggleable by a system preference.
Created attachment 7991 [details] [review] Suspend Reserves
Created attachment 7992 [details] [review] [SIGNED-OFF] Bug 7641: Suspend Reserves Tested with the preference on and off: 1. placed several holds in the staff client 2. suspended some with a date 3. suspended some without a date 4. triggered hold message by checking in for hold with suspensions 5. the suspended hold was skipped as it should be 6. tested suspending holds in the OPAC w and w/out dates 7. ran the cron to clear suspensions with dates All the above tests worked as expected. Signing off.
I forgot to mention that I did test it from the patron record, from the bib record, and from the opac
This patch is marked as blocked by 7711, that has a "NEW" status. Does it mean it should not be pushed before something is submitted for 7711 ? Why has it been signed-off in this case ? I'm OK to QA it, but I need some enlightment ;-)
I think this is backwards. I think 7711 is blocked by 7641, rather than the other way around. I'll fix that. (In reply to comment #4) > This patch is marked as blocked by 7711, that has a "NEW" status. Does it > mean it should not be pushed before something is submitted for 7711 ? Why > has it been signed-off in this case ? > > I'm OK to QA it, but I need some enlightment ;-)
tests comments: * I tested many cases, could not find any problem * a possible enhancement could be for a patron to suspend holds one by one from the opac-user page. The common use-case is a patron that is unavailable for a long trip, another is "i placed a hold on volume 1 and 2, but want to suspend volume 2 until I have volume 1 in my hands". but that could be a future ENH QA comments: * There are 2 unconditional warnings in the logs: [Thu Mar 29 14:46:24 2012] [error] [client 127.0.0.1] [Thu Mar 29 14:46:24 2012] opac-modrequest-suspend.pl: C4::Reserves::SuspendAll( borrowernumber => 7852, biblionumber => , suspend_until => , suspend => 0 ) at /home/paul/koha.dev/koha-community/C4/Reserves.pm line 1562., referer: http://lecannet/cgi-bin/koha/opac-user.pl I've removed them in a follow-up * not worth failing QA because the rule is still at early stage, but a follow-up is requested for : + my $query = "UPDATE reserves SET suspend = 0, suspend_until = NULL WHERE DATE( suspend_until ) < DATE( CURDATE() )"; CURDATE is a mysqlism (see http://wiki.koha-community.org/wiki/PostgreSQL => CAST(now as date) * MARC BALMER (our mysqlism specialist :-) ) putting you in the loop: this patch contains + "UPDATE reserves SET suspend = NOT suspend, + suspend_until = CASE WHEN suspend = 0 THEN NULL ELSE suspend_until END => is it a mysqlism or no ? If yes, add it to your wiki page * prove translatable_templates and valid_templates are OK * misc/cronjobs/holds/auto_unsuspend_holds.pl had use strict/use warning commented, I've uncommented them in a follow-up, works fine * opac/opac-modrequest-suspend.pl and reserve/modrequest_suspendall.pl passes perlcritic * the + <td>[% IF ( reservloo.suspend ) %]Suspended [% IF ( reservloo.suspend_until ) %] until [% reservloo.suspend_until %][% END %][% END %]</td> should use kohaDates, not + $getreserv{suspend_until} = C4::Dates->new( $num_res->{'suspend_until'}, "iso")->output("syspref");; I'll pass QA & push because everything is OK except thig kohaDates display, but please provide a follow-up ASAP
PostgreSQL has the CASE statement as well. I will check if it is compatible with the standard and MySQL. For now, I say treat CASE as non mysqlism, I took a note to recheck this file in case it is one
patch pushed. (In reply to comment #7) > PostgreSQL has the CASE statement as well. I will check if it is compatible > with the standard and MySQL. For now, I say treat CASE as non mysqlism, I > took a note to recheck this file in case it is one thx marc for the very quick reply !
Created attachment 8695 [details] [review] Bug 7641 - Add ability to suspend reserves - Followup - Switch from C4::Dates to $KohaDates
Created attachment 8697 [details] [review] Bug 7641 - Add ability to suspend reserves - Followup - Switch from C4::Dates to $KohaDates
Created attachment 8698 [details] [review] Bug 7641 - Add ability to suspend reserves - Followup - Switch from C4::Dates to $KohaDates
Created attachment 8795 [details] screenshot I am not so happy with the display here, especially the last button: 'Resume All Suspended All Requests' Also the new buttons don't follow the new capitalization rules :(
Created attachment 8881 [details] [review] Bug 7641 - Add ability to suspend reserves - Followup * Switch from C4::Dates to $KohaDates * Fix 'Resume All Suspended All Requests' * Change button text to follow capitalization rules * Change 'requests' to 'holds'
(In reply to comment #12) Wow, how did that slip by! Also, I was just following the capitalization of the existing button. The new followup addresses these issues. > Created attachment 8795 [details] > screenshot > > I am not so happy with the display here, especially the last button: > 'Resume All Suspended All Requests' > Also the new buttons don't follow the new capitalization rules :(
Followup is mostly template changes to wording, with an implementation of KohaDates. Marking as Signed Off and Passed QA in one go.
the follow-up has been pushed on master (not on new/bug_7641, for an unknown reason, I can't push it on this branch)
Have begun testing the suspend holds on our test Koha server. After creating and suspending a hold on 2 different patron records, can no longer access those patron records. The patrons affected by this so far are ADKINS, DORIS J (22003000177152) and STANCLIFF, JUNE (22003000000073). Instead we get this error message: Software error: Template process failed: undef error - : filter not found at /home/koha/kohaclone/C4/Templates.pm line 127. For help, please send mail to the webmaster (webmaster@kwala), giving this error message and the time and date of the error. June Stancliff
I am unable to recreate your error. Perhaps it involves the $KohaDates filter somehow?
Kyle - This happens after you try to "Suspend all holds" I was able to recreate with "suspend all"
(In reply to comment #0) > The use of this features shall be toggleable by a system preference. Looks like this is missing.
It should be possible to turn this feature off by a system preference like it is possible for other hold related features. I filed a separate bug - bug 7951.
The 500 error will need to be fixed before release
Should I be able to see the date until which the hold is suspended? If I suspend a hold from request.pl and specify a date I don't see it when request.pl reloads and I don't see it on the holds tab of moremember.pl or circulation.pl. If I "suspend all" with a date from moremember.pl I do see a date in the form field on request.pl, so I assume this is the correct behavior. I think this feature could have used some more testing.
(In reply to comment #22) > The 500 error will need to be fixed before release The problem appears to be that the KohaDates plugin is not being included in moremember.pl and circulation.pl. I tested this be removing the filter on the suspend_until date and adding it to a different date outside that code. I still got the error. Chris, it appears that you wrote the KohaDates plugin. Do you have any idea why the plugin wouldn't be included?
Created attachment 9178 [details] [review] Bug 7641 - Followup - Suspend Until not set on by suspend button. For request.pl, there are two ways to suspend a reserve, either by using the 'suspend' button for an individual reserve, or by using the 'Update hold(s)' button with suspend until dates set. If the 'suspend' button is used, any date in the 'suspend until' field is ignored. This commit fixes this issue. * Add suspend_until date to suspend button link via jquery * Add optional date to ToggleSuspend() * Add KohaDates plugin where necessary
Created attachment 9183 [details] [review] Bug 7641 - Followup - Suspend Until not set on by suspend button. For request.pl, there are two ways to suspend a reserve, either by using the 'suspend' button for an individual reserve, or by using the 'Update hold(s)' button with suspend until dates set. If the 'suspend' button is used, any date in the 'suspend until' field is ignored. This commit fixes this issue. * Add suspend_until date to suspend button link via jquery * Add optional date to ToggleSuspend() * Add KohaDates plugin where necessary Signed-off-by: Liz Rea <wizzyrea@gmail.com> passes tests, tested: * suspend all holds from circ.pl * suspend one hold from circ.pl * suspend all holds from moremember.pl * suspend one hold from moremember.pl --- NOTE: clicking suspend all holds without setting a date will mean the holds must be manually unsuspended. I'm not sure this is intentional? * suspend a specific hold using the in-table button on reserves * suspend a specific hold using the "update hold" button 500 error is gone.
Also tested that the cronjob unsuspends the holds when the suspension date has passed.
*** Bug 7946 has been marked as a duplicate of this bug. ***
Adds unconditional warn to ModReserve. ToggleSuspend uses C4::Dates instead of Koha::Calendar and DateTime; we're moving away from C4::Dates, so best not to introduce new usage. Marking Failed QA until these issues are resolved.
Created attachment 9225 [details] [review] Bug 7641 - Followup - Suspend Until not set on by suspend button. For request.pl, there are two ways to suspend a reserve, either by using the 'suspend' button for an individual reserve, or by using the 'Update hold(s)' button with suspend until dates set. If the 'suspend' button is used, any date in the 'suspend until' field is ignored. This commit fixes this issue. * Add suspend_until date to suspend button link via jquery * Add optional date to ToggleSuspend() * Add KohaDates plugin where necessary
--- NOTE: clicking suspend all holds without setting a date will mean the holds must be manually unsuspended. I'm not sure this is intentional? Yes, this is intentional. In fact, the ability to set a date can be turned off entirely, using the system preference AutoResumeSuspendedHolds.
Created attachment 9440 [details] [review] [SIGNED-OFF] Bug 7641 - Followup - Suspend Until not set on by suspend button. For request.pl, there are two ways to suspend a reserve, either by using the 'suspend' button for an individual reserve, or by using the 'Update hold(s)' button with suspend until dates set. If the 'suspend' button is used, any date in the 'suspend until' field is ignored. This commit fixes this issue. * Add suspend_until date to suspend button link via jquery * Add optional date to ToggleSuspend() * Add KohaDates plugin where necessary Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>
Created attachment 9444 [details] [review] Bug 7641 - Followup - Suspend Until not set on by suspend button. For request.pl, there are two ways to suspend a reserve, either by using the 'suspend' button for an individual reserve, or by using the 'Update hold(s)' button with suspend until dates set. If the 'suspend' button is used, any date in the 'suspend until' field is ignored. This commit fixes this issue. * Add suspend_until date to suspend button link via jquery * Add optional date to ToggleSuspend() * Add KohaDates plugin where necessary Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com> passes tests, tested: * suspend all holds from circ.pl * suspend one hold from circ.pl * suspend all holds from moremember.pl * suspend one hold from moremember.pl --- NOTE: clicking suspend all holds without setting a date will mean the holds must be manually unsuspended. I'm not sure this is intentional? * suspend a specific hold using the in-table button on reserves * suspend a specific hold using the "update hold" button 500 error is gone.
Adds more consideration for NULL suspend_untils, and formats them in ISO with Koha::DateUtils::output_pref. Modifies display layer to use KohaDates, including in passed value in form. Looks like all the necessary input points are properly scrubbed for this formatting, so it should be fine. Marking Passed QA.
The problem with follow-up that are submitted after a feature has been pushed, is that: * the bugzilla entry becomes looonnggg and very hard to read and understand. * I can't push this follow-up as usual. Here is what i usually do: * git checkout new/bug_7641 * git bz apply 7641 * testing * pushing on the branch * git checkout master * git merge the branch into master Unfortunately, for this follow-up, I can't do that, because some other changes in the code make it valid only for branch master. So I could only push it directly on master, which would be confusing a lot. That's why I'd preffered, from far, a separate bug entry, with it's own number. Could you please file a new entry, with a clean description, and the patch. You can also link the new bug to this one and switch this one to pushed to master and the follow-up passed QA. thanks
There have been no further reports of problems so I am marking this bug resolved.